From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH v2 07/13] drm/i915: Populate gamma_mode for all platforms
Date: Thu, 7 Feb 2019 19:27:47 +0200 [thread overview]
Message-ID: <20190207172747.GL20097@intel.com> (raw)
In-Reply-To: <20190207162719.GI20097@intel.com>
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
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
next prev parent reply other threads:[~2019-02-07 17:27 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-02-05 16:08 [PATCH v2 00/13] Enable/disable gamma/csc dynamically and fix C8 Ville Syrjala
2019-02-05 16:08 ` [PATCH v2 01/13] drm/i915: Split the gamma/csc enable bits from the plane_ctl() function Ville Syrjala
2019-02-05 16:08 ` [PATCH v2 02/13] drm/i915: Precompute gamma_mode Ville Syrjala
2019-02-05 16:08 ` [PATCH v2 03/13] drm/i915: Constify the state arguments to the color management stuff Ville Syrjala
2019-02-05 16:08 ` [PATCH v2 04/13] drm/i915: Pull GAMMA_MODE write out from haswell_load_luts() Ville Syrjala
2019-02-05 16:08 ` [PATCH v2 05/13] drm/i915: Split color mgmt based on single vs. double buffered registers Ville Syrjala
2019-02-05 16:08 ` [PATCH v2 06/13] drm/i915: Move LUT programming to happen after vblank waits Ville Syrjala
2019-02-07 15:46 ` Maarten Lankhorst
2019-02-07 19:24 ` Ville Syrjälä
2019-02-05 16:08 ` [PATCH v2 07/13] drm/i915: Populate gamma_mode for all platforms Ville Syrjala
2019-02-07 15:49 ` Maarten Lankhorst
2019-02-07 16:27 ` Ville Syrjälä
2019-02-07 17:27 ` Ville Syrjälä [this message]
2019-02-08 8:51 ` Maarten Lankhorst
2019-02-05 16:08 ` [PATCH v2 08/13] drm/i915: Track pipe gamma enable/disable in crtc state Ville Syrjala
2019-02-05 16:08 ` [PATCH v2 09/13] drm/i915: Track pipe csc enable " Ville Syrjala
2019-02-05 16:08 ` [PATCH v2 10/13] drm/i915: Turn off pipe gamma when it's not needed Ville Syrjala
2019-02-05 16:08 ` [PATCH v2 11/13] drm/i915: Turn off pipe CSC " Ville Syrjala
2019-02-05 16:08 ` [PATCH v2 12/13] drm/i915: Disable pipe gamma when C8 pixel format is used Ville Syrjala
2019-02-05 16:08 ` [PATCH v2 13/13] drm/i915: Update DSPCNTR gamma/csc bits during crtc_enable() Ville Syrjala
2019-02-07 15:58 ` Maarten Lankhorst
2019-02-07 16:29 ` Ville Syrjälä
2019-02-05 16:57 ` ✗ Fi.CI.CHECKPATCH: warning for Enable/disable gamma/csc dynamically and fix C8 (rev2) Patchwork
2019-02-05 17:02 ` ✗ Fi.CI.SPARSE: " Patchwork
2019-02-05 17:24 ` ✓ Fi.CI.BAT: success " Patchwork
2019-02-05 21:19 ` ✓ Fi.CI.IGT: " Patchwork
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20190207172747.GL20097@intel.com \
--to=ville.syrjala@linux.intel.com \
--cc=intel-gfx@lists.freedesktop.org \
--cc=maarten.lankhorst@linux.intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox