Sorry for making the code complex. I will cook a V2.
As ras is enabled by vbios, I am going to skip the ras TA enablement cmd again.
Now I think adding one check of ras enablement in each block is better.
-----Original Message-----
From: Alex Deucher <alexdeucher@xxxxxxxxx>
Sent: 2019年4月4日 23:14
To: Pan, Xinhui <Xinhui.Pan@xxxxxxx>
Cc: amd-gfx@xxxxxxxxxxxxxxxxxxxxx; Deucher, Alexander <Alexander.Deucher@xxxxxxx>
Subject: Re: [PATCH 1/2] drm/amdgpu: Allow IP to skip the first ras enablement
On Wed, Apr 3, 2019 at 2:29 AM Pan, Xinhui <Xinhui.Pan@xxxxxxx> wrote:
>
> If vbios has enabled ras for us, skip it
>
> Signed-off-by: xinhui pan <xinhui.pan@xxxxxxx>
I'm having trouble following the logic in this patch. Can you walk me through it?
Alex
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c | 25
> +++++++++++++++++++++++--
> 1 file changed, 23 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
> index 2b7a420d5a1f..fc4bf7237d4b 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
> @@ -118,8 +118,11 @@ const char *ras_block_string[] = { #define
> ras_err_str(i) (ras_error_string[ffs(i)]) #define ras_block_str(i)
> (ras_block_string[i])
>
> -#define AMDGPU_RAS_FLAG_INIT_BY_VBIOS 1 -#define RAS_DEFAULT_FLAGS
> (AMDGPU_RAS_FLAG_INIT_BY_VBIOS)
> +#define AMDGPU_RAS_FLAG(x) ({BUILD_BUG_ON(BIT(x) & AMDGPU_RAS_BLOCK_MASK);\
> + BIT(x); })
> +#define AMDGPU_RAS_FLAG_INIT_BY_VBIOS AMDGPU_RAS_FLAG(31) #define
> +RAS_DEFAULT_FLAGS (AMDGPU_RAS_FLAG_INIT_BY_VBIOS |\
> + AMDGPU_RAS_BLOCK_MASK)
>
> static int amdgpu_ras_reserve_vram(struct amdgpu_device *adev,
> uint64_t offset, uint64_t size, @@ -551,6 +554,20 @@
> static int amdgpu_ras_is_feature_enabled(struct amdgpu_device *adev,
> return con->features & BIT(head->block); }
>
> +static bool amdgpu_ras_cmpxchg_feature_initialized(struct amdgpu_device *adev,
> + struct ras_common_if *head, bool new) {
> + struct amdgpu_ras *con = amdgpu_ras_get_context(adev);
> + bool old = (con->flags & AMDGPU_RAS_FLAG_INIT_BY_VBIOS) &&
> + (con->flags & BIT(head->block));
> +
> + if (new)
> + con->flags |= BIT(head->block);
> + else
> + con->flags &= ~BIT(head->block);
> +
> + return old;
> +}
> /*
> * if obj is not created, then create one.
> * set feature enable flag.
> @@ -620,6 +637,9 @@ int amdgpu_ras_feature_enable(struct amdgpu_device *adev,
> /* Are we alerady in that state we are going to set? */
> if (!(!!enable ^ !!amdgpu_ras_is_feature_enabled(adev, head)))
> return 0;
> + /* check if vbios has enabled ras for us on the block. */
> + if (enable && amdgpu_ras_cmpxchg_feature_initialized(adev, head, false))
> + goto first_time_bypass_psp;
>
> ret = psp_ras_enable_features(&adev->psp, &info, enable);
> if (ret) {
> @@ -630,6 +650,7 @@ int amdgpu_ras_feature_enable(struct amdgpu_device *adev,
> return -EINVAL;
> }
>
> +first_time_bypass_psp:
> /* setup the obj */
> __amdgpu_ras_feature_enable(adev, head, enable);
>
> --
> 2.17.1
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
