On Tue, Feb 20, 2018 at 07:05:22PM +0200, Ville Syrjala wrote: > From: Ville Syrjälä <[email protected]> > > 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ä <[email protected]> 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 <[email protected]> > --- > 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); > } > } > > static i915_reg_t g4x_aux_data_reg(struct drm_i915_private *dev_priv, > - enum port port, int index) > + enum aux_ch aux_ch, int index) > { > - switch (port) { > - case PORT_B: > - case PORT_C: > - case PORT_D: > - return DP_AUX_CH_DATA(port, index); > + switch (aux_ch) { > + case AUX_CH_B: > + case AUX_CH_C: > + case AUX_CH_D: > + return DP_AUX_CH_DATA(aux_ch, index); > default: > - MISSING_CASE(port); > - return DP_AUX_CH_DATA(PORT_B, index); > + MISSING_CASE(aux_ch); > + return DP_AUX_CH_DATA(AUX_CH_B, index); > } > } > > static i915_reg_t ilk_aux_ctl_reg(struct drm_i915_private *dev_priv, > - enum port port) > -{ > - switch (port) { > - case PORT_A: > - return DP_AUX_CH_CTL(port); > - case PORT_B: > - case PORT_C: > - case PORT_D: > - return PCH_DP_AUX_CH_CTL(port); > + enum aux_ch aux_ch) > +{ > + switch (aux_ch) { > + case AUX_CH_A: > + return DP_AUX_CH_CTL(aux_ch); > + case AUX_CH_B: > + case AUX_CH_C: > + case AUX_CH_D: > + return PCH_DP_AUX_CH_CTL(aux_ch); > default: > - MISSING_CASE(port); > - return DP_AUX_CH_CTL(PORT_A); > + MISSING_CASE(aux_ch); > + return DP_AUX_CH_CTL(AUX_CH_A); > } > } > > static i915_reg_t ilk_aux_data_reg(struct drm_i915_private *dev_priv, > - enum port port, int index) > -{ > - switch (port) { > - case PORT_A: > - return DP_AUX_CH_DATA(port, index); > - case PORT_B: > - case PORT_C: > - case PORT_D: > - return PCH_DP_AUX_CH_DATA(port, index); > + enum aux_ch aux_ch, int index) > +{ > + switch (aux_ch) { > + case AUX_CH_A: > + return DP_AUX_CH_DATA(aux_ch, index); > + case AUX_CH_B: > + case AUX_CH_C: > + case AUX_CH_D: > + return PCH_DP_AUX_CH_DATA(aux_ch, index); > default: > - MISSING_CASE(port); > - return DP_AUX_CH_DATA(PORT_A, index); > + MISSING_CASE(aux_ch); > + return DP_AUX_CH_DATA(AUX_CH_A, index); > } > } > > static i915_reg_t skl_aux_ctl_reg(struct drm_i915_private *dev_priv, > - enum port port) > -{ > - switch (port) { > - case PORT_A: > - case PORT_B: > - case PORT_C: > - case PORT_D: > - case PORT_F: > - return DP_AUX_CH_CTL(port); > + enum aux_ch aux_ch) > +{ > + switch (aux_ch) { > + case AUX_CH_A: > + case AUX_CH_B: > + case AUX_CH_C: > + case AUX_CH_D: > + case AUX_CH_F: > + return DP_AUX_CH_CTL(aux_ch); > default: > - MISSING_CASE(port); > - return DP_AUX_CH_CTL(PORT_A); > + MISSING_CASE(aux_ch); > + return DP_AUX_CH_CTL(AUX_CH_A); > } > } > > static i915_reg_t skl_aux_data_reg(struct drm_i915_private *dev_priv, > - enum port port, int index) > -{ > - switch (port) { > - case PORT_A: > - case PORT_B: > - case PORT_C: > - case PORT_D: > - case PORT_F: > - return DP_AUX_CH_DATA(port, index); > + enum aux_ch aux_ch, int index) > +{ > + switch (aux_ch) { > + case AUX_CH_A: > + case AUX_CH_B: > + case AUX_CH_C: > + case AUX_CH_D: > + case AUX_CH_F: > + return DP_AUX_CH_DATA(aux_ch, index); > default: > - MISSING_CASE(port); > - return DP_AUX_CH_DATA(PORT_A, index); > + MISSING_CASE(aux_ch); > + return DP_AUX_CH_DATA(AUX_CH_A, index); > } > } > > static i915_reg_t intel_aux_ctl_reg(struct drm_i915_private *dev_priv, > - enum port port) > + enum aux_ch aux_ch) > { > if (INTEL_GEN(dev_priv) >= 9) > - return skl_aux_ctl_reg(dev_priv, port); > + return skl_aux_ctl_reg(dev_priv, aux_ch); > else if (HAS_PCH_SPLIT(dev_priv)) > - return ilk_aux_ctl_reg(dev_priv, port); > + return ilk_aux_ctl_reg(dev_priv, aux_ch); > else > - return g4x_aux_ctl_reg(dev_priv, port); > + return g4x_aux_ctl_reg(dev_priv, aux_ch); > } > > static i915_reg_t intel_aux_data_reg(struct drm_i915_private *dev_priv, > - enum port port, int index) > + enum aux_ch aux_ch, int index) > { > if (INTEL_GEN(dev_priv) >= 9) > - return skl_aux_data_reg(dev_priv, port, index); > + return skl_aux_data_reg(dev_priv, aux_ch, index); > else if (HAS_PCH_SPLIT(dev_priv)) > - return ilk_aux_data_reg(dev_priv, port, index); > + return ilk_aux_data_reg(dev_priv, aux_ch, index); > else > - return g4x_aux_data_reg(dev_priv, port, index); > + return g4x_aux_data_reg(dev_priv, aux_ch, index); > } > > static void intel_aux_reg_init(struct intel_dp *intel_dp) > { > struct drm_i915_private *dev_priv = to_i915(intel_dp_to_dev(intel_dp)); > - enum port port = intel_aux_port(dev_priv, > - dp_to_dig_port(intel_dp)->base.port); > + enum aux_ch aux_ch = intel_dp->aux_ch; > int i; > > - intel_dp->aux_ch_ctl_reg = intel_aux_ctl_reg(dev_priv, port); > + intel_dp->aux_ch_ctl_reg = intel_aux_ctl_reg(dev_priv, aux_ch); > for (i = 0; i < ARRAY_SIZE(intel_dp->aux_ch_data_reg); i++) > - intel_dp->aux_ch_data_reg[i] = intel_aux_data_reg(dev_priv, > port, i); > + intel_dp->aux_ch_data_reg[i] = intel_aux_data_reg(dev_priv, > aux_ch, i); > } > > static void > @@ -1507,14 +1530,17 @@ intel_dp_aux_fini(struct intel_dp *intel_dp) > static void > intel_dp_aux_init(struct intel_dp *intel_dp) > { > - struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp); > - enum port port = intel_dig_port->base.port; > + struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base; > + > + intel_dp->aux_ch = intel_aux_ch(intel_dp); > + intel_dp->aux_power_domain = intel_aux_power_domain(intel_dp); > > intel_aux_reg_init(intel_dp); > drm_dp_aux_init(&intel_dp->aux); > > /* Failure to allocate our preferred name is not critical */ > - intel_dp->aux.name = kasprintf(GFP_KERNEL, "DPDDC-%c", port_name(port)); > + intel_dp->aux.name = kasprintf(GFP_KERNEL, "DPDDC-%c", > + port_name(encoder->port)); > intel_dp->aux.transfer = intel_dp_aux_transfer; > } > > @@ -6266,42 +6292,6 @@ static bool intel_edp_init_connector(struct intel_dp > *intel_dp, > return false; > } > > -/* Set up the hotplug pin and aux power domain. */ > -static void > -intel_dp_init_connector_port_info(struct intel_digital_port *intel_dig_port) > -{ > - struct intel_encoder *encoder = &intel_dig_port->base; > - struct intel_dp *intel_dp = &intel_dig_port->dp; > - struct intel_encoder *intel_encoder = &intel_dig_port->base; > - struct drm_i915_private *dev_priv = to_i915(intel_encoder->base.dev); > - > - encoder->hpd_pin = intel_hpd_pin_default(dev_priv, encoder->port); > - > - switch (encoder->port) { > - case PORT_A: > - intel_dp->aux_power_domain = POWER_DOMAIN_AUX_A; > - break; > - case PORT_B: > - intel_dp->aux_power_domain = POWER_DOMAIN_AUX_B; > - break; > - case PORT_C: > - intel_dp->aux_power_domain = POWER_DOMAIN_AUX_C; > - break; > - case PORT_D: > - intel_dp->aux_power_domain = POWER_DOMAIN_AUX_D; > - break; > - case PORT_E: > - /* FIXME: Check VBT for actual wiring of PORT E */ > - intel_dp->aux_power_domain = POWER_DOMAIN_AUX_D; > - break; > - case PORT_F: > - intel_dp->aux_power_domain = POWER_DOMAIN_AUX_F; > - break; > - default: > - MISSING_CASE(encoder->port); > - } > -} > - > static void intel_dp_modeset_retry_work_fn(struct work_struct *work) > { > struct intel_connector *intel_connector; > @@ -6407,7 +6397,7 @@ intel_dp_init_connector(struct intel_digital_port > *intel_dig_port, > connector->interlace_allowed = true; > connector->doublescan_allowed = 0; > > - intel_dp_init_connector_port_info(intel_dig_port); > + intel_encoder->hpd_pin = intel_hpd_pin_default(dev_priv, port); > > intel_dp_aux_init(intel_dp); > > diff --git a/drivers/gpu/drm/i915/intel_drv.h > b/drivers/gpu/drm/i915/intel_drv.h > index 898064e8bea7..7f6a7f592fe6 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -1049,6 +1049,7 @@ struct intel_dp { > bool detect_done; > bool channel_eq_status; > bool reset_link_params; > + enum aux_ch aux_ch; > uint8_t dpcd[DP_RECEIVER_CAP_SIZE]; > uint8_t psr_dpcd[EDP_PSR_RECEIVER_CAP_SIZE]; > uint8_t downstream_ports[DP_MAX_DOWNSTREAM_PORTS]; > -- > 2.13.6 > > _______________________________________________ > Intel-gfx mailing list > [email protected] > https://lists.freedesktop.org/mailman/listinfo/intel-gfx _______________________________________________ Intel-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/intel-gfx
