[PATCH] drm/amdgpu/sriov: omit fbcon error under sriov or passthrough
Koenig, Christian
Christian.Koenig at amd.com
Thu Sep 19 09:06:20 UTC 2019
I can create a patch based on this today and push it on Monday.
Christian.
Am 19.09.2019 11:05 schrieb "Deng, Emily" <Emily.Deng at amd.com>:
Hi Christian,
Could you please help to push the code?
Best wishes
Emily Deng
From: Zhang, Jack (Jian) <Jack.Zhang1 at amd.com>
Sent: Thursday, September 19, 2019 11:33 AM
To: Deng, Emily <Emily.Deng at amd.com>; Koenig, Christian <Christian.Koenig at amd.com>
Cc: amd-gfx at lists.freedesktop.org; Teng, Rui <Rui.Teng at amd.com>; Cui, Flora <Flora.Cui at amd.com>
Subject: RE: [PATCH] drm/amdgpu/sriov: omit fbcon error under sriov or passthrough
Reviewed & Tested-by: Jack Zhang <Jack.Zhang1 at amd.com<mailto:Jack.Zhang1 at amd.com>>
BR,
Jack
From: Deng, Emily <Emily.Deng at amd.com<mailto:Emily.Deng at amd.com>>
Sent: Thursday, September 19, 2019 10:58 AM
To: Koenig, Christian <Christian.Koenig at amd.com<mailto:Christian.Koenig at amd.com>>
Cc: Zhang, Jack (Jian) <Jack.Zhang1 at amd.com<mailto:Jack.Zhang1 at amd.com>>; amd-gfx at lists.freedesktop.org<mailto:amd-gfx at lists.freedesktop.org>; Teng, Rui <Rui.Teng at amd.com<mailto:Rui.Teng at amd.com>>; Cui, Flora <Flora.Cui at amd.com<mailto:Flora.Cui at amd.com>>
Subject: RE: [PATCH] drm/amdgpu/sriov: omit fbcon error under sriov or passthrough
Hi Jack,
Could you please give a try about this? Both for passthrough and sriov.
Best wishes
Emily Deng
From: Koenig, Christian <Christian.Koenig at amd.com<mailto:Christian.Koenig at amd.com>>
Sent: Wednesday, September 18, 2019 6:47 PM
To: Deng, Emily <Emily.Deng at amd.com<mailto:Emily.Deng at amd.com>>
Cc: Zhang, Jack (Jian) <Jack.Zhang1 at amd.com<mailto:Jack.Zhang1 at amd.com>>; amd-gfx at lists.freedesktop.org<mailto:amd-gfx at lists.freedesktop.org>; Teng, Rui <Rui.Teng at amd.com<mailto:Rui.Teng at amd.com>>; Cui, Flora <Flora.Cui at amd.com<mailto:Flora.Cui at amd.com>>
Subject: Re: [PATCH] drm/amdgpu/sriov: omit fbcon error under sriov or passthrough
Hi Jack & Emily,
asking around a bit helped, we should be able to take a look at the module state to distinct the two use cases like this:
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 6b96a5738e57..0af134eb03e2 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -1074,7 +1074,10 @@ amdgpu_pci_remove(struct pci_dev *pdev)
{
struct drm_device *dev = pci_get_drvdata(pdev);
- DRM_ERROR("Device removal is currently not supported outside of fbcon\n");
+#ifdef MODULE
+ if (THIS_MODULE->state != MODULE_STATE_GOING)
+#endif
+ DRM_ERROR("Device removal is currently not supported outside of fbcon\n");
drm_dev_unplug(dev);
drm_dev_put(dev);
pci_disable_device(pdev);
It's a bit of a hack, but I think that this should work.
Regards,
Christian.
Am 18.09.19 um 12:29 schrieb Christian König:
Hi Emily,
Do you think this is because the wrong use case?
Well Jack's use case is correct, but the PCIe hot plug removal use case is incorrect.
Changing it to a warning is most likely not a good idea either because it is indeed an error to try to use PCIe hot plug removal.
What we need to distinct is why the function is called, if it's because of pci_unregister_driver(&amdgpu_kms_pci_driver) in amdgpu_exit() then the use case is valid and we should not print the error.
But if it's because somebody does something like "echo 1 > /sys/bus/pci/devices/0000\:0b\:00.1/remove" then that is invalid and we should print it.
We could do some hack and look at the stack trace, but that is probably not reliable either.
Maybe we can look at the module reference count or something like that.
Regards,
Christian.
Am 18.09.19 um 12:04 schrieb Deng, Emily:
Hi Christian,
Do you think this is because the wrong use case? Even we add the error log, the issue still happens. Could we change this error to warning? As for the right method to remove the driver, it shouldn’t occur issues.
Best wishes
Emily Deng
From: Koenig, Christian <Christian.Koenig at amd.com><mailto:Christian.Koenig at amd.com>
Sent: Wednesday, September 18, 2019 5:45 PM
To: Deng, Emily <Emily.Deng at amd.com><mailto:Emily.Deng at amd.com>
Cc: Zhang, Jack (Jian) <Jack.Zhang1 at amd.com><mailto:Jack.Zhang1 at amd.com>; amd-gfx at lists.freedesktop.org<mailto:amd-gfx at lists.freedesktop.org>; Teng, Rui <Rui.Teng at amd.com><mailto:Rui.Teng at amd.com>; Cui, Flora <Flora.Cui at amd.com><mailto:Flora.Cui at amd.com>
Subject: RE: [PATCH] drm/amdgpu/sriov: omit fbcon error under sriov or passthrough
Yes, exactly.
The problem is that even when output is qxl or the Intel driver our driver is still loaded and forcefully removing it renders the desktop unusable.
Christian.
Am 18.09.2019 11:24 schrieb "Deng, Emily" <Emily.Deng at amd.com<mailto:Emily.Deng at amd.com>>:
Hi Christian,
Do you mean, for passthrough mode, the desktop is rendered by our asic, but enduser is trying to remove the driver by hot plug?
Best wishes
Emily Deng
From: Koenig, Christian <Christian.Koenig at amd.com<mailto:Christian.Koenig at amd.com>>
Sent: Wednesday, September 18, 2019 4:44 PM
To: Deng, Emily <Emily.Deng at amd.com<mailto:Emily.Deng at amd.com>>
Cc: Zhang, Jack (Jian) <Jack.Zhang1 at amd.com<mailto:Jack.Zhang1 at amd.com>>; amd-gfx at lists.freedesktop.org<mailto:amd-gfx at lists.freedesktop.org>; Teng, Rui <Rui.Teng at amd.com<mailto:Rui.Teng at amd.com>>; Cui, Flora <Flora.Cui at amd.com<mailto:Flora.Cui at amd.com>>
Subject: RE: [PATCH] drm/amdgpu/sriov: omit fbcon error under sriov or passthrough
Hi Emily,
Yeah, exactly that's the problem: In some cases the driver can be unloaded while it is still in use!
See we added this error message because endusers tried to use PCIe hot plug to unload the driver to use the hardware for paththrough.
But this will completely nuke your desktop, even when amdgpu is only a secondary device like in the qxl case.
Jack is using the correct way of doing it, e.g. using "modprobe -r" or rmmod. Both commands check the use count before unloading the driver instances.
I don't see a way to distingt the two cases and what Jack is doing is unfortunate not the common one.
Regards,
Christian.
Am 18.09.2019 10:08 schrieb "Deng, Emily" <Emily.Deng at amd.com<mailto:Emily.Deng at amd.com>>:
Hi Christian,
Before unloading driver, user must sure there is not any userspace to use of amdgpu, if not, it will report driver is in use. And the unloading driver is a feature about amdgpu driver which will be easier to replace driver without rebooting VM. Do you think it has any issue about driver unloading path?
Best wishes
Emily Deng
From: Koenig, Christian <Christian.Koenig at amd.com<mailto:Christian.Koenig at amd.com>>
Sent: Wednesday, September 18, 2019 3:54 PM
To: Zhang, Jack (Jian) <Jack.Zhang1 at amd.com<mailto:Jack.Zhang1 at amd.com>>
Cc: amd-gfx at lists.freedesktop.org<mailto:amd-gfx at lists.freedesktop.org>; Teng, Rui <Rui.Teng at amd.com<mailto:Rui.Teng at amd.com>>; Deng, Emily <Emily.Deng at amd.com<mailto:Emily.Deng at amd.com>>; Cui, Flora <Flora.Cui at amd.com<mailto:Flora.Cui at amd.com>>
Subject: RE: [PATCH] drm/amdgpu/sriov: omit fbcon error under sriov or passthrough
Hi Jack,
Well that believe is unfortunately completely wrong.
The point is that ANY use of amdgpu by userspace will prevent correct driver unload, that qxl is used for the fbcon doesn't change anything here.
So the patch is a clear NAK. Driver unload is not supposed to work even under SRIOV.
Regards,
Christian.
Am 18.09.2019 09:32 schrieb "Zhang, Jack (Jian)" <Jack.Zhang1 at amd.com<mailto:Jack.Zhang1 at amd.com>>:
Hi, Christian and folks,
In virtual machines(such virt-manager), there's always a virtual graphics device existed like "qxl" as the default gfx device.
So under such condition, we believe it's reasonable to unload amdgpu driver as it is not treated as the default fbcon device.
Would you please help to review this patch?
Best wishes,
Jack
-----Original Message-----
From: Jack Zhang <Jack.Zhang1 at amd.com<mailto:Jack.Zhang1 at amd.com>>
Sent: Wednesday, September 18, 2019 3:25 PM
To: amd-gfx at lists.freedesktop.org<mailto:amd-gfx at lists.freedesktop.org>
Cc: Zhang, Jack (Jian) <Jack.Zhang1 at amd.com<mailto:Jack.Zhang1 at amd.com>>
Subject: [PATCH] drm/amdgpu/sriov: omit fbcon error under sriov or passthrough
In virtual machine, there would be a qxl or cirrus graphics device as the default master fbcon device.
So for PF(passthrough mode) or SRIOV VF, it is reasonable to unload amdgpu driver. Amdgpu doesn't have to be the only fbcon device under this condition.
Signed-off-by: Jack Zhang <Jack.Zhang1 at amd.com<mailto:Jack.Zhang1 at amd.com>>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 420888e..ada2b25 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -1103,8 +1103,9 @@ static void
amdgpu_pci_remove(struct pci_dev *pdev) {
struct drm_device *dev = pci_get_drvdata(pdev);
-
- DRM_ERROR("Device removal is currently not supported outside of fbcon\n");
+ struct amdgpu_device *adev = dev->dev_private;
+ if (!amdgpu_sriov_vf(adev) && !amdgpu_passthrough(adev))
+ DRM_ERROR(" currently not supported outside of
+fbcon\n");
drm_dev_unplug(dev);
drm_dev_put(dev);
pci_disable_device(pdev);
--
2.7.4
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/amd-gfx/attachments/20190919/1fd69d7b/attachment-0001.html>
More information about the amd-gfx
mailing list