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

From: Alex Deucher <alexdeucher-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
To: Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Cc: "David Airlie" <airlied-cv59FeDIM0c@public.gmane.org>,
	"amd-gfx list"
	<amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org>,
	dri-devel
	<dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org>,
	"Daniel Vetter" <daniel-/w4YWyX8dFk@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: Fri, 11 Oct 2019 09:25:59 -0400	[thread overview]
Message-ID: <CADnq5_PUYY6AtCLBGsFqM8KWNUQ=cOqw6gpTczipK3+jdzjoiw@mail.gmail.com> (raw)
In-Reply-To: <69b96e00-6ee7-4887-1f79-04ebd1b470c7-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

On Fri, Oct 11, 2019 at 5:27 AM Hans de Goede <hdegoede@redhat.com> wrote:
>
> Hi,
>
> On 10-10-2019 18:59, Daniel Vetter wrote:
> > 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>
>
> 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 <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
> >>
> >
> >
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx