Message ID | 20230405-add-dsc-support-v2-1-1072c70e9786@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 b10csp701080vqo; Fri, 5 May 2023 14:34:16 -0700 (PDT) X-Google-Smtp-Source: ACHHUZ4E0JjU/KFZP0yYx5HWPCLBIaqh93mYaJjz2oqrtnfwVSEEqnfWUMc/PqOvXKE8VVOA71Dj X-Received: by 2002:a17:90a:6ac4:b0:24e:1ae5:1bed with SMTP id b4-20020a17090a6ac400b0024e1ae51bedmr2741158pjm.16.1683322455943; Fri, 05 May 2023 14:34:15 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1683322455; cv=none; d=google.com; s=arc-20160816; b=jlHBmzjp0dUoLcRWLs5uwMjeUGI6Qxg6QAPZ+bnLN8sWC+FMav4Fk/VTWaW4gSzXuO /S0xZqoT005uGw2tG1B13UJ6QPSsOGpzTCpjKgN/2I7+fOLGyYGod3etDbLNe0qIZHUB QeUOGwbNRAm6yfckmPNZi147m3XNCOb6I9qCiYIFBOvG/DvIUE4/xpqDm/LJVGcHVE4D /18bz5YcxR3Q0vmVX/mfMqWcgWPcozDdkjIPK8RoPO9YrzqQ2EE9cWlvpOLm5eSSR+Ax sonZdETRjPoq1aQVvFjzUN4VCS6J7AjdAM6JKx160ijK6qrw/d3kiBtHPaDbUUGVV2kf wbNQ== 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=NNgxuTByx6ume4kdD6nXKqWOnG0hiha6GhDtLcO0Rcg=; b=z54waBT1ZKRoLse++mZDBzeXyYDyapU/6p6ODh1fPpGScEc7awv9Ge/MTb8yf6vnrC vYIuo/3IfizMfRboQSqriZ0DUwRdghVsEBSP6Ew4BTrNDW49Wk/IwUoaCSdGjrklD5Cp pQB9FE1//vEwxD6rOM9ER82yks5LDbNICKTlHMkJYUdWOlb0N7rEPmbWZqE6Paj/A38a V8N5+jt1i6MGNfOpcyrpYy6JwFUI4K7PszbU9heo+LYv1y+k5QkgkEphdMqSmn6zY1Gh qJVJhTPogbpNIMMypnLEPy+nvz6Uhu59G76IcmKHmcQOnUi3hJpPE3DN8J+YW2mX8WhQ SClg== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@quicinc.com header.s=qcppdkim1 header.b=hLVahI2Z; 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 m24-20020a17090aab1800b0024e2bb99e67si7124396pjq.7.2023.05.05.14.34.03; Fri, 05 May 2023 14:34:15 -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=hLVahI2Z; 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 S233183AbjEEVYY (ORCPT <rfc822;baris.duru.linux@gmail.com> + 99 others); Fri, 5 May 2023 17:24:24 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:54982 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232814AbjEEVYN (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Fri, 5 May 2023 17:24:13 -0400 Received: from mx0a-0031df01.pphosted.com (mx0a-0031df01.pphosted.com [205.220.168.131]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id C58BE2681; Fri, 5 May 2023 14:24:11 -0700 (PDT) Received: from pps.filterd (m0279862.ppops.net [127.0.0.1]) by mx0a-0031df01.pphosted.com (8.17.1.19/8.17.1.19) with ESMTP id 345Jvmgf009940; Fri, 5 May 2023 21:24:04 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=NNgxuTByx6ume4kdD6nXKqWOnG0hiha6GhDtLcO0Rcg=; b=hLVahI2ZdJ3TXswyhloVdwK4TMHfEYX84EboQKkIyOTI+m/0lSxWWPOwZyVU3CqqJd05 2jHltdqtZMw+q12dOcqGiVndRp/fK/QttaceF+ZzgWs2du6268FFM/gE0ptIhLidS6nQ ICxhQ+hwhITydJISUCTSJlilkMHsKNJyccuWvhbkdUJqAxZTQWviF5SZrBpMM28pJZ3w g5+lUQIF7FJyp2fodsv14YORN/qHA0h7fMz+cjIWohdKp3Db6hNf0MQEQq2PXwJOTnfF 0HPNeUN5vkM1dPsHtbYsJZMz2vsP/986L01UH/e06oLuzsCdkyTz6Zk8z1nOE7dS3MWm ZA== Received: from nasanppmta01.qualcomm.com (i-global254.qualcomm.com [199.106.103.254]) by mx0a-0031df01.pphosted.com (PPS) with ESMTPS id 3qckf72qts-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Fri, 05 May 2023 21:24:04 +0000 Received: from nasanex01b.na.qualcomm.com (nasanex01b.na.qualcomm.com [10.46.141.250]) by NASANPPMTA01.qualcomm.com (8.17.1.5/8.17.1.5) with ESMTPS id 345LO3jg004028 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Fri, 5 May 2023 21:24:04 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; Fri, 5 May 2023 14:24:03 -0700 From: Jessica Zhang <quic_jesszhan@quicinc.com> Date: Fri, 5 May 2023 14:23:48 -0700 Subject: [PATCH v2 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-v2-1-1072c70e9786@quicinc.com> References: <20230405-add-dsc-support-v2-0-1072c70e9786@quicinc.com> In-Reply-To: <20230405-add-dsc-support-v2-0-1072c70e9786@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=1683321843; l=2828; i=quic_jesszhan@quicinc.com; s=20230329; h=from:subject:message-id; bh=6n1T0L02ntmxE0+q2uWO5/DF/EkRCHj6r3frjVC2bss=; b=Bt1jmzQ16O2P1SrQWe4cARrdUa4KWBYBdCVkDsycyRPTseHF6hXmT1cdc+tejphR+r8L1Tt6B NvTKbj34AnPBqT3phrgorguxVu96pV3SnHCuiEcOsb9+2QxAc7wKbes 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-GUID: 4tRVdtU4hSjpjd_Gb7YAn7VDF02ypx5u X-Proofpoint-ORIG-GUID: 4tRVdtU4hSjpjd_Gb7YAn7VDF02ypx5u 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-05_27,2023-05-05_01,2023-02-09_01 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 clxscore=1015 spamscore=0 bulkscore=0 phishscore=0 adultscore=0 malwarescore=0 mlxlogscore=999 suspectscore=0 priorityscore=1501 lowpriorityscore=0 impostorscore=0 mlxscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2303200000 definitions=main-2305050173 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 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?1765091527559028025?= X-GMAIL-MSGID: =?utf-8?q?1765091527559028025?= |
Series |
Add DSC v1.2 Support for DSI
|
|
Commit Message
Jessica Zhang
May 5, 2023, 9:23 p.m. UTC
Adjust the pclk rate to divide hdisplay by the compression ratio when DSC
is enabled.
Changes in v2:
- Adjusted pclk_rate math to divide only the hdisplay value by
compression ratio
Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.com>
---
drivers/gpu/drm/msm/dsi/dsi_host.c | 17 +++++++++++++----
1 file changed, 13 insertions(+), 4 deletions(-)
Comments
On 5/5/2023 2:23 PM, Jessica Zhang wrote: > Adjust the pclk rate to divide hdisplay by the compression ratio when DSC > is enabled. > > Changes in v2: > - Adjusted pclk_rate math to divide only the hdisplay value by > compression ratio > > Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.com> > --- > drivers/gpu/drm/msm/dsi/dsi_host.c | 17 +++++++++++++---- > 1 file changed, 13 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..0e5778e8091f 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,14 @@ 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 hdisplay by compression ratio */ > + if (dsc) { > + int new_hdisplay = DIV_ROUND_UP(mode->hdisplay * msm_dsc_get_bpp_int(dsc), > + dsc->bits_per_component * 3); > + int fps = DIV_ROUND_UP(pclk_rate, mode->htotal * mode->vtotal); Should've used drm_mode_vrefresh() here... Will spin a v3 with that change (along with any additional comments) > + pclk_rate = (new_hdisplay + (mode->htotal - mode->hdisplay)) * mode->vtotal * fps; > + } > + > return pclk_rate; > } > > @@ -585,7 +594,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 +613,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 +643,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-05 14:49:08, Jessica Zhang wrote: > On 5/5/2023 2:23 PM, Jessica Zhang wrote: > > Adjust the pclk rate to divide hdisplay by the compression ratio when DSC > > is enabled. > > > > Changes in v2: > > - Adjusted pclk_rate math to divide only the hdisplay value by > > compression ratio > > > > Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.com> > > --- > > drivers/gpu/drm/msm/dsi/dsi_host.c | 17 +++++++++++++---- > > 1 file changed, 13 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..0e5778e8091f 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,14 @@ 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 hdisplay by compression ratio */ > > + if (dsc) { > > + int new_hdisplay = DIV_ROUND_UP(mode->hdisplay * msm_dsc_get_bpp_int(dsc), > > + dsc->bits_per_component * 3); > > + int fps = DIV_ROUND_UP(pclk_rate, mode->htotal * mode->vtotal); > > Should've used drm_mode_vrefresh() here... Will spin a v3 with that > change (along with any additional comments) Perhaps unsigned long on some of these? Overall the computations and multi-lines look rather cluttered, perhaps (parts of) this is/are a prime candidate to go into the new helpers? Note that I cannot get the 4k mode working at 60Hz on one of my panels (30Hz works with minor corruption), regardless of this patch. See also: https://gitlab.freedesktop.org/drm/msm/-/issues/24#note_1900031 > > + pclk_rate = (new_hdisplay + (mode->htotal - mode->hdisplay)) * mode->vtotal * fps; > > + } > > + > > return pclk_rate; > > } > > > > @@ -585,7 +594,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 +613,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 +643,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; Let's rebase on top of "drm/msm/dsi: simplify pixel clk rate handling" [1] to clean this up. [1]: https://lore.kernel.org/linux-arm-msm/20230118130031.2345941-1-dmitry.baryshkov@linaro.org/ - Marijn > > do_div(pclk_bpp, 8); > > msm_host->src_clk_rate = pclk_bpp; > > > > > > -- > > 2.40.1 > >
On 5/8/2023 2:56 PM, Marijn Suijten wrote: > On 2023-05-05 14:49:08, Jessica Zhang wrote: >> On 5/5/2023 2:23 PM, Jessica Zhang wrote: >>> Adjust the pclk rate to divide hdisplay by the compression ratio when DSC >>> is enabled. >>> >>> Changes in v2: >>> - Adjusted pclk_rate math to divide only the hdisplay value by >>> compression ratio >>> >>> Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.com> >>> --- >>> drivers/gpu/drm/msm/dsi/dsi_host.c | 17 +++++++++++++---- >>> 1 file changed, 13 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..0e5778e8091f 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,14 @@ 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 hdisplay by compression ratio */ >>> + if (dsc) { >>> + int new_hdisplay = DIV_ROUND_UP(mode->hdisplay * msm_dsc_get_bpp_int(dsc), >>> + dsc->bits_per_component * 3); >>> + int fps = DIV_ROUND_UP(pclk_rate, mode->htotal * mode->vtotal); >> >> Should've used drm_mode_vrefresh() here... Will spin a v3 with that >> change (along with any additional comments) > > Perhaps unsigned long on some of these? Overall the computations and > multi-lines look rather cluttered, perhaps (parts of) this is/are a > prime candidate to go into the new helpers? Hi Marijn, Sorry for the late reply, wanted to get the MSM DSC helpers series settled first before addressing these changes. Sounds good, I'll put these calculations in a separate dsi_adjust_compressed_pclk() helper. > > Note that I cannot get the 4k mode working at 60Hz on one of my panels > (30Hz works with minor corruption), regardless of this patch. See also: > https://gitlab.freedesktop.org/drm/msm/-/issues/24#note_1900031 As discussed elsewhere, we suspect that this is unrelated to DSC specifically and might be an issue with the upstream driver not taking transfer time into account with calculating pclk_rate. We will look into this as a separate issue. > >>> + pclk_rate = (new_hdisplay + (mode->htotal - mode->hdisplay)) * mode->vtotal * fps; >>> + } >>> + >>> return pclk_rate; >>> } >>> >>> @@ -585,7 +594,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 +613,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 +643,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; > > Let's rebase on top of "drm/msm/dsi: simplify pixel clk rate handling" > [1] to clean this up. > > [1]: https://lore.kernel.org/linux-arm-msm/20230118130031.2345941-1-dmitry.baryshkov@linaro.org/ I've looked into this patch and have made a comment on it. Just have some reservations about it as it changes the functionality of a clk handler op. I will hold off on rebasing and wait for that thread to resolve first. Thanks, Jessica Zhang > > - Marijn > >>> do_div(pclk_bpp, 8); >>> msm_host->src_clk_rate = pclk_bpp; >>> >>> >>> -- >>> 2.40.1 >>>
On 2023-05-19 12:04:00, Jessica Zhang wrote: <snip> > >>> + /* If DSC is enabled, divide hdisplay by compression ratio */ > >>> + if (dsc) { > >>> + int new_hdisplay = DIV_ROUND_UP(mode->hdisplay * msm_dsc_get_bpp_int(dsc), > >>> + dsc->bits_per_component * 3); > >>> + int fps = DIV_ROUND_UP(pclk_rate, mode->htotal * mode->vtotal); > >> > >> Should've used drm_mode_vrefresh() here... Will spin a v3 with that > >> change (along with any additional comments) > > > > Perhaps unsigned long on some of these? Overall the computations and > > multi-lines look rather cluttered, perhaps (parts of) this is/are a > > prime candidate to go into the new helpers? > > Hi Marijn, > > Sorry for the late reply, wanted to get the MSM DSC helpers series > settled first before addressing these changes. No hurry and no worry, that is exactly why I requested this to be split across multiple series so that we can make progress on that in isolation (or rather, make progress on the first series in the chain before iterating on the next). On the other hand Dmitry made the right remark that it does cause contention for some patches that only become relevant in future series... but that's mostly down to how the patches are distributed across series. > Sounds good, I'll put these calculations in a separate > dsi_adjust_compressed_pclk() helper. Not sure if "adjust" carries the meaning, but I'll leave it to you to come up with an initial revision and then we can discuss. I am mostly curious if there are generic (DSI) timing rules that apply DRM-wide, or if these would be MSM-specific. Otherwise assigning them to properly named local variables is the perfect way to self-document the code. > > Note that I cannot get the 4k mode working at 60Hz on one of my panels > > (30Hz works with minor corruption), regardless of this patch. See also: > > https://gitlab.freedesktop.org/drm/msm/-/issues/24#note_1900031 > As discussed elsewhere, we suspect that this is unrelated to DSC > specifically and might be an issue with the upstream driver not taking > transfer time into account with calculating pclk_rate. > > We will look into this as a separate issue. Yes that is very likely, but it is still a good idea to take into account when looking into adjusting DSC timing: can we do that in any sensible way without first accounting for transfer time? <snip> > >>> 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; > > > > Let's rebase on top of "drm/msm/dsi: simplify pixel clk rate handling" > > [1] to clean this up. > > > > [1]: https://lore.kernel.org/linux-arm-msm/20230118130031.2345941-1-dmitry.baryshkov@linaro.org/ > > I've looked into this patch and have made a comment on it. Just have > some reservations about it as it changes the functionality of a clk > handler op. > > I will hold off on rebasing and wait for that thread to resolve first. Looks like the resolution was to drop it, but we should still first apply the following hunk from it so that this line in your patch can be skipped entirely. - pclk_bpp = (u64)dsi_get_pclk_rate(msm_host->mode, is_bonded_dsi) * bpp; + pclk_bpp = msm_host->pixel_clk_rate * bpp; - Marijn
diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c index 43a5ec33eee8..0e5778e8091f 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,14 @@ 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 hdisplay by compression ratio */ + if (dsc) { + int new_hdisplay = DIV_ROUND_UP(mode->hdisplay * msm_dsc_get_bpp_int(dsc), + dsc->bits_per_component * 3); + int fps = DIV_ROUND_UP(pclk_rate, mode->htotal * mode->vtotal); + pclk_rate = (new_hdisplay + (mode->htotal - mode->hdisplay)) * mode->vtotal * fps; + } + return pclk_rate; } @@ -585,7 +594,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 +613,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 +643,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;