Re: [PATCH RFT v2 13/15] soc: qcom: ubwc: Fix SM6125's ubwc_swizzle value

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

On 5/15/25 6:21 PM, Dmitry Baryshkov wrote:
> On 15/05/2025 19:18, Konrad Dybcio wrote:
>> On 5/14/25 10:33 PM, Dmitry Baryshkov wrote:
>>> On 14/05/2025 23:05, Konrad Dybcio wrote:
>>>> On 5/14/25 9:23 PM, Dmitry Baryshkov wrote:
>>>>> On Wed, May 14, 2025 at 05:10:33PM +0200, Konrad Dybcio wrote:
>>>>>> From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
>>>>>>
>>>>>> The value of 7 (a.k.a. GENMASK(2, 0), a.k.a. disabling levels 1-3 of
>>>>>> swizzling) is what we want on this platform (and others with a UBWC
>>>>>> 1.0 encoder).
>>>>>>
>>>>>> Fix it to make mesa happy (the hardware doesn't care about the 2 higher
>>>>>> bits, as they weren't consumed on this platform).
>>>>>>
>>>>>> Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
>>>>>> ---
>>>>>>    drivers/soc/qcom/ubwc_config.c | 2 +-
>>>>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/drivers/soc/qcom/ubwc_config.c b/drivers/soc/qcom/ubwc_config.c
>>>>>> index 9caecd071035ccb03f14464e9b7129ba34a7f862..96b94cf01218cce2dacdba22c7573ba6148fcdd1 100644
>>>>>> --- a/drivers/soc/qcom/ubwc_config.c
>>>>>> +++ b/drivers/soc/qcom/ubwc_config.c
>>>>>> @@ -103,7 +103,7 @@ static const struct qcom_ubwc_cfg_data sm6115_data = {
>>>>>>    static const struct qcom_ubwc_cfg_data sm6125_data = {
>>>>>>        .ubwc_enc_version = UBWC_1_0,
>>>>>>        .ubwc_dec_version = UBWC_3_0,
>>>>>> -    .ubwc_swizzle = 1,
>>>>>> +    .ubwc_swizzle = 7,
>>>>>>        .highest_bank_bit = 14,
>>>>>>    };
>>>>>
>>>>> Add a comment and squash into the patch 1.
>>>>
>>>> I don't think that's a good idea, plus this series should be merged
>>>> together anyway
>>>
>>> Well... Granted Rob's comment, I really think the patches should be reordered a bit:
>>>
>>> - MDSS: offset HBB by 13 (patch 2)
>>> - switch drm/msm/mdss and display to common DB (patches 1+3 squashed)
>>> - get a handle (patch 4)
>>> - resolve / simplify (patches 5-10, not squashed)
>>> - fix sm6125 (patch 13)
>>> - WARN_ON (swizzle != swizzle) or (HBB != HBB)
>>> - switch to common R/O config, keeping WARN_ON for the calculated values (with the hope to drop them after testing)
>>
>> Does this bring any functional benefit? This series is unfun to remix
> 
> I know the pain.
> 
> The functional benefit is to have the WARN_ON and side-by-side comparison of common_ubwc_config vs computed ubwc_config for HBB and swizzle.

HBB I agree, since we'll be outsourcing it to yet another driver, swizzle
should be good enough (tm) - I scanned through the values in the driver
and couldn't find anything wrong just by eye

I realize this sounds funny, but all in all I don't think it's worth the
effort just for that one

Konrad