Re: [Intel-gfx] [PATCH 08/13] drm/i915: Populate gamma_mode for all platforms

2019-01-29 Thread Ville Syrjälä 
On Wed, Jan 16, 2019 at 09:51:01PM +0200, Ville Syrjälä wrote:
> On Wed, Jan 16, 2019 at 08:58:04PM +0200, Ville Syrjälä wrote:
> > On Wed, Jan 16, 2019 at 10:31:56AM -0800, Matt Roper wrote:
> > > On Fri, Jan 11, 2019 at 07:08:18PM +0200, Ville Syrjala wrote:
> > > > From: Ville Syrjälä 
> > > > 
> > > > 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ä 
> > > > ---
> > > >  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 44958d994bfa..9d17ba199be4 100644
> > > > --- a/drivers/gpu/drm/i915/i915_reg.h
> > > > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > > > @@ -5578,9 +5578,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_SHIFT24
> > > >  #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 0c0da7ed0fd7..6fdbfa8c4008 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);
> > > > +}
> > > 
> > > Could we just set color_commit to i9xx_set_pipeconf and
> > > ironlake_set_pipeconf to handle these without the r-m-w?
> > 
> > Perhaps. But not quite sure if we have any magic restrictions
> > in the crtc enable sequence that would prevent that.
> 
> I guess we could always keep the double set_pipeconf() in the
> crtc enable path so that we won't have to think whether to move the
> color_commit() earlier or the set_pipeconf() later.

Actually on second thought we can't avoid the RMW without more
restructuring because ironlake_set_pipeconf() & co. get called before
we enable the pipe and thus won't set the enable bit. So I think I'm
going to stick with the original patch for now.

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

Re: [Intel-gfx] [PATCH 08/13] drm/i915: Populate gamma_mode for all platforms

2019-01-16 Thread Ville Syrjälä 
On Wed, Jan 16, 2019 at 08:58:04PM +0200, Ville Syrjälä wrote:
> On Wed, Jan 16, 2019 at 10:31:56AM -0800, Matt Roper wrote:
> > On Fri, Jan 11, 2019 at 07:08:18PM +0200, Ville Syrjala wrote:
> > > From: Ville Syrjälä 
> > > 
> > > 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ä 
> > > ---
> > >  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 44958d994bfa..9d17ba199be4 100644
> > > --- a/drivers/gpu/drm/i915/i915_reg.h
> > > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > > @@ -5578,9 +5578,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 0c0da7ed0fd7..6fdbfa8c4008 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);
> > > +}
> > 
> > Could we just set color_commit to i9xx_set_pipeconf and
> > ironlake_set_pipeconf to handle these without the r-m-w?
> 
> Perhaps. But not quite sure if we have any magic restrictions
> in the crtc enable sequence that would prevent that.

I guess we could always keep the double set_pipeconf() in the
crtc enable path so that we won't have to think whether to move the
color_commit() earlier or the set_pipeconf() later.

> 
> > 
> > 
> > Matt
> > 
> > > +
> > >  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);
> > >  }
> > >  
> > >  int intel_color_check(struct intel_crtc_state *crtc_state)
> > > @@ -634,20 +659,25 @@ void intel_color_init(struct intel_crtc *crtc)
> > >  
> > >   drm_mode_crtc_set_gamma_size(>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

Re: [Intel-gfx] [PATCH 08/13] drm/i915: Populate gamma_mode for all platforms

2019-01-16 Thread Ville Syrjälä 
On Wed, Jan 16, 2019 at 10:31:56AM -0800, Matt Roper wrote:
> On Fri, Jan 11, 2019 at 07:08:18PM +0200, Ville Syrjala wrote:
> > From: Ville Syrjälä 
> > 
> > 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ä 
> > ---
> >  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 44958d994bfa..9d17ba199be4 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -5578,9 +5578,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_SHIFT24
> >  #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 0c0da7ed0fd7..6fdbfa8c4008 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);
> > +}
> 
> Could we just set color_commit to i9xx_set_pipeconf and
> ironlake_set_pipeconf to handle these without the r-m-w?

Perhaps. But not quite sure if we have any magic restrictions
in the crtc enable sequence that would prevent that.

> 
> 
> Matt
> 
> > +
> >  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);
> >  }
> >  
> >  int intel_color_check(struct intel_crtc_state *crtc_state)
> > @@ -634,20 +659,25 @@ void intel_color_init(struct intel_crtc *crtc)
> >  
> > drm_mode_crtc_set_gamma_size(>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 =

Re: [Intel-gfx] [PATCH 08/13] drm/i915: Populate gamma_mode for all platforms

2019-01-16 Thread Matt Roper 
On Fri, Jan 11, 2019 at 07:08:18PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä 
> 
> 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ä 
> ---
>  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 44958d994bfa..9d17ba199be4 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -5578,9 +5578,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 0c0da7ed0fd7..6fdbfa8c4008 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);
> +}

Could we just set color_commit to i9xx_set_pipeconf and
ironlake_set_pipeconf to handle these without the r-m-w?


Matt

> +
>  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);
>  }
>  
>  int intel_color_check(struct intel_crtc_state *crtc_state)
> @@ -634,20 +659,25 @@ void intel_color_init(struct intel_crtc *crtc)
>  
>   drm_mode_crtc_set_gamma_size(>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 =

[Intel-gfx] [PATCH 08/13] drm/i915: Populate gamma_mode for all platforms

2019-01-11 Thread Ville Syrjala 
From: Ville Syrjälä 

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ä 
---
 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 44958d994bfa..9d17ba199be4 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -5578,9 +5578,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_SHIFT24
 #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 0c0da7ed0fd7..6fdbfa8c4008 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);
 }
 
 int intel_color_check(struct intel_crtc_state *crtc_state)
@@ -634,20 +659,25 @@ void intel_color_init(struct intel_crtc *crtc)
 
drm_mode_crtc_set_gamma_size(>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))
+