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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.

[1]: https://lore.kernel.org/linux-arm-msm/ju7647tlogo25fnhswgp7zn67syvsjy2ldjugvygh3z4rxtdrx@kb76evjvulgw/

> Thanks,

Thanks for looking into improving this!

- Marijn



[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux