Messages in this thread | | | Date | Fri, 5 May 2023 11:57:43 -0700 | Subject | Re: [PATCH 1/4] drm/msm/dsi: Adjust pclk rate for compression | From | Jessica Zhang <> |
| |
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 >>>
| |