drm/i915: Remove AUX CH sanitation

Stop with the VBT AUX CH sanitation, and instead just check
that the appropriate AUX CH is still available when initializing
a DP/TC port.

The reason being that we want to start initializing ports in
VBT order to deal with VBTs that declare child devices with
seemingly conflicting ports. As the encoder initialization can
fail for other reasons (at least for eDP+AUX) we can't know
upfront which way the conflicts should be resolved.

Note that the old way of sanitizing gave priority to the last
port declared in the VBT, but now we sort of do the opposite by
favoring the first encoder to successfully initialize. The reason
for the old "last port wins" preference was eg. Asrock B250M-HDV
where port A (eDP) and port E (DP->VGA) have an AUX CH conflict
and we need to prefer port E. However with the new way port A (eDP)
will be probed first, but will fail to probe due to HPD and thus
port E will still win in the end.

v2: Pimp the commit message (Jani)

Reviewed-by: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20230630155846.29931-5-ville.syrjala@linux.intel.com

diff --git a/drivers/gpu/drm/i915/display/g4x_dp.c b/drivers/gpu/drm/i915/display/g4x_dp.c
index 0cab599..4c7187f 100644 (file)
--- a/drivers/gpu/drm/i915/display/g4x_dp.c
+++ b/drivers/gpu/drm/i915/display/g4x_dp.c
@@ -1378,6 +1378,9 @@ bool g4x_dp_init(struct drm_i915_private *dev_priv,
                intel_infoframe_init(dig_port);
 
        dig_port->aux_ch = intel_dp_aux_ch(intel_encoder);
+       if (dig_port->aux_ch == AUX_CH_NONE)
+               goto err_init_connector;
+
        if (!intel_dp_init_connector(dig_port, intel_connector))
                goto err_init_connector;
 

diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c
index 6aeebd3..ae83788 100644 (file)
--- a/drivers/gpu/drm/i915/display/intel_bios.c
+++ b/drivers/gpu/drm/i915/display/intel_bios.c
@@ -2230,56 +2230,6 @@ static u8 map_ddc_pin(struct drm_i915_private *i915, u8 vbt_pin)
        return 0;
 }
 
-static enum port get_port_by_aux_ch(struct drm_i915_private *i915, u8 aux_ch)
-{
-       enum port port;
-
-       if (!aux_ch)
-               return PORT_NONE;
-
-       for_each_port(port) {
-               const struct intel_bios_encoder_data *devdata =
-                       i915->display.vbt.ports[port];
-
-               if (devdata && aux_ch == devdata->child.aux_channel)
-                       return port;
-       }
-
-       return PORT_NONE;
-}
-
-static void sanitize_aux_ch(struct intel_bios_encoder_data *devdata,
-                           enum port port)
-{
-       struct drm_i915_private *i915 = devdata->i915;
-       struct child_device_config *child;
-       enum port p;
-
-       p = get_port_by_aux_ch(i915, devdata->child.aux_channel);
-       if (p == PORT_NONE)
-               return;
-
-       drm_dbg_kms(&i915->drm,
-                   "port %c trying to use the same AUX CH (0x%x) as port %c, "
-                   "disabling port %c DP support\n",
-                   port_name(port), devdata->child.aux_channel,
-                   port_name(p), port_name(p));
-
-       /*
-        * If we have multiple ports supposedly sharing the aux channel, then DP
-        * couldn't exist on the shared port. Otherwise they share the same aux
-        * channel and system couldn't communicate with them separately.
-        *
-        * Give inverse child device order the priority, last one wins. Yes,
-        * there are real machines (eg. Asrock B250M-HDV) where VBT has both
-        * port A and port E with the same AUX ch and we must pick port E :(
-        */
-       child = &i915->display.vbt.ports[p]->child;
-
-       child->device_type &= ~DEVICE_TYPE_DISPLAYPORT_OUTPUT;
-       child->aux_channel = 0;
-}
-
 static u8 dvo_port_type(u8 dvo_port)
 {
        switch (dvo_port) {
@@ -2688,9 +2638,6 @@ static void parse_ddi_port(struct intel_bios_encoder_data *devdata)
 
        sanitize_device_type(devdata, port);
 
-       if (intel_bios_encoder_supports_dp(devdata))
-               sanitize_aux_ch(devdata, port);
-
        i915->display.vbt.ports[port] = devdata;
 }
 

diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
index 662b5ce..9e4e648 100644 (file)
--- a/drivers/gpu/drm/i915/display/intel_ddi.c
+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
@@ -4938,8 +4938,11 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
        dig_port->dp.output_reg = INVALID_MMIO_REG;
        dig_port->max_lanes = intel_ddi_max_lanes(dig_port);
 
-       if (need_aux_ch(encoder, init_dp))
+       if (need_aux_ch(encoder, init_dp)) {
                dig_port->aux_ch = intel_dp_aux_ch(encoder);
+               if (dig_port->aux_ch == AUX_CH_NONE)
+                       goto err;
+       }
 
        if (intel_phy_is_tc(dev_priv, phy)) {
                bool is_legacy =

diff --git a/drivers/gpu/drm/i915/display/intel_dp_aux.c b/drivers/gpu/drm/i915/display/intel_dp_aux.c
index 21b50a5..2d173bd 100644 (file)
--- a/drivers/gpu/drm/i915/display/intel_dp_aux.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_aux.c
@@ -792,25 +792,60 @@ static enum aux_ch default_aux_ch(struct intel_encoder *encoder)
        return (enum aux_ch)encoder->port;
 }
 
+static struct intel_encoder *
+get_encoder_by_aux_ch(struct intel_encoder *encoder,
+                     enum aux_ch aux_ch)
+{
+       struct drm_i915_private *i915 = to_i915(encoder->base.dev);
+       struct intel_encoder *other;
+
+       for_each_intel_encoder(&i915->drm, other) {
+               if (other == encoder)
+                       continue;
+
+               if (!intel_encoder_is_dig_port(other))
+                       continue;
+
+               if (enc_to_dig_port(other)->aux_ch == aux_ch)
+                       return other;
+       }
+
+       return NULL;
+}
+
 enum aux_ch intel_dp_aux_ch(struct intel_encoder *encoder)
 {
        struct drm_i915_private *i915 = to_i915(encoder->base.dev);
+       struct intel_encoder *other;
+       const char *source;
        enum aux_ch aux_ch;
 
        aux_ch = intel_bios_dp_aux_ch(encoder->devdata);
-       if (aux_ch != AUX_CH_NONE) {
-               drm_dbg_kms(&i915->drm, "[ENCODER:%d:%s] using AUX %c (VBT)\n",
-                           encoder->base.base.id, encoder->base.name,
-                           aux_ch_name(aux_ch));
-               return aux_ch;
+       source = "VBT";
+
+       if (aux_ch == AUX_CH_NONE) {
+               aux_ch = default_aux_ch(encoder);
+               source = "platform default";
        }
 
-       aux_ch = default_aux_ch(encoder);
+       if (aux_ch == AUX_CH_NONE)
+               return AUX_CH_NONE;
+
+       /* FIXME validate aux_ch against platform caps */
+
+       other = get_encoder_by_aux_ch(encoder, aux_ch);
+       if (other) {
+               drm_dbg_kms(&i915->drm,
+                           "[ENCODER:%d:%s] AUX CH %c already claimed by [ENCODER:%d:%s]\n",
+                           encoder->base.base.id, encoder->base.name, aux_ch_name(aux_ch),
+                           other->base.base.id, other->base.name);
+               return AUX_CH_NONE;
+       }
 
        drm_dbg_kms(&i915->drm,
-                   "[ENCODER:%d:%s] using AUX %c (platform default)\n",
+                   "[ENCODER:%d:%s] Using AUX CH %c (%s)\n",
                    encoder->base.base.id, encoder->base.name,
-                   aux_ch_name(aux_ch));
+                   aux_ch_name(aux_ch), source);
 
        return aux_ch;
 }