| Message ID | 20190907203238.232005-1-hdegoede@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/radeon: Bail earlier when radeon.cik_/si_support=0 is passed | expand |
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 <hdegoede@redhat.com> Reviewed-by: Michel Dänzer <mdaenzer@redhat.com> amdgpu should be changed correspondingly as well.
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 <hdegoede@redhat.com> > > Reviewed-by: Michel Dänzer <mdaenzer@redhat.com> 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
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 <hdegoede@redhat.com> >> >> Reviewed-by: Michel Dänzer <mdaenzer@redhat.com> > > 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.
> -----Original Message----- > From: Hans de Goede <hdegoede@redhat.com> > Sent: Tuesday, September 10, 2019 5:36 AM > To: Michel Dänzer <michel@daenzer.net>; Deucher, Alexander > <Alexander.Deucher@amd.com>; Koenig, Christian > <Christian.Koenig@amd.com>; Zhou, David(ChunMing) > <David1.Zhou@amd.com> > Cc: David Airlie <airlied@linux.ie>; dri-devel@lists.freedesktop.org; amd- > gfx@lists.freedesktop.org; Daniel Vetter <daniel@ffwll.ch> > 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 <hdegoede@redhat.com> > > > > Reviewed-by: Michel Dänzer <mdaenzer@redhat.com> > > 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
On Wed, Sep 11, 2019 at 11:29 AM Deucher, Alexander <Alexander.Deucher@amd.com> wrote: > > > -----Original Message----- > > From: Hans de Goede <hdegoede@redhat.com> > > Sent: Tuesday, September 10, 2019 5:36 AM > > To: Michel Dänzer <michel@daenzer.net>; Deucher, Alexander > > <Alexander.Deucher@amd.com>; Koenig, Christian > > <Christian.Koenig@amd.com>; Zhou, David(ChunMing) > > <David1.Zhou@amd.com> > > Cc: David Airlie <airlied@linux.ie>; dri-devel@lists.freedesktop.org; amd- > > gfx@lists.freedesktop.org; Daniel Vetter <daniel@ffwll.ch> > > 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 <hdegoede@redhat.com> > > > > > > Reviewed-by: Michel Dänzer <mdaenzer@redhat.com> > > > > 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
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(&pdev->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(&pdev->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;
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 <hdegoede@redhat.com> --- drivers/gpu/drm/radeon/radeon_drv.c | 31 +++++++++++++++++++++++++++++ drivers/gpu/drm/radeon/radeon_kms.c | 25 ----------------------- 2 files changed, 31 insertions(+), 25 deletions(-)