On Fri, 22 Mar 2019, Jani Nikula <jani.nikula@xxxxxxxxx> 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.
>
> Reviewed-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> Signed-off-by: Jani Nikula <jani.nikula@xxxxxxxxx>
Pushed to dinq, thanks for the review.
BR,
Jani.
> ---
> 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 fefcb39aefc4..d82d63dfa5a1 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -954,6 +954,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 64f20175a6dc..1dc8d03ff127 100644
> --- a/drivers/gpu/drm/i915/intel_bios.c
> +++ b/drivers/gpu/drm/i915/intel_bios.c
> @@ -1247,10 +1247,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, "
> @@ -1264,8 +1265,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;
> @@ -1283,10 +1284,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, "
> @@ -1300,8 +1302,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;
> @@ -1349,49 +1351,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;
> @@ -1523,19 +1534,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
--
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
