[PATCH v10 4/8] drm/msm: Add MSM-specific DSC helper methods

[Jessica Zhang]

On 5/16/2023 3:49 PM, Marijn Suijten wrote:
> On 2023-05-16 01:07:05, Dmitry Baryshkov wrote:
>>
>> On 16/05/2023 01:01, Marijn Suijten wrote:
>>> On 2023-05-15 13:29:21, Jessica Zhang wrote:
>>> <snip>
>>>>> Const, as requested elsewhere.  But this function is not used anywhere
>>>>> in any of the series (because we replaced the usages with more sensible
>>>>> member accesses like slice_chunk_size).
>>>>
>>>> Acked.
>>>>
>>>> I would prefer to keep this helper so that we have a way to easily get
>>>> BPP information from the DRM DSC config in the future, but if you'd
>>>> prefer we add this helper then, I'm also ok with that.
>>>
>>> The inverse helper is available ad DSC_BPP in drm_dsc_helper.c.  Perhaps
>>> we can move the two variants to the header and define them uniformly?
>>> This isn't MSM-specific it seems (i.e. the format supports fractional
>>> bpp but no RC parameters appear to be defined for such a format yet).
>>
>> I think DSC_BPP was removed (around v2 or v3 if I read changelog correctly).
> 
> Seems like it was removed at some point indeed, and now the helper file
> picked up an identically named DSC_BPP macro but with the inverse
> implementation :) - at least it's a *.c file.
> 
> Perhaps we can make it more consistent by defining both ways with
> concise macros in a header.

The changes in this series don't touch DSC_BPP so I think moving that 
might be a change for another time, but I can at least move 
msm_dsc_get_bpp_int to drm_dsc_helper.h.

Thanks,

Jessica Zhang

> 
>> As for the fraction-point BPP, I think AMD supports .5 bpp granularity,
>> see drivers/gpu/drm/amd/display/dc/dml/dsc/qp_tables.h
> 
> That won't use the helper then.
> 
>> With best wishes
>> Dmitry
> 
> - Marijn