On 2025-02-10 18:13:58, Konrad Dybcio wrote: > On 10.02.2025 6:10 PM, Konrad Dybcio wrote: > > On 8.02.2025 11:09 PM, Marijn Suijten wrote: > >> On 2025-02-03 21:14:26, Danila Tikhonov wrote: ... > >>> dsc->simple_422 = 0; > >>> dsc->convert_rgb = 1; > >>> dsc->vbr_enable = 0; > >> > >> This seems supicous on the dpu1 side, in the original DSC 1.1 (not 1.2) block in > >> dpu_hw_dsc_config(), which has: > >> > >> data |= (dsc->line_buf_depth << 3); > >> data |= (dsc->simple_422 << 2); > >> data |= (dsc->convert_rgb << 1); > >> data |= dsc->bits_per_component; > >> > >> The original value of `8` would overlap with the lowest bit of line_buf_depth > >> (4th bit in `data`). Now, the 2nd bit which will take the value from > >> convert_rgb, which is already set to 1 above, will overlap with the 2nd bit in > >> your new bpc value of 10. > >> > >> Can you double-check that this code in DPU1 is actually valid? I assume you > >> have tested this panel at least and it is working (worthy mention in the cover > >> letter?), this just seems like yet another mistake in the original bindings > >> (though the register always had a matching value with downstream on 8 BPC panels > >> for me). > > > > It seems like the lowest bit should be set iff the input is 10bpc, the > > current situation where our '8' bleeds into the following (correctly named > > fields) is bad. > > See also > > https://github.com/Wikidepia/kernel_xiaomi_santoni-4.9/blob/master/drivers/gpu/drm/msm/sde/sde_hw_dsc.c#L67-L80 Correct, this is also what Dmitry already replied on Sunday and what I formulated into a patch earlier today (now sent), which I hope you can ack. - Marijn > > Konrad
