Re: [PATCH] drm/tests: Remove CONFIG_DRM_FBDEV_EMULATION on .kunitconfig

[Javier Martinez Canillas <javierm@redhat.com>]

"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.

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat