Hi Jocelyn,
On Thu, Oct 05, 2023 at 11:16:15AM +0200, Jocelyn Falempe wrote:
> On 05/10/2023 10:18, Maxime Ripard wrote:
> > Hi,
> >
> > On Tue, Oct 03, 2023 at 04:22:45PM +0200, Jocelyn Falempe wrote:
> > > diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
> > > index 89e2706cac56..e538c87116d3 100644
> > > --- a/include/drm/drm_drv.h
> > > +++ b/include/drm/drm_drv.h
> > > @@ -43,6 +43,7 @@ struct dma_buf_attachment;
> > > struct drm_display_mode;
> > > struct drm_mode_create_dumb;
> > > struct drm_printer;
> > > +struct drm_scanout_buffer;
> > > struct sg_table;
> > > /**
> > > @@ -408,6 +409,19 @@ struct drm_driver {
> > > */
> > > void (*show_fdinfo)(struct drm_printer *p, struct drm_file *f);
> > > + /**
> > > + * @get_scanout_buffer:
> > > + *
> > > + * Get the current scanout buffer, to display a panic message with drm_panic.
> > > + * It is called from a panic callback, and must follow its restrictions.
> > > + *
> > > + * Returns:
> > > + *
> > > + * Zero on success, negative errno on failure.
> > > + */
> > > + int (*get_scanout_buffer)(struct drm_device *dev,
> > > + struct drm_scanout_buffer *sb);
> > > +
> >
> > What is the format of that buffer? What is supposed to happen if the
> > planes / CRTC are setup in a way that is incompatible with the buffer
> > format?
>
> Currently, it only supports linear format, either in system memory, or
> iomem.
> But really what is needed is the screen size, and a way to write pixels to
> it.
> For more complex GPU, I don't know if it's easier to reprogram the GPU to
> linear format, or to add a simple "tiled" support to drm_panic.
> What would you propose as a panic interface to handle those complex format ?
It's not just about tiling, but also about YUV formats. If the display
engine is currently playing a video at the moment, it's probably going
to output some variation of multi-planar YUV and you won't have an RGB
buffer available.
Same story if you're using a dma-buf buffer. You might not even be able
to access that buffer at all from the CPU or the kernel.
I really think we should have some emergency state ready to commit on
the side, and possibly a panic_commit function to prevent things like
sleeping or waiting that regular atomic_commit can use.
That way, you know have all the resources available to you any time.
> Sometime it's also just not possible to write pixels to the screen, like if
> the panic occurs in the middle of suspend/resume, or during a mode-setting,
> and the hardware state is broken. In this case it's ok to return an error,
> and nothing will get displayed.
And yeah, you won't be able to do it every time, but if it's never for
some workload it's going to be a concern.
Anyway, we should at the very least document what we expect here.
Maxime
Attachment:
signature.asc
Description: PGP signature
