On Thu, Aug 25, 2022 at 10:22 AM Lazar, Lijo <lijo.lazar@xxxxxxx> wrote: > > > > On 8/25/2022 7:37 PM, Alex Deucher wrote: > > On Thu, Aug 25, 2022 at 4:58 AM Lijo Lazar <lijo.lazar@xxxxxxx> wrote: > >> > >> HDP flush is used early in the init sequence as part of memory controller > >> block initialization. Hence remapping of HDP registers needed for flush > >> needs to happen earlier. > >> > >> This also fixes the AER error reported as Unsupported Request during > >> driver load. > >> > >> Link: https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.kernel.org%2Fshow_bug.cgi%3Fid%3D216373&data=05%7C01%7Clijo.lazar%40amd.com%7Caeec5a5e8ec7402e546708da86a31e41%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637970332414985963%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=EQuUjHTaVPSKZdCUhL6iI4EJ56UMhKTLl86uVpSL8AU%3D&reserved=0 > >> > >> Reported-by: Tom Seewald <tseewald@xxxxxxxxx> > >> Signed-off-by: Lijo Lazar <lijo.lazar@xxxxxxx> > >> --- > >> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 9 +++++++++ > >> drivers/gpu/drm/amd/amdgpu/nv.c | 6 ------ > >> drivers/gpu/drm/amd/amdgpu/soc15.c | 6 ------ > >> drivers/gpu/drm/amd/amdgpu/soc21.c | 6 ------ > >> 4 files changed, 9 insertions(+), 18 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > >> index ce7d117efdb5..53d753e94a71 100644 > >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > >> @@ -2376,6 +2376,15 @@ static int amdgpu_device_ip_init(struct amdgpu_device *adev) > >> DRM_ERROR("amdgpu_vram_scratch_init failed %d\n", r); > >> goto init_failed; > >> } > >> + > >> + /* remap HDP registers to a hole in mmio space, > >> + * for the purpose of expose those registers > >> + * to process space. This is needed for any early HDP > >> + * flush operation during gmc initialization. > >> + */ > >> + if (adev->nbio.funcs->remap_hdp_registers && !amdgpu_sriov_vf(adev)) > >> + adev->nbio.funcs->remap_hdp_registers(adev); > >> + > > > > We probably also need this in ip_resume() as well to handle the > > suspend and resume case. Thinking about this more, maybe it's easier > > to just track whether the remap has happened yet and use the old or > > new offset based on that. > > If we can use the default offset without a remap, does it make sense to > remap? What about calling the same in ip_resume? The remap is necessary so that userspace drivers can access this to flush the HDP registers when they need to since normally it's in a non-accessible region of the MMIO space. I'm fine with updating it in ip_resume as well. Alex > > Thanks, > Lijo > > > > > Alex > > > > > >> r = adev->ip_blocks[i].version->funcs->hw_init((void *)adev); > >> if (r) { > >> DRM_ERROR("hw_init %d failed %d\n", i, r); > >> diff --git a/drivers/gpu/drm/amd/amdgpu/nv.c b/drivers/gpu/drm/amd/amdgpu/nv.c > >> index b3fba8dea63c..3ac7fef74277 100644 > >> --- a/drivers/gpu/drm/amd/amdgpu/nv.c > >> +++ b/drivers/gpu/drm/amd/amdgpu/nv.c > >> @@ -1032,12 +1032,6 @@ static int nv_common_hw_init(void *handle) > >> nv_program_aspm(adev); > >> /* setup nbio registers */ > >> adev->nbio.funcs->init_registers(adev); > >> - /* remap HDP registers to a hole in mmio space, > >> - * for the purpose of expose those registers > >> - * to process space > >> - */ > >> - if (adev->nbio.funcs->remap_hdp_registers && !amdgpu_sriov_vf(adev)) > >> - adev->nbio.funcs->remap_hdp_registers(adev); > >> /* enable the doorbell aperture */ > >> nv_enable_doorbell_aperture(adev, true); > >> > >> diff --git a/drivers/gpu/drm/amd/amdgpu/soc15.c b/drivers/gpu/drm/amd/amdgpu/soc15.c > >> index fde6154f2009..a0481e37d7cf 100644 > >> --- a/drivers/gpu/drm/amd/amdgpu/soc15.c > >> +++ b/drivers/gpu/drm/amd/amdgpu/soc15.c > >> @@ -1240,12 +1240,6 @@ static int soc15_common_hw_init(void *handle) > >> soc15_program_aspm(adev); > >> /* setup nbio registers */ > >> adev->nbio.funcs->init_registers(adev); > >> - /* remap HDP registers to a hole in mmio space, > >> - * for the purpose of expose those registers > >> - * to process space > >> - */ > >> - if (adev->nbio.funcs->remap_hdp_registers && !amdgpu_sriov_vf(adev)) > >> - adev->nbio.funcs->remap_hdp_registers(adev); > >> > >> /* enable the doorbell aperture */ > >> soc15_enable_doorbell_aperture(adev, true); > >> diff --git a/drivers/gpu/drm/amd/amdgpu/soc21.c b/drivers/gpu/drm/amd/amdgpu/soc21.c > >> index 55284b24f113..16b447055102 100644 > >> --- a/drivers/gpu/drm/amd/amdgpu/soc21.c > >> +++ b/drivers/gpu/drm/amd/amdgpu/soc21.c > >> @@ -660,12 +660,6 @@ static int soc21_common_hw_init(void *handle) > >> soc21_program_aspm(adev); > >> /* setup nbio registers */ > >> adev->nbio.funcs->init_registers(adev); > >> - /* remap HDP registers to a hole in mmio space, > >> - * for the purpose of expose those registers > >> - * to process space > >> - */ > >> - if (adev->nbio.funcs->remap_hdp_registers) > >> - adev->nbio.funcs->remap_hdp_registers(adev); > >> /* enable the doorbell aperture */ > >> soc21_enable_doorbell_aperture(adev, true); > >> > >> -- > >> 2.25.1 > >>
