RE: [PATCH] drm/amdgpu: limit harvest bit read on several ASICs

["Chen, Guchun" <Guchun.Chen@amd.com>]

Hi Alex,

Exactly, I have the same idea, but I need to double confirm the harvest bit settings are correct on all these listed ASICs. Once confirmed, I will drop the redundant check.

Regards,
Guchun

-----Original Message-----
From: Alex Deucher <alexdeucher@gmail.com> 
Sent: Thursday, February 24, 2022 11:58 PM
To: Chen, Guchun <Guchun.Chen@amd.com>
Cc: amd-gfx list <amd-gfx@lists.freedesktop.org>; Deucher, Alexander <Alexander.Deucher@amd.com>; Zhang, Hawking <Hawking.Zhang@amd.com>; Koenig, Christian <Christian.Koenig@amd.com>; Pan, Xinhui <Xinhui.Pan@amd.com>
Subject: Re: [PATCH] drm/amdgpu: limit harvest bit read on several ASICs

On Tue, Feb 22, 2022 at 10:07 AM Guchun Chen <guchun.chen@amd.com> wrote:
>
> Due to faulty VBIOS out there, harvest bit setting is not consistently 
> correct especially for display IP. So far, it's hard to work out a 
> solution on all the legacy Navi1x ASICs in a short time, so to avoid 
> regression, limit harvest bit read on several ASICs. Will revisit 
> later once VBIOS has corrected it in long term.
>
> Fixes: b3f4ea887d5f("drm/amdgpu: read harvest bit per IP data on 
> legacy GPUs")
> Signed-off-by: Guchun Chen <guchun.chen@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c | 18 +++++++++++++-----
>  1 file changed, 13 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
> index 11255290f117..2e0ff1ace6fc 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
> @@ -1129,12 +1129,20 @@ void amdgpu_discovery_harvest_ip(struct amdgpu_device *adev)
>          * so read harvest bit per IP data structure to set
>          * harvest configuration.
>          */
> -       if (adev->ip_versions[GC_HWIP][0] < IP_VERSION(10, 2, 0))
> -               amdgpu_discovery_read_harvest_bit_per_ip(adev,
> -                                                       &vcn_harvest_count);
> -       else
> +       if (adev->ip_versions[GC_HWIP][0] < IP_VERSION(10, 2, 0)) {
> +               if ((adev->pdev->device == 0x731E &&
> +                       (adev->pdev->revision == 0xC6 ||
> +                        adev->pdev->revision == 0xC7)) ||
> +                       (adev->pdev->device == 0x7340 &&
> +                        adev->pdev->revision == 0xC9) ||
> +                       (adev->pdev->device == 0x7360 &&
> +                        adev->pdev->revision == 0xC7))
> +                       amdgpu_discovery_read_harvest_bit_per_ip(adev,
> +                               &vcn_harvest_count);

Now that we have this code in place, can we drop the hardcoded harvest settings below?  E.g.,

        if ((adev->pdev->device == 0x731E &&
             (adev->pdev->revision == 0xC6 || adev->pdev->revision == 0xC7)) ||
            (adev->pdev->device == 0x7340 && adev->pdev->revision == 0xC9)  ||
            (adev->pdev->device == 0x7360 && adev->pdev->revision == 0xC7)) {
                adev->harvest_ip_mask |= AMD_HARVEST_IP_VCN_MASK;
                adev->harvest_ip_mask |= AMD_HARVEST_IP_JPEG_MASK;
        }

Looks like the same board variants in both places.

Alex

> +       } else {
>                 amdgpu_disocvery_read_from_harvest_table(adev,
> -                                                       &vcn_harvest_count);
> +                       &vcn_harvest_count);
> +       }
>
>         amdgpu_discovery_harvest_config_quirk(adev);
>
> --
> 2.17.1
>