All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/radeon: Bail earlier when radeon.cik_/si_support=0 is passed
@ 2019-09-07 20:32 Hans de Goede
  2019-09-10  8:50 ` Michel Dänzer
  0 siblings, 1 reply; 6+ messages in thread
From: Hans de Goede @ 2019-09-07 20:32 UTC (permalink / raw)
  To: Alex Deucher, Christian König, David Zhou
  Cc: David Airlie, Hans de Goede, dri-devel, amd-gfx

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(-)

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

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] drm/radeon: Bail earlier when radeon.cik_/si_support=0 is passed
  2019-09-07 20:32 [PATCH] drm/radeon: Bail earlier when radeon.cik_/si_support=0 is passed Hans de Goede
@ 2019-09-10  8:50 ` Michel Dänzer
  2019-09-10  9:36   ` Hans de Goede
  0 siblings, 1 reply; 6+ messages in thread
From: Michel Dänzer @ 2019-09-10  8:50 UTC (permalink / raw)
  To: Hans de Goede, Alex Deucher, Christian König, David Zhou
  Cc: David Airlie, amd-gfx, dri-devel

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.


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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] drm/radeon: Bail earlier when radeon.cik_/si_support=0 is passed
  2019-09-10  8:50 ` Michel Dänzer
@ 2019-09-10  9:36   ` Hans de Goede
       [not found]     ` <3c0eb318-6c30-fc98-4005-80b5c00b8f5e-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
  2019-09-11 15:29     ` Deucher, Alexander
  0 siblings, 2 replies; 6+ messages in thread
From: Hans de Goede @ 2019-09-10  9:36 UTC (permalink / raw)
  To: Michel Dänzer, Alex Deucher, Christian König, David Zhou
  Cc: David Airlie, amd-gfx, dri-devel

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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] drm/radeon: Bail earlier when radeon.cik_/si_support=0 is passed
       [not found]     ` <3c0eb318-6c30-fc98-4005-80b5c00b8f5e-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
@ 2019-09-11  9:04       ` Michel Dänzer
  0 siblings, 0 replies; 6+ messages in thread
From: Michel Dänzer @ 2019-09-11  9:04 UTC (permalink / raw)
  To: Hans de Goede, Alex Deucher, Christian König, David Zhou
  Cc: David Airlie, dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Daniel Vetter

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.


-- 
Earthling Michel Dänzer               |               https://redhat.com
Libre software enthusiast             |             Mesa and X developer
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 6+ messages in thread

* RE: [PATCH] drm/radeon: Bail earlier when radeon.cik_/si_support=0 is passed
  2019-09-10  9:36   ` Hans de Goede
       [not found]     ` <3c0eb318-6c30-fc98-4005-80b5c00b8f5e-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
@ 2019-09-11 15:29     ` Deucher, Alexander
  2019-09-13 20:31       ` Alex Deucher
  1 sibling, 1 reply; 6+ messages in thread
From: Deucher, Alexander @ 2019-09-11 15:29 UTC (permalink / raw)
  To: Hans de Goede, Michel Dänzer, Koenig, Christian, Zhou,
	David(ChunMing)
  Cc: David Airlie, amd-gfx, dri-devel

> -----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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] drm/radeon: Bail earlier when radeon.cik_/si_support=0 is passed
  2019-09-11 15:29     ` Deucher, Alexander
@ 2019-09-13 20:31       ` Alex Deucher
  0 siblings, 0 replies; 6+ messages in thread
From: Alex Deucher @ 2019-09-13 20:31 UTC (permalink / raw)
  To: Deucher, Alexander
  Cc: David Airlie, Michel Dänzer, amd-gfx, Hans de Goede,
	dri-devel, Koenig, Christian

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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2019-09-13 20:31 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-07 20:32 [PATCH] drm/radeon: Bail earlier when radeon.cik_/si_support=0 is passed Hans de Goede
2019-09-10  8:50 ` Michel Dänzer
2019-09-10  9:36   ` Hans de Goede
     [not found]     ` <3c0eb318-6c30-fc98-4005-80b5c00b8f5e-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2019-09-11  9:04       ` Michel Dänzer
2019-09-11 15:29     ` Deucher, Alexander
2019-09-13 20:31       ` Alex Deucher

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.