[PATCH v8 5/8] drm/simpledrm: Add drm_panic support

[Jocelyn Falempe]

On 29/02/2024 12:17, Daniel Vetter wrote:
> On Tue, Feb 27, 2024 at 11:04:16AM +0100, Jocelyn Falempe wrote:
>> Add support for the drm_panic module, which displays a user-friendly
>> message to the screen when a kernel panic occurs.
>>
>> v8:
>>   * Replace get_scanout_buffer() with drm_panic_set_buffer()
>>     (Thomas Zimmermann)
>>
>> Signed-off-by: Jocelyn Falempe <jfalempe at redhat.com>
>> ---
>>   drivers/gpu/drm/tiny/simpledrm.c | 17 +++++++++++++++++
>>   1 file changed, 17 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/tiny/simpledrm.c b/drivers/gpu/drm/tiny/simpledrm.c
>> index 7ce1c4617675..a2190995354a 100644
>> --- a/drivers/gpu/drm/tiny/simpledrm.c
>> +++ b/drivers/gpu/drm/tiny/simpledrm.c
>> @@ -25,6 +25,7 @@
>>   #include <drm/drm_gem_shmem_helper.h>
>>   #include <drm/drm_managed.h>
>>   #include <drm/drm_modeset_helper_vtables.h>
>> +#include <drm/drm_panic.h>
>>   #include <drm/drm_probe_helper.h>
>>   
>>   #define DRIVER_NAME	"simpledrm"
>> @@ -735,6 +736,20 @@ static const struct drm_connector_funcs simpledrm_connector_funcs = {
>>   	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
>>   };
>>   
>> +static void simpledrm_init_panic_buffer(struct drm_plane *plane)
>> +{
>> +	struct simpledrm_device *sdev = simpledrm_device_of_dev(plane->dev);
>> +	struct drm_framebuffer fb;
>> +
>> +	/* Fake framebuffer struct for drm_panic_set_buffer */
>> +	fb.width = sdev->mode.hdisplay;
>> +	fb.height = sdev->mode.vdisplay;
>> +	fb.format = sdev->format;
>> +	fb.pitches[0] = sdev->pitch;
>> +
>> +	drm_panic_set_buffer(plane->panic_scanout, &fb, &sdev->screen_base);
>> +}
>> +
>>   static const struct drm_mode_config_funcs simpledrm_mode_config_funcs = {
>>   	.fb_create = drm_gem_fb_create_with_dirty,
>>   	.atomic_check = drm_atomic_helper_check,
>> @@ -945,6 +960,8 @@ static struct simpledrm_device *simpledrm_device_create(struct drm_driver *drv,
>>   		return ERR_PTR(ret);
>>   	drm_plane_helper_add(primary_plane, &simpledrm_primary_plane_helper_funcs);
>>   	drm_plane_enable_fb_damage_clips(primary_plane);
>> +	drm_panic_register(primary_plane);
> 
> Just a quick comment on this:
> 
> This does not work, the driver is not ready to handle panic calls at this
> stage. Instead we need to automatically register all planes that support
> panic handling in drm_dev_register(), and we need to remove them all again
> in drm_dev_unregister(). Outside of these functions it is not safe to call
> into driver code.

If you register the primary plane and didn't call drm_panic_set_buffer() 
yet, the panic handler will not do anything, so it should be safe.

But if we revert to using the get_scanout_buffer(), this makes sense.
> 
> At that point it might be simpler to only register one panic notifier per
> drm_device, and push the loop into the panic handler again.
> 
> Cheers, Sima
> 
>> +	simpledrm_init_panic_buffer(primary_plane);
>>   
>>   	/* CRTC */
>>   
>> -- 
>> 2.43.0
>>
>