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

[Dmitry Baryshkov]

On Thu, 15 May 2025 at 19:36, Konrad Dybcio
<konrad.dybcio at oss.qualcomm.com> wrote:
>
> 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 at 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 at 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

Well. What is the ubwc_swizzle value used for SDM845? I think it
should be 6 according to a6xx_gpu.c and 0 according to msm_mdss.c.
Yes, higher bits are most likely ignored. Still, we'd better have one
correct value.

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



-- 
With best wishes
Dmitry