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

2018-02-22 Thread Ville Syrjälä
On Tue, Feb 20, 2018 at 11:31:09AM -0800, Rodrigo Vivi wrote:
> On Tue, Feb 20, 2018 at 07:05:22PM +0200, Ville Syrjala wrote:
> > From: Ville Syrjälä 
> > 
> > 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ä 
> 
> 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.

I suppose we might want to backport just the power_domain fix.
So might be worth extracting that into a separate patch. Should
be a simple matter of s/encoder->port/intel_dp_aux_port()/

> 
> Reviewed-by: Rodrigo Vivi 
> 
> > ---
> >  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_n

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

2018-02-22 Thread Ville Syrjälä
On Thu, Feb 22, 2018 at 07:25:22AM +, Pandiyan, Dhinakaran wrote:
> 
> 
> 
> 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ä 
> > > 
> > > 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ä 
> > 
> > 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 
> > 
> > > ---
> > >  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_DATA40xe4320
> > >  #define _PCH_DPD_AUX_CH_DATA50xe4324
> > >  
> > > -#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_CPT0
> > > 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_DE

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

2018-02-21 Thread Pandiyan, Dhinakaran



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ä 
> > 
> > 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ä 
> 
> 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 
> 
> > ---
> >  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

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

2018-02-21 Thread Pandiyan, Dhinakaran



On Tue, 2018-02-20 at 19:05 +0200, Ville Syrjala wrote:
> From: Ville Syrjälä 
> 
> 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.

Neat, this solves the problem I noticed while adding cnl aux io power
domain. Thanks.

> 
> v2: Rebase due to AUX F
> 
> Signed-off-by: Ville Syrjälä 
> ---
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


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

2018-02-20 Thread Rodrigo Vivi
On Tue, Feb 20, 2018 at 07:05:22PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä 
> 
> 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ä 

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 

> ---
>  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_DATA40xe4320
>  #define _PCH_DPD_AUX_CH_DATA50xe4324
>  
> -#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_CPT0
> 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_

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

2018-02-20 Thread Chris Wilson
Quoting Ville Syrjala (2018-02-20 17:05:22)
> From: Ville Syrjälä 
> 
> 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.
> 
> v2: Rebase due to AUX F
> 
> Signed-off-by: Ville Syrjälä 

The translation looks fine; nothing stood out and utterly confused me.
Reviewed-by: Chris Wilson 

Should definitely seek a second opinion on the new enum vs abusing port.
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx