Re: [PATCH 1/3] drm/i915: Add enum aux_ch and clean up the aux init to use it

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

 





On Tue, 2018-02-20 at 11:31 -0800, Rodrigo Vivi wrote:
> On Tue, Feb 20, 2018 at 07:05:22PM +0200, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> > 
> > Since we no longer have a 1:1 correspondence between ports and AUX
> > channels, let's give AUX channels their own enum. Makes it easier
> > to tell the apples from the oranges, and we get rid of the
> > port E AUX power domain FIXME since we now derive the power domain
> > from the actual AUX CH.
> 
> 
> Beautiful clean-up. I had this in a todo list years ago and
> after staying on the bottom for so long I had removed it from there...
> > 
> > v2: Rebase due to AUX F
> 
> sorry and thanks! :)
> 
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> 
> I wondered if at least aux_power_domain part could be in a
> separated patch because by the end I was a bit confused...
> But in the end I could put all pieces together and it made sense
> again. So one patch for easy back porting seems better.
> 
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
> 
> > ---
> >  drivers/gpu/drm/i915/i915_reg.h      |   8 +-
> >  drivers/gpu/drm/i915/intel_display.h |  11 ++
> >  drivers/gpu/drm/i915/intel_dp.c      | 240 +++++++++++++++++------------------
> >  drivers/gpu/drm/i915/intel_drv.h     |   1 +
> >  4 files changed, 131 insertions(+), 129 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index 1412abcb27d4..39d624083a17 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -5347,8 +5347,8 @@ enum {
> >  #define _DPF_AUX_CH_DATA4	(dev_priv->info.display_mmio_offset + 0x64520)
> >  #define _DPF_AUX_CH_DATA5	(dev_priv->info.display_mmio_offset + 0x64524)
> >  
> > -#define DP_AUX_CH_CTL(port)	_MMIO_PORT(port, _DPA_AUX_CH_CTL, _DPB_AUX_CH_CTL)
> > -#define DP_AUX_CH_DATA(port, i)	_MMIO(_PORT(port, _DPA_AUX_CH_DATA1, _DPB_AUX_CH_DATA1) + (i) * 4) /* 5 registers */
> > +#define DP_AUX_CH_CTL(aux_ch)	_MMIO_PORT(aux_ch, _DPA_AUX_CH_CTL, _DPB_AUX_CH_CTL)
> > +#define DP_AUX_CH_DATA(aux_ch, i)	_MMIO(_PORT(aux_ch, _DPA_AUX_CH_DATA1, _DPB_AUX_CH_DATA1) + (i) * 4) /* 5 registers */
> >  
> >  #define   DP_AUX_CH_CTL_SEND_BUSY	    (1 << 31)
> >  #define   DP_AUX_CH_CTL_DONE		    (1 << 30)
> > @@ -7875,8 +7875,8 @@ enum {
> >  #define _PCH_DPD_AUX_CH_DATA4	0xe4320
> >  #define _PCH_DPD_AUX_CH_DATA5	0xe4324
> >  
> > -#define PCH_DP_AUX_CH_CTL(port)		_MMIO_PORT((port) - PORT_B, _PCH_DPB_AUX_CH_CTL, _PCH_DPC_AUX_CH_CTL)
> > -#define PCH_DP_AUX_CH_DATA(port, i)	_MMIO(_PORT((port) - PORT_B, _PCH_DPB_AUX_CH_DATA1, _PCH_DPC_AUX_CH_DATA1) + (i) * 4) /* 5 registers */
> > +#define PCH_DP_AUX_CH_CTL(aux_ch)		_MMIO_PORT((aux_ch) - AUX_CH_B, _PCH_DPB_AUX_CH_CTL, _PCH_DPC_AUX_CH_CTL)
> > +#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
> > diff --git a/drivers/gpu/drm/i915/intel_display.h b/drivers/gpu/drm/i915/intel_display.h
> > index c4042e342f50..f5733a2576e7 100644
> > --- a/drivers/gpu/drm/i915/intel_display.h
> > +++ b/drivers/gpu/drm/i915/intel_display.h
> > @@ -139,6 +139,17 @@ enum dpio_phy {
> >  
> >  #define I915_NUM_PHYS_VLV 2
> >  
> > +enum aux_ch {
> > +	AUX_CH_A,
> > +	AUX_CH_B,
> > +	AUX_CH_C,
> > +	AUX_CH_D,
> > +	_AUX_CH_E, /* does not exist */
> > +	AUX_CH_F,
> > +};
> > +
> > +#define aux_ch_name(a) ((a) + 'A')
> > +
> >  enum intel_display_power_domain {
> >  	POWER_DOMAIN_PIPE_A,
> >  	POWER_DOMAIN_PIPE_B,
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > index f20b25f98e5a..eeb8a026fd08 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -1331,171 +1331,194 @@ intel_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
> >  	return ret;
> >  }
> >  
> > -static enum port intel_aux_port(struct drm_i915_private *dev_priv,
> > -				enum port port)
> > +static enum aux_ch intel_aux_ch(struct intel_dp *intel_dp)
> >  {
> > +	struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
> > +	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> > +	enum port port = encoder->port;
> >  	const struct ddi_vbt_port_info *info =
> >  		&dev_priv->vbt.ddi_port_info[port];
> > -	enum port aux_port;
> > +	enum aux_ch aux_ch;
> >  
> >  	if (!info->alternate_aux_channel) {
> > +		aux_ch = (enum aux_ch) port;
> > +
> >  		DRM_DEBUG_KMS("using AUX %c for port %c (platform default)\n",
> > -			      port_name(port), port_name(port));
> > -		return port;
> > +			      aux_ch_name(aux_ch), port_name(port));
> > +		return aux_ch;
> >  	}
> >  
> >  	switch (info->alternate_aux_channel) {
> >  	case DP_AUX_A:
> > -		aux_port = PORT_A;
> > +		aux_ch = AUX_CH_A;
> >  		break;
> >  	case DP_AUX_B:
> > -		aux_port = PORT_B;
> > +		aux_ch = AUX_CH_B;
> >  		break;
> >  	case DP_AUX_C:
> > -		aux_port = PORT_C;
> > +		aux_ch = AUX_CH_C;
> >  		break;
> >  	case DP_AUX_D:
> > -		aux_port = PORT_D;
> > +		aux_ch = AUX_CH_D;
> >  		break;
> >  	case DP_AUX_F:
> > -		aux_port = PORT_F;
> > +		aux_ch = AUX_CH_F;
> >  		break;
> >  	default:
> >  		MISSING_CASE(info->alternate_aux_channel);
> > -		aux_port = PORT_A;
> > +		aux_ch = AUX_CH_A;
> >  		break;
> >  	}
> >  
> >  	DRM_DEBUG_KMS("using AUX %c for port %c (VBT)\n",
> > -		      port_name(aux_port), port_name(port));
> > +		      aux_ch_name(aux_ch), port_name(port));
> >  
> > -	return aux_port;
> > +	return aux_ch;
> > +}
> > +
> > +static enum intel_display_power_domain
> > +intel_aux_power_domain(struct intel_dp *intel_dp)
> > +{
> > +	switch (intel_dp->aux_ch) {
> > +	case AUX_CH_A:
> > +		return POWER_DOMAIN_AUX_A;
> > +	case AUX_CH_B:
> > +		return POWER_DOMAIN_AUX_B;
> > +	case AUX_CH_C:
> > +		return POWER_DOMAIN_AUX_C;
> > +	case AUX_CH_D:
> > +		return POWER_DOMAIN_AUX_D;
> > +	case AUX_CH_F:
> > +		return POWER_DOMAIN_AUX_F;
> > +	default:
> > +		MISSING_CASE(intel_dp->aux_ch);
> > +		return POWER_DOMAIN_AUX_A;
> > +	}
> >  }
> >  
> >  static i915_reg_t g4x_aux_ctl_reg(struct drm_i915_private *dev_priv,
> > -				  enum port port)
> > +				  enum aux_ch aux_ch)
> >  {
> > -	switch (port) {
> > -	case PORT_B:
> > -	case PORT_C:
> > -	case PORT_D:
> > -		return DP_AUX_CH_CTL(port);
> > +	switch (aux_ch) {
> > +	case AUX_CH_B:
> > +	case AUX_CH_C:
> > +	case AUX_CH_D:
> > +		return DP_AUX_CH_CTL(aux_ch);
> >  	default:
> > -		MISSING_CASE(port);
> > -		return DP_AUX_CH_CTL(PORT_B);
> > +		MISSING_CASE(aux_ch);
> > +		return DP_AUX_CH_CTL(AUX_CH_B);

Shouldn't intel_dp->aux_ch change too if we are switching to fallback
default registers?

Patch 3/3 assigns intel_dp->aux_ch and then assigns control and data
registers.
_______________________________________________
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