Re: [PATCH] drm/pl111: Advertise no pixel blending

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The initial atomic plane state defaults to an invalid pixel blend mode, which contradicts the newly added blend mode property that only supports `DRM_MODE_BLEND_PIXEL_NONE`.
--

--- Patch [1]: [PATCH] drm/pl111: Advertise no pixel blending ---
commit e96a716603035744ccddd019e74ade69db24a36e
Author: Roshan Kumar <[email protected]>

drm/pl111: Advertise no pixel blending

This commit adds a pixel blend mode property to the PL111 display plane to
satisfy validation introduced in commit 860e748bddcc, advertising only
DRM_MODE_BLEND_PIXEL_NONE. This resolves a syzkaller panic during boot on
vexpress-a15 managers.

Link: https://github.com/google/syzkaller/issues/7805

> diff --git a/drivers/gpu/drm/pl111/pl111_display.c b/drivers/gpu/drm/pl111/pl111_display.c
> index 5d10bc5fdf1f6..758b297cbdea5 100644
> --- a/drivers/gpu/drm/pl111/pl111_display.c
> +++ b/drivers/gpu/drm/pl111/pl111_display.c
[ ... ]
> @@ -596,5 +597,10 @@ int pl111_display_init(struct drm_device *drm)
>  	if (ret)
>  		return ret;
>  
> +	ret = drm_plane_create_blend_mode_property(&priv->pipe.plane,
> +						   BIT(DRM_MODE_BLEND_PIXEL_NONE));

[Severity: High]
Will this cause atomic commits to fail with -EINVAL for standard userspace
compositors?

Because pl111 uses drm_simple_display_pipe_init() and lacks a custom
reset_plane hook, the DRM core falls back to drm_atomic_helper_plane_reset()
which unconditionally sets plane_state->pixel_blend_mode = 
DRM_MODE_BLEND_PREMULTI during __drm_atomic_helper_plane_state_init().

Since this newly created property only supports DRM_MODE_BLEND_PIXEL_NONE,
userspace reading the initial state and submitting it back will provide an
enum value that is not in the allowed mask. This causes 
drm_property_change_valid_get() to reject the commit. 

Does the driver need a custom reset hook to initialize the plane state to
DRM_MODE_BLEND_PIXEL_NONE instead of relying on the default initialization?

> +	if (ret)
> +		return ret;
> +
>  	return 0;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1