Jocelyn Falempe <jfalempe@xxxxxxxxxx> writes: > On 17/06/2024 10:25, Geert Uytterhoeven wrote: >> Hi Jocelyn, >> >> On Mon, Jun 17, 2024 at 10:16 AM Jocelyn Falempe <jfalempe@xxxxxxxxxx> wrote: >>> On 16/06/2024 14:43, Javier Martinez Canillas wrote: >>>> Jocelyn Falempe <jfalempe@xxxxxxxxxx> writes: >>>>> The race condition between fbcon and drm_panic can only occurs if >>>>> VT_CONSOLE is set. So update drm_panic dependency accordingly. >>>>> This will make it easier for Linux distributions to enable drm_panic >>>>> by disabling VT_CONSOLE, and keeping fbcon terminal. >>>>> The only drawback is that fbcon won't display the boot kmsg, so it >>>>> should rely on userspace to do that. >>>>> At least plymouth already handle this case with >>>>> https://gitlab.freedesktop.org/plymouth/plymouth/-/merge_requests/224 >>>>> >>>>> Suggested-by: Daniel Vetter <daniel.vetter@xxxxxxxx> >>>>> Signed-off-by: Jocelyn Falempe <jfalempe@xxxxxxxxxx> >>>>> --- >>>>> drivers/gpu/drm/Kconfig | 2 +- >>>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>>> >>>>> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig >>>>> index a9df94291622..f5c989aed7e9 100644 >>>>> --- a/drivers/gpu/drm/Kconfig >>>>> +++ b/drivers/gpu/drm/Kconfig >>>>> @@ -107,7 +107,7 @@ config DRM_KMS_HELPER >>>>> >>>>> config DRM_PANIC >>>>> bool "Display a user-friendly message when a kernel panic occurs" >>>>> - depends on DRM && !FRAMEBUFFER_CONSOLE >>>>> + depends on DRM && !(FRAMEBUFFER_CONSOLE && VT_CONSOLE) >>>> >>>> I thought the idea was to only make it depend on !VT_CONSOLE, so that >>>> distros could also enable fbcon / VT but prevent the race condition to >>>> happen due the VT not being a system console for the kernel to print >>>> messages ? >>> >>> Yes, but when writing the patch, I thought that if you have VT_CONSOLE=y >>> and FRAMEBUFFER_CONSOLE=n, then there won't be any race condition, and >>> drm_panic can be enabled safely. >>> I don't know if that really matters, and if VT_CONSOLE has any usage >>> apart from fbcon. >> >> It is used for any kind of virtual terminal, so also for vgacon. > > Ah thanks, but I think vgacon is safe to use with drm_panic. > > The problem with fbcon, is that it can also uses the fbdev emulation > from the drm driver, and in this case, drm_panic will write to the same > framebuffer as fbcon during a panic, leading to corrupted output. > This can't occur with other graphical console, so !(FRAMEBUFFER_CONSOLE > && VT_CONSOLE) is probably good. > Yeah, I agre that !(FRAMEBUFFER_CONSOLE && VT_CONSOLE) as you have in your patch is what makes sense. !VT_CONSOLE alone as I argued in my first email isn't correct since as you pointed out, it shouldn't affect other consoles besides fbcon. So please feel free to also add: Reviewed-by: Javier Martinez Canillas <javierm@xxxxxxxxxx> -- Best regards, Javier Martinez Canillas Core Platforms Red Hat
