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

[Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>]

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.

You can say that I dislike the idea of copying & modifying config as 
this is the code that we will drop later (hopefully).



-- 
With best wishes
Dmitry