[Intel-gfx] [PATCH] drm/i915/icl: Fix checks for userspace ctm + ycbcr output

[Shankar, Uma]

>-----Original Message-----
>From: Roper, Matthew D
>Sent: Saturday, February 16, 2019 5:06 AM
>To: intel-gfx at lists.freedesktop.org
>Cc: Roper, Matthew D <matthew.d.roper at intel.com>; Shankar, Uma
><uma.shankar at intel.com>; Maarten Lankhorst
><maarten.lankhorst at linux.intel.com>; Ville Syrjälä <ville.syrjala at linux.intel.com>
>Subject: [PATCH] drm/i915/icl: Fix checks for userspace ctm + ycbcr output
>
>We recently added support for gen11's new "output csc" which can be used
>independently of the regular pipe CSC.  The idea is that this new output csc allows us
>to use a userspace-provided color transformation matrix at the same time we drive a
>YCBCR display mode requiring RGB->YUV conversion.  However when landing that
>support we only updated the color management code and overlooked that there was
>an additional atomic check in the modeset path (in intel_crtc_compute_config()) that
>still assumes we only have a single CSC unit to work with.  Let's update that check to
>only apply to pre-gen11 platforms and move it into the color management code to
>ensure it gets called on all commits, not just modesets.
>
>Also, if we're *only* using the output CSC and not the pipe CSC, there's no need to set
>crtc_state->csc_enable, so let's also not consider the output format when setting
>csc_enable on gen11+ platforms.

This looks good to me. Thanks for fixing this.
Reviewed-by: Uma Shankar <uma.shankar at intel.com>

>Fixes: a91de580541c ("drm/i915/icl: Enable pipe output csc")
>Cc: Uma Shankar <uma.shankar at intel.com>
>Cc: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
>Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
>Signed-off-by: Matt Roper <matthew.d.roper at intel.com>
>---
>It might also be worth moving the full->limited range RGB conversion from the pipe
>CSC to the output CSC on gen11 at some point.
>
> drivers/gpu/drm/i915/intel_color.c   | 16 +++++++++++++---
> drivers/gpu/drm/i915/intel_display.c | 12 ------------
> 2 files changed, 13 insertions(+), 15 deletions(-)
>
>diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
>index da7a07d5ccea..b47e6d1aaaec 100644
>--- a/drivers/gpu/drm/i915/intel_color.c
>+++ b/drivers/gpu/drm/i915/intel_color.c
>@@ -780,9 +780,10 @@ int intel_color_check(struct intel_crtc_state *crtc_state)
> 	    IS_BROADWELL(dev_priv) || IS_HASWELL(dev_priv))
> 		limited_color_range = crtc_state->limited_color_range;
>
>-	crtc_state->csc_enable =
>-		crtc_state->output_format != INTEL_OUTPUT_FORMAT_RGB ||
>-		crtc_state->base.ctm || limited_color_range;
>+	crtc_state->csc_enable = crtc_state->base.ctm || limited_color_range;
>+	if (INTEL_GEN(dev_priv) < 11)
>+		crtc_state->csc_enable |=
>+			crtc_state->output_format !=
>INTEL_OUTPUT_FORMAT_RGB;
>
> 	ret = intel_color_add_affected_planes(crtc_state);
> 	if (ret)
>@@ -822,6 +823,15 @@ int intel_color_check(struct intel_crtc_state *crtc_state)
> 			crtc_state->csc_mode |= ICL_OUTPUT_CSC_ENABLE;
>
> 		crtc_state->csc_mode |= ICL_CSC_ENABLE;
>+	} else if (crtc_state->output_format != INTEL_OUTPUT_FORMAT_RGB &&
>+		   crtc_state->base.ctm) {
>+		/*
>+		 * There is only one pipe CSC unit per pipe on pre-gen11, and
>+		 * we need that for output conversion from RGB->YCBCR. So if
>+		 * CTM is already applied we can't support YCBCR output.
>+		 */
>+		DRM_DEBUG_KMS("YCBCR420 and CTM together are not
>possible\n");
>+		return -EINVAL;
> 	}
>
> 	return 0;
>diff --git a/drivers/gpu/drm/i915/intel_display.c
>b/drivers/gpu/drm/i915/intel_display.c
>index afa21daaae51..81bfdbd99092 100644
>--- a/drivers/gpu/drm/i915/intel_display.c
>+++ b/drivers/gpu/drm/i915/intel_display.c
>@@ -6855,18 +6855,6 @@ static int intel_crtc_compute_config(struct intel_crtc
>*crtc,
> 		return -EINVAL;
> 	}
>
>-	if ((pipe_config->output_format == INTEL_OUTPUT_FORMAT_YCBCR420 ||
>-	     pipe_config->output_format == INTEL_OUTPUT_FORMAT_YCBCR444) &&
>-	     pipe_config->base.ctm) {
>-		/*
>-		 * There is only one pipe CSC unit per pipe, and we need that
>-		 * for output conversion from RGB->YCBCR. So if CTM is already
>-		 * applied we can't support YCBCR420 output.
>-		 */
>-		DRM_DEBUG_KMS("YCBCR420 and CTM together are not
>possible\n");
>-		return -EINVAL;
>-	}
>-
> 	/*
> 	 * Pipe horizontal size must be even in:
> 	 * - DVO ganged mode
>--
>2.14.5