Re: [PATCH] drm/amdgpu: Enable doorbell selfring if resize BAR successfully

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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?

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





[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux