[v1] Add a single source of truth for UBWC configuration data

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

Posted by Konrad Dybcio 9 months, 1 week 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 e1eab0906b6c460528da82a94a285ef181e0b479..d47726ea8818a9660eadd52d97dde1489a884684 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
@@ -663,10 +663,10 @@ static void a6xx_set_ubwc_config(struct msm_gpu *gpu)
 	u8 uavflagprd_inv = adreno_is_a650_family(adreno_gpu) || adreno_is_a7xx(adreno_gpu) ? 2 : 0;
 	const struct qcom_ubwc_cfg_data *cfg = adreno_gpu->common_ubwc_cfg;
 	u32 hbb = adreno_gpu->ubwc_config.highest_bank_bit;
+	bool ubwc_mode = cfg->ubwc_enc_version == UBWC_1_0;
 	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 07/14] drm/msm/a6xx: Resolve the meaning of UBWC_MODE

Posted by Connor Abbott 9 months, 1 week ago

On Thu, May 8, 2025 at 2:14 PM Konrad Dybcio <konradybcio@kernel.org> 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".

While this is technically true, AFAIK the only difference between UBWC
1.0 and 2.0 is that newer UBWC disables level 1 bank swizzling, which
is why I originally wrote it this way. There's a bit of redundancy
between the UBWC version and ubwc_swizzle bit 0.

Connor

>
> 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 e1eab0906b6c460528da82a94a285ef181e0b479..d47726ea8818a9660eadd52d97dde1489a884684 100644
> --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> @@ -663,10 +663,10 @@ static void a6xx_set_ubwc_config(struct msm_gpu *gpu)
>         u8 uavflagprd_inv = adreno_is_a650_family(adreno_gpu) || adreno_is_a7xx(adreno_gpu) ? 2 : 0;
>         const struct qcom_ubwc_cfg_data *cfg = adreno_gpu->common_ubwc_cfg;
>         u32 hbb = adreno_gpu->ubwc_config.highest_bank_bit;
> +       bool ubwc_mode = cfg->ubwc_enc_version == UBWC_1_0;
>         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 07/14] drm/msm/a6xx: Resolve the meaning of UBWC_MODE

Posted by Konrad Dybcio 9 months ago

On 5/8/25 8:25 PM, Connor Abbott wrote:
> On Thu, May 8, 2025 at 2:14 PM Konrad Dybcio <konradybcio@kernel.org> 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".
> 
> While this is technically true, AFAIK the only difference between UBWC
> 1.0 and 2.0 is that newer UBWC disables level 1 bank swizzling, which
> is why I originally wrote it this way. There's a bit of redundancy
> between the UBWC version and ubwc_swizzle bit 0.

It turns out to be a hardware matter

Konrad

Re: [PATCH RFT 07/14] drm/msm/a6xx: Resolve the meaning of UBWC_MODE

Posted by Konrad Dybcio 9 months ago

On 5/9/25 2:37 PM, Konrad Dybcio wrote:
> On 5/8/25 8:25 PM, Connor Abbott wrote:
>> On Thu, May 8, 2025 at 2:14 PM Konrad Dybcio <konradybcio@kernel.org> 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".
>>
>> While this is technically true, AFAIK the only difference between UBWC
>> 1.0 and 2.0 is that newer UBWC disables level 1 bank swizzling, which
>> is why I originally wrote it this way. There's a bit of redundancy
>> between the UBWC version and ubwc_swizzle bit 0.
> 
> It turns out to be a hardware matter

Well you just said that.. in any case, i can do either (or both with a
sanity check)

Konrad