On 2023-06-16 12:41:52, Dmitry Baryshkov wrote:
> Provide actual documentation for the pclk and hdisplay calculations in
> the case of DSC compression being used.
> 
> Signed-off-by: Dmitry Baryshkov <[email protected]>
> ---
>  drivers/gpu/drm/msm/dsi/dsi_host.c | 35 ++++++++++++++++++++++++++++--
>  1 file changed, 33 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c 
> b/drivers/gpu/drm/msm/dsi/dsi_host.c
> index 3f6dfb4f9d5a..72c377c9c7be 100644
> --- a/drivers/gpu/drm/msm/dsi/dsi_host.c
> +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
> @@ -528,6 +528,21 @@ void dsi_link_clk_disable_v2(struct msm_dsi_host 
> *msm_host)
>       clk_disable_unprepare(msm_host->byte_clk);
>  }
>  
> +/*
> + * Adjust the pclk rate by calculating a new hdisplay proportional to

Make this a kerneldoc with:

    /**
     * dsi_adjust_pclk_for_compression() - Adjust ...

> + * the compression ratio such that:
> + *     new_hdisplay = old_hdisplay * compressed_bpp / uncompressed_bpp
> + *
> + * Porches do not need to be adjusted:
> + * - For the VIDEO mode they are not compressed by DSC and are passed as is.

as-is

Though this was never tested nor confirmed by QUIC, but we can assume it
is the case for now?

> + * - For the CMD mode the are no actual porches. Instead they represent the

the are no -> these are not

they currently* represent.  Let's make sure that folks read the FIXME
below by perhaps rewording it right into this entry?

> + *   overhead to the image data transfer. As such, they are calculated for 
> the
> + *   final mode parameters (after the compression) and are not to be adjusted
> + *   too.
> + *
> + *  FIXME: Reconsider this if/when CMD mode handling is rewritten to use
> + *  refresh rate and data overhead as a starting point of the calculations.
> + */
>  static unsigned long dsi_adjust_pclk_for_compression(const struct 
> drm_display_mode *mode,
>               const struct drm_dsc_config *dsc)
>  {
> @@ -926,8 +941,24 @@ static void dsi_timing_setup(struct msm_dsi_host 
> *msm_host, bool is_bonded_dsi)
>               if (ret)
>                       return;
>  
> -             /* Divide the display by 3 but keep back/font porch and
> -              * pulse width same
> +             /*
> +              * DPU sends 3 bytes per pclk cycle to DSI. If compression is
> +              * not used, a single pixel is transferred at each pulse, no
> +              * matter what bpp or pixel format is used. In case of DSC
> +              * compression this results (due to data alignment
> +              * requirements) in a transfer of 3 compressed pixel per pclk

3 compressed bytes*, not pixels.

> +              * cycle.
> +              *
> +              * If widebus is enabled, bus width is extended to 6 bytes.
> +              * This way the DPU can transfer 6 compressed pixels with bpp

pixels -> bytes?

> +              * less or equal to 8 or 3 compressed pyxels in case bpp is

pixels*... bytes?

And I will ask this **again**: does this mean we can halve pclk?

> +              * greater than 8.
> +              *
> +              * The back/font porch and pulse width are kept intact.  They
> +              * represent timing parameters rather than actual data
> +              * transfer.

See FIXME above on dsi_adjust_pclk_for_compression()?

Thanks so much for finally putting some of this to paper.

- Marijn

> +              *
> +              * XXX: widebus is not supported by the driver (yet).
>                */
>               h_total -= hdisplay;
>               hdisplay = 
> DIV_ROUND_UP(msm_dsc_get_bytes_per_line(msm_host->dsc), 3);
> -- 
> 2.39.2
> 

Reply via email to