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
