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 <alexdeuc...@gmail.com> Sent: Thursday, February 24, 2022 11:58 PM To: Chen, Guchun <guchun.c...@amd.com> Cc: amd-gfx list <amd-gfx@lists.freedesktop.org>; Deucher, Alexander <alexander.deuc...@amd.com>; Zhang, Hawking <hawking.zh...@amd.com>; Koenig, Christian <christian.koe...@amd.com>; Pan, Xinhui <xinhui....@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.c...@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.c...@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 >