Re: [PATCH] drm/amdgpu: Bail earlier when amdgpu.cik_/si_support is not set to 1
On Fri, Oct 11, 2019 at 5:27 AM Hans de Goede wrote:
>
> Hi,
>
> On 10-10-2019 18:59, Daniel Vetter wrote:
> > On Thu, Oct 10, 2019 at 6:28 PM 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 a userspace bug, not sending unnecessary
> >> udev events is a good idea in general.
> >
> > I think even better would be getting rid of the load/unload callbacks,
> > this issue here isn't the only problem with them.
> >
> > Reviewed-by: Daniel Vetter
>
> Thanks,
>
> > I guess also cc: stable material?
>
> Yes.
>
> amdgpu maintainers, can you please add a Cc: stable while merging?
> Let me know if you want a new version with this added.
I'll take care of it when I merge this. Thanks!
Alex
>
> Regards,
>
> Hans
>
>
>
> >> BugLink: https://bugzilla.redhat.com/show_bug.cgi?id=1490490
> >> Signed-off-by: Hans de Goede
> >> ---
> >> drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 35 +
> >> drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 35 -
> >> 2 files changed, 35 insertions(+), 35 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> >> b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> >> index 6f8aaf655a9f..2a00a36106b2 100644
> >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> >> @@ -1048,6 +1048,41 @@ static int amdgpu_pci_probe(struct pci_dev *pdev,
> >> return -ENODEV;
> >> }
> >>
> >> +#ifdef CONFIG_DRM_AMDGPU_SI
> >> + if (!amdgpu_si_support) {
> >> + switch (flags & AMD_ASIC_MASK) {
> >> + case CHIP_TAHITI:
> >> + case CHIP_PITCAIRN:
> >> + case CHIP_VERDE:
> >> + case CHIP_OLAND:
> >> + case CHIP_HAINAN:
> >> + dev_info(&pdev->dev,
> >> +"SI support provided by radeon.\n");
> >> + dev_info(&pdev->dev,
> >> +"Use radeon.si_support=0
> >> amdgpu.si_support=1 to override.\n"
> >> + );
> >> + return -ENODEV;
> >> + }
> >> + }
> >> +#endif
> >> +#ifdef CONFIG_DRM_AMDGPU_CIK
> >> + if (!amdgpu_cik_support) {
> >> + switch (flags & AMD_ASIC_MASK) {
> >> + case CHIP_KAVERI:
> >> + case CHIP_BONAIRE:
> >> + case CHIP_HAWAII:
> >> + case CHIP_KABINI:
> >> + case CHIP_MULLINS:
> >> + dev_info(&pdev->dev,
> >> +"CIK support provided by radeon.\n");
> >> + dev_info(&pdev->dev,
> >> +"Use radeon.cik_support=0
> >> amdgpu.cik_support=1 to override.\n"
> >> + );
> >> + return -ENODEV;
> >> + }
> >> + }
> >> +#endif
> >> +
> >> /* Get rid of things like offb */
> >> ret = drm_fb_helper_remove_conflicting_pci_framebuffers(pdev, 0,
> >> "amdgpudrmfb");
> >> if (ret)
> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> >> b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> >> index f2c097983f48..d55f5baa83d3 100644
> >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> >> @@ -144,41 +144,6 @@ int amdgpu_driver_load_kms(struct drm_device *dev,
> >> unsigned long flags)
> >> struct amdgpu_device *adev;
> >> int r, acpi_status;
> >>
> >> -#ifdef CONFIG_DRM_AMDGPU_SI
> >> - if (!amdgpu_si_support) {
> >> - switch (flags & AMD_ASIC_MASK) {
> >> - case CHIP_TAHITI:
> >> - case CHIP_PITCAIRN:
> >> - case CHIP_VERDE:
> >> - case CHIP_OLAND:
> >> - case CHIP_HAINAN:
> >> - dev_info(dev->dev,
> >> -"SI support provided by radeon.\n");
> >> - dev_info(dev->dev,
> >> -"Use radeon.si_support=0
> >> amdgpu.si_support=1 to override.\n"
> >> - );
> >> - return -ENODEV;
> >> - }
> >> - }
> >> -#endif
> >> -#ifdef CONFIG_DRM_AMDGPU_CIK
> >> - if (!amdgpu_cik_support) {
> >> - switch (flags & AMD_ASIC_MASK) {
> >> - case CHIP_KAVERI:
> >> - case CHIP_BONAIRE:
> >> - case CHIP_HAWAII:
> >> - c
Re: [PATCH] drm/amdgpu: Bail earlier when amdgpu.cik_/si_support is not set to 1
Hi,
On 10-10-2019 18:59, Daniel Vetter wrote:
On Thu, Oct 10, 2019 at 6:28 PM 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 a userspace bug, not sending unnecessary
udev events is a good idea in general.
I think even better would be getting rid of the load/unload callbacks,
this issue here isn't the only problem with them.
Reviewed-by: Daniel Vetter
Thanks,
I guess also cc: stable material?
Yes.
amdgpu maintainers, can you please add a Cc: stable while merging?
Let me know if you want a new version with this added.
Regards,
Hans
BugLink: https://bugzilla.redhat.com/show_bug.cgi?id=1490490
Signed-off-by: Hans de Goede
---
drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 35 +
drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 35 -
2 files changed, 35 insertions(+), 35 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 6f8aaf655a9f..2a00a36106b2 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -1048,6 +1048,41 @@ static int amdgpu_pci_probe(struct pci_dev *pdev,
return -ENODEV;
}
+#ifdef CONFIG_DRM_AMDGPU_SI
+ if (!amdgpu_si_support) {
+ switch (flags & AMD_ASIC_MASK) {
+ case CHIP_TAHITI:
+ case CHIP_PITCAIRN:
+ case CHIP_VERDE:
+ case CHIP_OLAND:
+ case CHIP_HAINAN:
+ dev_info(&pdev->dev,
+"SI support provided by radeon.\n");
+ dev_info(&pdev->dev,
+"Use radeon.si_support=0 amdgpu.si_support=1 to
override.\n"
+ );
+ return -ENODEV;
+ }
+ }
+#endif
+#ifdef CONFIG_DRM_AMDGPU_CIK
+ if (!amdgpu_cik_support) {
+ switch (flags & AMD_ASIC_MASK) {
+ case CHIP_KAVERI:
+ case CHIP_BONAIRE:
+ case CHIP_HAWAII:
+ case CHIP_KABINI:
+ case CHIP_MULLINS:
+ dev_info(&pdev->dev,
+"CIK support provided by radeon.\n");
+ dev_info(&pdev->dev,
+"Use radeon.cik_support=0 amdgpu.cik_support=1 to
override.\n"
+ );
+ return -ENODEV;
+ }
+ }
+#endif
+
/* Get rid of things like offb */
ret = drm_fb_helper_remove_conflicting_pci_framebuffers(pdev, 0,
"amdgpudrmfb");
if (ret)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
index f2c097983f48..d55f5baa83d3 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
@@ -144,41 +144,6 @@ int amdgpu_driver_load_kms(struct drm_device *dev,
unsigned long flags)
struct amdgpu_device *adev;
int r, acpi_status;
-#ifdef CONFIG_DRM_AMDGPU_SI
- if (!amdgpu_si_support) {
- switch (flags & AMD_ASIC_MASK) {
- case CHIP_TAHITI:
- case CHIP_PITCAIRN:
- case CHIP_VERDE:
- case CHIP_OLAND:
- case CHIP_HAINAN:
- dev_info(dev->dev,
-"SI support provided by radeon.\n");
- dev_info(dev->dev,
-"Use radeon.si_support=0 amdgpu.si_support=1 to
override.\n"
- );
- return -ENODEV;
- }
- }
-#endif
-#ifdef CONFIG_DRM_AMDGPU_CIK
- if (!amdgpu_cik_support) {
- switch (flags & AMD_ASIC_MASK) {
- case CHIP_KAVERI:
- case CHIP_BONAIRE:
- case CHIP_HAWAII:
- case CHIP_KABINI:
- case CHIP_MULLINS:
- dev_info(dev->dev,
-"CIK support provided by radeon.\n");
- dev_info(dev->dev,
-"Use radeon.cik_support=0 amdgpu.cik_support=1 to
override.\n"
- );
- return -ENODEV;
- }
- }
-#endif
-
adev = kzalloc(sizeof(struct amdgpu_device), GFP_KERNEL);
if (adev == NULL) {
return -ENOMEM;
--
2.23.0
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.fre
[PATCH] drm/amdgpu: Bail earlier when amdgpu.cik_/si_support is not set to 1
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 a 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/amd/amdgpu/amdgpu_drv.c | 35 +
drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 35 -
2 files changed, 35 insertions(+), 35 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 6f8aaf655a9f..2a00a36106b2 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -1048,6 +1048,41 @@ static int amdgpu_pci_probe(struct pci_dev *pdev,
return -ENODEV;
}
+#ifdef CONFIG_DRM_AMDGPU_SI
+ if (!amdgpu_si_support) {
+ switch (flags & AMD_ASIC_MASK) {
+ case CHIP_TAHITI:
+ case CHIP_PITCAIRN:
+ case CHIP_VERDE:
+ case CHIP_OLAND:
+ case CHIP_HAINAN:
+ dev_info(&pdev->dev,
+"SI support provided by radeon.\n");
+ dev_info(&pdev->dev,
+"Use radeon.si_support=0 amdgpu.si_support=1
to override.\n"
+ );
+ return -ENODEV;
+ }
+ }
+#endif
+#ifdef CONFIG_DRM_AMDGPU_CIK
+ if (!amdgpu_cik_support) {
+ switch (flags & AMD_ASIC_MASK) {
+ case CHIP_KAVERI:
+ case CHIP_BONAIRE:
+ case CHIP_HAWAII:
+ case CHIP_KABINI:
+ case CHIP_MULLINS:
+ dev_info(&pdev->dev,
+"CIK support provided by radeon.\n");
+ dev_info(&pdev->dev,
+"Use radeon.cik_support=0 amdgpu.cik_support=1
to override.\n"
+ );
+ return -ENODEV;
+ }
+ }
+#endif
+
/* Get rid of things like offb */
ret = drm_fb_helper_remove_conflicting_pci_framebuffers(pdev, 0,
"amdgpudrmfb");
if (ret)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
index f2c097983f48..d55f5baa83d3 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
@@ -144,41 +144,6 @@ int amdgpu_driver_load_kms(struct drm_device *dev,
unsigned long flags)
struct amdgpu_device *adev;
int r, acpi_status;
-#ifdef CONFIG_DRM_AMDGPU_SI
- if (!amdgpu_si_support) {
- switch (flags & AMD_ASIC_MASK) {
- case CHIP_TAHITI:
- case CHIP_PITCAIRN:
- case CHIP_VERDE:
- case CHIP_OLAND:
- case CHIP_HAINAN:
- dev_info(dev->dev,
-"SI support provided by radeon.\n");
- dev_info(dev->dev,
-"Use radeon.si_support=0 amdgpu.si_support=1
to override.\n"
- );
- return -ENODEV;
- }
- }
-#endif
-#ifdef CONFIG_DRM_AMDGPU_CIK
- if (!amdgpu_cik_support) {
- switch (flags & AMD_ASIC_MASK) {
- case CHIP_KAVERI:
- case CHIP_BONAIRE:
- case CHIP_HAWAII:
- case CHIP_KABINI:
- case CHIP_MULLINS:
- dev_info(dev->dev,
-"CIK support provided by radeon.\n");
- dev_info(dev->dev,
-"Use radeon.cik_support=0 amdgpu.cik_support=1
to override.\n"
- );
- return -ENODEV;
- }
- }
-#endif
-
adev = kzalloc(sizeof(struct amdgpu_device), GFP_KERNEL);
if (adev == NULL) {
return -ENOMEM;
--
2.23.0
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amdgpu: Bail earlier when amdgpu.cik_/si_support is not set to 1
On Thu, Oct 10, 2019 at 6:28 PM 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 a userspace bug, not sending unnecessary
> udev events is a good idea in general.
I think even better would be getting rid of the load/unload callbacks,
this issue here isn't the only problem with them.
Reviewed-by: Daniel Vetter
I guess also cc: stable material?
-Daniel
>
> BugLink: https://bugzilla.redhat.com/show_bug.cgi?id=1490490
> Signed-off-by: Hans de Goede
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 35 +
> drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 35 -
> 2 files changed, 35 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> index 6f8aaf655a9f..2a00a36106b2 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> @@ -1048,6 +1048,41 @@ static int amdgpu_pci_probe(struct pci_dev *pdev,
> return -ENODEV;
> }
>
> +#ifdef CONFIG_DRM_AMDGPU_SI
> + if (!amdgpu_si_support) {
> + switch (flags & AMD_ASIC_MASK) {
> + case CHIP_TAHITI:
> + case CHIP_PITCAIRN:
> + case CHIP_VERDE:
> + case CHIP_OLAND:
> + case CHIP_HAINAN:
> + dev_info(&pdev->dev,
> +"SI support provided by radeon.\n");
> + dev_info(&pdev->dev,
> +"Use radeon.si_support=0 amdgpu.si_support=1
> to override.\n"
> + );
> + return -ENODEV;
> + }
> + }
> +#endif
> +#ifdef CONFIG_DRM_AMDGPU_CIK
> + if (!amdgpu_cik_support) {
> + switch (flags & AMD_ASIC_MASK) {
> + case CHIP_KAVERI:
> + case CHIP_BONAIRE:
> + case CHIP_HAWAII:
> + case CHIP_KABINI:
> + case CHIP_MULLINS:
> + dev_info(&pdev->dev,
> +"CIK support provided by radeon.\n");
> + dev_info(&pdev->dev,
> +"Use radeon.cik_support=0
> amdgpu.cik_support=1 to override.\n"
> + );
> + return -ENODEV;
> + }
> + }
> +#endif
> +
> /* Get rid of things like offb */
> ret = drm_fb_helper_remove_conflicting_pci_framebuffers(pdev, 0,
> "amdgpudrmfb");
> if (ret)
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> index f2c097983f48..d55f5baa83d3 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> @@ -144,41 +144,6 @@ int amdgpu_driver_load_kms(struct drm_device *dev,
> unsigned long flags)
> struct amdgpu_device *adev;
> int r, acpi_status;
>
> -#ifdef CONFIG_DRM_AMDGPU_SI
> - if (!amdgpu_si_support) {
> - switch (flags & AMD_ASIC_MASK) {
> - case CHIP_TAHITI:
> - case CHIP_PITCAIRN:
> - case CHIP_VERDE:
> - case CHIP_OLAND:
> - case CHIP_HAINAN:
> - dev_info(dev->dev,
> -"SI support provided by radeon.\n");
> - dev_info(dev->dev,
> -"Use radeon.si_support=0 amdgpu.si_support=1
> to override.\n"
> - );
> - return -ENODEV;
> - }
> - }
> -#endif
> -#ifdef CONFIG_DRM_AMDGPU_CIK
> - if (!amdgpu_cik_support) {
> - switch (flags & AMD_ASIC_MASK) {
> - case CHIP_KAVERI:
> - case CHIP_BONAIRE:
> - case CHIP_HAWAII:
> - case CHIP_KABINI:
> - case CHIP_MULLINS:
> - dev_info(dev->dev,
> -"CIK support provided by radeon.\n");
> - dev_info(dev->dev,
> -"Use radeon.cik_support=0
> amdgpu.cik_support=1 to override.\n"
> - );
> - return -ENODEV;
> - }
> - }
> -#endif
> -
> adev = kzalloc(sizeof(struct amdgpu_device), GFP_KERNEL);
> if (adev == NULL) {
> return -ENOMEM;
> --
> 2.23.0
>
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwl
