[Freedreno] [PATCH v10 8/8] drm/msm/dsi: update hdisplay calculation for dsi_timing_setup

[Jessica Zhang]

On 5/16/2023 3:45 PM, Marijn Suijten wrote:
> On 2023-05-16 11:18:17, Jessica Zhang wrote:
>> On 5/14/2023 2:29 PM, Marijn Suijten wrote:
>>> On 2023-05-12 14:32:18, Jessica Zhang wrote:
>>>>
>>>> hdisplay for compressed images should be calculated as bytes_per_slice *
>>>> slice_count. Thus, use MSM DSC helper to calculate hdisplay for
>>>> dsi_timing_setup instead of directly using mode->hdisplay.
>>>
>>> As mentioned in review on an earlier revision, is there any sort of
>>> clarification you can provide here to explain the cases where
>>> hdisplay!=bytes_per_line?  That goes a long way towards justifying this
>>> change.  Thanks!
>>
>> Hi Marijn,
>>
>> Sorry for not responding to this in the earlier revision, I think I
>> missed the original comment.
>>
>> Please correct me if I'm wrong, but I'm guessing the question here is
>> why we can't keep the hdisplay adjustment as `hdisplay /= 3` and have to
>> go out of our way to recalculate hdisplay before doing the `/ 3`.
>>
>> This is because the original adjustment only works for BPP = 8. By using
>> the msm_dsc_get_bytes_per_line() here, we can generalize this adjustment
>> to work for cases where BPP != 8.
> 
> I am fully aware that the original computation only works for BPP=8 and
> even mentioned so in v7 review [1].  The question / request is instead
> to include such context in your commit message, rather than the
> nondescriptive "should be calculated as" -> who says that and why?
> Stating that the current approach was only working for BPP=8 (hence why
> currently tested panels are working fine) but that this isn't a
> long-term solution if we starts upporting other BPP is a proper
> justification to make this change.

Sounds good, will add this to the commit message.

Thanks,

Jessica Zhang

> 
> [1]: https://lore.kernel.org/linux-arm-msm/ju7647tlogo25fnhswgp7zn67syvsjy2ldjugvygh3z4rxtdrx@kb76evjvulgw/
> 
>> Thanks,
> 
> Thanks for looking into improving this!
> 
> - Marijn