[Freedreno] [PATCH v2] drm/msm/dsi: Document DSC related pclk_rate and hdisplay calculations

[Marijn Suijten]

On 2023-06-23 23:10:56, Dmitry Baryshkov wrote:
<snip>
> >> There is no confusion between what was said earlier and now.
> >>
> >> This line is calculating the number of pclks needed to transmit one line
> >> of the compressed data:
> >>
> >> hdisplay = DIV_ROUND_UP(msm_dsc_get_bytes_per_line(msm_host->dsc), 3);
> >>
> >> msm_dsc_get_bytes_per_line() is calculating the number of compressed
> >> bytes as it uses the target bits_per_pixel
> >>
> >> 126 	 * @bits_per_pixel:
> >> 127 	 * Target bits per pixel with 4 fractional bits, bits_per_pixel << 4
> >> 128 	 */
> >> 129 	u16 bits_per_pixel;
> >>
> >> (like I have said a few times, hdisplay is perhaps confusing us)
> >>
> >> If you calculate the bytes this way you are already accounting for the
> >> compression, so where is the confusion.
> >>
> >> The pclk calculation does the same thing of using the ratio instead.
> > 
> > This is not answering my question whether the ratio for pclk calculation
> > should also be adjusted to account for widebus.  And if the ratio is
> > fixed, why use a fixed factor here but the ratio between
> > src_bpp:target_bpp here?  It only adds extra confusion.
> 
> Wide bus is dicussed separately. I think the question you are trying to 
> ask is "why are we not using msm_dsc_get_bytes_per_line() in 
> dsi_adjust_pclk_for_compression()?"

I have asked that question before, and the answer was something
incomprehensible.  But indeed, it would look more natural if
dsi_adjust_pclk_for_compression() replaces:

    int new_hdisplay = DIV_ROUND_UP(mode->hdisplay * drm_dsc_get_bpp_int(dsc),
        dsc->bits_per_component * 3)

With:

    int new_hdisplay = DIV_ROUND_UP(msm_dsc_get_bytes_per_line(dsc), 3);

Which is the same value as we have here.

And then it becomes more clear how widebus affects this calculation.

- Marijn