2023-05-19 13:37:20

by Konrad Dybcio

Subject: [PATCH v2 2/6] drm/msm/a6xx: Explain CP_PROTECT_CNTL writes in a6xx_set_cp_protect

We have the necessary information, so explain which bit does what.

Signed-off-by: Konrad Dybcio <[email protected]>
---
drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
index 9fb214f150dd..deed42675fe2 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
@@ -771,9 +771,10 @@ static void a6xx_set_cp_protect(struct msm_gpu *gpu)
}

/*
- * Enable access protection to privileged registers, fault on an access
- * protect violation and select the last span to protect from the start
- * address all the way to the end of the register address space
+ * BIT(0) - Enable access protection to privileged registers
+ * BIT(1) - Enable fault on an access protect violation
+ * BIT(3) - Select the last span to protect from the start
+ * address all the way to the end of the register address space
*/
gpu_write(gpu, REG_A6XX_CP_PROTECT_CNTL, BIT(0) | BIT(1) | BIT(3));


--
2.40.1

2023-05-25 17:45:05

by Rob Clark

Subject: Re: [PATCH v2 2/6] drm/msm/a6xx: Explain CP_PROTECT_CNTL writes in a6xx_set_cp_protect

On Fri, May 19, 2023 at 6:29 AM Konrad Dybcio <[email protected]> wrote:
>
> We have the necessary information, so explain which bit does what.
>
> Signed-off-by: Konrad Dybcio <[email protected]>
> ---
> drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> index 9fb214f150dd..deed42675fe2 100644
> --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> @@ -771,9 +771,10 @@ static void a6xx_set_cp_protect(struct msm_gpu *gpu)
> }
>
> /*
> - * Enable access protection to privileged registers, fault on an access
> - * protect violation and select the last span to protect from the start
> - * address all the way to the end of the register address space
> + * BIT(0) - Enable access protection to privileged registers
> + * BIT(1) - Enable fault on an access protect violation
> + * BIT(3) - Select the last span to protect from the start
> + * address all the way to the end of the register address space

Even better would be to give these bitfields names in the xml, which
also gets you more meaningful decoding. We've been better about not
open-coding a lot of stuff on the mesa side, but it is a bit easier
with header generation integrated into the build.. but less
open-coding on the kernel side is still a noble goal

BR,
-R

> */
> gpu_write(gpu, REG_A6XX_CP_PROTECT_CNTL, BIT(0) | BIT(1) | BIT(3));
>
>
> --
> 2.40.1
>