| Message ID | 1449597590-6971-11-git-send-email-ville.syrjala@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Ville,
[auto build test WARNING on next-20151208]
[cannot apply to drm-intel/for-linux-next v4.4-rc4 v4.4-rc3 v4.4-rc2 v4.4-rc4]
url: https://github.com/0day-ci/linux/commits/ville-syrjala-linux-intel-com/drm-i915-Cure-DDI-from-multiple-personality-disorder/20151209-020401
config: i386-randconfig-x007-12070758 (attached as .config)
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
All warnings (new ones prefixed by >>):
drivers/gpu/drm/i915/intel_display.c: In function 'check_digital_port_conflicts':
>> drivers/gpu/drm/i915/intel_display.c:12176:21: warning: unused variable 'dev' [-Wunused-variable]
struct drm_device *dev = state->dev;
^
vim +/dev +12176 drivers/gpu/drm/i915/intel_display.c
6a60cd87 Chandra Konduru 2015-04-07 12160 crtc->base.primary == plane ? 0 : intel_plane->plane + 1,
6a60cd87 Chandra Konduru 2015-04-07 12161 drm_plane_index(plane));
6a60cd87 Chandra Konduru 2015-04-07 12162 DRM_DEBUG_KMS("\tFB:%d, fb = %ux%u format = 0x%x",
6a60cd87 Chandra Konduru 2015-04-07 12163 fb->base.id, fb->width, fb->height, fb->pixel_format);
6a60cd87 Chandra Konduru 2015-04-07 12164 DRM_DEBUG_KMS("\tscaler:%d src (%u, %u) %ux%u dst (%u, %u) %ux%u\n",
6a60cd87 Chandra Konduru 2015-04-07 12165 state->scaler_id,
6a60cd87 Chandra Konduru 2015-04-07 12166 state->src.x1 >> 16, state->src.y1 >> 16,
6a60cd87 Chandra Konduru 2015-04-07 12167 drm_rect_width(&state->src) >> 16,
6a60cd87 Chandra Konduru 2015-04-07 12168 drm_rect_height(&state->src) >> 16,
6a60cd87 Chandra Konduru 2015-04-07 12169 state->dst.x1, state->dst.y1,
6a60cd87 Chandra Konduru 2015-04-07 12170 drm_rect_width(&state->dst), drm_rect_height(&state->dst));
6a60cd87 Chandra Konduru 2015-04-07 12171 }
c0b03411 Daniel Vetter 2013-05-28 12172 }
c0b03411 Daniel Vetter 2013-05-28 12173
5448a00d Ander Conselvan de Oliveira 2015-04-02 12174 static bool check_digital_port_conflicts(struct drm_atomic_state *state)
00f0b378 Ville Syrjälä 2014-12-02 12175 {
5448a00d Ander Conselvan de Oliveira 2015-04-02 @12176 struct drm_device *dev = state->dev;
5448a00d Ander Conselvan de Oliveira 2015-04-02 12177 struct intel_encoder *encoder;
da3ced29 Ander Conselvan de Oliveira 2015-04-21 12178 struct drm_connector *connector;
5448a00d Ander Conselvan de Oliveira 2015-04-02 12179 struct drm_connector_state *connector_state;
00f0b378 Ville Syrjälä 2014-12-02 12180 unsigned int used_ports = 0;
5448a00d Ander Conselvan de Oliveira 2015-04-02 12181 int i;
00f0b378 Ville Syrjälä 2014-12-02 12182
00f0b378 Ville Syrjälä 2014-12-02 12183 /*
00f0b378 Ville Syrjälä 2014-12-02 12184 * Walk the connector list instead of the encoder
:::::: The code at line 12176 was first introduced by commit
:::::: 5448a00d3f0640949c2a9a4d16e163b0b40a3b2e drm/i915: Don't use staged config in check_digital_port_conflicts()
:::::: TO: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
:::::: CC: Daniel Vetter <daniel.vetter@ffwll.ch>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
On Tue, Dec 08, 2015 at 07:59:45PM +0200, ville.syrjala@linux.intel.com wrote: > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Eliminate the troublesome role switching DDI encoder, and just register > a separate encoder for each role (DP and HDMI). > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> Caveat about max_lanes and pre-DDI platforms still apply. > --- > drivers/gpu/drm/i915/intel_ddi.c | 232 +++++++++++++++++++++++++--------- > drivers/gpu/drm/i915/intel_display.c | 18 --- > drivers/gpu/drm/i915/intel_dp.c | 23 +--- > drivers/gpu/drm/i915/intel_drv.h | 3 +- > drivers/gpu/drm/i915/intel_hdmi.c | 3 + > drivers/gpu/drm/i915/intel_opregion.c | 1 - > 6 files changed, 180 insertions(+), 100 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c > index 074121efb265..5f008f0fdc13 100644 > --- a/drivers/gpu/drm/i915/intel_ddi.c > +++ b/drivers/gpu/drm/i915/intel_ddi.c > @@ -318,7 +318,6 @@ static void ddi_get_encoder_port(struct intel_encoder *intel_encoder, > case INTEL_OUTPUT_DISPLAYPORT: > case INTEL_OUTPUT_EDP: > case INTEL_OUTPUT_HDMI: > - case INTEL_OUTPUT_UNKNOWN: > *dig_port = enc_to_dig_port(encoder); > *port = (*dig_port)->port; > break; > @@ -1942,19 +1941,19 @@ bool intel_ddi_connector_get_hw_state(struct intel_connector *intel_connector) > switch (tmp & TRANS_DDI_MODE_SELECT_MASK) { > case TRANS_DDI_MODE_SELECT_HDMI: > case TRANS_DDI_MODE_SELECT_DVI: > - return (type == DRM_MODE_CONNECTOR_HDMIA); > + return type == DRM_MODE_CONNECTOR_HDMIA; > > case TRANS_DDI_MODE_SELECT_DP_SST: > - if (type == DRM_MODE_CONNECTOR_eDP) > - return true; > - return (type == DRM_MODE_CONNECTOR_DisplayPort); > + return type == DRM_MODE_CONNECTOR_DisplayPort || > + type == DRM_MODE_CONNECTOR_eDP; > + > case TRANS_DDI_MODE_SELECT_DP_MST: > /* if the transcoder is in MST state then > * connector isn't connected */ > return false; > > case TRANS_DDI_MODE_SELECT_FDI: > - return (type == DRM_MODE_CONNECTOR_VGA); > + return type == DRM_MODE_CONNECTOR_VGA; > > default: > return false; > @@ -1981,8 +1980,23 @@ bool intel_ddi_get_hw_state(struct intel_encoder *encoder, > return false; > > if (port == PORT_A) { > + WARN_ON(encoder->type != INTEL_OUTPUT_EDP); > + > tmp = I915_READ(TRANS_DDI_FUNC_CTL(TRANSCODER_EDP)); > > + if ((tmp & TRANS_DDI_FUNC_ENABLE) == 0) > + goto out; > + > + switch (tmp & TRANS_DDI_MODE_SELECT_MASK) { > + case TRANS_DDI_MODE_SELECT_DP_SST: > + break; > + default: > + WARN(1, > + "Bad transcoder EDP DDI mode 0x%08x for port %c\n", > + tmp, port_name(port)); > + return false; > + } > + > switch (tmp & TRANS_DDI_EDP_INPUT_MASK) { > case TRANS_DDI_EDP_INPUT_A_ON: > case TRANS_DDI_EDP_INPUT_A_ONOFF: > @@ -1994,25 +2008,98 @@ bool intel_ddi_get_hw_state(struct intel_encoder *encoder, > case TRANS_DDI_EDP_INPUT_C_ONOFF: > *pipe = PIPE_C; > break; > + default: > + WARN(1, > + "Bad transcoder EDP input select 0x%08x for port %c\n", > + tmp, port_name(port)); > + return false; > } > > return true; > } else { > + int num_mst_transcoders = 0; > + int num_sst_transcoders = 0; > + int num_fdi_transcoders = 0; > + int num_hdmi_transcoders = 0; > + int num_transcoders = 0; > + bool enabled = false; > + > for (i = TRANSCODER_A; i <= TRANSCODER_C; i++) { > tmp = I915_READ(TRANS_DDI_FUNC_CTL(i)); > > - if ((tmp & TRANS_DDI_PORT_MASK) > - == TRANS_DDI_SELECT_PORT(port)) { > - if ((tmp & TRANS_DDI_MODE_SELECT_MASK) == TRANS_DDI_MODE_SELECT_DP_MST) > - return false; > + if ((tmp & TRANS_DDI_FUNC_ENABLE) == 0) > + continue; > + > + if ((tmp & TRANS_DDI_PORT_MASK) != TRANS_DDI_SELECT_PORT(port)) > + continue; > + > + if ((tmp & TRANS_DDI_MODE_SELECT_MASK) == TRANS_DDI_MODE_SELECT_DP_MST) { > + num_mst_transcoders++; > + WARN_ON(port == PORT_E); > + continue; > + } > + > + > + switch (tmp & TRANS_DDI_MODE_SELECT_MASK) { > + case TRANS_DDI_MODE_SELECT_DP_SST: > + WARN_ON(port == PORT_E && INTEL_INFO(dev_priv)->gen < 9); > + > + num_sst_transcoders++; > + if (encoder->type == INTEL_OUTPUT_DISPLAYPORT || > + encoder->type == INTEL_OUTPUT_EDP) { > + enabled = true; > + *pipe = i; > + } > + break; > + case TRANS_DDI_MODE_SELECT_HDMI: > + case TRANS_DDI_MODE_SELECT_DVI: > + WARN_ON(port == PORT_E); Hm, previous patches made it look like hdmi on port E is possible - at least you added piles of checks to make sure we have at least 4 lanes everywhere. Am I mistaken? > + > + num_hdmi_transcoders++; > + if (encoder->type == INTEL_OUTPUT_HDMI) { > + enabled = true; > + *pipe = i; > + } > + break; > + > + case TRANS_DDI_MODE_SELECT_FDI: > + WARN_ON(port != PORT_E || INTEL_INFO(dev_priv)->gen >= 9); > + > + num_fdi_transcoders++; > + if (encoder->type == INTEL_OUTPUT_ANALOG) { > + enabled = true; > + *pipe = i; > + } > + break; > > - *pipe = i; > - return true; > + default: > + WARN(1, "Bad transcoder %c DDI mode 0x%08x for port %c\n", > + transcoder_name(i), tmp, port_name(port)); > + return false; > } > } > + > + num_transcoders = num_sst_transcoders + > + num_fdi_transcoders + num_hdmi_transcoders; > + > + if (WARN(num_transcoders && num_mst_transcoders, > + "MST and non-MST transcoders enabled for port %c (%d sst, %d mst, %d fdi, %d hdmi)\n", > + port_name(port), num_sst_transcoders, num_mst_transcoders, > + num_fdi_transcoders, num_hdmi_transcoders)) > + return false; > + > + if (WARN(num_transcoders > 1, > + "Multiple transcoders enabled for port %c (%d sst, %d mst, %d fdi, %d hdmi)\n", > + port_name(port), num_sst_transcoders, num_mst_transcoders, > + num_fdi_transcoders, num_hdmi_transcoders)) > + return false; > + > + if (enabled) > + return true; This is too big and needs to be extracted into a static function. Otherwise I didn't spot anything, and getting rid of OUTPUT_UNKNOWN is really nice. I also wonder whether we should long term rework mst to use the fake encoder as the real one. The fake encoder was just done to not confuse the ddi encoder, but now that we have multiples of those we might as well embrace that everywhere. Cheers, Daniel > } > > - DRM_DEBUG_KMS("No pipe for ddi port %c found\n", port_name(port)); > +out: > + DRM_DEBUG_KMS("No pipe for DDI port %c found\n", port_name(port)); > > return false; > } > @@ -3174,8 +3261,6 @@ static bool intel_ddi_compute_config(struct intel_encoder *encoder, > int type = encoder->type; > int port = intel_ddi_get_encoder_port(encoder); > > - WARN(type == INTEL_OUTPUT_UNKNOWN, "compute_config() on unknown output!\n"); > - > if (port == PORT_A) > pipe_config->cpu_transcoder = TRANSCODER_EDP; > > @@ -3224,53 +3309,18 @@ intel_ddi_init_hdmi_connector(struct intel_digital_port *intel_dig_port) > return connector; > } > > -void intel_ddi_init(struct drm_device *dev, enum port port) > +static int intel_ddi_init_role(struct drm_device *dev, enum port port, > + int encoder_type, uint32_t saved_port_bits, > + int max_lanes) > { > - struct drm_i915_private *dev_priv = dev->dev_private; > + struct drm_i915_private *dev_priv = to_i915(dev); > struct intel_digital_port *intel_dig_port; > struct intel_encoder *intel_encoder; > struct drm_encoder *encoder; > - bool init_hdmi, init_dp; > - int max_lanes; > - > - if (I915_READ(DDI_BUF_CTL(PORT_A)) & DDI_A_4_LANES) { > - switch (port) { > - case PORT_A: > - max_lanes = 4; > - break; > - case PORT_E: > - max_lanes = 0; > - break; > - default: > - max_lanes = 4; > - break; > - } > - } else { > - switch (port) { > - case PORT_A: > - max_lanes = 2; > - break; > - case PORT_E: > - max_lanes = 2; > - break; > - default: > - max_lanes = 4; > - break; > - } > - } > - > - init_hdmi = (dev_priv->vbt.ddi_port_info[port].supports_dvi || > - dev_priv->vbt.ddi_port_info[port].supports_hdmi); > - init_dp = dev_priv->vbt.ddi_port_info[port].supports_dp; > - if (!init_dp && !init_hdmi) { > - DRM_DEBUG_KMS("VBT says port %c is not DVI/HDMI/DP compatible, respect it\n", > - port_name(port)); > - return; > - } > > intel_dig_port = kzalloc(sizeof(*intel_dig_port), GFP_KERNEL); > if (!intel_dig_port) > - return; > + return -ENOMEM; > > intel_encoder = &intel_dig_port->base; > encoder = &intel_encoder->base; > @@ -3287,9 +3337,7 @@ void intel_ddi_init(struct drm_device *dev, enum port port) > intel_encoder->get_config = intel_ddi_get_config; > > intel_dig_port->port = port; > - intel_dig_port->saved_port_bits = I915_READ(DDI_BUF_CTL(port)) & > - (DDI_BUF_PORT_REVERSAL | > - DDI_A_4_LANES); > + intel_dig_port->saved_port_bits = saved_port_bits; > intel_dig_port->max_lanes = max_lanes; > > /* > @@ -3306,11 +3354,11 @@ void intel_ddi_init(struct drm_device *dev, enum port port) > } > } > > - intel_encoder->type = INTEL_OUTPUT_UNKNOWN; > + intel_encoder->type = encoder_type; > intel_encoder->crtc_mask = (1 << 0) | (1 << 1) | (1 << 2); > intel_encoder->cloneable = 0; > > - if (init_dp) { > + if (intel_encoder->type == INTEL_OUTPUT_DISPLAYPORT) { > if (!intel_ddi_init_dp_connector(intel_dig_port)) > goto err; > > @@ -3327,14 +3375,74 @@ void intel_ddi_init(struct drm_device *dev, enum port port) > > /* In theory we don't need the encoder->type check, but leave it just in > * case we have some really bad VBTs... */ > - if (intel_encoder->type != INTEL_OUTPUT_EDP && init_hdmi) { > + if (intel_encoder->type == INTEL_OUTPUT_HDMI) { > if (!intel_ddi_init_hdmi_connector(intel_dig_port)) > goto err; > } > > - return; > + return intel_encoder->type; > > err: > drm_encoder_cleanup(encoder); > kfree(intel_dig_port); > + > + return -ENODEV; > +} > + > +void intel_ddi_init(struct drm_device *dev, enum port port) > +{ > + struct drm_i915_private *dev_priv = to_i915(dev); > + uint32_t saved_port_bits; > + bool init_hdmi, init_dp; > + int max_lanes; > + > + if (I915_READ(DDI_BUF_CTL(PORT_A)) & DDI_A_4_LANES) { > + switch (port) { > + case PORT_A: > + max_lanes = 4; > + break; > + case PORT_E: > + max_lanes = 0; > + break; > + default: > + max_lanes = 4; > + break; > + } > + } else { > + switch (port) { > + case PORT_A: > + max_lanes = 2; > + break; > + case PORT_E: > + max_lanes = 2; > + break; > + default: > + max_lanes = 4; > + break; > + } > + } > + > + init_hdmi = (dev_priv->vbt.ddi_port_info[port].supports_dvi || > + dev_priv->vbt.ddi_port_info[port].supports_hdmi); > + init_dp = dev_priv->vbt.ddi_port_info[port].supports_dp; > + if (!init_dp && !init_hdmi) { > + DRM_DEBUG_KMS("VBT says port %c is not DVI/HDMI/DP compatible, respect it\n", > + port_name(port)); > + return; > + } > + > + saved_port_bits = I915_READ(DDI_BUF_CTL(port)) & > + (DDI_BUF_PORT_REVERSAL | DDI_A_4_LANES); > + > + if (init_dp) { > + int ret = intel_ddi_init_role(dev, port, INTEL_OUTPUT_DISPLAYPORT, > + saved_port_bits, max_lanes); > + /* Don't register the HDMI connector/encoder when we have eDP */ > + if (ret == INTEL_OUTPUT_EDP) > + init_hdmi = false; > + } > + > + if (init_hdmi) > + intel_ddi_init_role(dev, port, INTEL_OUTPUT_HDMI, > + saved_port_bits, max_lanes); > } > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index bc7aaa3c431e..fc1d7387eb12 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -5252,13 +5252,9 @@ static enum intel_display_power_domain port_to_aux_power_domain(enum port port) > enum intel_display_power_domain > intel_display_port_power_domain(struct intel_encoder *intel_encoder) > { > - struct drm_device *dev = intel_encoder->base.dev; > struct intel_digital_port *intel_dig_port; > > switch (intel_encoder->type) { > - case INTEL_OUTPUT_UNKNOWN: > - /* Only DDI platforms should ever use this output type */ > - WARN_ON_ONCE(!HAS_DDI(dev)); > case INTEL_OUTPUT_DISPLAYPORT: > case INTEL_OUTPUT_HDMI: > case INTEL_OUTPUT_EDP: > @@ -5279,20 +5275,9 @@ intel_display_port_power_domain(struct intel_encoder *intel_encoder) > enum intel_display_power_domain > intel_display_port_aux_power_domain(struct intel_encoder *intel_encoder) > { > - struct drm_device *dev = intel_encoder->base.dev; > struct intel_digital_port *intel_dig_port; > > switch (intel_encoder->type) { > - case INTEL_OUTPUT_UNKNOWN: > - case INTEL_OUTPUT_HDMI: > - /* > - * Only DDI platforms should ever use these output types. > - * We can get here after the HDMI detect code has already set > - * the type of the shared encoder. Since we can't be sure > - * what's the status of the given connectors, play safe and > - * run the DP detection too. > - */ > - WARN_ON_ONCE(!HAS_DDI(dev)); > case INTEL_OUTPUT_DISPLAYPORT: > case INTEL_OUTPUT_EDP: > intel_dig_port = enc_to_dig_port(&intel_encoder->base); > @@ -12283,9 +12268,6 @@ static bool check_digital_port_conflicts(struct drm_atomic_state *state) > > switch (encoder->type) { > unsigned int port_mask; > - case INTEL_OUTPUT_UNKNOWN: > - if (WARN_ON(!HAS_DDI(dev))) > - break; > case INTEL_OUTPUT_DISPLAYPORT: > case INTEL_OUTPUT_HDMI: > case INTEL_OUTPUT_EDP: > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > index 7d354b1e5e5f..1d31aa296aaa 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -4601,8 +4601,6 @@ intel_dp_detect(struct drm_connector *connector, bool force) > > if (intel_dp->is_mst) { > /* MST devices are disconnected from a monitor POV */ > - if (intel_encoder->type != INTEL_OUTPUT_EDP) > - intel_encoder->type = INTEL_OUTPUT_DISPLAYPORT; > return connector_status_disconnected; > } > > @@ -4632,8 +4630,6 @@ intel_dp_detect(struct drm_connector *connector, bool force) > if (ret) { > /* if we are in MST mode then this connector > won't appear connected or have anything with EDID on it */ > - if (intel_encoder->type != INTEL_OUTPUT_EDP) > - intel_encoder->type = INTEL_OUTPUT_DISPLAYPORT; > status = connector_status_disconnected; > goto out; > } > @@ -4648,8 +4644,6 @@ intel_dp_detect(struct drm_connector *connector, bool force) > > intel_dp_set_edid(intel_dp); > > - if (intel_encoder->type != INTEL_OUTPUT_EDP) > - intel_encoder->type = INTEL_OUTPUT_DISPLAYPORT; > status = connector_status_connected; > > /* Try to read the source of the interrupt */ > @@ -4692,9 +4686,6 @@ intel_dp_force(struct drm_connector *connector) > intel_dp_set_edid(intel_dp); > > intel_display_power_put(dev_priv, power_domain); > - > - if (intel_encoder->type != INTEL_OUTPUT_EDP) > - intel_encoder->type = INTEL_OUTPUT_DISPLAYPORT; > } > > static int intel_dp_get_modes(struct drm_connector *connector) > @@ -4969,9 +4960,9 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd) > enum intel_display_power_domain power_domain; > enum irqreturn ret = IRQ_NONE; > > - if (intel_dig_port->base.type != INTEL_OUTPUT_EDP && > - intel_dig_port->base.type != INTEL_OUTPUT_HDMI) > - intel_dig_port->base.type = INTEL_OUTPUT_DISPLAYPORT; > + if (WARN_ON_ONCE(intel_dig_port->base.type != INTEL_OUTPUT_EDP && > + intel_dig_port->base.type != INTEL_OUTPUT_DISPLAYPORT)) > + return IRQ_HANDLED; > > if (long_hpd && intel_dig_port->base.type == INTEL_OUTPUT_EDP) { > /* > @@ -5815,6 +5806,9 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port, > enum port port = intel_dig_port->port; > int type, ret; > > + if (WARN_ON(intel_encoder->type != INTEL_OUTPUT_DISPLAYPORT)) > + return false; > + > if (WARN(intel_dig_port->max_lanes < 1, > "Not enough lanes (%d) for DP on port %c\n", > intel_dig_port->max_lanes, port_name(port))) > @@ -5851,11 +5845,6 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port, > else > type = DRM_MODE_CONNECTOR_DisplayPort; > > - /* > - * For eDP we always set the encoder type to INTEL_OUTPUT_EDP, but > - * for DP the encoder type can be set by the caller to > - * INTEL_OUTPUT_UNKNOWN for DDI, so don't rewrite it. > - */ > if (type == DRM_MODE_CONNECTOR_eDP) > intel_encoder->type = INTEL_OUTPUT_EDP; > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index a8a84b8c2bac..9e5db3d71e12 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -103,8 +103,7 @@ enum intel_output_type { > INTEL_OUTPUT_DISPLAYPORT = 7, > INTEL_OUTPUT_EDP = 8, > INTEL_OUTPUT_DSI = 9, > - INTEL_OUTPUT_UNKNOWN = 10, > - INTEL_OUTPUT_DP_MST = 11, > + INTEL_OUTPUT_DP_MST = 10, > }; > > #define INTEL_DVO_CHIP_NONE 0 > diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c > index 895189abfd56..75ea9515a9ce 100644 > --- a/drivers/gpu/drm/i915/intel_hdmi.c > +++ b/drivers/gpu/drm/i915/intel_hdmi.c > @@ -2034,6 +2034,9 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port, > enum port port = intel_dig_port->port; > uint8_t alternate_ddc_pin; > > + if (WARN_ON(intel_encoder->type != INTEL_OUTPUT_HDMI)) > + return; > + > if (WARN(intel_dig_port->max_lanes < 4, > "Not enough lanes (%d) for HDMI on port %c\n", > intel_dig_port->max_lanes, port_name(port))) > diff --git a/drivers/gpu/drm/i915/intel_opregion.c b/drivers/gpu/drm/i915/intel_opregion.c > index e362a30776fa..a15459a451c2 100644 > --- a/drivers/gpu/drm/i915/intel_opregion.c > +++ b/drivers/gpu/drm/i915/intel_opregion.c > @@ -360,7 +360,6 @@ int intel_opregion_notify_encoder(struct intel_encoder *intel_encoder, > case INTEL_OUTPUT_ANALOG: > type = DISPLAY_TYPE_CRT; > break; > - case INTEL_OUTPUT_UNKNOWN: > case INTEL_OUTPUT_DISPLAYPORT: > case INTEL_OUTPUT_HDMI: > case INTEL_OUTPUT_DP_MST: > -- > 2.4.10 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Thu, Dec 10, 2015 at 02:47:12PM +0100, Daniel Vetter wrote: > On Tue, Dec 08, 2015 at 07:59:45PM +0200, ville.syrjala@linux.intel.com wrote: > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > Eliminate the troublesome role switching DDI encoder, and just register > > a separate encoder for each role (DP and HDMI). > > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Caveat about max_lanes and pre-DDI platforms still apply. > > > --- > > drivers/gpu/drm/i915/intel_ddi.c | 232 +++++++++++++++++++++++++--------- > > drivers/gpu/drm/i915/intel_display.c | 18 --- > > drivers/gpu/drm/i915/intel_dp.c | 23 +--- > > drivers/gpu/drm/i915/intel_drv.h | 3 +- > > drivers/gpu/drm/i915/intel_hdmi.c | 3 + > > drivers/gpu/drm/i915/intel_opregion.c | 1 - > > 6 files changed, 180 insertions(+), 100 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c > > index 074121efb265..5f008f0fdc13 100644 > > --- a/drivers/gpu/drm/i915/intel_ddi.c > > +++ b/drivers/gpu/drm/i915/intel_ddi.c > > @@ -318,7 +318,6 @@ static void ddi_get_encoder_port(struct intel_encoder *intel_encoder, > > case INTEL_OUTPUT_DISPLAYPORT: > > case INTEL_OUTPUT_EDP: > > case INTEL_OUTPUT_HDMI: > > - case INTEL_OUTPUT_UNKNOWN: > > *dig_port = enc_to_dig_port(encoder); > > *port = (*dig_port)->port; > > break; > > @@ -1942,19 +1941,19 @@ bool intel_ddi_connector_get_hw_state(struct intel_connector *intel_connector) > > switch (tmp & TRANS_DDI_MODE_SELECT_MASK) { > > case TRANS_DDI_MODE_SELECT_HDMI: > > case TRANS_DDI_MODE_SELECT_DVI: > > - return (type == DRM_MODE_CONNECTOR_HDMIA); > > + return type == DRM_MODE_CONNECTOR_HDMIA; > > > > case TRANS_DDI_MODE_SELECT_DP_SST: > > - if (type == DRM_MODE_CONNECTOR_eDP) > > - return true; > > - return (type == DRM_MODE_CONNECTOR_DisplayPort); > > + return type == DRM_MODE_CONNECTOR_DisplayPort || > > + type == DRM_MODE_CONNECTOR_eDP; > > + > > case TRANS_DDI_MODE_SELECT_DP_MST: > > /* if the transcoder is in MST state then > > * connector isn't connected */ > > return false; > > > > case TRANS_DDI_MODE_SELECT_FDI: > > - return (type == DRM_MODE_CONNECTOR_VGA); > > + return type == DRM_MODE_CONNECTOR_VGA; > > > > default: > > return false; > > @@ -1981,8 +1980,23 @@ bool intel_ddi_get_hw_state(struct intel_encoder *encoder, > > return false; > > > > if (port == PORT_A) { > > + WARN_ON(encoder->type != INTEL_OUTPUT_EDP); > > + > > tmp = I915_READ(TRANS_DDI_FUNC_CTL(TRANSCODER_EDP)); > > > > + if ((tmp & TRANS_DDI_FUNC_ENABLE) == 0) > > + goto out; > > + > > + switch (tmp & TRANS_DDI_MODE_SELECT_MASK) { > > + case TRANS_DDI_MODE_SELECT_DP_SST: > > + break; > > + default: > > + WARN(1, > > + "Bad transcoder EDP DDI mode 0x%08x for port %c\n", > > + tmp, port_name(port)); > > + return false; > > + } > > + > > switch (tmp & TRANS_DDI_EDP_INPUT_MASK) { > > case TRANS_DDI_EDP_INPUT_A_ON: > > case TRANS_DDI_EDP_INPUT_A_ONOFF: > > @@ -1994,25 +2008,98 @@ bool intel_ddi_get_hw_state(struct intel_encoder *encoder, > > case TRANS_DDI_EDP_INPUT_C_ONOFF: > > *pipe = PIPE_C; > > break; > > + default: > > + WARN(1, > > + "Bad transcoder EDP input select 0x%08x for port %c\n", > > + tmp, port_name(port)); > > + return false; > > } > > > > return true; > > } else { > > + int num_mst_transcoders = 0; > > + int num_sst_transcoders = 0; > > + int num_fdi_transcoders = 0; > > + int num_hdmi_transcoders = 0; > > + int num_transcoders = 0; > > + bool enabled = false; > > + > > for (i = TRANSCODER_A; i <= TRANSCODER_C; i++) { > > tmp = I915_READ(TRANS_DDI_FUNC_CTL(i)); > > > > - if ((tmp & TRANS_DDI_PORT_MASK) > > - == TRANS_DDI_SELECT_PORT(port)) { > > - if ((tmp & TRANS_DDI_MODE_SELECT_MASK) == TRANS_DDI_MODE_SELECT_DP_MST) > > - return false; > > + if ((tmp & TRANS_DDI_FUNC_ENABLE) == 0) > > + continue; > > + > > + if ((tmp & TRANS_DDI_PORT_MASK) != TRANS_DDI_SELECT_PORT(port)) > > + continue; > > + > > + if ((tmp & TRANS_DDI_MODE_SELECT_MASK) == TRANS_DDI_MODE_SELECT_DP_MST) { > > + num_mst_transcoders++; > > + WARN_ON(port == PORT_E); > > + continue; > > + } > > + > > + > > + switch (tmp & TRANS_DDI_MODE_SELECT_MASK) { > > + case TRANS_DDI_MODE_SELECT_DP_SST: > > + WARN_ON(port == PORT_E && INTEL_INFO(dev_priv)->gen < 9); > > + > > + num_sst_transcoders++; > > + if (encoder->type == INTEL_OUTPUT_DISPLAYPORT || > > + encoder->type == INTEL_OUTPUT_EDP) { > > + enabled = true; > > + *pipe = i; > > + } > > + break; > > + case TRANS_DDI_MODE_SELECT_HDMI: > > + case TRANS_DDI_MODE_SELECT_DVI: > > + WARN_ON(port == PORT_E); > > Hm, previous patches made it look like hdmi on port E is possible - at > least you added piles of checks to make sure we have at least 4 lanes > everywhere. Am I mistaken? DDI E can only have two lanes at most. So it can't do HDMI. Actually it's documented that it can only do FDI (or SST on SKL+). Now that I think about it, I'm not sure an external DP port with < 4 lanes is even legal, so perhaps in practice it can only be eDP or FDI? > > > + > > + num_hdmi_transcoders++; > > + if (encoder->type == INTEL_OUTPUT_HDMI) { > > + enabled = true; > > + *pipe = i; > > + } > > + break; > > + > > + case TRANS_DDI_MODE_SELECT_FDI: > > + WARN_ON(port != PORT_E || INTEL_INFO(dev_priv)->gen >= 9); > > + > > + num_fdi_transcoders++; > > + if (encoder->type == INTEL_OUTPUT_ANALOG) { > > + enabled = true; > > + *pipe = i; > > + } > > + break; > > > > - *pipe = i; > > - return true; > > + default: > > + WARN(1, "Bad transcoder %c DDI mode 0x%08x for port %c\n", > > + transcoder_name(i), tmp, port_name(port)); > > + return false; > > } > > } > > + > > + num_transcoders = num_sst_transcoders + > > + num_fdi_transcoders + num_hdmi_transcoders; > > + > > + if (WARN(num_transcoders && num_mst_transcoders, > > + "MST and non-MST transcoders enabled for port %c (%d sst, %d mst, %d fdi, %d hdmi)\n", > > + port_name(port), num_sst_transcoders, num_mst_transcoders, > > + num_fdi_transcoders, num_hdmi_transcoders)) > > + return false; > > + > > + if (WARN(num_transcoders > 1, > > + "Multiple transcoders enabled for port %c (%d sst, %d mst, %d fdi, %d hdmi)\n", > > + port_name(port), num_sst_transcoders, num_mst_transcoders, > > + num_fdi_transcoders, num_hdmi_transcoders)) > > + return false; > > + > > + if (enabled) > > + return true; > > This is too big and needs to be extracted into a static function. > > Otherwise I didn't spot anything, and getting rid of OUTPUT_UNKNOWN is > really nice. I also wonder whether we should long term rework mst to use > the fake encoder as the real one. The fake encoder was just done to not > confuse the ddi encoder, but now that we have multiples of those we might > as well embrace that everywhere. Hmm. One idea for MST I've had is that we'd pull out the main link management from the mst code, and instead use atomic to set up the main link as just another crtc (would need a fake crtc for that), and then the streams would just deal with their fake encoders and not mess about with the main encoder at all. But sounds like you've had pretty much the opposite idea. > > Cheers, Daniel > > > } > > > > - DRM_DEBUG_KMS("No pipe for ddi port %c found\n", port_name(port)); > > +out: > > + DRM_DEBUG_KMS("No pipe for DDI port %c found\n", port_name(port)); > > > > return false; > > } > > @@ -3174,8 +3261,6 @@ static bool intel_ddi_compute_config(struct intel_encoder *encoder, > > int type = encoder->type; > > int port = intel_ddi_get_encoder_port(encoder); > > > > - WARN(type == INTEL_OUTPUT_UNKNOWN, "compute_config() on unknown output!\n"); > > - > > if (port == PORT_A) > > pipe_config->cpu_transcoder = TRANSCODER_EDP; > > > > @@ -3224,53 +3309,18 @@ intel_ddi_init_hdmi_connector(struct intel_digital_port *intel_dig_port) > > return connector; > > } > > > > -void intel_ddi_init(struct drm_device *dev, enum port port) > > +static int intel_ddi_init_role(struct drm_device *dev, enum port port, > > + int encoder_type, uint32_t saved_port_bits, > > + int max_lanes) > > { > > - struct drm_i915_private *dev_priv = dev->dev_private; > > + struct drm_i915_private *dev_priv = to_i915(dev); > > struct intel_digital_port *intel_dig_port; > > struct intel_encoder *intel_encoder; > > struct drm_encoder *encoder; > > - bool init_hdmi, init_dp; > > - int max_lanes; > > - > > - if (I915_READ(DDI_BUF_CTL(PORT_A)) & DDI_A_4_LANES) { > > - switch (port) { > > - case PORT_A: > > - max_lanes = 4; > > - break; > > - case PORT_E: > > - max_lanes = 0; > > - break; > > - default: > > - max_lanes = 4; > > - break; > > - } > > - } else { > > - switch (port) { > > - case PORT_A: > > - max_lanes = 2; > > - break; > > - case PORT_E: > > - max_lanes = 2; > > - break; > > - default: > > - max_lanes = 4; > > - break; > > - } > > - } > > - > > - init_hdmi = (dev_priv->vbt.ddi_port_info[port].supports_dvi || > > - dev_priv->vbt.ddi_port_info[port].supports_hdmi); > > - init_dp = dev_priv->vbt.ddi_port_info[port].supports_dp; > > - if (!init_dp && !init_hdmi) { > > - DRM_DEBUG_KMS("VBT says port %c is not DVI/HDMI/DP compatible, respect it\n", > > - port_name(port)); > > - return; > > - } > > > > intel_dig_port = kzalloc(sizeof(*intel_dig_port), GFP_KERNEL); > > if (!intel_dig_port) > > - return; > > + return -ENOMEM; > > > > intel_encoder = &intel_dig_port->base; > > encoder = &intel_encoder->base; > > @@ -3287,9 +3337,7 @@ void intel_ddi_init(struct drm_device *dev, enum port port) > > intel_encoder->get_config = intel_ddi_get_config; > > > > intel_dig_port->port = port; > > - intel_dig_port->saved_port_bits = I915_READ(DDI_BUF_CTL(port)) & > > - (DDI_BUF_PORT_REVERSAL | > > - DDI_A_4_LANES); > > + intel_dig_port->saved_port_bits = saved_port_bits; > > intel_dig_port->max_lanes = max_lanes; > > > > /* > > @@ -3306,11 +3354,11 @@ void intel_ddi_init(struct drm_device *dev, enum port port) > > } > > } > > > > - intel_encoder->type = INTEL_OUTPUT_UNKNOWN; > > + intel_encoder->type = encoder_type; > > intel_encoder->crtc_mask = (1 << 0) | (1 << 1) | (1 << 2); > > intel_encoder->cloneable = 0; > > > > - if (init_dp) { > > + if (intel_encoder->type == INTEL_OUTPUT_DISPLAYPORT) { > > if (!intel_ddi_init_dp_connector(intel_dig_port)) > > goto err; > > > > @@ -3327,14 +3375,74 @@ void intel_ddi_init(struct drm_device *dev, enum port port) > > > > /* In theory we don't need the encoder->type check, but leave it just in > > * case we have some really bad VBTs... */ > > - if (intel_encoder->type != INTEL_OUTPUT_EDP && init_hdmi) { > > + if (intel_encoder->type == INTEL_OUTPUT_HDMI) { > > if (!intel_ddi_init_hdmi_connector(intel_dig_port)) > > goto err; > > } > > > > - return; > > + return intel_encoder->type; > > > > err: > > drm_encoder_cleanup(encoder); > > kfree(intel_dig_port); > > + > > + return -ENODEV; > > +} > > + > > +void intel_ddi_init(struct drm_device *dev, enum port port) > > +{ > > + struct drm_i915_private *dev_priv = to_i915(dev); > > + uint32_t saved_port_bits; > > + bool init_hdmi, init_dp; > > + int max_lanes; > > + > > + if (I915_READ(DDI_BUF_CTL(PORT_A)) & DDI_A_4_LANES) { > > + switch (port) { > > + case PORT_A: > > + max_lanes = 4; > > + break; > > + case PORT_E: > > + max_lanes = 0; > > + break; > > + default: > > + max_lanes = 4; > > + break; > > + } > > + } else { > > + switch (port) { > > + case PORT_A: > > + max_lanes = 2; > > + break; > > + case PORT_E: > > + max_lanes = 2; > > + break; > > + default: > > + max_lanes = 4; > > + break; > > + } > > + } > > + > > + init_hdmi = (dev_priv->vbt.ddi_port_info[port].supports_dvi || > > + dev_priv->vbt.ddi_port_info[port].supports_hdmi); > > + init_dp = dev_priv->vbt.ddi_port_info[port].supports_dp; > > + if (!init_dp && !init_hdmi) { > > + DRM_DEBUG_KMS("VBT says port %c is not DVI/HDMI/DP compatible, respect it\n", > > + port_name(port)); > > + return; > > + } > > + > > + saved_port_bits = I915_READ(DDI_BUF_CTL(port)) & > > + (DDI_BUF_PORT_REVERSAL | DDI_A_4_LANES); > > + > > + if (init_dp) { > > + int ret = intel_ddi_init_role(dev, port, INTEL_OUTPUT_DISPLAYPORT, > > + saved_port_bits, max_lanes); > > + /* Don't register the HDMI connector/encoder when we have eDP */ > > + if (ret == INTEL_OUTPUT_EDP) > > + init_hdmi = false; > > + } > > + > > + if (init_hdmi) > > + intel_ddi_init_role(dev, port, INTEL_OUTPUT_HDMI, > > + saved_port_bits, max_lanes); > > } > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > > index bc7aaa3c431e..fc1d7387eb12 100644 > > --- a/drivers/gpu/drm/i915/intel_display.c > > +++ b/drivers/gpu/drm/i915/intel_display.c > > @@ -5252,13 +5252,9 @@ static enum intel_display_power_domain port_to_aux_power_domain(enum port port) > > enum intel_display_power_domain > > intel_display_port_power_domain(struct intel_encoder *intel_encoder) > > { > > - struct drm_device *dev = intel_encoder->base.dev; > > struct intel_digital_port *intel_dig_port; > > > > switch (intel_encoder->type) { > > - case INTEL_OUTPUT_UNKNOWN: > > - /* Only DDI platforms should ever use this output type */ > > - WARN_ON_ONCE(!HAS_DDI(dev)); > > case INTEL_OUTPUT_DISPLAYPORT: > > case INTEL_OUTPUT_HDMI: > > case INTEL_OUTPUT_EDP: > > @@ -5279,20 +5275,9 @@ intel_display_port_power_domain(struct intel_encoder *intel_encoder) > > enum intel_display_power_domain > > intel_display_port_aux_power_domain(struct intel_encoder *intel_encoder) > > { > > - struct drm_device *dev = intel_encoder->base.dev; > > struct intel_digital_port *intel_dig_port; > > > > switch (intel_encoder->type) { > > - case INTEL_OUTPUT_UNKNOWN: > > - case INTEL_OUTPUT_HDMI: > > - /* > > - * Only DDI platforms should ever use these output types. > > - * We can get here after the HDMI detect code has already set > > - * the type of the shared encoder. Since we can't be sure > > - * what's the status of the given connectors, play safe and > > - * run the DP detection too. > > - */ > > - WARN_ON_ONCE(!HAS_DDI(dev)); > > case INTEL_OUTPUT_DISPLAYPORT: > > case INTEL_OUTPUT_EDP: > > intel_dig_port = enc_to_dig_port(&intel_encoder->base); > > @@ -12283,9 +12268,6 @@ static bool check_digital_port_conflicts(struct drm_atomic_state *state) > > > > switch (encoder->type) { > > unsigned int port_mask; > > - case INTEL_OUTPUT_UNKNOWN: > > - if (WARN_ON(!HAS_DDI(dev))) > > - break; > > case INTEL_OUTPUT_DISPLAYPORT: > > case INTEL_OUTPUT_HDMI: > > case INTEL_OUTPUT_EDP: > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > > index 7d354b1e5e5f..1d31aa296aaa 100644 > > --- a/drivers/gpu/drm/i915/intel_dp.c > > +++ b/drivers/gpu/drm/i915/intel_dp.c > > @@ -4601,8 +4601,6 @@ intel_dp_detect(struct drm_connector *connector, bool force) > > > > if (intel_dp->is_mst) { > > /* MST devices are disconnected from a monitor POV */ > > - if (intel_encoder->type != INTEL_OUTPUT_EDP) > > - intel_encoder->type = INTEL_OUTPUT_DISPLAYPORT; > > return connector_status_disconnected; > > } > > > > @@ -4632,8 +4630,6 @@ intel_dp_detect(struct drm_connector *connector, bool force) > > if (ret) { > > /* if we are in MST mode then this connector > > won't appear connected or have anything with EDID on it */ > > - if (intel_encoder->type != INTEL_OUTPUT_EDP) > > - intel_encoder->type = INTEL_OUTPUT_DISPLAYPORT; > > status = connector_status_disconnected; > > goto out; > > } > > @@ -4648,8 +4644,6 @@ intel_dp_detect(struct drm_connector *connector, bool force) > > > > intel_dp_set_edid(intel_dp); > > > > - if (intel_encoder->type != INTEL_OUTPUT_EDP) > > - intel_encoder->type = INTEL_OUTPUT_DISPLAYPORT; > > status = connector_status_connected; > > > > /* Try to read the source of the interrupt */ > > @@ -4692,9 +4686,6 @@ intel_dp_force(struct drm_connector *connector) > > intel_dp_set_edid(intel_dp); > > > > intel_display_power_put(dev_priv, power_domain); > > - > > - if (intel_encoder->type != INTEL_OUTPUT_EDP) > > - intel_encoder->type = INTEL_OUTPUT_DISPLAYPORT; > > } > > > > static int intel_dp_get_modes(struct drm_connector *connector) > > @@ -4969,9 +4960,9 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd) > > enum intel_display_power_domain power_domain; > > enum irqreturn ret = IRQ_NONE; > > > > - if (intel_dig_port->base.type != INTEL_OUTPUT_EDP && > > - intel_dig_port->base.type != INTEL_OUTPUT_HDMI) > > - intel_dig_port->base.type = INTEL_OUTPUT_DISPLAYPORT; > > + if (WARN_ON_ONCE(intel_dig_port->base.type != INTEL_OUTPUT_EDP && > > + intel_dig_port->base.type != INTEL_OUTPUT_DISPLAYPORT)) > > + return IRQ_HANDLED; > > > > if (long_hpd && intel_dig_port->base.type == INTEL_OUTPUT_EDP) { > > /* > > @@ -5815,6 +5806,9 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port, > > enum port port = intel_dig_port->port; > > int type, ret; > > > > + if (WARN_ON(intel_encoder->type != INTEL_OUTPUT_DISPLAYPORT)) > > + return false; > > + > > if (WARN(intel_dig_port->max_lanes < 1, > > "Not enough lanes (%d) for DP on port %c\n", > > intel_dig_port->max_lanes, port_name(port))) > > @@ -5851,11 +5845,6 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port, > > else > > type = DRM_MODE_CONNECTOR_DisplayPort; > > > > - /* > > - * For eDP we always set the encoder type to INTEL_OUTPUT_EDP, but > > - * for DP the encoder type can be set by the caller to > > - * INTEL_OUTPUT_UNKNOWN for DDI, so don't rewrite it. > > - */ > > if (type == DRM_MODE_CONNECTOR_eDP) > > intel_encoder->type = INTEL_OUTPUT_EDP; > > > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > > index a8a84b8c2bac..9e5db3d71e12 100644 > > --- a/drivers/gpu/drm/i915/intel_drv.h > > +++ b/drivers/gpu/drm/i915/intel_drv.h > > @@ -103,8 +103,7 @@ enum intel_output_type { > > INTEL_OUTPUT_DISPLAYPORT = 7, > > INTEL_OUTPUT_EDP = 8, > > INTEL_OUTPUT_DSI = 9, > > - INTEL_OUTPUT_UNKNOWN = 10, > > - INTEL_OUTPUT_DP_MST = 11, > > + INTEL_OUTPUT_DP_MST = 10, > > }; > > > > #define INTEL_DVO_CHIP_NONE 0 > > diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c > > index 895189abfd56..75ea9515a9ce 100644 > > --- a/drivers/gpu/drm/i915/intel_hdmi.c > > +++ b/drivers/gpu/drm/i915/intel_hdmi.c > > @@ -2034,6 +2034,9 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port, > > enum port port = intel_dig_port->port; > > uint8_t alternate_ddc_pin; > > > > + if (WARN_ON(intel_encoder->type != INTEL_OUTPUT_HDMI)) > > + return; > > + > > if (WARN(intel_dig_port->max_lanes < 4, > > "Not enough lanes (%d) for HDMI on port %c\n", > > intel_dig_port->max_lanes, port_name(port))) > > diff --git a/drivers/gpu/drm/i915/intel_opregion.c b/drivers/gpu/drm/i915/intel_opregion.c > > index e362a30776fa..a15459a451c2 100644 > > --- a/drivers/gpu/drm/i915/intel_opregion.c > > +++ b/drivers/gpu/drm/i915/intel_opregion.c > > @@ -360,7 +360,6 @@ int intel_opregion_notify_encoder(struct intel_encoder *intel_encoder, > > case INTEL_OUTPUT_ANALOG: > > type = DISPLAY_TYPE_CRT; > > break; > > - case INTEL_OUTPUT_UNKNOWN: > > case INTEL_OUTPUT_DISPLAYPORT: > > case INTEL_OUTPUT_HDMI: > > case INTEL_OUTPUT_DP_MST: > > -- > > 2.4.10 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch
On Thu, Dec 10, 2015 at 04:10:34PM +0200, Ville Syrjälä wrote: > On Thu, Dec 10, 2015 at 02:47:12PM +0100, Daniel Vetter wrote: > > On Tue, Dec 08, 2015 at 07:59:45PM +0200, ville.syrjala@linux.intel.com wrote: > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > > > Eliminate the troublesome role switching DDI encoder, and just register > > > a separate encoder for each role (DP and HDMI). > > > > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > Caveat about max_lanes and pre-DDI platforms still apply. Caveats been resolved, including my question about port E. Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> > > > > > --- > > > drivers/gpu/drm/i915/intel_ddi.c | 232 +++++++++++++++++++++++++--------- > > > drivers/gpu/drm/i915/intel_display.c | 18 --- > > > drivers/gpu/drm/i915/intel_dp.c | 23 +--- > > > drivers/gpu/drm/i915/intel_drv.h | 3 +- > > > drivers/gpu/drm/i915/intel_hdmi.c | 3 + > > > drivers/gpu/drm/i915/intel_opregion.c | 1 - > > > 6 files changed, 180 insertions(+), 100 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c > > > index 074121efb265..5f008f0fdc13 100644 > > > --- a/drivers/gpu/drm/i915/intel_ddi.c > > > +++ b/drivers/gpu/drm/i915/intel_ddi.c > > > @@ -318,7 +318,6 @@ static void ddi_get_encoder_port(struct intel_encoder *intel_encoder, > > > case INTEL_OUTPUT_DISPLAYPORT: > > > case INTEL_OUTPUT_EDP: > > > case INTEL_OUTPUT_HDMI: > > > - case INTEL_OUTPUT_UNKNOWN: > > > *dig_port = enc_to_dig_port(encoder); > > > *port = (*dig_port)->port; > > > break; > > > @@ -1942,19 +1941,19 @@ bool intel_ddi_connector_get_hw_state(struct intel_connector *intel_connector) > > > switch (tmp & TRANS_DDI_MODE_SELECT_MASK) { > > > case TRANS_DDI_MODE_SELECT_HDMI: > > > case TRANS_DDI_MODE_SELECT_DVI: > > > - return (type == DRM_MODE_CONNECTOR_HDMIA); > > > + return type == DRM_MODE_CONNECTOR_HDMIA; > > > > > > case TRANS_DDI_MODE_SELECT_DP_SST: > > > - if (type == DRM_MODE_CONNECTOR_eDP) > > > - return true; > > > - return (type == DRM_MODE_CONNECTOR_DisplayPort); > > > + return type == DRM_MODE_CONNECTOR_DisplayPort || > > > + type == DRM_MODE_CONNECTOR_eDP; > > > + > > > case TRANS_DDI_MODE_SELECT_DP_MST: > > > /* if the transcoder is in MST state then > > > * connector isn't connected */ > > > return false; > > > > > > case TRANS_DDI_MODE_SELECT_FDI: > > > - return (type == DRM_MODE_CONNECTOR_VGA); > > > + return type == DRM_MODE_CONNECTOR_VGA; > > > > > > default: > > > return false; > > > @@ -1981,8 +1980,23 @@ bool intel_ddi_get_hw_state(struct intel_encoder *encoder, > > > return false; > > > > > > if (port == PORT_A) { > > > + WARN_ON(encoder->type != INTEL_OUTPUT_EDP); > > > + > > > tmp = I915_READ(TRANS_DDI_FUNC_CTL(TRANSCODER_EDP)); > > > > > > + if ((tmp & TRANS_DDI_FUNC_ENABLE) == 0) > > > + goto out; > > > + > > > + switch (tmp & TRANS_DDI_MODE_SELECT_MASK) { > > > + case TRANS_DDI_MODE_SELECT_DP_SST: > > > + break; > > > + default: > > > + WARN(1, > > > + "Bad transcoder EDP DDI mode 0x%08x for port %c\n", > > > + tmp, port_name(port)); > > > + return false; > > > + } > > > + > > > switch (tmp & TRANS_DDI_EDP_INPUT_MASK) { > > > case TRANS_DDI_EDP_INPUT_A_ON: > > > case TRANS_DDI_EDP_INPUT_A_ONOFF: > > > @@ -1994,25 +2008,98 @@ bool intel_ddi_get_hw_state(struct intel_encoder *encoder, > > > case TRANS_DDI_EDP_INPUT_C_ONOFF: > > > *pipe = PIPE_C; > > > break; > > > + default: > > > + WARN(1, > > > + "Bad transcoder EDP input select 0x%08x for port %c\n", > > > + tmp, port_name(port)); > > > + return false; > > > } > > > > > > return true; > > > } else { > > > + int num_mst_transcoders = 0; > > > + int num_sst_transcoders = 0; > > > + int num_fdi_transcoders = 0; > > > + int num_hdmi_transcoders = 0; > > > + int num_transcoders = 0; > > > + bool enabled = false; > > > + > > > for (i = TRANSCODER_A; i <= TRANSCODER_C; i++) { > > > tmp = I915_READ(TRANS_DDI_FUNC_CTL(i)); > > > > > > - if ((tmp & TRANS_DDI_PORT_MASK) > > > - == TRANS_DDI_SELECT_PORT(port)) { > > > - if ((tmp & TRANS_DDI_MODE_SELECT_MASK) == TRANS_DDI_MODE_SELECT_DP_MST) > > > - return false; > > > + if ((tmp & TRANS_DDI_FUNC_ENABLE) == 0) > > > + continue; > > > + > > > + if ((tmp & TRANS_DDI_PORT_MASK) != TRANS_DDI_SELECT_PORT(port)) > > > + continue; > > > + > > > + if ((tmp & TRANS_DDI_MODE_SELECT_MASK) == TRANS_DDI_MODE_SELECT_DP_MST) { > > > + num_mst_transcoders++; > > > + WARN_ON(port == PORT_E); > > > + continue; > > > + } > > > + > > > + > > > + switch (tmp & TRANS_DDI_MODE_SELECT_MASK) { > > > + case TRANS_DDI_MODE_SELECT_DP_SST: > > > + WARN_ON(port == PORT_E && INTEL_INFO(dev_priv)->gen < 9); > > > + > > > + num_sst_transcoders++; > > > + if (encoder->type == INTEL_OUTPUT_DISPLAYPORT || > > > + encoder->type == INTEL_OUTPUT_EDP) { > > > + enabled = true; > > > + *pipe = i; > > > + } > > > + break; > > > + case TRANS_DDI_MODE_SELECT_HDMI: > > > + case TRANS_DDI_MODE_SELECT_DVI: > > > + WARN_ON(port == PORT_E); > > > > Hm, previous patches made it look like hdmi on port E is possible - at > > least you added piles of checks to make sure we have at least 4 lanes > > everywhere. Am I mistaken? > > DDI E can only have two lanes at most. So it can't do HDMI. Actually > it's documented that it can only do FDI (or SST on SKL+). Now that I > think about it, I'm not sure an external DP port with < 4 lanes is > even legal, so perhaps in practice it can only be eDP or FDI? > > > > > > + > > > + num_hdmi_transcoders++; > > > + if (encoder->type == INTEL_OUTPUT_HDMI) { > > > + enabled = true; > > > + *pipe = i; > > > + } > > > + break; > > > + > > > + case TRANS_DDI_MODE_SELECT_FDI: > > > + WARN_ON(port != PORT_E || INTEL_INFO(dev_priv)->gen >= 9); > > > + > > > + num_fdi_transcoders++; > > > + if (encoder->type == INTEL_OUTPUT_ANALOG) { > > > + enabled = true; > > > + *pipe = i; > > > + } > > > + break; > > > > > > - *pipe = i; > > > - return true; > > > + default: > > > + WARN(1, "Bad transcoder %c DDI mode 0x%08x for port %c\n", > > > + transcoder_name(i), tmp, port_name(port)); > > > + return false; > > > } > > > } > > > + > > > + num_transcoders = num_sst_transcoders + > > > + num_fdi_transcoders + num_hdmi_transcoders; > > > + > > > + if (WARN(num_transcoders && num_mst_transcoders, > > > + "MST and non-MST transcoders enabled for port %c (%d sst, %d mst, %d fdi, %d hdmi)\n", > > > + port_name(port), num_sst_transcoders, num_mst_transcoders, > > > + num_fdi_transcoders, num_hdmi_transcoders)) > > > + return false; > > > + > > > + if (WARN(num_transcoders > 1, > > > + "Multiple transcoders enabled for port %c (%d sst, %d mst, %d fdi, %d hdmi)\n", > > > + port_name(port), num_sst_transcoders, num_mst_transcoders, > > > + num_fdi_transcoders, num_hdmi_transcoders)) > > > + return false; > > > + > > > + if (enabled) > > > + return true; > > > > This is too big and needs to be extracted into a static function. > > > > Otherwise I didn't spot anything, and getting rid of OUTPUT_UNKNOWN is > > really nice. I also wonder whether we should long term rework mst to use > > the fake encoder as the real one. The fake encoder was just done to not > > confuse the ddi encoder, but now that we have multiples of those we might > > as well embrace that everywhere. > > Hmm. One idea for MST I've had is that we'd pull out the main link > management from the mst code, and instead use atomic to set up the main > link as just another crtc (would need a fake crtc for that), and then > the streams would just deal with their fake encoders and not mess about > with the main encoder at all. But sounds like you've had pretty much the > opposite idea. > > > > > Cheers, Daniel > > > > > } > > > > > > - DRM_DEBUG_KMS("No pipe for ddi port %c found\n", port_name(port)); > > > +out: > > > + DRM_DEBUG_KMS("No pipe for DDI port %c found\n", port_name(port)); > > > > > > return false; > > > } > > > @@ -3174,8 +3261,6 @@ static bool intel_ddi_compute_config(struct intel_encoder *encoder, > > > int type = encoder->type; > > > int port = intel_ddi_get_encoder_port(encoder); > > > > > > - WARN(type == INTEL_OUTPUT_UNKNOWN, "compute_config() on unknown output!\n"); > > > - > > > if (port == PORT_A) > > > pipe_config->cpu_transcoder = TRANSCODER_EDP; > > > > > > @@ -3224,53 +3309,18 @@ intel_ddi_init_hdmi_connector(struct intel_digital_port *intel_dig_port) > > > return connector; > > > } > > > > > > -void intel_ddi_init(struct drm_device *dev, enum port port) > > > +static int intel_ddi_init_role(struct drm_device *dev, enum port port, > > > + int encoder_type, uint32_t saved_port_bits, > > > + int max_lanes) > > > { > > > - struct drm_i915_private *dev_priv = dev->dev_private; > > > + struct drm_i915_private *dev_priv = to_i915(dev); > > > struct intel_digital_port *intel_dig_port; > > > struct intel_encoder *intel_encoder; > > > struct drm_encoder *encoder; > > > - bool init_hdmi, init_dp; > > > - int max_lanes; > > > - > > > - if (I915_READ(DDI_BUF_CTL(PORT_A)) & DDI_A_4_LANES) { > > > - switch (port) { > > > - case PORT_A: > > > - max_lanes = 4; > > > - break; > > > - case PORT_E: > > > - max_lanes = 0; > > > - break; > > > - default: > > > - max_lanes = 4; > > > - break; > > > - } > > > - } else { > > > - switch (port) { > > > - case PORT_A: > > > - max_lanes = 2; > > > - break; > > > - case PORT_E: > > > - max_lanes = 2; > > > - break; > > > - default: > > > - max_lanes = 4; > > > - break; > > > - } > > > - } > > > - > > > - init_hdmi = (dev_priv->vbt.ddi_port_info[port].supports_dvi || > > > - dev_priv->vbt.ddi_port_info[port].supports_hdmi); > > > - init_dp = dev_priv->vbt.ddi_port_info[port].supports_dp; > > > - if (!init_dp && !init_hdmi) { > > > - DRM_DEBUG_KMS("VBT says port %c is not DVI/HDMI/DP compatible, respect it\n", > > > - port_name(port)); > > > - return; > > > - } > > > > > > intel_dig_port = kzalloc(sizeof(*intel_dig_port), GFP_KERNEL); > > > if (!intel_dig_port) > > > - return; > > > + return -ENOMEM; > > > > > > intel_encoder = &intel_dig_port->base; > > > encoder = &intel_encoder->base; > > > @@ -3287,9 +3337,7 @@ void intel_ddi_init(struct drm_device *dev, enum port port) > > > intel_encoder->get_config = intel_ddi_get_config; > > > > > > intel_dig_port->port = port; > > > - intel_dig_port->saved_port_bits = I915_READ(DDI_BUF_CTL(port)) & > > > - (DDI_BUF_PORT_REVERSAL | > > > - DDI_A_4_LANES); > > > + intel_dig_port->saved_port_bits = saved_port_bits; > > > intel_dig_port->max_lanes = max_lanes; > > > > > > /* > > > @@ -3306,11 +3354,11 @@ void intel_ddi_init(struct drm_device *dev, enum port port) > > > } > > > } > > > > > > - intel_encoder->type = INTEL_OUTPUT_UNKNOWN; > > > + intel_encoder->type = encoder_type; > > > intel_encoder->crtc_mask = (1 << 0) | (1 << 1) | (1 << 2); > > > intel_encoder->cloneable = 0; > > > > > > - if (init_dp) { > > > + if (intel_encoder->type == INTEL_OUTPUT_DISPLAYPORT) { > > > if (!intel_ddi_init_dp_connector(intel_dig_port)) > > > goto err; > > > > > > @@ -3327,14 +3375,74 @@ void intel_ddi_init(struct drm_device *dev, enum port port) > > > > > > /* In theory we don't need the encoder->type check, but leave it just in > > > * case we have some really bad VBTs... */ > > > - if (intel_encoder->type != INTEL_OUTPUT_EDP && init_hdmi) { > > > + if (intel_encoder->type == INTEL_OUTPUT_HDMI) { > > > if (!intel_ddi_init_hdmi_connector(intel_dig_port)) > > > goto err; > > > } > > > > > > - return; > > > + return intel_encoder->type; > > > > > > err: > > > drm_encoder_cleanup(encoder); > > > kfree(intel_dig_port); > > > + > > > + return -ENODEV; > > > +} > > > + > > > +void intel_ddi_init(struct drm_device *dev, enum port port) > > > +{ > > > + struct drm_i915_private *dev_priv = to_i915(dev); > > > + uint32_t saved_port_bits; > > > + bool init_hdmi, init_dp; > > > + int max_lanes; > > > + > > > + if (I915_READ(DDI_BUF_CTL(PORT_A)) & DDI_A_4_LANES) { > > > + switch (port) { > > > + case PORT_A: > > > + max_lanes = 4; > > > + break; > > > + case PORT_E: > > > + max_lanes = 0; > > > + break; > > > + default: > > > + max_lanes = 4; > > > + break; > > > + } > > > + } else { > > > + switch (port) { > > > + case PORT_A: > > > + max_lanes = 2; > > > + break; > > > + case PORT_E: > > > + max_lanes = 2; > > > + break; > > > + default: > > > + max_lanes = 4; > > > + break; > > > + } > > > + } > > > + > > > + init_hdmi = (dev_priv->vbt.ddi_port_info[port].supports_dvi || > > > + dev_priv->vbt.ddi_port_info[port].supports_hdmi); > > > + init_dp = dev_priv->vbt.ddi_port_info[port].supports_dp; > > > + if (!init_dp && !init_hdmi) { > > > + DRM_DEBUG_KMS("VBT says port %c is not DVI/HDMI/DP compatible, respect it\n", > > > + port_name(port)); > > > + return; > > > + } > > > + > > > + saved_port_bits = I915_READ(DDI_BUF_CTL(port)) & > > > + (DDI_BUF_PORT_REVERSAL | DDI_A_4_LANES); > > > + > > > + if (init_dp) { > > > + int ret = intel_ddi_init_role(dev, port, INTEL_OUTPUT_DISPLAYPORT, > > > + saved_port_bits, max_lanes); > > > + /* Don't register the HDMI connector/encoder when we have eDP */ > > > + if (ret == INTEL_OUTPUT_EDP) > > > + init_hdmi = false; > > > + } > > > + > > > + if (init_hdmi) > > > + intel_ddi_init_role(dev, port, INTEL_OUTPUT_HDMI, > > > + saved_port_bits, max_lanes); > > > } > > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > > > index bc7aaa3c431e..fc1d7387eb12 100644 > > > --- a/drivers/gpu/drm/i915/intel_display.c > > > +++ b/drivers/gpu/drm/i915/intel_display.c > > > @@ -5252,13 +5252,9 @@ static enum intel_display_power_domain port_to_aux_power_domain(enum port port) > > > enum intel_display_power_domain > > > intel_display_port_power_domain(struct intel_encoder *intel_encoder) > > > { > > > - struct drm_device *dev = intel_encoder->base.dev; > > > struct intel_digital_port *intel_dig_port; > > > > > > switch (intel_encoder->type) { > > > - case INTEL_OUTPUT_UNKNOWN: > > > - /* Only DDI platforms should ever use this output type */ > > > - WARN_ON_ONCE(!HAS_DDI(dev)); > > > case INTEL_OUTPUT_DISPLAYPORT: > > > case INTEL_OUTPUT_HDMI: > > > case INTEL_OUTPUT_EDP: > > > @@ -5279,20 +5275,9 @@ intel_display_port_power_domain(struct intel_encoder *intel_encoder) > > > enum intel_display_power_domain > > > intel_display_port_aux_power_domain(struct intel_encoder *intel_encoder) > > > { > > > - struct drm_device *dev = intel_encoder->base.dev; > > > struct intel_digital_port *intel_dig_port; > > > > > > switch (intel_encoder->type) { > > > - case INTEL_OUTPUT_UNKNOWN: > > > - case INTEL_OUTPUT_HDMI: > > > - /* > > > - * Only DDI platforms should ever use these output types. > > > - * We can get here after the HDMI detect code has already set > > > - * the type of the shared encoder. Since we can't be sure > > > - * what's the status of the given connectors, play safe and > > > - * run the DP detection too. > > > - */ > > > - WARN_ON_ONCE(!HAS_DDI(dev)); > > > case INTEL_OUTPUT_DISPLAYPORT: > > > case INTEL_OUTPUT_EDP: > > > intel_dig_port = enc_to_dig_port(&intel_encoder->base); > > > @@ -12283,9 +12268,6 @@ static bool check_digital_port_conflicts(struct drm_atomic_state *state) > > > > > > switch (encoder->type) { > > > unsigned int port_mask; > > > - case INTEL_OUTPUT_UNKNOWN: > > > - if (WARN_ON(!HAS_DDI(dev))) > > > - break; > > > case INTEL_OUTPUT_DISPLAYPORT: > > > case INTEL_OUTPUT_HDMI: > > > case INTEL_OUTPUT_EDP: > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > > > index 7d354b1e5e5f..1d31aa296aaa 100644 > > > --- a/drivers/gpu/drm/i915/intel_dp.c > > > +++ b/drivers/gpu/drm/i915/intel_dp.c > > > @@ -4601,8 +4601,6 @@ intel_dp_detect(struct drm_connector *connector, bool force) > > > > > > if (intel_dp->is_mst) { > > > /* MST devices are disconnected from a monitor POV */ > > > - if (intel_encoder->type != INTEL_OUTPUT_EDP) > > > - intel_encoder->type = INTEL_OUTPUT_DISPLAYPORT; > > > return connector_status_disconnected; > > > } > > > > > > @@ -4632,8 +4630,6 @@ intel_dp_detect(struct drm_connector *connector, bool force) > > > if (ret) { > > > /* if we are in MST mode then this connector > > > won't appear connected or have anything with EDID on it */ > > > - if (intel_encoder->type != INTEL_OUTPUT_EDP) > > > - intel_encoder->type = INTEL_OUTPUT_DISPLAYPORT; > > > status = connector_status_disconnected; > > > goto out; > > > } > > > @@ -4648,8 +4644,6 @@ intel_dp_detect(struct drm_connector *connector, bool force) > > > > > > intel_dp_set_edid(intel_dp); > > > > > > - if (intel_encoder->type != INTEL_OUTPUT_EDP) > > > - intel_encoder->type = INTEL_OUTPUT_DISPLAYPORT; > > > status = connector_status_connected; > > > > > > /* Try to read the source of the interrupt */ > > > @@ -4692,9 +4686,6 @@ intel_dp_force(struct drm_connector *connector) > > > intel_dp_set_edid(intel_dp); > > > > > > intel_display_power_put(dev_priv, power_domain); > > > - > > > - if (intel_encoder->type != INTEL_OUTPUT_EDP) > > > - intel_encoder->type = INTEL_OUTPUT_DISPLAYPORT; > > > } > > > > > > static int intel_dp_get_modes(struct drm_connector *connector) > > > @@ -4969,9 +4960,9 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd) > > > enum intel_display_power_domain power_domain; > > > enum irqreturn ret = IRQ_NONE; > > > > > > - if (intel_dig_port->base.type != INTEL_OUTPUT_EDP && > > > - intel_dig_port->base.type != INTEL_OUTPUT_HDMI) > > > - intel_dig_port->base.type = INTEL_OUTPUT_DISPLAYPORT; > > > + if (WARN_ON_ONCE(intel_dig_port->base.type != INTEL_OUTPUT_EDP && > > > + intel_dig_port->base.type != INTEL_OUTPUT_DISPLAYPORT)) > > > + return IRQ_HANDLED; > > > > > > if (long_hpd && intel_dig_port->base.type == INTEL_OUTPUT_EDP) { > > > /* > > > @@ -5815,6 +5806,9 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port, > > > enum port port = intel_dig_port->port; > > > int type, ret; > > > > > > + if (WARN_ON(intel_encoder->type != INTEL_OUTPUT_DISPLAYPORT)) > > > + return false; > > > + > > > if (WARN(intel_dig_port->max_lanes < 1, > > > "Not enough lanes (%d) for DP on port %c\n", > > > intel_dig_port->max_lanes, port_name(port))) > > > @@ -5851,11 +5845,6 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port, > > > else > > > type = DRM_MODE_CONNECTOR_DisplayPort; > > > > > > - /* > > > - * For eDP we always set the encoder type to INTEL_OUTPUT_EDP, but > > > - * for DP the encoder type can be set by the caller to > > > - * INTEL_OUTPUT_UNKNOWN for DDI, so don't rewrite it. > > > - */ > > > if (type == DRM_MODE_CONNECTOR_eDP) > > > intel_encoder->type = INTEL_OUTPUT_EDP; > > > > > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > > > index a8a84b8c2bac..9e5db3d71e12 100644 > > > --- a/drivers/gpu/drm/i915/intel_drv.h > > > +++ b/drivers/gpu/drm/i915/intel_drv.h > > > @@ -103,8 +103,7 @@ enum intel_output_type { > > > INTEL_OUTPUT_DISPLAYPORT = 7, > > > INTEL_OUTPUT_EDP = 8, > > > INTEL_OUTPUT_DSI = 9, > > > - INTEL_OUTPUT_UNKNOWN = 10, > > > - INTEL_OUTPUT_DP_MST = 11, > > > + INTEL_OUTPUT_DP_MST = 10, > > > }; > > > > > > #define INTEL_DVO_CHIP_NONE 0 > > > diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c > > > index 895189abfd56..75ea9515a9ce 100644 > > > --- a/drivers/gpu/drm/i915/intel_hdmi.c > > > +++ b/drivers/gpu/drm/i915/intel_hdmi.c > > > @@ -2034,6 +2034,9 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port, > > > enum port port = intel_dig_port->port; > > > uint8_t alternate_ddc_pin; > > > > > > + if (WARN_ON(intel_encoder->type != INTEL_OUTPUT_HDMI)) > > > + return; > > > + > > > if (WARN(intel_dig_port->max_lanes < 4, > > > "Not enough lanes (%d) for HDMI on port %c\n", > > > intel_dig_port->max_lanes, port_name(port))) > > > diff --git a/drivers/gpu/drm/i915/intel_opregion.c b/drivers/gpu/drm/i915/intel_opregion.c > > > index e362a30776fa..a15459a451c2 100644 > > > --- a/drivers/gpu/drm/i915/intel_opregion.c > > > +++ b/drivers/gpu/drm/i915/intel_opregion.c > > > @@ -360,7 +360,6 @@ int intel_opregion_notify_encoder(struct intel_encoder *intel_encoder, > > > case INTEL_OUTPUT_ANALOG: > > > type = DISPLAY_TYPE_CRT; > > > break; > > > - case INTEL_OUTPUT_UNKNOWN: > > > case INTEL_OUTPUT_DISPLAYPORT: > > > case INTEL_OUTPUT_HDMI: > > > case INTEL_OUTPUT_DP_MST: > > > -- > > > 2.4.10 > > > > > > _______________________________________________ > > > Intel-gfx mailing list > > > Intel-gfx@lists.freedesktop.org > > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > > > -- > > Daniel Vetter > > Software Engineer, Intel Corporation > > http://blog.ffwll.ch > > -- > Ville Syrjälä > Intel OTC
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c index 074121efb265..5f008f0fdc13 100644 --- a/drivers/gpu/drm/i915/intel_ddi.c +++ b/drivers/gpu/drm/i915/intel_ddi.c @@ -318,7 +318,6 @@ static void ddi_get_encoder_port(struct intel_encoder *intel_encoder, case INTEL_OUTPUT_DISPLAYPORT: case INTEL_OUTPUT_EDP: case INTEL_OUTPUT_HDMI: - case INTEL_OUTPUT_UNKNOWN: *dig_port = enc_to_dig_port(encoder); *port = (*dig_port)->port; break; @@ -1942,19 +1941,19 @@ bool intel_ddi_connector_get_hw_state(struct intel_connector *intel_connector) switch (tmp & TRANS_DDI_MODE_SELECT_MASK) { case TRANS_DDI_MODE_SELECT_HDMI: case TRANS_DDI_MODE_SELECT_DVI: - return (type == DRM_MODE_CONNECTOR_HDMIA); + return type == DRM_MODE_CONNECTOR_HDMIA; case TRANS_DDI_MODE_SELECT_DP_SST: - if (type == DRM_MODE_CONNECTOR_eDP) - return true; - return (type == DRM_MODE_CONNECTOR_DisplayPort); + return type == DRM_MODE_CONNECTOR_DisplayPort || + type == DRM_MODE_CONNECTOR_eDP; + case TRANS_DDI_MODE_SELECT_DP_MST: /* if the transcoder is in MST state then * connector isn't connected */ return false; case TRANS_DDI_MODE_SELECT_FDI: - return (type == DRM_MODE_CONNECTOR_VGA); + return type == DRM_MODE_CONNECTOR_VGA; default: return false; @@ -1981,8 +1980,23 @@ bool intel_ddi_get_hw_state(struct intel_encoder *encoder, return false; if (port == PORT_A) { + WARN_ON(encoder->type != INTEL_OUTPUT_EDP); + tmp = I915_READ(TRANS_DDI_FUNC_CTL(TRANSCODER_EDP)); + if ((tmp & TRANS_DDI_FUNC_ENABLE) == 0) + goto out; + + switch (tmp & TRANS_DDI_MODE_SELECT_MASK) { + case TRANS_DDI_MODE_SELECT_DP_SST: + break; + default: + WARN(1, + "Bad transcoder EDP DDI mode 0x%08x for port %c\n", + tmp, port_name(port)); + return false; + } + switch (tmp & TRANS_DDI_EDP_INPUT_MASK) { case TRANS_DDI_EDP_INPUT_A_ON: case TRANS_DDI_EDP_INPUT_A_ONOFF: @@ -1994,25 +2008,98 @@ bool intel_ddi_get_hw_state(struct intel_encoder *encoder, case TRANS_DDI_EDP_INPUT_C_ONOFF: *pipe = PIPE_C; break; + default: + WARN(1, + "Bad transcoder EDP input select 0x%08x for port %c\n", + tmp, port_name(port)); + return false; } return true; } else { + int num_mst_transcoders = 0; + int num_sst_transcoders = 0; + int num_fdi_transcoders = 0; + int num_hdmi_transcoders = 0; + int num_transcoders = 0; + bool enabled = false; + for (i = TRANSCODER_A; i <= TRANSCODER_C; i++) { tmp = I915_READ(TRANS_DDI_FUNC_CTL(i)); - if ((tmp & TRANS_DDI_PORT_MASK) - == TRANS_DDI_SELECT_PORT(port)) { - if ((tmp & TRANS_DDI_MODE_SELECT_MASK) == TRANS_DDI_MODE_SELECT_DP_MST) - return false; + if ((tmp & TRANS_DDI_FUNC_ENABLE) == 0) + continue; + + if ((tmp & TRANS_DDI_PORT_MASK) != TRANS_DDI_SELECT_PORT(port)) + continue; + + if ((tmp & TRANS_DDI_MODE_SELECT_MASK) == TRANS_DDI_MODE_SELECT_DP_MST) { + num_mst_transcoders++; + WARN_ON(port == PORT_E); + continue; + } + + + switch (tmp & TRANS_DDI_MODE_SELECT_MASK) { + case TRANS_DDI_MODE_SELECT_DP_SST: + WARN_ON(port == PORT_E && INTEL_INFO(dev_priv)->gen < 9); + + num_sst_transcoders++; + if (encoder->type == INTEL_OUTPUT_DISPLAYPORT || + encoder->type == INTEL_OUTPUT_EDP) { + enabled = true; + *pipe = i; + } + break; + case TRANS_DDI_MODE_SELECT_HDMI: + case TRANS_DDI_MODE_SELECT_DVI: + WARN_ON(port == PORT_E); + + num_hdmi_transcoders++; + if (encoder->type == INTEL_OUTPUT_HDMI) { + enabled = true; + *pipe = i; + } + break; + + case TRANS_DDI_MODE_SELECT_FDI: + WARN_ON(port != PORT_E || INTEL_INFO(dev_priv)->gen >= 9); + + num_fdi_transcoders++; + if (encoder->type == INTEL_OUTPUT_ANALOG) { + enabled = true; + *pipe = i; + } + break; - *pipe = i; - return true; + default: + WARN(1, "Bad transcoder %c DDI mode 0x%08x for port %c\n", + transcoder_name(i), tmp, port_name(port)); + return false; } } + + num_transcoders = num_sst_transcoders + + num_fdi_transcoders + num_hdmi_transcoders; + + if (WARN(num_transcoders && num_mst_transcoders, + "MST and non-MST transcoders enabled for port %c (%d sst, %d mst, %d fdi, %d hdmi)\n", + port_name(port), num_sst_transcoders, num_mst_transcoders, + num_fdi_transcoders, num_hdmi_transcoders)) + return false; + + if (WARN(num_transcoders > 1, + "Multiple transcoders enabled for port %c (%d sst, %d mst, %d fdi, %d hdmi)\n", + port_name(port), num_sst_transcoders, num_mst_transcoders, + num_fdi_transcoders, num_hdmi_transcoders)) + return false; + + if (enabled) + return true; } - DRM_DEBUG_KMS("No pipe for ddi port %c found\n", port_name(port)); +out: + DRM_DEBUG_KMS("No pipe for DDI port %c found\n", port_name(port)); return false; } @@ -3174,8 +3261,6 @@ static bool intel_ddi_compute_config(struct intel_encoder *encoder, int type = encoder->type; int port = intel_ddi_get_encoder_port(encoder); - WARN(type == INTEL_OUTPUT_UNKNOWN, "compute_config() on unknown output!\n"); - if (port == PORT_A) pipe_config->cpu_transcoder = TRANSCODER_EDP; @@ -3224,53 +3309,18 @@ intel_ddi_init_hdmi_connector(struct intel_digital_port *intel_dig_port) return connector; } -void intel_ddi_init(struct drm_device *dev, enum port port) +static int intel_ddi_init_role(struct drm_device *dev, enum port port, + int encoder_type, uint32_t saved_port_bits, + int max_lanes) { - struct drm_i915_private *dev_priv = dev->dev_private; + struct drm_i915_private *dev_priv = to_i915(dev); struct intel_digital_port *intel_dig_port; struct intel_encoder *intel_encoder; struct drm_encoder *encoder; - bool init_hdmi, init_dp; - int max_lanes; - - if (I915_READ(DDI_BUF_CTL(PORT_A)) & DDI_A_4_LANES) { - switch (port) { - case PORT_A: - max_lanes = 4; - break; - case PORT_E: - max_lanes = 0; - break; - default: - max_lanes = 4; - break; - } - } else { - switch (port) { - case PORT_A: - max_lanes = 2; - break; - case PORT_E: - max_lanes = 2; - break; - default: - max_lanes = 4; - break; - } - } - - init_hdmi = (dev_priv->vbt.ddi_port_info[port].supports_dvi || - dev_priv->vbt.ddi_port_info[port].supports_hdmi); - init_dp = dev_priv->vbt.ddi_port_info[port].supports_dp; - if (!init_dp && !init_hdmi) { - DRM_DEBUG_KMS("VBT says port %c is not DVI/HDMI/DP compatible, respect it\n", - port_name(port)); - return; - } intel_dig_port = kzalloc(sizeof(*intel_dig_port), GFP_KERNEL); if (!intel_dig_port) - return; + return -ENOMEM; intel_encoder = &intel_dig_port->base; encoder = &intel_encoder->base; @@ -3287,9 +3337,7 @@ void intel_ddi_init(struct drm_device *dev, enum port port) intel_encoder->get_config = intel_ddi_get_config; intel_dig_port->port = port; - intel_dig_port->saved_port_bits = I915_READ(DDI_BUF_CTL(port)) & - (DDI_BUF_PORT_REVERSAL | - DDI_A_4_LANES); + intel_dig_port->saved_port_bits = saved_port_bits; intel_dig_port->max_lanes = max_lanes; /* @@ -3306,11 +3354,11 @@ void intel_ddi_init(struct drm_device *dev, enum port port) } } - intel_encoder->type = INTEL_OUTPUT_UNKNOWN; + intel_encoder->type = encoder_type; intel_encoder->crtc_mask = (1 << 0) | (1 << 1) | (1 << 2); intel_encoder->cloneable = 0; - if (init_dp) { + if (intel_encoder->type == INTEL_OUTPUT_DISPLAYPORT) { if (!intel_ddi_init_dp_connector(intel_dig_port)) goto err; @@ -3327,14 +3375,74 @@ void intel_ddi_init(struct drm_device *dev, enum port port) /* In theory we don't need the encoder->type check, but leave it just in * case we have some really bad VBTs... */ - if (intel_encoder->type != INTEL_OUTPUT_EDP && init_hdmi) { + if (intel_encoder->type == INTEL_OUTPUT_HDMI) { if (!intel_ddi_init_hdmi_connector(intel_dig_port)) goto err; } - return; + return intel_encoder->type; err: drm_encoder_cleanup(encoder); kfree(intel_dig_port); + + return -ENODEV; +} + +void intel_ddi_init(struct drm_device *dev, enum port port) +{ + struct drm_i915_private *dev_priv = to_i915(dev); + uint32_t saved_port_bits; + bool init_hdmi, init_dp; + int max_lanes; + + if (I915_READ(DDI_BUF_CTL(PORT_A)) & DDI_A_4_LANES) { + switch (port) { + case PORT_A: + max_lanes = 4; + break; + case PORT_E: + max_lanes = 0; + break; + default: + max_lanes = 4; + break; + } + } else { + switch (port) { + case PORT_A: + max_lanes = 2; + break; + case PORT_E: + max_lanes = 2; + break; + default: + max_lanes = 4; + break; + } + } + + init_hdmi = (dev_priv->vbt.ddi_port_info[port].supports_dvi || + dev_priv->vbt.ddi_port_info[port].supports_hdmi); + init_dp = dev_priv->vbt.ddi_port_info[port].supports_dp; + if (!init_dp && !init_hdmi) { + DRM_DEBUG_KMS("VBT says port %c is not DVI/HDMI/DP compatible, respect it\n", + port_name(port)); + return; + } + + saved_port_bits = I915_READ(DDI_BUF_CTL(port)) & + (DDI_BUF_PORT_REVERSAL | DDI_A_4_LANES); + + if (init_dp) { + int ret = intel_ddi_init_role(dev, port, INTEL_OUTPUT_DISPLAYPORT, + saved_port_bits, max_lanes); + /* Don't register the HDMI connector/encoder when we have eDP */ + if (ret == INTEL_OUTPUT_EDP) + init_hdmi = false; + } + + if (init_hdmi) + intel_ddi_init_role(dev, port, INTEL_OUTPUT_HDMI, + saved_port_bits, max_lanes); } diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index bc7aaa3c431e..fc1d7387eb12 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -5252,13 +5252,9 @@ static enum intel_display_power_domain port_to_aux_power_domain(enum port port) enum intel_display_power_domain intel_display_port_power_domain(struct intel_encoder *intel_encoder) { - struct drm_device *dev = intel_encoder->base.dev; struct intel_digital_port *intel_dig_port; switch (intel_encoder->type) { - case INTEL_OUTPUT_UNKNOWN: - /* Only DDI platforms should ever use this output type */ - WARN_ON_ONCE(!HAS_DDI(dev)); case INTEL_OUTPUT_DISPLAYPORT: case INTEL_OUTPUT_HDMI: case INTEL_OUTPUT_EDP: @@ -5279,20 +5275,9 @@ intel_display_port_power_domain(struct intel_encoder *intel_encoder) enum intel_display_power_domain intel_display_port_aux_power_domain(struct intel_encoder *intel_encoder) { - struct drm_device *dev = intel_encoder->base.dev; struct intel_digital_port *intel_dig_port; switch (intel_encoder->type) { - case INTEL_OUTPUT_UNKNOWN: - case INTEL_OUTPUT_HDMI: - /* - * Only DDI platforms should ever use these output types. - * We can get here after the HDMI detect code has already set - * the type of the shared encoder. Since we can't be sure - * what's the status of the given connectors, play safe and - * run the DP detection too. - */ - WARN_ON_ONCE(!HAS_DDI(dev)); case INTEL_OUTPUT_DISPLAYPORT: case INTEL_OUTPUT_EDP: intel_dig_port = enc_to_dig_port(&intel_encoder->base); @@ -12283,9 +12268,6 @@ static bool check_digital_port_conflicts(struct drm_atomic_state *state) switch (encoder->type) { unsigned int port_mask; - case INTEL_OUTPUT_UNKNOWN: - if (WARN_ON(!HAS_DDI(dev))) - break; case INTEL_OUTPUT_DISPLAYPORT: case INTEL_OUTPUT_HDMI: case INTEL_OUTPUT_EDP: diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 7d354b1e5e5f..1d31aa296aaa 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -4601,8 +4601,6 @@ intel_dp_detect(struct drm_connector *connector, bool force) if (intel_dp->is_mst) { /* MST devices are disconnected from a monitor POV */ - if (intel_encoder->type != INTEL_OUTPUT_EDP) - intel_encoder->type = INTEL_OUTPUT_DISPLAYPORT; return connector_status_disconnected; } @@ -4632,8 +4630,6 @@ intel_dp_detect(struct drm_connector *connector, bool force) if (ret) { /* if we are in MST mode then this connector won't appear connected or have anything with EDID on it */ - if (intel_encoder->type != INTEL_OUTPUT_EDP) - intel_encoder->type = INTEL_OUTPUT_DISPLAYPORT; status = connector_status_disconnected; goto out; } @@ -4648,8 +4644,6 @@ intel_dp_detect(struct drm_connector *connector, bool force) intel_dp_set_edid(intel_dp); - if (intel_encoder->type != INTEL_OUTPUT_EDP) - intel_encoder->type = INTEL_OUTPUT_DISPLAYPORT; status = connector_status_connected; /* Try to read the source of the interrupt */ @@ -4692,9 +4686,6 @@ intel_dp_force(struct drm_connector *connector) intel_dp_set_edid(intel_dp); intel_display_power_put(dev_priv, power_domain); - - if (intel_encoder->type != INTEL_OUTPUT_EDP) - intel_encoder->type = INTEL_OUTPUT_DISPLAYPORT; } static int intel_dp_get_modes(struct drm_connector *connector) @@ -4969,9 +4960,9 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd) enum intel_display_power_domain power_domain; enum irqreturn ret = IRQ_NONE; - if (intel_dig_port->base.type != INTEL_OUTPUT_EDP && - intel_dig_port->base.type != INTEL_OUTPUT_HDMI) - intel_dig_port->base.type = INTEL_OUTPUT_DISPLAYPORT; + if (WARN_ON_ONCE(intel_dig_port->base.type != INTEL_OUTPUT_EDP && + intel_dig_port->base.type != INTEL_OUTPUT_DISPLAYPORT)) + return IRQ_HANDLED; if (long_hpd && intel_dig_port->base.type == INTEL_OUTPUT_EDP) { /* @@ -5815,6 +5806,9 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port, enum port port = intel_dig_port->port; int type, ret; + if (WARN_ON(intel_encoder->type != INTEL_OUTPUT_DISPLAYPORT)) + return false; + if (WARN(intel_dig_port->max_lanes < 1, "Not enough lanes (%d) for DP on port %c\n", intel_dig_port->max_lanes, port_name(port))) @@ -5851,11 +5845,6 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port, else type = DRM_MODE_CONNECTOR_DisplayPort; - /* - * For eDP we always set the encoder type to INTEL_OUTPUT_EDP, but - * for DP the encoder type can be set by the caller to - * INTEL_OUTPUT_UNKNOWN for DDI, so don't rewrite it. - */ if (type == DRM_MODE_CONNECTOR_eDP) intel_encoder->type = INTEL_OUTPUT_EDP; diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index a8a84b8c2bac..9e5db3d71e12 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -103,8 +103,7 @@ enum intel_output_type { INTEL_OUTPUT_DISPLAYPORT = 7, INTEL_OUTPUT_EDP = 8, INTEL_OUTPUT_DSI = 9, - INTEL_OUTPUT_UNKNOWN = 10, - INTEL_OUTPUT_DP_MST = 11, + INTEL_OUTPUT_DP_MST = 10, }; #define INTEL_DVO_CHIP_NONE 0 diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c index 895189abfd56..75ea9515a9ce 100644 --- a/drivers/gpu/drm/i915/intel_hdmi.c +++ b/drivers/gpu/drm/i915/intel_hdmi.c @@ -2034,6 +2034,9 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port, enum port port = intel_dig_port->port; uint8_t alternate_ddc_pin; + if (WARN_ON(intel_encoder->type != INTEL_OUTPUT_HDMI)) + return; + if (WARN(intel_dig_port->max_lanes < 4, "Not enough lanes (%d) for HDMI on port %c\n", intel_dig_port->max_lanes, port_name(port))) diff --git a/drivers/gpu/drm/i915/intel_opregion.c b/drivers/gpu/drm/i915/intel_opregion.c index e362a30776fa..a15459a451c2 100644 --- a/drivers/gpu/drm/i915/intel_opregion.c +++ b/drivers/gpu/drm/i915/intel_opregion.c @@ -360,7 +360,6 @@ int intel_opregion_notify_encoder(struct intel_encoder *intel_encoder, case INTEL_OUTPUT_ANALOG: type = DISPLAY_TYPE_CRT; break; - case INTEL_OUTPUT_UNKNOWN: case INTEL_OUTPUT_DISPLAYPORT: case INTEL_OUTPUT_HDMI: case INTEL_OUTPUT_DP_MST: