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@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 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@xxxxxxxxxxxxxxx>
>
> ~Maarten
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
