[Intel-gfx] [PATCH 4/4] drm/i915: Don't mangle the CTM on pre-HSW

[Shankar, Uma]

>-----Original Message-----
>From: Ville Syrjala [mailto:ville.syrjala at linux.intel.com]
>Sent: Friday, February 23, 2018 3:13 AM
>To: intel-gfx at lists.freedesktop.org
>Cc: Lin, Johnson <johnson.lin at intel.com>; Shankar, Uma
><uma.shankar at intel.com>; Sharma, Shashank <shashank.sharma at intel.com>
>Subject: [PATCH 4/4] drm/i915: Don't mangle the CTM on pre-HSW
>
>From: Ville Syrjälä <ville.syrjala at linux.intel.com>
>
>On pre-HSW we have dedicated hardware for the RGB limited range handling, and
>so we don't want to compress with the CSC matrix.
>
>Toss in a FIXME about gamma LUT vs. limited range using the CSC.
>
>Cc: Johnson Lin <johnson.lin at intel.com>
>Cc: Uma Shankar <uma.shankar at intel.com>
>Cc: Shashank Sharma <shashank.sharma at intel.com>
>Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
>---
> drivers/gpu/drm/i915/intel_color.c | 16 ++++++++++++----
> 1 file changed, 12 insertions(+), 4 deletions(-)
>
>diff --git a/drivers/gpu/drm/i915/intel_color.c
>b/drivers/gpu/drm/i915/intel_color.c
>index af1e61d3bacd..89ab0f70aa22 100644
>--- a/drivers/gpu/drm/i915/intel_color.c
>+++ b/drivers/gpu/drm/i915/intel_color.c
>@@ -140,6 +140,14 @@ static void ilk_load_csc_matrix(struct drm_crtc_state
>*crtc_state)
> 	int i, pipe = intel_crtc->pipe;
> 	uint16_t coeffs[9] = { 0, };
> 	struct intel_crtc_state *intel_crtc_state = to_intel_crtc_state(crtc_state);
>+	bool limited_color_range = false;
>+
>+	/*
>+	 * FIXME if there's a gamma LUT after the CSC, we should
>+	 * do the range compression using the gamma LUT instead.
>+	 */
>+	if (INTEL_GEN(dev_priv) >= 8 || IS_HASWELL(dev_priv))
>+		limited_color_range = intel_crtc_state->limited_color_range;
>

Hi Ville,
For VLV or similar platforms (having dedicated color range h/w) for limited_range this matrix
would have been getting used. Though they have a dedicated h/w but I don't think it's getting
programmed currently. Not sure, with removing this CSC scaling logic, it may break the
limited_color scenarios on those platforms. I believe using the dedicated h/w or this scaled_down
CSC should be giving similar results making the things work currently. Not sure if we are using
any limited_range combinations on those platforms though :)

Regards,
Uma Shankar

> 	if (intel_crtc_state->ycbcr420) {
> 		ilk_load_ycbcr_conversion_matrix(intel_crtc);
>@@ -150,7 +158,7 @@ static void ilk_load_csc_matrix(struct drm_crtc_state
>*crtc_state)
> 		const u64 *input;
> 		u64 temp[9];
>
>-		if (intel_crtc_state->limited_color_range)
>+		if (limited_color_range)
> 			input = ctm_mult_by_limited(temp, ctm->matrix);
> 		else
> 			input = ctm->matrix;
>@@ -200,7 +208,7 @@ static void ilk_load_csc_matrix(struct drm_crtc_state
>*crtc_state)
> 		 * into consideration.
> 		 */
> 		for (i = 0; i < 3; i++) {
>-			if (intel_crtc_state->limited_color_range)
>+			if (limited_color_range)
> 				coeffs[i * 3 + i] =
> 					ILK_CSC_COEFF_LIMITED_RANGE;
> 			else
>@@ -224,7 +232,7 @@ static void ilk_load_csc_matrix(struct drm_crtc_state
>*crtc_state)
> 	if (INTEL_GEN(dev_priv) > 6) {
> 		uint16_t postoff = 0;
>
>-		if (intel_crtc_state->limited_color_range)
>+		if (limited_color_range)
> 			postoff = (16 * (1 << 12) / 255) & 0x1fff;
>
> 		I915_WRITE(PIPE_CSC_POSTOFF_HI(pipe), postoff); @@ -235,7
>+243,7 @@ static void ilk_load_csc_matrix(struct drm_crtc_state *crtc_state)
> 	} else {
> 		uint32_t mode = CSC_MODE_YUV_TO_RGB;
>
>-		if (intel_crtc_state->limited_color_range)
>+		if (limited_color_range)
> 			mode |= CSC_BLACK_SCREEN_OFFSET;
>
> 		I915_WRITE(PIPE_CSC_MODE(pipe), mode);
>--
>2.16.1