[PATCH] nouveau: add fbdev dependency

[PATCH] nouveau: add fbdev dependency

[Arnd Bergmann]

Calling directly into the fbdev stack only works when the
fbdev layer is built into the kernel as well, or both are
loadable modules:

drivers/gpu/drm/nouveau/nouveau_drm.o: in function `nouveau_drm_probe':
nouveau_drm.c:(.text+0x1f90): undefined reference to `remove_conflicting_pci_framebuffers'

The change seems to have been intentional, so add an explicit
dependency here but allow it to still be compiled if FBDEV
is completely disabled.

Fixes: 2dd4d163cd9c ("drm/nouveau: remove open-coded version of remove_conflicting_pci_framebuffers()")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/gpu/drm/nouveau/Kconfig       | 1 +
 drivers/gpu/drm/nouveau/nouveau_drm.c | 3 ++-
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/nouveau/Kconfig b/drivers/gpu/drm/nouveau/Kconfig
index 980ed09bd7f6..8c640f003358 100644
--- a/drivers/gpu/drm/nouveau/Kconfig
+++ b/drivers/gpu/drm/nouveau/Kconfig
@@ -18,6 +18,7 @@ config DRM_NOUVEAU
 	select THERMAL if ACPI && X86
 	select ACPI_VIDEO if ACPI && X86
 	select SND_HDA_COMPONENT if SND_HDA_CORE
+	depends on FBDEV || !FBDEV
 	help
 	  Choose this option for open-source NVIDIA support.
 
diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c b/drivers/gpu/drm/nouveau/nouveau_drm.c
index eb10c80ed853..e8560444ab57 100644
--- a/drivers/gpu/drm/nouveau/nouveau_drm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
@@ -697,7 +697,8 @@ static int nouveau_drm_probe(struct pci_dev *pdev,
 	nvkm_device_del(&device);
 
 	/* Remove conflicting drivers (vesafb, efifb etc). */
-	ret = remove_conflicting_pci_framebuffers(pdev, "nouveaufb");
+	if (IS_ENABLED(CONFIG_FBDEV))
+		ret = remove_conflicting_pci_framebuffers(pdev, "nouveaufb");
 	if (ret)
 		return ret;
 
-- 
2.26.2

Re: [Nouveau] [PATCH] nouveau: add fbdev dependency

[Ilia Mirkin]

Isn't this already fixed by

https://cgit.freedesktop.org/drm/drm/commit/?id=7dbbdd37f2ae7dd4175ba3f86f4335c463b18403

On Wed, May 27, 2020 at 9:43 AM Arnd Bergmann <arnd@arndb.de> wrote:
>
> Calling directly into the fbdev stack only works when the
> fbdev layer is built into the kernel as well, or both are
> loadable modules:
>
> drivers/gpu/drm/nouveau/nouveau_drm.o: in function `nouveau_drm_probe':
> nouveau_drm.c:(.text+0x1f90): undefined reference to `remove_conflicting_pci_framebuffers'
>
> The change seems to have been intentional, so add an explicit
> dependency here but allow it to still be compiled if FBDEV
> is completely disabled.
>
> Fixes: 2dd4d163cd9c ("drm/nouveau: remove open-coded version of remove_conflicting_pci_framebuffers()")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  drivers/gpu/drm/nouveau/Kconfig       | 1 +
>  drivers/gpu/drm/nouveau/nouveau_drm.c | 3 ++-
>  2 files changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/nouveau/Kconfig b/drivers/gpu/drm/nouveau/Kconfig
> index 980ed09bd7f6..8c640f003358 100644
> --- a/drivers/gpu/drm/nouveau/Kconfig
> +++ b/drivers/gpu/drm/nouveau/Kconfig
> @@ -18,6 +18,7 @@ config DRM_NOUVEAU
>         select THERMAL if ACPI && X86
>         select ACPI_VIDEO if ACPI && X86
>         select SND_HDA_COMPONENT if SND_HDA_CORE
> +       depends on FBDEV || !FBDEV
>         help
>           Choose this option for open-source NVIDIA support.
>
> diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c b/drivers/gpu/drm/nouveau/nouveau_drm.c
> index eb10c80ed853..e8560444ab57 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_drm.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
> @@ -697,7 +697,8 @@ static int nouveau_drm_probe(struct pci_dev *pdev,
>         nvkm_device_del(&device);
>
>         /* Remove conflicting drivers (vesafb, efifb etc). */
> -       ret = remove_conflicting_pci_framebuffers(pdev, "nouveaufb");
> +       if (IS_ENABLED(CONFIG_FBDEV))
> +               ret = remove_conflicting_pci_framebuffers(pdev, "nouveaufb");
>         if (ret)
>                 return ret;
>
> --
> 2.26.2
>
> _______________________________________________
> Nouveau mailing list
> Nouveau@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/nouveau

Re: [Nouveau] [PATCH] nouveau: add fbdev dependency

[Arnd Bergmann]

On Wed, May 27, 2020 at 4:05 PM Ilia Mirkin <imirkin@alum.mit.edu> wrote:
>
> Isn't this already fixed by
>
> https://cgit.freedesktop.org/drm/drm/commit/?id=7dbbdd37f2ae7dd4175ba3f86f4335c463b18403

Ok, I see that fixes the link error, but I when I created my fix, that did
not seem like the correct solution because it reverts part of the original
patch without reverting the rest of it. Unfortunately there was no
changelog text in the first patch to explain why this is safe.

Could you double-check if the behavior is still correct after the two patches?

      Arnd

Re: [Nouveau] [PATCH] nouveau: add fbdev dependency

[Dave Airlie]

On Thu, 28 May 2020 at 00:36, Arnd Bergmann <arnd@arndb.de> wrote:
>
> On Wed, May 27, 2020 at 4:05 PM Ilia Mirkin <imirkin@alum.mit.edu> wrote:
> >
> > Isn't this already fixed by
> >
> > https://cgit.freedesktop.org/drm/drm/commit/?id=7dbbdd37f2ae7dd4175ba3f86f4335c463b18403
>
> Ok, I see that fixes the link error, but I when I created my fix, that did
> not seem like the correct solution because it reverts part of the original
> patch without reverting the rest of it. Unfortunately there was no
> changelog text in the first patch to explain why this is safe.

No it doesn't, I think you missed the pci in API name.

The initial behaviour doesn't use the pci version of the API, the
replacement did, and the fix used the drm wrapper around the pci one.

So this patch isn't necessary now that I've fixed it the other way,

Thanks,
Dave.

Re: [Nouveau] [PATCH] nouveau: add fbdev dependency

[Arnd Bergmann]

On Thu, May 28, 2020 at 7:37 AM Dave Airlie <airlied@gmail.com> wrote:
>
> On Thu, 28 May 2020 at 00:36, Arnd Bergmann <arnd@arndb.de> wrote:
> >
> > On Wed, May 27, 2020 at 4:05 PM Ilia Mirkin <imirkin@alum.mit.edu> wrote:
> > >
> > > Isn't this already fixed by
> > >
> > > https://cgit.freedesktop.org/drm/drm/commit/?id=7dbbdd37f2ae7dd4175ba3f86f4335c463b18403
> >
> > Ok, I see that fixes the link error, but I when I created my fix, that did
> > not seem like the correct solution because it reverts part of the original
> > patch without reverting the rest of it. Unfortunately there was no
> > changelog text in the first patch to explain why this is safe.
>
> No it doesn't, I think you missed the pci in API name.
>
> The initial behaviour doesn't use the pci version of the API, the
> replacement did, and the fix used the drm wrapper around the pci one.
>
> So this patch isn't necessary now that I've fixed it the other way,

Ok, got it, thanks for explaining!

       Arnd