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ä <ville.syrjala@xxxxxxxxxxxxxxx> > > > > 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@xxxxxxxxxxxxxxx> > > --- > > 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 Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx