Re: [PATCH v3 14/23] drm/i915/tgl: move DP_TP_* to transcoder

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Aug 23, 2019 at 03:25:18PM +0300, Ville Syrjälä wrote:
> On Fri, Aug 23, 2019 at 01:20:46AM -0700, Lucas De Marchi wrote:
> > Gen 12 onwards moves the DP_TP_* registers to be transcoder-based rather
> > than port-based. This add the new register address and changes the
> > functions that are used with DDI on gen 12 to use the new registers. On
> > MST the master transcoder is the one to be used.
> > 
> > Cc: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
> > Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> > Cc: José Roberto de Souza <jose.souza@xxxxxxxxx>
> > Signed-off-by: Lucas De Marchi <lucas.demarchi@xxxxxxxxx>
> > ---
> >  drivers/gpu/drm/i915/display/intel_ddi.c      | 42 ++++++++----
> >  .../drm/i915/display/intel_display_types.h    |  1 +
> >  drivers/gpu/drm/i915/display/intel_dp.c       | 66 +++++++++++++++++--
> >  drivers/gpu/drm/i915/display/intel_dp.h       |  9 +++
> >  drivers/gpu/drm/i915/display/intel_dp_mst.c   | 20 ++++--
> >  drivers/gpu/drm/i915/i915_reg.h               |  4 ++
> >  6 files changed, 119 insertions(+), 23 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> > index f4cb6bd74421..3eb73dbaf9fd 100644
> > --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> > @@ -3136,17 +3136,22 @@ static void intel_ddi_enable_fec(struct intel_encoder *encoder,
> >  				 const struct intel_crtc_state *crtc_state)
> >  {
> >  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> > +	enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
> >  	enum port port = encoder->port;
> > +	i915_reg_t ctl, status;
> >  	u32 val;
> >  
> >  	if (!crtc_state->fec_enable)
> >  		return;
> >  
> > -	val = I915_READ(DP_TP_CTL(port));
> > +	ctl = intel_dp_tp_ctl_reg(dev_priv, cpu_transcoder, port);
> > +	status = intel_dp_tp_status_reg(dev_priv, cpu_transcoder, port);
> > +
> > +	val = I915_READ(ctl);
> >  	val |= DP_TP_CTL_FEC_ENABLE;
> > -	I915_WRITE(DP_TP_CTL(port), val);
> > +	I915_WRITE(ctl, val);
> >  
> > -	if (intel_de_wait_for_set(dev_priv, DP_TP_STATUS(port),
> > +	if (intel_de_wait_for_set(dev_priv, status,
> >  				  DP_TP_STATUS_FEC_ENABLE_LIVE, 1))
> >  		DRM_ERROR("Timed out waiting for FEC Enable Status\n");
> >  }
> > @@ -3155,16 +3160,19 @@ static void intel_ddi_disable_fec_state(struct intel_encoder *encoder,
> >  					const struct intel_crtc_state *crtc_state)
> >  {
> >  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> > +	enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
> >  	enum port port = encoder->port;
> > +	i915_reg_t ctl;
> >  	u32 val;
> >  
> >  	if (!crtc_state->fec_enable)
> >  		return;
> >  
> > -	val = I915_READ(DP_TP_CTL(port));
> > +	ctl = intel_dp_tp_ctl_reg(dev_priv, cpu_transcoder, port);
> > +	val = I915_READ(ctl);
> >  	val &= ~DP_TP_CTL_FEC_ENABLE;
> > -	I915_WRITE(DP_TP_CTL(port), val);
> > -	POSTING_READ(DP_TP_CTL(port));
> > +	I915_WRITE(ctl, val);
> > +	POSTING_READ(ctl);
> >  }
> >  
> >  static void intel_ddi_pre_enable_dp(struct intel_encoder *encoder,
> > @@ -3326,7 +3334,9 @@ static void intel_disable_ddi_buf(struct intel_encoder *encoder,
> >  				  const struct intel_crtc_state *crtc_state)
> >  {
> >  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> > +	enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
> >  	enum port port = encoder->port;
> > +	i915_reg_t ctl;
> >  	bool wait = false;
> >  	u32 val;
> >  
> > @@ -3337,10 +3347,11 @@ static void intel_disable_ddi_buf(struct intel_encoder *encoder,
> >  		wait = true;
> >  	}
> >  
> > -	val = I915_READ(DP_TP_CTL(port));
> > +	ctl = intel_dp_tp_ctl_reg(dev_priv, cpu_transcoder, port);
> > +	val = I915_READ(ctl);
> >  	val &= ~(DP_TP_CTL_ENABLE | DP_TP_CTL_LINK_TRAIN_MASK);
> >  	val |= DP_TP_CTL_LINK_TRAIN_PAT1;
> > -	I915_WRITE(DP_TP_CTL(port), val);
> > +	I915_WRITE(ctl, val);
> >  
> >  	/* Disable FEC in DP Sink */
> >  	intel_ddi_disable_fec_state(encoder, crtc_state);
> > @@ -3765,10 +3776,13 @@ static void intel_ddi_prepare_link_retrain(struct intel_dp *intel_dp)
> >  	struct drm_i915_private *dev_priv =
> >  		to_i915(intel_dig_port->base.base.dev);
> >  	enum port port = intel_dig_port->base.port;
> > +	i915_reg_t ctl;
> >  	u32 val;
> >  	bool wait = false;
> > +	enum transcoder cpu_transcoder = intel_dp_get_transcoder(intel_dp);
> >  
> > -	if (I915_READ(DP_TP_CTL(port)) & DP_TP_CTL_ENABLE) {
> > +	ctl = intel_dp_tp_ctl_reg(dev_priv, cpu_transcoder, port);
> > +	if (I915_READ(ctl) & DP_TP_CTL_ENABLE) {
> >  		val = I915_READ(DDI_BUF_CTL(port));
> >  		if (val & DDI_BUF_CTL_ENABLE) {
> >  			val &= ~DDI_BUF_CTL_ENABLE;
> > @@ -3776,11 +3790,11 @@ static void intel_ddi_prepare_link_retrain(struct intel_dp *intel_dp)
> >  			wait = true;
> >  		}
> >  
> > -		val = I915_READ(DP_TP_CTL(port));
> > +		val = I915_READ(ctl);
> >  		val &= ~(DP_TP_CTL_ENABLE | DP_TP_CTL_LINK_TRAIN_MASK);
> >  		val |= DP_TP_CTL_LINK_TRAIN_PAT1;
> > -		I915_WRITE(DP_TP_CTL(port), val);
> > -		POSTING_READ(DP_TP_CTL(port));
> > +		I915_WRITE(ctl, val);
> > +		POSTING_READ(ctl);
> >  
> >  		if (wait)
> >  			intel_wait_ddi_buf_idle(dev_priv, port);
> > @@ -3795,8 +3809,8 @@ static void intel_ddi_prepare_link_retrain(struct intel_dp *intel_dp)
> >  		if (drm_dp_enhanced_frame_cap(intel_dp->dpcd))
> >  			val |= DP_TP_CTL_ENHANCED_FRAME_ENABLE;
> >  	}
> > -	I915_WRITE(DP_TP_CTL(port), val);
> > -	POSTING_READ(DP_TP_CTL(port));
> > +	I915_WRITE(ctl, val);
> > +	POSTING_READ(ctl);
> >  
> >  	intel_dp->DP |= DDI_BUF_CTL_ENABLE;
> >  	I915_WRITE(DDI_BUF_CTL(port), intel_dp->DP);
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> > index d84a66459daf..a87fa06994ad 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> > @@ -1214,6 +1214,7 @@ struct intel_dp {
> >  	bool can_mst; /* this port supports mst */
> >  	bool is_mst;
> >  	int active_mst_links;
> > +	enum transcoder mst_master_trans; /* Only valid on TGL+ */

Hmm. So maybe just s/mst_master_trans/dp_tp_transcoder/ etc. and set
this unconditionally in eg. intel_dp_set_link_params().

> >  	/* connector directly attached - won't be use for modeset in mst world */
> >  	struct intel_connector *attached_connector;
> >  
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> > index 23908da1cd5d..943392faaea2 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > @@ -3209,6 +3209,51 @@ static void chv_post_disable_dp(struct intel_encoder *encoder,
> >  	vlv_dpio_put(dev_priv);
> >  }
> >  
> > +i915_reg_t intel_dp_tp_ctl_reg(const struct drm_i915_private *dev_priv,
> > +			       enum transcoder cpu_transcoder,
> > +			       enum port port)
> > +{
> > +	if (INTEL_GEN(dev_priv) >= 12) {
> > +		WARN_ON(cpu_transcoder == TRANSCODER_INVALID);
> > +		return TGL_DP_TP_CTL(cpu_transcoder);
> > +	} else {
> > +		return DP_TP_CTL(port);
> > +	}
> > +}
> > +
> > +i915_reg_t intel_dp_tp_status_reg(const struct drm_i915_private *dev_priv,
> > +				  enum transcoder cpu_transcoder,
> > +				  enum port port)
> > +{
> > +	if (INTEL_GEN(dev_priv) >= 12) {
> > +		WARN_ON(cpu_transcoder == TRANSCODER_INVALID);
> > +		return TGL_DP_TP_STATUS(cpu_transcoder);
> > +	} else {
> > +		return DP_TP_STATUS(port);
> > +	}
> > +}
> > +
> > +/*
> > + * Return the transcoder that this intel_dp port is driven.
> > + * When in MST mode it will return the master transcoder of the MST so do not
> > + * use it when reading or writing registers in the slave transcoders.
> > + */
> > +enum transcoder intel_dp_get_transcoder(struct intel_dp *intel_dp)
> > +{
> > +	struct intel_connector *connector;
> > +	struct drm_connector_state *conn_state;
> > +	struct intel_crtc_state *crtc_state;
> > +
> > +	if (intel_dp->is_mst)
> > +		return intel_dp->mst_master_trans;
> > +
> > +	connector = intel_dp->attached_connector;
> > +	conn_state = connector->base.state;
> > +	crtc_state = to_intel_crtc_state(conn_state->crtc->state);
> 
> That's illegal. Ideally we'd plumb the crtc state into the link training
> stuff, but IIRC last time I tried that we still had at least one case
> where that wasn't trivial. I can't remember right now where that case
> was though.
> 
> I guess the alternative is to continue duplicating things in intel_dp.
> 
> > +
> > +	return crtc_state->cpu_transcoder;
> > +}
> > +
> >  static void
> >  _intel_dp_set_link_train(struct intel_dp *intel_dp,
> >  			 u32 *DP,
> > @@ -3224,8 +3269,13 @@ _intel_dp_set_link_train(struct intel_dp *intel_dp,
> >  			      dp_train_pat & train_pat_mask);
> >  
> >  	if (HAS_DDI(dev_priv)) {
> > -		u32 temp = I915_READ(DP_TP_CTL(port));
> > +		enum transcoder cpu_transcoder;
> > +		i915_reg_t ctl;
> > +		u32 temp;
> >  
> > +		cpu_transcoder = intel_dp_get_transcoder(intel_dp);
> > +		ctl = intel_dp_tp_ctl_reg(dev_priv, cpu_transcoder, port);
> > +		temp = I915_READ(ctl);
> >  		if (dp_train_pat & DP_LINK_SCRAMBLING_DISABLE)
> >  			temp |= DP_TP_CTL_SCRAMBLE_DISABLE;
> >  		else
> > @@ -3250,7 +3300,7 @@ _intel_dp_set_link_train(struct intel_dp *intel_dp,
> >  			temp |= DP_TP_CTL_LINK_TRAIN_PAT4;
> >  			break;
> >  		}
> > -		I915_WRITE(DP_TP_CTL(port), temp);
> > +		I915_WRITE(ctl, temp);
> >  
> >  	} else if ((IS_IVYBRIDGE(dev_priv) && port == PORT_A) ||
> >  		   (HAS_PCH_CPT(dev_priv) && port != PORT_A)) {
> > @@ -3943,15 +3993,21 @@ void intel_dp_set_idle_link_train(struct intel_dp *intel_dp)
> >  	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> >  	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> >  	enum port port = intel_dig_port->base.port;
> > +	enum transcoder cpu_transcoder;
> > +	i915_reg_t ctl, status;
> >  	u32 val;
> >  
> >  	if (!HAS_DDI(dev_priv))
> >  		return;
> >  
> > -	val = I915_READ(DP_TP_CTL(port));
> > +	cpu_transcoder = intel_dp_get_transcoder(intel_dp);
> > +	ctl = intel_dp_tp_ctl_reg(dev_priv, cpu_transcoder, port);
> > +	status = intel_dp_tp_status_reg(dev_priv, cpu_transcoder, port);
> > +
> > +	val = I915_READ(ctl);
> >  	val &= ~DP_TP_CTL_LINK_TRAIN_MASK;
> >  	val |= DP_TP_CTL_LINK_TRAIN_IDLE;
> > -	I915_WRITE(DP_TP_CTL(port), val);
> > +	I915_WRITE(ctl, val);
> >  
> >  	/*
> >  	 * On PORT_A we can have only eDP in SST mode. There the only reason
> > @@ -3963,7 +4019,7 @@ void intel_dp_set_idle_link_train(struct intel_dp *intel_dp)
> >  	if (port == PORT_A)
> >  		return;
> >  
> > -	if (intel_de_wait_for_set(dev_priv, DP_TP_STATUS(port),
> > +	if (intel_de_wait_for_set(dev_priv, status,
> >  				  DP_TP_STATUS_IDLE_DONE, 1))
> >  		DRM_ERROR("Timed out waiting for DP idle patterns\n");
> >  }
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp.h b/drivers/gpu/drm/i915/display/intel_dp.h
> > index 657bbb1f5ed0..107129b5d9a3 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp.h
> > +++ b/drivers/gpu/drm/i915/display/intel_dp.h
> > @@ -11,6 +11,7 @@
> >  #include <drm/i915_drm.h>
> >  
> >  #include "i915_reg.h"
> > +#include "intel_display.h"
> >  
> >  enum pipe;
> >  struct drm_connector_state;
> > @@ -113,6 +114,14 @@ int intel_dp_link_required(int pixel_clock, int bpp);
> >  int intel_dp_max_data_rate(int max_link_clock, int max_lanes);
> >  bool intel_digital_port_connected(struct intel_encoder *encoder);
> >  
> > +i915_reg_t intel_dp_tp_ctl_reg(const struct drm_i915_private *dev_priv,
> > +			       enum transcoder cpu_transcoder,
> > +			       enum port port);
> > +i915_reg_t intel_dp_tp_status_reg(const struct drm_i915_private *dev_priv,
> > +				  enum transcoder cpu_transcoder,
> > +				  enum port port);
> > +enum transcoder intel_dp_get_transcoder(struct intel_dp *intel_dp);
> > +
> >  static inline unsigned int intel_dp_unused_lane_mask(int lane_count)
> >  {
> >  	return ~((1 << lane_count) - 1) & 0xf;
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > index 8002dca9b734..630db2fadaca 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > @@ -354,9 +354,11 @@ static void intel_mst_pre_pll_enable_dp(struct intel_encoder *encoder,
> >  	struct intel_digital_port *intel_dig_port = intel_mst->primary;
> >  	struct intel_dp *intel_dp = &intel_dig_port->dp;
> >  
> > -	if (intel_dp->active_mst_links == 0)
> > +	if (intel_dp->active_mst_links == 0) {
> > +		intel_dp->mst_master_trans = pipe_config->mst_master_trans;

The current transcoder should be the master here, no?

Maybe sprinkle some

assert_is_master_transcoder(crtc_state)
{
	WARN(crtc_has_type(MST) &&
		crtc_state->cpu_trans != crtc_state->master_trans);
}

stuff into the ddi code?

> >  		intel_dig_port->base.pre_pll_enable(&intel_dig_port->base,
> >  						    pipe_config, NULL);
> > +	}
> >  }
> >  
> >  static void intel_mst_post_pll_disable_dp(struct intel_encoder *encoder,
> > @@ -384,6 +386,7 @@ static void intel_mst_pre_enable_dp(struct intel_encoder *encoder,
> >  	enum port port = intel_dig_port->base.port;
> >  	struct intel_connector *connector =
> >  		to_intel_connector(conn_state->connector);
> > +	i915_reg_t status;
> >  	int ret;
> >  	u32 temp;
> >  
> > @@ -412,8 +415,12 @@ static void intel_mst_pre_enable_dp(struct intel_encoder *encoder,
> >  		DRM_ERROR("failed to allocate vcpi\n");
> >  
> >  	intel_dp->active_mst_links++;
> > -	temp = I915_READ(DP_TP_STATUS(port));
> > -	I915_WRITE(DP_TP_STATUS(port), temp);
> > +
> > +	status = intel_dp_tp_status_reg(dev_priv,
> > +					pipe_config->mst_master_trans,
> > +					port);
> > +	temp = I915_READ(status);
> > +	I915_WRITE(status, temp);
> >  
> >  	ret = drm_dp_update_payload_part1(&intel_dp->mst_mgr);
> >  
> > @@ -429,10 +436,15 @@ static void intel_mst_enable_dp(struct intel_encoder *encoder,
> >  	struct intel_dp *intel_dp = &intel_dig_port->dp;
> >  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> >  	enum port port = intel_dig_port->base.port;
> > +	i915_reg_t status;
> >  
> >  	DRM_DEBUG_KMS("active links %d\n", intel_dp->active_mst_links);
> >  
> > -	if (intel_de_wait_for_set(dev_priv, DP_TP_STATUS(port),
> > +	status = intel_dp_tp_status_reg(dev_priv,
> > +					pipe_config->mst_master_trans,
> > +					port);
> > +
> > +	if (intel_de_wait_for_set(dev_priv, status,
> >  				  DP_TP_STATUS_ACT_SENT, 1))
> >  		DRM_ERROR("Timed out waiting for ACT sent\n");
> >  
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index bff9ee191832..fffaed0ca3cd 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -9454,7 +9454,9 @@ enum skl_power_gate {
> >  /* DisplayPort Transport Control */
> >  #define _DP_TP_CTL_A			0x64040
> >  #define _DP_TP_CTL_B			0x64140
> > +#define _TGL_DP_TP_CTL_A		0x60540
> >  #define DP_TP_CTL(port) _MMIO_PORT(port, _DP_TP_CTL_A, _DP_TP_CTL_B)
> > +#define TGL_DP_TP_CTL(tran) _MMIO_TRANS2((tran), _TGL_DP_TP_CTL_A)
> >  #define  DP_TP_CTL_ENABLE			(1 << 31)
> >  #define  DP_TP_CTL_FEC_ENABLE			(1 << 30)
> >  #define  DP_TP_CTL_MODE_SST			(0 << 27)
> > @@ -9474,7 +9476,9 @@ enum skl_power_gate {
> >  /* DisplayPort Transport Status */
> >  #define _DP_TP_STATUS_A			0x64044
> >  #define _DP_TP_STATUS_B			0x64144
> > +#define _TGL_DP_TP_STATUS_A		0x60544
> >  #define DP_TP_STATUS(port) _MMIO_PORT(port, _DP_TP_STATUS_A, _DP_TP_STATUS_B)
> > +#define TGL_DP_TP_STATUS(tran) _MMIO_TRANS2((tran), _TGL_DP_TP_STATUS_A)
> >  #define  DP_TP_STATUS_FEC_ENABLE_LIVE		(1 << 28)
> >  #define  DP_TP_STATUS_IDLE_DONE			(1 << 25)
> >  #define  DP_TP_STATUS_ACT_SENT			(1 << 24)
> > -- 
> > 2.23.0
> 
> -- 
> Ville Syrjälä
> Intel

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux