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
