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ä <[email protected]>
> > 
> > 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ä <[email protected]>
> > ---
> >  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.

> 
> 
> 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(&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 1caee4128974..90afcae91b30 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -3415,7 +3415,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);
> >  
> > @@ -7627,6 +7627,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));
> >  }
> > @@ -8077,6 +8079,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;
> >  
> > @@ -8616,6 +8621,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));
> >  }
> > @@ -9147,6 +9154,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;
> > @@ -11977,6 +11987,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);
> > -- 
> > 2.19.2
> > 
> 
> -- 
> Matt Roper
> Graphics Software Engineer
> IoTG Platform Enabling & Development
> Intel Corporation
> (916) 356-2795

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to