Re: [PATCH v2 07/13] drm/i915: Populate gamma_mode for all platforms

Maarten Lankhorst
2019-02-07T15:49:49Z

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.

With that fixed:

Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>

~Maarten