Message-ID: <1aef3a8f-f93e-2824-6b6e-8df8a1460f15@synopsys.com>
Date:   Tue, 8 Feb 2022 22:42:36 +0000
From:   Thinh Nguyen <Thinh.Nguyen@...opsys.com>
To:     Randy Dunlap <rdunlap@...radead.org>,
        Thinh Nguyen <Thinh.Nguyen@...opsys.com>,
        Arnd Bergmann <arnd@...nel.org>
CC:     Fabio Estevam <festevam@...il.com>,
        Kees Cook <keescook@...omium.org>,
        Daniel Vetter <daniel.vetter@...ll.ch>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "dri-devel@...ts.freedesktop.org" <dri-devel@...ts.freedesktop.org>,
        John Youn <John.Youn@...opsys.com>,
        Bing Yuan <Bing.Yuan@...opsys.com>,
        "linux-mediatek@...ts.infradead.org" 
        <linux-mediatek@...ts.infradead.org>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>
Subject: Re: Kconfig CONFIG_FB dependency regression

Randy Dunlap wrote:
> Hi--
> 
> On 2/8/22 12:10, Thinh Nguyen wrote:
>> Randy Dunlap wrote:
>>>
>>>
>>> On 2/3/22 19:21, Thinh Nguyen wrote:
>>>> Arnd Bergmann wrote:
>>>>> On Thu, Feb 3, 2022 at 12:55 AM Thinh Nguyen <Thinh.Nguyen@...opsys.com> wrote:
>>>>>> Arnd Bergmann wrote:
>>>>>>> On Wed, Feb 2, 2022 at 1:14 AM Thinh Nguyen <Thinh.Nguyen@...opsys.com> wrote:
>>>>>>>> Fabio Estevam wrote:
>>>>>>>
>>>>>>> CONFIG_FB should not normally be needed for booting, so unless
>>>>>>> you have a graphical application in your initramfs that requires the /dev/fb0
>>>>>>> device to work, it is not supposed to make a difference.
>>>>>>>
>>>>>>
>>>>>> I'm not sure, but it seems like the setup we have isn't the only one
>>>>>> that needed it. Fabio also noted that the imx_v6_v7_defconfig also needs
>>>>>> to have CONFIG_FB set.
>>>>>
>>>>> No, that one is different: the change for imx_v6_v7_defconfig was
>>>>> done because they actually use a framebuffer console on some devices,
>>>>> so the patch just adds the symbol to enable the drivers they are using.
>>>>>
>>>>> This is expected with my original patch that doesn't implicitly enable
>>>>> the framebuffer layer any more. What is not expected is for the kernel
>>>>> to hang during boot as you reported for your unidentified platform.
>>>>>
>>>>>>> Are there any other differences in your .config before and after the patch?
>>>>>>> It's possible that you use some other driver that in turn depends on
>>>>>>> CONFIG_FB. Does your machine have any graphical output device?
>>>>>>> If yes, which driver do you use?
>>>>>>
>>>>>> I don't have the answer to those questions yet. Need more investigation.
>>>>>> I'm new to this particular test setup.
>>>>>
>>>>> Do you mean you don't know if there is a screen attached to the system?
>>>>>
>>>>
>>>> It does have a graphical output device, but I didn't check what it is or
>>>> what driver is driving it. I just notice that after the reported commit,
>>>> something stopped working.
>>>>
>>>>>>>
>>>>>>> You may also want to make sure that you have 9d6366e743f3 ("drm:
>>>>>>> fb_helper: improve CONFIG_FB dependency") in your kernel, which
>>>>>>> fixes a minor problem with my original patch.
>>>>>>>
>>>>>>
>>>>>> The issue also occurs in mainline, which has your minor fix commit
>>>>>> above. The revert isn't clean for the latest kernel version. I also have
>>>>>> to revert some of the changes along with CONFIG_FB. The revert looks
>>>>>> more like this for the latest kernel:
>>>>>>
>>>>>> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
>>>>>> index b1f22e457fd0..7cbc733a8569 100644
>>>>>> --- a/drivers/gpu/drm/Kconfig
>>>>>> +++ b/drivers/gpu/drm/Kconfig
>>>>>> @@ -118,8 +118,9 @@ config DRM_DEBUG_MODESET_LOCK
>>>>>>
>>>>>>  config DRM_FBDEV_EMULATION
>>>>>>         bool "Enable legacy fbdev support for your modesetting driver"
>>>>>> -       depends on DRM_KMS_HELPER
>>>>>> -       depends on FB=y || FB=DRM_KMS_HELPER
>>>>>> +       depends on DRM
>>>>>> +       select DRM_KMS_HELPER
>>>>>> +       select FB
>>>>>>         select FB_CFB_FILLRECT
>>>>>>         select FB_CFB_COPYAREA
>>>>>>         select FB_CFB_IMAGEBLIT
>>>>>>
>>>>>>
>>>>>>
>>>>>> I attached the configs for kernel v5.17-rc1. The "bad" config is without
>>>>>> any revert, the "good" config is with the change above.
>>>>>
>>>>> Looking at the config, I see that this is for an x86 machine,
>>>>> and you have the FB_EFI driver and EFI_EARLYCON enabled.
>>>>>
>>>>> What I suspec is going on is that you are looking at a screen rather
>>>>> than a serial console, and the kernel doesn't actually hang but you
>>>>> just don't see any more messages after the DRM driver takes
>>>>> over from EFI_EARLYCON because there is no console driver.
>>>>>
>>>>> In this case, what you see is the intended behavior, not a bug.
>>>>> If you want a graphical console in your system, you need to
>>>>> enable the support for this in your config.
>>>>>
>>>>
>>>> It sounds like that's the case. Unfortunately I'm not familiar with this
>>>> subsystem to say that's what happening. If there's nothing actually
>>>> broken from review, we can ignore this email thread.
>>>
>>> Hi,
>>> I don't know of anything that is broken...
>>>
>>> I am curious how CONFIG_FB_EFI came to be set when going from bad.config to
>>> good.config.  Can you explain that?
>>>
>>
>> I just use the change above and "make" with olddefconfig option. Is it
>> not expected?
> 
> Maybe I am not doing the same as you. If I take your previous bad.config
> with kernel v5.17-rc2 and use your Kconfig patch:
> 
> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> index b1f22e457fd0..7cbc733a8569 100644
> --- a/drivers/gpu/drm/Kconfig
> +++ b/drivers/gpu/drm/Kconfig
> @@ -118,8 +118,9 @@ config DRM_DEBUG_MODESET_LOCK
> 
>  config DRM_FBDEV_EMULATION
>         bool "Enable legacy fbdev support for your modesetting driver"
> -       depends on DRM_KMS_HELPER
> -       depends on FB=y || FB=DRM_KMS_HELPER
> +       depends on DRM
> +       select DRM_KMS_HELPER
> +       select FB
>         select FB_CFB_FILLRECT
>         select FB_CFB_COPYAREA
>         select FB_CFB_IMAGEBLIT
> 
> and then run
> 'make olddefconfig', I see:
> 
> # CONFIG_FB_EFI is not set
> 
> which is what I would expect to see.
> 

Ah.. It's because I don't use old.config as the base config. I use
x86_64_defconfig as the base plus some additional configs I need, and it
has CONFIG_FB_EFI set by default.

BR,
Thinh

Powered by blists - more mailing lists