Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues: - [High] Activating a color pipeline on an HDR plane silently disables the hardware Input CSC block, completely breaking YUV framebuffers. Pre-existing issues: - [High] NULL pointer dereference in the error cleanup path of `intel_color_pipeline_plane_add_colorop()` when colorop initialization fails. -- --- Patch [8]: [CI v4 08/14] drm/i915/color: Add YUV range correction to SDR plane pipeline --- commit 596a494a01ab45eb529185d68401cfd391408e46 Author: Chaitanya Kumar Borah <[email protected]> drm/i915/color: Add YUV range correction to SDR plane pipeline This commit inserts 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. > 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, [Severity: High] Does adding this color pipeline block expose a regression with HDR planes? When activating a color pipeline on an HDR plane, the hardware Input CSC block appears to be disabled, breaking YUV framebuffers. Looking at glk_plane_color_ctl_input_csc() in skl_universal_plane.c: } 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; If a color pipeline is active, it skips the legacy block. For HDR planes, icl_is_hdr_plane() evaluates to true, so this else-if block is skipped and it returns 0 without setting the PLANE_COLOR_INPUT_CSC_ENABLE bit. Will this cause visual corruption for YUV framebuffers on HDR planes when a color pipeline is active, as raw non-linear YUV data would be passed directly to the PRE_CSC_LUT? [Severity: High] This is a pre-existing issue, but can this lead to a NULL pointer dereference in the error cleanup path? If intel_color_pipeline_plane_add_colorop() in intel_color_pipeline.c fails: cleanup: intel_colorop_destroy(&colorop->base); return ERR_PTR(ret); The colorop is allocated zero-initialized in intel_colorop_alloc(): colorop = kzalloc_obj(*colorop); If the initialization fails before colorop->dev is set or the list head is initialized, intel_colorop_destroy() unconditionally calls drm_colorop_cleanup(): struct drm_device *dev = colorop->dev; struct drm_mode_config *config = &dev->mode_config; list_del(&colorop->head); Could this dereference the NULL dev pointer and call list_del() on an invalid list head? > INTEL_PLANE_CB_CSC_FF, > }; -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=8
