Re: [PATCH] drm/radeon: Bail earlier when radeon.cik_/si_support=0 is passed
On Wed, Sep 11, 2019 at 11:29 AM Deucher, Alexander wrote: > > > -Original Message- > > From: Hans de Goede > > Sent: Tuesday, September 10, 2019 5:36 AM > > To: Michel Dänzer ; Deucher, Alexander > > ; Koenig, Christian > > ; Zhou, David(ChunMing) > > > > Cc: David Airlie ; dri-devel@lists.freedesktop.org; amd- > > g...@lists.freedesktop.org; Daniel Vetter > > Subject: Re: [PATCH] drm/radeon: Bail earlier when > > radeon.cik_/si_support=0 is passed > > > > Hi, > > > > On 9/10/19 9:50 AM, Michel Dänzer wrote: > > > On 2019-09-07 10:32 p.m., Hans de Goede wrote: > > >> Bail from the pci_driver probe function instead of from the > > >> drm_driver load function. > > >> > > >> This avoid /dev/dri/card0 temporarily getting registered and then > > >> unregistered again, sending unwanted add / remove udev events to > > >> userspace. > > >> > > >> Specifically this avoids triggering the (userspace) bug fixed by this > > >> plymouth merge-request: > > >> https://gitlab.freedesktop.org/plymouth/plymouth/merge_requests/59 > > >> > > >> Note that despite that being an userspace bug, not sending > > >> unnecessary udev events is a good idea in general. > > >> > > >> BugLink: https://bugzilla.redhat.com/show_bug.cgi?id=1490490 > > >> Signed-off-by: Hans de Goede > > > > > > Reviewed-by: Michel Dänzer > > > > Thank you for the review. I've drm push rights, but I guess that radeon/amd > > GPU patches are collected by someone @AMD, so I do not need to / should > > not push this myself, right? > > I'll pick this up later this week when I get home from travel. Applied. Thanks! Alex > > Thanks! > > Alex > > > > > > amdgpu should be changed correspondingly as well. > > > > Good point. I'm currently travelling (@plumbers) I can do this when I'm back > > home, but feel free to beat me to it (if you do please Cc me to avoid double > > work). > > > > Regards, > > > > Hans > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
RE: [PATCH] drm/radeon: Bail earlier when radeon.cik_/si_support=0 is passed
> -Original Message- > From: Hans de Goede > Sent: Tuesday, September 10, 2019 5:36 AM > To: Michel Dänzer ; Deucher, Alexander > ; Koenig, Christian > ; Zhou, David(ChunMing) > > Cc: David Airlie ; dri-devel@lists.freedesktop.org; amd- > g...@lists.freedesktop.org; Daniel Vetter > Subject: Re: [PATCH] drm/radeon: Bail earlier when > radeon.cik_/si_support=0 is passed > > Hi, > > On 9/10/19 9:50 AM, Michel Dänzer wrote: > > On 2019-09-07 10:32 p.m., Hans de Goede wrote: > >> Bail from the pci_driver probe function instead of from the > >> drm_driver load function. > >> > >> This avoid /dev/dri/card0 temporarily getting registered and then > >> unregistered again, sending unwanted add / remove udev events to > >> userspace. > >> > >> Specifically this avoids triggering the (userspace) bug fixed by this > >> plymouth merge-request: > >> https://gitlab.freedesktop.org/plymouth/plymouth/merge_requests/59 > >> > >> Note that despite that being an userspace bug, not sending > >> unnecessary udev events is a good idea in general. > >> > >> BugLink: https://bugzilla.redhat.com/show_bug.cgi?id=1490490 > >> Signed-off-by: Hans de Goede > > > > Reviewed-by: Michel Dänzer > > Thank you for the review. I've drm push rights, but I guess that radeon/amd > GPU patches are collected by someone @AMD, so I do not need to / should > not push this myself, right? I'll pick this up later this week when I get home from travel. Thanks! Alex > > > amdgpu should be changed correspondingly as well. > > Good point. I'm currently travelling (@plumbers) I can do this when I'm back > home, but feel free to beat me to it (if you do please Cc me to avoid double > work). > > Regards, > > Hans ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/radeon: Bail earlier when radeon.cik_/si_support=0 is passed
On 2019-09-10 11:36 a.m., Hans de Goede wrote: > On 9/10/19 9:50 AM, Michel Dänzer wrote: >> On 2019-09-07 10:32 p.m., Hans de Goede wrote: >>> Bail from the pci_driver probe function instead of from the drm_driver >>> load function. >>> >>> This avoid /dev/dri/card0 temporarily getting registered and then >>> unregistered again, sending unwanted add / remove udev events to >>> userspace. >>> >>> Specifically this avoids triggering the (userspace) bug fixed by this >>> plymouth merge-request: >>> https://gitlab.freedesktop.org/plymouth/plymouth/merge_requests/59 >>> >>> Note that despite that being an userspace bug, not sending unnecessary >>> udev events is a good idea in general. >>> >>> BugLink: https://bugzilla.redhat.com/show_bug.cgi?id=1490490 >>> Signed-off-by: Hans de Goede >> >> Reviewed-by: Michel Dänzer > > Thank you for the review. I've drm push rights, but I guess that radeon/amd > GPU patches are collected by someone @AMD, so I do not need to / should not > push this myself, right? I think so, unless one of the driver maintainers says otherwise. >> amdgpu should be changed correspondingly as well. > > Good point. I'm currently travelling (@plumbers) I can do this when I'm > back home, > but feel free to beat me to it (if you do please Cc me to avoid double > work). I'm happy to leave it to you, if you don't mind. -- Earthling Michel Dänzer | https://redhat.com Libre software enthusiast | Mesa and X developer ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/radeon: Bail earlier when radeon.cik_/si_support=0 is passed
Hi, On 9/10/19 9:50 AM, Michel Dänzer wrote: On 2019-09-07 10:32 p.m., Hans de Goede wrote: Bail from the pci_driver probe function instead of from the drm_driver load function. This avoid /dev/dri/card0 temporarily getting registered and then unregistered again, sending unwanted add / remove udev events to userspace. Specifically this avoids triggering the (userspace) bug fixed by this plymouth merge-request: https://gitlab.freedesktop.org/plymouth/plymouth/merge_requests/59 Note that despite that being an userspace bug, not sending unnecessary udev events is a good idea in general. BugLink: https://bugzilla.redhat.com/show_bug.cgi?id=1490490 Signed-off-by: Hans de Goede Reviewed-by: Michel Dänzer Thank you for the review. I've drm push rights, but I guess that radeon/amd GPU patches are collected by someone @AMD, so I do not need to / should not push this myself, right? amdgpu should be changed correspondingly as well. Good point. I'm currently travelling (@plumbers) I can do this when I'm back home, but feel free to beat me to it (if you do please Cc me to avoid double work). Regards, Hans ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/radeon: Bail earlier when radeon.cik_/si_support=0 is passed
On 2019-09-07 10:32 p.m., Hans de Goede wrote: > Bail from the pci_driver probe function instead of from the drm_driver > load function. > > This avoid /dev/dri/card0 temporarily getting registered and then > unregistered again, sending unwanted add / remove udev events to > userspace. > > Specifically this avoids triggering the (userspace) bug fixed by this > plymouth merge-request: > https://gitlab.freedesktop.org/plymouth/plymouth/merge_requests/59 > > Note that despite that being an userspace bug, not sending unnecessary > udev events is a good idea in general. > > BugLink: https://bugzilla.redhat.com/show_bug.cgi?id=1490490 > Signed-off-by: Hans de Goede Reviewed-by: Michel Dänzer amdgpu should be changed correspondingly as well. -- Earthling Michel Dänzer | https://redhat.com Libre software enthusiast | Mesa and X developer ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/radeon: Bail earlier when radeon.cik_/si_support=0 is passed
Bail from the pci_driver probe function instead of from the drm_driver
load function.
This avoid /dev/dri/card0 temporarily getting registered and then
unregistered again, sending unwanted add / remove udev events to
userspace.
Specifically this avoids triggering the (userspace) bug fixed by this
plymouth merge-request:
https://gitlab.freedesktop.org/plymouth/plymouth/merge_requests/59
Note that despite that being an userspace bug, not sending unnecessary
udev events is a good idea in general.
BugLink: https://bugzilla.redhat.com/show_bug.cgi?id=1490490
Signed-off-by: Hans de Goede
---
drivers/gpu/drm/radeon/radeon_drv.c | 31 +
drivers/gpu/drm/radeon/radeon_kms.c | 25 ---
2 files changed, 31 insertions(+), 25 deletions(-)
diff --git a/drivers/gpu/drm/radeon/radeon_drv.c
b/drivers/gpu/drm/radeon/radeon_drv.c
index a6cbe11f79c6..7033f3a38c87 100644
--- a/drivers/gpu/drm/radeon/radeon_drv.c
+++ b/drivers/gpu/drm/radeon/radeon_drv.c
@@ -325,8 +325,39 @@ bool radeon_device_is_virtual(void);
static int radeon_pci_probe(struct pci_dev *pdev,
const struct pci_device_id *ent)
{
+ unsigned long flags = 0;
int ret;
+ if (!ent)
+ return -ENODEV; /* Avoid NULL-ptr deref in drm_get_pci_dev */
+
+ flags = ent->driver_data;
+
+ if (!radeon_si_support) {
+ switch (flags & RADEON_FAMILY_MASK) {
+ case CHIP_TAHITI:
+ case CHIP_PITCAIRN:
+ case CHIP_VERDE:
+ case CHIP_OLAND:
+ case CHIP_HAINAN:
+ dev_info(>dev,
+"SI support disabled by module param\n");
+ return -ENODEV;
+ }
+ }
+ if (!radeon_cik_support) {
+ switch (flags & RADEON_FAMILY_MASK) {
+ case CHIP_KAVERI:
+ case CHIP_BONAIRE:
+ case CHIP_HAWAII:
+ case CHIP_KABINI:
+ case CHIP_MULLINS:
+ dev_info(>dev,
+"CIK support disabled by module param\n");
+ return -ENODEV;
+ }
+ }
+
if (vga_switcheroo_client_probe_defer(pdev))
return -EPROBE_DEFER;
diff --git a/drivers/gpu/drm/radeon/radeon_kms.c
b/drivers/gpu/drm/radeon/radeon_kms.c
index 07f7ace42c4b..e85c554eeaa9 100644
--- a/drivers/gpu/drm/radeon/radeon_kms.c
+++ b/drivers/gpu/drm/radeon/radeon_kms.c
@@ -100,31 +100,6 @@ int radeon_driver_load_kms(struct drm_device *dev,
unsigned long flags)
struct radeon_device *rdev;
int r, acpi_status;
- if (!radeon_si_support) {
- switch (flags & RADEON_FAMILY_MASK) {
- case CHIP_TAHITI:
- case CHIP_PITCAIRN:
- case CHIP_VERDE:
- case CHIP_OLAND:
- case CHIP_HAINAN:
- dev_info(dev->dev,
-"SI support disabled by module param\n");
- return -ENODEV;
- }
- }
- if (!radeon_cik_support) {
- switch (flags & RADEON_FAMILY_MASK) {
- case CHIP_KAVERI:
- case CHIP_BONAIRE:
- case CHIP_HAWAII:
- case CHIP_KABINI:
- case CHIP_MULLINS:
- dev_info(dev->dev,
-"CIK support disabled by module param\n");
- return -ENODEV;
- }
- }
-
rdev = kzalloc(sizeof(struct radeon_device), GFP_KERNEL);
if (rdev == NULL) {
return -ENOMEM;
--
2.23.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
