| Message ID | 20191112163812.22075-4-ville.syrjala@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/4] drm/i915/fbc: Nuke bogus single pipe fbc1 restriction | expand |
On Tue, 2019-11-12 at 18:38 +0200, Ville Syrjala wrote: > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > In order to eliminate intel_pipe_to_cpu_transcoder() (and its > crtc->config usage) let's pass the cpu transcoder to > assert_pipe() so we don't have to do the pipe->cpu transcoder > lookup on HSW+. > > On VLV/CHV this can get called during eDP init, which > happens before crtc->config->cpu_transcoder is even > populated. So currently we're always reading PIPECONF(A) > there even if we're trying to check the state of some > other pipe. > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > --- > drivers/gpu/drm/i915/display/intel_display.c | 36 ++++++++-------- > ---- > drivers/gpu/drm/i915/display/intel_display.h | 9 +++-- > 2 files changed, 18 insertions(+), 27 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c > b/drivers/gpu/drm/i915/display/intel_display.c > index cabd88337822..6d2112f5bdd0 100644 > --- a/drivers/gpu/drm/i915/display/intel_display.c > +++ b/drivers/gpu/drm/i915/display/intel_display.c > @@ -1040,14 +1040,6 @@ bool bxt_find_best_dpll(struct > intel_crtc_state *crtc_state, > NULL, best_clock); > } > > -enum transcoder intel_pipe_to_cpu_transcoder(struct drm_i915_private > *dev_priv, > - enum pipe pipe) > -{ > - struct intel_crtc *crtc = intel_get_crtc_for_pipe(dev_priv, > pipe); > - > - return crtc->config->cpu_transcoder; > -} > - > static bool pipe_scanline_is_moving(struct drm_i915_private > *dev_priv, > enum pipe pipe) > { > @@ -1266,11 +1258,9 @@ void assert_panel_unlocked(struct > drm_i915_private *dev_priv, enum pipe pipe) > } > > void assert_pipe(struct drm_i915_private *dev_priv, > - enum pipe pipe, bool state) > + enum transcoder cpu_transcoder, bool state) > { > bool cur_state; > - enum transcoder cpu_transcoder = > intel_pipe_to_cpu_transcoder(dev_priv, > - p > ipe); > enum intel_display_power_domain power_domain; > intel_wakeref_t wakeref; > > @@ -1290,8 +1280,9 @@ void assert_pipe(struct drm_i915_private > *dev_priv, > } > > I915_STATE_WARN(cur_state != state, > - "pipe %c assertion failure (expected %s, current %s)\n", > - pipe_name(pipe), onoff(state), > onoff(cur_state)); > + "transcoder %s assertion failure (expected %s, > current %s)\n", > + transcoder_name(cpu_transcoder), > + onoff(state), onoff(cur_state)); > } > > static void assert_plane(struct intel_plane *plane, bool state) > @@ -1418,7 +1409,7 @@ static void vlv_enable_pll(struct intel_crtc > *crtc, > struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); > enum pipe pipe = crtc->pipe; > > - assert_pipe_disabled(dev_priv, pipe); > + assert_pipe_disabled(dev_priv, pipe_config->cpu_transcoder); > > /* PLL is protected by panel, make sure we can write it */ > assert_panel_unlocked(dev_priv, pipe); > @@ -1467,7 +1458,7 @@ static void chv_enable_pll(struct intel_crtc > *crtc, > struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); > enum pipe pipe = crtc->pipe; > > - assert_pipe_disabled(dev_priv, pipe); > + assert_pipe_disabled(dev_priv, pipe_config->cpu_transcoder); > > /* PLL is protected by panel, make sure we can write it */ > assert_panel_unlocked(dev_priv, pipe); > @@ -1514,7 +1505,7 @@ static void i9xx_enable_pll(struct intel_crtc > *crtc, > u32 dpll = crtc_state->dpll_hw_state.dpll; > int i; > > - assert_pipe_disabled(dev_priv, crtc->pipe); > + assert_pipe_disabled(dev_priv, crtc_state->cpu_transcoder); > > /* PLL is protected by panel, make sure we can write it */ > if (i9xx_has_pps(dev_priv)) > @@ -1563,7 +1554,7 @@ static void i9xx_disable_pll(const struct > intel_crtc_state *crtc_state) > return; > > /* Make sure the pipe isn't still relying on us */ > - assert_pipe_disabled(dev_priv, pipe); > + assert_pipe_disabled(dev_priv, crtc_state->cpu_transcoder); > > I915_WRITE(DPLL(pipe), DPLL_VGA_MODE_DIS); > POSTING_READ(DPLL(pipe)); > @@ -1574,7 +1565,7 @@ static void vlv_disable_pll(struct > drm_i915_private *dev_priv, enum pipe pipe) > u32 val; > > /* Make sure the pipe isn't still relying on us */ > - assert_pipe_disabled(dev_priv, pipe); > + assert_pipe_disabled(dev_priv, (enum transcoder)pipe); > > val = DPLL_INTEGRATED_REF_CLK_VLV | > DPLL_REF_CLK_ENABLE_VLV | DPLL_VGA_MODE_DIS; > @@ -1591,7 +1582,7 @@ static void chv_disable_pll(struct > drm_i915_private *dev_priv, enum pipe pipe) > u32 val; > > /* Make sure the pipe isn't still relying on us */ > - assert_pipe_disabled(dev_priv, pipe); > + assert_pipe_disabled(dev_priv, (enum transcoder)pipe); > > val = DPLL_SSC_REF_CLK_CHV | > DPLL_REF_CLK_ENABLE_VLV | DPLL_VGA_MODE_DIS; > @@ -4617,7 +4608,7 @@ static void ironlake_fdi_link_train(struct > intel_crtc *crtc, > u32 temp, tries; > > /* FDI needs bits from pipe first */ > - assert_pipe_enabled(dev_priv, pipe); > + assert_pipe_enabled(dev_priv, crtc_state->cpu_transcoder); > > /* Train 1: umask FDI RX Interrupt symbol_lock and bit_lock bit > for train result */ > @@ -6805,7 +6796,7 @@ static void i9xx_pfit_enable(const struct > intel_crtc_state *crtc_state) > * 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, crtc_state->cpu_transcoder); > > I915_WRITE(PFIT_PGM_RATIOS, crtc_state->gmch_pfit.pgm_ratios); > I915_WRITE(PFIT_CONTROL, crtc_state->gmch_pfit.control); > @@ -7116,7 +7107,7 @@ static void i9xx_pfit_disable(const struct > intel_crtc_state *old_crtc_state) > if (!old_crtc_state->gmch_pfit.control) > return; > > - assert_pipe_disabled(dev_priv, crtc->pipe); > + assert_pipe_disabled(dev_priv, old_crtc_state->cpu_transcoder); > > DRM_DEBUG_KMS("disabling pfit, current: 0x%08x\n", > I915_READ(PFIT_CONTROL)); > @@ -8128,6 +8119,7 @@ int vlv_force_pll_on(struct drm_i915_private > *dev_priv, enum pipe pipe, > return -ENOMEM; > > pipe_config->uapi.crtc = &crtc->base; > + pipe_config->cpu_transcoder = (enum transcoder)pipe; > pipe_config->pixel_multiplier = 1; > pipe_config->dpll = *dpll; > > diff --git a/drivers/gpu/drm/i915/display/intel_display.h > b/drivers/gpu/drm/i915/display/intel_display.h > index d18dc260fe83..f33096876a9a 100644 > --- a/drivers/gpu/drm/i915/display/intel_display.h > +++ b/drivers/gpu/drm/i915/display/intel_display.h > @@ -510,8 +510,6 @@ enum tc_port intel_port_to_tc(struct > drm_i915_private *dev_priv, > enum port port); > int intel_get_pipe_from_crtc_id_ioctl(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); > u32 intel_crtc_get_vblank_counter(struct intel_crtc *crtc); > > int ironlake_get_lanes_required(int target_clock, int link_bw, int > bpp); > @@ -613,9 +611,10 @@ 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, bool state); > +#define assert_pipe_enabled(d, t) assert_pipe(d, t, true) > +#define assert_pipe_disabled(d, t) assert_pipe(d, t, false) Maybe while doing all those already rename it to assert_transcoder_enabled/disabled()? Also why not just squash with the previous patch? Other than that: Reviewed-by: José Roberto de Souza <jose.souza@intel.com> > > /* Use I915_STATE_WARN(x) and I915_STATE_WARN_ON() (rather than > WARN() and > * WARN_ON()) for hw state sanity checks to check for unexpected > conditions
On Tue, Dec 10, 2019 at 06:23:36PM +0000, Souza, Jose wrote: > On Tue, 2019-11-12 at 18:38 +0200, Ville Syrjala wrote: > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > In order to eliminate intel_pipe_to_cpu_transcoder() (and its > > crtc->config usage) let's pass the cpu transcoder to > > assert_pipe() so we don't have to do the pipe->cpu transcoder > > lookup on HSW+. > > > > On VLV/CHV this can get called during eDP init, which > > happens before crtc->config->cpu_transcoder is even > > populated. So currently we're always reading PIPECONF(A) > > there even if we're trying to check the state of some > > other pipe. > > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > --- > > drivers/gpu/drm/i915/display/intel_display.c | 36 ++++++++-------- > > ---- > > drivers/gpu/drm/i915/display/intel_display.h | 9 +++-- > > 2 files changed, 18 insertions(+), 27 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c > > b/drivers/gpu/drm/i915/display/intel_display.c > > index cabd88337822..6d2112f5bdd0 100644 > > --- a/drivers/gpu/drm/i915/display/intel_display.c > > +++ b/drivers/gpu/drm/i915/display/intel_display.c > > @@ -1040,14 +1040,6 @@ bool bxt_find_best_dpll(struct > > intel_crtc_state *crtc_state, > > NULL, best_clock); > > } > > > > -enum transcoder intel_pipe_to_cpu_transcoder(struct drm_i915_private > > *dev_priv, > > - enum pipe pipe) > > -{ > > - struct intel_crtc *crtc = intel_get_crtc_for_pipe(dev_priv, > > pipe); > > - > > - return crtc->config->cpu_transcoder; > > -} > > - > > static bool pipe_scanline_is_moving(struct drm_i915_private > > *dev_priv, > > enum pipe pipe) > > { > > @@ -1266,11 +1258,9 @@ void assert_panel_unlocked(struct > > drm_i915_private *dev_priv, enum pipe pipe) > > } > > > > void assert_pipe(struct drm_i915_private *dev_priv, > > - enum pipe pipe, bool state) > > + enum transcoder cpu_transcoder, bool state) > > { > > bool cur_state; > > - enum transcoder cpu_transcoder = > > intel_pipe_to_cpu_transcoder(dev_priv, > > - p > > ipe); > > enum intel_display_power_domain power_domain; > > intel_wakeref_t wakeref; > > > > @@ -1290,8 +1280,9 @@ void assert_pipe(struct drm_i915_private > > *dev_priv, > > } > > > > I915_STATE_WARN(cur_state != state, > > - "pipe %c assertion failure (expected %s, current %s)\n", > > - pipe_name(pipe), onoff(state), > > onoff(cur_state)); > > + "transcoder %s assertion failure (expected %s, > > current %s)\n", > > + transcoder_name(cpu_transcoder), > > + onoff(state), onoff(cur_state)); > > } > > > > static void assert_plane(struct intel_plane *plane, bool state) > > @@ -1418,7 +1409,7 @@ static void vlv_enable_pll(struct intel_crtc > > *crtc, > > struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); > > enum pipe pipe = crtc->pipe; > > > > - assert_pipe_disabled(dev_priv, pipe); > > + assert_pipe_disabled(dev_priv, pipe_config->cpu_transcoder); > > > > /* PLL is protected by panel, make sure we can write it */ > > assert_panel_unlocked(dev_priv, pipe); > > @@ -1467,7 +1458,7 @@ static void chv_enable_pll(struct intel_crtc > > *crtc, > > struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); > > enum pipe pipe = crtc->pipe; > > > > - assert_pipe_disabled(dev_priv, pipe); > > + assert_pipe_disabled(dev_priv, pipe_config->cpu_transcoder); > > > > /* PLL is protected by panel, make sure we can write it */ > > assert_panel_unlocked(dev_priv, pipe); > > @@ -1514,7 +1505,7 @@ static void i9xx_enable_pll(struct intel_crtc > > *crtc, > > u32 dpll = crtc_state->dpll_hw_state.dpll; > > int i; > > > > - assert_pipe_disabled(dev_priv, crtc->pipe); > > + assert_pipe_disabled(dev_priv, crtc_state->cpu_transcoder); > > > > /* PLL is protected by panel, make sure we can write it */ > > if (i9xx_has_pps(dev_priv)) > > @@ -1563,7 +1554,7 @@ static void i9xx_disable_pll(const struct > > intel_crtc_state *crtc_state) > > return; > > > > /* Make sure the pipe isn't still relying on us */ > > - assert_pipe_disabled(dev_priv, pipe); > > + assert_pipe_disabled(dev_priv, crtc_state->cpu_transcoder); > > > > I915_WRITE(DPLL(pipe), DPLL_VGA_MODE_DIS); > > POSTING_READ(DPLL(pipe)); > > @@ -1574,7 +1565,7 @@ static void vlv_disable_pll(struct > > drm_i915_private *dev_priv, enum pipe pipe) > > u32 val; > > > > /* Make sure the pipe isn't still relying on us */ > > - assert_pipe_disabled(dev_priv, pipe); > > + assert_pipe_disabled(dev_priv, (enum transcoder)pipe); > > > > val = DPLL_INTEGRATED_REF_CLK_VLV | > > DPLL_REF_CLK_ENABLE_VLV | DPLL_VGA_MODE_DIS; > > @@ -1591,7 +1582,7 @@ static void chv_disable_pll(struct > > drm_i915_private *dev_priv, enum pipe pipe) > > u32 val; > > > > /* Make sure the pipe isn't still relying on us */ > > - assert_pipe_disabled(dev_priv, pipe); > > + assert_pipe_disabled(dev_priv, (enum transcoder)pipe); > > > > val = DPLL_SSC_REF_CLK_CHV | > > DPLL_REF_CLK_ENABLE_VLV | DPLL_VGA_MODE_DIS; > > @@ -4617,7 +4608,7 @@ static void ironlake_fdi_link_train(struct > > intel_crtc *crtc, > > u32 temp, tries; > > > > /* FDI needs bits from pipe first */ > > - assert_pipe_enabled(dev_priv, pipe); > > + assert_pipe_enabled(dev_priv, crtc_state->cpu_transcoder); > > > > /* Train 1: umask FDI RX Interrupt symbol_lock and bit_lock bit > > for train result */ > > @@ -6805,7 +6796,7 @@ static void i9xx_pfit_enable(const struct > > intel_crtc_state *crtc_state) > > * 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, crtc_state->cpu_transcoder); > > > > I915_WRITE(PFIT_PGM_RATIOS, crtc_state->gmch_pfit.pgm_ratios); > > I915_WRITE(PFIT_CONTROL, crtc_state->gmch_pfit.control); > > @@ -7116,7 +7107,7 @@ static void i9xx_pfit_disable(const struct > > intel_crtc_state *old_crtc_state) > > if (!old_crtc_state->gmch_pfit.control) > > return; > > > > - assert_pipe_disabled(dev_priv, crtc->pipe); > > + assert_pipe_disabled(dev_priv, old_crtc_state->cpu_transcoder); > > > > DRM_DEBUG_KMS("disabling pfit, current: 0x%08x\n", > > I915_READ(PFIT_CONTROL)); > > @@ -8128,6 +8119,7 @@ int vlv_force_pll_on(struct drm_i915_private > > *dev_priv, enum pipe pipe, > > return -ENOMEM; > > > > pipe_config->uapi.crtc = &crtc->base; > > + pipe_config->cpu_transcoder = (enum transcoder)pipe; > > pipe_config->pixel_multiplier = 1; > > pipe_config->dpll = *dpll; > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display.h > > b/drivers/gpu/drm/i915/display/intel_display.h > > index d18dc260fe83..f33096876a9a 100644 > > --- a/drivers/gpu/drm/i915/display/intel_display.h > > +++ b/drivers/gpu/drm/i915/display/intel_display.h > > @@ -510,8 +510,6 @@ enum tc_port intel_port_to_tc(struct > > drm_i915_private *dev_priv, > > enum port port); > > int intel_get_pipe_from_crtc_id_ioctl(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); > > u32 intel_crtc_get_vblank_counter(struct intel_crtc *crtc); > > > > int ironlake_get_lanes_required(int target_clock, int link_bw, int > > bpp); > > @@ -613,9 +611,10 @@ 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, bool state); > > +#define assert_pipe_enabled(d, t) assert_pipe(d, t, true) > > +#define assert_pipe_disabled(d, t) assert_pipe(d, t, false) > > Maybe while doing all those already rename it to > assert_transcoder_enabled/disabled()? Perhaps. Although then it's inconsistent with intel_{enable,disable}_pipe() so should maybe rename those as well. I think I'll leave all that for another day. > > Also why not just squash with the previous patch? Why? The two patches are touching two totally different things. > > Other than that: > > Reviewed-by: José Roberto de Souza <jose.souza@intel.com> > > > > > /* Use I915_STATE_WARN(x) and I915_STATE_WARN_ON() (rather than > > WARN() and > > * WARN_ON()) for hw state sanity checks to check for unexpected > > conditions
On Tue, 2019-12-10 at 21:58 +0200, Ville Syrjälä wrote: > On Tue, Dec 10, 2019 at 06:23:36PM +0000, Souza, Jose wrote: > > On Tue, 2019-11-12 at 18:38 +0200, Ville Syrjala wrote: > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > > > In order to eliminate intel_pipe_to_cpu_transcoder() (and its > > > crtc->config usage) let's pass the cpu transcoder to > > > assert_pipe() so we don't have to do the pipe->cpu transcoder > > > lookup on HSW+. > > > > > > On VLV/CHV this can get called during eDP init, which > > > happens before crtc->config->cpu_transcoder is even > > > populated. So currently we're always reading PIPECONF(A) > > > there even if we're trying to check the state of some > > > other pipe. > > > > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > --- > > > drivers/gpu/drm/i915/display/intel_display.c | 36 ++++++++---- > > > ---- > > > ---- > > > drivers/gpu/drm/i915/display/intel_display.h | 9 +++-- > > > 2 files changed, 18 insertions(+), 27 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c > > > b/drivers/gpu/drm/i915/display/intel_display.c > > > index cabd88337822..6d2112f5bdd0 100644 > > > --- a/drivers/gpu/drm/i915/display/intel_display.c > > > +++ b/drivers/gpu/drm/i915/display/intel_display.c > > > @@ -1040,14 +1040,6 @@ bool bxt_find_best_dpll(struct > > > intel_crtc_state *crtc_state, > > > NULL, best_clock); > > > } > > > > > > -enum transcoder intel_pipe_to_cpu_transcoder(struct > > > drm_i915_private > > > *dev_priv, > > > - enum pipe pipe) > > > -{ > > > - struct intel_crtc *crtc = intel_get_crtc_for_pipe(dev_priv, > > > pipe); > > > - > > > - return crtc->config->cpu_transcoder; > > > -} > > > - > > > static bool pipe_scanline_is_moving(struct drm_i915_private > > > *dev_priv, > > > enum pipe pipe) > > > { > > > @@ -1266,11 +1258,9 @@ void assert_panel_unlocked(struct > > > drm_i915_private *dev_priv, enum pipe pipe) > > > } > > > > > > void assert_pipe(struct drm_i915_private *dev_priv, > > > - enum pipe pipe, bool state) > > > + enum transcoder cpu_transcoder, bool state) > > > { > > > bool cur_state; > > > - enum transcoder cpu_transcoder = > > > intel_pipe_to_cpu_transcoder(dev_priv, > > > - p > > > ipe); > > > enum intel_display_power_domain power_domain; > > > intel_wakeref_t wakeref; > > > > > > @@ -1290,8 +1280,9 @@ void assert_pipe(struct drm_i915_private > > > *dev_priv, > > > } > > > > > > I915_STATE_WARN(cur_state != state, > > > - "pipe %c assertion failure (expected %s, current %s)\n", > > > - pipe_name(pipe), onoff(state), > > > onoff(cur_state)); > > > + "transcoder %s assertion failure (expected %s, > > > current %s)\n", > > > + transcoder_name(cpu_transcoder), > > > + onoff(state), onoff(cur_state)); > > > } > > > > > > static void assert_plane(struct intel_plane *plane, bool state) > > > @@ -1418,7 +1409,7 @@ static void vlv_enable_pll(struct > > > intel_crtc > > > *crtc, > > > struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); > > > enum pipe pipe = crtc->pipe; > > > > > > - assert_pipe_disabled(dev_priv, pipe); > > > + assert_pipe_disabled(dev_priv, pipe_config->cpu_transcoder); > > > > > > /* PLL is protected by panel, make sure we can write it */ > > > assert_panel_unlocked(dev_priv, pipe); > > > @@ -1467,7 +1458,7 @@ static void chv_enable_pll(struct > > > intel_crtc > > > *crtc, > > > struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); > > > enum pipe pipe = crtc->pipe; > > > > > > - assert_pipe_disabled(dev_priv, pipe); > > > + assert_pipe_disabled(dev_priv, pipe_config->cpu_transcoder); > > > > > > /* PLL is protected by panel, make sure we can write it */ > > > assert_panel_unlocked(dev_priv, pipe); > > > @@ -1514,7 +1505,7 @@ static void i9xx_enable_pll(struct > > > intel_crtc > > > *crtc, > > > u32 dpll = crtc_state->dpll_hw_state.dpll; > > > int i; > > > > > > - assert_pipe_disabled(dev_priv, crtc->pipe); > > > + assert_pipe_disabled(dev_priv, crtc_state->cpu_transcoder); > > > > > > /* PLL is protected by panel, make sure we can write it */ > > > if (i9xx_has_pps(dev_priv)) > > > @@ -1563,7 +1554,7 @@ static void i9xx_disable_pll(const struct > > > intel_crtc_state *crtc_state) > > > return; > > > > > > /* Make sure the pipe isn't still relying on us */ > > > - assert_pipe_disabled(dev_priv, pipe); > > > + assert_pipe_disabled(dev_priv, crtc_state->cpu_transcoder); > > > > > > I915_WRITE(DPLL(pipe), DPLL_VGA_MODE_DIS); > > > POSTING_READ(DPLL(pipe)); > > > @@ -1574,7 +1565,7 @@ static void vlv_disable_pll(struct > > > drm_i915_private *dev_priv, enum pipe pipe) > > > u32 val; > > > > > > /* Make sure the pipe isn't still relying on us */ > > > - assert_pipe_disabled(dev_priv, pipe); > > > + assert_pipe_disabled(dev_priv, (enum transcoder)pipe); > > > > > > val = DPLL_INTEGRATED_REF_CLK_VLV | > > > DPLL_REF_CLK_ENABLE_VLV | DPLL_VGA_MODE_DIS; > > > @@ -1591,7 +1582,7 @@ static void chv_disable_pll(struct > > > drm_i915_private *dev_priv, enum pipe pipe) > > > u32 val; > > > > > > /* Make sure the pipe isn't still relying on us */ > > > - assert_pipe_disabled(dev_priv, pipe); > > > + assert_pipe_disabled(dev_priv, (enum transcoder)pipe); > > > > > > val = DPLL_SSC_REF_CLK_CHV | > > > DPLL_REF_CLK_ENABLE_VLV | DPLL_VGA_MODE_DIS; > > > @@ -4617,7 +4608,7 @@ static void ironlake_fdi_link_train(struct > > > intel_crtc *crtc, > > > u32 temp, tries; > > > > > > /* FDI needs bits from pipe first */ > > > - assert_pipe_enabled(dev_priv, pipe); > > > + assert_pipe_enabled(dev_priv, crtc_state->cpu_transcoder); > > > > > > /* Train 1: umask FDI RX Interrupt symbol_lock and bit_lock bit > > > for train result */ > > > @@ -6805,7 +6796,7 @@ static void i9xx_pfit_enable(const struct > > > intel_crtc_state *crtc_state) > > > * 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, crtc_state->cpu_transcoder); > > > > > > I915_WRITE(PFIT_PGM_RATIOS, crtc_state->gmch_pfit.pgm_ratios); > > > I915_WRITE(PFIT_CONTROL, crtc_state->gmch_pfit.control); > > > @@ -7116,7 +7107,7 @@ static void i9xx_pfit_disable(const struct > > > intel_crtc_state *old_crtc_state) > > > if (!old_crtc_state->gmch_pfit.control) > > > return; > > > > > > - assert_pipe_disabled(dev_priv, crtc->pipe); > > > + assert_pipe_disabled(dev_priv, old_crtc_state->cpu_transcoder); > > > > > > DRM_DEBUG_KMS("disabling pfit, current: 0x%08x\n", > > > I915_READ(PFIT_CONTROL)); > > > @@ -8128,6 +8119,7 @@ int vlv_force_pll_on(struct > > > drm_i915_private > > > *dev_priv, enum pipe pipe, > > > return -ENOMEM; > > > > > > pipe_config->uapi.crtc = &crtc->base; > > > + pipe_config->cpu_transcoder = (enum transcoder)pipe; > > > pipe_config->pixel_multiplier = 1; > > > pipe_config->dpll = *dpll; > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display.h > > > b/drivers/gpu/drm/i915/display/intel_display.h > > > index d18dc260fe83..f33096876a9a 100644 > > > --- a/drivers/gpu/drm/i915/display/intel_display.h > > > +++ b/drivers/gpu/drm/i915/display/intel_display.h > > > @@ -510,8 +510,6 @@ enum tc_port intel_port_to_tc(struct > > > drm_i915_private *dev_priv, > > > enum port port); > > > int intel_get_pipe_from_crtc_id_ioctl(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); > > > u32 intel_crtc_get_vblank_counter(struct intel_crtc *crtc); > > > > > > int ironlake_get_lanes_required(int target_clock, int link_bw, > > > int > > > bpp); > > > @@ -613,9 +611,10 @@ 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, bool state); > > > +#define assert_pipe_enabled(d, t) assert_pipe(d, t, true) > > > +#define assert_pipe_disabled(d, t) assert_pipe(d, t, false) > > > > Maybe while doing all those already rename it to > > assert_transcoder_enabled/disabled()? > > Perhaps. Although then it's inconsistent with > intel_{enable,disable}_pipe() so should maybe rename those > as well. I think I'll leave all that for another day. Okay > > > Also why not just squash with the previous patch? > > Why? The two patches are touching two totally different things. Both are getting rid of intel_pipe_to_cpu_transcoder()... Anyways now that I'm sure the previous one is not wrong for HSW both looks good. > > > Other than that: > > > > Reviewed-by: José Roberto de Souza <jose.souza@intel.com> > > > > > > > > /* Use I915_STATE_WARN(x) and I915_STATE_WARN_ON() (rather than > > > WARN() and > > > * WARN_ON()) for hw state sanity checks to check for unexpected > > > conditions
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index cabd88337822..6d2112f5bdd0 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -1040,14 +1040,6 @@ bool bxt_find_best_dpll(struct intel_crtc_state *crtc_state, NULL, best_clock); } -enum transcoder intel_pipe_to_cpu_transcoder(struct drm_i915_private *dev_priv, - enum pipe pipe) -{ - struct intel_crtc *crtc = intel_get_crtc_for_pipe(dev_priv, pipe); - - return crtc->config->cpu_transcoder; -} - static bool pipe_scanline_is_moving(struct drm_i915_private *dev_priv, enum pipe pipe) { @@ -1266,11 +1258,9 @@ void assert_panel_unlocked(struct drm_i915_private *dev_priv, enum pipe pipe) } void assert_pipe(struct drm_i915_private *dev_priv, - enum pipe pipe, bool state) + enum transcoder cpu_transcoder, bool state) { bool cur_state; - enum transcoder cpu_transcoder = intel_pipe_to_cpu_transcoder(dev_priv, - pipe); enum intel_display_power_domain power_domain; intel_wakeref_t wakeref; @@ -1290,8 +1280,9 @@ void assert_pipe(struct drm_i915_private *dev_priv, } I915_STATE_WARN(cur_state != state, - "pipe %c assertion failure (expected %s, current %s)\n", - pipe_name(pipe), onoff(state), onoff(cur_state)); + "transcoder %s assertion failure (expected %s, current %s)\n", + transcoder_name(cpu_transcoder), + onoff(state), onoff(cur_state)); } static void assert_plane(struct intel_plane *plane, bool state) @@ -1418,7 +1409,7 @@ static void vlv_enable_pll(struct intel_crtc *crtc, struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); enum pipe pipe = crtc->pipe; - assert_pipe_disabled(dev_priv, pipe); + assert_pipe_disabled(dev_priv, pipe_config->cpu_transcoder); /* PLL is protected by panel, make sure we can write it */ assert_panel_unlocked(dev_priv, pipe); @@ -1467,7 +1458,7 @@ static void chv_enable_pll(struct intel_crtc *crtc, struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); enum pipe pipe = crtc->pipe; - assert_pipe_disabled(dev_priv, pipe); + assert_pipe_disabled(dev_priv, pipe_config->cpu_transcoder); /* PLL is protected by panel, make sure we can write it */ assert_panel_unlocked(dev_priv, pipe); @@ -1514,7 +1505,7 @@ static void i9xx_enable_pll(struct intel_crtc *crtc, u32 dpll = crtc_state->dpll_hw_state.dpll; int i; - assert_pipe_disabled(dev_priv, crtc->pipe); + assert_pipe_disabled(dev_priv, crtc_state->cpu_transcoder); /* PLL is protected by panel, make sure we can write it */ if (i9xx_has_pps(dev_priv)) @@ -1563,7 +1554,7 @@ static void i9xx_disable_pll(const struct intel_crtc_state *crtc_state) return; /* Make sure the pipe isn't still relying on us */ - assert_pipe_disabled(dev_priv, pipe); + assert_pipe_disabled(dev_priv, crtc_state->cpu_transcoder); I915_WRITE(DPLL(pipe), DPLL_VGA_MODE_DIS); POSTING_READ(DPLL(pipe)); @@ -1574,7 +1565,7 @@ static void vlv_disable_pll(struct drm_i915_private *dev_priv, enum pipe pipe) u32 val; /* Make sure the pipe isn't still relying on us */ - assert_pipe_disabled(dev_priv, pipe); + assert_pipe_disabled(dev_priv, (enum transcoder)pipe); val = DPLL_INTEGRATED_REF_CLK_VLV | DPLL_REF_CLK_ENABLE_VLV | DPLL_VGA_MODE_DIS; @@ -1591,7 +1582,7 @@ static void chv_disable_pll(struct drm_i915_private *dev_priv, enum pipe pipe) u32 val; /* Make sure the pipe isn't still relying on us */ - assert_pipe_disabled(dev_priv, pipe); + assert_pipe_disabled(dev_priv, (enum transcoder)pipe); val = DPLL_SSC_REF_CLK_CHV | DPLL_REF_CLK_ENABLE_VLV | DPLL_VGA_MODE_DIS; @@ -4617,7 +4608,7 @@ static void ironlake_fdi_link_train(struct intel_crtc *crtc, u32 temp, tries; /* FDI needs bits from pipe first */ - assert_pipe_enabled(dev_priv, pipe); + assert_pipe_enabled(dev_priv, crtc_state->cpu_transcoder); /* Train 1: umask FDI RX Interrupt symbol_lock and bit_lock bit for train result */ @@ -6805,7 +6796,7 @@ static void i9xx_pfit_enable(const struct intel_crtc_state *crtc_state) * 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, crtc_state->cpu_transcoder); I915_WRITE(PFIT_PGM_RATIOS, crtc_state->gmch_pfit.pgm_ratios); I915_WRITE(PFIT_CONTROL, crtc_state->gmch_pfit.control); @@ -7116,7 +7107,7 @@ static void i9xx_pfit_disable(const struct intel_crtc_state *old_crtc_state) if (!old_crtc_state->gmch_pfit.control) return; - assert_pipe_disabled(dev_priv, crtc->pipe); + assert_pipe_disabled(dev_priv, old_crtc_state->cpu_transcoder); DRM_DEBUG_KMS("disabling pfit, current: 0x%08x\n", I915_READ(PFIT_CONTROL)); @@ -8128,6 +8119,7 @@ int vlv_force_pll_on(struct drm_i915_private *dev_priv, enum pipe pipe, return -ENOMEM; pipe_config->uapi.crtc = &crtc->base; + pipe_config->cpu_transcoder = (enum transcoder)pipe; pipe_config->pixel_multiplier = 1; pipe_config->dpll = *dpll; diff --git a/drivers/gpu/drm/i915/display/intel_display.h b/drivers/gpu/drm/i915/display/intel_display.h index d18dc260fe83..f33096876a9a 100644 --- a/drivers/gpu/drm/i915/display/intel_display.h +++ b/drivers/gpu/drm/i915/display/intel_display.h @@ -510,8 +510,6 @@ enum tc_port intel_port_to_tc(struct drm_i915_private *dev_priv, enum port port); int intel_get_pipe_from_crtc_id_ioctl(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); u32 intel_crtc_get_vblank_counter(struct intel_crtc *crtc); int ironlake_get_lanes_required(int target_clock, int link_bw, int bpp); @@ -613,9 +611,10 @@ 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, bool state); +#define assert_pipe_enabled(d, t) assert_pipe(d, t, true) +#define assert_pipe_disabled(d, t) assert_pipe(d, t, false) /* Use I915_STATE_WARN(x) and I915_STATE_WARN_ON() (rather than WARN() and * WARN_ON()) for hw state sanity checks to check for unexpected conditions