[PATCH 1/2] drm/amdgpu: Move HDP remapping earlier during init
Christian König
ckoenig.leichtzumerken at gmail.com
Thu Aug 25 14:53:27 UTC 2022
Am 25.08.22 um 16:26 schrieb Alex Deucher:
> On Thu, Aug 25, 2022 at 10:22 AM Lazar, Lijo <lijo.lazar at amd.com> wrote:
>>
>>
>> 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?
> 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.
Correct me if I'm wrong but I think remap means it is available at an
additional location, the privileged region still works as well.
So Lijo suggestion is to use the privileged area in the kernel instead
of the remapped one.
Right?
Christian.
>
> 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
>>>>
More information about the amd-gfx
mailing list