| Message ID | 4d90cd6097b7b30daa54ebf078fe18fbd6d1ef2c.1551886309.git.jani.nikula@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915/bios: ddi port parsing changes | expand |
On Wed, Mar 06, 2019 at 05:34:14PM +0200, Jani Nikula wrote: > Iterate over child devices instead of ports in parse_ddi_ports() to > initialize dri_port_info. We'll eventually need to decide some stuff > based on the child device order, which may be different from the port > order. > > As a bonus, this allows better abstractions for e.g. dvo port mapping. > > There's a subtle change in the DDC pin and AUX channel sanitization as > we change the order. Otherwise, this should not change behaviour. I have a feeling we had some odd cases with the port A vs. port E, but I think those were probably due to the non-VBT defaults for port A smashing into the VBT provided values of port E. So yeah, I think (or at least hope) that this is fine. Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Signed-off-by: Jani Nikula <jani.nikula@intel.com> > --- > drivers/gpu/drm/i915/i915_drv.h | 1 + > drivers/gpu/drm/i915/intel_bios.c | 104 +++++++++++++++++------------- > 2 files changed, 59 insertions(+), 46 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index ff039750069d..eeeb0d9cfdcd 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -946,6 +946,7 @@ struct ddi_vbt_port_info { > #define HDMI_LEVEL_SHIFT_UNKNOWN 0xff > u8 hdmi_level_shift; > > + u8 present:1; > u8 supports_dvi:1; > u8 supports_hdmi:1; > u8 supports_dp:1; > diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c > index b508d8a735e0..fc27ffe81c14 100644 > --- a/drivers/gpu/drm/i915/intel_bios.c > +++ b/drivers/gpu/drm/i915/intel_bios.c > @@ -1222,10 +1222,11 @@ static void sanitize_ddc_pin(struct drm_i915_private *dev_priv, > if (!info->alternate_ddc_pin) > return; > > - for_each_port_masked(p, (1 << port) - 1) { > + for (p = PORT_A; p < I915_MAX_PORTS; p++) { > struct ddi_vbt_port_info *i = &dev_priv->vbt.ddi_port_info[p]; > > - if (info->alternate_ddc_pin != i->alternate_ddc_pin) > + if (p == port || !i->present || > + info->alternate_ddc_pin != i->alternate_ddc_pin) > continue; > > DRM_DEBUG_KMS("port %c trying to use the same DDC pin (0x%x) as port %c, " > @@ -1239,8 +1240,8 @@ static void sanitize_ddc_pin(struct drm_i915_private *dev_priv, > * port. Otherwise they share the same ddc bin and > * system couldn't communicate with them separately. > * > - * Due to parsing the ports in alphabetical order, > - * a higher port will always clobber a lower one. > + * Due to parsing the ports in child device order, > + * a later device will always clobber an earlier one. > */ > i->supports_dvi = false; > i->supports_hdmi = false; > @@ -1258,10 +1259,11 @@ static void sanitize_aux_ch(struct drm_i915_private *dev_priv, > if (!info->alternate_aux_channel) > return; > > - for_each_port_masked(p, (1 << port) - 1) { > + for (p = PORT_A; p < I915_MAX_PORTS; p++) { > struct ddi_vbt_port_info *i = &dev_priv->vbt.ddi_port_info[p]; > > - if (info->alternate_aux_channel != i->alternate_aux_channel) > + if (p == port || !i->present || > + info->alternate_aux_channel != i->alternate_aux_channel) > continue; > > DRM_DEBUG_KMS("port %c trying to use the same AUX CH (0x%x) as port %c, " > @@ -1275,8 +1277,8 @@ static void sanitize_aux_ch(struct drm_i915_private *dev_priv, > * port. Otherwise they share the same aux channel > * and system couldn't communicate with them separately. > * > - * Due to parsing the ports in alphabetical order, > - * a higher port will always clobber a lower one. > + * Due to parsing the ports in child device order, > + * a later device will always clobber an earlier one. > */ > i->supports_dp = false; > i->alternate_aux_channel = 0; > @@ -1324,49 +1326,58 @@ static u8 map_ddc_pin(struct drm_i915_private *dev_priv, u8 vbt_pin) > return 0; > } > > -static void parse_ddi_port(struct drm_i915_private *dev_priv, enum port port, > - u8 bdb_version) > +static enum port dvo_port_to_port(u8 dvo_port) > { > - struct child_device_config *it, *child = NULL; > - struct ddi_vbt_port_info *info = &dev_priv->vbt.ddi_port_info[port]; > - int i, j; > - bool is_dvi, is_hdmi, is_dp, is_edp, is_crt; > - /* Each DDI port can have more than one value on the "DVO Port" field, > + /* > + * Each DDI port can have more than one value on the "DVO Port" field, > * so look for all the possible values for each port. > */ > - int dvo_ports[][3] = { > - {DVO_PORT_HDMIA, DVO_PORT_DPA, -1}, > - {DVO_PORT_HDMIB, DVO_PORT_DPB, -1}, > - {DVO_PORT_HDMIC, DVO_PORT_DPC, -1}, > - {DVO_PORT_HDMID, DVO_PORT_DPD, -1}, > - {DVO_PORT_CRT, DVO_PORT_HDMIE, DVO_PORT_DPE}, > - {DVO_PORT_HDMIF, DVO_PORT_DPF, -1}, > + static const int dvo_ports[][3] = { > + [PORT_A] = { DVO_PORT_HDMIA, DVO_PORT_DPA, -1}, > + [PORT_B] = { DVO_PORT_HDMIB, DVO_PORT_DPB, -1}, > + [PORT_C] = { DVO_PORT_HDMIC, DVO_PORT_DPC, -1}, > + [PORT_D] = { DVO_PORT_HDMID, DVO_PORT_DPD, -1}, > + [PORT_E] = { DVO_PORT_CRT, DVO_PORT_HDMIE, DVO_PORT_DPE}, > + [PORT_F] = { DVO_PORT_HDMIF, DVO_PORT_DPF, -1}, > }; > + enum port port; > + int i; > > - /* > - * Find the first child device to reference the port, report if more > - * than one found. > - */ > - for (i = 0; i < dev_priv->vbt.child_dev_num; i++) { > - it = dev_priv->vbt.child_dev + i; > - > - for (j = 0; j < 3; j++) { > - if (dvo_ports[port][j] == -1) > + for (port = PORT_A; port < ARRAY_SIZE(dvo_ports); port++) { > + for (i = 0; i < ARRAY_SIZE(dvo_ports[port]); i++) { > + if (dvo_ports[port][i] == -1) > break; > > - if (it->dvo_port == dvo_ports[port][j]) { > - if (child) { > - DRM_DEBUG_KMS("More than one child device for port %c in VBT, using the first.\n", > - port_name(port)); > - } else { > - child = it; > - } > - } > + if (dvo_port == dvo_ports[port][i]) > + return port; > } > } > - if (!child) > + > + return PORT_NONE; > +} > + > +static void parse_ddi_port(struct drm_i915_private *dev_priv, > + const struct child_device_config *child, > + u8 bdb_version) > +{ > + struct ddi_vbt_port_info *info; > + bool is_dvi, is_hdmi, is_dp, is_edp, is_crt; > + enum port port; > + > + port = dvo_port_to_port(child->dvo_port); > + if (port == PORT_NONE) > return; > > + info = &dev_priv->vbt.ddi_port_info[port]; > + > + if (info->present) { > + DRM_DEBUG_KMS("More than one child device for port %c in VBT, using the first.\n", > + port_name(port)); > + return; > + } > + > + info->present = true; > + > is_dvi = child->device_type & DEVICE_TYPE_TMDS_DVI_SIGNALING; > is_dp = child->device_type & DEVICE_TYPE_DISPLAYPORT_OUTPUT; > is_crt = child->device_type & DEVICE_TYPE_ANALOG_OUTPUT; > @@ -1498,19 +1509,20 @@ static void parse_ddi_port(struct drm_i915_private *dev_priv, enum port port, > > static void parse_ddi_ports(struct drm_i915_private *dev_priv, u8 bdb_version) > { > - enum port port; > + const struct child_device_config *child; > + int i; > > if (!HAS_DDI(dev_priv) && !IS_CHERRYVIEW(dev_priv)) > return; > > - if (!dev_priv->vbt.child_dev_num) > - return; > - > if (bdb_version < 155) > return; > > - for (port = PORT_A; port < I915_MAX_PORTS; port++) > - parse_ddi_port(dev_priv, port, bdb_version); > + for (i = 0; i < dev_priv->vbt.child_dev_num; i++) { > + child = dev_priv->vbt.child_dev + i; > + > + parse_ddi_port(dev_priv, child, bdb_version); > + } > } > > static void > -- > 2.20.1
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index ff039750069d..eeeb0d9cfdcd 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -946,6 +946,7 @@ struct ddi_vbt_port_info { #define HDMI_LEVEL_SHIFT_UNKNOWN 0xff u8 hdmi_level_shift; + u8 present:1; u8 supports_dvi:1; u8 supports_hdmi:1; u8 supports_dp:1; diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c index b508d8a735e0..fc27ffe81c14 100644 --- a/drivers/gpu/drm/i915/intel_bios.c +++ b/drivers/gpu/drm/i915/intel_bios.c @@ -1222,10 +1222,11 @@ static void sanitize_ddc_pin(struct drm_i915_private *dev_priv, if (!info->alternate_ddc_pin) return; - for_each_port_masked(p, (1 << port) - 1) { + for (p = PORT_A; p < I915_MAX_PORTS; p++) { struct ddi_vbt_port_info *i = &dev_priv->vbt.ddi_port_info[p]; - if (info->alternate_ddc_pin != i->alternate_ddc_pin) + if (p == port || !i->present || + info->alternate_ddc_pin != i->alternate_ddc_pin) continue; DRM_DEBUG_KMS("port %c trying to use the same DDC pin (0x%x) as port %c, " @@ -1239,8 +1240,8 @@ static void sanitize_ddc_pin(struct drm_i915_private *dev_priv, * port. Otherwise they share the same ddc bin and * system couldn't communicate with them separately. * - * Due to parsing the ports in alphabetical order, - * a higher port will always clobber a lower one. + * Due to parsing the ports in child device order, + * a later device will always clobber an earlier one. */ i->supports_dvi = false; i->supports_hdmi = false; @@ -1258,10 +1259,11 @@ static void sanitize_aux_ch(struct drm_i915_private *dev_priv, if (!info->alternate_aux_channel) return; - for_each_port_masked(p, (1 << port) - 1) { + for (p = PORT_A; p < I915_MAX_PORTS; p++) { struct ddi_vbt_port_info *i = &dev_priv->vbt.ddi_port_info[p]; - if (info->alternate_aux_channel != i->alternate_aux_channel) + if (p == port || !i->present || + info->alternate_aux_channel != i->alternate_aux_channel) continue; DRM_DEBUG_KMS("port %c trying to use the same AUX CH (0x%x) as port %c, " @@ -1275,8 +1277,8 @@ static void sanitize_aux_ch(struct drm_i915_private *dev_priv, * port. Otherwise they share the same aux channel * and system couldn't communicate with them separately. * - * Due to parsing the ports in alphabetical order, - * a higher port will always clobber a lower one. + * Due to parsing the ports in child device order, + * a later device will always clobber an earlier one. */ i->supports_dp = false; i->alternate_aux_channel = 0; @@ -1324,49 +1326,58 @@ static u8 map_ddc_pin(struct drm_i915_private *dev_priv, u8 vbt_pin) return 0; } -static void parse_ddi_port(struct drm_i915_private *dev_priv, enum port port, - u8 bdb_version) +static enum port dvo_port_to_port(u8 dvo_port) { - struct child_device_config *it, *child = NULL; - struct ddi_vbt_port_info *info = &dev_priv->vbt.ddi_port_info[port]; - int i, j; - bool is_dvi, is_hdmi, is_dp, is_edp, is_crt; - /* Each DDI port can have more than one value on the "DVO Port" field, + /* + * Each DDI port can have more than one value on the "DVO Port" field, * so look for all the possible values for each port. */ - int dvo_ports[][3] = { - {DVO_PORT_HDMIA, DVO_PORT_DPA, -1}, - {DVO_PORT_HDMIB, DVO_PORT_DPB, -1}, - {DVO_PORT_HDMIC, DVO_PORT_DPC, -1}, - {DVO_PORT_HDMID, DVO_PORT_DPD, -1}, - {DVO_PORT_CRT, DVO_PORT_HDMIE, DVO_PORT_DPE}, - {DVO_PORT_HDMIF, DVO_PORT_DPF, -1}, + static const int dvo_ports[][3] = { + [PORT_A] = { DVO_PORT_HDMIA, DVO_PORT_DPA, -1}, + [PORT_B] = { DVO_PORT_HDMIB, DVO_PORT_DPB, -1}, + [PORT_C] = { DVO_PORT_HDMIC, DVO_PORT_DPC, -1}, + [PORT_D] = { DVO_PORT_HDMID, DVO_PORT_DPD, -1}, + [PORT_E] = { DVO_PORT_CRT, DVO_PORT_HDMIE, DVO_PORT_DPE}, + [PORT_F] = { DVO_PORT_HDMIF, DVO_PORT_DPF, -1}, }; + enum port port; + int i; - /* - * Find the first child device to reference the port, report if more - * than one found. - */ - for (i = 0; i < dev_priv->vbt.child_dev_num; i++) { - it = dev_priv->vbt.child_dev + i; - - for (j = 0; j < 3; j++) { - if (dvo_ports[port][j] == -1) + for (port = PORT_A; port < ARRAY_SIZE(dvo_ports); port++) { + for (i = 0; i < ARRAY_SIZE(dvo_ports[port]); i++) { + if (dvo_ports[port][i] == -1) break; - if (it->dvo_port == dvo_ports[port][j]) { - if (child) { - DRM_DEBUG_KMS("More than one child device for port %c in VBT, using the first.\n", - port_name(port)); - } else { - child = it; - } - } + if (dvo_port == dvo_ports[port][i]) + return port; } } - if (!child) + + return PORT_NONE; +} + +static void parse_ddi_port(struct drm_i915_private *dev_priv, + const struct child_device_config *child, + u8 bdb_version) +{ + struct ddi_vbt_port_info *info; + bool is_dvi, is_hdmi, is_dp, is_edp, is_crt; + enum port port; + + port = dvo_port_to_port(child->dvo_port); + if (port == PORT_NONE) return; + info = &dev_priv->vbt.ddi_port_info[port]; + + if (info->present) { + DRM_DEBUG_KMS("More than one child device for port %c in VBT, using the first.\n", + port_name(port)); + return; + } + + info->present = true; + is_dvi = child->device_type & DEVICE_TYPE_TMDS_DVI_SIGNALING; is_dp = child->device_type & DEVICE_TYPE_DISPLAYPORT_OUTPUT; is_crt = child->device_type & DEVICE_TYPE_ANALOG_OUTPUT; @@ -1498,19 +1509,20 @@ static void parse_ddi_port(struct drm_i915_private *dev_priv, enum port port, static void parse_ddi_ports(struct drm_i915_private *dev_priv, u8 bdb_version) { - enum port port; + const struct child_device_config *child; + int i; if (!HAS_DDI(dev_priv) && !IS_CHERRYVIEW(dev_priv)) return; - if (!dev_priv->vbt.child_dev_num) - return; - if (bdb_version < 155) return; - for (port = PORT_A; port < I915_MAX_PORTS; port++) - parse_ddi_port(dev_priv, port, bdb_version); + for (i = 0; i < dev_priv->vbt.child_dev_num; i++) { + child = dev_priv->vbt.child_dev + i; + + parse_ddi_port(dev_priv, child, bdb_version); + } } static void
Iterate over child devices instead of ports in parse_ddi_ports() to initialize dri_port_info. We'll eventually need to decide some stuff based on the child device order, which may be different from the port order. As a bonus, this allows better abstractions for e.g. dvo port mapping. There's a subtle change in the DDC pin and AUX channel sanitization as we change the order. Otherwise, this should not change behaviour. Signed-off-by: Jani Nikula <jani.nikula@intel.com> --- drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/intel_bios.c | 104 +++++++++++++++++------------- 2 files changed, 59 insertions(+), 46 deletions(-)