[PATCH 1/2] drm/amdgpu: Allow IP to skip the first ras enablement

[Pan, Xinhui]

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 at gmail.com> 
Sent: 2019年4月4日 23:14
To: Pan, Xinhui <Xinhui.Pan at amd.com>
Cc: amd-gfx at lists.freedesktop.org; Deucher, Alexander <Alexander.Deucher at amd.com>
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 at amd.com> wrote:
>
> If vbios has enabled ras for us, skip it
>
> Signed-off-by: xinhui pan <xinhui.pan at amd.com>

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 at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx