Re: [PATCH v3] drm/pl111: drop alpha formats the hardware cannot scan out

[Thomas Zimmermann <tzimmermann@suse.de>]

Am 10.09.26 um 21:25 schrieb Roshan Kumar:
> The PL110/PL111 controller scans out a single framebuffer and never
> blends its alpha channel with anything. In every pixel-format table
> each alpha-carrying entry (ARGB/ABGR in 8888, 1555 and 4444) maps to
> exactly the same register configuration as its XRGB/XBGR counterpart,
> so the alpha byte was never used by the hardware.
>
> Commit 860e748bddcc ("drm: ensure blend mode supported if pixel format
> with alpha exposed") added validation that warns when a plane exposes
> alpha formats without a pixel blend mode property, and PL111 does exactly
> that. With panic_on_warn enabled the warning prevents the syzkaller
> vexpress-a15 manager from booting.
>
> Drivers should not report formats they do not support, so remove the
> alpha formats from all variant tables: the generic PL110, PL111 and
> Nomadik tables in pl111_drv.c and the board-specific Integrator, IM-PD1,
> Versatile and RealView/Versatile-Express tables in pl111_versatile.c,
> plus the now dead case labels in the display setup and the Versatile
> syscon connector switch, instead of advertising a blend mode property.
> Behavior is unchanged for userspace that picks an XRGB/XBGR/RGB format;
> alpha-picking clients fall back to the identical X variant.
>
> Link: https://github.com/google/syzkaller/issues/7805
> Fixes: 860e748bddcc ("drm: ensure blend mode supported if pixel format with alpha exposed")
> Signed-off-by: Roshan Kumar <roshaen09@gmail.com>

The tags always go last, but we can fix this when we merge the patch. 
Thanks for fixing the issue.

Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>

>
> Changes in v2: drop the alpha formats entirely instead of advertising a
> DRM_MODE_BLEND_PIXEL_NONE property, per review feedback from Thomas
> Zimmermann. Leandro's Reviewed-by from v1 is not carried as the
> approach changed.
>
> Changes in v3: v2 missed the board-specific variant tables in
> pl111_versatile.c (Integrator, IM-PD1, Versatile, RealView and
> Versatile Express), which still exposed alpha formats and would fall
> through to the WARN_ONCE default in pl111_display_enable() on those
> boards. Pointed out by the sashiko AI reviewer.
> ---
>   drivers/gpu/drm/pl111/pl111_display.c   |  6 ------
>   drivers/gpu/drm/pl111/pl111_drv.c       | 16 ----------------
>   drivers/gpu/drm/pl111/pl111_versatile.c | 18 ------------------
>   3 files changed, 40 deletions(-)
>
> diff --git a/drivers/gpu/drm/pl111/pl111_display.c b/drivers/gpu/drm/pl111/pl111_display.c
> index 5d10bc5fdf1f..e1b513d36c27 100644
> --- a/drivers/gpu/drm/pl111/pl111_display.c
> +++ b/drivers/gpu/drm/pl111/pl111_display.c
> @@ -267,14 +267,12 @@ static void pl111_display_enable(struct drm_simple_display_pipe *pipe,
>   		if (priv->variant->st_bitmux_control)
>   			cntl |= CNTL_ST_LCDBPP24_PACKED;
>   		break;
> -	case DRM_FORMAT_ABGR8888:
>   	case DRM_FORMAT_XBGR8888:
>   		if (priv->variant->st_bitmux_control)
>   			cntl |= CNTL_LCDBPP24 | CNTL_BGR;
>   		else
>   			cntl |= CNTL_LCDBPP24;
>   		break;
> -	case DRM_FORMAT_ARGB8888:
>   	case DRM_FORMAT_XRGB8888:
>   		if (priv->variant->st_bitmux_control)
>   			cntl |= CNTL_LCDBPP24;
> @@ -297,13 +295,11 @@ static void pl111_display_enable(struct drm_simple_display_pipe *pipe,
>   		else
>   			cntl |= CNTL_LCDBPP16_565 | CNTL_BGR;
>   		break;
> -	case DRM_FORMAT_ABGR1555:
>   	case DRM_FORMAT_XBGR1555:
>   		cntl |= CNTL_LCDBPP16;
>   		if (priv->variant->st_bitmux_control)
>   			cntl |= CNTL_ST_1XBPP_5551 | CNTL_BGR;
>   		break;
> -	case DRM_FORMAT_ARGB1555:
>   	case DRM_FORMAT_XRGB1555:
>   		cntl |= CNTL_LCDBPP16;
>   		if (priv->variant->st_bitmux_control)
> @@ -311,13 +307,11 @@ static void pl111_display_enable(struct drm_simple_display_pipe *pipe,
>   		else
>   			cntl |= CNTL_BGR;
>   		break;
> -	case DRM_FORMAT_ABGR4444:
>   	case DRM_FORMAT_XBGR4444:
>   		cntl |= CNTL_LCDBPP16_444;
>   		if (priv->variant->st_bitmux_control)
>   			cntl |= CNTL_ST_1XBPP_444 | CNTL_BGR;
>   		break;
> -	case DRM_FORMAT_ARGB4444:
>   	case DRM_FORMAT_XRGB4444:
>   		cntl |= CNTL_LCDBPP16_444;
>   		if (priv->variant->st_bitmux_control)
> diff --git a/drivers/gpu/drm/pl111/pl111_drv.c b/drivers/gpu/drm/pl111/pl111_drv.c
> index ac7b1d12a0f5..2175ac953795 100644
> --- a/drivers/gpu/drm/pl111/pl111_drv.c
> +++ b/drivers/gpu/drm/pl111/pl111_drv.c
> @@ -341,13 +341,9 @@ static void pl111_amba_shutdown(struct amba_device *amba_dev)
>    * This early variant lacks the 565 and 444 pixel formats.
>    */
>   static const u32 pl110_pixel_formats[] = {
> -	DRM_FORMAT_ABGR8888,
>   	DRM_FORMAT_XBGR8888,
> -	DRM_FORMAT_ARGB8888,
>   	DRM_FORMAT_XRGB8888,
> -	DRM_FORMAT_ABGR1555,
>   	DRM_FORMAT_XBGR1555,
> -	DRM_FORMAT_ARGB1555,
>   	DRM_FORMAT_XRGB1555,
>   };
>   
> @@ -361,19 +357,13 @@ static const struct pl111_variant_data pl110_variant = {
>   
>   /* RealView, Versatile Express etc use this modern variant */
>   static const u32 pl111_pixel_formats[] = {
> -	DRM_FORMAT_ABGR8888,
>   	DRM_FORMAT_XBGR8888,
> -	DRM_FORMAT_ARGB8888,
>   	DRM_FORMAT_XRGB8888,
>   	DRM_FORMAT_BGR565,
>   	DRM_FORMAT_RGB565,
> -	DRM_FORMAT_ABGR1555,
>   	DRM_FORMAT_XBGR1555,
> -	DRM_FORMAT_ARGB1555,
>   	DRM_FORMAT_XRGB1555,
> -	DRM_FORMAT_ABGR4444,
>   	DRM_FORMAT_XBGR4444,
> -	DRM_FORMAT_ARGB4444,
>   	DRM_FORMAT_XRGB4444,
>   };
>   
> @@ -387,19 +377,13 @@ static const struct pl111_variant_data pl111_variant = {
>   static const u32 pl110_nomadik_pixel_formats[] = {
>   	DRM_FORMAT_RGB888,
>   	DRM_FORMAT_BGR888,
> -	DRM_FORMAT_ABGR8888,
>   	DRM_FORMAT_XBGR8888,
> -	DRM_FORMAT_ARGB8888,
>   	DRM_FORMAT_XRGB8888,
>   	DRM_FORMAT_BGR565,
>   	DRM_FORMAT_RGB565,
> -	DRM_FORMAT_ABGR1555,
>   	DRM_FORMAT_XBGR1555,
> -	DRM_FORMAT_ARGB1555,
>   	DRM_FORMAT_XRGB1555,
> -	DRM_FORMAT_ABGR4444,
>   	DRM_FORMAT_XBGR4444,
> -	DRM_FORMAT_ARGB4444,
>   	DRM_FORMAT_XRGB4444,
>   };
>   
> diff --git a/drivers/gpu/drm/pl111/pl111_versatile.c b/drivers/gpu/drm/pl111/pl111_versatile.c
> index 0d8331a3909f..9a60f10231b1 100644
> --- a/drivers/gpu/drm/pl111/pl111_versatile.c
> +++ b/drivers/gpu/drm/pl111/pl111_versatile.c
> @@ -209,9 +209,7 @@ static void pl111_versatile_enable(struct drm_device *drm, u32 format)
>   	drm_info(drm, "enable Versatile CLCD connectors\n");
>   
>   	switch (format) {
> -	case DRM_FORMAT_ABGR8888:
>   	case DRM_FORMAT_XBGR8888:
> -	case DRM_FORMAT_ARGB8888:
>   	case DRM_FORMAT_XRGB8888:
>   		val |= SYS_CLCD_MODE_888;
>   		break;
> @@ -221,9 +219,7 @@ static void pl111_versatile_enable(struct drm_device *drm, u32 format)
>   	case DRM_FORMAT_RGB565:
>   		val |= SYS_CLCD_MODE_565_B_LSB;
>   		break;
> -	case DRM_FORMAT_ABGR1555:
>   	case DRM_FORMAT_XBGR1555:
> -	case DRM_FORMAT_ARGB1555:
>   	case DRM_FORMAT_XRGB1555:
>   		val |= SYS_CLCD_MODE_5551;
>   		break;
> @@ -266,44 +262,30 @@ static void pl111_realview_clcd_enable(struct drm_device *drm, u32 format)
>   
>   /* PL110 pixel formats for Integrator, vanilla PL110 */
>   static const u32 pl110_integrator_pixel_formats[] = {
> -	DRM_FORMAT_ABGR8888,
>   	DRM_FORMAT_XBGR8888,
> -	DRM_FORMAT_ARGB8888,
>   	DRM_FORMAT_XRGB8888,
> -	DRM_FORMAT_ABGR1555,
>   	DRM_FORMAT_XBGR1555,
> -	DRM_FORMAT_ARGB1555,
>   	DRM_FORMAT_XRGB1555,
>   };
>   
>   /* Extended PL110 pixel formats for Integrator and Versatile */
>   static const u32 pl110_versatile_pixel_formats[] = {
> -	DRM_FORMAT_ABGR8888,
>   	DRM_FORMAT_XBGR8888,
> -	DRM_FORMAT_ARGB8888,
>   	DRM_FORMAT_XRGB8888,
>   	DRM_FORMAT_BGR565, /* Uses external PLD */
>   	DRM_FORMAT_RGB565, /* Uses external PLD */
> -	DRM_FORMAT_ABGR1555,
>   	DRM_FORMAT_XBGR1555,
> -	DRM_FORMAT_ARGB1555,
>   	DRM_FORMAT_XRGB1555,
>   };
>   
>   static const u32 pl111_realview_pixel_formats[] = {
> -	DRM_FORMAT_ABGR8888,
>   	DRM_FORMAT_XBGR8888,
> -	DRM_FORMAT_ARGB8888,
>   	DRM_FORMAT_XRGB8888,
>   	DRM_FORMAT_BGR565,
>   	DRM_FORMAT_RGB565,
> -	DRM_FORMAT_ABGR1555,
>   	DRM_FORMAT_XBGR1555,
> -	DRM_FORMAT_ARGB1555,
>   	DRM_FORMAT_XRGB1555,
> -	DRM_FORMAT_ABGR4444,
>   	DRM_FORMAT_XBGR4444,
> -	DRM_FORMAT_ARGB4444,
>   	DRM_FORMAT_XRGB4444,
>   };
>   

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, (HRB 36809, AG Nürnberg)