On Mon, Aug 29, 2022 at 4:18 AM Lijo Lazar 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 Unsupported Request error reported through AER during > driver load. The error happens as a write happens to the remap offset > before real remapping is done. > > Link: https://bugzilla.kernel.org/show_bug.cgi?id=216373 > > The error was unnoticed before and got visible because of the commit > referenced below. This doesn't fix anything in the commit below, rather > fixes the issue in amdgpu exposed by the commit. The reference is only > to associate this commit with below one so that both go together. > > Fixes: 8795e182b02d ("PCI/portdrv: Don't disable AER reporting in get_port_device_capability()") > > Reported-by: Tom Seewald > Signed-off-by: Lijo Lazar > Cc: stable@vger.kernel.org How about something like the attached patch rather than these two patches? It's a bit bigger but seems cleaner and more defensive in my opinion. Alex > --- > v2: > Take care of IP resume cases (Alex Deucher) > Add NULL check to nbio.funcs to cover older (GFXv8) ASICs (Felix Kuehling) > Add more details in commit message and associate with AER patch (Bjorn > Helgaas) > > drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 24 ++++++++++++++++++++++ > 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, 24 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..e420118769a5 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > @@ -2334,6 +2334,26 @@ static int amdgpu_device_init_schedulers(struct amdgpu_device *adev) > return 0; > } > > +/** > + * amdgpu_device_prepare_ip - prepare IPs for hardware initialization > + * > + * @adev: amdgpu_device pointer > + * > + * Any common hardware initialization sequence that needs to be done before > + * hw init of individual IPs is performed here. This is different from the > + * 'common block' which initializes a set of IPs. > + */ > +static void amdgpu_device_prepare_ip(struct amdgpu_device *adev) > +{ > + /* Remap HDP registers to a hole in mmio space, for the purpose > + * of exposing those registers to process space. This needs to be > + * done before hw init of ip blocks to take care of HDP flush > + * operations through registers during hw_init. > + */ > + if (adev->nbio.funcs && adev->nbio.funcs->remap_hdp_registers && > + !amdgpu_sriov_vf(adev)) > + adev->nbio.funcs->remap_hdp_registers(adev); > +} > > /** > * amdgpu_device_ip_init - run init for hardware IPs > @@ -2376,6 +2396,8 @@ static int amdgpu_device_ip_init(struct amdgpu_device *adev) > DRM_ERROR("amdgpu_vram_scratch_init failed %d\n", r); > goto init_failed; > } > + > + amdgpu_device_prepare_ip(adev); > r = adev->ip_blocks[i].version->funcs->hw_init((void *)adev); > if (r) { > DRM_ERROR("hw_init %d failed %d\n", i, r); > @@ -3058,6 +3080,7 @@ static int amdgpu_device_ip_reinit_early_sriov(struct amdgpu_device *adev) > AMD_IP_BLOCK_TYPE_IH, > }; > > + amdgpu_device_prepare_ip(adev); > for (i = 0; i < adev->num_ip_blocks; i++) { > int j; > struct amdgpu_ip_block *block; > @@ -3139,6 +3162,7 @@ static int amdgpu_device_ip_resume_phase1(struct amdgpu_device *adev) > { > int i, r; > > + amdgpu_device_prepare_ip(adev); > for (i = 0; i < adev->num_ip_blocks; i++) { > if (!adev->ip_blocks[i].status.valid || adev->ip_blocks[i].status.hw) > continue; > 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 >