[PATCH RFT v2 07/15] drm/msm/a6xx: Resolve the meaning of UBWC_MODE

Konrad Dybcio posted 15 patches 9 months ago
There is a newer version of this series
[PATCH RFT v2 07/15] drm/msm/a6xx: Resolve the meaning of UBWC_MODE
Posted by Konrad Dybcio 9 months ago
From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>

This bit is set iff the UBWC version is 1.0. That notably does not
include QCM2290's "no UBWC".

Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
---
 drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
index e7c89f9c7d89798699848743843eed6a58b94bd3..6af4e70c1b936a30c1934dd49f2889be13c9780d 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
@@ -669,10 +669,10 @@ static void a6xx_set_ubwc_config(struct msm_gpu *gpu)
 	 */
 	BUG_ON(adreno_gpu->ubwc_config.highest_bank_bit < 13);
 	u32 hbb = adreno_gpu->ubwc_config.highest_bank_bit - 13;
+	bool ubwc_mode = qcom_ubwc_get_ubwc_mode(cfg);
 	bool amsbc = cfg->ubwc_enc_version >= UBWC_3_0;
 	u32 hbb_hi = hbb >> 2;
 	u32 hbb_lo = hbb & 3;
-	u32 ubwc_mode = adreno_gpu->ubwc_config.ubwc_swizzle & 1;
 	u32 level2_swizzling_dis = !(adreno_gpu->ubwc_config.ubwc_swizzle & 2);
 
 	gpu_write(gpu, REG_A6XX_RB_NC_MODE_CNTL,

-- 
2.49.0
Re: [PATCH RFT v2 07/15] drm/msm/a6xx: Resolve the meaning of UBWC_MODE
Posted by Dmitry Baryshkov 9 months ago
On Wed, May 14, 2025 at 05:10:27PM +0200, Konrad Dybcio wrote:
> From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
> 
> This bit is set iff the UBWC version is 1.0. That notably does not
> include QCM2290's "no UBWC".
> 
> Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
> ---
>  drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> index e7c89f9c7d89798699848743843eed6a58b94bd3..6af4e70c1b936a30c1934dd49f2889be13c9780d 100644
> --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> @@ -669,10 +669,10 @@ static void a6xx_set_ubwc_config(struct msm_gpu *gpu)
>  	 */
>  	BUG_ON(adreno_gpu->ubwc_config.highest_bank_bit < 13);
>  	u32 hbb = adreno_gpu->ubwc_config.highest_bank_bit - 13;
> +	bool ubwc_mode = qcom_ubwc_get_ubwc_mode(cfg);

I'd really prefer if the function came in this patch rather than being
added at some earlier point. I understand that you want to simplify
cross-tree merging, but I think we should also simplify reviewing.

>  	bool amsbc = cfg->ubwc_enc_version >= UBWC_3_0;
>  	u32 hbb_hi = hbb >> 2;
>  	u32 hbb_lo = hbb & 3;
> -	u32 ubwc_mode = adreno_gpu->ubwc_config.ubwc_swizzle & 1;
>  	u32 level2_swizzling_dis = !(adreno_gpu->ubwc_config.ubwc_swizzle & 2);
>  
>  	gpu_write(gpu, REG_A6XX_RB_NC_MODE_CNTL,
> 
> -- 
> 2.49.0
> 

-- 
With best wishes
Dmitry
Re: [PATCH RFT v2 07/15] drm/msm/a6xx: Resolve the meaning of UBWC_MODE
Posted by Rob Clark 9 months ago
On Wed, May 14, 2025 at 12:15 PM Dmitry Baryshkov
<dmitry.baryshkov@oss.qualcomm.com> wrote:
>
> On Wed, May 14, 2025 at 05:10:27PM +0200, Konrad Dybcio wrote:
> > From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
> >
> > This bit is set iff the UBWC version is 1.0. That notably does not
> > include QCM2290's "no UBWC".
> >
> > Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
> > ---
> >  drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> > index e7c89f9c7d89798699848743843eed6a58b94bd3..6af4e70c1b936a30c1934dd49f2889be13c9780d 100644
> > --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> > +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> > @@ -669,10 +669,10 @@ static void a6xx_set_ubwc_config(struct msm_gpu *gpu)
> >        */
> >       BUG_ON(adreno_gpu->ubwc_config.highest_bank_bit < 13);
> >       u32 hbb = adreno_gpu->ubwc_config.highest_bank_bit - 13;
> > +     bool ubwc_mode = qcom_ubwc_get_ubwc_mode(cfg);
>
> I'd really prefer if the function came in this patch rather than being
> added at some earlier point. I understand that you want to simplify
> cross-tree merging, but I think we should also simplify reviewing.

Also, since it is so far just used by display and gpu, we probably
don't need to care about cross tree too much... we could just land via
drm

BR,
-R

> >       bool amsbc = cfg->ubwc_enc_version >= UBWC_3_0;
> >       u32 hbb_hi = hbb >> 2;
> >       u32 hbb_lo = hbb & 3;
> > -     u32 ubwc_mode = adreno_gpu->ubwc_config.ubwc_swizzle & 1;
> >       u32 level2_swizzling_dis = !(adreno_gpu->ubwc_config.ubwc_swizzle & 2);
> >
> >       gpu_write(gpu, REG_A6XX_RB_NC_MODE_CNTL,
> >
> > --
> > 2.49.0
> >
>
> --
> With best wishes
> Dmitry