Re: [PATCH] drm/tests: Remove CONFIG_DRM_FBDEV_EMULATION on .kunitconfig
Javier Martinez Canillas writes: > "Arnd Bergmann" 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 writes: On 27/07/23 13:07, Javier Martinez Canillas wrote: > "Arnd Bergmann" 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 -- Best regards, Javier Martinez Canillas Core Platforms Red Hat
Re: [PATCH] drm/tests: Remove CONFIG_DRM_FBDEV_EMULATION on .kunitconfig
"Arnd Bergmann" 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 writes:
>>> On 27/07/23 13:07, Javier Martinez Canillas wrote:
"Arnd Bergmann" 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.
--
Best regards,
Javier Martinez Canillas
Core Platforms
Red Hat
Re: [PATCH] drm/tests: Remove CONFIG_DRM_FBDEV_EMULATION on .kunitconfig
On Thu, Jul 27, 2023, at 18:45, Javier Martinez Canillas wrote:
> Arthur Grillo Queiroz Cabral writes:
>> On 27/07/23 13:07, Javier Martinez Canillas wrote:
>>> "Arnd Bergmann" 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
Re: [PATCH] drm/tests: Remove CONFIG_DRM_FBDEV_EMULATION on .kunitconfig
Arthur Grillo Queiroz Cabral writes: Hello Arthur, > On 27/07/23 13:07, Javier Martinez Canillas wrote: >> "Arnd Bergmann" 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. -- Best regards, Javier Martinez Canillas Core Platforms Red Hat
Re: [PATCH] drm/tests: Remove CONFIG_DRM_FBDEV_EMULATION on .kunitconfig
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
Re: [PATCH] drm/tests: Remove CONFIG_DRM_FBDEV_EMULATION on .kunitconfig
On 27/07/23 13:07, Javier Martinez Canillas wrote: > "Arnd Bergmann" 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. >
Re: [PATCH] drm/tests: Remove CONFIG_DRM_FBDEV_EMULATION on .kunitconfig
"Arnd Bergmann" 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. -- Best regards, Javier Martinez Canillas Core Platforms Red Hat
Re: [PATCH] drm/tests: Remove CONFIG_DRM_FBDEV_EMULATION on .kunitconfig
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
