Jocelyn Falempe <jfalempe@xxxxxxxxxx> writes:
Hello Jocelyn, thanks for your feedback!
> On 21/06/2024 00:22, Javier Martinez Canillas wrote:
>> Add support for the drm_panic infrastructure, which allows to display
>> a user friendly message on the screen when a Linux kernel panic occurs.
>>
>> The display controller doesn't scanout the framebuffer, but instead the
>> pixels are sent to the device using a transport bus. For this reason, a
>> .panic_flush handler is needed to flush the panic image to the display.
>
> Thanks for this patch, that's really cool that drm_panic can work on
> this device too.
>
Indeed, that's why I did it. Just to see if it could work :)
[...]
>> +static void ssd130x_primary_plane_helper_panic_flush(struct drm_plane *plane)
>> +{
>> + struct drm_plane_state *plane_state = plane->state;
>> + struct ssd130x_plane_state *ssd130x_plane_state = to_ssd130x_plane_state(plane_state);
>> + struct drm_shadow_plane_state *shadow_plane_state = to_drm_shadow_plane_state(plane_state);
>> + struct drm_crtc *crtc = plane_state->crtc;
>> + struct ssd130x_crtc_state *ssd130x_crtc_state = to_ssd130x_crtc_state(crtc->state);
>> +
>> + ssd130x_fb_blit_rect(plane_state->fb, &shadow_plane_state->data[0], &plane_state->dst,
>> + ssd130x_plane_state->buffer, ssd130x_crtc_state->data_array,
>> + &shadow_plane_state->fmtcnv_state);
>
> ssd130x_fb_blit_rect() will call regmap->write(), which involve mutex
> and might sleep. And if the mutex is taken when the panic occurs, it
> might deadlock the panic handling.
That's a good point and I something haven't considered...
> One solution would be to configure the regmap with config->fast_io and
> config->use_raw_spinlock, and check that the lock is available with
> try_lock(map->raw_spin_lock)
> But that means it will waste cpu cycle with busy waiting for normal
> operation, which is not good.
>
Yeah, I would prefer to not change the driver for normal operation.
> So for this particular device, I think it's ok, because it's unlikely
> you'll run kdump or other kernel panic handlers.
> But I would like to know what others think about it, and if it's
> acceptable or not.
>
I don't know either. I guess that after a panic everything is best effort
anyways so it may be acceptable? But let's see what others think about it.
> --
>
> Jocelyn
--
Best regards,
Javier Martinez Canillas
Core Platforms
Red Hat
