Re: [PATCH v1 3/4] drm/i915: Add DRIVER_PLANE_COLOR_PIPELINE driver feature

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The i915 driver API contract violation: fails to ignore legacy pre-blend color operations (COLOR_ENCODING and COLOR_RANGE) when the DRM_CLIENT_CAP_PLANE_COLOR_PIPELINE capability is set.
- [Medium] The DRIVER_PLANE_COLOR_PIPELINE feature flag is missing for the Xe driver, making the plane color pipeline capability unavailable despite shared and implemented support.
--

commit be3a060691e463c76013f01322ae2330b3817960
Author: Robert Mader <robert.mader@collabora.com>

drm/i915: Add DRIVER_PLANE_COLOR_PIPELINE driver feature
    
Which is now required for DRM_CLIENT_CAP_PLANE_COLOR_PIPELINE.

> diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c
> index f0105c5b49a71..139848bf903cb 100644
> --- a/drivers/gpu/drm/i915/i915_driver.c
> +++ b/drivers/gpu/drm/i915/i915_driver.c
> @@ -1860,7 +1860,7 @@ static const struct drm_driver i915_drm_driver = {
>  	.driver_features =
>  	    DRIVER_GEM |
>  	    DRIVER_RENDER | DRIVER_MODESET | DRIVER_ATOMIC | DRIVER_SYNCOBJ |
> -	    DRIVER_SYNCOBJ_TIMELINE,
> +	    DRIVER_SYNCOBJ_TIMELINE | DRIVER_PLANE_COLOR_PIPELINE,

[Severity: High]
Does enabling this feature reveal a logic bug where legacy color properties
aren't ignored when the client uses plane color pipelines?

When a client uses DRM_CLIENT_CAP_PLANE_COLOR_PIPELINE and configures a color
pipeline on a plane with a YUV framebuffer, the driver unconditionally copies
legacy UAPI properties to the hardware state in
drivers/gpu/drm/i915/display/intel_plane.c:

intel_plane_copy_uapi_to_hw_state() {
	...
	plane_state->hw.color_encoding = from_plane_state->uapi.color_encoding;
	plane_state->hw.color_range = from_plane_state->uapi.color_range;
	...
}

This leads to the hardware input CSC being programmed using the legacy
color_encoding in drivers/gpu/drm/i915/display/skl_universal_plane.c:

icl_program_input_csc() {
	...
	const u16 *csc = input_csc_matrix[plane_state->hw.color_encoding];
	...
}

Will this cause a hidden YUV-to-RGB conversion to be applied, violating the
explicit color pipeline contract?

[Severity: Medium]
Since the Xe driver shares the i915 display stack and fully implements plane
color pipelines, does it also need this driver feature flag?

Userspace attempting to set DRM_CLIENT_CAP_PLANE_COLOR_PIPELINE on Xe hardware
will currently get -EOPNOTSUPP because the feature is missing in
drivers/gpu/drm/xe/display/xe_display.c:

xe_display_driver_set_hooks() {
	...
	driver->driver_features |= DRIVER_MODESET | DRIVER_ATOMIC;
	...
}

Should DRIVER_PLANE_COLOR_PIPELINE be added to xe_display_driver_set_hooks()
as well?

>  	.release = i915_driver_release,
>  	.open = i915_driver_open,
>  	.postclose = i915_driver_postclose,

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260630084229.529682-1-robert.mader@collabora.com?part=3