[PATCH v6 6/7] drm/msm/dsi: update hdisplay calculation for dsi_timing_setup

[Jessica Zhang]

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 at quicinc.com>
>>>>> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov at linaro.org>
>>>>> ---
>>>>>    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().

Thanks,

Jessica Zhang

> 
> - Marijn