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

From: "Shankar, Uma" <uma.shankar@intel.com>
To: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
Cc: "intel-gfx@lists.freedesktop.org"
	<intel-gfx@lists.freedesktop.org>,
	"Lin, Johnson" <johnson.lin@intel.com>
Subject: Re: [PATCH 4/4] drm/i915: Don't mangle the CTM on pre-HSW
Date: Wed, 28 Feb 2018 13:11:12 +0000	[thread overview]
Message-ID: <E7C9878FBA1C6D42A1CA3F62AEB6945F7F125595@BGSMSX104.gar.corp.intel.com> (raw)
In-Reply-To: <20180223135250.GH5453@intel.com>

>-----Original Message-----
>From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com]
>Sent: Friday, February 23, 2018 7:23 PM
>To: Shankar, Uma <uma.shankar@intel.com>
>Cc: intel-gfx@lists.freedesktop.org; Lin, Johnson <johnson.lin@intel.com>;
>Sharma, Shashank <shashank.sharma@intel.com>
>Subject: Re: [PATCH 4/4] drm/i915: Don't mangle the CTM on pre-HSW
>
>On Fri, Feb 23, 2018 at 01:33:42PM +0000, Shankar, Uma wrote:
>>
>>
>> >-----Original Message-----
>> >From: Ville Syrjala [mailto:ville.syrjala@linux.intel.com]
>> >Sent: Friday, February 23, 2018 3:13 AM
>> >To: intel-gfx@lists.freedesktop.org
>> >Cc: Lin, Johnson <johnson.lin@intel.com>; Shankar, Uma
>> ><uma.shankar@intel.com>; Sharma, Shashank <shashank.sharma@intel.com>
>> >Subject: [PATCH 4/4] drm/i915: Don't mangle the CTM on pre-HSW
>> >
>> >From: Ville Syrjälä <ville.syrjala@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@intel.com>
>> >Cc: Uma Shankar <uma.shankar@intel.com>
>> >Cc: Shashank Sharma <shashank.sharma@intel.com>
>> >Signed-off-by: Ville Syrjälä <ville.syrjala@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.
>

Sounds good. 
Reviewed-by: Uma Shankar <uma.shankar@intel.com>

>>
>> 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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx