Am 25.04.23 um 05:29 schrieb Xiao, Shane:
[AMD Official Use Only - General]
-----Original Message-----
From: Liu, Aaron <Aaron.Liu@xxxxxxx>
Sent: Tuesday, April 25, 2023 9:14 AM
To: Alex Deucher <alexdeucher@xxxxxxxxx>; Koenig, Christian
<Christian.Koenig@xxxxxxx>
Cc: Xiao, Shane <shane.xiao@xxxxxxx>; Christian König
<ckoenig.leichtzumerken@xxxxxxxxx>; amd-gfx@xxxxxxxxxxxxxxxxxxxxx;
Deucher, Alexander <Alexander.Deucher@xxxxxxx>; Zhang, Hawking
<Hawking.Zhang@xxxxxxx>; Kuehling, Felix <Felix.Kuehling@xxxxxxx>; Hou,
Xiaomeng (Matthew) <Xiaomeng.Hou@xxxxxxx>
Subject: RE: [PATCH] drm/amdgpu: Enable doorbell selfring if resize BAR
successfully
[AMD Official Use Only - General]
-----Original Message-----
From: Alex Deucher <alexdeucher@xxxxxxxxx>
Sent: Tuesday, April 25, 2023 4:20 AM
To: Koenig, Christian <Christian.Koenig@xxxxxxx>
Cc: Xiao, Shane <shane.xiao@xxxxxxx>; Christian König
<ckoenig.leichtzumerken@xxxxxxxxx>; amd-gfx@xxxxxxxxxxxxxxxxxxxxx;
Deucher, Alexander <Alexander.Deucher@xxxxxxx>; Zhang, Hawking
<Hawking.Zhang@xxxxxxx>; Kuehling, Felix <Felix.Kuehling@xxxxxxx>;
Hou, Xiaomeng (Matthew) <Xiaomeng.Hou@xxxxxxx>; Liu, Aaron
<Aaron.Liu@xxxxxxx>
Subject: Re: [PATCH] drm/amdgpu: Enable doorbell selfring if resize
BAR successfully
On Mon, Apr 24, 2023 at 3:11 PM Christian König
<christian.koenig@xxxxxxx>
wrote:
Am 24.04.23 um 16:06 schrieb Xiao, Shane:
[AMD Official Use Only - General]
-----Original Message-----
From: Xiao, Shane
Sent: Monday, April 24, 2023 6:31 PM
To: Christian König <ckoenig.leichtzumerken@xxxxxxxxx>; amd-
gfx@xxxxxxxxxxxxxxxxxxxxx; Deucher, Alexander
<Alexander.Deucher@xxxxxxx>; Zhang, Hawking
<Hawking.Zhang@xxxxxxx>; Kuehling, Felix <Felix.Kuehling@xxxxxxx>
Cc: Hou, Xiaomeng (Matthew) <Xiaomeng.Hou@xxxxxxx>; Liu, Aaron
<Aaron.Liu@xxxxxxx>
Subject: RE: [PATCH] drm/amdgpu: Enable doorbell selfring if
resize BAR successfully
[AMD Official Use Only - General]
-----Original Message-----
From: Christian König <ckoenig.leichtzumerken@xxxxxxxxx>
Sent: Monday, April 24, 2023 5:07 PM
To: Xiao, Shane <shane.xiao@xxxxxxx>;
amd-gfx@xxxxxxxxxxxxxxxxxxxxx; Deucher, Alexander
<Alexander.Deucher@xxxxxxx>; Zhang, Hawking
<Hawking.Zhang@xxxxxxx>; Kuehling, Felix
<Felix.Kuehling@xxxxxxx>
Cc: Hou, Xiaomeng (Matthew) <Xiaomeng.Hou@xxxxxxx>; Liu, Aaron
<Aaron.Liu@xxxxxxx>
Subject: Re: [PATCH] drm/amdgpu: Enable doorbell selfring if
resize BAR successfully
Am 18.04.23 um 08:54 schrieb Shane Xiao:
[Why]
The selfring doorbell aperture will change when we resize FB
BAR successfully during gmc sw init, we should reorder the
sequence of enabling doorbell selfring aperture.
That's a good catch.
[How]
Move enable_doorbell_selfring_aperture from *_common_hw_init to
*_common_late_init.
But that sounds like a bad idea. Instead the full call to
nv_enable_doorbell_aperture() should be moved around.
Hi Christian,
Yes, I get your idea. But as far as I can understand that, the
gfx hw init will use doorbell.
If so, we cannot enable doorbell after gfx hw init.
We have come up with two ways to resolve the issue.
1) Separate enable_doorbell_aperture and
enable_doorbell_selfring_aperture. However, the
enable_doorbell_selfring_aperture should be moved in
*_common_ip_funcs-
late_init.
I'm not an expert for this part of the driver, but of hand that
sounds like the right way of doing it.
Alex any objections?
Yeah, seems reasonable.
Alex
enable_doorbell_aperture and enable_doorbell_selfring_aperture should be in
common_*_init instead of gmc_hw_init.
The order of execution of Shane's 1st way is :
1) common_sw_init
2) common_hw_init -> enable_doorbell_aperture
3) gmc_sw_init -> amdgpu_device_resize_fb_bar ///This relies
gmc.real_vram_size to determine resize_fb_bar, so moving
amdgpu_device_resize_fb_bar to common_sw_init is not a good idea.
4) gmc_hw_init
5) common_late_init -> enable_doorbell_selfring_aperture
The 1st way looks good to me and reviewed-by me.
Hi Alex & Christian,
Since this patch has already implemented it in this way, is there any other comments on this patch body?
Or can I add you R-B or Acked-by on this patch?
At least remove the functions
soc15_enable_doorbell_aperture()/nv_enable_doorbell_aperture()/soc21_enable_doorbell_aperture()
and open code the respective calls.
Those don't make sense any more since we don't have a central point when
the apertures are enabled/disabled.
Regards,
Christian.
Best Regards,
Shane
Regards,
Christian.
2) The full call can be moved to gmc hw init. But it seems
strange to move
nbio configuration into gmc hw init.
If neither of the above methods is suitable, could you please give
us some
advice on this issue?
Best Regards,
Shane
Best Regards,
Shane
Regards,
Christian.
This fixes the potential issue that GPU ring its own doorbell
when this device is in translated mode with iommu is on.
Signed-off-by: Shane Xiao <shane.xiao@xxxxxxx>
Signed-off-by: Aaron Liu <aaron.liu@xxxxxxx>
Tested-by: Xiaomeng Hou <Xiaomeng.Hou@xxxxxxx>
---
drivers/gpu/drm/amd/amdgpu/nv.c | 4 +++-
drivers/gpu/drm/amd/amdgpu/soc15.c | 4 +++-
drivers/gpu/drm/amd/amdgpu/soc21.c | 4 +++-
3 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/nv.c
b/drivers/gpu/drm/amd/amdgpu/nv.c index
47420b403871..f4c85634a4c8
100644
--- a/drivers/gpu/drm/amd/amdgpu/nv.c
+++ b/drivers/gpu/drm/amd/amdgpu/nv.c
@@ -535,7 +535,8 @@ static void
nv_enable_doorbell_aperture(struct
amdgpu_device *adev,
bool enable)
{
adev->nbio.funcs->enable_doorbell_aperture(adev,
enable);
- adev->nbio.funcs->enable_doorbell_selfring_aperture(adev,
enable);
+ if (!enable)
+
+ adev->nbio.funcs->enable_doorbell_selfring_aperture(adev,
false);
}
const struct amdgpu_ip_block_version nv_common_ip_block = @@
-999,6
+1000,7 @@ static int nv_common_late_init(void *handle)
}
}
+ adev->nbio.funcs->enable_doorbell_selfring_aperture(adev,
+ true);
return 0;
}
diff --git a/drivers/gpu/drm/amd/amdgpu/soc15.c
b/drivers/gpu/drm/amd/amdgpu/soc15.c
index bc5dd80f10c1..0202de79a389 100644
--- a/drivers/gpu/drm/amd/amdgpu/soc15.c
+++ b/drivers/gpu/drm/amd/amdgpu/soc15.c
@@ -623,7 +623,8 @@ static void
soc15_enable_doorbell_aperture(struct
amdgpu_device *adev,
bool enable)
{
adev->nbio.funcs->enable_doorbell_aperture(adev,
enable);
- adev->nbio.funcs->enable_doorbell_selfring_aperture(adev,
enable);
+ if (!enable)
+
+ adev->nbio.funcs->enable_doorbell_selfring_aperture(adev,
false);
}
const struct amdgpu_ip_block_version vega10_common_ip_block
= @@
-1125,6 +1126,7 @@ static int soc15_common_late_init(void *handle)
if (amdgpu_sriov_vf(adev))
xgpu_ai_mailbox_get_irq(adev);
+ adev->nbio.funcs->enable_doorbell_selfring_aperture(adev,
+ true);
return 0;
}
diff --git a/drivers/gpu/drm/amd/amdgpu/soc21.c
b/drivers/gpu/drm/amd/amdgpu/soc21.c
index 514bfc705d5a..cd4619085d67 100644
--- a/drivers/gpu/drm/amd/amdgpu/soc21.c
+++ b/drivers/gpu/drm/amd/amdgpu/soc21.c
@@ -454,7 +454,8 @@ static void
soc21_enable_doorbell_aperture(struct
amdgpu_device *adev,
bool enable)
{
adev->nbio.funcs->enable_doorbell_aperture(adev,
enable);
- adev->nbio.funcs->enable_doorbell_selfring_aperture(adev,
enable);
+ if (!enable)
+
+ adev->nbio.funcs->enable_doorbell_selfring_aperture(adev,
false);
}
const struct amdgpu_ip_block_version soc21_common_ip_block =
@@
-764,6 +765,7 @@ static int soc21_common_late_init(void *handle)
amdgpu_irq_get(adev, &adev-
nbio.ras_err_event_athub_irq, 0);
}
+ adev->nbio.funcs->enable_doorbell_selfring_aperture(adev,
+ true);
return 0;
}
