Re: [PATCH v2 01/25] drm/msm/mdss: correct UBWC programming sequences

[Dmitry Baryshkov]

On Wed, Mar 11, 2026 at 10:01:35AM +0100, Konrad Dybcio wrote:
> On 3/11/26 4:22 AM, Dmitry Baryshkov wrote:
> > The UBWC registers in the MDSS region are not dependent on the UBWC
> > version (it is an invalid assumption we inherited from the vendor SDE
> > driver). Instead they are dependent only on the MDSS core revision.
> > 
> > Rework UBWC programming to follow MDSS revision and to use required (aka
> > encoder) UBWC version instead of the ubwc_dec_version.
> > 
> > Fixes: d68db6069a8e ("drm/msm/mdss: convert UBWC setup to use match data")
> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxxxxxxxx>
> > ---
> >  drivers/gpu/drm/msm/msm_mdss.c | 120 ++++++++++++++++-------------------------
> >  1 file changed, 45 insertions(+), 75 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/msm/msm_mdss.c b/drivers/gpu/drm/msm/msm_mdss.c
> > index 9047e8d9ee89..d8b0288f0040 100644
> > --- a/drivers/gpu/drm/msm/msm_mdss.c
> > +++ b/drivers/gpu/drm/msm/msm_mdss.c
> > @@ -166,27 +166,27 @@ static int _msm_mdss_irq_domain_add(struct msm_mdss *msm_mdss)
> >  	return 0;
> >  }
> >  
> > -static void msm_mdss_setup_ubwc_dec_20(struct msm_mdss *msm_mdss)
> > +static void msm_mdss_4x_setup_ubwc(struct msm_mdss *msm_mdss)
> >  {
> >  	const struct qcom_ubwc_cfg_data *data = msm_mdss->mdss_data;
> > -	u32 value = MDSS_UBWC_STATIC_UBWC_SWIZZLE(data->ubwc_swizzle) |
> > +	u32 value = MDSS_UBWC_STATIC_UBWC_SWIZZLE(data->ubwc_swizzle & 0x1) |
> >  		    MDSS_UBWC_STATIC_HIGHEST_BANK_BIT(data->highest_bank_bit - 13);
> >  
> > -	if (data->ubwc_bank_spread)
> > -		value |= MDSS_UBWC_STATIC_UBWC_BANK_SPREAD;
> > -
> >  	if (data->ubwc_enc_version == UBWC_1_0)
> >  		value |= MDSS_UBWC_STATIC_UBWC_MIN_ACC_LEN(1);
> >  
> >  	writel_relaxed(value, msm_mdss->mmio + REG_MDSS_UBWC_STATIC);
> >  }
> >  
> > -static void msm_mdss_setup_ubwc_dec_30(struct msm_mdss *msm_mdss)
> > +static void msm_mdss_5x_setup_ubwc(struct msm_mdss *msm_mdss)
> >  {
> >  	const struct qcom_ubwc_cfg_data *data = msm_mdss->mdss_data;
> > -	u32 value = MDSS_UBWC_STATIC_UBWC_SWIZZLE(data->ubwc_swizzle & 0x1) |
> > +	u32 value = MDSS_UBWC_STATIC_UBWC_SWIZZLE(data->ubwc_swizzle) |
> >  		    MDSS_UBWC_STATIC_HIGHEST_BANK_BIT(data->highest_bank_bit - 13);
> >  
> > +	if (data->ubwc_bank_spread)
> > +		value |= MDSS_UBWC_STATIC_UBWC_BANK_SPREAD;
> 
> FWIW neither BANK_SPREAD nor the higher bits of UBWC_SWIZZLE exist on 8150
> (MDP 5.0) or 8180 (MDP 5.1). They do on 8250 (MDP 6.0)

Interesting. I was using a register definitions as a reference and it
had those regs. I'll update the patchset.

> 
> [...]
> 
> > +	writel_relaxed(ver, msm_mdss->mmio + REG_MDSS_UBWC_CTRL_2);
> > +	writel_relaxed(prediction_mode, msm_mdss->mmio + REG_MDSS_UBWC_PREDICTION_MODE);
> 
> IDK if this is a concern, but this register has a note "Valid for UBWC_RGB565
> only"

UBWC >= 4.0

-- 
With best wishes
Dmitry