[v2,07/13] drm/i915: Populate gamma_mode for all platforms

Message ID	20190205160848.24662-8-ville.syrjala@linux.intel.com (mailing list archive)
State	New, archived
Headers	show Return-Path: <intel-gfx-bounces@lists.freedesktop.org> From: Ville Syrjala <ville.syrjala@linux.intel.com> To: intel-gfx@lists.freedesktop.org Date: Tue, 5 Feb 2019 18:08:42 +0200 Message-Id: <20190205160848.24662-8-ville.syrjala@linux.intel.com> In-Reply-To: <20190205160848.24662-1-ville.syrjala@linux.intel.com> References: <20190205160848.24662-1-ville.syrjala@linux.intel.com> MIME-Version: 1.0 Subject: [Intel-gfx] [PATCH v2 07/13] drm/i915: Populate gamma_mode for all platforms Precedence: list Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" <intel-gfx-bounces@lists.freedesktop.org>
Series	Enable/disable gamma/csc dynamically and fix C8 \| expand [v2,00/13] Enable/disable gamma/csc dynamically and fix C8 [v2,01/13] drm/i915: Split the gamma/csc enable bits from the plane_ctl() function [v2,02/13] drm/i915: Precompute gamma_mode [v2,03/13] drm/i915: Constify the state arguments to the color management stuff [v2,04/13] drm/i915: Pull GAMMA_MODE write out from haswell_load_luts() [v2,05/13] drm/i915: Split color mgmt based on single vs. double buffered registers [v2,06/13] drm/i915: Move LUT programming to happen after vblank waits [v2,07/13] drm/i915: Populate gamma_mode for all platforms [v2,08/13] drm/i915: Track pipe gamma enable/disable in crtc state [v2,09/13] drm/i915: Track pipe csc enable in crtc state [v2,10/13] drm/i915: Turn off pipe gamma when it's not needed [v2,11/13] drm/i915: Turn off pipe CSC when it's not needed [v2,12/13] drm/i915: Disable pipe gamma when C8 pixel format is used [v2,13/13] drm/i915: Update DSPCNTR gamma/csc bits during crtc_enable()

Message ID

20190205160848.24662-8-ville.syrjala@linux.intel.com (mailing list archive)

State

New, archived

Headers

From: Ville Syrjala <ville.syrjala@linux.intel.com>
To: intel-gfx@lists.freedesktop.org
Date: Tue,  5 Feb 2019 18:08:42 +0200
Message-Id: <20190205160848.24662-8-ville.syrjala@linux.intel.com>
In-Reply-To: <20190205160848.24662-1-ville.syrjala@linux.intel.com>
References: <20190205160848.24662-1-ville.syrjala@linux.intel.com>
MIME-Version: 1.0
Subject: [Intel-gfx] [PATCH v2 07/13] drm/i915: Populate gamma_mode for all
 platforms
Precedence: list
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: base64
Errors-To: intel-gfx-bounces@lists.freedesktop.org
Sender: "Intel-gfx" <intel-gfx-bounces@lists.freedesktop.org>

Series

Enable/disable gamma/csc dynamically and fix C8 | expand

Commit Message

Ville Syrjälä Feb. 5, 2019, 4:08 p.m. UTC

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

On pre-HSW gamma mode is configured via PIPECONF. The bits are
the same except shifted up, so we can reuse just store them in
crtc_state->gamma_mode in the HSW+ way, allowing us to share
some code later.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h      | 10 ++++-
 drivers/gpu/drm/i915/intel_color.c   | 60 +++++++++++++++++++++-------
 drivers/gpu/drm/i915/intel_display.c | 14 ++++++-
 3 files changed, 66 insertions(+), 18 deletions(-)

Comments

Maarten Lankhorst Feb. 7, 2019, 3:49 p.m. UTC | #1

Op 05-02-2019 om 17:08 schreef Ville Syrjala:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> On pre-HSW gamma mode is configured via PIPECONF. The bits are
> the same except shifted up, so we can reuse just store them in
> crtc_state->gamma_mode in the HSW+ way, allowing us to share
> some code later.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h      | 10 ++++-
>  drivers/gpu/drm/i915/intel_color.c   | 60 +++++++++++++++++++++-------
>  drivers/gpu/drm/i915/intel_display.c | 14 ++++++-
>  3 files changed, 66 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index ede54fdc1676..41111a17e519 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -5590,9 +5590,15 @@ enum {
>  #define   PIPECONF_SINGLE_WIDE	0
>  #define   PIPECONF_PIPE_UNLOCKED 0
>  #define   PIPECONF_PIPE_LOCKED	(1 << 25)
> -#define   PIPECONF_PALETTE	0
> -#define   PIPECONF_GAMMA		(1 << 24)
>  #define   PIPECONF_FORCE_BORDER	(1 << 25)
> +#define   PIPECONF_GAMMA_MODE_MASK_I9XX	(1 << 24) /* gmch */
> +#define   PIPECONF_GAMMA_MODE_MASK_ILK	(3 << 24) /* ilk-ivb */
> +#define   PIPECONF_GAMMA_MODE_8BIT	(0 << 24) /* gmch,ilk-ivb */
> +#define   PIPECONF_GAMMA_MODE_10BIT	(1 << 24) /* gmch,ilk-ivb */
> +#define   PIPECONF_GAMMA_MODE_12BIT	(2 << 24) /* ilk-ivb */
> +#define   PIPECONF_GAMMA_MODE_SPLIT	(3 << 24) /* ivb */
> +#define   PIPECONF_GAMMA_MODE(x)	((x)<<24) /* pass in GAMMA_MODE_MODE_* */
> +#define   PIPECONF_GAMMA_MODE_SHIFT	24
>  #define   PIPECONF_INTERLACE_MASK	(7 << 21)
>  #define   PIPECONF_INTERLACE_MASK_HSW	(3 << 21)
>  /* Note that pre-gen3 does not support interlaced display directly. Panel
> diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
> index 2a371eed8061..e7dd07490c68 100644
> --- a/drivers/gpu/drm/i915/intel_color.c
> +++ b/drivers/gpu/drm/i915/intel_color.c
> @@ -351,6 +351,32 @@ static void i9xx_load_luts(const struct intel_crtc_state *crtc_state)
>  	i9xx_load_luts_internal(crtc_state, crtc_state->base.gamma_lut);
>  }
>  
> +static void i9xx_color_commit(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);
> +	enum pipe pipe = crtc->pipe;
> +	u32 val;
> +
> +	val = I915_READ(PIPECONF(pipe));
> +	val &= ~PIPECONF_GAMMA_MODE_MASK_I9XX;
> +	val |= PIPECONF_GAMMA_MODE(crtc_state->gamma_mode);
> +	I915_WRITE(PIPECONF(pipe), val);
> +}
> +
> +static void ilk_color_commit(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);
> +	enum pipe pipe = crtc->pipe;
> +	u32 val;
> +
> +	val = I915_READ(PIPECONF(pipe));
> +	val &= ~PIPECONF_GAMMA_MODE_MASK_ILK;
> +	val |= PIPECONF_GAMMA_MODE(crtc_state->gamma_mode);
> +	I915_WRITE(PIPECONF(pipe), val);
> +}
> +
>  static void hsw_color_commit(const struct intel_crtc_state *crtc_state)
>  {
>  	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
> @@ -585,8 +611,7 @@ void intel_color_commit(const struct intel_crtc_state *crtc_state)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev);
>  
> -	if (dev_priv->display.color_commit)
> -		dev_priv->display.color_commit(crtc_state);
> +	dev_priv->display.color_commit(crtc_state);
>  }
>  
>  static int check_lut_size(const struct drm_property_blob *lut, int expected)
> @@ -649,20 +674,25 @@ void intel_color_init(struct intel_crtc *crtc)
>  
>  	drm_mode_crtc_set_gamma_size(&crtc->base, 256);
>  
> -	if (IS_CHERRYVIEW(dev_priv)) {
> -		dev_priv->display.load_luts = cherryview_load_luts;
> -	} else if (IS_HASWELL(dev_priv)) {
> -		dev_priv->display.load_luts = i9xx_load_luts;
> -		dev_priv->display.color_commit = hsw_color_commit;
> -	} else if (IS_BROADWELL(dev_priv) || IS_GEN9_BC(dev_priv) ||
> -		   IS_BROXTON(dev_priv)) {
> -		dev_priv->display.load_luts = broadwell_load_luts;
> -		dev_priv->display.color_commit = hsw_color_commit;
> -	} else if (IS_GEMINILAKE(dev_priv) || IS_CANNONLAKE(dev_priv)) {
> -		dev_priv->display.load_luts = glk_load_luts;
> -		dev_priv->display.color_commit = hsw_color_commit;
> +	if (HAS_GMCH_DISPLAY(dev_priv)) {
> +		if (IS_CHERRYVIEW(dev_priv))
> +			dev_priv->display.load_luts = cherryview_load_luts;
> +		else
> +			dev_priv->display.load_luts = i9xx_load_luts;
> +
> +		dev_priv->display.color_commit = i9xx_color_commit;
>  	} else {
> -		dev_priv->display.load_luts = i9xx_load_luts;
> +		if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
> +			dev_priv->display.load_luts = glk_load_luts;
> +		else if (INTEL_GEN(dev_priv) >= 9 || IS_BROADWELL(dev_priv))
> +			dev_priv->display.load_luts = broadwell_load_luts;
> +		else
> +			dev_priv->display.load_luts = i9xx_load_luts;
> +
> +		if (INTEL_GEN(dev_priv) >= 8 || IS_HASWELL(dev_priv))
> +			dev_priv->display.color_commit = hsw_color_commit;
> +		else
> +			dev_priv->display.color_commit = ilk_color_commit;
>  	}
>  
>  	/* Enable color management support when we have degamma & gamma LUTs. */
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index ad5d39d81d6e..8e93459f312f 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -3450,7 +3450,7 @@ static void i9xx_disable_plane(struct intel_plane *plane,
>  	 *
>  	 * On pre-g4x there is no way to gamma correct the
>  	 * pipe bottom color but we'll keep on doing this
> -	 * anyway.
> +	 * anyway so that the crtc state readout works correctly.
>  	 */
>  	dspcntr = i9xx_plane_ctl_crtc(crtc_state);
>  
> @@ -7674,6 +7674,8 @@ static void i9xx_set_pipeconf(const struct intel_crtc_state *crtc_state)
>  	     crtc_state->limited_color_range)
>  		pipeconf |= PIPECONF_COLOR_RANGE_SELECT;
>  
> +	pipeconf |= PIPECONF_GAMMA_MODE(crtc_state->gamma_mode);
> +
>  	I915_WRITE(PIPECONF(crtc->pipe), pipeconf);
>  	POSTING_READ(PIPECONF(crtc->pipe));
>  }
> @@ -8126,6 +8128,9 @@ static bool i9xx_get_pipe_config(struct intel_crtc *crtc,
>  	    (tmp & PIPECONF_COLOR_RANGE_SELECT))
>  		pipe_config->limited_color_range = true;
>  
> +	pipe_config->gamma_mode = (tmp & PIPECONF_GAMMA_MODE_MASK_I9XX) >>
> +		PIPECONF_GAMMA_MODE_SHIFT;
> +
>  	if (INTEL_GEN(dev_priv) < 4)
>  		pipe_config->double_wide = tmp & PIPECONF_DOUBLE_WIDE;
>  
> @@ -8665,6 +8670,8 @@ static void ironlake_set_pipeconf(const struct intel_crtc_state *crtc_state)
>  	if (crtc_state->limited_color_range)
>  		val |= PIPECONF_COLOR_RANGE_SELECT;
>  
> +	val |= PIPECONF_GAMMA_MODE(crtc_state->gamma_mode);
> +
>  	I915_WRITE(PIPECONF(pipe), val);
>  	POSTING_READ(PIPECONF(pipe));
>  }
> @@ -9199,6 +9206,9 @@ static bool ironlake_get_pipe_config(struct intel_crtc *crtc,
>  	if (tmp & PIPECONF_COLOR_RANGE_SELECT)
>  		pipe_config->limited_color_range = true;
>  
> +	pipe_config->gamma_mode = (tmp & PIPECONF_GAMMA_MODE_MASK_ILK) >>
> +		PIPECONF_GAMMA_MODE_SHIFT;
> +
>  	if (I915_READ(PCH_TRANSCONF(crtc->pipe)) & TRANS_ENABLE) {
>  		struct intel_shared_dpll *pll;
>  		enum intel_dpll_id pll_id;
> @@ -12069,6 +12079,8 @@ intel_pipe_config_compare(struct drm_i915_private *dev_priv,
>  
>  	PIPE_CONF_CHECK_BOOL(double_wide);
>  
> +	PIPE_CONF_CHECK_X(gamma_mode);

Hmm...  We should only check this if adjust is unset, or we will start skipping fastset on a lot of platforms.

With that fixed:

Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>

~Maarten

Ville Syrjälä Feb. 7, 2019, 4:27 p.m. UTC | #2

On Thu, Feb 07, 2019 at 04:49:47PM +0100, Maarten Lankhorst wrote:
> Op 05-02-2019 om 17:08 schreef Ville Syrjala:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > On pre-HSW gamma mode is configured via PIPECONF. The bits are
> > the same except shifted up, so we can reuse just store them in
> > crtc_state->gamma_mode in the HSW+ way, allowing us to share
> > some code later.
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_reg.h      | 10 ++++-
> >  drivers/gpu/drm/i915/intel_color.c   | 60 +++++++++++++++++++++-------
> >  drivers/gpu/drm/i915/intel_display.c | 14 ++++++-
> >  3 files changed, 66 insertions(+), 18 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index ede54fdc1676..41111a17e519 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -5590,9 +5590,15 @@ enum {
> >  #define   PIPECONF_SINGLE_WIDE	0
> >  #define   PIPECONF_PIPE_UNLOCKED 0
> >  #define   PIPECONF_PIPE_LOCKED	(1 << 25)
> > -#define   PIPECONF_PALETTE	0
> > -#define   PIPECONF_GAMMA		(1 << 24)
> >  #define   PIPECONF_FORCE_BORDER	(1 << 25)
> > +#define   PIPECONF_GAMMA_MODE_MASK_I9XX	(1 << 24) /* gmch */
> > +#define   PIPECONF_GAMMA_MODE_MASK_ILK	(3 << 24) /* ilk-ivb */
> > +#define   PIPECONF_GAMMA_MODE_8BIT	(0 << 24) /* gmch,ilk-ivb */
> > +#define   PIPECONF_GAMMA_MODE_10BIT	(1 << 24) /* gmch,ilk-ivb */
> > +#define   PIPECONF_GAMMA_MODE_12BIT	(2 << 24) /* ilk-ivb */
> > +#define   PIPECONF_GAMMA_MODE_SPLIT	(3 << 24) /* ivb */
> > +#define   PIPECONF_GAMMA_MODE(x)	((x)<<24) /* pass in GAMMA_MODE_MODE_* */
> > +#define   PIPECONF_GAMMA_MODE_SHIFT	24
> >  #define   PIPECONF_INTERLACE_MASK	(7 << 21)
> >  #define   PIPECONF_INTERLACE_MASK_HSW	(3 << 21)
> >  /* Note that pre-gen3 does not support interlaced display directly. Panel
> > diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
> > index 2a371eed8061..e7dd07490c68 100644
> > --- a/drivers/gpu/drm/i915/intel_color.c
> > +++ b/drivers/gpu/drm/i915/intel_color.c
> > @@ -351,6 +351,32 @@ static void i9xx_load_luts(const struct intel_crtc_state *crtc_state)
> >  	i9xx_load_luts_internal(crtc_state, crtc_state->base.gamma_lut);
> >  }
> >  
> > +static void i9xx_color_commit(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);
> > +	enum pipe pipe = crtc->pipe;
> > +	u32 val;
> > +
> > +	val = I915_READ(PIPECONF(pipe));
> > +	val &= ~PIPECONF_GAMMA_MODE_MASK_I9XX;
> > +	val |= PIPECONF_GAMMA_MODE(crtc_state->gamma_mode);
> > +	I915_WRITE(PIPECONF(pipe), val);
> > +}
> > +
> > +static void ilk_color_commit(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);
> > +	enum pipe pipe = crtc->pipe;
> > +	u32 val;
> > +
> > +	val = I915_READ(PIPECONF(pipe));
> > +	val &= ~PIPECONF_GAMMA_MODE_MASK_ILK;
> > +	val |= PIPECONF_GAMMA_MODE(crtc_state->gamma_mode);
> > +	I915_WRITE(PIPECONF(pipe), val);
> > +}
> > +
> >  static void hsw_color_commit(const struct intel_crtc_state *crtc_state)
> >  {
> >  	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
> > @@ -585,8 +611,7 @@ void intel_color_commit(const struct intel_crtc_state *crtc_state)
> >  {
> >  	struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev);
> >  
> > -	if (dev_priv->display.color_commit)
> > -		dev_priv->display.color_commit(crtc_state);
> > +	dev_priv->display.color_commit(crtc_state);
> >  }
> >  
> >  static int check_lut_size(const struct drm_property_blob *lut, int expected)
> > @@ -649,20 +674,25 @@ void intel_color_init(struct intel_crtc *crtc)
> >  
> >  	drm_mode_crtc_set_gamma_size(&crtc->base, 256);
> >  
> > -	if (IS_CHERRYVIEW(dev_priv)) {
> > -		dev_priv->display.load_luts = cherryview_load_luts;
> > -	} else if (IS_HASWELL(dev_priv)) {
> > -		dev_priv->display.load_luts = i9xx_load_luts;
> > -		dev_priv->display.color_commit = hsw_color_commit;
> > -	} else if (IS_BROADWELL(dev_priv) || IS_GEN9_BC(dev_priv) ||
> > -		   IS_BROXTON(dev_priv)) {
> > -		dev_priv->display.load_luts = broadwell_load_luts;
> > -		dev_priv->display.color_commit = hsw_color_commit;
> > -	} else if (IS_GEMINILAKE(dev_priv) || IS_CANNONLAKE(dev_priv)) {
> > -		dev_priv->display.load_luts = glk_load_luts;
> > -		dev_priv->display.color_commit = hsw_color_commit;
> > +	if (HAS_GMCH_DISPLAY(dev_priv)) {
> > +		if (IS_CHERRYVIEW(dev_priv))
> > +			dev_priv->display.load_luts = cherryview_load_luts;
> > +		else
> > +			dev_priv->display.load_luts = i9xx_load_luts;
> > +
> > +		dev_priv->display.color_commit = i9xx_color_commit;
> >  	} else {
> > -		dev_priv->display.load_luts = i9xx_load_luts;
> > +		if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
> > +			dev_priv->display.load_luts = glk_load_luts;
> > +		else if (INTEL_GEN(dev_priv) >= 9 || IS_BROADWELL(dev_priv))
> > +			dev_priv->display.load_luts = broadwell_load_luts;
> > +		else
> > +			dev_priv->display.load_luts = i9xx_load_luts;
> > +
> > +		if (INTEL_GEN(dev_priv) >= 8 || IS_HASWELL(dev_priv))
> > +			dev_priv->display.color_commit = hsw_color_commit;
> > +		else
> > +			dev_priv->display.color_commit = ilk_color_commit;
> >  	}
> >  
> >  	/* Enable color management support when we have degamma & gamma LUTs. */
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index ad5d39d81d6e..8e93459f312f 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -3450,7 +3450,7 @@ static void i9xx_disable_plane(struct intel_plane *plane,
> >  	 *
> >  	 * On pre-g4x there is no way to gamma correct the
> >  	 * pipe bottom color but we'll keep on doing this
> > -	 * anyway.
> > +	 * anyway so that the crtc state readout works correctly.
> >  	 */
> >  	dspcntr = i9xx_plane_ctl_crtc(crtc_state);
> >  
> > @@ -7674,6 +7674,8 @@ static void i9xx_set_pipeconf(const struct intel_crtc_state *crtc_state)
> >  	     crtc_state->limited_color_range)
> >  		pipeconf |= PIPECONF_COLOR_RANGE_SELECT;
> >  
> > +	pipeconf |= PIPECONF_GAMMA_MODE(crtc_state->gamma_mode);
> > +
> >  	I915_WRITE(PIPECONF(crtc->pipe), pipeconf);
> >  	POSTING_READ(PIPECONF(crtc->pipe));
> >  }
> > @@ -8126,6 +8128,9 @@ static bool i9xx_get_pipe_config(struct intel_crtc *crtc,
> >  	    (tmp & PIPECONF_COLOR_RANGE_SELECT))
> >  		pipe_config->limited_color_range = true;
> >  
> > +	pipe_config->gamma_mode = (tmp & PIPECONF_GAMMA_MODE_MASK_I9XX) >>
> > +		PIPECONF_GAMMA_MODE_SHIFT;
> > +
> >  	if (INTEL_GEN(dev_priv) < 4)
> >  		pipe_config->double_wide = tmp & PIPECONF_DOUBLE_WIDE;
> >  
> > @@ -8665,6 +8670,8 @@ static void ironlake_set_pipeconf(const struct intel_crtc_state *crtc_state)
> >  	if (crtc_state->limited_color_range)
> >  		val |= PIPECONF_COLOR_RANGE_SELECT;
> >  
> > +	val |= PIPECONF_GAMMA_MODE(crtc_state->gamma_mode);
> > +
> >  	I915_WRITE(PIPECONF(pipe), val);
> >  	POSTING_READ(PIPECONF(pipe));
> >  }
> > @@ -9199,6 +9206,9 @@ static bool ironlake_get_pipe_config(struct intel_crtc *crtc,
> >  	if (tmp & PIPECONF_COLOR_RANGE_SELECT)
> >  		pipe_config->limited_color_range = true;
> >  
> > +	pipe_config->gamma_mode = (tmp & PIPECONF_GAMMA_MODE_MASK_ILK) >>
> > +		PIPECONF_GAMMA_MODE_SHIFT;
> > +
> >  	if (I915_READ(PCH_TRANSCONF(crtc->pipe)) & TRANS_ENABLE) {
> >  		struct intel_shared_dpll *pll;
> >  		enum intel_dpll_id pll_id;
> > @@ -12069,6 +12079,8 @@ intel_pipe_config_compare(struct drm_i915_private *dev_priv,
> >  
> >  	PIPE_CONF_CHECK_BOOL(double_wide);
> >  
> > +	PIPE_CONF_CHECK_X(gamma_mode);
> 
> Hmm...  We should only check this if adjust is unset, or we will start skipping fastset on a lot of platforms.

There's no sane macros for this sort of stuff AFAICS. Maybe someone
should add those first? Heck, I don't even know what 'adjust'
really means in this context.

Also on a related note, this fastboot thing has made the logs rather
confusing. It's not at all obvious that all the state mismatch prints
are harmless (especially when the function is called something_err()).
Some people (myself included) were already lead astray by these
apparent "errors".

> 
> With that fixed:
> 
> Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> 
> ~Maarten

Ville Syrjälä Feb. 7, 2019, 5:27 p.m. UTC | #3

On Thu, Feb 07, 2019 at 06:27:19PM +0200, Ville Syrjälä wrote:
> On Thu, Feb 07, 2019 at 04:49:47PM +0100, Maarten Lankhorst wrote:
> > Op 05-02-2019 om 17:08 schreef Ville Syrjala:
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > >
> > > On pre-HSW gamma mode is configured via PIPECONF. The bits are
> > > the same except shifted up, so we can reuse just store them in
> > > crtc_state->gamma_mode in the HSW+ way, allowing us to share
> > > some code later.
> > >
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/i915_reg.h      | 10 ++++-
> > >  drivers/gpu/drm/i915/intel_color.c   | 60 +++++++++++++++++++++-------
> > >  drivers/gpu/drm/i915/intel_display.c | 14 ++++++-
> > >  3 files changed, 66 insertions(+), 18 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > > index ede54fdc1676..41111a17e519 100644
> > > --- a/drivers/gpu/drm/i915/i915_reg.h
> > > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > > @@ -5590,9 +5590,15 @@ enum {
> > >  #define   PIPECONF_SINGLE_WIDE	0
> > >  #define   PIPECONF_PIPE_UNLOCKED 0
> > >  #define   PIPECONF_PIPE_LOCKED	(1 << 25)
> > > -#define   PIPECONF_PALETTE	0
> > > -#define   PIPECONF_GAMMA		(1 << 24)
> > >  #define   PIPECONF_FORCE_BORDER	(1 << 25)
> > > +#define   PIPECONF_GAMMA_MODE_MASK_I9XX	(1 << 24) /* gmch */
> > > +#define   PIPECONF_GAMMA_MODE_MASK_ILK	(3 << 24) /* ilk-ivb */
> > > +#define   PIPECONF_GAMMA_MODE_8BIT	(0 << 24) /* gmch,ilk-ivb */
> > > +#define   PIPECONF_GAMMA_MODE_10BIT	(1 << 24) /* gmch,ilk-ivb */
> > > +#define   PIPECONF_GAMMA_MODE_12BIT	(2 << 24) /* ilk-ivb */
> > > +#define   PIPECONF_GAMMA_MODE_SPLIT	(3 << 24) /* ivb */
> > > +#define   PIPECONF_GAMMA_MODE(x)	((x)<<24) /* pass in GAMMA_MODE_MODE_* */
> > > +#define   PIPECONF_GAMMA_MODE_SHIFT	24
> > >  #define   PIPECONF_INTERLACE_MASK	(7 << 21)
> > >  #define   PIPECONF_INTERLACE_MASK_HSW	(3 << 21)
> > >  /* Note that pre-gen3 does not support interlaced display directly. Panel
> > > diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
> > > index 2a371eed8061..e7dd07490c68 100644
> > > --- a/drivers/gpu/drm/i915/intel_color.c
> > > +++ b/drivers/gpu/drm/i915/intel_color.c
> > > @@ -351,6 +351,32 @@ static void i9xx_load_luts(const struct intel_crtc_state *crtc_state)
> > >  	i9xx_load_luts_internal(crtc_state, crtc_state->base.gamma_lut);
> > >  }
> > >  
> > > +static void i9xx_color_commit(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);
> > > +	enum pipe pipe = crtc->pipe;
> > > +	u32 val;
> > > +
> > > +	val = I915_READ(PIPECONF(pipe));
> > > +	val &= ~PIPECONF_GAMMA_MODE_MASK_I9XX;
> > > +	val |= PIPECONF_GAMMA_MODE(crtc_state->gamma_mode);
> > > +	I915_WRITE(PIPECONF(pipe), val);
> > > +}
> > > +
> > > +static void ilk_color_commit(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);
> > > +	enum pipe pipe = crtc->pipe;
> > > +	u32 val;
> > > +
> > > +	val = I915_READ(PIPECONF(pipe));
> > > +	val &= ~PIPECONF_GAMMA_MODE_MASK_ILK;
> > > +	val |= PIPECONF_GAMMA_MODE(crtc_state->gamma_mode);
> > > +	I915_WRITE(PIPECONF(pipe), val);
> > > +}
> > > +
> > >  static void hsw_color_commit(const struct intel_crtc_state *crtc_state)
> > >  {
> > >  	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
> > > @@ -585,8 +611,7 @@ void intel_color_commit(const struct intel_crtc_state *crtc_state)
> > >  {
> > >  	struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev);
> > >  
> > > -	if (dev_priv->display.color_commit)
> > > -		dev_priv->display.color_commit(crtc_state);
> > > +	dev_priv->display.color_commit(crtc_state);
> > >  }
> > >  
> > >  static int check_lut_size(const struct drm_property_blob *lut, int expected)
> > > @@ -649,20 +674,25 @@ void intel_color_init(struct intel_crtc *crtc)
> > >  
> > >  	drm_mode_crtc_set_gamma_size(&crtc->base, 256);
> > >  
> > > -	if (IS_CHERRYVIEW(dev_priv)) {
> > > -		dev_priv->display.load_luts = cherryview_load_luts;
> > > -	} else if (IS_HASWELL(dev_priv)) {
> > > -		dev_priv->display.load_luts = i9xx_load_luts;
> > > -		dev_priv->display.color_commit = hsw_color_commit;
> > > -	} else if (IS_BROADWELL(dev_priv) || IS_GEN9_BC(dev_priv) ||
> > > -		   IS_BROXTON(dev_priv)) {
> > > -		dev_priv->display.load_luts = broadwell_load_luts;
> > > -		dev_priv->display.color_commit = hsw_color_commit;
> > > -	} else if (IS_GEMINILAKE(dev_priv) || IS_CANNONLAKE(dev_priv)) {
> > > -		dev_priv->display.load_luts = glk_load_luts;
> > > -		dev_priv->display.color_commit = hsw_color_commit;
> > > +	if (HAS_GMCH_DISPLAY(dev_priv)) {
> > > +		if (IS_CHERRYVIEW(dev_priv))
> > > +			dev_priv->display.load_luts = cherryview_load_luts;
> > > +		else
> > > +			dev_priv->display.load_luts = i9xx_load_luts;
> > > +
> > > +		dev_priv->display.color_commit = i9xx_color_commit;
> > >  	} else {
> > > -		dev_priv->display.load_luts = i9xx_load_luts;
> > > +		if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
> > > +			dev_priv->display.load_luts = glk_load_luts;
> > > +		else if (INTEL_GEN(dev_priv) >= 9 || IS_BROADWELL(dev_priv))
> > > +			dev_priv->display.load_luts = broadwell_load_luts;
> > > +		else
> > > +			dev_priv->display.load_luts = i9xx_load_luts;
> > > +
> > > +		if (INTEL_GEN(dev_priv) >= 8 || IS_HASWELL(dev_priv))
> > > +			dev_priv->display.color_commit = hsw_color_commit;
> > > +		else
> > > +			dev_priv->display.color_commit = ilk_color_commit;
> > >  	}
> > >  
> > >  	/* Enable color management support when we have degamma & gamma LUTs. */
> > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > > index ad5d39d81d6e..8e93459f312f 100644
> > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > @@ -3450,7 +3450,7 @@ static void i9xx_disable_plane(struct intel_plane *plane,
> > >  	 *
> > >  	 * On pre-g4x there is no way to gamma correct the
> > >  	 * pipe bottom color but we'll keep on doing this
> > > -	 * anyway.
> > > +	 * anyway so that the crtc state readout works correctly.
> > >  	 */
> > >  	dspcntr = i9xx_plane_ctl_crtc(crtc_state);
> > >  
> > > @@ -7674,6 +7674,8 @@ static void i9xx_set_pipeconf(const struct intel_crtc_state *crtc_state)
> > >  	     crtc_state->limited_color_range)
> > >  		pipeconf |= PIPECONF_COLOR_RANGE_SELECT;
> > >  
> > > +	pipeconf |= PIPECONF_GAMMA_MODE(crtc_state->gamma_mode);
> > > +
> > >  	I915_WRITE(PIPECONF(crtc->pipe), pipeconf);
> > >  	POSTING_READ(PIPECONF(crtc->pipe));
> > >  }
> > > @@ -8126,6 +8128,9 @@ static bool i9xx_get_pipe_config(struct intel_crtc *crtc,
> > >  	    (tmp & PIPECONF_COLOR_RANGE_SELECT))
> > >  		pipe_config->limited_color_range = true;
> > >  
> > > +	pipe_config->gamma_mode = (tmp & PIPECONF_GAMMA_MODE_MASK_I9XX) >>
> > > +		PIPECONF_GAMMA_MODE_SHIFT;
> > > +
> > >  	if (INTEL_GEN(dev_priv) < 4)
> > >  		pipe_config->double_wide = tmp & PIPECONF_DOUBLE_WIDE;
> > >  
> > > @@ -8665,6 +8670,8 @@ static void ironlake_set_pipeconf(const struct intel_crtc_state *crtc_state)
> > >  	if (crtc_state->limited_color_range)
> > >  		val |= PIPECONF_COLOR_RANGE_SELECT;
> > >  
> > > +	val |= PIPECONF_GAMMA_MODE(crtc_state->gamma_mode);
> > > +
> > >  	I915_WRITE(PIPECONF(pipe), val);
> > >  	POSTING_READ(PIPECONF(pipe));
> > >  }
> > > @@ -9199,6 +9206,9 @@ static bool ironlake_get_pipe_config(struct intel_crtc *crtc,
> > >  	if (tmp & PIPECONF_COLOR_RANGE_SELECT)
> > >  		pipe_config->limited_color_range = true;
> > >  
> > > +	pipe_config->gamma_mode = (tmp & PIPECONF_GAMMA_MODE_MASK_ILK) >>
> > > +		PIPECONF_GAMMA_MODE_SHIFT;
> > > +
> > >  	if (I915_READ(PCH_TRANSCONF(crtc->pipe)) & TRANS_ENABLE) {
> > >  		struct intel_shared_dpll *pll;
> > >  		enum intel_dpll_id pll_id;
> > > @@ -12069,6 +12079,8 @@ intel_pipe_config_compare(struct drm_i915_private *dev_priv,
> > >  
> > >  	PIPE_CONF_CHECK_BOOL(double_wide);
> > >  
> > > +	PIPE_CONF_CHECK_X(gamma_mode);
> > 
> > Hmm...  We should only check this if adjust is unset, or we will start skipping fastset on a lot of platforms.
> 
> There's no sane macros for this sort of stuff AFAICS. Maybe someone
> should add those first? Heck, I don't even know what 'adjust'
> really means in this context.

I guess it means "ignore the mismatch for select pieces of state".
Hmm. Looks like we've just added a bunch of things to the
'if (!adjust)' block. I guess I can move these things there.

> 
> Also on a related note, this fastboot thing has made the logs rather
> confusing. It's not at all obvious that all the state mismatch prints
> are harmless (especially when the function is called something_err()).
> Some people (myself included) were already lead astray by these
> apparent "errors".
> 
> > 
> > With that fixed:
> > 
> > Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > 
> > ~Maarten
> 
> -- 
> Ville Syrjälä
> Intel

Maarten Lankhorst Feb. 8, 2019, 8:51 a.m. UTC | #4

Op 07-02-2019 om 17:27 schreef Ville Syrjälä:
> On Thu, Feb 07, 2019 at 04:49:47PM +0100, Maarten Lankhorst wrote:
>> Op 05-02-2019 om 17:08 schreef Ville Syrjala:
>>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>>
>>> On pre-HSW gamma mode is configured via PIPECONF. The bits are
>>> the same except shifted up, so we can reuse just store them in
>>> crtc_state->gamma_mode in the HSW+ way, allowing us to share
>>> some code later.
>>>
>>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>> ---
>>>  drivers/gpu/drm/i915/i915_reg.h      | 10 ++++-
>>>  drivers/gpu/drm/i915/intel_color.c   | 60 +++++++++++++++++++++-------
>>>  drivers/gpu/drm/i915/intel_display.c | 14 ++++++-
>>>  3 files changed, 66 insertions(+), 18 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>>> index ede54fdc1676..41111a17e519 100644
>>> --- a/drivers/gpu/drm/i915/i915_reg.h
>>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>>> @@ -5590,9 +5590,15 @@ enum {
>>>  #define   PIPECONF_SINGLE_WIDE	0
>>>  #define   PIPECONF_PIPE_UNLOCKED 0
>>>  #define   PIPECONF_PIPE_LOCKED	(1 << 25)
>>> -#define   PIPECONF_PALETTE	0
>>> -#define   PIPECONF_GAMMA		(1 << 24)
>>>  #define   PIPECONF_FORCE_BORDER	(1 << 25)
>>> +#define   PIPECONF_GAMMA_MODE_MASK_I9XX	(1 << 24) /* gmch */
>>> +#define   PIPECONF_GAMMA_MODE_MASK_ILK	(3 << 24) /* ilk-ivb */
>>> +#define   PIPECONF_GAMMA_MODE_8BIT	(0 << 24) /* gmch,ilk-ivb */
>>> +#define   PIPECONF_GAMMA_MODE_10BIT	(1 << 24) /* gmch,ilk-ivb */
>>> +#define   PIPECONF_GAMMA_MODE_12BIT	(2 << 24) /* ilk-ivb */
>>> +#define   PIPECONF_GAMMA_MODE_SPLIT	(3 << 24) /* ivb */
>>> +#define   PIPECONF_GAMMA_MODE(x)	((x)<<24) /* pass in GAMMA_MODE_MODE_* */
>>> +#define   PIPECONF_GAMMA_MODE_SHIFT	24
>>>  #define   PIPECONF_INTERLACE_MASK	(7 << 21)
>>>  #define   PIPECONF_INTERLACE_MASK_HSW	(3 << 21)
>>>  /* Note that pre-gen3 does not support interlaced display directly. Panel
>>> diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
>>> index 2a371eed8061..e7dd07490c68 100644
>>> --- a/drivers/gpu/drm/i915/intel_color.c
>>> +++ b/drivers/gpu/drm/i915/intel_color.c
>>> @@ -351,6 +351,32 @@ static void i9xx_load_luts(const struct intel_crtc_state *crtc_state)
>>>  	i9xx_load_luts_internal(crtc_state, crtc_state->base.gamma_lut);
>>>  }
>>>  
>>> +static void i9xx_color_commit(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);
>>> +	enum pipe pipe = crtc->pipe;
>>> +	u32 val;
>>> +
>>> +	val = I915_READ(PIPECONF(pipe));
>>> +	val &= ~PIPECONF_GAMMA_MODE_MASK_I9XX;
>>> +	val |= PIPECONF_GAMMA_MODE(crtc_state->gamma_mode);
>>> +	I915_WRITE(PIPECONF(pipe), val);
>>> +}
>>> +
>>> +static void ilk_color_commit(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);
>>> +	enum pipe pipe = crtc->pipe;
>>> +	u32 val;
>>> +
>>> +	val = I915_READ(PIPECONF(pipe));
>>> +	val &= ~PIPECONF_GAMMA_MODE_MASK_ILK;
>>> +	val |= PIPECONF_GAMMA_MODE(crtc_state->gamma_mode);
>>> +	I915_WRITE(PIPECONF(pipe), val);
>>> +}
>>> +
>>>  static void hsw_color_commit(const struct intel_crtc_state *crtc_state)
>>>  {
>>>  	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
>>> @@ -585,8 +611,7 @@ void intel_color_commit(const struct intel_crtc_state *crtc_state)
>>>  {
>>>  	struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev);
>>>  
>>> -	if (dev_priv->display.color_commit)
>>> -		dev_priv->display.color_commit(crtc_state);
>>> +	dev_priv->display.color_commit(crtc_state);
>>>  }
>>>  
>>>  static int check_lut_size(const struct drm_property_blob *lut, int expected)
>>> @@ -649,20 +674,25 @@ void intel_color_init(struct intel_crtc *crtc)
>>>  
>>>  	drm_mode_crtc_set_gamma_size(&crtc->base, 256);
>>>  
>>> -	if (IS_CHERRYVIEW(dev_priv)) {
>>> -		dev_priv->display.load_luts = cherryview_load_luts;
>>> -	} else if (IS_HASWELL(dev_priv)) {
>>> -		dev_priv->display.load_luts = i9xx_load_luts;
>>> -		dev_priv->display.color_commit = hsw_color_commit;
>>> -	} else if (IS_BROADWELL(dev_priv) || IS_GEN9_BC(dev_priv) ||
>>> -		   IS_BROXTON(dev_priv)) {
>>> -		dev_priv->display.load_luts = broadwell_load_luts;
>>> -		dev_priv->display.color_commit = hsw_color_commit;
>>> -	} else if (IS_GEMINILAKE(dev_priv) || IS_CANNONLAKE(dev_priv)) {
>>> -		dev_priv->display.load_luts = glk_load_luts;
>>> -		dev_priv->display.color_commit = hsw_color_commit;
>>> +	if (HAS_GMCH_DISPLAY(dev_priv)) {
>>> +		if (IS_CHERRYVIEW(dev_priv))
>>> +			dev_priv->display.load_luts = cherryview_load_luts;
>>> +		else
>>> +			dev_priv->display.load_luts = i9xx_load_luts;
>>> +
>>> +		dev_priv->display.color_commit = i9xx_color_commit;
>>>  	} else {
>>> -		dev_priv->display.load_luts = i9xx_load_luts;
>>> +		if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
>>> +			dev_priv->display.load_luts = glk_load_luts;
>>> +		else if (INTEL_GEN(dev_priv) >= 9 || IS_BROADWELL(dev_priv))
>>> +			dev_priv->display.load_luts = broadwell_load_luts;
>>> +		else
>>> +			dev_priv->display.load_luts = i9xx_load_luts;
>>> +
>>> +		if (INTEL_GEN(dev_priv) >= 8 || IS_HASWELL(dev_priv))
>>> +			dev_priv->display.color_commit = hsw_color_commit;
>>> +		else
>>> +			dev_priv->display.color_commit = ilk_color_commit;
>>>  	}
>>>  
>>>  	/* Enable color management support when we have degamma & gamma LUTs. */
>>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>>> index ad5d39d81d6e..8e93459f312f 100644
>>> --- a/drivers/gpu/drm/i915/intel_display.c
>>> +++ b/drivers/gpu/drm/i915/intel_display.c
>>> @@ -3450,7 +3450,7 @@ static void i9xx_disable_plane(struct intel_plane *plane,
>>>  	 *
>>>  	 * On pre-g4x there is no way to gamma correct the
>>>  	 * pipe bottom color but we'll keep on doing this
>>> -	 * anyway.
>>> +	 * anyway so that the crtc state readout works correctly.
>>>  	 */
>>>  	dspcntr = i9xx_plane_ctl_crtc(crtc_state);
>>>  
>>> @@ -7674,6 +7674,8 @@ static void i9xx_set_pipeconf(const struct intel_crtc_state *crtc_state)
>>>  	     crtc_state->limited_color_range)
>>>  		pipeconf |= PIPECONF_COLOR_RANGE_SELECT;
>>>  
>>> +	pipeconf |= PIPECONF_GAMMA_MODE(crtc_state->gamma_mode);
>>> +
>>>  	I915_WRITE(PIPECONF(crtc->pipe), pipeconf);
>>>  	POSTING_READ(PIPECONF(crtc->pipe));
>>>  }
>>> @@ -8126,6 +8128,9 @@ static bool i9xx_get_pipe_config(struct intel_crtc *crtc,
>>>  	    (tmp & PIPECONF_COLOR_RANGE_SELECT))
>>>  		pipe_config->limited_color_range = true;
>>>  
>>> +	pipe_config->gamma_mode = (tmp & PIPECONF_GAMMA_MODE_MASK_I9XX) >>
>>> +		PIPECONF_GAMMA_MODE_SHIFT;
>>> +
>>>  	if (INTEL_GEN(dev_priv) < 4)
>>>  		pipe_config->double_wide = tmp & PIPECONF_DOUBLE_WIDE;
>>>  
>>> @@ -8665,6 +8670,8 @@ static void ironlake_set_pipeconf(const struct intel_crtc_state *crtc_state)
>>>  	if (crtc_state->limited_color_range)
>>>  		val |= PIPECONF_COLOR_RANGE_SELECT;
>>>  
>>> +	val |= PIPECONF_GAMMA_MODE(crtc_state->gamma_mode);
>>> +
>>>  	I915_WRITE(PIPECONF(pipe), val);
>>>  	POSTING_READ(PIPECONF(pipe));
>>>  }
>>> @@ -9199,6 +9206,9 @@ static bool ironlake_get_pipe_config(struct intel_crtc *crtc,
>>>  	if (tmp & PIPECONF_COLOR_RANGE_SELECT)
>>>  		pipe_config->limited_color_range = true;
>>>  
>>> +	pipe_config->gamma_mode = (tmp & PIPECONF_GAMMA_MODE_MASK_ILK) >>
>>> +		PIPECONF_GAMMA_MODE_SHIFT;
>>> +
>>>  	if (I915_READ(PCH_TRANSCONF(crtc->pipe)) & TRANS_ENABLE) {
>>>  		struct intel_shared_dpll *pll;
>>>  		enum intel_dpll_id pll_id;
>>> @@ -12069,6 +12079,8 @@ intel_pipe_config_compare(struct drm_i915_private *dev_priv,
>>>  
>>>  	PIPE_CONF_CHECK_BOOL(double_wide);
>>>  
>>> +	PIPE_CONF_CHECK_X(gamma_mode);
>> Hmm...  We should only check this if adjust is unset, or we will start skipping fastset on a lot of platforms.
> There's no sane macros for this sort of stuff AFAICS. Maybe someone
> should add those first? Heck, I don't even know what 'adjust'
> really means in this context.
>
> Also on a related note, this fastboot thing has made the logs rather
> confusing. It's not at all obvious that all the state mismatch prints
> are harmless (especially when the function is called something_err()).
> Some people (myself included) were already lead astray by these
> apparent "errors".

Could we change the messages then for fastset?

We already pass adjust to pipe_config_err, so we could add something like fastset cannot be done to the message.

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index ede54fdc1676..41111a17e519 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -5590,9 +5590,15 @@  enum {
 #define   PIPECONF_SINGLE_WIDE	0
 #define   PIPECONF_PIPE_UNLOCKED 0
 #define   PIPECONF_PIPE_LOCKED	(1 << 25)
-#define   PIPECONF_PALETTE	0
-#define   PIPECONF_GAMMA		(1 << 24)
 #define   PIPECONF_FORCE_BORDER	(1 << 25)
+#define   PIPECONF_GAMMA_MODE_MASK_I9XX	(1 << 24) /* gmch */
+#define   PIPECONF_GAMMA_MODE_MASK_ILK	(3 << 24) /* ilk-ivb */
+#define   PIPECONF_GAMMA_MODE_8BIT	(0 << 24) /* gmch,ilk-ivb */
+#define   PIPECONF_GAMMA_MODE_10BIT	(1 << 24) /* gmch,ilk-ivb */
+#define   PIPECONF_GAMMA_MODE_12BIT	(2 << 24) /* ilk-ivb */
+#define   PIPECONF_GAMMA_MODE_SPLIT	(3 << 24) /* ivb */
+#define   PIPECONF_GAMMA_MODE(x)	((x)<<24) /* pass in GAMMA_MODE_MODE_* */
+#define   PIPECONF_GAMMA_MODE_SHIFT	24
 #define   PIPECONF_INTERLACE_MASK	(7 << 21)
 #define   PIPECONF_INTERLACE_MASK_HSW	(3 << 21)
 /* Note that pre-gen3 does not support interlaced display directly. Panel
diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
index 2a371eed8061..e7dd07490c68 100644
--- a/drivers/gpu/drm/i915/intel_color.c
+++ b/drivers/gpu/drm/i915/intel_color.c
@@ -351,6 +351,32 @@  static void i9xx_load_luts(const struct intel_crtc_state *crtc_state)
 	i9xx_load_luts_internal(crtc_state, crtc_state->base.gamma_lut);
 }
 
+static void i9xx_color_commit(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);
+	enum pipe pipe = crtc->pipe;
+	u32 val;
+
+	val = I915_READ(PIPECONF(pipe));
+	val &= ~PIPECONF_GAMMA_MODE_MASK_I9XX;
+	val |= PIPECONF_GAMMA_MODE(crtc_state->gamma_mode);
+	I915_WRITE(PIPECONF(pipe), val);
+}
+
+static void ilk_color_commit(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);
+	enum pipe pipe = crtc->pipe;
+	u32 val;
+
+	val = I915_READ(PIPECONF(pipe));
+	val &= ~PIPECONF_GAMMA_MODE_MASK_ILK;
+	val |= PIPECONF_GAMMA_MODE(crtc_state->gamma_mode);
+	I915_WRITE(PIPECONF(pipe), val);
+}
+
 static void hsw_color_commit(const struct intel_crtc_state *crtc_state)
 {
 	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
@@ -585,8 +611,7 @@  void intel_color_commit(const struct intel_crtc_state *crtc_state)
 {
 	struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev);
 
-	if (dev_priv->display.color_commit)
-		dev_priv->display.color_commit(crtc_state);
+	dev_priv->display.color_commit(crtc_state);
 }
 
 static int check_lut_size(const struct drm_property_blob *lut, int expected)
@@ -649,20 +674,25 @@  void intel_color_init(struct intel_crtc *crtc)
 
 	drm_mode_crtc_set_gamma_size(&crtc->base, 256);
 
-	if (IS_CHERRYVIEW(dev_priv)) {
-		dev_priv->display.load_luts = cherryview_load_luts;
-	} else if (IS_HASWELL(dev_priv)) {
-		dev_priv->display.load_luts = i9xx_load_luts;
-		dev_priv->display.color_commit = hsw_color_commit;
-	} else if (IS_BROADWELL(dev_priv) || IS_GEN9_BC(dev_priv) ||
-		   IS_BROXTON(dev_priv)) {
-		dev_priv->display.load_luts = broadwell_load_luts;
-		dev_priv->display.color_commit = hsw_color_commit;
-	} else if (IS_GEMINILAKE(dev_priv) || IS_CANNONLAKE(dev_priv)) {
-		dev_priv->display.load_luts = glk_load_luts;
-		dev_priv->display.color_commit = hsw_color_commit;
+	if (HAS_GMCH_DISPLAY(dev_priv)) {
+		if (IS_CHERRYVIEW(dev_priv))
+			dev_priv->display.load_luts = cherryview_load_luts;
+		else
+			dev_priv->display.load_luts = i9xx_load_luts;
+
+		dev_priv->display.color_commit = i9xx_color_commit;
 	} else {
-		dev_priv->display.load_luts = i9xx_load_luts;
+		if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
+			dev_priv->display.load_luts = glk_load_luts;
+		else if (INTEL_GEN(dev_priv) >= 9 || IS_BROADWELL(dev_priv))
+			dev_priv->display.load_luts = broadwell_load_luts;
+		else
+			dev_priv->display.load_luts = i9xx_load_luts;
+
+		if (INTEL_GEN(dev_priv) >= 8 || IS_HASWELL(dev_priv))
+			dev_priv->display.color_commit = hsw_color_commit;
+		else
+			dev_priv->display.color_commit = ilk_color_commit;
 	}
 
 	/* Enable color management support when we have degamma & gamma LUTs. */
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index ad5d39d81d6e..8e93459f312f 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3450,7 +3450,7 @@  static void i9xx_disable_plane(struct intel_plane *plane,
 	 *
 	 * On pre-g4x there is no way to gamma correct the
 	 * pipe bottom color but we'll keep on doing this
-	 * anyway.
+	 * anyway so that the crtc state readout works correctly.
 	 */
 	dspcntr = i9xx_plane_ctl_crtc(crtc_state);
 
@@ -7674,6 +7674,8 @@  static void i9xx_set_pipeconf(const struct intel_crtc_state *crtc_state)
 	     crtc_state->limited_color_range)
 		pipeconf |= PIPECONF_COLOR_RANGE_SELECT;
 
+	pipeconf |= PIPECONF_GAMMA_MODE(crtc_state->gamma_mode);
+
 	I915_WRITE(PIPECONF(crtc->pipe), pipeconf);
 	POSTING_READ(PIPECONF(crtc->pipe));
 }
@@ -8126,6 +8128,9 @@  static bool i9xx_get_pipe_config(struct intel_crtc *crtc,
 	    (tmp & PIPECONF_COLOR_RANGE_SELECT))
 		pipe_config->limited_color_range = true;
 
+	pipe_config->gamma_mode = (tmp & PIPECONF_GAMMA_MODE_MASK_I9XX) >>
+		PIPECONF_GAMMA_MODE_SHIFT;
+
 	if (INTEL_GEN(dev_priv) < 4)
 		pipe_config->double_wide = tmp & PIPECONF_DOUBLE_WIDE;
 
@@ -8665,6 +8670,8 @@  static void ironlake_set_pipeconf(const struct intel_crtc_state *crtc_state)
 	if (crtc_state->limited_color_range)
 		val |= PIPECONF_COLOR_RANGE_SELECT;
 
+	val |= PIPECONF_GAMMA_MODE(crtc_state->gamma_mode);
+
 	I915_WRITE(PIPECONF(pipe), val);
 	POSTING_READ(PIPECONF(pipe));
 }
@@ -9199,6 +9206,9 @@  static bool ironlake_get_pipe_config(struct intel_crtc *crtc,
 	if (tmp & PIPECONF_COLOR_RANGE_SELECT)
 		pipe_config->limited_color_range = true;
 
+	pipe_config->gamma_mode = (tmp & PIPECONF_GAMMA_MODE_MASK_ILK) >>
+		PIPECONF_GAMMA_MODE_SHIFT;
+
 	if (I915_READ(PCH_TRANSCONF(crtc->pipe)) & TRANS_ENABLE) {
 		struct intel_shared_dpll *pll;
 		enum intel_dpll_id pll_id;
@@ -12069,6 +12079,8 @@  intel_pipe_config_compare(struct drm_i915_private *dev_priv,
 
 	PIPE_CONF_CHECK_BOOL(double_wide);
 
+	PIPE_CONF_CHECK_X(gamma_mode);
+
 	PIPE_CONF_CHECK_P(shared_dpll);
 	PIPE_CONF_CHECK_X(dpll_hw_state.dpll);
 	PIPE_CONF_CHECK_X(dpll_hw_state.dpll_md);

[v2,07/13] drm/i915: Populate gamma_mode for all platforms

Commit Message

Comments

Patch