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

[Thomas Zimmermann <tzimmermann@suse.de>]

Hi

Am 10.09.26 um 07:45 schrieb Roshan Kumar:
> The PL110/PL111 controller scans out a single framebuffer and never
> blends its alpha channel with anything. In the pixel-format tables every
> 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 the PL110, PL111 and Nomadik PL110 variant tables and
> the now dead case labels in the display setup, 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.

Please see the comment from the Sashiko bot about additional alpha 
formats. Apart from that that patch looks good.

BTW, do you have this hardware available for testing?

Best regards
Thomas

>
> 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>
> 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.
>
> ---
>   drivers/gpu/drm/pl111/pl111_display.c |  6 ------
>   drivers/gpu/drm/pl111/pl111_drv.c     | 16 ----------------
>   2 files changed, 22 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,
>   };
>   

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