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

[Abhinav Kumar]

On 6/23/2023 1:18 PM, Marijn Suijten wrote:
> 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

Ok, I finally got your question I think. That "why don't we account for 
widebus in the pixel clock calculation but only for calculating pclk 
cycles for REG_DSI_CMD_MDP_STREAM0_TOTAL"

Even though DPU shall transmit 48 bits worth of data , DSI will be able 
to consume only one uncompressed pixel worth of data. Thats the rule.

Hence even though we are able to account for widebus in the 
stream0_total to indicate DPU's increased data transfer rate with 
widebus, since DSI is still going to consume only one uncompressed pixel 
worth of data, we can only scale the pixel clock with compression ratio.

Hope that clarifies it.