[PATCH 1/2] drm/amdgpu: Move HDP remapping earlier during init

[Lazar, Lijo]

On 8/25/2022 7:37 PM, Alex Deucher wrote:
> On Thu, Aug 25, 2022 at 4:58 AM Lijo Lazar <lijo.lazar at amd.com> 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 at gmail.com>
>> Signed-off-by: Lijo Lazar <lijo.lazar at amd.com>
>> ---
>>   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?

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
>>