| Message ID | 20230726220325.278976-1-arthurgrillo@riseup.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/tests: Remove CONFIG_DRM_FBDEV_EMULATION on .kunitconfig | expand |
On Thu, Jul 27, 2023, at 00:03, Arthur Grillo wrote: > Using the `kunit_tool` with the command: > > tools/testing/kunit/kunit.py run --kunitconfig=drivers/gpu/drm/tests/ > > Lead to this error[0]. Fix it by expliciting removing the > CONFIG_DRM_FBDEV_EMULATION. > > [0] > ERROR:root: > WARNING: unmet direct dependencies detected for FRAMEBUFFER_CONSOLE > Depends on [n]: VT [=n] && FB_CORE [=y] && !UML [=y] > Selected by [y]: > - DRM_FBDEV_EMULATION [=y] && HAS_IOMEM [=y] && DRM [=y] && !EXPERT [=n] > I think that's a bug in the Kconfig files that should be fixed by enforcing the correct set of dependencies. I have not encountered this in my randconfig builds (with a lot of other fixes applied) In linux-next, CONFIG_VT cannot be disabled if CONFIG_EXPERT=n, so this does not happen. > diff --git a/drivers/gpu/drm/tests/.kunitconfig > b/drivers/gpu/drm/tests/.kunitconfig > index 6ec04b4c979d..c50b5a12faae 100644 > --- a/drivers/gpu/drm/tests/.kunitconfig > +++ b/drivers/gpu/drm/tests/.kunitconfig > @@ -1,3 +1,4 @@ > CONFIG_KUNIT=y > CONFIG_DRM=y > CONFIG_DRM_KUNIT_TEST=y > +CONFIG_DRM_FBDEV_EMULATION=n > > base-commit: 45b58669e532bcdfd6e1593488d1f23eabd55428 Changing the local config should not be required after fixing the Kconfig files. Arnd
"Arnd Bergmann" <arnd@arndb.de> writes: > On Thu, Jul 27, 2023, at 00:03, Arthur Grillo wrote: >> Using the `kunit_tool` with the command: >> >> tools/testing/kunit/kunit.py run --kunitconfig=drivers/gpu/drm/tests/ >> >> Lead to this error[0]. Fix it by expliciting removing the >> CONFIG_DRM_FBDEV_EMULATION. >> >> [0] >> ERROR:root: >> WARNING: unmet direct dependencies detected for FRAMEBUFFER_CONSOLE >> Depends on [n]: VT [=n] && FB_CORE [=y] && !UML [=y] >> Selected by [y]: >> - DRM_FBDEV_EMULATION [=y] && HAS_IOMEM [=y] && DRM [=y] && !EXPERT [=n] >> > > I think that's a bug in the Kconfig files that should be fixed > by enforcing the correct set of dependencies. I have not encountered > this in my randconfig builds (with a lot of other fixes applied) > > In linux-next, CONFIG_VT cannot be disabled if CONFIG_EXPERT=n, > so this does not happen. > >> diff --git a/drivers/gpu/drm/tests/.kunitconfig >> b/drivers/gpu/drm/tests/.kunitconfig >> index 6ec04b4c979d..c50b5a12faae 100644 >> --- a/drivers/gpu/drm/tests/.kunitconfig >> +++ b/drivers/gpu/drm/tests/.kunitconfig >> @@ -1,3 +1,4 @@ >> CONFIG_KUNIT=y >> CONFIG_DRM=y >> CONFIG_DRM_KUNIT_TEST=y >> +CONFIG_DRM_FBDEV_EMULATION=n >> >> base-commit: 45b58669e532bcdfd6e1593488d1f23eabd55428 > > Changing the local config should not be required after fixing > the Kconfig files. > CONFIG_VT can only be disabled if CONFIG_EXPERT=y but I also see that it does not default to 'y' if !UML. Also FRAMEBUFFER_CONSOLE depends on !UML but DRM_FBDEV_EMULATION selects FRAMEBUFFER_CONSOLE if !EXPERT. Maybe we should include !UML in that condition to? Something like this: diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig index 0d499669d653..734332f222ea 100644 --- a/drivers/gpu/drm/Kconfig +++ b/drivers/gpu/drm/Kconfig @@ -135,7 +135,7 @@ config DRM_DEBUG_MODESET_LOCK config DRM_FBDEV_EMULATION bool "Enable legacy fbdev support for your modesetting driver" depends on DRM - select FRAMEBUFFER_CONSOLE if !EXPERT + select FRAMEBUFFER_CONSOLE if (!EXPERT && !UML) select FRAMEBUFFER_CONSOLE_DETECT_PRIMARY if FRAMEBUFFER_CONSOLE default y help With that I'm able to run the DRM kunit tests wihtout the mentioned problem. But I'm not sure if that is the correct fix or not.
On 27/07/23 13:07, Javier Martinez Canillas wrote: > "Arnd Bergmann" <arnd@arndb.de> writes: > >> On Thu, Jul 27, 2023, at 00:03, Arthur Grillo wrote: >>> Using the `kunit_tool` with the command: >>> >>> tools/testing/kunit/kunit.py run --kunitconfig=drivers/gpu/drm/tests/ >>> >>> Lead to this error[0]. Fix it by expliciting removing the >>> CONFIG_DRM_FBDEV_EMULATION. >>> >>> [0] >>> ERROR:root: >>> WARNING: unmet direct dependencies detected for FRAMEBUFFER_CONSOLE >>> Depends on [n]: VT [=n] && FB_CORE [=y] && !UML [=y] >>> Selected by [y]: >>> - DRM_FBDEV_EMULATION [=y] && HAS_IOMEM [=y] && DRM [=y] && !EXPERT [=n] >>> >> >> I think that's a bug in the Kconfig files that should be fixed >> by enforcing the correct set of dependencies. I have not encountered >> this in my randconfig builds (with a lot of other fixes applied) >> >> In linux-next, CONFIG_VT cannot be disabled if CONFIG_EXPERT=n, >> so this does not happen. >> > >>> diff --git a/drivers/gpu/drm/tests/.kunitconfig >>> b/drivers/gpu/drm/tests/.kunitconfig >>> index 6ec04b4c979d..c50b5a12faae 100644 >>> --- a/drivers/gpu/drm/tests/.kunitconfig >>> +++ b/drivers/gpu/drm/tests/.kunitconfig >>> @@ -1,3 +1,4 @@ >>> CONFIG_KUNIT=y >>> CONFIG_DRM=y >>> CONFIG_DRM_KUNIT_TEST=y >>> +CONFIG_DRM_FBDEV_EMULATION=n >>> >>> base-commit: 45b58669e532bcdfd6e1593488d1f23eabd55428 >> >> Changing the local config should not be required after fixing >> the Kconfig files. >> > > CONFIG_VT can only be disabled if CONFIG_EXPERT=y but I also see that it > does not default to 'y' if !UML. Also FRAMEBUFFER_CONSOLE depends on !UML > but DRM_FBDEV_EMULATION selects FRAMEBUFFER_CONSOLE if !EXPERT. > > Maybe we should include !UML in that condition to? Something like this: > > diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig > index 0d499669d653..734332f222ea 100644 > --- a/drivers/gpu/drm/Kconfig > +++ b/drivers/gpu/drm/Kconfig > @@ -135,7 +135,7 @@ config DRM_DEBUG_MODESET_LOCK > config DRM_FBDEV_EMULATION > bool "Enable legacy fbdev support for your modesetting driver" > depends on DRM > - select FRAMEBUFFER_CONSOLE if !EXPERT > + select FRAMEBUFFER_CONSOLE if (!EXPERT && !UML) > select FRAMEBUFFER_CONSOLE_DETECT_PRIMARY if FRAMEBUFFER_CONSOLE > default y > help > > > With that I'm able to run the DRM kunit tests wihtout the mentioned > problem. But I'm not sure if that is the correct fix or not. It works here too, I just don't understand why this commit caused this bug, as it did not touch this line. >
On 27/07/23 05:33, Arnd Bergmann wrote: > On Thu, Jul 27, 2023, at 00:03, Arthur Grillo wrote: >> Using the `kunit_tool` with the command: >> >> tools/testing/kunit/kunit.py run --kunitconfig=drivers/gpu/drm/tests/ >> >> Lead to this error[0]. Fix it by expliciting removing the >> CONFIG_DRM_FBDEV_EMULATION. >> >> [0] >> ERROR:root: >> WARNING: unmet direct dependencies detected for FRAMEBUFFER_CONSOLE >> Depends on [n]: VT [=n] && FB_CORE [=y] && !UML [=y] >> Selected by [y]: >> - DRM_FBDEV_EMULATION [=y] && HAS_IOMEM [=y] && DRM [=y] && !EXPERT [=n] >> > > I think that's a bug in the Kconfig files that should be fixed > by enforcing the correct set of dependencies. I have not encountered Agree, I also didn't find the error on the dependencies, so I made this patch to see what you guys thought. Maybe Javier's take is the correct fix. ~Arthur Grillo > this in my randconfig builds (with a lot of other fixes applied) > > In linux-next, CONFIG_VT cannot be disabled if CONFIG_EXPERT=n, > so this does not happen. > >> diff --git a/drivers/gpu/drm/tests/.kunitconfig >> b/drivers/gpu/drm/tests/.kunitconfig >> index 6ec04b4c979d..c50b5a12faae 100644 >> --- a/drivers/gpu/drm/tests/.kunitconfig >> +++ b/drivers/gpu/drm/tests/.kunitconfig >> @@ -1,3 +1,4 @@ >> CONFIG_KUNIT=y >> CONFIG_DRM=y >> CONFIG_DRM_KUNIT_TEST=y >> +CONFIG_DRM_FBDEV_EMULATION=n >> >> base-commit: 45b58669e532bcdfd6e1593488d1f23eabd55428 > > Changing the local config should not be required after fixing > the Kconfig files. > > Arnd
Arthur Grillo Queiroz Cabral <arthurgrillo@riseup.net> writes: Hello Arthur, > On 27/07/23 13:07, Javier Martinez Canillas wrote: >> "Arnd Bergmann" <arnd@arndb.de> writes: >> [...] >>> Changing the local config should not be required after fixing >>> the Kconfig files. >>> >> >> CONFIG_VT can only be disabled if CONFIG_EXPERT=y but I also see that it >> does not default to 'y' if !UML. Also FRAMEBUFFER_CONSOLE depends on !UML >> but DRM_FBDEV_EMULATION selects FRAMEBUFFER_CONSOLE if !EXPERT. >> >> Maybe we should include !UML in that condition to? Something like this: >> >> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig >> index 0d499669d653..734332f222ea 100644 >> --- a/drivers/gpu/drm/Kconfig >> +++ b/drivers/gpu/drm/Kconfig >> @@ -135,7 +135,7 @@ config DRM_DEBUG_MODESET_LOCK >> config DRM_FBDEV_EMULATION >> bool "Enable legacy fbdev support for your modesetting driver" >> depends on DRM >> - select FRAMEBUFFER_CONSOLE if !EXPERT >> + select FRAMEBUFFER_CONSOLE if (!EXPERT && !UML) >> select FRAMEBUFFER_CONSOLE_DETECT_PRIMARY if FRAMEBUFFER_CONSOLE >> default y >> help >> >> >> With that I'm able to run the DRM kunit tests wihtout the mentioned >> problem. But I'm not sure if that is the correct fix or not. > > It works here too, I just don't understand why this commit caused this > bug, as it did not touch this line. Yes, I also don't understand why the FB_CORE split made it more likely to happen since AFAICT the same problem could had happen with just CONFIG_FB.
On Thu, Jul 27, 2023, at 18:45, Javier Martinez Canillas wrote: > Arthur Grillo Queiroz Cabral <arthurgrillo@riseup.net> writes: >> On 27/07/23 13:07, Javier Martinez Canillas wrote: >>> "Arnd Bergmann" <arnd@arndb.de> writes: >>>> Changing the local config should not be required after fixing >>>> the Kconfig files. >>>> >>> >>> CONFIG_VT can only be disabled if CONFIG_EXPERT=y but I also see that it >>> does not default to 'y' if !UML. Also FRAMEBUFFER_CONSOLE depends on !UML >>> but DRM_FBDEV_EMULATION selects FRAMEBUFFER_CONSOLE if !EXPERT. >>> >>> Maybe we should include !UML in that condition to? Something like this: >>> >>> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig >>> index 0d499669d653..734332f222ea 100644 >>> --- a/drivers/gpu/drm/Kconfig >>> +++ b/drivers/gpu/drm/Kconfig >>> @@ -135,7 +135,7 @@ config DRM_DEBUG_MODESET_LOCK >>> config DRM_FBDEV_EMULATION >>> bool "Enable legacy fbdev support for your modesetting driver" >>> depends on DRM >>> - select FRAMEBUFFER_CONSOLE if !EXPERT >>> + select FRAMEBUFFER_CONSOLE if (!EXPERT && !UML) Yes, that should work. Was the original bug report about UML then? I'm not actually sure we need the select at all. When I tried to narrow down how fbdev is used in the previous thread, the answer was pretty much that it could be used in any possible way, so there might be users that only want the /dev/fb0 interface but not the console, or even just the logo. Another thing we could do here would be config DRM_FBDEV_EMULATION select FRAMEBUFFER_CONSOLE if VT which is simpler and probably just as good. Or if we decide that DRM_FBDEV_EMULATION is in fact only useful for FRAMEBUFFER_CONSOLE and add 'depends on VT' and removed the "if (...)" >>> With that I'm able to run the DRM kunit tests wihtout the mentioned >>> problem. But I'm not sure if that is the correct fix or not. >> >> It works here too, I just don't understand why this commit caused this >> bug, as it did not touch this line. > > Yes, I also don't understand why the FB_CORE split made it more likely to > happen since AFAICT the same problem could had happen with just CONFIG_FB. c242f48433e79 ("drm: Make FB_CORE to be selected if DRM fbdev emulation is enabled") changed DRM_FBDEV_EMULATION from 'depends on FB' to an effective 'select FB_CORE', so any config that previously had DRM=y and FB=n now has FB_CORE=y and FRAMEBUFFER_CONSOLE=y. Arnd
"Arnd Bergmann" <arnd@arndb.de> writes: [adding Randy Dunlap who also reported the same issue] Hello Arnd, > On Thu, Jul 27, 2023, at 18:45, Javier Martinez Canillas wrote: >> Arthur Grillo Queiroz Cabral <arthurgrillo@riseup.net> writes: >>> On 27/07/23 13:07, Javier Martinez Canillas wrote: >>>> "Arnd Bergmann" <arnd@arndb.de> writes: >>>>> Changing the local config should not be required after fixing >>>>> the Kconfig files. >>>>> >>>> >>>> CONFIG_VT can only be disabled if CONFIG_EXPERT=y but I also see that it >>>> does not default to 'y' if !UML. Also FRAMEBUFFER_CONSOLE depends on !UML >>>> but DRM_FBDEV_EMULATION selects FRAMEBUFFER_CONSOLE if !EXPERT. >>>> >>>> Maybe we should include !UML in that condition to? Something like this: >>>> >>>> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig >>>> index 0d499669d653..734332f222ea 100644 >>>> --- a/drivers/gpu/drm/Kconfig >>>> +++ b/drivers/gpu/drm/Kconfig >>>> @@ -135,7 +135,7 @@ config DRM_DEBUG_MODESET_LOCK >>>> config DRM_FBDEV_EMULATION >>>> bool "Enable legacy fbdev support for your modesetting driver" >>>> depends on DRM >>>> - select FRAMEBUFFER_CONSOLE if !EXPERT >>>> + select FRAMEBUFFER_CONSOLE if (!EXPERT && !UML) > > Yes, that should work. Was the original bug report about UML then? > By original do you mean Arthur's report or Randy's? But yes, in both cases CONFIG_UML=y when the issue was seen. > I'm not actually sure we need the select at all. When I tried > to narrow down how fbdev is used in the previous > thread, the answer was pretty much that it could be used > in any possible way, so there might be users that only want > the /dev/fb0 interface but not the console, or even just > the logo. > Yes, I agree with you. Maybe then the fix could be to just drop that select? > Another thing we could do here would be > > config DRM_FBDEV_EMULATION > select FRAMEBUFFER_CONSOLE if VT > > which is simpler and probably just as good. Or if we decide that > DRM_FBDEV_EMULATION is in fact only useful for FRAMEBUFFER_CONSOLE > and add 'depends on VT' and removed the "if (...)" > As mentioned I don't think thatis only useful with fbcon/vt and there should be possible to enable the fbdev DRM emulation even without it. >>>> With that I'm able to run the DRM kunit tests wihtout the mentioned >>>> problem. But I'm not sure if that is the correct fix or not. >>> >>> It works here too, I just don't understand why this commit caused this >>> bug, as it did not touch this line. >> >> Yes, I also don't understand why the FB_CORE split made it more likely to >> happen since AFAICT the same problem could had happen with just CONFIG_FB. > > c242f48433e79 ("drm: Make FB_CORE to be selected if DRM fbdev emulation > is enabled") changed DRM_FBDEV_EMULATION from 'depends on FB' to > an effective 'select FB_CORE', so any config that previously had > DRM=y and FB=n now has FB_CORE=y and FRAMEBUFFER_CONSOLE=y. > Ah, right. I see it now. Thanks for the explanation.
Javier Martinez Canillas <javierm@redhat.com> writes: > "Arnd Bergmann" <arnd@arndb.de> writes: > > [adding Randy Dunlap who also reported the same issue] > > Hello Arnd, > >> On Thu, Jul 27, 2023, at 18:45, Javier Martinez Canillas wrote: >>> Arthur Grillo Queiroz Cabral <arthurgrillo@riseup.net> writes: >>>> On 27/07/23 13:07, Javier Martinez Canillas wrote: >>>>> "Arnd Bergmann" <arnd@arndb.de> writes: >>>>>> Changing the local config should not be required after fixing >>>>>> the Kconfig files. >>>>>> >>>>> >>>>> CONFIG_VT can only be disabled if CONFIG_EXPERT=y but I also see that it >>>>> does not default to 'y' if !UML. Also FRAMEBUFFER_CONSOLE depends on !UML >>>>> but DRM_FBDEV_EMULATION selects FRAMEBUFFER_CONSOLE if !EXPERT. >>>>> >>>>> Maybe we should include !UML in that condition to? Something like this: >>>>> >>>>> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig >>>>> index 0d499669d653..734332f222ea 100644 >>>>> --- a/drivers/gpu/drm/Kconfig >>>>> +++ b/drivers/gpu/drm/Kconfig >>>>> @@ -135,7 +135,7 @@ config DRM_DEBUG_MODESET_LOCK >>>>> config DRM_FBDEV_EMULATION >>>>> bool "Enable legacy fbdev support for your modesetting driver" >>>>> depends on DRM >>>>> - select FRAMEBUFFER_CONSOLE if !EXPERT >>>>> + select FRAMEBUFFER_CONSOLE if (!EXPERT && !UML) >> >> Yes, that should work. Was the original bug report about UML then? >> > > By original do you mean Arthur's report or Randy's? But yes, in both cases > CONFIG_UML=y when the issue was seen. > >> I'm not actually sure we need the select at all. When I tried >> to narrow down how fbdev is used in the previous >> thread, the answer was pretty much that it could be used >> in any possible way, so there might be users that only want >> the /dev/fb0 interface but not the console, or even just >> the logo. >> > > Yes, I agree with you. Maybe then the fix could be to just drop that select? > I've posted a patch [0] doing this as suggested by Arnd. Let me know what you think. [0]: https://lists.freedesktop.org/archives/dri-devel/2023-August/417565.html
diff --git a/drivers/gpu/drm/tests/.kunitconfig b/drivers/gpu/drm/tests/.kunitconfig index 6ec04b4c979d..c50b5a12faae 100644 --- a/drivers/gpu/drm/tests/.kunitconfig +++ b/drivers/gpu/drm/tests/.kunitconfig @@ -1,3 +1,4 @@ CONFIG_KUNIT=y CONFIG_DRM=y CONFIG_DRM_KUNIT_TEST=y +CONFIG_DRM_FBDEV_EMULATION=n
Using the `kunit_tool` with the command: tools/testing/kunit/kunit.py run --kunitconfig=drivers/gpu/drm/tests/ Lead to this error[0]. Fix it by expliciting removing the CONFIG_DRM_FBDEV_EMULATION. [0] ERROR:root: WARNING: unmet direct dependencies detected for FRAMEBUFFER_CONSOLE Depends on [n]: VT [=n] && FB_CORE [=y] && !UML [=y] Selected by [y]: - DRM_FBDEV_EMULATION [=y] && HAS_IOMEM [=y] && DRM [=y] && !EXPERT [=n] WARNING: unmet direct dependencies detected for FRAMEBUFFER_CONSOLE_DETECT_PRIMARY Depends on [n]: VT [=n] && FRAMEBUFFER_CONSOLE [=y] Selected by [y]: - DRM_FBDEV_EMULATION [=y] && HAS_IOMEM [=y] && DRM [=y] && FRAMEBUFFER_CONSOLE [=y] WARNING: unmet direct dependencies detected for FRAMEBUFFER_CONSOLE Depends on [n]: VT [=n] && FB_CORE [=y] && !UML [=y] Selected by [y]: - DRM_FBDEV_EMULATION [=y] && HAS_IOMEM [=y] && DRM [=y] && !EXPERT [=n] WARNING: unmet direct dependencies detected for FRAMEBUFFER_CONSOLE_DETECT_PRIMARY Depends on [n]: VT [=n] && FRAMEBUFFER_CONSOLE [=y] Selected by [y]: - DRM_FBDEV_EMULATION [=y] && HAS_IOMEM [=y] && DRM [=y] && FRAMEBUFFER_CONSOLE [=y] WARNING: unmet direct dependencies detected for FRAMEBUFFER_CONSOLE Depends on [n]: VT [=n] && FB_CORE [=y] && !UML [=y] Selected by [y]: - DRM_FBDEV_EMULATION [=y] && HAS_IOMEM [=y] && DRM [=y] && !EXPERT [=n] WARNING: unmet direct dependencies detected for FRAMEBUFFER_CONSOLE_DETECT_PRIMARY Depends on [n]: VT [=n] && FRAMEBUFFER_CONSOLE [=y] Selected by [y]: - DRM_FBDEV_EMULATION [=y] && HAS_IOMEM [=y] && DRM [=y] && FRAMEBUFFER_CONSOLE [=y] /usr/bin/ld: drivers/video/fbdev/core/fbcon.o: in function `fbcon_suspended': fbcon.c:(.text+0x1c): undefined reference to `vc_cons' /usr/bin/ld: drivers/video/fbdev/core/fbcon.o: in function `fbcon_cursor': fbcon.c:(.text+0x1ac): undefined reference to `console_blanked' /usr/bin/ld: drivers/video/fbdev/core/fbcon.o: in function `fbcon_resumed': fbcon.c:(.text+0x39c): undefined reference to `vc_cons' /usr/bin/ld: fbcon.c:(.text+0x3aa): undefined reference to `redraw_screen' /usr/bin/ld: drivers/video/fbdev/core/fbcon.o: in function `fbcon_update_vcs': . . . make[3]: *** [../scripts/Makefile.vmlinux:36: vmlinux] Error 1 make[2]: *** [/home/grillo/projects/linux/Makefile:1238: vmlinux] Error 2 make[1]: *** [/home/grillo/projects/linux/Makefile:234: __sub-make] Error 2 make: *** [Makefile:234: __sub-make] Error 2 Fixes: c242f48433e7 ("drm: Make FB_CORE to be selected if DRM fbdev emulation is enabled") Signed-off-by: Arthur Grillo <arthurgrillo@riseup.net> --- drivers/gpu/drm/tests/.kunitconfig | 1 + 1 file changed, 1 insertion(+) base-commit: 45b58669e532bcdfd6e1593488d1f23eabd55428