Re: [v5 08/14] drm/i915/color: Add YUV range correction to SDR plane pipeline

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

New issues:
- [Medium] The SDR plane color pipeline is never exposed to userspace because `intel_color_pipeline_plane_init()` unconditionally early-returns for non-HDR planes.
- [Low] The fixed matrix support mask validation fails to check bits 32-63 on 32-bit architectures due to unsigned integer zero-extension.

Pre-existing issues:
- [High] If colorop initialization fails early, the cleanup path dereferences a NULL `colorop->dev` pointer and uninitialized list head, causing a kernel panic.
--

--- Patch [8]: [v5 08/14] drm/i915/color: Add YUV range correction to SDR plane pipeline ---
commit cf2ffd1a2b5d8ea9f91da1ef444dd3fa882dd254
Author: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>

drm/i915/color: Add YUV range correction to SDR plane pipeline

Insert the INTEL_PLANE_CB_YUV_RANGE_CORRECT color block into the SDR
plane color pipeline. This enables YCbCr limited-to-full range expansion
ahead of the CSC block that expects full range pixel data. With this,
the pipeline can accept limited range framebuffers.

> diff --git a/drivers/gpu/drm/i915/display/intel_color_pipeline.c b/drivers/gpu/drm/i915/display/intel_color_pipeline.c
> index 7d25f4fbfa9b7..7457fa51cb97a 100644
> --- a/drivers/gpu/drm/i915/display/intel_color_pipeline.c
> +++ b/drivers/gpu/drm/i915/display/intel_color_pipeline.c
> @@ -44,6 +44,7 @@ static const enum intel_color_block hdr_plane_pipeline[] = {
>  };
>  
>  static const enum intel_color_block sdr_plane_pipeline[] = {
> +	INTEL_PLANE_CB_YUV_RANGE_CORRECT,
>  	INTEL_PLANE_CB_CSC_FF,
>  };

[Severity: Medium]
Will this newly added SDR plane pipeline actually be exposed to userspace?

Looking at intel_color_pipeline_plane_init(), there appears to be an early
return that completely prevents color pipelines from being initialized for SDR
planes:

drivers/gpu/drm/i915/display/intel_color_pipeline.c:intel_color_pipeline_plane_init() {
	...
	/* Currently expose pipeline only for HDR planes */
	if (!icl_is_hdr_plane(display, to_intel_plane(plane)->id))
		return 0;
	...
}

Because of this early return, doesn't the assignment in
_intel_color_pipeline_plane_init() become dead code, leaving the new SDR
pipeline features inaccessible?

[Severity: High]
This is a pre-existing issue, but could the error path in
intel_color_pipeline_plane_add_colorop() cause a kernel panic?

If drm_plane_colorop_init() fails early (e.g., if drm_mode_object_add()
returns -ENOMEM), it returns before colorop->dev is set and before
colorop->head is added to the list.

When intel_color_pipeline_plane_add_colorop() jumps to the cleanup label:

drivers/gpu/drm/i915/display/intel_color_pipeline.c:intel_color_pipeline_plane_add_colorop() {
    ...
cleanup:
	intel_colorop_destroy(&colorop->base);
	return ERR_PTR(ret);
}

This calls drm_colorop_cleanup(), which unconditionally dereferences
colorop->dev and calls list_del() on the zeroed list head:

drivers/gpu/drm/drm_colorop.c:drm_colorop_cleanup() {
	struct drm_device *dev = colorop->dev;
	struct drm_mode_config *config = &dev->mode_config;

	list_del(&colorop->head);
    ...
}

Would this lead to a NULL pointer dereference for colorop->dev and a list
corruption panic?

[Severity: Low]
Does the bitwise validation in drm_plane_colorop_fixed_matrix_init()
correctly handle checking on 32-bit architectures?

Looking at drivers/gpu/drm/drm_colorop.c:drm_plane_colorop_fixed_matrix_init():

	if ((supported_fm & -BIT(DRM_COLOROP_FM_COUNT)) != 0) {

The check evaluates -BIT(5). On 32-bit platforms, this becomes a 32-bit
unsigned value (0xFFFFFFE0). When bitwise ANDed with the u64 supported_fm,
C integer promotion zero-extends it to 0x00000000FFFFFFE0.

Would this allow any invalid bits set in supported_fm between 32 and 63 to
silently bypass the check?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260720045131.971650-1-chaitanya.kumar.borah@intel.com?part=8