[PATCH] drm/panic: depends on !VT_CONSOLE

[Jocelyn Falempe]

On 16/06/2024 14:43, Javier Martinez Canillas wrote:
> Jocelyn Falempe <jfalempe at redhat.com> writes:
> 
> Hello Jocelyn,
> 
>> 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 at ffwll.ch>
>> Signed-off-by: Jocelyn Falempe <jfalempe at redhat.com>
>> ---
>>   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.

> 
> In other words, my understanding from the discussion with Sima was that
> this should be instead:
> 
>   +	depends on DRM && !VT_CONSOLE
> 
> I've tested that and at least I see that a framebuffer console is present
> and `echo c > /proc/sysrq-trigger` triggers the DRM panic handler message
> (but don't know if the race exists and is just that I was not hitting it).
> 
> If my understanding is correct and should only be a depends on !VT_CONSOLE
> then feel free to add my:
> 
> Tested-by: Javier Martinez Canillas <javierm at redhat.com>
> 

Best regards,

-- 

Jocelyn