Hi Am 18.01.24 um 11:17 schrieb Jocelyn Falempe:
On 17/01/2024 16:49, Thomas Zimmermann wrote:Hi Am 04.01.24 um 17:00 schrieb Jocelyn Falempe: [...]+ /** + * @get_scanout_buffer: + *+ * Get the current scanout buffer, to display a panic message with drm_panic. + * The driver should do the minimum changes to provide a linear buffer, that+ * can be used to display the panic screen.+ * It is called from a panic callback, and must follow its restrictions. + * (no locks, no memory allocation, no sleep, no thread/workqueue, ...) + * It's a best effort mode, so it's expected that in some complex cases the+ * panic screen won't be displayed.+ * Some hardware cannot provide a linear buffer, so there is a draw_pixel_xy() + * callback in the struct drm_scanout_buffer that can be used in this case.+ * + * Returns: + * + * Zero on success, negative errno on failure. + */ + int (*get_scanout_buffer)(struct drm_device *dev, + struct drm_scanout_buffer *sb); +After reading through Sima's comments on (try-)locking, I'd like to propose a different interface: instead of having the panic handler search for the scanout buffer, let each driver explicitly set the scanout buffer after each page flip. The algorithm for mode programming then looks like this:1) Maybe clear the panic handler's buffer at the beginning of atomic_commit_tail, if necessary2) Do the mode setting as usual 3) In the driver's atomic_flush or atomic_update, call something like void drm_panic_set_scanout_buffer(dev, scanout_buffer) to set the panic handler's new output.This avoids all the locking and the second guessing about the pipeline status.I don't see an easy way of reliably showing a panic screen during a modeset. But during a modeset, the old scanout buffer should (theoretically) not disappear until the new scanout buffer is in place. So if the panic happens, it would blit to the old address at worst. Well, that assumption needs to be verified per driver.That's an interesting approach, and I will give it a try.I think you still need a callback in the driver, to actually send the data to the GPU.
Sure, you could add this callback directly to the scanout buffer.
Also one thing that I don't handle yet, is when there are multiple outputs, so we may want to set and update multiple scanout buffers ?
That makes me think about adding panic handling directly to the plane, something like this
drm_plane_enable_panic_output(struct drm_plane*)drm_plane_set_panic_output_buffer(struct drm_plane*, struct drm_scanout_buffer*)
First time the driver calls drm_plane_enable_panic_output(), it sets up the panic handling for the given plane and maybe for DRM as a whole.
Each instance of the DRM panic handler operates on a single plane. Then during the pageflips, drm_plane_set_panic_output_buffer() updates the output buffer. The necessary sync/flush callbacks can be part of the drm_plane_funcs.
If/when the plane gets removed from the modesetting pipeline, the panic handling would be removed automatically.
Best regards Thomas
Best regards,
-- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Frankenstrasse 146, 90461 Nuernberg, Germany GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman HRB 36809 (AG Nuernberg)
Attachment:
OpenPGP_signature.asc
Description: OpenPGP digital signature
