Re: [CI v4 07/14] drm/i915/color: Add YCbCr limited-to-full range color block support

[sashiko-bot@kernel.org]

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

New issues:
- [Medium] Hardware state for YUV range correction is incorrectly restricted to SDR planes when a color pipeline is active, preventing HDR planes from bypassing range correction.
- [Low] The new YUV range correction color block (INTEL_PLANE_CB_YUV_RANGE_CORRECT) is never added to any plane's color pipeline, making the feature unreachable.

Pre-existing issues:
- [High] Error paths in colorop initialization lead to a kernel panic because they unconditionally clean up partially initialized colorop objects.
- [High] Missing `drm_mode_object_unregister()` in the colorop teardown sequence leaks modeset IDR identifiers, resulting in a Use-After-Free.
--

--- Patch [7]: [CI v4 07/14] drm/i915/color: Add YCbCr limited-to-full range color block support ---
commit 0f5fc4ef9d6c7498eb8e7e7dcd0a849594fe9377
Author: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>
Subject: drm/i915/color: Add YCbCr limited-to-full range color block support

This patch adds support for a color block that performs YCbCr limited-to-full
range expansion in the plane color pipeline. It represents this new color
block using the DRM_COLOROP_FIXED_MATRIX colorop.

> diff --git a/drivers/gpu/drm/i915/display/intel_color_pipeline.c b/drivers/gpu/drm/i915/display/intel_color_pipeline.c
> --- a/drivers/gpu/drm/i915/display/intel_color_pipeline.c
> +++ b/drivers/gpu/drm/i915/display/intel_color_pipeline.c
> @@ -108,6 +108,12 @@ struct intel_colorop *intel_color_pipeline_plane_add_colorop(struct drm_plane *p
>  							  intel_plane_supported_csc_ff,
>  							  DRM_COLOROP_FLAG_ALLOW_BYPASS);
>  		break;
> +	case INTEL_PLANE_CB_YUV_RANGE_CORRECT:
> +		ret = drm_plane_colorop_fixed_matrix_init(dev, &colorop->base, plane,
> +							  &intel_colorop_funcs,
> +							  BIT(DRM_COLOROP_FM_YCBCR_LIMITED_FULL),
> +							  DRM_COLOROP_FLAG_ALLOW_BYPASS);
> +		break;

[Severity: Low]
Is this new color block missing from the pipeline definition arrays?

It appears INTEL_PLANE_CB_YUV_RANGE_CORRECT is defined and its configuration
is implemented, but it is never added to the static pipeline definition arrays
like sdr_plane_pipeline. 

Without being appended to the arrays that dictate pipeline construction, does
this feature remain uninstantiated and unreachable by userspace?

[Severity: High]
This isn't a bug introduced by this patch, but can the error path here lead to
a kernel panic?

If drm_plane_colorop_fixed_matrix_init() fails, colorop->dev remains
uninitialized. The error path jumps to the cleanup label and calls
intel_colorop_destroy(), which then calls drm_colorop_cleanup():

drm_colorop_cleanup()
    struct drm_device *dev = colorop->dev;
    struct drm_mode_config *config = &dev->mode_config;
    list_del(&colorop->head);

Since dev is NULL, will this blindly dereference a zero-initialized structure
and oops on the driver probe error path?

[Severity: High]
This is a pre-existing issue, but does the colorop teardown sequence leak
modeset IDR identifiers and cause a Use-After-Free?

When drm_plane_colorop_init() is called, it generates an ID for the
colorop->base pointer via drm_mode_object_add().

During teardown, drm_colorop_cleanup() is called but it omits the
drm_mode_object_unregister() call. Then, intel_colorop_destroy() frees the
memory:

intel_colorop_destroy()
    drm_colorop_cleanup(colorop);
    kfree(to_intel_colorop(colorop));

Does this leave a dangling pointer in the modeset IDR, where a subsequent
userspace lookup of the object ID would fetch and use freed memory?

[ ... ]
> diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> --- a/drivers/gpu/drm/i915/display/skl_universal_plane.c
> +++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> @@ -1296,6 +1296,8 @@ static u32 glk_plane_color_ctl_input_csc(const struct intel_plane_state *plane_s
>  	} else if (!icl_is_hdr_plane(display, plane->id)) {
>  		if (plane_state->hw.csc_ff_enable)
>  			ctl |= intel_csc_ff_type_to_csc_mode(plane_state->hw.csc_ff_type);
> +		if (plane_state->hw.yuv_range_correction_disable)
> +			ctl |= PLANE_COLOR_YUV_RANGE_CORRECTION_DISABLE;
>  	}
>  
>  	return ctl;

[Severity: Medium]
Does this restrict the YUV range correction bypass to SDR planes only?

The new check for hw.yuv_range_correction_disable is placed strictly inside
the else if (!icl_is_hdr_plane()) block. This seems to force range correction
on for HDR planes when a color pipeline is active, bypassing the disable flag
completely.

Should this check be moved outside the SDR-only restriction to match the legacy
path which correctly applies the bypass to both?

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