Re: [PATCH] drm/amdgpu: Apply flags after amdgpu_device_ip_init()
at 21:33, Deucher, Alexander wrote: Thanks for finding this! I think the attached patch should fix the issue and it's much less invasive. Yes it also fix the issue, please add by tested-by: Tested-by: Kai-Heng Feng I took this more or less future proof approach because I think this won’t be the last chip that needs firmware information, which isn’t available in early init, to decides its flags. Yes it’s intrusive to carve out all flags from early init callbacks, but I don’t think it’s that ugly. Kai-Heng Alex From: Kai-Heng Feng Sent: Thursday, August 15, 2019 1:11 AM To: Deucher, Alexander ; Koenig, Christian ; Zhou, David(ChunMing) Cc: Huang, Ray ; anthony.w...@canonical.com ; amd-...@lists.freedesktop.org ; dri-devel@lists.freedesktop.org ; linux-ker...@vger.kernel.org ; Kai-Heng Feng Subject: [PATCH] drm/amdgpu: Apply flags after amdgpu_device_ip_init() After commit 005440066f92 ("drm/amdgpu: enable gfxoff again on raven series (v2)"), some Raven Ridge systems start to have rendering corruption in browser [1]. Chip specific flags like cg_flags and pg_flags are applied in amdgpu_device_ip_early_init(). For Raven Ridge, the flags may depend on pp_feature's PP_GFXOFF_MASK bit, which can be negated in amdgpu_device_ip_init() based on firmware information. At that time it's already too late, since cg_flags and pg_flags are already set. Apply flags after amdgpu_device_ip_init() and consolidate all flags to one place, to solve the issue. [1] https://lore.kernel.org/lkml/3eb0e920-31d7-4c91-a360-dbfb4417a...@canonical.com/ Fixes: 005440066f92 ("drm/amdgpu: enable gfxoff again on raven series (v2)") Signed-off-by: Kai-Heng Feng --- drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 589 + drivers/gpu/drm/amd/amdgpu/cik.c | 87 --- drivers/gpu/drm/amd/amdgpu/nv.c| 50 -- drivers/gpu/drm/amd/amdgpu/si.c| 83 --- drivers/gpu/drm/amd/amdgpu/soc15.c | 140 - drivers/gpu/drm/amd/amdgpu/vi.c| 162 -- 6 files changed, 589 insertions(+), 522 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c index 275277364a8a..10ea4899c338 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c @@ -1852,6 +1852,591 @@ static int amdgpu_device_ip_init(struct amdgpu_device *adev) return r; } +#define CZ_REV_BRISTOL(rev) \ + ((rev >= 0xC8 && rev <= 0xCE) || (rev >= 0xE1 && rev <= 0xE6)) + +static int amdgpu_device_apply_flags(struct amdgpu_device *adev) +{ + switch (adev->asic_type) { + case CHIP_TAHITI: + adev->cg_flags = + AMD_CG_SUPPORT_GFX_MGCG | + AMD_CG_SUPPORT_GFX_MGLS | + /*AMD_CG_SUPPORT_GFX_CGCG |*/ + AMD_CG_SUPPORT_GFX_CGLS | + AMD_CG_SUPPORT_GFX_CGTS | + AMD_CG_SUPPORT_GFX_CP_LS | + AMD_CG_SUPPORT_MC_MGCG | + AMD_CG_SUPPORT_SDMA_MGCG | + AMD_CG_SUPPORT_BIF_LS | + AMD_CG_SUPPORT_VCE_MGCG | + AMD_CG_SUPPORT_UVD_MGCG | + AMD_CG_SUPPORT_HDP_LS | + AMD_CG_SUPPORT_HDP_MGCG; + adev->pg_flags = 0; + break; + case CHIP_PITCAIRN: + adev->cg_flags = + AMD_CG_SUPPORT_GFX_MGCG | + AMD_CG_SUPPORT_GFX_MGLS | + /*AMD_CG_SUPPORT_GFX_CGCG |*/ + AMD_CG_SUPPORT_GFX_CGLS | + AMD_CG_SUPPORT_GFX_CGTS | + AMD_CG_SUPPORT_GFX_CP_LS | + AMD_CG_SUPPORT_GFX_RLC_LS | + AMD_CG_SUPPORT_MC_LS | + AMD_CG_SUPPORT_MC_MGCG | + AMD_CG_SUPPORT_SDMA_MGCG | + AMD_CG_SUPPORT_BIF_LS | + AMD_CG_SUPPORT_VCE_MGCG | + AMD_CG_SUPPORT_UVD_MGCG | + AMD_CG_SUPPORT_HDP_LS | + AMD_CG_SUPPORT_HDP_MGCG; + adev->pg_flags = 0; + break; + case CHIP_VERDE: + adev->cg_flags = + AMD_CG_SUPPORT_GFX_MGCG | + AMD_CG_SUPPORT_GFX_MGLS | + AMD_CG_SUPPORT_GFX_CGLS | + AMD_CG_SUPPORT_GFX_CGTS | + AMD_CG_SUPPORT_GFX_CGTS_LS | + AMD_CG_SUPPORT_GFX_CP_LS | + AMD_CG_SUPPORT_MC_LS | + AMD_CG_SUPPORT_MC_MGCG | + AMD_CG_SUPPORT_SDMA_MGCG | + AMD_CG_SUPPORT_SDMA_LS | +
Re: [PATCH] drm/amdgpu: Apply flags after amdgpu_device_ip_init()
On Thu, Aug 15, 2019 at 10:59 AM Kai-Heng Feng wrote: > > at 21:33, Deucher, Alexander wrote: > > > Thanks for finding this! I think the attached patch should fix the issue > > and it's much less invasive. > > Yes it also fix the issue, please add by tested-by: > Tested-by: Kai-Heng Feng > Thanks! > I took this more or less future proof approach because I think this won’t > be the last chip that needs firmware information, which isn’t available in > early init, to decides its flags. > > Yes it’s intrusive to carve out all flags from early init callbacks, but I > don’t think it’s that ugly. Not a bad approach, but I'd prefer to keep the power and clock gating flags in the asic specifc code rather than in the common code. Alex ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/amdgpu: Apply flags after amdgpu_device_ip_init()
After commit 005440066f92 ("drm/amdgpu: enable gfxoff again on raven series (v2)"), some Raven Ridge systems start to have rendering corruption in browser [1]. Chip specific flags like cg_flags and pg_flags are applied in amdgpu_device_ip_early_init(). For Raven Ridge, the flags may depend on pp_feature's PP_GFXOFF_MASK bit, which can be negated in amdgpu_device_ip_init() based on firmware information. At that time it's already too late, since cg_flags and pg_flags are already set. Apply flags after amdgpu_device_ip_init() and consolidate all flags to one place, to solve the issue. [1] https://lore.kernel.org/lkml/3eb0e920-31d7-4c91-a360-dbfb4417a...@canonical.com/ Fixes: 005440066f92 ("drm/amdgpu: enable gfxoff again on raven series (v2)") Signed-off-by: Kai-Heng Feng --- drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 589 + drivers/gpu/drm/amd/amdgpu/cik.c | 87 --- drivers/gpu/drm/amd/amdgpu/nv.c| 50 -- drivers/gpu/drm/amd/amdgpu/si.c| 83 --- drivers/gpu/drm/amd/amdgpu/soc15.c | 140 - drivers/gpu/drm/amd/amdgpu/vi.c| 162 -- 6 files changed, 589 insertions(+), 522 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c index 275277364a8a..10ea4899c338 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c @@ -1852,6 +1852,591 @@ static int amdgpu_device_ip_init(struct amdgpu_device *adev) return r; } +#define CZ_REV_BRISTOL(rev) \ + ((rev >= 0xC8 && rev <= 0xCE) || (rev >= 0xE1 && rev <= 0xE6)) + +static int amdgpu_device_apply_flags(struct amdgpu_device *adev) +{ + switch (adev->asic_type) { + case CHIP_TAHITI: + adev->cg_flags = + AMD_CG_SUPPORT_GFX_MGCG | + AMD_CG_SUPPORT_GFX_MGLS | + /*AMD_CG_SUPPORT_GFX_CGCG |*/ + AMD_CG_SUPPORT_GFX_CGLS | + AMD_CG_SUPPORT_GFX_CGTS | + AMD_CG_SUPPORT_GFX_CP_LS | + AMD_CG_SUPPORT_MC_MGCG | + AMD_CG_SUPPORT_SDMA_MGCG | + AMD_CG_SUPPORT_BIF_LS | + AMD_CG_SUPPORT_VCE_MGCG | + AMD_CG_SUPPORT_UVD_MGCG | + AMD_CG_SUPPORT_HDP_LS | + AMD_CG_SUPPORT_HDP_MGCG; + adev->pg_flags = 0; + break; + case CHIP_PITCAIRN: + adev->cg_flags = + AMD_CG_SUPPORT_GFX_MGCG | + AMD_CG_SUPPORT_GFX_MGLS | + /*AMD_CG_SUPPORT_GFX_CGCG |*/ + AMD_CG_SUPPORT_GFX_CGLS | + AMD_CG_SUPPORT_GFX_CGTS | + AMD_CG_SUPPORT_GFX_CP_LS | + AMD_CG_SUPPORT_GFX_RLC_LS | + AMD_CG_SUPPORT_MC_LS | + AMD_CG_SUPPORT_MC_MGCG | + AMD_CG_SUPPORT_SDMA_MGCG | + AMD_CG_SUPPORT_BIF_LS | + AMD_CG_SUPPORT_VCE_MGCG | + AMD_CG_SUPPORT_UVD_MGCG | + AMD_CG_SUPPORT_HDP_LS | + AMD_CG_SUPPORT_HDP_MGCG; + adev->pg_flags = 0; + break; + case CHIP_VERDE: + adev->cg_flags = + AMD_CG_SUPPORT_GFX_MGCG | + AMD_CG_SUPPORT_GFX_MGLS | + AMD_CG_SUPPORT_GFX_CGLS | + AMD_CG_SUPPORT_GFX_CGTS | + AMD_CG_SUPPORT_GFX_CGTS_LS | + AMD_CG_SUPPORT_GFX_CP_LS | + AMD_CG_SUPPORT_MC_LS | + AMD_CG_SUPPORT_MC_MGCG | + AMD_CG_SUPPORT_SDMA_MGCG | + AMD_CG_SUPPORT_SDMA_LS | + AMD_CG_SUPPORT_BIF_LS | + AMD_CG_SUPPORT_VCE_MGCG | + AMD_CG_SUPPORT_UVD_MGCG | + AMD_CG_SUPPORT_HDP_LS | + AMD_CG_SUPPORT_HDP_MGCG; + adev->pg_flags = 0; + //??? + break; + case CHIP_OLAND: + adev->cg_flags = + AMD_CG_SUPPORT_GFX_MGCG | + AMD_CG_SUPPORT_GFX_MGLS | + /*AMD_CG_SUPPORT_GFX_CGCG |*/ + AMD_CG_SUPPORT_GFX_CGLS | + AMD_CG_SUPPORT_GFX_CGTS | + AMD_CG_SUPPORT_GFX_CP_LS | + AMD_CG_SUPPORT_GFX_RLC_LS | + AMD_CG_SUPPORT_MC_LS | + AMD_CG_SUPPORT_MC_MGCG | + AMD_CG_SUPPORT_SDMA_MGCG | + AMD_CG_SUPPORT_BIF_LS | + AMD_CG_SUPPORT_UVD_MGCG | + AMD_CG_SUPPORT_HDP_LS | +