| Message ID | 20190218193137.22914-4-ville.syrjala@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915: Clean up ilk+ csc stuff | expand |
>-----Original Message----- >From: Ville Syrjala [mailto:ville.syrjala@linux.intel.com] >Sent: Tuesday, February 19, 2019 1:02 AM >To: intel-gfx@lists.freedesktop.org >Cc: Shankar, Uma <uma.shankar@intel.com>; Roper, Matthew D ><matthew.d.roper@intel.com> >Subject: [PATCH 3/7] drm/i915: Extract ilk_csc_limited_range() > >From: Ville Syrjälä <ville.syrjala@linux.intel.com> > >Extract a helper which determines if we need to use the pipe CSC for limited range >RGB output. > >Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> >--- > drivers/gpu/drm/i915/intel_color.c | 22 ++++++++++++++-------- > 1 file changed, 14 insertions(+), 8 deletions(-) > >diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c >index 93428d86510a..ddc48c0d45ac 100644 >--- a/drivers/gpu/drm/i915/intel_color.c >+++ b/drivers/gpu/drm/i915/intel_color.c >@@ -161,22 +161,28 @@ static void ilk_load_ycbcr_conversion_matrix(struct >intel_crtc *crtc) > } > } > >+static bool ilk_csc_limited_range(const struct intel_crtc_state >+*crtc_state) { >+ struct drm_i915_private *dev_priv = >+to_i915(crtc_state->base.crtc->dev); >+ >+ /* >+ * FIXME if there's a gamma LUT after the CSC, we should >+ * do the range compression using the gamma LUT instead. >+ */ >+ return crtc_state->limited_color_range && >+ (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv) || >+ IS_GEN_RANGE(dev_priv, 9, 10)); We should include Gen8 also to this list. Is it intentional to drop that ? With this fixed or justified reasoning, Reviewed-by: Uma Shankar <uma.shankar@intel.com> >+} >+ > static void ilk_load_csc_matrix(const struct intel_crtc_state *crtc_state) { > struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc); > struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); >- bool limited_color_range = false; >+ bool limited_color_range = ilk_csc_limited_range(crtc_state); > enum pipe pipe = crtc->pipe; > u16 coeffs[9] = {}; > int i; > >- /* >- * 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 = crtc_state->limited_color_range; >- > if (crtc_state->output_format == INTEL_OUTPUT_FORMAT_YCBCR420 || > crtc_state->output_format == INTEL_OUTPUT_FORMAT_YCBCR444) { > ilk_load_ycbcr_conversion_matrix(crtc); >-- >2.19.2
On Wed, Mar 13, 2019 at 03:30:43PM +0000, Shankar, Uma wrote: > > > >-----Original Message----- > >From: Ville Syrjala [mailto:ville.syrjala@linux.intel.com] > >Sent: Tuesday, February 19, 2019 1:02 AM > >To: intel-gfx@lists.freedesktop.org > >Cc: Shankar, Uma <uma.shankar@intel.com>; Roper, Matthew D > ><matthew.d.roper@intel.com> > >Subject: [PATCH 3/7] drm/i915: Extract ilk_csc_limited_range() > > > >From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > >Extract a helper which determines if we need to use the pipe CSC for limited range > >RGB output. > > > >Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > >--- > > drivers/gpu/drm/i915/intel_color.c | 22 ++++++++++++++-------- > > 1 file changed, 14 insertions(+), 8 deletions(-) > > > >diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c > >index 93428d86510a..ddc48c0d45ac 100644 > >--- a/drivers/gpu/drm/i915/intel_color.c > >+++ b/drivers/gpu/drm/i915/intel_color.c > >@@ -161,22 +161,28 @@ static void ilk_load_ycbcr_conversion_matrix(struct > >intel_crtc *crtc) > > } > > } > > > >+static bool ilk_csc_limited_range(const struct intel_crtc_state > >+*crtc_state) { > >+ struct drm_i915_private *dev_priv = > >+to_i915(crtc_state->base.crtc->dev); > >+ > >+ /* > >+ * FIXME if there's a gamma LUT after the CSC, we should > >+ * do the range compression using the gamma LUT instead. > >+ */ > >+ return crtc_state->limited_color_range && > >+ (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv) || > >+ IS_GEN_RANGE(dev_priv, 9, 10)); > > We should include Gen8 also to this list. Is it intentional to drop that ? IS_BROADWELL is the gen8 we care about. > With this fixed or justified reasoning, > Reviewed-by: Uma Shankar <uma.shankar@intel.com> > > >+} > >+ > > static void ilk_load_csc_matrix(const struct intel_crtc_state *crtc_state) { > > struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc); > > struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); > >- bool limited_color_range = false; > >+ bool limited_color_range = ilk_csc_limited_range(crtc_state); > > enum pipe pipe = crtc->pipe; > > u16 coeffs[9] = {}; > > int i; > > > >- /* > >- * 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 = crtc_state->limited_color_range; > >- > > if (crtc_state->output_format == INTEL_OUTPUT_FORMAT_YCBCR420 || > > crtc_state->output_format == INTEL_OUTPUT_FORMAT_YCBCR444) { > > ilk_load_ycbcr_conversion_matrix(crtc); > >-- > >2.19.2 >
>-----Original Message----- >From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com] >Sent: Wednesday, March 13, 2019 10:01 PM >To: Shankar, Uma <uma.shankar@intel.com> >Cc: intel-gfx@lists.freedesktop.org; Roper, Matthew D ><matthew.d.roper@intel.com> >Subject: Re: [PATCH 3/7] drm/i915: Extract ilk_csc_limited_range() > >On Wed, Mar 13, 2019 at 03:30:43PM +0000, Shankar, Uma wrote: >> >> >> >-----Original Message----- >> >From: Ville Syrjala [mailto:ville.syrjala@linux.intel.com] >> >Sent: Tuesday, February 19, 2019 1:02 AM >> >To: intel-gfx@lists.freedesktop.org >> >Cc: Shankar, Uma <uma.shankar@intel.com>; Roper, Matthew D >> ><matthew.d.roper@intel.com> >> >Subject: [PATCH 3/7] drm/i915: Extract ilk_csc_limited_range() >> > >> >From: Ville Syrjälä <ville.syrjala@linux.intel.com> >> > >> >Extract a helper which determines if we need to use the pipe CSC for >> >limited range RGB output. >> > >> >Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> >> >--- >> > drivers/gpu/drm/i915/intel_color.c | 22 ++++++++++++++-------- >> > 1 file changed, 14 insertions(+), 8 deletions(-) >> > >> >diff --git a/drivers/gpu/drm/i915/intel_color.c >> >b/drivers/gpu/drm/i915/intel_color.c >> >index 93428d86510a..ddc48c0d45ac 100644 >> >--- a/drivers/gpu/drm/i915/intel_color.c >> >+++ b/drivers/gpu/drm/i915/intel_color.c >> >@@ -161,22 +161,28 @@ static void >> >ilk_load_ycbcr_conversion_matrix(struct >> >intel_crtc *crtc) >> > } >> > } >> > >> >+static bool ilk_csc_limited_range(const struct intel_crtc_state >> >+*crtc_state) { >> >+ struct drm_i915_private *dev_priv = >> >+to_i915(crtc_state->base.crtc->dev); >> >+ >> >+ /* >> >+ * FIXME if there's a gamma LUT after the CSC, we should >> >+ * do the range compression using the gamma LUT instead. >> >+ */ >> >+ return crtc_state->limited_color_range && >> >+ (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv) || >> >+ IS_GEN_RANGE(dev_priv, 9, 10)); >> >> We should include Gen8 also to this list. Is it intentional to drop that ? > >IS_BROADWELL is the gen8 we care about. Ok, thanks for clarification. >> With this fixed or justified reasoning, >> Reviewed-by: Uma Shankar <uma.shankar@intel.com> >> >> >+} >> >+ >> > static void ilk_load_csc_matrix(const struct intel_crtc_state *crtc_state) { >> > struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc); >> > struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); >> >- bool limited_color_range = false; >> >+ bool limited_color_range = ilk_csc_limited_range(crtc_state); >> > enum pipe pipe = crtc->pipe; >> > u16 coeffs[9] = {}; >> > int i; >> > >> >- /* >> >- * 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 = crtc_state->limited_color_range; >> >- >> > if (crtc_state->output_format == INTEL_OUTPUT_FORMAT_YCBCR420 || >> > crtc_state->output_format == INTEL_OUTPUT_FORMAT_YCBCR444) { >> > ilk_load_ycbcr_conversion_matrix(crtc); >> >-- >> >2.19.2 >> > >-- >Ville Syrjälä >Intel
diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c index 93428d86510a..ddc48c0d45ac 100644 --- a/drivers/gpu/drm/i915/intel_color.c +++ b/drivers/gpu/drm/i915/intel_color.c @@ -161,22 +161,28 @@ static void ilk_load_ycbcr_conversion_matrix(struct intel_crtc *crtc) } } +static bool ilk_csc_limited_range(const struct intel_crtc_state *crtc_state) +{ + struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev); + + /* + * FIXME if there's a gamma LUT after the CSC, we should + * do the range compression using the gamma LUT instead. + */ + return crtc_state->limited_color_range && + (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv) || + IS_GEN_RANGE(dev_priv, 9, 10)); +} + static void ilk_load_csc_matrix(const struct intel_crtc_state *crtc_state) { struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc); struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); - bool limited_color_range = false; + bool limited_color_range = ilk_csc_limited_range(crtc_state); enum pipe pipe = crtc->pipe; u16 coeffs[9] = {}; int i; - /* - * 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 = crtc_state->limited_color_range; - if (crtc_state->output_format == INTEL_OUTPUT_FORMAT_YCBCR420 || crtc_state->output_format == INTEL_OUTPUT_FORMAT_YCBCR444) { ilk_load_ycbcr_conversion_matrix(crtc);