| Message ID | 20191024122138.25065-3-ville.syrjala@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/3] drm/i915: Use _PICK() for CHICKEN_TRANS() | expand |
>-----Original Message----- >From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of Ville Syrjala >Sent: Thursday, October 24, 2019 5:52 PM >To: intel-gfx@lists.freedesktop.org >Subject: [Intel-gfx] [PATCH 3/3] drm/i915: Fix frame start delay programming > >From: Ville Syrjälä <ville.syrjala@linux.intel.com> > >Currently we're blindly poking at the frame start delay bits in PIPECONF when trying to >sanitize the hardware state. Those bits decided to move elsewhere on HSW, so on >many platforms we're not doing anything at all here. Also we're forgetting about the >PCH transcoder entirely. > >Add all the bit definitions for the various homes these bits have had throughout the >years, and reset them all to zero. > >However I'm not entirely sure this is a safe thing to do. If not I guess we'd want full >readout+statecheck for this stuff. >For now let's stick to the current logic and hope for the best. > >Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> >--- > drivers/gpu/drm/i915/display/intel_display.c | 101 ++++++++++++++++--- > drivers/gpu/drm/i915/i915_reg.h | 12 ++- > drivers/gpu/drm/i915/intel_pm.c | 1 - > 3 files changed, 95 insertions(+), 19 deletions(-) > >diff --git a/drivers/gpu/drm/i915/display/intel_display.c >b/drivers/gpu/drm/i915/display/intel_display.c >index 579655675b08..2896cf864f61 100644 >--- a/drivers/gpu/drm/i915/display/intel_display.c >+++ b/drivers/gpu/drm/i915/display/intel_display.c >@@ -1645,11 +1645,16 @@ static void ironlake_enable_pch_transcoder(const struct >intel_crtc_state *crtc_s > assert_fdi_rx_enabled(dev_priv, pipe); > > if (HAS_PCH_CPT(dev_priv)) { >- /* Workaround: Set the timing override bit before enabling the >- * pch transcoder. */ > reg = TRANS_CHICKEN2(pipe); > val = I915_READ(reg); >+ /* >+ * Workaround: Set the timing override bit >+ * before enabling the pch transcoder. >+ */ > val |= TRANS_CHICKEN2_TIMING_OVERRIDE; >+ /* Configure frame start delay to match the CPU */ >+ val &= ~TRANS_CHICKEN2_FRAME_START_DELAY_MASK; >+ val |= TRANS_CHICKEN2_FRAME_START_DELAY(0); > I915_WRITE(reg, val); > } > >@@ -1658,6 +1663,10 @@ static void ironlake_enable_pch_transcoder(const struct >intel_crtc_state *crtc_s > pipeconf_val = I915_READ(PIPECONF(pipe)); > > if (HAS_PCH_IBX(dev_priv)) { >+ /* Configure frame start delay to match the CPU */ >+ val &= ~TRANS_FRAME_START_DELAY_MASK; >+ val |= TRANS_FRAME_START_DELAY(0); >+ > /* > * Make the BPC in transcoder be consistent with > * that in pipeconf reg. For HDMI we must use 8bpc @@ -1695,9 >+1704,12 @@ static void lpt_enable_pch_transcoder(struct drm_i915_private >*dev_priv, > assert_fdi_tx_enabled(dev_priv, (enum pipe) cpu_transcoder); > assert_fdi_rx_enabled(dev_priv, PIPE_A); > >+ val = I915_READ(TRANS_CHICKEN2(PIPE_A)); > /* Workaround: set timing override bit. */ >- val = I915_READ(TRANS_CHICKEN2(PIPE_A)); > val |= TRANS_CHICKEN2_TIMING_OVERRIDE; >+ /* Configure frame start delay to match the CPU */ >+ val &= ~TRANS_CHICKEN2_FRAME_START_DELAY_MASK; >+ val |= TRANS_CHICKEN2_FRAME_START_DELAY(0); > I915_WRITE(TRANS_CHICKEN2(PIPE_A), val); > > val = TRANS_ENABLE; >@@ -6452,6 +6464,19 @@ static void icl_pipe_mbus_enable(struct intel_crtc *crtc) > I915_WRITE(PIPE_MBUS_DBOX_CTL(pipe), val); } > >+static void hsw_set_frame_start_delay(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); >+ i915_reg_t reg = CHICKEN_TRANS(crtc_state->cpu_transcoder); >+ u32 val; >+ >+ val = I915_READ(reg); >+ val &= ~HSW_FRAME_START_DELAY_MASK; >+ val |= HSW_FRAME_START_DELAY(0); >+ I915_WRITE(reg, val); >+} >+ > static void haswell_crtc_enable(struct intel_crtc_state *pipe_config, > struct intel_atomic_state *state) > { >@@ -6494,8 +6519,10 @@ static void haswell_crtc_enable(struct intel_crtc_state >*pipe_config, > &pipe_config->fdi_m_n, NULL); > } > >- if (!transcoder_is_dsi(cpu_transcoder)) >+ if (!transcoder_is_dsi(cpu_transcoder)) { >+ hsw_set_frame_start_delay(pipe_config); > haswell_set_pipeconf(pipe_config); >+ } > > if (INTEL_GEN(dev_priv) >= 9 || IS_BROADWELL(dev_priv)) > bdw_set_pipemisc(pipe_config); >@@ -8394,6 +8421,8 @@ static void i9xx_set_pipeconf(const struct intel_crtc_state >*crtc_state) > > pipeconf |= PIPECONF_GAMMA_MODE(crtc_state->gamma_mode); > >+ pipeconf |= PIPECONF_FRAME_START_DELAY(0); >+ > I915_WRITE(PIPECONF(crtc->pipe), pipeconf); > POSTING_READ(PIPECONF(crtc->pipe)); > } >@@ -9474,6 +9503,8 @@ static void ironlake_set_pipeconf(const struct >intel_crtc_state *crtc_state) > > val |= PIPECONF_GAMMA_MODE(crtc_state->gamma_mode); > >+ val |= PIPECONF_FRAME_START_DELAY(0); >+ > I915_WRITE(PIPECONF(pipe), val); > POSTING_READ(PIPECONF(pipe)); > } >@@ -16919,25 +16950,69 @@ static bool has_pch_trancoder(struct >drm_i915_private *dev_priv, > (HAS_PCH_LPT_H(dev_priv) && pch_transcoder == PIPE_A); } > >+static void intel_sanitize_frame_start_delay(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 transcoder cpu_transcoder = crtc_state->cpu_transcoder; >+ >+ if (INTEL_GEN(dev_priv) >= 9 || >+ IS_BROADWELL(dev_priv) || IS_HASWELL(dev_priv)) { >+ i915_reg_t reg = CHICKEN_TRANS(cpu_transcoder); >+ u32 val; >+ >+ if (transcoder_is_dsi(cpu_transcoder)) >+ return; >+ >+ val = I915_READ(reg); >+ val &= ~HSW_FRAME_START_DELAY_MASK; >+ val |= HSW_FRAME_START_DELAY(0); >+ I915_WRITE(reg, val); >+ } else { >+ i915_reg_t reg = PIPECONF(cpu_transcoder); >+ u32 val; >+ >+ val = I915_READ(reg); >+ val &= ~PIPECONF_FRAME_START_DELAY_MASK; >+ val |= PIPECONF_FRAME_START_DELAY(0); >+ I915_WRITE(reg, val); >+ } >+ >+ if (!crtc_state->has_pch_encoder) >+ return; >+ >+ if (HAS_PCH_IBX(dev_priv)) { >+ i915_reg_t reg = PCH_TRANSCONF(crtc->pipe); >+ u32 val; >+ >+ val = I915_READ(reg); >+ val &= ~TRANS_FRAME_START_DELAY_MASK; >+ val |= TRANS_FRAME_START_DELAY(0); >+ I915_WRITE(reg, val); >+ } else { >+ i915_reg_t reg = TRANS_CHICKEN2(crtc->pipe); >+ u32 val; >+ >+ val = I915_READ(reg); >+ val &= ~TRANS_CHICKEN2_FRAME_START_DELAY_MASK; >+ val |= TRANS_CHICKEN2_FRAME_START_DELAY(0); >+ I915_WRITE(reg, val); >+ } >+} >+ > static void intel_sanitize_crtc(struct intel_crtc *crtc, > struct drm_modeset_acquire_ctx *ctx) { > struct drm_device *dev = crtc->base.dev; > struct drm_i915_private *dev_priv = to_i915(dev); > struct intel_crtc_state *crtc_state = to_intel_crtc_state(crtc->base.state); >- enum transcoder cpu_transcoder = crtc_state->cpu_transcoder; >- >- /* Clear any frame start delays used for debugging left by the BIOS */ >- if (crtc->active && !transcoder_is_dsi(cpu_transcoder)) { >- i915_reg_t reg = PIPECONF(cpu_transcoder); >- >- I915_WRITE(reg, >- I915_READ(reg) & >~PIPECONF_FRAME_START_DELAY_MASK); >- } > > if (crtc_state->base.active) { > struct intel_plane *plane; > >+ /* Clear any frame start delays used for debugging left by the BIOS */ >+ intel_sanitize_frame_start_delay(crtc_state); >+ > /* Disable everything but the primary plane */ > for_each_intel_plane_on_crtc(dev, crtc, plane) { > const struct intel_plane_state *plane_state = diff --git >a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index >50c2fa0f2cab..cb2e0f4679c4 100644 >--- a/drivers/gpu/drm/i915/i915_reg.h >+++ b/drivers/gpu/drm/i915/i915_reg.h >@@ -5671,7 +5671,8 @@ enum { > #define PIPECONF_DOUBLE_WIDE (1 << 30) > #define I965_PIPECONF_ACTIVE (1 << 30) > #define PIPECONF_DSI_PLL_LOCKED (1 << 29) /* vlv & pipe A only */ >-#define PIPECONF_FRAME_START_DELAY_MASK (3 << 27) >+#define PIPECONF_FRAME_START_DELAY_MASK (3 << 27) /* pre-hsw */ >+#define PIPECONF_FRAME_START_DELAY(x) ((x) << 27) /* pre-hsw: 0-3 >*/ > #define PIPECONF_SINGLE_WIDE 0 > #define PIPECONF_PIPE_UNLOCKED 0 > #define PIPECONF_PIPE_LOCKED (1 << 25) >@@ -7627,6 +7628,8 @@ enum { > [TRANSCODER_B] = _CHICKEN_TRANS_B, >\ > [TRANSCODER_C] = _CHICKEN_TRANS_C, >\ > [TRANSCODER_D] = >_CHICKEN_TRANS_D)) >+#define HSW_FRAME_START_DELAY_MASK (3 << 27) >+#define HSW_FRAME_START_DELAY(x) ((x) << 27) /* 0-3 */ > #define VSC_DATA_SEL_SOFTWARE_CONTROL (1 << 25) /* GLK and CNL+ */ > #define DDI_TRAINING_OVERRIDE_ENABLE (1 << 19) > #define DDI_TRAINING_OVERRIDE_VALUE (1 << 18) >@@ -8341,10 +8344,8 @@ enum { > #define TRANS_STATE_MASK (1 << 30) > #define TRANS_STATE_DISABLE (0 << 30) > #define TRANS_STATE_ENABLE (1 << 30) >-#define TRANS_FSYNC_DELAY_HB1 (0 << 27) -#define TRANS_FSYNC_DELAY_HB2 >(1 << 27) -#define TRANS_FSYNC_DELAY_HB3 (2 << 27) -#define >TRANS_FSYNC_DELAY_HB4 (3 << 27) >+#define TRANS_FRAME_START_DELAY_MASK (3 << 27) /* ibx */ >+#define TRANS_FRAME_START_DELAY(x) ((x) << 27) /* ibx: 0-3 */ > #define TRANS_INTERLACE_MASK (7 << 21) > #define TRANS_PROGRESSIVE (0 << 21) > #define TRANS_INTERLACED (3 << 21) >@@ -8365,6 +8366,7 @@ enum { > #define TRANS_CHICKEN2_TIMING_OVERRIDE (1 << 31) > #define TRANS_CHICKEN2_FDI_POLARITY_REVERSED (1 << 29) > #define TRANS_CHICKEN2_FRAME_START_DELAY_MASK (3 << 27) >+#define TRANS_CHICKEN2_FRAME_START_DELAY(x) ((x) << 27) /* 0-3 */ > #define TRANS_CHICKEN2_DISABLE_DEEP_COLOR_COUNTER (1 << 26) > #define TRANS_CHICKEN2_DISABLE_DEEP_COLOR_MODESWITCH (1 << 25) > >diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index >362234449087..8b1fbdb36537 100644 >--- a/drivers/gpu/drm/i915/intel_pm.c >+++ b/drivers/gpu/drm/i915/intel_pm.c >@@ -8079,7 +8079,6 @@ static void cpt_init_clock_gating(struct drm_i915_private >*dev_priv) > val &= ~TRANS_CHICKEN2_FDI_POLARITY_REVERSED; > if (dev_priv->vbt.fdi_rx_polarity_inverted) > val |= TRANS_CHICKEN2_FDI_POLARITY_REVERSED; >- val &= ~TRANS_CHICKEN2_FRAME_START_DELAY_MASK; Is there any reason not to let it be set at 0. Overall, I looked at the register and bit definitions for various platforms and this looks a very reasonable change rectifying the frame start delay programming. However I am not sure if for all platforms 0 will be the preferred value. If the default values of these bits are 0 in hardware register (or the BIOS set them at 0 itself), this should work seamlessly. Only caveat will be if the defaults are not 0 on all platforms, we may have issues. Good way to figure this out I guess is our CI results. So if the CI passes, this is Reviewed-by: Uma Shankar <uma.shankar@intel.com> > val &= ~TRANS_CHICKEN2_DISABLE_DEEP_COLOR_COUNTER; > val &= ~TRANS_CHICKEN2_DISABLE_DEEP_COLOR_MODESWITCH; > I915_WRITE(TRANS_CHICKEN2(pipe), val); >-- >2.21.0 > >_______________________________________________ >Intel-gfx mailing list >Intel-gfx@lists.freedesktop.org >https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Fri, Nov 15, 2019 at 04:08:45PM +0000, Shankar, Uma wrote: > > > >-----Original Message----- > >From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of Ville Syrjala > >Sent: Thursday, October 24, 2019 5:52 PM > >To: intel-gfx@lists.freedesktop.org > >Subject: [Intel-gfx] [PATCH 3/3] drm/i915: Fix frame start delay programming > > > >From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > >Currently we're blindly poking at the frame start delay bits in PIPECONF when trying to > >sanitize the hardware state. Those bits decided to move elsewhere on HSW, so on > >many platforms we're not doing anything at all here. Also we're forgetting about the > >PCH transcoder entirely. > > > >Add all the bit definitions for the various homes these bits have had throughout the > >years, and reset them all to zero. > > > >However I'm not entirely sure this is a safe thing to do. If not I guess we'd want full > >readout+statecheck for this stuff. > >For now let's stick to the current logic and hope for the best. > > > >Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > >--- > > drivers/gpu/drm/i915/display/intel_display.c | 101 ++++++++++++++++--- > > drivers/gpu/drm/i915/i915_reg.h | 12 ++- > > drivers/gpu/drm/i915/intel_pm.c | 1 - > > 3 files changed, 95 insertions(+), 19 deletions(-) > > > >diff --git a/drivers/gpu/drm/i915/display/intel_display.c > >b/drivers/gpu/drm/i915/display/intel_display.c > >index 579655675b08..2896cf864f61 100644 > >--- a/drivers/gpu/drm/i915/display/intel_display.c > >+++ b/drivers/gpu/drm/i915/display/intel_display.c > >@@ -1645,11 +1645,16 @@ static void ironlake_enable_pch_transcoder(const struct > >intel_crtc_state *crtc_s > > assert_fdi_rx_enabled(dev_priv, pipe); > > > > if (HAS_PCH_CPT(dev_priv)) { > >- /* Workaround: Set the timing override bit before enabling the > >- * pch transcoder. */ > > reg = TRANS_CHICKEN2(pipe); > > val = I915_READ(reg); > >+ /* > >+ * Workaround: Set the timing override bit > >+ * before enabling the pch transcoder. > >+ */ > > val |= TRANS_CHICKEN2_TIMING_OVERRIDE; > >+ /* Configure frame start delay to match the CPU */ > >+ val &= ~TRANS_CHICKEN2_FRAME_START_DELAY_MASK; > >+ val |= TRANS_CHICKEN2_FRAME_START_DELAY(0); > > I915_WRITE(reg, val); > > } > > > >@@ -1658,6 +1663,10 @@ static void ironlake_enable_pch_transcoder(const struct > >intel_crtc_state *crtc_s > > pipeconf_val = I915_READ(PIPECONF(pipe)); > > > > if (HAS_PCH_IBX(dev_priv)) { > >+ /* Configure frame start delay to match the CPU */ > >+ val &= ~TRANS_FRAME_START_DELAY_MASK; > >+ val |= TRANS_FRAME_START_DELAY(0); > >+ > > /* > > * Make the BPC in transcoder be consistent with > > * that in pipeconf reg. For HDMI we must use 8bpc @@ -1695,9 > >+1704,12 @@ static void lpt_enable_pch_transcoder(struct drm_i915_private > >*dev_priv, > > assert_fdi_tx_enabled(dev_priv, (enum pipe) cpu_transcoder); > > assert_fdi_rx_enabled(dev_priv, PIPE_A); > > > >+ val = I915_READ(TRANS_CHICKEN2(PIPE_A)); > > /* Workaround: set timing override bit. */ > >- val = I915_READ(TRANS_CHICKEN2(PIPE_A)); > > val |= TRANS_CHICKEN2_TIMING_OVERRIDE; > >+ /* Configure frame start delay to match the CPU */ > >+ val &= ~TRANS_CHICKEN2_FRAME_START_DELAY_MASK; > >+ val |= TRANS_CHICKEN2_FRAME_START_DELAY(0); > > I915_WRITE(TRANS_CHICKEN2(PIPE_A), val); > > > > val = TRANS_ENABLE; > >@@ -6452,6 +6464,19 @@ static void icl_pipe_mbus_enable(struct intel_crtc *crtc) > > I915_WRITE(PIPE_MBUS_DBOX_CTL(pipe), val); } > > > >+static void hsw_set_frame_start_delay(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); > >+ i915_reg_t reg = CHICKEN_TRANS(crtc_state->cpu_transcoder); > >+ u32 val; > >+ > >+ val = I915_READ(reg); > >+ val &= ~HSW_FRAME_START_DELAY_MASK; > >+ val |= HSW_FRAME_START_DELAY(0); > >+ I915_WRITE(reg, val); > >+} > >+ > > static void haswell_crtc_enable(struct intel_crtc_state *pipe_config, > > struct intel_atomic_state *state) > > { > >@@ -6494,8 +6519,10 @@ static void haswell_crtc_enable(struct intel_crtc_state > >*pipe_config, > > &pipe_config->fdi_m_n, NULL); > > } > > > >- if (!transcoder_is_dsi(cpu_transcoder)) > >+ if (!transcoder_is_dsi(cpu_transcoder)) { > >+ hsw_set_frame_start_delay(pipe_config); > > haswell_set_pipeconf(pipe_config); > >+ } > > > > if (INTEL_GEN(dev_priv) >= 9 || IS_BROADWELL(dev_priv)) > > bdw_set_pipemisc(pipe_config); > >@@ -8394,6 +8421,8 @@ static void i9xx_set_pipeconf(const struct intel_crtc_state > >*crtc_state) > > > > pipeconf |= PIPECONF_GAMMA_MODE(crtc_state->gamma_mode); > > > >+ pipeconf |= PIPECONF_FRAME_START_DELAY(0); > >+ > > I915_WRITE(PIPECONF(crtc->pipe), pipeconf); > > POSTING_READ(PIPECONF(crtc->pipe)); > > } > >@@ -9474,6 +9503,8 @@ static void ironlake_set_pipeconf(const struct > >intel_crtc_state *crtc_state) > > > > val |= PIPECONF_GAMMA_MODE(crtc_state->gamma_mode); > > > >+ val |= PIPECONF_FRAME_START_DELAY(0); > >+ > > I915_WRITE(PIPECONF(pipe), val); > > POSTING_READ(PIPECONF(pipe)); > > } > >@@ -16919,25 +16950,69 @@ static bool has_pch_trancoder(struct > >drm_i915_private *dev_priv, > > (HAS_PCH_LPT_H(dev_priv) && pch_transcoder == PIPE_A); } > > > >+static void intel_sanitize_frame_start_delay(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 transcoder cpu_transcoder = crtc_state->cpu_transcoder; > >+ > >+ if (INTEL_GEN(dev_priv) >= 9 || > >+ IS_BROADWELL(dev_priv) || IS_HASWELL(dev_priv)) { > >+ i915_reg_t reg = CHICKEN_TRANS(cpu_transcoder); > >+ u32 val; > >+ > >+ if (transcoder_is_dsi(cpu_transcoder)) > >+ return; > >+ > >+ val = I915_READ(reg); > >+ val &= ~HSW_FRAME_START_DELAY_MASK; > >+ val |= HSW_FRAME_START_DELAY(0); > >+ I915_WRITE(reg, val); > >+ } else { > >+ i915_reg_t reg = PIPECONF(cpu_transcoder); > >+ u32 val; > >+ > >+ val = I915_READ(reg); > >+ val &= ~PIPECONF_FRAME_START_DELAY_MASK; > >+ val |= PIPECONF_FRAME_START_DELAY(0); > >+ I915_WRITE(reg, val); > >+ } > >+ > >+ if (!crtc_state->has_pch_encoder) > >+ return; > >+ > >+ if (HAS_PCH_IBX(dev_priv)) { > >+ i915_reg_t reg = PCH_TRANSCONF(crtc->pipe); > >+ u32 val; > >+ > >+ val = I915_READ(reg); > >+ val &= ~TRANS_FRAME_START_DELAY_MASK; > >+ val |= TRANS_FRAME_START_DELAY(0); > >+ I915_WRITE(reg, val); > >+ } else { > >+ i915_reg_t reg = TRANS_CHICKEN2(crtc->pipe); > >+ u32 val; > >+ > >+ val = I915_READ(reg); > >+ val &= ~TRANS_CHICKEN2_FRAME_START_DELAY_MASK; > >+ val |= TRANS_CHICKEN2_FRAME_START_DELAY(0); > >+ I915_WRITE(reg, val); > >+ } > >+} > >+ > > static void intel_sanitize_crtc(struct intel_crtc *crtc, > > struct drm_modeset_acquire_ctx *ctx) { > > struct drm_device *dev = crtc->base.dev; > > struct drm_i915_private *dev_priv = to_i915(dev); > > struct intel_crtc_state *crtc_state = to_intel_crtc_state(crtc->base.state); > >- enum transcoder cpu_transcoder = crtc_state->cpu_transcoder; > >- > >- /* Clear any frame start delays used for debugging left by the BIOS */ > >- if (crtc->active && !transcoder_is_dsi(cpu_transcoder)) { > >- i915_reg_t reg = PIPECONF(cpu_transcoder); > >- > >- I915_WRITE(reg, > >- I915_READ(reg) & > >~PIPECONF_FRAME_START_DELAY_MASK); > >- } > > > > if (crtc_state->base.active) { > > struct intel_plane *plane; > > > >+ /* Clear any frame start delays used for debugging left by the BIOS */ > >+ intel_sanitize_frame_start_delay(crtc_state); > >+ > > /* Disable everything but the primary plane */ > > for_each_intel_plane_on_crtc(dev, crtc, plane) { > > const struct intel_plane_state *plane_state = diff --git > >a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index > >50c2fa0f2cab..cb2e0f4679c4 100644 > >--- a/drivers/gpu/drm/i915/i915_reg.h > >+++ b/drivers/gpu/drm/i915/i915_reg.h > >@@ -5671,7 +5671,8 @@ enum { > > #define PIPECONF_DOUBLE_WIDE (1 << 30) > > #define I965_PIPECONF_ACTIVE (1 << 30) > > #define PIPECONF_DSI_PLL_LOCKED (1 << 29) /* vlv & pipe A only */ > >-#define PIPECONF_FRAME_START_DELAY_MASK (3 << 27) > >+#define PIPECONF_FRAME_START_DELAY_MASK (3 << 27) /* pre-hsw */ > >+#define PIPECONF_FRAME_START_DELAY(x) ((x) << 27) /* pre-hsw: 0-3 > >*/ > > #define PIPECONF_SINGLE_WIDE 0 > > #define PIPECONF_PIPE_UNLOCKED 0 > > #define PIPECONF_PIPE_LOCKED (1 << 25) > >@@ -7627,6 +7628,8 @@ enum { > > [TRANSCODER_B] = _CHICKEN_TRANS_B, > >\ > > [TRANSCODER_C] = _CHICKEN_TRANS_C, > >\ > > [TRANSCODER_D] = > >_CHICKEN_TRANS_D)) > >+#define HSW_FRAME_START_DELAY_MASK (3 << 27) > >+#define HSW_FRAME_START_DELAY(x) ((x) << 27) /* 0-3 */ > > #define VSC_DATA_SEL_SOFTWARE_CONTROL (1 << 25) /* GLK and CNL+ */ > > #define DDI_TRAINING_OVERRIDE_ENABLE (1 << 19) > > #define DDI_TRAINING_OVERRIDE_VALUE (1 << 18) > >@@ -8341,10 +8344,8 @@ enum { > > #define TRANS_STATE_MASK (1 << 30) > > #define TRANS_STATE_DISABLE (0 << 30) > > #define TRANS_STATE_ENABLE (1 << 30) > >-#define TRANS_FSYNC_DELAY_HB1 (0 << 27) -#define TRANS_FSYNC_DELAY_HB2 > >(1 << 27) -#define TRANS_FSYNC_DELAY_HB3 (2 << 27) -#define > >TRANS_FSYNC_DELAY_HB4 (3 << 27) > >+#define TRANS_FRAME_START_DELAY_MASK (3 << 27) /* ibx */ > >+#define TRANS_FRAME_START_DELAY(x) ((x) << 27) /* ibx: 0-3 */ > > #define TRANS_INTERLACE_MASK (7 << 21) > > #define TRANS_PROGRESSIVE (0 << 21) > > #define TRANS_INTERLACED (3 << 21) > >@@ -8365,6 +8366,7 @@ enum { > > #define TRANS_CHICKEN2_TIMING_OVERRIDE (1 << 31) > > #define TRANS_CHICKEN2_FDI_POLARITY_REVERSED (1 << 29) > > #define TRANS_CHICKEN2_FRAME_START_DELAY_MASK (3 << 27) > >+#define TRANS_CHICKEN2_FRAME_START_DELAY(x) ((x) << 27) /* 0-3 */ > > #define TRANS_CHICKEN2_DISABLE_DEEP_COLOR_COUNTER (1 << 26) > > #define TRANS_CHICKEN2_DISABLE_DEEP_COLOR_MODESWITCH (1 << 25) > > > >diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index > >362234449087..8b1fbdb36537 100644 > >--- a/drivers/gpu/drm/i915/intel_pm.c > >+++ b/drivers/gpu/drm/i915/intel_pm.c > >@@ -8079,7 +8079,6 @@ static void cpt_init_clock_gating(struct drm_i915_private > >*dev_priv) > > val &= ~TRANS_CHICKEN2_FDI_POLARITY_REVERSED; > > if (dev_priv->vbt.fdi_rx_polarity_inverted) > > val |= TRANS_CHICKEN2_FDI_POLARITY_REVERSED; > >- val &= ~TRANS_CHICKEN2_FRAME_START_DELAY_MASK; > > Is there any reason not to let it be set at 0. > > Overall, I looked at the register and bit definitions for various platforms and this looks > a very reasonable change rectifying the frame start delay programming. > > However I am not sure if for all platforms 0 will be the preferred value. If the default values > of these bits are 0 in hardware register (or the BIOS set them at 0 itself), this should > work seamlessly. Only caveat will be if the defaults are not 0 on all platforms, we may have > issues. Default is 0, but supposedly some VBIOSen have left other values in there for whatever reason. Or else we probably wouldn't have this code to sanitize things. I *may* want to try bumping this to 3 across the board to give gamma programming more breathing room during the vblank, but I'm not sure that's entirely safe to do. Hence I started with just cleaning up the current mess. IIRC there was also some obnoxious workaround on LVDS+IBX (or maybe it was CPT) that would require some extra handling if we were to program this to some non-zero value. > > Good way to figure this out I guess is our CI results. So if the CI passes, this is > Reviewed-by: Uma Shankar <uma.shankar@intel.com> Ta. > > > val &= ~TRANS_CHICKEN2_DISABLE_DEEP_COLOR_COUNTER; > > val &= ~TRANS_CHICKEN2_DISABLE_DEEP_COLOR_MODESWITCH; > > I915_WRITE(TRANS_CHICKEN2(pipe), val); > >-- > >2.21.0 > > > >_______________________________________________ > >Intel-gfx mailing list > >Intel-gfx@lists.freedesktop.org > >https://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index 579655675b08..2896cf864f61 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -1645,11 +1645,16 @@ static void ironlake_enable_pch_transcoder(const struct intel_crtc_state *crtc_s assert_fdi_rx_enabled(dev_priv, pipe); if (HAS_PCH_CPT(dev_priv)) { - /* Workaround: Set the timing override bit before enabling the - * pch transcoder. */ reg = TRANS_CHICKEN2(pipe); val = I915_READ(reg); + /* + * Workaround: Set the timing override bit + * before enabling the pch transcoder. + */ val |= TRANS_CHICKEN2_TIMING_OVERRIDE; + /* Configure frame start delay to match the CPU */ + val &= ~TRANS_CHICKEN2_FRAME_START_DELAY_MASK; + val |= TRANS_CHICKEN2_FRAME_START_DELAY(0); I915_WRITE(reg, val); } @@ -1658,6 +1663,10 @@ static void ironlake_enable_pch_transcoder(const struct intel_crtc_state *crtc_s pipeconf_val = I915_READ(PIPECONF(pipe)); if (HAS_PCH_IBX(dev_priv)) { + /* Configure frame start delay to match the CPU */ + val &= ~TRANS_FRAME_START_DELAY_MASK; + val |= TRANS_FRAME_START_DELAY(0); + /* * Make the BPC in transcoder be consistent with * that in pipeconf reg. For HDMI we must use 8bpc @@ -1695,9 +1704,12 @@ static void lpt_enable_pch_transcoder(struct drm_i915_private *dev_priv, assert_fdi_tx_enabled(dev_priv, (enum pipe) cpu_transcoder); assert_fdi_rx_enabled(dev_priv, PIPE_A); + val = I915_READ(TRANS_CHICKEN2(PIPE_A)); /* Workaround: set timing override bit. */ - val = I915_READ(TRANS_CHICKEN2(PIPE_A)); val |= TRANS_CHICKEN2_TIMING_OVERRIDE; + /* Configure frame start delay to match the CPU */ + val &= ~TRANS_CHICKEN2_FRAME_START_DELAY_MASK; + val |= TRANS_CHICKEN2_FRAME_START_DELAY(0); I915_WRITE(TRANS_CHICKEN2(PIPE_A), val); val = TRANS_ENABLE; @@ -6452,6 +6464,19 @@ static void icl_pipe_mbus_enable(struct intel_crtc *crtc) I915_WRITE(PIPE_MBUS_DBOX_CTL(pipe), val); } +static void hsw_set_frame_start_delay(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); + i915_reg_t reg = CHICKEN_TRANS(crtc_state->cpu_transcoder); + u32 val; + + val = I915_READ(reg); + val &= ~HSW_FRAME_START_DELAY_MASK; + val |= HSW_FRAME_START_DELAY(0); + I915_WRITE(reg, val); +} + static void haswell_crtc_enable(struct intel_crtc_state *pipe_config, struct intel_atomic_state *state) { @@ -6494,8 +6519,10 @@ static void haswell_crtc_enable(struct intel_crtc_state *pipe_config, &pipe_config->fdi_m_n, NULL); } - if (!transcoder_is_dsi(cpu_transcoder)) + if (!transcoder_is_dsi(cpu_transcoder)) { + hsw_set_frame_start_delay(pipe_config); haswell_set_pipeconf(pipe_config); + } if (INTEL_GEN(dev_priv) >= 9 || IS_BROADWELL(dev_priv)) bdw_set_pipemisc(pipe_config); @@ -8394,6 +8421,8 @@ static void i9xx_set_pipeconf(const struct intel_crtc_state *crtc_state) pipeconf |= PIPECONF_GAMMA_MODE(crtc_state->gamma_mode); + pipeconf |= PIPECONF_FRAME_START_DELAY(0); + I915_WRITE(PIPECONF(crtc->pipe), pipeconf); POSTING_READ(PIPECONF(crtc->pipe)); } @@ -9474,6 +9503,8 @@ static void ironlake_set_pipeconf(const struct intel_crtc_state *crtc_state) val |= PIPECONF_GAMMA_MODE(crtc_state->gamma_mode); + val |= PIPECONF_FRAME_START_DELAY(0); + I915_WRITE(PIPECONF(pipe), val); POSTING_READ(PIPECONF(pipe)); } @@ -16919,25 +16950,69 @@ static bool has_pch_trancoder(struct drm_i915_private *dev_priv, (HAS_PCH_LPT_H(dev_priv) && pch_transcoder == PIPE_A); } +static void intel_sanitize_frame_start_delay(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 transcoder cpu_transcoder = crtc_state->cpu_transcoder; + + if (INTEL_GEN(dev_priv) >= 9 || + IS_BROADWELL(dev_priv) || IS_HASWELL(dev_priv)) { + i915_reg_t reg = CHICKEN_TRANS(cpu_transcoder); + u32 val; + + if (transcoder_is_dsi(cpu_transcoder)) + return; + + val = I915_READ(reg); + val &= ~HSW_FRAME_START_DELAY_MASK; + val |= HSW_FRAME_START_DELAY(0); + I915_WRITE(reg, val); + } else { + i915_reg_t reg = PIPECONF(cpu_transcoder); + u32 val; + + val = I915_READ(reg); + val &= ~PIPECONF_FRAME_START_DELAY_MASK; + val |= PIPECONF_FRAME_START_DELAY(0); + I915_WRITE(reg, val); + } + + if (!crtc_state->has_pch_encoder) + return; + + if (HAS_PCH_IBX(dev_priv)) { + i915_reg_t reg = PCH_TRANSCONF(crtc->pipe); + u32 val; + + val = I915_READ(reg); + val &= ~TRANS_FRAME_START_DELAY_MASK; + val |= TRANS_FRAME_START_DELAY(0); + I915_WRITE(reg, val); + } else { + i915_reg_t reg = TRANS_CHICKEN2(crtc->pipe); + u32 val; + + val = I915_READ(reg); + val &= ~TRANS_CHICKEN2_FRAME_START_DELAY_MASK; + val |= TRANS_CHICKEN2_FRAME_START_DELAY(0); + I915_WRITE(reg, val); + } +} + static void intel_sanitize_crtc(struct intel_crtc *crtc, struct drm_modeset_acquire_ctx *ctx) { struct drm_device *dev = crtc->base.dev; struct drm_i915_private *dev_priv = to_i915(dev); struct intel_crtc_state *crtc_state = to_intel_crtc_state(crtc->base.state); - enum transcoder cpu_transcoder = crtc_state->cpu_transcoder; - - /* Clear any frame start delays used for debugging left by the BIOS */ - if (crtc->active && !transcoder_is_dsi(cpu_transcoder)) { - i915_reg_t reg = PIPECONF(cpu_transcoder); - - I915_WRITE(reg, - I915_READ(reg) & ~PIPECONF_FRAME_START_DELAY_MASK); - } if (crtc_state->base.active) { struct intel_plane *plane; + /* Clear any frame start delays used for debugging left by the BIOS */ + intel_sanitize_frame_start_delay(crtc_state); + /* Disable everything but the primary plane */ for_each_intel_plane_on_crtc(dev, crtc, plane) { const struct intel_plane_state *plane_state = diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 50c2fa0f2cab..cb2e0f4679c4 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -5671,7 +5671,8 @@ enum { #define PIPECONF_DOUBLE_WIDE (1 << 30) #define I965_PIPECONF_ACTIVE (1 << 30) #define PIPECONF_DSI_PLL_LOCKED (1 << 29) /* vlv & pipe A only */ -#define PIPECONF_FRAME_START_DELAY_MASK (3 << 27) +#define PIPECONF_FRAME_START_DELAY_MASK (3 << 27) /* pre-hsw */ +#define PIPECONF_FRAME_START_DELAY(x) ((x) << 27) /* pre-hsw: 0-3 */ #define PIPECONF_SINGLE_WIDE 0 #define PIPECONF_PIPE_UNLOCKED 0 #define PIPECONF_PIPE_LOCKED (1 << 25) @@ -7627,6 +7628,8 @@ enum { [TRANSCODER_B] = _CHICKEN_TRANS_B, \ [TRANSCODER_C] = _CHICKEN_TRANS_C, \ [TRANSCODER_D] = _CHICKEN_TRANS_D)) +#define HSW_FRAME_START_DELAY_MASK (3 << 27) +#define HSW_FRAME_START_DELAY(x) ((x) << 27) /* 0-3 */ #define VSC_DATA_SEL_SOFTWARE_CONTROL (1 << 25) /* GLK and CNL+ */ #define DDI_TRAINING_OVERRIDE_ENABLE (1 << 19) #define DDI_TRAINING_OVERRIDE_VALUE (1 << 18) @@ -8341,10 +8344,8 @@ enum { #define TRANS_STATE_MASK (1 << 30) #define TRANS_STATE_DISABLE (0 << 30) #define TRANS_STATE_ENABLE (1 << 30) -#define TRANS_FSYNC_DELAY_HB1 (0 << 27) -#define TRANS_FSYNC_DELAY_HB2 (1 << 27) -#define TRANS_FSYNC_DELAY_HB3 (2 << 27) -#define TRANS_FSYNC_DELAY_HB4 (3 << 27) +#define TRANS_FRAME_START_DELAY_MASK (3 << 27) /* ibx */ +#define TRANS_FRAME_START_DELAY(x) ((x) << 27) /* ibx: 0-3 */ #define TRANS_INTERLACE_MASK (7 << 21) #define TRANS_PROGRESSIVE (0 << 21) #define TRANS_INTERLACED (3 << 21) @@ -8365,6 +8366,7 @@ enum { #define TRANS_CHICKEN2_TIMING_OVERRIDE (1 << 31) #define TRANS_CHICKEN2_FDI_POLARITY_REVERSED (1 << 29) #define TRANS_CHICKEN2_FRAME_START_DELAY_MASK (3 << 27) +#define TRANS_CHICKEN2_FRAME_START_DELAY(x) ((x) << 27) /* 0-3 */ #define TRANS_CHICKEN2_DISABLE_DEEP_COLOR_COUNTER (1 << 26) #define TRANS_CHICKEN2_DISABLE_DEEP_COLOR_MODESWITCH (1 << 25) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 362234449087..8b1fbdb36537 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -8079,7 +8079,6 @@ static void cpt_init_clock_gating(struct drm_i915_private *dev_priv) val &= ~TRANS_CHICKEN2_FDI_POLARITY_REVERSED; if (dev_priv->vbt.fdi_rx_polarity_inverted) val |= TRANS_CHICKEN2_FDI_POLARITY_REVERSED; - val &= ~TRANS_CHICKEN2_FRAME_START_DELAY_MASK; val &= ~TRANS_CHICKEN2_DISABLE_DEEP_COLOR_COUNTER; val &= ~TRANS_CHICKEN2_DISABLE_DEEP_COLOR_MODESWITCH; I915_WRITE(TRANS_CHICKEN2(pipe), val);