Re: [PATCH v3 2/2] drm: ensure blend mode supported if alpha exposed

[Leandro Ribeiro <leandro.ribeiro@collabora.com>]

On 5/22/26 3:18 PM, Ville Syrjälä wrote:
> On Mon, May 18, 2026 at 02:54:29PM -0300, Leandro Ribeiro wrote:
>> Before "drm/drm_blend: allow blend mode property without PREMULTI",
>> userspace would have to assume that only PREMULTI was supported by
>> drivers that didn't expose the blend mode property. But now userspace
>> shouldn't relly on that, as they can't count with drivers always
>> supporting PREMULTI.
>>
>> Error out if a driver expose alpha property or pixel formats with alpha
>> and does not expose the blend mode property. This way userspace don't
>> have to guess. Drivers that hit such error must be fixed.
> 
> I don't think the error handling is a good idea. It's just another
> completely untested error path that is more or less guaranteed to
> break eventually. So IMO just WARN and plow ahead like we do for
> everything else there.
> 

Sounds good to me, I'll change that in the next revision.

>>
>> Signed-off-by: Leandro Ribeiro <leandro.ribeiro@collabora.com>
>> ---
>>  drivers/gpu/drm/drm_crtc_internal.h |  2 +-
>>  drivers/gpu/drm/drm_drv.c           |  7 ++++--
>>  drivers/gpu/drm/drm_mode_config.c   | 37 +++++++++++++++++++++++++++--
>>  3 files changed, 41 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_crtc_internal.h b/drivers/gpu/drm/drm_crtc_internal.h
>> index c09409229644..2a4862202496 100644
>> --- a/drivers/gpu/drm/drm_crtc_internal.h
>> +++ b/drivers/gpu/drm/drm_crtc_internal.h
>> @@ -95,7 +95,7 @@ int drm_mode_setcrtc(struct drm_device *dev,
>>  /* drm_mode_config.c */
>>  int drm_modeset_register_all(struct drm_device *dev);
>>  void drm_modeset_unregister_all(struct drm_device *dev);
>> -void drm_mode_config_validate(struct drm_device *dev);
>> +int drm_mode_config_validate(struct drm_device *dev);
>>  
>>  /* drm_modes.c */
>>  const char *drm_get_mode_status_name(enum drm_mode_status status);
>> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
>> index 985c283cf59f..def78046a963 100644
>> --- a/drivers/gpu/drm/drm_drv.c
>> +++ b/drivers/gpu/drm/drm_drv.c
>> @@ -1059,8 +1059,11 @@ int drm_dev_register(struct drm_device *dev, unsigned long flags)
>>  	const struct drm_driver *driver = dev->driver;
>>  	int ret;
>>  
>> -	if (!driver->load)
>> -		drm_mode_config_validate(dev);
>> +	if (!driver->load) {
>> +		ret = drm_mode_config_validate(dev);
>> +		if (ret)
>> +			return ret;
>> +	}
>>  
>>  	WARN_ON(!dev->managed.final_kfree);
>>  
>> diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c
>> index 66f7dc37b597..18c6b5707532 100644
>> --- a/drivers/gpu/drm/drm_mode_config.c
>> +++ b/drivers/gpu/drm/drm_mode_config.c
>> @@ -674,16 +674,45 @@ static void validate_encoder_possible_crtcs(struct drm_encoder *encoder)
>>  	     encoder->possible_crtcs, crtc_mask);
>>  }
>>  
>> -void drm_mode_config_validate(struct drm_device *dev)
>> +static int plane_alpha_require_blend_mode(struct drm_plane *plane)
>> +{
>> +	struct drm_device *dev = plane->dev;
>> +	const struct drm_format_info *fmt;
>> +	u32 i;
>> +
>> +	/* blend mode property supported, no need to check anything */
>> +	if (plane->blend_mode_property)
>> +		return 0;
>> +
>> +	if (plane->alpha_property) {
>> +		drm_err(dev, "[PLANE:%d:%s] alpha property exposed but blend mode not setup",
>> +			plane->base.id, plane->name);
>> +		return -EINVAL;
>> +	}
>> +
>> +	for (i = 0; i < plane->format_count; i++) {
>> +		fmt = drm_format_info(plane->format_types[i]);
>> +		if (fmt->has_alpha) {
>> +			drm_err(dev, "[PLANE:%d:%s] pixel format with alpha exposed but blend mode not setup",
>> +				plane->base.id, plane->name);
>> +			return -EINVAL;
>> +		}
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +int drm_mode_config_validate(struct drm_device *dev)
>>  {
>>  	struct drm_encoder *encoder;
>>  	struct drm_crtc *crtc;
>>  	struct drm_plane *plane;
>>  	u32 primary_with_crtc = 0, cursor_with_crtc = 0;
>>  	unsigned int num_primary = 0;
>> +	int ret = 0;
>>  
>>  	if (!drm_core_check_feature(dev, DRIVER_MODESET))
>> -		return;
>> +		return ret;
>>  
>>  	drm_for_each_encoder(encoder, dev)
>>  		fixup_encoder_possible_clones(encoder);
>> @@ -732,9 +761,13 @@ void drm_mode_config_validate(struct drm_device *dev)
>>  	drm_for_each_plane(plane, dev) {
>>  		if (plane->type == DRM_PLANE_TYPE_PRIMARY)
>>  			num_primary++;
>> +
>> +		ret |= plane_alpha_require_blend_mode(plane);
>>  	}
>>  
>>  	WARN(num_primary != dev->mode_config.num_crtc,
>>  	     "Must have as many primary planes as there are CRTCs, but have %u primary planes and %u CRTCs",
>>  	     num_primary, dev->mode_config.num_crtc);
>> +
>> +	return ret;
>>  }
>> -- 
>> 2.54.0
> 

-- 
Leandro Ribeiro