[Intel-gfx] [PATCH 22/42] drm/i915: Pass old state to crtc_disable and use it.
Daniel Vetter
daniel at ffwll.ch
Tue May 12 02:13:49 PDT 2015
On Mon, May 11, 2015 at 04:24:58PM +0200, Maarten Lankhorst wrote:
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
Why? Please not quite so terse commit messages ...
> ---
> drivers/gpu/drm/i915/i915_drv.h | 3 +-
> drivers/gpu/drm/i915/intel_display.c | 207 ++++++++++++++++++-----------------
> drivers/gpu/drm/i915/intel_dp.c | 2 +
> drivers/gpu/drm/i915/intel_drv.h | 10 +-
> drivers/gpu/drm/i915/intel_panel.c | 3 +-
> drivers/gpu/drm/i915/intel_tv.c | 2 +-
> 6 files changed, 120 insertions(+), 107 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index a6d3ab94ec15..3ba15df4c93a 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -573,7 +573,8 @@ struct drm_i915_display_funcs {
> int (*crtc_compute_clock)(struct intel_crtc *crtc,
> struct intel_crtc_state *crtc_state);
> void (*crtc_enable)(struct drm_crtc *crtc);
> - void (*crtc_disable)(struct drm_crtc *crtc);
> + void (*crtc_disable)(struct drm_crtc *crtc,
> + struct intel_crtc_state *old_state);
> void (*audio_codec_enable)(struct drm_connector *connector,
> struct intel_encoder *encoder,
> struct drm_display_mode *mode);
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 9bf31371ed49..311a4b44bebe 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -982,8 +982,6 @@ bool bxt_find_best_dpll(struct intel_crtc_state *crtc_state, int target_clock,
>
> bool intel_crtc_active(struct drm_crtc *crtc)
> {
> - struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> -
> /* Be paranoid as we can arrive here with only partial
> * state retrieved from the hardware during setup.
> *
> @@ -997,17 +995,8 @@ bool intel_crtc_active(struct drm_crtc *crtc)
> * crtc->state->active once we have proper CRTC states wired up
> * for atomic.
> */
> - return intel_crtc->active && crtc->primary->state->fb &&
> - intel_crtc->config->base.adjusted_mode.crtc_clock;
> -}
> -
> -enum transcoder intel_pipe_to_cpu_transcoder(struct drm_i915_private *dev_priv,
> - enum pipe pipe)
> -{
> - struct drm_crtc *crtc = dev_priv->pipe_to_crtc_mapping[pipe];
> - struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> -
> - return intel_crtc->config->cpu_transcoder;
> + return crtc->state->active && crtc->primary->state->fb &&
> + crtc->state->adjusted_mode.crtc_clock;
> }
Separate commit imo.
>
> static bool pipe_dsl_stopped(struct drm_device *dev, enum pipe pipe)
> @@ -1029,9 +1018,10 @@ static bool pipe_dsl_stopped(struct drm_device *dev, enum pipe pipe)
> return line1 == line2;
> }
>
> -/*
> +/**
> * intel_wait_for_pipe_off - wait for pipe to turn off
> * @crtc: crtc whose pipe to wait for
> + * @old_state: currently applied config
> *
> * After disabling a pipe, we can't wait for vblank in the usual way,
> * spinning on the vblank interrupt status bit, since we won't actually
> @@ -1045,11 +1035,12 @@ static bool pipe_dsl_stopped(struct drm_device *dev, enum pipe pipe)
> * ends up stopping at the start of the next frame).
> *
> */
> -static void intel_wait_for_pipe_off(struct intel_crtc *crtc)
> +static void intel_wait_for_pipe_off(struct intel_crtc *crtc,
> + struct intel_crtc_state *old_state)
> {
> struct drm_device *dev = crtc->base.dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> - enum transcoder cpu_transcoder = crtc->config->cpu_transcoder;
> + enum transcoder cpu_transcoder = old_state->cpu_transcoder;
> enum pipe pipe = crtc->pipe;
>
> if (INTEL_INFO(dev)->gen >= 4) {
> @@ -1180,13 +1171,12 @@ void assert_shared_dpll(struct drm_i915_private *dev_priv,
> }
>
> static void assert_fdi_tx(struct drm_i915_private *dev_priv,
> + enum transcoder cpu_transcoder,
> enum pipe pipe, bool state)
> {
> int reg;
> u32 val;
> bool cur_state;
> - enum transcoder cpu_transcoder = intel_pipe_to_cpu_transcoder(dev_priv,
> - pipe);
>
> if (HAS_DDI(dev_priv->dev)) {
> /* DDI does not have a specific FDI_TX register */
> @@ -1202,8 +1192,8 @@ static void assert_fdi_tx(struct drm_i915_private *dev_priv,
> "FDI TX state assertion failure (expected %s, current %s)\n",
> state_string(state), state_string(cur_state));
> }
> -#define assert_fdi_tx_enabled(d, p) assert_fdi_tx(d, p, true)
> -#define assert_fdi_tx_disabled(d, p) assert_fdi_tx(d, p, false)
> +#define assert_fdi_tx_enabled(d, c, p) assert_fdi_tx(d, c, p, true)
> +#define assert_fdi_tx_disabled(d, c, p) assert_fdi_tx(d, c, p, false)
>
> static void assert_fdi_rx(struct drm_i915_private *dev_priv,
> enum pipe pipe, bool state)
> @@ -1317,13 +1307,12 @@ static void assert_cursor(struct drm_i915_private *dev_priv,
> #define assert_cursor_disabled(d, p) assert_cursor(d, p, false)
>
> void assert_pipe(struct drm_i915_private *dev_priv,
> + enum transcoder cpu_transcoder,
> enum pipe pipe, bool state)
> {
> int reg;
> u32 val;
> bool cur_state;
> - enum transcoder cpu_transcoder = intel_pipe_to_cpu_transcoder(dev_priv,
> - pipe);
>
> /* if we need the pipe quirk it must be always on */
> if ((pipe == PIPE_A && dev_priv->quirks & QUIRK_PIPEA_FORCE) ||
> @@ -1615,7 +1604,7 @@ static void vlv_enable_pll(struct intel_crtc *crtc,
> int reg = DPLL(crtc->pipe);
> u32 dpll = pipe_config->dpll_hw_state.dpll;
>
> - assert_pipe_disabled(dev_priv, crtc->pipe);
> + assert_pipe_disabled(dev_priv, pipe_config->cpu_transcoder, crtc->pipe);
>
> /* No really, not for ILK+ */
> BUG_ON(!IS_VALLEYVIEW(dev_priv->dev));
> @@ -1655,7 +1644,7 @@ static void chv_enable_pll(struct intel_crtc *crtc,
> enum dpio_channel port = vlv_pipe_to_channel(pipe);
> u32 tmp;
>
> - assert_pipe_disabled(dev_priv, crtc->pipe);
> + assert_pipe_disabled(dev_priv, pipe_config->cpu_transcoder, crtc->pipe);
>
> BUG_ON(!IS_CHERRYVIEW(dev_priv->dev));
>
> @@ -1697,14 +1686,14 @@ static int intel_num_dvo_pipes(struct drm_device *dev)
> return count;
> }
>
> -static void i9xx_enable_pll(struct intel_crtc *crtc)
> +static void i9xx_enable_pll(struct intel_crtc *crtc, struct intel_crtc_state *pipe_config)
> {
> struct drm_device *dev = crtc->base.dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> int reg = DPLL(crtc->pipe);
> u32 dpll = crtc->config->dpll_hw_state.dpll;
>
> - assert_pipe_disabled(dev_priv, crtc->pipe);
> + assert_pipe_disabled(dev_priv, pipe_config->cpu_transcoder, crtc->pipe);
>
> /* No really, not for ILK+ */
> BUG_ON(INTEL_INFO(dev)->gen >= 5);
> @@ -1757,13 +1746,14 @@ static void i9xx_enable_pll(struct intel_crtc *crtc)
> /**
> * i9xx_disable_pll - disable a PLL
> * @dev_priv: i915 private structure
> - * @pipe: pipe PLL to disable
> + * @pipe_config: currently applied config
> *
> * Disable the PLL for @pipe, making sure the pipe is off first.
> *
> * Note! This is for pre-ILK only.
> */
> -static void i9xx_disable_pll(struct intel_crtc *crtc)
> +static void i9xx_disable_pll(struct intel_crtc *crtc,
> + struct intel_crtc_state *pipe_config)
> {
> struct drm_device *dev = crtc->base.dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -1785,7 +1775,7 @@ static void i9xx_disable_pll(struct intel_crtc *crtc)
> return;
>
> /* Make sure the pipe isn't still relying on us */
> - assert_pipe_disabled(dev_priv, pipe);
> + assert_pipe_disabled(dev_priv, pipe_config->cpu_transcoder, pipe);
>
> I915_WRITE(DPLL(pipe), 0);
> POSTING_READ(DPLL(pipe));
> @@ -1795,9 +1785,6 @@ static void vlv_disable_pll(struct drm_i915_private *dev_priv, enum pipe pipe)
> {
> u32 val = 0;
>
> - /* Make sure the pipe isn't still relying on us */
> - assert_pipe_disabled(dev_priv, pipe);
> -
> /*
> * Leave integrated clock source and reference clock enabled for pipe B.
> * The latter is needed for VGA hotplug / manual detection.
> @@ -1814,9 +1801,6 @@ static void chv_disable_pll(struct drm_i915_private *dev_priv, enum pipe pipe)
> enum dpio_channel port = vlv_pipe_to_channel(pipe);
> u32 val;
>
> - /* Make sure the pipe isn't still relying on us */
> - assert_pipe_disabled(dev_priv, pipe);
> -
> /* Set PLL en = 0 */
> val = DPLL_SSC_REF_CLOCK_CHV | DPLL_REFA_CLK_ENABLE_VLV;
> if (pipe != PIPE_A)
> @@ -1968,7 +1952,8 @@ static void intel_disable_shared_dpll(struct intel_crtc *crtc)
> }
>
> static void ironlake_enable_pch_transcoder(struct drm_i915_private *dev_priv,
> - enum pipe pipe)
> + enum pipe pipe,
> + struct intel_crtc_state *pipe_config)
> {
> struct drm_device *dev = dev_priv->dev;
> struct drm_crtc *crtc = dev_priv->pipe_to_crtc_mapping[pipe];
> @@ -1983,7 +1968,7 @@ static void ironlake_enable_pch_transcoder(struct drm_i915_private *dev_priv,
> intel_crtc_to_shared_dpll(intel_crtc));
>
> /* FDI must be feeding us bits for PCH ports */
> - assert_fdi_tx_enabled(dev_priv, pipe);
> + assert_fdi_tx_enabled(dev_priv, pipe_config->cpu_transcoder, pipe);
> assert_fdi_rx_enabled(dev_priv, pipe);
>
> if (HAS_PCH_CPT(dev)) {
> @@ -2032,7 +2017,7 @@ static void lpt_enable_pch_transcoder(struct drm_i915_private *dev_priv,
> BUG_ON(!HAS_PCH_SPLIT(dev_priv->dev));
>
> /* FDI must be feeding us bits for PCH ports */
> - assert_fdi_tx_enabled(dev_priv, (enum pipe) cpu_transcoder);
> + assert_fdi_tx_enabled(dev_priv, cpu_transcoder, (enum pipe) cpu_transcoder);
> assert_fdi_rx_enabled(dev_priv, TRANSCODER_A);
I think the assert changes would also look good in a separate commit.
>
> /* Workaround: set timing override bit. */
> @@ -2055,13 +2040,14 @@ static void lpt_enable_pch_transcoder(struct drm_i915_private *dev_priv,
> }
>
> static void ironlake_disable_pch_transcoder(struct drm_i915_private *dev_priv,
> + enum transcoder cpu_transcoder,
> enum pipe pipe)
> {
> struct drm_device *dev = dev_priv->dev;
> uint32_t reg, val;
>
> /* FDI relies on the transcoder */
> - assert_fdi_tx_disabled(dev_priv, pipe);
> + assert_fdi_tx_disabled(dev_priv, cpu_transcoder, pipe);
> assert_fdi_rx_disabled(dev_priv, pipe);
>
> /* Ports must be off as well */
> @@ -2113,8 +2099,9 @@ static void intel_enable_pipe(struct intel_crtc *crtc)
> struct drm_device *dev = crtc->base.dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> enum pipe pipe = crtc->pipe;
> - enum transcoder cpu_transcoder = intel_pipe_to_cpu_transcoder(dev_priv,
> - pipe);
> + struct intel_crtc_state *pipe_config =
> + to_intel_crtc_state(crtc->base.state);
> + enum transcoder cpu_transcoder = pipe_config->cpu_transcoder;
> enum pipe pch_transcoder;
> int reg;
> u32 val;
> @@ -2163,6 +2150,7 @@ static void intel_enable_pipe(struct intel_crtc *crtc)
> /**
> * intel_disable_pipe - disable a pipe, asserting requirements
> * @crtc: crtc whose pipes is to be disabled
> + * @old_state: currently applied config
> *
> * Disable the pipe of @crtc, making sure that various hardware
> * specific requirements are met, if applicable, e.g. plane
> @@ -2170,10 +2158,11 @@ static void intel_enable_pipe(struct intel_crtc *crtc)
> *
> * Will wait until the pipe has shut down before returning.
> */
> -static void intel_disable_pipe(struct intel_crtc *crtc)
> +static void intel_disable_pipe(struct intel_crtc *crtc,
> + struct intel_crtc_state *old_state)
> {
> struct drm_i915_private *dev_priv = crtc->base.dev->dev_private;
> - enum transcoder cpu_transcoder = crtc->config->cpu_transcoder;
> + enum transcoder cpu_transcoder = old_state->cpu_transcoder;
> enum pipe pipe = crtc->pipe;
> int reg;
> u32 val;
> @@ -2195,7 +2184,7 @@ static void intel_disable_pipe(struct intel_crtc *crtc)
> * Double wide has implications for planes
> * so best keep it disabled when not needed.
> */
> - if (crtc->config->double_wide)
> + if (old_state->double_wide)
> val &= ~PIPECONF_DOUBLE_WIDE;
>
> /* Don't disable pipe or pipe PLLs if needed */
> @@ -2205,7 +2194,7 @@ static void intel_disable_pipe(struct intel_crtc *crtc)
>
> I915_WRITE(reg, val);
> if ((val & PIPECONF_ENABLE) == 0)
> - intel_wait_for_pipe_off(crtc);
> + intel_wait_for_pipe_off(crtc, old_state);
> }
>
> /*
> @@ -3395,9 +3384,6 @@ static void ironlake_fdi_link_train(struct drm_crtc *crtc)
> int pipe = intel_crtc->pipe;
> u32 reg, temp, tries;
>
> - /* FDI needs bits from pipe first */
> - assert_pipe_enabled(dev_priv, pipe);
> -
> /* Train 1: umask FDI RX Interrupt symbol_lock and bit_lock bit
> for train result */
> reg = FDI_RX_IMR(pipe);
> @@ -3737,7 +3723,8 @@ train_done:
> DRM_DEBUG_KMS("FDI train done.\n");
> }
>
> -static void ironlake_fdi_pll_enable(struct intel_crtc *intel_crtc)
> +static void ironlake_fdi_pll_enable(struct intel_crtc *intel_crtc,
> + struct intel_crtc_state *pipe_config)
> {
> struct drm_device *dev = intel_crtc->base.dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -3749,7 +3736,7 @@ static void ironlake_fdi_pll_enable(struct intel_crtc *intel_crtc)
> reg = FDI_RX_CTL(pipe);
> temp = I915_READ(reg);
> temp &= ~(FDI_DP_PORT_WIDTH_MASK | (0x7 << 16));
> - temp |= FDI_DP_PORT_WIDTH(intel_crtc->config->fdi_lanes);
> + temp |= FDI_DP_PORT_WIDTH(pipe_config->fdi_lanes);
> temp |= (I915_READ(PIPECONF(pipe)) & PIPECONF_BPC_MASK) << 11;
> I915_WRITE(reg, temp | FDI_RX_PLL_ENABLE);
>
> @@ -4093,7 +4080,7 @@ static void ivybridge_update_fdi_bc_bifurcation(struct intel_crtc *intel_crtc)
> * - DP transcoding bits
> * - transcoder
> */
> -static void ironlake_pch_enable(struct drm_crtc *crtc)
> +static void ironlake_pch_enable(struct drm_crtc *crtc, struct intel_crtc_state *pipe_config)
> {
> struct drm_device *dev = crtc->dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -4122,7 +4109,7 @@ static void ironlake_pch_enable(struct drm_crtc *crtc)
> temp = I915_READ(PCH_DPLL_SEL);
> temp |= TRANS_DPLL_ENABLE(pipe);
> sel = TRANS_DPLLB_SEL(pipe);
> - if (intel_crtc->config->shared_dpll == DPLL_ID_PCH_PLL_B)
> + if (pipe_config->shared_dpll == DPLL_ID_PCH_PLL_B)
> temp |= sel;
> else
> temp &= ~sel;
> @@ -4145,7 +4132,7 @@ static void ironlake_pch_enable(struct drm_crtc *crtc)
> intel_fdi_normal_train(crtc);
>
> /* For PCH DP, enable TRANS_DP_CTL */
> - if (HAS_PCH_CPT(dev) && intel_crtc->config->has_dp_encoder) {
> + if (HAS_PCH_CPT(dev) && pipe_config->has_dp_encoder) {
> u32 bpc = (I915_READ(PIPECONF(pipe)) & PIPECONF_BPC_MASK) >> 5;
> reg = TRANS_DP_CTL(pipe);
> temp = I915_READ(reg);
> @@ -4178,7 +4165,7 @@ static void ironlake_pch_enable(struct drm_crtc *crtc)
> I915_WRITE(reg, temp);
> }
>
> - ironlake_enable_pch_transcoder(dev_priv, pipe);
> + ironlake_enable_pch_transcoder(dev_priv, pipe, pipe_config);
> }
>
> static void lpt_pch_enable(struct drm_crtc *crtc)
> @@ -4789,23 +4776,25 @@ static void ironlake_crtc_enable(struct drm_crtc *crtc)
> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> struct intel_encoder *encoder;
> int pipe = intel_crtc->pipe;
> + struct intel_crtc_state *pipe_config;
>
> WARN_ON(!crtc->state->enable);
>
> - if (intel_crtc->active)
> + if (WARN_ON(intel_crtc->active))
> return;
> + pipe_config = to_intel_crtc_state(crtc->state);
>
> - if (intel_crtc->config->has_pch_encoder)
> + if (pipe_config->has_pch_encoder)
> intel_prepare_shared_dpll(intel_crtc);
>
> - if (intel_crtc->config->has_dp_encoder)
> + if (pipe_config->has_dp_encoder)
> intel_dp_set_m_n(intel_crtc, M1_N1);
>
> intel_set_pipe_timings(intel_crtc);
>
> - if (intel_crtc->config->has_pch_encoder) {
> + if (pipe_config->has_pch_encoder) {
> intel_cpu_transcoder_set_m_n(intel_crtc,
> - &intel_crtc->config->fdi_m_n, NULL);
> + &pipe_config->fdi_m_n, NULL);
> }
>
> ironlake_set_pipeconf(crtc);
> @@ -4823,9 +4812,9 @@ static void ironlake_crtc_enable(struct drm_crtc *crtc)
> /* Note: FDI PLL enabling _must_ be done before we enable the
> * cpu pipes, hence this is separate from all the other fdi/pch
> * enabling. */
> - ironlake_fdi_pll_enable(intel_crtc);
> + ironlake_fdi_pll_enable(intel_crtc, pipe_config);
Tons of changes in _enable code which really shouldn't be part of this
patch here.
> } else {
> - assert_fdi_tx_disabled(dev_priv, pipe);
> + assert_fdi_tx_disabled(dev_priv, pipe_config->cpu_transcoder, pipe);
> assert_fdi_rx_disabled(dev_priv, pipe);
> }
>
> @@ -4841,7 +4830,7 @@ static void ironlake_crtc_enable(struct drm_crtc *crtc)
> intel_enable_pipe(intel_crtc);
>
> if (intel_crtc->config->has_pch_encoder)
> - ironlake_pch_enable(crtc);
> + ironlake_pch_enable(crtc, pipe_config);
>
> assert_vblank_disabled(crtc);
> drm_crtc_vblank_on(crtc);
> @@ -4893,30 +4882,32 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
> struct drm_device *dev = crtc->dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> + struct intel_crtc_state *pipe_config;
> struct intel_encoder *encoder;
> int pipe = intel_crtc->pipe;
>
> WARN_ON(!crtc->state->enable);
>
> - if (intel_crtc->active)
> + if (WARN_ON(intel_crtc->active))
> return;
>
> + pipe_config = to_intel_crtc_state(crtc->state);
> if (intel_crtc_to_shared_dpll(intel_crtc))
> intel_enable_shared_dpll(intel_crtc);
>
> - if (intel_crtc->config->has_dp_encoder)
> + if (pipe_config->has_dp_encoder)
> intel_dp_set_m_n(intel_crtc, M1_N1);
>
> intel_set_pipe_timings(intel_crtc);
>
> - if (intel_crtc->config->cpu_transcoder != TRANSCODER_EDP) {
> - I915_WRITE(PIPE_MULT(intel_crtc->config->cpu_transcoder),
> - intel_crtc->config->pixel_multiplier - 1);
> + if (pipe_config->cpu_transcoder != TRANSCODER_EDP) {
> + I915_WRITE(PIPE_MULT(pipe_config->cpu_transcoder),
> + pipe_config->pixel_multiplier - 1);
> }
>
> - if (intel_crtc->config->has_pch_encoder) {
> + if (pipe_config->has_pch_encoder) {
> intel_cpu_transcoder_set_m_n(intel_crtc,
> - &intel_crtc->config->fdi_m_n, NULL);
> + &pipe_config->fdi_m_n, NULL);
> }
>
> haswell_set_pipeconf(crtc);
> @@ -4930,9 +4921,10 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
> if (encoder->pre_enable)
> encoder->pre_enable(encoder);
>
> - if (intel_crtc->config->has_pch_encoder) {
> + if (pipe_config->has_pch_encoder) {
> intel_set_pch_fifo_underrun_reporting(dev_priv, TRANSCODER_A,
> true);
> +
> dev_priv->display.fdi_link_train(crtc);
> }
>
> @@ -4991,7 +4983,8 @@ static void ironlake_pfit_disable(struct intel_crtc *crtc)
> }
> }
>
> -static void ironlake_crtc_disable(struct drm_crtc *crtc)
> +static void ironlake_crtc_disable(struct drm_crtc *crtc,
> + struct intel_crtc_state *old_state)
> {
> struct drm_device *dev = crtc->dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -5006,10 +4999,10 @@ static void ironlake_crtc_disable(struct drm_crtc *crtc)
> drm_crtc_vblank_off(crtc);
> assert_vblank_disabled(crtc);
>
> - if (intel_crtc->config->has_pch_encoder)
> + if (old_state->has_pch_encoder)
> intel_set_pch_fifo_underrun_reporting(dev_priv, pipe, false);
>
> - intel_disable_pipe(intel_crtc);
> + intel_disable_pipe(intel_crtc, old_state);
>
> ironlake_pfit_disable(intel_crtc);
>
> @@ -5017,10 +5010,10 @@ static void ironlake_crtc_disable(struct drm_crtc *crtc)
> if (encoder->post_disable)
> encoder->post_disable(encoder);
>
> - if (intel_crtc->config->has_pch_encoder) {
> + if (old_state->has_pch_encoder) {
> ironlake_fdi_disable(crtc);
>
> - ironlake_disable_pch_transcoder(dev_priv, pipe);
> + ironlake_disable_pch_transcoder(dev_priv, old_state->cpu_transcoder, pipe);
>
> if (HAS_PCH_CPT(dev)) {
> /* disable TRANS_DP_CTL */
> @@ -5041,7 +5034,8 @@ static void ironlake_crtc_disable(struct drm_crtc *crtc)
> }
> }
>
> -static void haswell_crtc_disable(struct drm_crtc *crtc)
> +static void haswell_crtc_disable(struct drm_crtc *crtc,
> + struct intel_crtc_state *old_state)
> {
> struct drm_device *dev = crtc->dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -5060,7 +5054,7 @@ static void haswell_crtc_disable(struct drm_crtc *crtc)
> if (intel_crtc->config->has_pch_encoder)
> intel_set_pch_fifo_underrun_reporting(dev_priv, TRANSCODER_A,
> false);
> - intel_disable_pipe(intel_crtc);
> + intel_disable_pipe(intel_crtc, old_state);
>
> if (intel_crtc->config->dp_encoder_is_mst)
> intel_ddi_set_vc_payload_alloc(crtc, false);
> @@ -5086,11 +5080,11 @@ static void haswell_crtc_disable(struct drm_crtc *crtc)
> encoder->post_disable(encoder);
> }
>
> -static void i9xx_pfit_enable(struct intel_crtc *crtc)
> +static void i9xx_pfit_enable(struct intel_crtc *crtc,
> + struct intel_crtc_state *pipe_config)
> {
> struct drm_device *dev = crtc->base.dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> - struct intel_crtc_state *pipe_config = crtc->config;
>
> if (!pipe_config->gmch_pfit.control)
> return;
> @@ -5100,7 +5094,7 @@ static void i9xx_pfit_enable(struct intel_crtc *crtc)
> * according to register description and PRM.
> */
> WARN_ON(I915_READ(PFIT_CONTROL) & PFIT_ENABLE);
> - assert_pipe_disabled(dev_priv, crtc->pipe);
> + assert_pipe_disabled(dev_priv, pipe_config->cpu_transcoder, crtc->pipe);
>
> I915_WRITE(PFIT_PGM_RATIOS, pipe_config->gmch_pfit.pgm_ratios);
> I915_WRITE(PFIT_CONTROL, pipe_config->gmch_pfit.control);
> @@ -5167,7 +5161,7 @@ static unsigned long get_crtc_power_domains(struct drm_crtc *crtc)
> unsigned long mask;
> enum transcoder transcoder;
>
> - transcoder = intel_pipe_to_cpu_transcoder(dev->dev_private, pipe);
> + transcoder = to_intel_crtc_state(crtc->state)->cpu_transcoder;
>
> mask = BIT(POWER_DOMAIN_PIPE(pipe));
> mask |= BIT(POWER_DOMAIN_TRANSCODER(transcoder));
> @@ -5767,22 +5761,24 @@ static void valleyview_crtc_enable(struct drm_crtc *crtc)
> struct intel_encoder *encoder;
> int pipe = intel_crtc->pipe;
> bool is_dsi;
> + struct intel_crtc_state *pipe_config;
>
> WARN_ON(!crtc->state->active);
>
> - if (intel_crtc->active)
> + if (WARN_ON(intel_crtc->active))
> return;
> + pipe_config = to_intel_crtc_state(crtc->state);
>
> is_dsi = intel_pipe_has_type(intel_crtc, INTEL_OUTPUT_DSI);
>
> if (!is_dsi) {
> if (IS_CHERRYVIEW(dev))
> - chv_prepare_pll(intel_crtc, intel_crtc->config);
> + chv_prepare_pll(intel_crtc, pipe_config);
> else
> - vlv_prepare_pll(intel_crtc, intel_crtc->config);
> + vlv_prepare_pll(intel_crtc, pipe_config);
> }
>
> - if (intel_crtc->config->has_dp_encoder)
> + if (pipe_config->has_dp_encoder)
> intel_dp_set_m_n(intel_crtc, M1_N1);
>
> intel_set_pipe_timings(intel_crtc);
> @@ -5806,16 +5802,16 @@ static void valleyview_crtc_enable(struct drm_crtc *crtc)
>
> if (!is_dsi) {
> if (IS_CHERRYVIEW(dev))
> - chv_enable_pll(intel_crtc, intel_crtc->config);
> + chv_enable_pll(intel_crtc, pipe_config);
> else
> - vlv_enable_pll(intel_crtc, intel_crtc->config);
> + vlv_enable_pll(intel_crtc, pipe_config);
> }
>
> for_each_encoder_on_crtc(dev, crtc, encoder)
> if (encoder->pre_enable)
> encoder->pre_enable(encoder);
>
> - i9xx_pfit_enable(intel_crtc);
> + i9xx_pfit_enable(intel_crtc, pipe_config);
>
> intel_crtc_load_lut(crtc);
>
> @@ -5844,11 +5840,13 @@ static void i9xx_crtc_enable(struct drm_crtc *crtc)
> struct drm_i915_private *dev_priv = to_i915(dev);
> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> struct intel_encoder *encoder;
> + struct intel_crtc_state *pipe_config;
> int pipe = intel_crtc->pipe;
>
> - WARN_ON(!crtc->state->active);
> + pipe_config = to_intel_crtc_state(crtc->state);
> + WARN_ON(!pipe_config->base.active);
>
> - if (intel_crtc->active)
> + if (WARN_ON(intel_crtc->active))
> return;
>
> i9xx_set_pll_dividers(intel_crtc);
> @@ -5869,9 +5867,9 @@ static void i9xx_crtc_enable(struct drm_crtc *crtc)
> if (encoder->pre_enable)
> encoder->pre_enable(encoder);
>
> - i9xx_enable_pll(intel_crtc);
> + i9xx_enable_pll(intel_crtc, pipe_config);
>
> - i9xx_pfit_enable(intel_crtc);
> + i9xx_pfit_enable(intel_crtc, pipe_config);
>
> intel_crtc_load_lut(crtc);
>
> @@ -5893,14 +5891,13 @@ static void i9xx_pfit_disable(struct intel_crtc *crtc)
> if (!crtc->config->gmch_pfit.control)
> return;
>
> - assert_pipe_disabled(dev_priv, crtc->pipe);
> -
> DRM_DEBUG_DRIVER("disabling pfit, current: 0x%08x\n",
> I915_READ(PFIT_CONTROL));
> I915_WRITE(PFIT_CONTROL, 0);
> }
>
> -static void i9xx_crtc_disable(struct drm_crtc *crtc)
> +static void i9xx_crtc_disable(struct drm_crtc *crtc,
> + struct intel_crtc_state *old_state)
> {
> struct drm_device *dev = crtc->dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -5922,7 +5919,7 @@ static void i9xx_crtc_disable(struct drm_crtc *crtc)
> drm_crtc_vblank_off(crtc);
> assert_vblank_disabled(crtc);
>
> - intel_disable_pipe(intel_crtc);
> + intel_disable_pipe(intel_crtc, old_state);
>
> i9xx_pfit_disable(intel_crtc);
>
> @@ -5936,7 +5933,7 @@ static void i9xx_crtc_disable(struct drm_crtc *crtc)
> else if (IS_VALLEYVIEW(dev))
> vlv_disable_pll(dev_priv, pipe);
> else
> - i9xx_disable_pll(intel_crtc);
> + i9xx_disable_pll(intel_crtc, old_state);
> }
>
> if (!IS_GEN2(dev))
> @@ -6989,10 +6986,12 @@ void vlv_force_pll_on(struct drm_device *dev, enum pipe pipe,
> {
> struct intel_crtc *crtc =
> to_intel_crtc(intel_get_crtc_for_pipe(dev, pipe));
> + struct intel_crtc_state *state = to_intel_crtc_state(crtc->base.state);
> struct intel_crtc_state pipe_config = {
> .base.crtc = &crtc->base,
> .pixel_multiplier = 1,
> .dpll = *dpll,
> + .cpu_transcoder = state->cpu_transcoder,
> };
>
> if (IS_CHERRYVIEW(dev)) {
> @@ -7016,6 +7015,16 @@ void vlv_force_pll_on(struct drm_device *dev, enum pipe pipe,
> */
> void vlv_force_pll_off(struct drm_device *dev, enum pipe pipe)
> {
> + struct drm_i915_private *dev_priv = dev->dev_private;
> + struct drm_crtc *crtc = intel_get_crtc_for_pipe(dev, pipe);
> + struct intel_crtc_state *state = to_intel_crtc_state(crtc->state);
> +
> + /* Is state->cpu_transcoder correct to use? */
> + WARN_ON(crtc->state != &to_intel_crtc(crtc)->config->base);
> +
> + /* Make sure the pipe isn't still relying on us */
> + assert_pipe_disabled(dev_priv, state->cpu_transcoder, pipe);
> +
> if (IS_CHERRYVIEW(dev))
> chv_disable_pll(to_i915(dev), pipe);
> else
> @@ -12275,6 +12284,7 @@ static int __intel_set_mode(struct drm_atomic_state *state)
>
> for_each_crtc_in_state(state, crtc, old_crtc_state, i) {
> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> + struct intel_crtc_state *pipe_config;
>
> if (!needs_modeset(crtc->state))
> continue;
> @@ -12282,8 +12292,9 @@ static int __intel_set_mode(struct drm_atomic_state *state)
> if (!old_crtc_state->active)
> continue;
>
> + pipe_config = to_intel_crtc_state(old_crtc_state);
> intel_crtc_dpms_overlay_disable(to_intel_crtc(crtc));
> - dev_priv->display.crtc_disable(crtc);
> + dev_priv->display.crtc_disable(crtc, pipe_config);
>
> intel_crtc->active = false;
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 82dcc6ed8b1d..83de36bfddc1 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -2090,6 +2090,7 @@ static void ironlake_edp_pll_on(struct intel_dp *intel_dp)
> u32 dpa_ctl;
>
> assert_pipe_disabled(dev_priv,
> + to_intel_crtc_state(crtc->state)->cpu_transcoder,
> to_intel_crtc(crtc)->pipe);
>
> DRM_DEBUG_KMS("\n");
> @@ -2116,6 +2117,7 @@ static void ironlake_edp_pll_off(struct intel_dp *intel_dp)
> u32 dpa_ctl;
>
> assert_pipe_disabled(dev_priv,
> + to_intel_crtc(crtc)->config->cpu_transcoder,
> to_intel_crtc(crtc)->pipe);
>
> dpa_ctl = I915_READ(DP_A);
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 9ef89c91aa5c..58240e63630c 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -998,8 +998,6 @@ struct drm_display_mode *intel_crtc_mode_get(struct drm_device *dev,
> enum pipe intel_get_pipe_from_connector(struct intel_connector *connector);
> int intel_get_pipe_from_crtc_id(struct drm_device *dev, void *data,
> struct drm_file *file_priv);
> -enum transcoder intel_pipe_to_cpu_transcoder(struct drm_i915_private *dev_priv,
> - enum pipe pipe);
> bool intel_pipe_has_type(struct intel_crtc *crtc, enum intel_output_type type);
> static inline void
> intel_wait_for_vblank(struct drm_device *dev, int pipe)
> @@ -1085,9 +1083,11 @@ void assert_fdi_rx_pll(struct drm_i915_private *dev_priv,
> enum pipe pipe, bool state);
> #define assert_fdi_rx_pll_enabled(d, p) assert_fdi_rx_pll(d, p, true)
> #define assert_fdi_rx_pll_disabled(d, p) assert_fdi_rx_pll(d, p, false)
> -void assert_pipe(struct drm_i915_private *dev_priv, enum pipe pipe, bool state);
> -#define assert_pipe_enabled(d, p) assert_pipe(d, p, true)
> -#define assert_pipe_disabled(d, p) assert_pipe(d, p, false)
> +void assert_pipe(struct drm_i915_private *dev_priv,
> + enum transcoder cpu_transcoder,
> + enum pipe pipe, bool state);
> +#define assert_pipe_enabled(d, c, p) assert_pipe(d, c, p, true)
> +#define assert_pipe_disabled(d, c, p) assert_pipe(d, c, p, false)
> unsigned long intel_gen4_compute_page_offset(int *x, int *y,
> unsigned int tiling_mode,
> unsigned int bpp,
> diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
> index 7d83527f95f7..67eafa27400b 100644
> --- a/drivers/gpu/drm/i915/intel_panel.c
> +++ b/drivers/gpu/drm/i915/intel_panel.c
> @@ -837,9 +837,8 @@ static void pch_enable_backlight(struct intel_connector *connector)
> struct drm_device *dev = connector->base.dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> struct intel_panel *panel = &connector->panel;
> - enum pipe pipe = intel_get_pipe_from_connector(connector);
> enum transcoder cpu_transcoder =
> - intel_pipe_to_cpu_transcoder(dev_priv, pipe);
> + to_intel_crtc_state(connector->encoder->base.crtc->state)->cpu_transcoder;
> u32 cpu_ctl2, pch_ctl1, pch_ctl2;
>
> cpu_ctl2 = I915_READ(BLC_PWM_CPU_CTL2);
> diff --git a/drivers/gpu/drm/i915/intel_tv.c b/drivers/gpu/drm/i915/intel_tv.c
> index 8b9d325bda3c..b4aeb256128f 100644
> --- a/drivers/gpu/drm/i915/intel_tv.c
> +++ b/drivers/gpu/drm/i915/intel_tv.c
> @@ -1117,7 +1117,7 @@ static void intel_tv_pre_enable(struct intel_encoder *encoder)
> ((video_levels->black << TV_BLACK_LEVEL_SHIFT) |
> (video_levels->blank << TV_BLANK_LEVEL_SHIFT)));
>
> - assert_pipe_disabled(dev_priv, intel_crtc->pipe);
> + assert_pipe_disabled(dev_priv, to_intel_crtc_state(intel_crtc->base.state)->cpu_transcoder, intel_crtc->pipe);
These two hunks look misplaced, should be in a different patch. I didn't
comment about all of them, but imo the patch to roll out the old state for
crtc_disable shouldn't mix hunks with _enabl code, since then checking for
whether we need the old or new state gets really quickly confusing.
-Daniel
>
> /* Filter ctl must be set before TV_WIN_SIZE */
> I915_WRITE(TV_FILTER_CTL_1, TV_AUTO_SCALE);
> --
> 2.1.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
More information about the Intel-gfx
mailing list