Re: [PATCH v3 17/27] drm/msm/mdss: use new helper to set amsbc

[Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>]

On 3/16/26 3:20 PM, Dmitry Baryshkov wrote:
> On Mon, Mar 16, 2026 at 04:04:46PM +0200, Dmitry Baryshkov wrote:
>> On Mon, Mar 16, 2026 at 11:08:00AM +0100, Konrad Dybcio wrote:
>>> On 3/13/26 3:26 PM, Dmitry Baryshkov wrote:
>>>> On Fri, Mar 13, 2026 at 11:19:59AM +0100, Konrad Dybcio wrote:
>>>>> On 3/12/26 2:29 PM, Dmitry Baryshkov wrote:
>>>>>> Use freshly defined helper instead of checking the UBWC version
>>>>>> directly.
>>>>>>
>>>>>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
>>>>>> ---
>>>>>>  drivers/gpu/drm/msm/msm_mdss.c | 2 +-
>>>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/drivers/gpu/drm/msm/msm_mdss.c b/drivers/gpu/drm/msm/msm_mdss.c
>>>>>> index ca59bcdde7b2..04efc29f38cd 100644
>>>>>> --- a/drivers/gpu/drm/msm/msm_mdss.c
>>>>>> +++ b/drivers/gpu/drm/msm/msm_mdss.c
>>>>>> @@ -188,7 +188,7 @@ static void msm_mdss_5x_setup_ubwc(struct msm_mdss *msm_mdss)
>>>>>>  	if (qcom_ubwc_macrotile_mode(data))
>>>>>>  		value |= MDSS_UBWC_STATIC_MACROTILE_MODE;
>>>>>>  
>>>>>> -	if (data->ubwc_enc_version == UBWC_3_0)
>>>>>> +	if (qcom_ubwc_enable_amsbc(data))
>>>>>>  		value |= MDSS_UBWC_STATIC_UBWC_AMSBC;
>>>>>
>>>>> I know it's already there, but fwiw downstream doesn't seem to be
>>>>> concerned with toggling AMSBC in MDSS (perhaps incorrectly?)
>>>>
>>>> Downstream enables it only for the cases where encoder and decoder
>>>> use exactly UBWC 3.0. This is not completely correct as the bit doesn't
>>>> exist for MDSS 6.x+. For 5.x this is equivalent to the code from the
>>>> patch: enable AMSBC if targeting UBWC >= 3.0 (because MDSS 5.x don't
>>>> support UBWC 4.0 or higher).
>>>
>>> Right, this seems to indeed be the case
>>>
>>> While trying to confirm that, I noticed that some lower-end MDSS 5.x
>>> chips (like Talos) report UBWC3 in the register, but the MDSS docs say
>>> they're UBWC2 - any idea?
>>
>> I don't see a complete pixel format document for Talos, but my guess is
>> that one of the blocks (GPU, Venus or Camera) doesn't support UBWC 3.0
>> pixel formats. MDSS is just one piece of picture and unfortunately we
>> need to use format which is supported by all the blocks.
> 
> And after cross-checking the docs, it is really interesting. It really
> looks like for those chips we have used the older core for UBWC, but the
> registeres were not updated. One more reason not to trust those and to
> use the database instead.

Yeah, downstream seems to have passed:

qcom,sde-ubwc-version = <0x200>;

as well

Konrad