Re: [PATCH] drm/amdgpu: Bail earlier when amdgpu.cik_/si_support is not set to 1

From: Daniel Vetter <daniel-/w4YWyX8dFk@public.gmane.org>
To: Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Cc: "David Zhou" <David1.Zhou-5C7GfCeVMHo@public.gmane.org>,
	"David Airlie" <airlied-cv59FeDIM0c@public.gmane.org>,
	dri-devel
	<dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org>,
	"amd-gfx list"
	<amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org>,
	"Alex Deucher" <alexander.deucher-5C7GfCeVMHo@public.gmane.org>,
	"Christian König" <christian.koenig-5C7GfCeVMHo@public.gmane.org>
Subject: Re: [PATCH] drm/amdgpu: Bail earlier when amdgpu.cik_/si_support is not set to 1
Date: Thu, 10 Oct 2019 18:59:41 +0200	[thread overview]
Message-ID: <CAKMK7uECLjVPp4gHJ2k76rVaTPJN+Ebjx7Bx6n-_cqAk3nQDmg@mail.gmail.com> (raw)
In-Reply-To: <20191010162817.55446-1-hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

On Thu, Oct 10, 2019 at 6:28 PM Hans de Goede <hdegoede@redhat.com> 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 <daniel.vetter@ffwll.ch>

I guess also cc: stable material?
-Daniel
>
> BugLink: https://bugzilla.redhat.com/show_bug.cgi?id=1490490
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  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.ffwll.ch
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx