Re: [PATCH v11 2/9] drm/panic: Add a drm panic handler

[Thomas Zimmermann <tzimmermann@suse.de>]

Hi

Am 28.03.24 um 13:03 schrieb Jocelyn Falempe:
> +/**
> + * struct drm_scanout_buffer - DRM scanout buffer
> + *
> + * This structure holds the information necessary for drm_panic to draw the
> + * panic screen, and display it.
> + */
> +struct drm_scanout_buffer {
> +	/**
> +	 * @format:
> +	 *
> +	 * drm format of the scanout buffer.
> +	 */
> +	const struct drm_format_info *format;

Newline here and among the other fields please.

> +	/**
> +	 * @map:
> +	 *
> +	 * Virtual address of the scanout buffer, either in memory or iomem.
> +	 * The scanout buffer should be in linear format, and can be directly
> +	 * sent to the display hardware. Tearing is not an issue for the panic
> +	 * screen.
> +	 */
> +	struct iosys_map map;

I would make this an array of DRM_FORMAT_MAX_PLANES. Its functionality 
is then equivalent to the fields in struct drm_framebuffer. Supporting 
multiple color planes is the general expectation in the DRM code, even 
if not all parts actually implement it. In the panic code, you simply 
test that the scan-out format has only a single plane.

   struct iosys_map map[DRM_FORMAT_MAX_PLANES]


> +	/**
> +	 * @width: Width of the scanout buffer, in pixels.
> +	 */
> +	unsigned int width;
> +	/**
> +	 * @height: Height of the scanout buffer, in pixels.
> +	 */
> +	unsigned int height;
> +	/**
> +	 * @pitch: Length in bytes between the start of two consecutive lines.
> +	 */
> +	unsigned int pitch;

Also use an array of DRM_FORMAT_MAX_PLANES.

> +};

This data structure looks like something I could use for the 
shadow-plane helpers. Expect it to be moved elsewhere at some point.

Best regards
Thomas


-- 
--
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)