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

[Ville Syrjälä]

On Fri, Feb 23, 2018 at 01:33:42PM +0000, Shankar, Uma wrote:
> 
> 
> >-----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 :)

All pre-HSW platforms that have the pipe CSC (ILK,SNB,IVB) are using
the dedicated hardware for the limited range RGB output. We don't use
the CSC on those platforms for anything at the moment so it doesn't
actually matter what we program into it. But we want to start using
the CSC for ctm and ycbcr444 output which means we have to start
setting it up correctly.

> 
> 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
> 

-- 
Ville Syrjälä
Intel OTC