On 2023-05-08 17:51:08, Jessica Zhang wrote: > > > On 5/7/2023 11:34 AM, Marijn Suijten wrote: > > On 2023-05-07 17:27:33, Marijn Suijten wrote: > >> On 2023-05-04 15:05:15, Abhinav Kumar wrote: > >>> > >>> > >>> On 5/4/2023 2:56 PM, Marijn Suijten wrote: > >>>> On 2023-04-12 16:25:20, 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. > >>>>> > >>>>> Changes in v3: > >>>>> - Split from previous patch > >>>>> - Initialized hdisplay as uncompressed pclk per line at the beginning of > >>>>> dsi_timing_setup as to not break dual DSI calculations > >>>>> > >>>>> Changes in v4: > >>>>> - Moved pclk_per_intf calculations to DSC hdisplay adjustments > >>>>> > >>>>> Signed-off-by: Jessica Zhang <quic_jesszhan@xxxxxxxxxxx> > >>>>> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx> > >>>>> --- > >>>>> drivers/gpu/drm/msm/dsi/dsi_host.c | 2 +- > >>>>> 1 file changed, 1 insertion(+), 1 deletion(-) > >>>>> > >>>>> diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c > >>>>> index 508577c596ff..ae966d4e349d 100644 > >>>>> --- a/drivers/gpu/drm/msm/dsi/dsi_host.c > >>>>> +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c > >>>>> @@ -952,7 +952,7 @@ static void dsi_timing_setup(struct msm_dsi_host *msm_host, bool is_bonded_dsi) > >>>>> * pulse width same > >>>>> */ > >>>>> h_total -= hdisplay; > >>>>> - hdisplay /= 3; > >>>>> + hdisplay = msm_dsc_get_pclk_per_intf(msm_host->dsc) / 3; > >>>> > >>>> This patch is unfortunately regressing the Sony Xperia XZ3 (sdm845, > >>>> single DSI), which will only show garbage when it is applied. > >>>> > >>>> Are you sure this is correct, and the helper is returning the right > >>>> values? I'll see if I can help review and validate those later, and > >>>> debug if necessary. > >>>> > >>>> - Marijn > >>> > >>> To help us debug these kind of issues, can you pls point us to your > >>> panel driver? > >> > >> https://github.com/SoMainline/linux/commit/b154ea72e6c2ca0d4a33a28cc24e3a762dba4948 > > > > I found the fix myself after piecing together the hints provided across > > the many different patch series. This panel driver assigns > > slice_count=1 based on downstream's qcom,mdss-dsc-slice-per-pkt = <1>, > > but as per the many slice_count-related fixes the latter DT parameter is > > a QCOM-specific hardware feature, whereas slice_count is simply the > > number of slices per line. > > > > Since a line is a scanline, and that panel has a width of hdisplay=1440 > > and a slice_width of 720, the number of slices spanning a line is simply > > slice_count=hdisplay/slice_width=2. This makes the panel work again > > atop the four-or-so-series without a revert of this patch. > > > > Is it a big ask to request a single, coherent series fixing all uses of > > slice_count - and implementing support for slice-per-pkt - instead of > > having the patches spread across multiple series? That makes it much > > easier to cover ground here and review this series, as slice_count seems > > to be used everywhere where downstream used slice_per_pkt - even I > > mistakenly used it after assuming it was the same based on the original > > patches. > > Hi Marijn, > > Just want to document the changes we agreed on regarding the slice_count > fixes: > > I will move "drm/msm/dsi: Fix calculation for pkt_per_line" to the "Add > DSC v1.2 Support for DSI" series so that all the > slice_count/slice_per_pkt fixes are consolidated. > > In addition I will also add a patch in "Add DSC v1.2 Support for DSI" to > remove the incorrect `dsc->slice_count = 1` line in dsi_update_dsc_timing(). That sounds good, but be sure to check two things: - Can we squash some of the patches? - Can we make the wording and title consistent? That goes a long way in understanding that the patches in question are addressing the same "wrong use of dsc->slice_count because it was thought to be equal to slice_per_pkt" issue. - Marijn
