From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Matt Roper <matthew.d.roper@intel.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH 08/13] drm/i915: Populate gamma_mode for all platforms
Date: Tue, 29 Jan 2019 17:59:03 +0200 [thread overview]
Message-ID: <20190129155903.GC20097@intel.com> (raw)
In-Reply-To: <20190116195101.GW20097@intel.com>
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ä <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 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.
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 (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@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> 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-01-29 15:59 UTC|newest]
Thread overview: 52+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-01-11 17:08 [PATCH 00/13] Enable/disable gamma/csc dynamically and fix C8 Ville Syrjala
2019-01-11 17:08 ` [PATCH 01/13] drm/i915: Clean up intel_plane_atomic_check_with_state() Ville Syrjala
2019-01-12 0:41 ` Matt Roper
2019-01-16 16:08 ` Shankar, Uma
2019-01-11 17:08 ` [PATCH 02/13] drm/i915: Split the gamma/csc enable bits from the plane_ctl() function Ville Syrjala
2019-01-12 0:41 ` Matt Roper
2019-01-14 17:11 ` Ville Syrjälä
2019-01-14 19:11 ` Ville Syrjälä
2019-01-16 17:11 ` Shankar, Uma
2019-01-17 16:34 ` Ville Syrjälä
2019-01-11 17:08 ` [PATCH 03/13] drm/i915: Precompute gamma_mode Ville Syrjala
2019-01-12 0:41 ` Matt Roper
2019-01-16 17:18 ` Shankar, Uma
2019-01-11 17:08 ` [PATCH 04/13] drm/i915: Constify the state arguments to the color management stuff Ville Syrjala
2019-01-12 0:42 ` Matt Roper
2019-01-16 17:21 ` Shankar, Uma
2019-01-11 17:08 ` [PATCH 05/13] drm/i915: Pull GAMMA_MODE write out from haswell_load_luts() Ville Syrjala
2019-01-12 0:57 ` Matt Roper
2019-01-16 17:26 ` Shankar, Uma
2019-01-11 17:08 ` [PATCH 06/13] drm/i915: Split color mgmt based on single vs. double buffered registers Ville Syrjala
2019-01-15 0:56 ` Matt Roper
2019-01-16 18:22 ` Shankar, Uma
2019-01-11 17:08 ` [PATCH 07/13] drm/i915: Move LUT programming to happen after vblank waits Ville Syrjala
2019-01-16 17:38 ` Matt Roper
2019-01-16 18:02 ` Ville Syrjälä
2019-01-17 15:00 ` Ville Syrjälä
2019-01-11 17:08 ` [PATCH 08/13] drm/i915: Populate gamma_mode for all platforms Ville Syrjala
2019-01-16 18:31 ` Matt Roper
2019-01-16 18:58 ` Ville Syrjälä
2019-01-16 19:51 ` Ville Syrjälä
2019-01-29 15:59 ` Ville Syrjälä [this message]
2019-01-11 17:08 ` [PATCH 09/13] drm/i915: Track pipe gamma enable/disable in crtc state Ville Syrjala
2019-01-16 19:36 ` Matt Roper
2019-01-17 5:14 ` Shankar, Uma
2019-01-17 14:57 ` Ville Syrjälä
2019-01-11 17:08 ` [PATCH 10/13] drm/i915: Track pipe csc enable " Ville Syrjala
2019-01-16 19:43 ` Matt Roper
2019-01-17 5:17 ` Shankar, Uma
2019-01-11 17:08 ` [PATCH 11/13] drm/i915: Turn off pipe gamma when it's not needed Ville Syrjala
2019-01-17 5:32 ` Shankar, Uma
2019-01-17 18:40 ` Matt Roper
2019-01-17 18:48 ` Ville Syrjälä
2019-01-11 17:08 ` [PATCH 12/13] drm/i915: Turn off pipe CSC " Ville Syrjala
2019-01-17 5:37 ` Shankar, Uma
2019-01-17 18:54 ` Matt Roper
2019-01-11 17:08 ` [PATCH 13/13] drm/i915: Disable pipe gamma when C8 pixel format is used Ville Syrjala
2019-01-17 5:58 ` Shankar, Uma
2019-01-17 19:13 ` Matt Roper
2019-01-17 19:27 ` Ville Syrjälä
2019-01-11 17:25 ` ✗ Fi.CI.CHECKPATCH: warning for Enable/disable gamma/csc dynamically and fix C8 Patchwork
2019-01-11 17:44 ` ✓ Fi.CI.BAT: success " Patchwork
2019-01-11 22:03 ` ✓ 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=20190129155903.GC20097@intel.com \
--to=ville.syrjala@linux.intel.com \
--cc=intel-gfx@lists.freedesktop.org \
--cc=matthew.d.roper@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.