[PATCH 0/3] drm/i915/bios: ddi port parsing changes

[PATCH 0/3] drm/i915/bios: ddi port parsing changes

[Jani Nikula]

Some prep work for future changes which will be easier with ddi port info being
initialized in child device order instead of port order.

BR,
Jani.


Jani Nikula (3):
  drm/i915/bios: iterate over child devices to initialize ddi_port_info
  drm/i915/bios: parse dsi devices in parse_ddi_ports()
  drm/i915/bios: parse ddi ports also on VLV

 drivers/gpu/drm/i915/i915_drv.h   |   2 +
 drivers/gpu/drm/i915/intel_bios.c | 113 +++++++++++++++++-------------
 2 files changed, 66 insertions(+), 49 deletions(-)

-- 
2.20.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 1/3] drm/i915/bios: iterate over child devices to initialize ddi_port_info

[Jani Nikula]

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(-)

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

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 2/3] drm/i915/bios: parse dsi devices in parse_ddi_ports()

[Jani Nikula]

For the time being this is only for completeness and better debug
logging of DSI ports.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h   |  1 +
 drivers/gpu/drm/i915/intel_bios.c | 14 ++++++++------
 2 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index eeeb0d9cfdcd..e14563418aad 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -951,6 +951,7 @@ struct ddi_vbt_port_info {
 	u8 supports_hdmi:1;
 	u8 supports_dp:1;
 	u8 supports_edp:1;
+	u8 supports_dsi:1;
 	u8 supports_typec_usb:1;
 	u8 supports_tbt:1;
 
diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
index fc27ffe81c14..9beff569b010 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -1333,9 +1333,9 @@ static enum port dvo_port_to_port(u8 dvo_port)
 	 * so look for all the possible values for each port.
 	 */
 	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_A] = { DVO_PORT_HDMIA, DVO_PORT_DPA, DVO_PORT_MIPIA },
+		[PORT_B] = { DVO_PORT_HDMIB, DVO_PORT_DPB, DVO_PORT_MIPIB },
+		[PORT_C] = { DVO_PORT_HDMIC, DVO_PORT_DPC, DVO_PORT_MIPIC },
 		[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},
@@ -1361,7 +1361,7 @@ static void parse_ddi_port(struct drm_i915_private *dev_priv,
 			   u8 bdb_version)
 {
 	struct ddi_vbt_port_info *info;
-	bool is_dvi, is_hdmi, is_dp, is_edp, is_crt;
+	bool is_dvi, is_hdmi, is_dp, is_edp, is_crt, is_dsi;
 	enum port port;
 
 	port = dvo_port_to_port(child->dvo_port);
@@ -1383,6 +1383,7 @@ static void parse_ddi_port(struct drm_i915_private *dev_priv,
 	is_crt = child->device_type & DEVICE_TYPE_ANALOG_OUTPUT;
 	is_hdmi = is_dvi && (child->device_type & DEVICE_TYPE_NOT_HDMI_OUTPUT) == 0;
 	is_edp = is_dp && (child->device_type & DEVICE_TYPE_INTERNAL_CONNECTOR);
+	is_dsi = child->device_type & DEVICE_TYPE_MIPI_OUTPUT;
 
 	if (port == PORT_A && is_dvi) {
 		DRM_DEBUG_KMS("VBT claims port A supports DVI%s, ignoring\n",
@@ -1395,6 +1396,7 @@ static void parse_ddi_port(struct drm_i915_private *dev_priv,
 	info->supports_hdmi = is_hdmi;
 	info->supports_dp = is_dp;
 	info->supports_edp = is_edp;
+	info->supports_dsi = is_dsi;
 
 	if (bdb_version >= 195)
 		info->supports_typec_usb = child->dp_usb_type_c;
@@ -1402,8 +1404,8 @@ static void parse_ddi_port(struct drm_i915_private *dev_priv,
 	if (bdb_version >= 209)
 		info->supports_tbt = child->tbt;
 
-	DRM_DEBUG_KMS("Port %c VBT info: DP:%d HDMI:%d DVI:%d EDP:%d CRT:%d TCUSB:%d TBT:%d\n",
-		      port_name(port), is_dp, is_hdmi, is_dvi, is_edp, is_crt,
+	DRM_DEBUG_KMS("Port %c VBT info: DP:%d HDMI:%d DVI:%d EDP:%d DSI:%d CRT:%d TCUSB:%d TBT:%d\n",
+		      port_name(port), is_dp, is_hdmi, is_dvi, is_edp, is_dsi, is_crt,
 		      info->supports_typec_usb, info->supports_tbt);
 
 	if (is_edp && is_dvi)
-- 
2.20.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 3/3] drm/i915/bios: parse ddi ports also on VLV

[Jani Nikula]

The main benefit is improved debug logging of the ports also on VLV.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/intel_bios.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
index 9beff569b010..030a5e9c67b3 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -1514,7 +1514,8 @@ static void parse_ddi_ports(struct drm_i915_private *dev_priv, u8 bdb_version)
 	const struct child_device_config *child;
 	int i;
 
-	if (!HAS_DDI(dev_priv) && !IS_CHERRYVIEW(dev_priv))
+	if (!HAS_DDI(dev_priv) &&
+	    !IS_CHERRYVIEW(dev_priv) && !IS_VALLEYVIEW(dev_priv))
 		return;
 
 	if (bdb_version < 155)
-- 
2.20.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.SPARSE: warning for drm/i915/bios: ddi port parsing changes

[Patchwork]

== Series Details ==

Series: drm/i915/bios: ddi port parsing changes
URL   : https://patchwork.freedesktop.org/series/57651/
State : warning

== Summary ==

$ dim sparse origin/drm-tip
Sparse version: v0.5.2
Commit: drm/i915/bios: iterate over child devices to initialize ddi_port_info
-drivers/gpu/drm/i915/selftests/../i915_drv.h:3548:16: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/selftests/../i915_drv.h:3549:16: warning: expression using sizeof(void)

Commit: drm/i915/bios: parse dsi devices in parse_ddi_ports()
-drivers/gpu/drm/i915/selftests/../i915_drv.h:3549:16: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/selftests/../i915_drv.h:3550:16: warning: expression using sizeof(void)

Commit: drm/i915/bios: parse ddi ports also on VLV
Okay!

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.BAT: failure for drm/i915/bios: ddi port parsing changes

[Patchwork]

== Series Details ==

Series: drm/i915/bios: ddi port parsing changes
URL   : https://patchwork.freedesktop.org/series/57651/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_5712 -> Patchwork_12395
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_12395 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_12395, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/57651/revisions/1/mbox/

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_12395:

### IGT changes ###

#### Possible regressions ####

  * igt@i915_module_load@reload-with-fault-injection:
    - fi-kbl-7560u:       PASS -> INCOMPLETE

  
Known issues
------------

  Here are the changes found in Patchwork_12395 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_basic@readonly-bsd2:
    - fi-pnv-d510:        NOTRUN -> SKIP [fdo#109271] +76

  * igt@kms_busy@basic-flip-b:
    - fi-gdg-551:         PASS -> FAIL [fdo#103182]

  * igt@kms_busy@basic-flip-c:
    - fi-pnv-d510:        NOTRUN -> SKIP [fdo#109271] / [fdo#109278]

  * igt@kms_frontbuffer_tracking@basic:
    - fi-icl-u3:          PASS -> FAIL [fdo#103167]

  * igt@kms_pipe_crc_basic@read-crc-pipe-a-frame-sequence:
    - fi-byt-clapper:     PASS -> FAIL [fdo#103191] / [fdo#107362]

  
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103182]: https://bugs.freedesktop.org/show_bug.cgi?id=103182
  [fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
  [fdo#107362]: https://bugs.freedesktop.org/show_bug.cgi?id=107362
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278


Participating hosts (45 -> 40)
------------------------------

  Additional (1): fi-pnv-d510 
  Missing    (6): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-skl-lmem 


Build changes
-------------

    * Linux: CI_DRM_5712 -> Patchwork_12395

  CI_DRM_5712: 5d4de376b9a03c2f74e049ee6a8221df96687ba0 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4875: 91908d36d0d5c90eea86e29736d2748d5ec55335 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_12395: 24aa5327f54f0d3d4dbfaed57d4b92a7b2d9d5a1 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

24aa5327f54f drm/i915/bios: parse ddi ports also on VLV
dc783312f4fe drm/i915/bios: parse dsi devices in parse_ddi_ports()
0542ee4a7b9f drm/i915/bios: iterate over child devices to initialize ddi_port_info

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12395/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/3] drm/i915/bios: parse dsi devices in parse_ddi_ports()

[Ville Syrjälä]

On Wed, Mar 06, 2019 at 05:34:15PM +0200, Jani Nikula wrote:
> For the time being this is only for completeness and better debug
> logging of DSI ports.
> 
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h   |  1 +
>  drivers/gpu/drm/i915/intel_bios.c | 14 ++++++++------
>  2 files changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index eeeb0d9cfdcd..e14563418aad 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -951,6 +951,7 @@ struct ddi_vbt_port_info {
>  	u8 supports_hdmi:1;
>  	u8 supports_dp:1;
>  	u8 supports_edp:1;
> +	u8 supports_dsi:1;
>  	u8 supports_typec_usb:1;
>  	u8 supports_tbt:1;
>  
> diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
> index fc27ffe81c14..9beff569b010 100644
> --- a/drivers/gpu/drm/i915/intel_bios.c
> +++ b/drivers/gpu/drm/i915/intel_bios.c
> @@ -1333,9 +1333,9 @@ static enum port dvo_port_to_port(u8 dvo_port)
>  	 * so look for all the possible values for each port.
>  	 */
>  	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_A] = { DVO_PORT_HDMIA, DVO_PORT_DPA, DVO_PORT_MIPIA },
> +		[PORT_B] = { DVO_PORT_HDMIB, DVO_PORT_DPB, DVO_PORT_MIPIB },
> +		[PORT_C] = { DVO_PORT_HDMIC, DVO_PORT_DPC, DVO_PORT_MIPIC },

I wonder if this is going to end up badly on CHV...

>  		[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},
> @@ -1361,7 +1361,7 @@ static void parse_ddi_port(struct drm_i915_private *dev_priv,
>  			   u8 bdb_version)
>  {
>  	struct ddi_vbt_port_info *info;
> -	bool is_dvi, is_hdmi, is_dp, is_edp, is_crt;
> +	bool is_dvi, is_hdmi, is_dp, is_edp, is_crt, is_dsi;
>  	enum port port;
>  
>  	port = dvo_port_to_port(child->dvo_port);
> @@ -1383,6 +1383,7 @@ static void parse_ddi_port(struct drm_i915_private *dev_priv,
>  	is_crt = child->device_type & DEVICE_TYPE_ANALOG_OUTPUT;
>  	is_hdmi = is_dvi && (child->device_type & DEVICE_TYPE_NOT_HDMI_OUTPUT) == 0;
>  	is_edp = is_dp && (child->device_type & DEVICE_TYPE_INTERNAL_CONNECTOR);
> +	is_dsi = child->device_type & DEVICE_TYPE_MIPI_OUTPUT;
>  
>  	if (port == PORT_A && is_dvi) {
>  		DRM_DEBUG_KMS("VBT claims port A supports DVI%s, ignoring\n",
> @@ -1395,6 +1396,7 @@ static void parse_ddi_port(struct drm_i915_private *dev_priv,
>  	info->supports_hdmi = is_hdmi;
>  	info->supports_dp = is_dp;
>  	info->supports_edp = is_edp;
> +	info->supports_dsi = is_dsi;
>  
>  	if (bdb_version >= 195)
>  		info->supports_typec_usb = child->dp_usb_type_c;
> @@ -1402,8 +1404,8 @@ static void parse_ddi_port(struct drm_i915_private *dev_priv,
>  	if (bdb_version >= 209)
>  		info->supports_tbt = child->tbt;
>  
> -	DRM_DEBUG_KMS("Port %c VBT info: DP:%d HDMI:%d DVI:%d EDP:%d CRT:%d TCUSB:%d TBT:%d\n",
> -		      port_name(port), is_dp, is_hdmi, is_dvi, is_edp, is_crt,
> +	DRM_DEBUG_KMS("Port %c VBT info: DP:%d HDMI:%d DVI:%d EDP:%d DSI:%d CRT:%d TCUSB:%d TBT:%d\n",
> +		      port_name(port), is_dp, is_hdmi, is_dvi, is_edp, is_dsi, is_crt,
>  		      info->supports_typec_usb, info->supports_tbt);
>  
>  	if (is_edp && is_dvi)
> -- 
> 2.20.1

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/3] drm/i915/bios: parse ddi ports also on VLV

[Ville Syrjälä]

On Wed, Mar 06, 2019 at 05:34:16PM +0200, Jani Nikula wrote:
> The main benefit is improved debug logging of the ports also on VLV.

The fact that this causes VLV to start using the AUX/DDC pins from VBT
needs to be called out in the commit msg.

> 
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_bios.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
> index 9beff569b010..030a5e9c67b3 100644
> --- a/drivers/gpu/drm/i915/intel_bios.c
> +++ b/drivers/gpu/drm/i915/intel_bios.c
> @@ -1514,7 +1514,8 @@ static void parse_ddi_ports(struct drm_i915_private *dev_priv, u8 bdb_version)
>  	const struct child_device_config *child;
>  	int i;
>  
> -	if (!HAS_DDI(dev_priv) && !IS_CHERRYVIEW(dev_priv))
> +	if (!HAS_DDI(dev_priv) &&
> +	    !IS_CHERRYVIEW(dev_priv) && !IS_VALLEYVIEW(dev_priv))
>  		return;
>  
>  	if (bdb_version < 155)
> -- 
> 2.20.1

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/3] drm/i915/bios: iterate over child devices to initialize ddi_port_info

[Ville Syrjälä]

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

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/3] drm/i915/bios: parse dsi devices in parse_ddi_ports()

[Jani Nikula]

On Wed, 06 Mar 2019, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> On Wed, Mar 06, 2019 at 05:34:15PM +0200, Jani Nikula wrote:
>> For the time being this is only for completeness and better debug
>> logging of DSI ports.
>> 
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> ---
>>  drivers/gpu/drm/i915/i915_drv.h   |  1 +
>>  drivers/gpu/drm/i915/intel_bios.c | 14 ++++++++------
>>  2 files changed, 9 insertions(+), 6 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> index eeeb0d9cfdcd..e14563418aad 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -951,6 +951,7 @@ struct ddi_vbt_port_info {
>>  	u8 supports_hdmi:1;
>>  	u8 supports_dp:1;
>>  	u8 supports_edp:1;
>> +	u8 supports_dsi:1;
>>  	u8 supports_typec_usb:1;
>>  	u8 supports_tbt:1;
>>  
>> diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
>> index fc27ffe81c14..9beff569b010 100644
>> --- a/drivers/gpu/drm/i915/intel_bios.c
>> +++ b/drivers/gpu/drm/i915/intel_bios.c
>> @@ -1333,9 +1333,9 @@ static enum port dvo_port_to_port(u8 dvo_port)
>>  	 * so look for all the possible values for each port.
>>  	 */
>>  	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_A] = { DVO_PORT_HDMIA, DVO_PORT_DPA, DVO_PORT_MIPIA },
>> +		[PORT_B] = { DVO_PORT_HDMIB, DVO_PORT_DPB, DVO_PORT_MIPIB },
>> +		[PORT_C] = { DVO_PORT_HDMIC, DVO_PORT_DPC, DVO_PORT_MIPIC },
>
> I wonder if this is going to end up badly on CHV...

Yeah. We also have a bunch of these mapping tables, with various
DVO_PORT_* combos. There's a certain appeal to consolidating them.

So I can't but wonder why we have duplicate information with child
device type and child device specific port numbers. Is there a case
where the distinction helps?

BR,
Jani.


>
>>  		[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},
>> @@ -1361,7 +1361,7 @@ static void parse_ddi_port(struct drm_i915_private *dev_priv,
>>  			   u8 bdb_version)
>>  {
>>  	struct ddi_vbt_port_info *info;
>> -	bool is_dvi, is_hdmi, is_dp, is_edp, is_crt;
>> +	bool is_dvi, is_hdmi, is_dp, is_edp, is_crt, is_dsi;
>>  	enum port port;
>>  
>>  	port = dvo_port_to_port(child->dvo_port);
>> @@ -1383,6 +1383,7 @@ static void parse_ddi_port(struct drm_i915_private *dev_priv,
>>  	is_crt = child->device_type & DEVICE_TYPE_ANALOG_OUTPUT;
>>  	is_hdmi = is_dvi && (child->device_type & DEVICE_TYPE_NOT_HDMI_OUTPUT) == 0;
>>  	is_edp = is_dp && (child->device_type & DEVICE_TYPE_INTERNAL_CONNECTOR);
>> +	is_dsi = child->device_type & DEVICE_TYPE_MIPI_OUTPUT;
>>  
>>  	if (port == PORT_A && is_dvi) {
>>  		DRM_DEBUG_KMS("VBT claims port A supports DVI%s, ignoring\n",
>> @@ -1395,6 +1396,7 @@ static void parse_ddi_port(struct drm_i915_private *dev_priv,
>>  	info->supports_hdmi = is_hdmi;
>>  	info->supports_dp = is_dp;
>>  	info->supports_edp = is_edp;
>> +	info->supports_dsi = is_dsi;
>>  
>>  	if (bdb_version >= 195)
>>  		info->supports_typec_usb = child->dp_usb_type_c;
>> @@ -1402,8 +1404,8 @@ static void parse_ddi_port(struct drm_i915_private *dev_priv,
>>  	if (bdb_version >= 209)
>>  		info->supports_tbt = child->tbt;
>>  
>> -	DRM_DEBUG_KMS("Port %c VBT info: DP:%d HDMI:%d DVI:%d EDP:%d CRT:%d TCUSB:%d TBT:%d\n",
>> -		      port_name(port), is_dp, is_hdmi, is_dvi, is_edp, is_crt,
>> +	DRM_DEBUG_KMS("Port %c VBT info: DP:%d HDMI:%d DVI:%d EDP:%d DSI:%d CRT:%d TCUSB:%d TBT:%d\n",
>> +		      port_name(port), is_dp, is_hdmi, is_dvi, is_edp, is_dsi, is_crt,
>>  		      info->supports_typec_usb, info->supports_tbt);
>>  
>>  	if (is_edp && is_dvi)
>> -- 
>> 2.20.1

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/3] drm/i915/bios: parse dsi devices in parse_ddi_ports()

[Ville Syrjälä]

On Thu, Mar 07, 2019 at 12:18:15PM +0200, Jani Nikula wrote:
> On Wed, 06 Mar 2019, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> > On Wed, Mar 06, 2019 at 05:34:15PM +0200, Jani Nikula wrote:
> >> For the time being this is only for completeness and better debug
> >> logging of DSI ports.
> >> 
> >> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> >> ---
> >>  drivers/gpu/drm/i915/i915_drv.h   |  1 +
> >>  drivers/gpu/drm/i915/intel_bios.c | 14 ++++++++------
> >>  2 files changed, 9 insertions(+), 6 deletions(-)
> >> 
> >> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> >> index eeeb0d9cfdcd..e14563418aad 100644
> >> --- a/drivers/gpu/drm/i915/i915_drv.h
> >> +++ b/drivers/gpu/drm/i915/i915_drv.h
> >> @@ -951,6 +951,7 @@ struct ddi_vbt_port_info {
> >>  	u8 supports_hdmi:1;
> >>  	u8 supports_dp:1;
> >>  	u8 supports_edp:1;
> >> +	u8 supports_dsi:1;
> >>  	u8 supports_typec_usb:1;
> >>  	u8 supports_tbt:1;
> >>  
> >> diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
> >> index fc27ffe81c14..9beff569b010 100644
> >> --- a/drivers/gpu/drm/i915/intel_bios.c
> >> +++ b/drivers/gpu/drm/i915/intel_bios.c
> >> @@ -1333,9 +1333,9 @@ static enum port dvo_port_to_port(u8 dvo_port)
> >>  	 * so look for all the possible values for each port.
> >>  	 */
> >>  	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_A] = { DVO_PORT_HDMIA, DVO_PORT_DPA, DVO_PORT_MIPIA },
> >> +		[PORT_B] = { DVO_PORT_HDMIB, DVO_PORT_DPB, DVO_PORT_MIPIB },
> >> +		[PORT_C] = { DVO_PORT_HDMIC, DVO_PORT_DPC, DVO_PORT_MIPIC },
> >
> > I wonder if this is going to end up badly on CHV...
> 
> Yeah. We also have a bunch of these mapping tables, with various
> DVO_PORT_* combos. There's a certain appeal to consolidating them.
> 
> So I can't but wonder why we have duplicate information with child
> device type and child device specific port numbers. Is there a case
> where the distinction helps?

The one case I remeber is 7a17995a3dc8 ("drm/i915: Assume non-DP++
port if dvo_port is HDMI and there's no AUX ch specified in the VBT")

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx