>-----Original Message----- >From: Ville Syrjälä [mailto:ville.syrjala@xxxxxxxxxxxxxxx] >Sent: Friday, February 23, 2018 7:23 PM >To: Shankar, Uma <uma.shankar@xxxxxxxxx> >Cc: intel-gfx@xxxxxxxxxxxxxxxxxxxxx; Lin, Johnson <johnson.lin@xxxxxxxxx>; >Sharma, Shashank <shashank.sharma@xxxxxxxxx> >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@xxxxxxxxxxxxxxx] >> >Sent: Friday, February 23, 2018 3:13 AM >> >To: intel-gfx@xxxxxxxxxxxxxxxxxxxxx >> >Cc: Lin, Johnson <johnson.lin@xxxxxxxxx>; Shankar, Uma >> ><uma.shankar@xxxxxxxxx>; Sharma, Shashank <shashank.sharma@xxxxxxxxx> >> >Subject: [PATCH 4/4] drm/i915: Don't mangle the CTM on pre-HSW >> > >> >From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> >> > >> >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@xxxxxxxxx> >> >Cc: Uma Shankar <uma.shankar@xxxxxxxxx> >> >Cc: Shashank Sharma <shashank.sharma@xxxxxxxxx> >> >Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> >> >--- >> > 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@xxxxxxxxx> >> >> 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@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
