On Fri, 18 May 2018, Ville Syrjala <ville.syrjala@xxxxxxxxxxxxxxx> wrote:
> From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
>
> Clean up the DP pipe select bits. To make the whole situation a bit
> less ugly we'll start to share the same code between .get_hw_state(),
> the port state asserts, and the VLV power sequencer code.
>
> v2: Return PIPE_A for cpt/ppt when the port isn't selected by
> any transcoder. Returning INVALID_PIPE explodes *somewhere*
> on some machines (can't immediately see where though). This
> now matches the old behaviour.
> v3: Order the defines shift,mask,value (Jani)
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
I could've bikeshedded about some naming here, but I don't want to read
this particular patch again, so
Reviewed-by: Jani Nikula <jani.nikula@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/i915_reg.h | 24 +++-----
> drivers/gpu/drm/i915/intel_display.c | 48 +++++----------
> drivers/gpu/drm/i915/intel_dp.c | 113 ++++++++++++++++++++---------------
> drivers/gpu/drm/i915/intel_drv.h | 3 +
> 4 files changed, 92 insertions(+), 96 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 5a103496423c..b888da96caf7 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -5182,10 +5182,15 @@ enum {
> #define CHV_DP_D _MMIO(VLV_DISPLAY_BASE + 0x64300)
>
> #define DP_PORT_EN (1 << 31)
> -#define DP_PIPEB_SELECT (1 << 30)
> -#define DP_PIPE_MASK (1 << 30)
> -#define DP_PIPE_SELECT_CHV(pipe) ((pipe) << 16)
> -#define DP_PIPE_MASK_CHV (3 << 16)
> +#define DP_PIPE_SEL_SHIFT 30
> +#define DP_PIPE_SEL_MASK (1 << 30)
> +#define DP_PIPE_SEL(pipe) ((pipe) << 30)
> +#define DP_PIPE_SEL_SHIFT_IVB 29
> +#define DP_PIPE_SEL_MASK_IVB (3 << 29)
> +#define DP_PIPE_SEL_IVB(pipe) ((pipe) << 29)
> +#define DP_PIPE_SEL_SHIFT_CHV 16
> +#define DP_PIPE_SEL_MASK_CHV (3 << 16)
> +#define DP_PIPE_SEL_CHV(pipe) ((pipe) << 16)
>
> /* Link training mode - select a suitable mode for each stage */
> #define DP_LINK_TRAIN_PAT_1 (0 << 28)
> @@ -7872,16 +7877,6 @@ enum {
> #define PCH_DP_AUX_CH_DATA(aux_ch, i) _MMIO(_PORT((aux_ch) - AUX_CH_B, _PCH_DPB_AUX_CH_DATA1, _PCH_DPC_AUX_CH_DATA1) + (i) * 4) /* 5 registers */
>
> /* CPT */
> -#define PORT_TRANS_A_SEL_CPT 0
> -#define PORT_TRANS_B_SEL_CPT (1<<29)
> -#define PORT_TRANS_C_SEL_CPT (2<<29)
> -#define PORT_TRANS_SEL_MASK (3<<29)
> -#define PORT_TRANS_SEL_CPT(pipe) ((pipe) << 29)
> -#define PORT_TO_PIPE(val) (((val) & (1<<30)) >> 30)
> -#define PORT_TO_PIPE_CPT(val) (((val) & PORT_TRANS_SEL_MASK) >> 29)
> -#define SDVO_PORT_TO_PIPE_CHV(val) (((val) & (3<<24)) >> 24)
> -#define DP_PORT_TO_PIPE_CHV(val) (((val) & (3<<16)) >> 16)
> -
> #define _TRANS_DP_CTL_A 0xe0300
> #define _TRANS_DP_CTL_B 0xe1300
> #define _TRANS_DP_CTL_C 0xe2300
> @@ -7890,7 +7885,6 @@ enum {
> #define TRANS_DP_PORT_SEL_MASK (3 << 29)
> #define TRANS_DP_PORT_SEL_NONE (3 << 29)
> #define TRANS_DP_PORT_SEL(port) (((port) - PORT_B) << 29)
> -#define TRANS_DP_PIPE_TO_PORT(val) ((((val) & TRANS_DP_PORT_SEL_MASK) >> 29) + PORT_B)
> #define TRANS_DP_AUDIO_ONLY (1<<26)
> #define TRANS_DP_ENH_FRAMING (1<<18)
> #define TRANS_DP_8BPC (0<<9)
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index d36dea568b82..d397febb5a7f 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -1303,38 +1303,22 @@ void assert_pch_transcoder_disabled(struct drm_i915_private *dev_priv,
> pipe_name(pipe));
> }
>
> -static bool dp_pipe_enabled(struct drm_i915_private *dev_priv,
> - enum pipe pipe, u32 port_sel, u32 val)
> -{
> - if ((val & DP_PORT_EN) == 0)
> - return false;
> -
> - if (HAS_PCH_CPT(dev_priv)) {
> - u32 trans_dp_ctl = I915_READ(TRANS_DP_CTL(pipe));
> - if ((trans_dp_ctl & TRANS_DP_PORT_SEL_MASK) != port_sel)
> - return false;
> - } else if (IS_CHERRYVIEW(dev_priv)) {
> - if ((val & DP_PIPE_MASK_CHV) != DP_PIPE_SELECT_CHV(pipe))
> - return false;
> - } else {
> - if ((val & DP_PIPE_MASK) != (pipe << 30))
> - return false;
> - }
> - return true;
> -}
> -
> static void assert_pch_dp_disabled(struct drm_i915_private *dev_priv,
> - enum pipe pipe, i915_reg_t reg,
> - u32 port_sel)
> + enum pipe pipe, enum port port,
> + i915_reg_t dp_reg)
> {
> - u32 val = I915_READ(reg);
> - I915_STATE_WARN(dp_pipe_enabled(dev_priv, pipe, port_sel, val),
> - "PCH DP (0x%08x) enabled on transcoder %c, should be disabled\n",
> - i915_mmio_reg_offset(reg), pipe_name(pipe));
> + enum pipe port_pipe;
> + bool state;
>
> - I915_STATE_WARN(HAS_PCH_IBX(dev_priv) && (val & DP_PORT_EN) == 0
> - && (val & DP_PIPEB_SELECT),
> - "IBX PCH dp port still using transcoder B\n");
> + state = intel_dp_port_enabled(dev_priv, dp_reg, port, &port_pipe);
> +
> + I915_STATE_WARN(state && port_pipe == pipe,
> + "PCH DP %c enabled on transcoder %c, should be disabled\n",
> + port_name(port), pipe_name(pipe));
> +
> + I915_STATE_WARN(HAS_PCH_IBX(dev_priv) && !state && port_pipe == PIPE_B,
> + "IBX PCH DP %c still using transcoder B\n",
> + port_name(port));
> }
>
> static void assert_pch_hdmi_disabled(struct drm_i915_private *dev_priv,
> @@ -1360,9 +1344,9 @@ static void assert_pch_ports_disabled(struct drm_i915_private *dev_priv,
> {
> enum pipe port_pipe;
>
> - assert_pch_dp_disabled(dev_priv, pipe, PCH_DP_B, TRANS_DP_PORT_SEL(PORT_B));
> - assert_pch_dp_disabled(dev_priv, pipe, PCH_DP_C, TRANS_DP_PORT_SEL(PORT_C));
> - assert_pch_dp_disabled(dev_priv, pipe, PCH_DP_D, TRANS_DP_PORT_SEL(PORT_D));
> + assert_pch_dp_disabled(dev_priv, pipe, PORT_B, PCH_DP_B);
> + assert_pch_dp_disabled(dev_priv, pipe, PORT_C, PCH_DP_C);
> + assert_pch_dp_disabled(dev_priv, pipe, PORT_D, PCH_DP_D);
>
> I915_STATE_WARN(intel_crt_port_enabled(dev_priv, PCH_ADPA, &port_pipe) &&
> port_pipe == pipe,
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 102070940095..aab48283f155 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -529,9 +529,9 @@ vlv_power_sequencer_kick(struct intel_dp *intel_dp)
> DP |= DP_LINK_TRAIN_PAT_1;
>
> if (IS_CHERRYVIEW(dev_priv))
> - DP |= DP_PIPE_SELECT_CHV(pipe);
> - else if (pipe == PIPE_B)
> - DP |= DP_PIPEB_SELECT;
> + DP |= DP_PIPE_SEL_CHV(pipe);
> + else
> + DP |= DP_PIPE_SEL(pipe);
>
> pll_enabled = I915_READ(DPLL(pipe)) & DPLL_VCO_ENABLE;
>
> @@ -1999,7 +1999,7 @@ static void intel_dp_prepare(struct intel_encoder *encoder,
> if (drm_dp_enhanced_frame_cap(intel_dp->dpcd))
> intel_dp->DP |= DP_ENHANCED_FRAMING;
>
> - intel_dp->DP |= crtc->pipe << 29;
> + intel_dp->DP |= DP_PIPE_SEL_IVB(crtc->pipe);
> } else if (HAS_PCH_CPT(dev_priv) && port != PORT_A) {
> u32 trans_dp;
>
> @@ -2025,9 +2025,9 @@ static void intel_dp_prepare(struct intel_encoder *encoder,
> intel_dp->DP |= DP_ENHANCED_FRAMING;
>
> if (IS_CHERRYVIEW(dev_priv))
> - intel_dp->DP |= DP_PIPE_SELECT_CHV(crtc->pipe);
> - else if (crtc->pipe == PIPE_B)
> - intel_dp->DP |= DP_PIPEB_SELECT;
> + intel_dp->DP |= DP_PIPE_SEL_CHV(crtc->pipe);
> + else
> + intel_dp->DP |= DP_PIPE_SEL(crtc->pipe);
> }
> }
>
> @@ -2649,52 +2649,66 @@ void intel_dp_sink_dpms(struct intel_dp *intel_dp, int mode)
> mode == DRM_MODE_DPMS_ON ? "enable" : "disable");
> }
>
> +static bool cpt_dp_port_selected(struct drm_i915_private *dev_priv,
> + enum port port, enum pipe *pipe)
> +{
> + enum pipe p;
> +
> + for_each_pipe(dev_priv, p) {
> + u32 val = I915_READ(TRANS_DP_CTL(p));
> +
> + if ((val & TRANS_DP_PORT_SEL_MASK) == TRANS_DP_PORT_SEL(port)) {
> + *pipe = p;
> + return true;
> + }
> + }
> +
> + DRM_DEBUG_KMS("No pipe for DP port %c found\n", port_name(port));
> +
> + /* must initialize pipe to something for the asserts */
> + *pipe = PIPE_A;
> +
> + return false;
> +}
> +
> +bool intel_dp_port_enabled(struct drm_i915_private *dev_priv,
> + i915_reg_t dp_reg, enum port port,
> + enum pipe *pipe)
> +{
> + bool ret;
> + u32 val;
> +
> + val = I915_READ(dp_reg);
> +
> + ret = val & DP_PORT_EN;
> +
> + /* asserts want to know the pipe even if the port is disabled */
> + if (IS_IVYBRIDGE(dev_priv) && port == PORT_A)
> + *pipe = (val & DP_PIPE_SEL_MASK_IVB) >> DP_PIPE_SEL_SHIFT_IVB;
> + else if (HAS_PCH_CPT(dev_priv) && port != PORT_A)
> + ret &= cpt_dp_port_selected(dev_priv, port, pipe);
> + else if (IS_CHERRYVIEW(dev_priv))
> + *pipe = (val & DP_PIPE_SEL_MASK_CHV) >> DP_PIPE_SEL_SHIFT_CHV;
> + else
> + *pipe = (val & DP_PIPE_SEL_MASK) >> DP_PIPE_SEL_SHIFT;
> +
> + return ret;
> +}
> +
> static bool intel_dp_get_hw_state(struct intel_encoder *encoder,
> enum pipe *pipe)
> {
> struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
> - enum port port = encoder->port;
> - u32 tmp;
> bool ret;
>
> if (!intel_display_power_get_if_enabled(dev_priv,
> encoder->power_domain))
> return false;
>
> - ret = false;
> + ret = intel_dp_port_enabled(dev_priv, intel_dp->output_reg,
> + encoder->port, pipe);
>
> - tmp = I915_READ(intel_dp->output_reg);
> -
> - if (!(tmp & DP_PORT_EN))
> - goto out;
> -
> - if (IS_IVYBRIDGE(dev_priv) && port == PORT_A) {
> - *pipe = PORT_TO_PIPE_CPT(tmp);
> - } else if (HAS_PCH_CPT(dev_priv) && port != PORT_A) {
> - enum pipe p;
> -
> - for_each_pipe(dev_priv, p) {
> - u32 trans_dp = I915_READ(TRANS_DP_CTL(p));
> - if (TRANS_DP_PIPE_TO_PORT(trans_dp) == port) {
> - *pipe = p;
> - ret = true;
> -
> - goto out;
> - }
> - }
> -
> - DRM_DEBUG_KMS("No pipe for dp port 0x%x found\n",
> - i915_mmio_reg_offset(intel_dp->output_reg));
> - } else if (IS_CHERRYVIEW(dev_priv)) {
> - *pipe = DP_PORT_TO_PIPE_CHV(tmp);
> - } else {
> - *pipe = PORT_TO_PIPE(tmp);
> - }
> -
> - ret = true;
> -
> -out:
> intel_display_power_put(dev_priv, encoder->power_domain);
>
> return ret;
> @@ -3684,8 +3698,9 @@ intel_dp_link_down(struct intel_encoder *encoder,
> intel_set_pch_fifo_underrun_reporting(dev_priv, PIPE_A, false);
>
> /* always enable with pattern 1 (as per spec) */
> - DP &= ~(DP_PIPEB_SELECT | DP_LINK_TRAIN_MASK);
> - DP |= DP_PORT_EN | DP_LINK_TRAIN_PAT_1;
> + DP &= ~(DP_PIPE_SEL_MASK | DP_LINK_TRAIN_MASK);
> + DP |= DP_PORT_EN | DP_PIPE_SEL(PIPE_A) |
> + DP_LINK_TRAIN_PAT_1;
> I915_WRITE(intel_dp->output_reg, DP);
> POSTING_READ(intel_dp->output_reg);
>
> @@ -5320,14 +5335,14 @@ static void intel_edp_panel_vdd_sanitize(struct intel_dp *intel_dp)
> static enum pipe vlv_active_pipe(struct intel_dp *intel_dp)
> {
> struct drm_i915_private *dev_priv = to_i915(intel_dp_to_dev(intel_dp));
> + struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
> + enum pipe pipe;
>
> - if ((intel_dp->DP & DP_PORT_EN) == 0)
> - return INVALID_PIPE;
> + if (intel_dp_port_enabled(dev_priv, intel_dp->output_reg,
> + encoder->port, &pipe))
> + return pipe;
>
> - if (IS_CHERRYVIEW(dev_priv))
> - return DP_PORT_TO_PIPE_CHV(intel_dp->DP);
> - else
> - return PORT_TO_PIPE(intel_dp->DP);
> + return INVALID_PIPE;
> }
>
> void intel_dp_encoder_reset(struct drm_encoder *encoder)
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index a827a2245c18..dc7f81e1e20a 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1647,6 +1647,9 @@ void intel_csr_ucode_suspend(struct drm_i915_private *);
> void intel_csr_ucode_resume(struct drm_i915_private *);
>
> /* intel_dp.c */
> +bool intel_dp_port_enabled(struct drm_i915_private *dev_priv,
> + i915_reg_t dp_reg, enum port port,
> + enum pipe *pipe);
> bool intel_dp_init(struct drm_i915_private *dev_priv, i915_reg_t output_reg,
> enum port port);
> bool intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
--
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
