Message ID | 20230405-add-dsc-support-v1-1-6bc6f03ae735@quicinc.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:b0ea:0:b0:3b6:4342:cba0 with SMTP id b10csp1007819vqo; Tue, 2 May 2023 18:26:27 -0700 (PDT) X-Google-Smtp-Source: ACHHUZ6lmL/I25Y8qIq+/lRP6Tbo72Cxve2FPAWAnRn+4IoPl60y1WqhqhOBY7nG+p3CZBzWBx5f X-Received: by 2002:a05:6a20:8e0b:b0:f3:756e:e116 with SMTP id y11-20020a056a208e0b00b000f3756ee116mr25369755pzj.38.1683077187314; Tue, 02 May 2023 18:26:27 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1683077187; cv=none; d=google.com; s=arc-20160816; b=H820yv/17UVFXAY1lROQpADfimDObdmjmWWamCyAEuK7cGxF0LsJzO7CHYo73qAz0I 17N1uPuyMuSRrRipweUQfxLk+YKzw8n/+vccwt8pOVl+QGierymLRnqNUzorftSsxSii lXo+7ye7xLOnNEH4biSoU03Y1DP4j3oManb5QTplC6dsfoEQbf94Ztx8IXrTVmuDazbT tG4Xtv2gzcQhY3cit793E/qOKzuTpcfR3m7/QQ9O2jIxiUstEcfS8ZD53IczocUFBXPi 8T+YUYIhUeBEVprjGR/CR+ITq/Lx7wdoytP3ofFoNkZ3xcaJstmfogAsybuKJBa6J3Ux qPEg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:cc:to:in-reply-to:references:message-id :content-transfer-encoding:mime-version:subject:date:from :dkim-signature; bh=++vq3grPoz9MeaQo1ceDEHmqSkkTASiWLja7R0S6tGQ=; b=boVPsMADiE4r9zaAwGjeA+y6XwfP5heajQpoykMLn4aMMBKQ5qkv9LAu6b9svGDB8b PnapPUnLKfiGWJ9IlxjyJ6iA29MBB4UeYNMlWf+SgZ/xCoFyouDLYA8gAY8Qbv43E7G+ I4W+Sebo0H2AGfVpgoh6KzqqhMBjzBZ9aTxoJ7SU6dEbDnE6iIALKSlpwZ5OFgpgS2w5 KqeTpUDExBKAazUx3AxHKIEhIUexiznV9q2ymd8+YteuzpJq58IL0wFC5R1umOb5vBAg nF+StsXJMAHOSIHq0UUNhHTQ9mEA86rpUGm7saYXD93zMHRw0gZIWhGrx9hzbCwZ5XZo DSfQ== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@quicinc.com header.s=qcppdkim1 header.b=COX7K+Je; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=quicinc.com Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id p20-20020a63f454000000b0050fa04005e6si32086286pgk.412.2023.05.02.18.26.13; Tue, 02 May 2023 18:26:27 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@quicinc.com header.s=qcppdkim1 header.b=COX7K+Je; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=quicinc.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229586AbjECBTw (ORCPT <rfc822;heyuhang3455@gmail.com> + 99 others); Tue, 2 May 2023 21:19:52 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:56828 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229455AbjECBTu (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Tue, 2 May 2023 21:19:50 -0400 Received: from mx0a-0031df01.pphosted.com (mx0a-0031df01.pphosted.com [205.220.168.131]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id CD3A730CA; Tue, 2 May 2023 18:19:44 -0700 (PDT) Received: from pps.filterd (m0279864.ppops.net [127.0.0.1]) by mx0a-0031df01.pphosted.com (8.17.1.19/8.17.1.19) with ESMTP id 3430uCMi019769; Wed, 3 May 2023 01:19:37 GMT DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=quicinc.com; h=from : date : subject : mime-version : content-type : content-transfer-encoding : message-id : references : in-reply-to : to : cc; s=qcppdkim1; bh=++vq3grPoz9MeaQo1ceDEHmqSkkTASiWLja7R0S6tGQ=; b=COX7K+JeE/hUMDVyD10/idDK3zFRgEsF9UU956WEpzyB6N5QEDHg1GqyMKiohNIfctHg OP5f6LAJnQ7H4Sx8kkouYN5NERksGT0ARhDPj0sfPoGwxRHbEHgOSULtxJgxSMQLxJg7 VO40cszRIZphjEL66oF+f40cCP5MZoVbFXZKsLjsTZNaMS/FiFLSUm5gKOx6vOTpzWnB /ldbBeD2X4bmtaMfnsOqUdOcLWBMhej4Lq+faU3pFkriamU90Yk5zc8kFd82dJrRiIOT ucxigSaF///YmOlG/Le3hOjiYfIU5+lRjOERfoye011cnplBTzZhsU+fa8y+TakuiDHz Ng== Received: from nasanppmta04.qualcomm.com (i-global254.qualcomm.com [199.106.103.254]) by mx0a-0031df01.pphosted.com (PPS) with ESMTPS id 3qawama7fm-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Wed, 03 May 2023 01:19:37 +0000 Received: from nasanex01b.na.qualcomm.com (nasanex01b.na.qualcomm.com [10.46.141.250]) by NASANPPMTA04.qualcomm.com (8.17.1.5/8.17.1.5) with ESMTPS id 3431JaPu025044 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Wed, 3 May 2023 01:19:36 GMT Received: from jesszhan-linux.qualcomm.com (10.80.80.8) by nasanex01b.na.qualcomm.com (10.46.141.250) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.2.986.42; Tue, 2 May 2023 18:19:36 -0700 From: Jessica Zhang <quic_jesszhan@quicinc.com> Date: Tue, 2 May 2023 18:19:12 -0700 Subject: [PATCH 1/4] drm/msm/dsi: Adjust pclk rate for compression MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit Message-ID: <20230405-add-dsc-support-v1-1-6bc6f03ae735@quicinc.com> References: <20230405-add-dsc-support-v1-0-6bc6f03ae735@quicinc.com> In-Reply-To: <20230405-add-dsc-support-v1-0-6bc6f03ae735@quicinc.com> To: Rob Clark <robdclark@gmail.com>, Abhinav Kumar <quic_abhinavk@quicinc.com>, Dmitry Baryshkov <dmitry.baryshkov@linaro.org>, Sean Paul <sean@poorly.run>, David Airlie <airlied@gmail.com>, Daniel Vetter <daniel@ffwll.ch>, Marijn Suijten <marijn.suijten@somainline.org> CC: Konrad Dybcio <konrad.dybcio@linaro.org>, <linux-arm-msm@vger.kernel.org>, <dri-devel@lists.freedesktop.org>, <freedreno@lists.freedesktop.org>, <linux-kernel@vger.kernel.org>, Jessica Zhang <quic_jesszhan@quicinc.com> X-Mailer: b4 0.13-dev-bfdf5 X-Developer-Signature: v=1; a=ed25519-sha256; t=1683076775; l=2523; i=quic_jesszhan@quicinc.com; s=20230329; h=from:subject:message-id; bh=+nd3fKUDQE67UnP1gKxYdD/NiOvH3OI/CjTl6hloB/Q=; b=MXf+vGjMBODVmlI7EvaeiZBOM2KnDFUp5puLc8L9kUwc7jbmOl2sCuI/yMb/ZKes/dZhn8O6l EojpciWfDanDQSDY5/8G2CwMVDvVCw/ABGxgAukcrBBBG/QduWE15dZ X-Developer-Key: i=quic_jesszhan@quicinc.com; a=ed25519; pk=gAUCgHZ6wTJOzQa3U0GfeCDH7iZLlqIEPo4rrjfDpWE= X-Originating-IP: [10.80.80.8] X-ClientProxiedBy: nasanex01b.na.qualcomm.com (10.46.141.250) To nasanex01b.na.qualcomm.com (10.46.141.250) X-QCInternal: smtphost X-Proofpoint-Virus-Version: vendor=nai engine=6200 definitions=5800 signatures=585085 X-Proofpoint-ORIG-GUID: qyDk2NkvJMNWOp5O8OALQGftLPI8QYc0 X-Proofpoint-GUID: qyDk2NkvJMNWOp5O8OALQGftLPI8QYc0 X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.254,Aquarius:18.0.942,Hydra:6.0.573,FMLib:17.11.170.22 definitions=2023-05-02_14,2023-04-27_01,2023-02-09_01 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 malwarescore=0 priorityscore=1501 mlxlogscore=999 mlxscore=0 phishscore=0 clxscore=1015 lowpriorityscore=0 spamscore=0 suspectscore=0 adultscore=0 bulkscore=0 impostorscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2303200000 definitions=main-2305030009 X-Spam-Status: No, score=-2.8 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_LOW,SPF_HELO_NONE, SPF_PASS,T_SCC_BODY_TEXT_LINE,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1764834344531064154?= X-GMAIL-MSGID: =?utf-8?q?1764834344531064154?= |
Series |
Add DSC v1.2 Support for DSI
|
|
Commit Message
Jessica Zhang
May 3, 2023, 1:19 a.m. UTC
Divide the pclk rate by the compression ratio when DSC is enabled
Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.com>
---
drivers/gpu/drm/msm/dsi/dsi_host.c | 14 ++++++++++----
1 file changed, 10 insertions(+), 4 deletions(-)
Comments
On 03/05/2023 04:19, Jessica Zhang wrote: > Divide the pclk rate by the compression ratio when DSC is enabled > > Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.com> > --- > drivers/gpu/drm/msm/dsi/dsi_host.c | 14 ++++++++++---- > 1 file changed, 10 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c > index 43a5ec33eee8..35c69dbe5f6f 100644 > --- a/drivers/gpu/drm/msm/dsi/dsi_host.c > +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c > @@ -561,7 +561,8 @@ void dsi_link_clk_disable_v2(struct msm_dsi_host *msm_host) > clk_disable_unprepare(msm_host->byte_clk); > } > > -static unsigned long dsi_get_pclk_rate(const struct drm_display_mode *mode, bool is_bonded_dsi) > +static unsigned long dsi_get_pclk_rate(const struct drm_display_mode *mode, > + struct drm_dsc_config *dsc, bool is_bonded_dsi) > { > unsigned long pclk_rate; > > @@ -576,6 +577,11 @@ static unsigned long dsi_get_pclk_rate(const struct drm_display_mode *mode, bool > if (is_bonded_dsi) > pclk_rate /= 2; > > + /* If DSC is enabled, divide pclk by compression ratio */ > + if (dsc) > + pclk_rate = DIV_ROUND_UP(pclk_rate, > + dsc->bits_per_component * 3 / msm_dsc_get_bpp_int(dsc)); > + Don't we loose precision here? Would DIV_ROUND_UP(pclk_rate * bpp, dsc->bpc * 3) be better? > return pclk_rate; > } > > @@ -585,7 +591,7 @@ unsigned long dsi_byte_clk_get_rate(struct mipi_dsi_host *host, bool is_bonded_d > struct msm_dsi_host *msm_host = to_msm_dsi_host(host); > u8 lanes = msm_host->lanes; > u32 bpp = dsi_get_bpp(msm_host->format); > - unsigned long pclk_rate = dsi_get_pclk_rate(mode, is_bonded_dsi); > + unsigned long pclk_rate = dsi_get_pclk_rate(mode, msm_host->dsc, is_bonded_dsi); > u64 pclk_bpp = (u64)pclk_rate * bpp; > > if (lanes == 0) { > @@ -604,7 +610,7 @@ unsigned long dsi_byte_clk_get_rate(struct mipi_dsi_host *host, bool is_bonded_d > > static void dsi_calc_pclk(struct msm_dsi_host *msm_host, bool is_bonded_dsi) > { > - msm_host->pixel_clk_rate = dsi_get_pclk_rate(msm_host->mode, is_bonded_dsi); > + msm_host->pixel_clk_rate = dsi_get_pclk_rate(msm_host->mode, msm_host->dsc, is_bonded_dsi); > msm_host->byte_clk_rate = dsi_byte_clk_get_rate(&msm_host->base, is_bonded_dsi, > msm_host->mode); > > @@ -634,7 +640,7 @@ int dsi_calc_clk_rate_v2(struct msm_dsi_host *msm_host, bool is_bonded_dsi) > > dsi_calc_pclk(msm_host, is_bonded_dsi); > > - pclk_bpp = (u64)dsi_get_pclk_rate(msm_host->mode, is_bonded_dsi) * bpp; > + pclk_bpp = (u64)dsi_get_pclk_rate(msm_host->mode, msm_host->dsc, is_bonded_dsi) * bpp; > do_div(pclk_bpp, 8); > msm_host->src_clk_rate = pclk_bpp; > >
On 5/3/2023 1:33 AM, Dmitry Baryshkov wrote: > On 03/05/2023 04:19, Jessica Zhang wrote: >> Divide the pclk rate by the compression ratio when DSC is enabled >> >> Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.com> >> --- >> drivers/gpu/drm/msm/dsi/dsi_host.c | 14 ++++++++++---- >> 1 file changed, 10 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c >> b/drivers/gpu/drm/msm/dsi/dsi_host.c >> index 43a5ec33eee8..35c69dbe5f6f 100644 >> --- a/drivers/gpu/drm/msm/dsi/dsi_host.c >> +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c >> @@ -561,7 +561,8 @@ void dsi_link_clk_disable_v2(struct msm_dsi_host >> *msm_host) >> clk_disable_unprepare(msm_host->byte_clk); >> } >> -static unsigned long dsi_get_pclk_rate(const struct drm_display_mode >> *mode, bool is_bonded_dsi) >> +static unsigned long dsi_get_pclk_rate(const struct drm_display_mode >> *mode, >> + struct drm_dsc_config *dsc, bool is_bonded_dsi) >> { >> unsigned long pclk_rate; >> @@ -576,6 +577,11 @@ static unsigned long dsi_get_pclk_rate(const >> struct drm_display_mode *mode, bool >> if (is_bonded_dsi) >> pclk_rate /= 2; >> + /* If DSC is enabled, divide pclk by compression ratio */ >> + if (dsc) >> + pclk_rate = DIV_ROUND_UP(pclk_rate, >> + dsc->bits_per_component * 3 / msm_dsc_get_bpp_int(dsc)); >> + > > Don't we loose precision here? > Would DIV_ROUND_UP(pclk_rate * bpp, dsc->bpc * 3) be better? Hi Dmitry, Acked. Thanks, Jessica Zhang > >> return pclk_rate; >> } >> @@ -585,7 +591,7 @@ unsigned long dsi_byte_clk_get_rate(struct >> mipi_dsi_host *host, bool is_bonded_d >> struct msm_dsi_host *msm_host = to_msm_dsi_host(host); >> u8 lanes = msm_host->lanes; >> u32 bpp = dsi_get_bpp(msm_host->format); >> - unsigned long pclk_rate = dsi_get_pclk_rate(mode, is_bonded_dsi); >> + unsigned long pclk_rate = dsi_get_pclk_rate(mode, msm_host->dsc, >> is_bonded_dsi); >> u64 pclk_bpp = (u64)pclk_rate * bpp; >> if (lanes == 0) { >> @@ -604,7 +610,7 @@ unsigned long dsi_byte_clk_get_rate(struct >> mipi_dsi_host *host, bool is_bonded_d >> static void dsi_calc_pclk(struct msm_dsi_host *msm_host, bool >> is_bonded_dsi) >> { >> - msm_host->pixel_clk_rate = dsi_get_pclk_rate(msm_host->mode, >> is_bonded_dsi); >> + msm_host->pixel_clk_rate = dsi_get_pclk_rate(msm_host->mode, >> msm_host->dsc, is_bonded_dsi); >> msm_host->byte_clk_rate = dsi_byte_clk_get_rate(&msm_host->base, >> is_bonded_dsi, >> msm_host->mode); >> @@ -634,7 +640,7 @@ int dsi_calc_clk_rate_v2(struct msm_dsi_host >> *msm_host, bool is_bonded_dsi) >> dsi_calc_pclk(msm_host, is_bonded_dsi); >> - pclk_bpp = (u64)dsi_get_pclk_rate(msm_host->mode, is_bonded_dsi) >> * bpp; >> + pclk_bpp = (u64)dsi_get_pclk_rate(msm_host->mode, msm_host->dsc, >> is_bonded_dsi) * bpp; >> do_div(pclk_bpp, 8); >> msm_host->src_clk_rate = pclk_bpp; >> > > -- > With best wishes > Dmitry >
Title suggestion: use the wording "reduce pclk rate" :) (Eventually "when DSC is enabled", instead of "for compression") On 2023-05-02 18:19:12, Jessica Zhang wrote: > Divide the pclk rate by the compression ratio when DSC is enabled > > Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.com> Thank you so much for sending this. The compression ratio was applied to hdisplay, but not the clocks yet, and with this patch I get a massive reduction in clock speeds on the Xperia XZ3, without regressions nor affecting performance/fps: gcc_sys_noc_cpuss_ahb_clk 1 1 0 19200000 0 0 50000 Y gcc_cpuss_ahb_clk 1 1 0 19200000 0 0 50000 Y bi_tcxo 6 6 0 19200000 0 0 50000 Y dsi0vco_clk 1 1 0 [-1873793994-]{+1249195898+} 0 0 50000 Y dsi0_pll_out_div_clk 1 1 0 [-1873793994-]{+624597949+} 0 0 50000 Y dsi0_pll_post_out_div_clk 0 0 0 [-468448498-]{+156149487+} 0 0 50000 Y dsi0_pll_bit_clk 2 2 0 [-1873793994-]{+624597949+} 0 0 50000 Y dsi0_pclk_mux 1 1 0 [-1873793994-]{+624597949+} 0 0 50000 Y dsi0_phy_pll_out_dsiclk 1 1 0 [-312298999-]{+104099659+} 0 0 50000 Y disp_cc_mdss_pclk0_clk_src 1 1 0 [-312298999-]{+104099659+} 0 0 50000 Y disp_cc_mdss_pclk0_clk 1 1 0 [-312298999-]{+104099659+} 0 0 50000 Y dsi0_pll_by_2_bit_clk 0 0 0 [-936896997-]{+312298974+} 0 0 50000 Y dsi0_phy_pll_out_byteclk 1 1 0 [-234224249-]{+78074743+} 0 0 50000 Y disp_cc_mdss_byte0_clk_src 2 2 0 [-234224249-]{+78074743+} 0 0 50000 Y disp_cc_mdss_byte0_div_clk_src 1 1 0 [-117112125-]{+39037372+} 0 0 50000 Y disp_cc_mdss_byte0_intf_clk 1 1 0 [-117112125-]{+39037372+} 0 0 50000 Y disp_cc_mdss_byte0_clk 1 1 0 [-234224249-]{+78074743+} 0 0 50000 Y gpu_cc_pll1 0 0 0 500000097 0 0 50000 N disp_cc_mdss_dp_pixel_clk_src 0 0 0 19200000 0 0 50000 N disp_cc_mdss_dp_pixel_clk 0 0 0 19200000 0 0 50000 N Reviewed-by: Marijn Suijten <marijn.suijten@somainline.org> > --- > drivers/gpu/drm/msm/dsi/dsi_host.c | 14 ++++++++++---- > 1 file changed, 10 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c > index 43a5ec33eee8..35c69dbe5f6f 100644 > --- a/drivers/gpu/drm/msm/dsi/dsi_host.c > +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c > @@ -561,7 +561,8 @@ void dsi_link_clk_disable_v2(struct msm_dsi_host *msm_host) > clk_disable_unprepare(msm_host->byte_clk); > } > > -static unsigned long dsi_get_pclk_rate(const struct drm_display_mode *mode, bool is_bonded_dsi) > +static unsigned long dsi_get_pclk_rate(const struct drm_display_mode *mode, It is a bit unfortunate that this function is called so often with the same parameters, doing the same calculation over and over. > + struct drm_dsc_config *dsc, bool is_bonded_dsi) > { > unsigned long pclk_rate; > > @@ -576,6 +577,11 @@ static unsigned long dsi_get_pclk_rate(const struct drm_display_mode *mode, bool > if (is_bonded_dsi) > pclk_rate /= 2; > > + /* If DSC is enabled, divide pclk by compression ratio */ > + if (dsc) > + pclk_rate = DIV_ROUND_UP(pclk_rate, > + dsc->bits_per_component * 3 / msm_dsc_get_bpp_int(dsc)); Don't forget to mention that this series depends on the DSC helpers. I don't think the linked DSC 1.2 series depends on it (at least it doesn't mention it): https://lore.kernel.org/linux-arm-msm/20230329-rfc-msm-dsc-helper-v6-2-cb7f59f0f7fb@quicinc.com/ - Marijn > + > return pclk_rate; > } > > @@ -585,7 +591,7 @@ unsigned long dsi_byte_clk_get_rate(struct mipi_dsi_host *host, bool is_bonded_d > struct msm_dsi_host *msm_host = to_msm_dsi_host(host); > u8 lanes = msm_host->lanes; > u32 bpp = dsi_get_bpp(msm_host->format); > - unsigned long pclk_rate = dsi_get_pclk_rate(mode, is_bonded_dsi); > + unsigned long pclk_rate = dsi_get_pclk_rate(mode, msm_host->dsc, is_bonded_dsi); > u64 pclk_bpp = (u64)pclk_rate * bpp; > > if (lanes == 0) { > @@ -604,7 +610,7 @@ unsigned long dsi_byte_clk_get_rate(struct mipi_dsi_host *host, bool is_bonded_d > > static void dsi_calc_pclk(struct msm_dsi_host *msm_host, bool is_bonded_dsi) > { > - msm_host->pixel_clk_rate = dsi_get_pclk_rate(msm_host->mode, is_bonded_dsi); > + msm_host->pixel_clk_rate = dsi_get_pclk_rate(msm_host->mode, msm_host->dsc, is_bonded_dsi); > msm_host->byte_clk_rate = dsi_byte_clk_get_rate(&msm_host->base, is_bonded_dsi, > msm_host->mode); > > @@ -634,7 +640,7 @@ int dsi_calc_clk_rate_v2(struct msm_dsi_host *msm_host, bool is_bonded_dsi) > > dsi_calc_pclk(msm_host, is_bonded_dsi); > > - pclk_bpp = (u64)dsi_get_pclk_rate(msm_host->mode, is_bonded_dsi) * bpp; > + pclk_bpp = (u64)dsi_get_pclk_rate(msm_host->mode, msm_host->dsc, is_bonded_dsi) * bpp; > do_div(pclk_bpp, 8); > msm_host->src_clk_rate = pclk_bpp; > > > -- > 2.40.1 >
On 2023-05-04 22:33:17, Marijn Suijten wrote: > Title suggestion: use the wording "reduce pclk rate" :) > > (Eventually "when DSC is enabled", instead of "for compression") > > On 2023-05-02 18:19:12, Jessica Zhang wrote: > > Divide the pclk rate by the compression ratio when DSC is enabled > > > > Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.com> > > Thank you so much for sending this. The compression ratio was applied > to hdisplay In hindsight, on the note of hdisplay, dsi_timing_setup() actually only divides the visual portion - that is hdisplay out of htotal - without affecting the back and front porch. Since this clock inside the mode is based on the full htotal * vtotal * ..., should we compensate for that and only divide the visual portion of the clock signal by 3? Otherwise we might not have enough clockticks to perform the front and back porch (even though CMD mode doesn't really have porches, I have yet to properly understand that part of the signal). - Marijn > , but not the clocks yet, and with this patch I get a massive > reduction in clock speeds on the Xperia XZ3, without regressions nor > affecting performance/fps: > > gcc_sys_noc_cpuss_ahb_clk 1 1 0 19200000 0 0 50000 Y > gcc_cpuss_ahb_clk 1 1 0 19200000 0 0 50000 Y > bi_tcxo 6 6 0 19200000 0 0 50000 Y > dsi0vco_clk 1 1 0 [-1873793994-]{+1249195898+} 0 0 50000 Y > dsi0_pll_out_div_clk 1 1 0 [-1873793994-]{+624597949+} 0 0 50000 Y > dsi0_pll_post_out_div_clk 0 0 0 [-468448498-]{+156149487+} 0 0 50000 Y > dsi0_pll_bit_clk 2 2 0 [-1873793994-]{+624597949+} 0 0 50000 Y > dsi0_pclk_mux 1 1 0 [-1873793994-]{+624597949+} 0 0 50000 Y > dsi0_phy_pll_out_dsiclk 1 1 0 [-312298999-]{+104099659+} 0 0 50000 Y > disp_cc_mdss_pclk0_clk_src 1 1 0 [-312298999-]{+104099659+} 0 0 50000 Y > disp_cc_mdss_pclk0_clk 1 1 0 [-312298999-]{+104099659+} 0 0 50000 Y > dsi0_pll_by_2_bit_clk 0 0 0 [-936896997-]{+312298974+} 0 0 50000 Y > dsi0_phy_pll_out_byteclk 1 1 0 [-234224249-]{+78074743+} 0 0 50000 Y > disp_cc_mdss_byte0_clk_src 2 2 0 [-234224249-]{+78074743+} 0 0 50000 Y > disp_cc_mdss_byte0_div_clk_src 1 1 0 [-117112125-]{+39037372+} 0 0 50000 Y > disp_cc_mdss_byte0_intf_clk 1 1 0 [-117112125-]{+39037372+} 0 0 50000 Y > disp_cc_mdss_byte0_clk 1 1 0 [-234224249-]{+78074743+} 0 0 50000 Y > gpu_cc_pll1 0 0 0 500000097 0 0 50000 N > disp_cc_mdss_dp_pixel_clk_src 0 0 0 19200000 0 0 50000 N > disp_cc_mdss_dp_pixel_clk 0 0 0 19200000 0 0 50000 N > > Reviewed-by: Marijn Suijten <marijn.suijten@somainline.org> > > > --- > > drivers/gpu/drm/msm/dsi/dsi_host.c | 14 ++++++++++---- > > 1 file changed, 10 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c > > index 43a5ec33eee8..35c69dbe5f6f 100644 > > --- a/drivers/gpu/drm/msm/dsi/dsi_host.c > > +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c > > @@ -561,7 +561,8 @@ void dsi_link_clk_disable_v2(struct msm_dsi_host *msm_host) > > clk_disable_unprepare(msm_host->byte_clk); > > } > > > > -static unsigned long dsi_get_pclk_rate(const struct drm_display_mode *mode, bool is_bonded_dsi) > > +static unsigned long dsi_get_pclk_rate(const struct drm_display_mode *mode, > > It is a bit unfortunate that this function is called so often with the > same parameters, doing the same calculation over and over. > > > + struct drm_dsc_config *dsc, bool is_bonded_dsi) > > { > > unsigned long pclk_rate; > > > > @@ -576,6 +577,11 @@ static unsigned long dsi_get_pclk_rate(const struct drm_display_mode *mode, bool > > if (is_bonded_dsi) > > pclk_rate /= 2; > > > > + /* If DSC is enabled, divide pclk by compression ratio */ > > + if (dsc) > > + pclk_rate = DIV_ROUND_UP(pclk_rate, > > + dsc->bits_per_component * 3 / msm_dsc_get_bpp_int(dsc)); > > Don't forget to mention that this series depends on the DSC helpers. I > don't think the linked DSC 1.2 series depends on it (at least it doesn't > mention it): > > https://lore.kernel.org/linux-arm-msm/20230329-rfc-msm-dsc-helper-v6-2-cb7f59f0f7fb@quicinc.com/ > > - Marijn > > > + > > return pclk_rate; > > } > > > > @@ -585,7 +591,7 @@ unsigned long dsi_byte_clk_get_rate(struct mipi_dsi_host *host, bool is_bonded_d > > struct msm_dsi_host *msm_host = to_msm_dsi_host(host); > > u8 lanes = msm_host->lanes; > > u32 bpp = dsi_get_bpp(msm_host->format); > > - unsigned long pclk_rate = dsi_get_pclk_rate(mode, is_bonded_dsi); > > + unsigned long pclk_rate = dsi_get_pclk_rate(mode, msm_host->dsc, is_bonded_dsi); > > u64 pclk_bpp = (u64)pclk_rate * bpp; > > > > if (lanes == 0) { > > @@ -604,7 +610,7 @@ unsigned long dsi_byte_clk_get_rate(struct mipi_dsi_host *host, bool is_bonded_d > > > > static void dsi_calc_pclk(struct msm_dsi_host *msm_host, bool is_bonded_dsi) > > { > > - msm_host->pixel_clk_rate = dsi_get_pclk_rate(msm_host->mode, is_bonded_dsi); > > + msm_host->pixel_clk_rate = dsi_get_pclk_rate(msm_host->mode, msm_host->dsc, is_bonded_dsi); > > msm_host->byte_clk_rate = dsi_byte_clk_get_rate(&msm_host->base, is_bonded_dsi, > > msm_host->mode); > > > > @@ -634,7 +640,7 @@ int dsi_calc_clk_rate_v2(struct msm_dsi_host *msm_host, bool is_bonded_dsi) > > > > dsi_calc_pclk(msm_host, is_bonded_dsi); > > > > - pclk_bpp = (u64)dsi_get_pclk_rate(msm_host->mode, is_bonded_dsi) * bpp; > > + pclk_bpp = (u64)dsi_get_pclk_rate(msm_host->mode, msm_host->dsc, is_bonded_dsi) * bpp; > > do_div(pclk_bpp, 8); > > msm_host->src_clk_rate = pclk_bpp; > > > > > > -- > > 2.40.1 > >
On 5/4/2023 2:17 PM, Marijn Suijten wrote: > On 2023-05-04 22:33:17, Marijn Suijten wrote: >> Title suggestion: use the wording "reduce pclk rate" :) >> >> (Eventually "when DSC is enabled", instead of "for compression") >> >> On 2023-05-02 18:19:12, Jessica Zhang wrote: >>> Divide the pclk rate by the compression ratio when DSC is enabled >>> >>> Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.com> >> >> Thank you so much for sending this. The compression ratio was applied >> to hdisplay > > In hindsight, on the note of hdisplay, dsi_timing_setup() actually only > divides the visual portion - that is hdisplay out of htotal - without > affecting the back and front porch. > > Since this clock inside the mode is based on the full htotal * vtotal * > ..., should we compensate for that and only divide the visual portion of > the clock signal by 3? Otherwise we might not have enough clockticks to > perform the front and back porch (even though CMD mode doesn't really > have porches, I have yet to properly understand that part of the > signal). Hi Marijn, That's a fair point. Will change the pclk math accordingly. Thanks, Jessica Zhang > > - Marijn > >> , but not the clocks yet, and with this patch I get a massive >> reduction in clock speeds on the Xperia XZ3, without regressions nor >> affecting performance/fps: >> >> gcc_sys_noc_cpuss_ahb_clk 1 1 0 19200000 0 0 50000 Y >> gcc_cpuss_ahb_clk 1 1 0 19200000 0 0 50000 Y >> bi_tcxo 6 6 0 19200000 0 0 50000 Y >> dsi0vco_clk 1 1 0 [-1873793994-]{+1249195898+} 0 0 50000 Y >> dsi0_pll_out_div_clk 1 1 0 [-1873793994-]{+624597949+} 0 0 50000 Y >> dsi0_pll_post_out_div_clk 0 0 0 [-468448498-]{+156149487+} 0 0 50000 Y >> dsi0_pll_bit_clk 2 2 0 [-1873793994-]{+624597949+} 0 0 50000 Y >> dsi0_pclk_mux 1 1 0 [-1873793994-]{+624597949+} 0 0 50000 Y >> dsi0_phy_pll_out_dsiclk 1 1 0 [-312298999-]{+104099659+} 0 0 50000 Y >> disp_cc_mdss_pclk0_clk_src 1 1 0 [-312298999-]{+104099659+} 0 0 50000 Y >> disp_cc_mdss_pclk0_clk 1 1 0 [-312298999-]{+104099659+} 0 0 50000 Y >> dsi0_pll_by_2_bit_clk 0 0 0 [-936896997-]{+312298974+} 0 0 50000 Y >> dsi0_phy_pll_out_byteclk 1 1 0 [-234224249-]{+78074743+} 0 0 50000 Y >> disp_cc_mdss_byte0_clk_src 2 2 0 [-234224249-]{+78074743+} 0 0 50000 Y >> disp_cc_mdss_byte0_div_clk_src 1 1 0 [-117112125-]{+39037372+} 0 0 50000 Y >> disp_cc_mdss_byte0_intf_clk 1 1 0 [-117112125-]{+39037372+} 0 0 50000 Y >> disp_cc_mdss_byte0_clk 1 1 0 [-234224249-]{+78074743+} 0 0 50000 Y >> gpu_cc_pll1 0 0 0 500000097 0 0 50000 N >> disp_cc_mdss_dp_pixel_clk_src 0 0 0 19200000 0 0 50000 N >> disp_cc_mdss_dp_pixel_clk 0 0 0 19200000 0 0 50000 N >> >> Reviewed-by: Marijn Suijten <marijn.suijten@somainline.org> >> >>> --- >>> drivers/gpu/drm/msm/dsi/dsi_host.c | 14 ++++++++++---- >>> 1 file changed, 10 insertions(+), 4 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c >>> index 43a5ec33eee8..35c69dbe5f6f 100644 >>> --- a/drivers/gpu/drm/msm/dsi/dsi_host.c >>> +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c >>> @@ -561,7 +561,8 @@ void dsi_link_clk_disable_v2(struct msm_dsi_host *msm_host) >>> clk_disable_unprepare(msm_host->byte_clk); >>> } >>> >>> -static unsigned long dsi_get_pclk_rate(const struct drm_display_mode *mode, bool is_bonded_dsi) >>> +static unsigned long dsi_get_pclk_rate(const struct drm_display_mode *mode, >> >> It is a bit unfortunate that this function is called so often with the >> same parameters, doing the same calculation over and over. >> >>> + struct drm_dsc_config *dsc, bool is_bonded_dsi) >>> { >>> unsigned long pclk_rate; >>> >>> @@ -576,6 +577,11 @@ static unsigned long dsi_get_pclk_rate(const struct drm_display_mode *mode, bool >>> if (is_bonded_dsi) >>> pclk_rate /= 2; >>> >>> + /* If DSC is enabled, divide pclk by compression ratio */ >>> + if (dsc) >>> + pclk_rate = DIV_ROUND_UP(pclk_rate, >>> + dsc->bits_per_component * 3 / msm_dsc_get_bpp_int(dsc)); >> >> Don't forget to mention that this series depends on the DSC helpers. I >> don't think the linked DSC 1.2 series depends on it (at least it doesn't >> mention it): >> >> https://lore.kernel.org/linux-arm-msm/20230329-rfc-msm-dsc-helper-v6-2-cb7f59f0f7fb@quicinc.com/ >> >> - Marijn >> >>> + >>> return pclk_rate; >>> } >>> >>> @@ -585,7 +591,7 @@ unsigned long dsi_byte_clk_get_rate(struct mipi_dsi_host *host, bool is_bonded_d >>> struct msm_dsi_host *msm_host = to_msm_dsi_host(host); >>> u8 lanes = msm_host->lanes; >>> u32 bpp = dsi_get_bpp(msm_host->format); >>> - unsigned long pclk_rate = dsi_get_pclk_rate(mode, is_bonded_dsi); >>> + unsigned long pclk_rate = dsi_get_pclk_rate(mode, msm_host->dsc, is_bonded_dsi); >>> u64 pclk_bpp = (u64)pclk_rate * bpp; >>> >>> if (lanes == 0) { >>> @@ -604,7 +610,7 @@ unsigned long dsi_byte_clk_get_rate(struct mipi_dsi_host *host, bool is_bonded_d >>> >>> static void dsi_calc_pclk(struct msm_dsi_host *msm_host, bool is_bonded_dsi) >>> { >>> - msm_host->pixel_clk_rate = dsi_get_pclk_rate(msm_host->mode, is_bonded_dsi); >>> + msm_host->pixel_clk_rate = dsi_get_pclk_rate(msm_host->mode, msm_host->dsc, is_bonded_dsi); >>> msm_host->byte_clk_rate = dsi_byte_clk_get_rate(&msm_host->base, is_bonded_dsi, >>> msm_host->mode); >>> >>> @@ -634,7 +640,7 @@ int dsi_calc_clk_rate_v2(struct msm_dsi_host *msm_host, bool is_bonded_dsi) >>> >>> dsi_calc_pclk(msm_host, is_bonded_dsi); >>> >>> - pclk_bpp = (u64)dsi_get_pclk_rate(msm_host->mode, is_bonded_dsi) * bpp; >>> + pclk_bpp = (u64)dsi_get_pclk_rate(msm_host->mode, msm_host->dsc, is_bonded_dsi) * bpp; >>> do_div(pclk_bpp, 8); >>> msm_host->src_clk_rate = pclk_bpp; >>> >>> >>> -- >>> 2.40.1 >>>
diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c index 43a5ec33eee8..35c69dbe5f6f 100644 --- a/drivers/gpu/drm/msm/dsi/dsi_host.c +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c @@ -561,7 +561,8 @@ void dsi_link_clk_disable_v2(struct msm_dsi_host *msm_host) clk_disable_unprepare(msm_host->byte_clk); } -static unsigned long dsi_get_pclk_rate(const struct drm_display_mode *mode, bool is_bonded_dsi) +static unsigned long dsi_get_pclk_rate(const struct drm_display_mode *mode, + struct drm_dsc_config *dsc, bool is_bonded_dsi) { unsigned long pclk_rate; @@ -576,6 +577,11 @@ static unsigned long dsi_get_pclk_rate(const struct drm_display_mode *mode, bool if (is_bonded_dsi) pclk_rate /= 2; + /* If DSC is enabled, divide pclk by compression ratio */ + if (dsc) + pclk_rate = DIV_ROUND_UP(pclk_rate, + dsc->bits_per_component * 3 / msm_dsc_get_bpp_int(dsc)); + return pclk_rate; } @@ -585,7 +591,7 @@ unsigned long dsi_byte_clk_get_rate(struct mipi_dsi_host *host, bool is_bonded_d struct msm_dsi_host *msm_host = to_msm_dsi_host(host); u8 lanes = msm_host->lanes; u32 bpp = dsi_get_bpp(msm_host->format); - unsigned long pclk_rate = dsi_get_pclk_rate(mode, is_bonded_dsi); + unsigned long pclk_rate = dsi_get_pclk_rate(mode, msm_host->dsc, is_bonded_dsi); u64 pclk_bpp = (u64)pclk_rate * bpp; if (lanes == 0) { @@ -604,7 +610,7 @@ unsigned long dsi_byte_clk_get_rate(struct mipi_dsi_host *host, bool is_bonded_d static void dsi_calc_pclk(struct msm_dsi_host *msm_host, bool is_bonded_dsi) { - msm_host->pixel_clk_rate = dsi_get_pclk_rate(msm_host->mode, is_bonded_dsi); + msm_host->pixel_clk_rate = dsi_get_pclk_rate(msm_host->mode, msm_host->dsc, is_bonded_dsi); msm_host->byte_clk_rate = dsi_byte_clk_get_rate(&msm_host->base, is_bonded_dsi, msm_host->mode); @@ -634,7 +640,7 @@ int dsi_calc_clk_rate_v2(struct msm_dsi_host *msm_host, bool is_bonded_dsi) dsi_calc_pclk(msm_host, is_bonded_dsi); - pclk_bpp = (u64)dsi_get_pclk_rate(msm_host->mode, is_bonded_dsi) * bpp; + pclk_bpp = (u64)dsi_get_pclk_rate(msm_host->mode, msm_host->dsc, is_bonded_dsi) * bpp; do_div(pclk_bpp, 8); msm_host->src_clk_rate = pclk_bpp;