[Intel-gfx] [PATCH 0/7] drm/i915/bios: remove vbt ddi_port_info caching

[Intel-gfx] [PATCH 0/7] drm/i915/bios: remove vbt ddi_port_info caching

[Jani Nikula]

Simplify vbt child data access.

We still retain a port to struct intel_bios_encoder_data map, but going
forward we should prefer accessing the encoder specific data via
encoder->devdata, not the map nor the port.

That will still need some extra initialization, especially for older
platforms. One step at a time.

BR,
Jani.

Cc: José Roberto de Souza <jose.souza@intel.com>

Jani Nikula (7):
  drm/i915/bios: use hdmi level shift directly from child data
  drm/i915/bios: use max tmds clock directly from child data
  drm/i915/bios: use dp max link rate directly from child data
  drm/i915/bios: use alternate aux channel directly from child data
  drm/i915/bios: move ddc pin mapping code next to ddc pin sanitize
  drm/i915/bios: use ddc pin directly from child data
  drm/i915/bios: get rid of vbt ddi_port_info

 drivers/gpu/drm/i915/display/intel_bios.c | 372 +++++++++++-----------
 drivers/gpu/drm/i915/i915_drv.h           |  18 +-
 2 files changed, 187 insertions(+), 203 deletions(-)

-- 
2.20.1

[Intel-gfx] [PATCH 1/7] drm/i915/bios: use hdmi level shift directly from child data

[Jani Nikula]

Avoid extra caching of the data.

Cc: José Roberto de Souza <jose.souza@intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/display/intel_bios.c | 26 +++++++++++++----------
 drivers/gpu/drm/i915/i915_drv.h           |  4 ----
 2 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c
index e86e6ed2d3bf..afb5fcd9dd0c 100644
--- a/drivers/gpu/drm/i915/display/intel_bios.c
+++ b/drivers/gpu/drm/i915/display/intel_bios.c
@@ -1868,6 +1868,14 @@ intel_bios_encoder_supports_edp(const struct intel_bios_encoder_data *devdata)
 		devdata->child.device_type & DEVICE_TYPE_INTERNAL_CONNECTOR;
 }
 
+static int _intel_bios_hdmi_level_shift(const struct intel_bios_encoder_data *devdata)
+{
+	if (!devdata || devdata->i915->vbt.version < 158)
+		return -1;
+
+	return devdata->child.hdmi_level_shifter_value;
+}
+
 static bool is_port_valid(struct drm_i915_private *i915, enum port port)
 {
 	/*
@@ -1887,7 +1895,7 @@ static void parse_ddi_port(struct drm_i915_private *i915,
 	const struct child_device_config *child = &devdata->child;
 	struct ddi_vbt_port_info *info;
 	bool is_dvi, is_hdmi, is_dp, is_edp, is_crt, supports_typec_usb, supports_tbt;
-	int dp_boost_level, hdmi_boost_level;
+	int dp_boost_level, hdmi_boost_level, hdmi_level_shift;
 	enum port port;
 
 	port = dvo_port_to_port(i915, child->dvo_port);
@@ -1949,15 +1957,11 @@ static void parse_ddi_port(struct drm_i915_private *i915,
 		sanitize_aux_ch(i915, port);
 	}
 
-	if (i915->vbt.version >= 158) {
-		/* The VBT HDMI level shift values match the table we have. */
-		u8 hdmi_level_shift = child->hdmi_level_shifter_value;
+	hdmi_level_shift = _intel_bios_hdmi_level_shift(devdata);
+	if (hdmi_level_shift >= 0) {
 		drm_dbg_kms(&i915->drm,
 			    "Port %c VBT HDMI level shift: %d\n",
-			    port_name(port),
-			    hdmi_level_shift);
-		info->hdmi_level_shift = hdmi_level_shift;
-		info->hdmi_level_shift_set = true;
+			    port_name(port), hdmi_level_shift);
 	}
 
 	if (i915->vbt.version >= 204) {
@@ -2950,13 +2954,13 @@ int intel_bios_max_tmds_clock(struct intel_encoder *encoder)
 	return i915->vbt.ddi_port_info[encoder->port].max_tmds_clock;
 }
 
+/* This is an index in the HDMI/DVI DDI buffer translation table, or -1 */
 int intel_bios_hdmi_level_shift(struct intel_encoder *encoder)
 {
 	struct drm_i915_private *i915 = to_i915(encoder->base.dev);
-	const struct ddi_vbt_port_info *info =
-		&i915->vbt.ddi_port_info[encoder->port];
+	const struct intel_bios_encoder_data *devdata = i915->vbt.ddi_port_info[encoder->port].devdata;
 
-	return info->hdmi_level_shift_set ? info->hdmi_level_shift : -1;
+	return _intel_bios_hdmi_level_shift(devdata);
 }
 
 int intel_bios_encoder_dp_boost_level(const struct intel_bios_encoder_data *devdata)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 0a2b63072ff5..431408789826 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -642,10 +642,6 @@ struct ddi_vbt_port_info {
 
 	int max_tmds_clock;
 
-	/* This is an index in the HDMI/DVI DDI buffer translation table. */
-	u8 hdmi_level_shift;
-	u8 hdmi_level_shift_set:1;
-
 	u8 alternate_aux_channel;
 	u8 alternate_ddc_pin;
 
-- 
2.20.1

[Intel-gfx] [PATCH 2/7] drm/i915/bios: use max tmds clock directly from child data

[Jani Nikula]

Avoid extra caching of the data.

Cc: José Roberto de Souza <jose.souza@intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/display/intel_bios.c | 52 +++++++++++------------
 drivers/gpu/drm/i915/i915_drv.h           |  2 -
 2 files changed, 26 insertions(+), 28 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c
index afb5fcd9dd0c..253a528ba61a 100644
--- a/drivers/gpu/drm/i915/display/intel_bios.c
+++ b/drivers/gpu/drm/i915/display/intel_bios.c
@@ -1876,6 +1876,24 @@ static int _intel_bios_hdmi_level_shift(const struct intel_bios_encoder_data *de
 	return devdata->child.hdmi_level_shifter_value;
 }
 
+static int _intel_bios_max_tmds_clock(const struct intel_bios_encoder_data *devdata)
+{
+	if (!devdata || devdata->i915->vbt.version < 204)
+		return 0;
+
+	switch (devdata->child.hdmi_max_data_rate) {
+	default:
+		MISSING_CASE(devdata->child.hdmi_max_data_rate);
+		fallthrough;
+	case HDMI_MAX_DATA_RATE_PLATFORM:
+		return 0;
+	case HDMI_MAX_DATA_RATE_297:
+		return 297000;
+	case HDMI_MAX_DATA_RATE_165:
+		return 165000;
+	}
+}
+
 static bool is_port_valid(struct drm_i915_private *i915, enum port port)
 {
 	/*
@@ -1895,7 +1913,7 @@ static void parse_ddi_port(struct drm_i915_private *i915,
 	const struct child_device_config *child = &devdata->child;
 	struct ddi_vbt_port_info *info;
 	bool is_dvi, is_hdmi, is_dp, is_edp, is_crt, supports_typec_usb, supports_tbt;
-	int dp_boost_level, hdmi_boost_level, hdmi_level_shift;
+	int dp_boost_level, hdmi_boost_level, hdmi_level_shift, max_tmds_clock;
 	enum port port;
 
 	port = dvo_port_to_port(i915, child->dvo_port);
@@ -1964,30 +1982,11 @@ static void parse_ddi_port(struct drm_i915_private *i915,
 			    port_name(port), hdmi_level_shift);
 	}
 
-	if (i915->vbt.version >= 204) {
-		int max_tmds_clock;
-
-		switch (child->hdmi_max_data_rate) {
-		default:
-			MISSING_CASE(child->hdmi_max_data_rate);
-			fallthrough;
-		case HDMI_MAX_DATA_RATE_PLATFORM:
-			max_tmds_clock = 0;
-			break;
-		case HDMI_MAX_DATA_RATE_297:
-			max_tmds_clock = 297000;
-			break;
-		case HDMI_MAX_DATA_RATE_165:
-			max_tmds_clock = 165000;
-			break;
-		}
-
-		if (max_tmds_clock)
-			drm_dbg_kms(&i915->drm,
-				    "Port %c VBT HDMI max TMDS clock: %d kHz\n",
-				    port_name(port), max_tmds_clock);
-		info->max_tmds_clock = max_tmds_clock;
-	}
+	max_tmds_clock = _intel_bios_max_tmds_clock(devdata);
+	if (max_tmds_clock)
+		drm_dbg_kms(&i915->drm,
+			    "Port %c VBT HDMI max TMDS clock: %d kHz\n",
+			    port_name(port), max_tmds_clock);
 
 	/* I_boost config for SKL and above */
 	dp_boost_level = intel_bios_encoder_dp_boost_level(devdata);
@@ -2950,8 +2949,9 @@ enum aux_ch intel_bios_port_aux_ch(struct drm_i915_private *i915,
 int intel_bios_max_tmds_clock(struct intel_encoder *encoder)
 {
 	struct drm_i915_private *i915 = to_i915(encoder->base.dev);
+	const struct intel_bios_encoder_data *devdata = i915->vbt.ddi_port_info[encoder->port].devdata;
 
-	return i915->vbt.ddi_port_info[encoder->port].max_tmds_clock;
+	return _intel_bios_max_tmds_clock(devdata);
 }
 
 /* This is an index in the HDMI/DVI DDI buffer translation table, or -1 */
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 431408789826..973b899dbf36 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -640,8 +640,6 @@ struct ddi_vbt_port_info {
 	/* Non-NULL if port present. */
 	struct intel_bios_encoder_data *devdata;
 
-	int max_tmds_clock;
-
 	u8 alternate_aux_channel;
 	u8 alternate_ddc_pin;
 
-- 
2.20.1

[Intel-gfx] [PATCH 3/7] drm/i915/bios: use dp max link rate directly from child data

[Jani Nikula]

Avoid extra caching of the data.

Cc: José Roberto de Souza <jose.souza@intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/display/intel_bios.c | 28 ++++++++++++++---------
 drivers/gpu/drm/i915/i915_drv.h           |  2 --
 2 files changed, 17 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c
index 253a528ba61a..10b2beddc121 100644
--- a/drivers/gpu/drm/i915/display/intel_bios.c
+++ b/drivers/gpu/drm/i915/display/intel_bios.c
@@ -1815,6 +1815,17 @@ static int parse_bdb_216_dp_max_link_rate(const int vbt_max_link_rate)
 	}
 }
 
+static int _intel_bios_dp_max_link_rate(const struct intel_bios_encoder_data *devdata)
+{
+	if (!devdata || devdata->i915->vbt.version < 216)
+		return 0;
+
+	if (devdata->i915->vbt.version >= 230)
+		return parse_bdb_230_dp_max_link_rate(devdata->child.dp_max_link_rate);
+	else
+		return parse_bdb_216_dp_max_link_rate(devdata->child.dp_max_link_rate);
+}
+
 static void sanitize_device_type(struct intel_bios_encoder_data *devdata,
 				 enum port port)
 {
@@ -1913,7 +1924,7 @@ static void parse_ddi_port(struct drm_i915_private *i915,
 	const struct child_device_config *child = &devdata->child;
 	struct ddi_vbt_port_info *info;
 	bool is_dvi, is_hdmi, is_dp, is_edp, is_crt, supports_typec_usb, supports_tbt;
-	int dp_boost_level, hdmi_boost_level, hdmi_level_shift, max_tmds_clock;
+	int dp_boost_level, dp_max_link_rate, hdmi_boost_level, hdmi_level_shift, max_tmds_clock;
 	enum port port;
 
 	port = dvo_port_to_port(i915, child->dvo_port);
@@ -2001,17 +2012,11 @@ static void parse_ddi_port(struct drm_i915_private *i915,
 			    "Port %c VBT HDMI boost level: %d\n",
 			    port_name(port), hdmi_boost_level);
 
-	/* DP max link rate for GLK+ */
-	if (i915->vbt.version >= 216) {
-		if (i915->vbt.version >= 230)
-			info->dp_max_link_rate = parse_bdb_230_dp_max_link_rate(child->dp_max_link_rate);
-		else
-			info->dp_max_link_rate = parse_bdb_216_dp_max_link_rate(child->dp_max_link_rate);
-
+	dp_max_link_rate = _intel_bios_dp_max_link_rate(devdata);
+	if (dp_max_link_rate)
 		drm_dbg_kms(&i915->drm,
 			    "Port %c VBT DP max link rate: %d\n",
-			    port_name(port), info->dp_max_link_rate);
-	}
+			    port_name(port), dp_max_link_rate);
 
 	info->devdata = devdata;
 }
@@ -2982,8 +2987,9 @@ int intel_bios_encoder_hdmi_boost_level(const struct intel_bios_encoder_data *de
 int intel_bios_dp_max_link_rate(struct intel_encoder *encoder)
 {
 	struct drm_i915_private *i915 = to_i915(encoder->base.dev);
+	const struct intel_bios_encoder_data *devdata = i915->vbt.ddi_port_info[encoder->port].devdata;
 
-	return i915->vbt.ddi_port_info[encoder->port].dp_max_link_rate;
+	return _intel_bios_dp_max_link_rate(devdata);
 }
 
 int intel_bios_alternate_ddc_pin(struct intel_encoder *encoder)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 973b899dbf36..a0dead9f9222 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -642,8 +642,6 @@ struct ddi_vbt_port_info {
 
 	u8 alternate_aux_channel;
 	u8 alternate_ddc_pin;
-
-	int dp_max_link_rate;		/* 0 for not limited by VBT */
 };
 
 enum psr_lines_to_wait {
-- 
2.20.1

[Intel-gfx] [PATCH 4/7] drm/i915/bios: use alternate aux channel directly from child data

[Jani Nikula]

Avoid extra caching of the data.

Cc: José Roberto de Souza <jose.souza@intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/display/intel_bios.c | 26 +++++++++++------------
 drivers/gpu/drm/i915/i915_drv.h           |  1 -
 2 files changed, 12 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c
index 10b2beddc121..674f1424fcc2 100644
--- a/drivers/gpu/drm/i915/display/intel_bios.c
+++ b/drivers/gpu/drm/i915/display/intel_bios.c
@@ -1565,28 +1565,29 @@ static enum port get_port_by_aux_ch(struct drm_i915_private *i915, u8 aux_ch)
 	for_each_port(port) {
 		info = &i915->vbt.ddi_port_info[port];
 
-		if (info->devdata && aux_ch == info->alternate_aux_channel)
+		if (info->devdata && aux_ch == info->devdata->child.aux_channel)
 			return port;
 	}
 
 	return PORT_NONE;
 }
 
-static void sanitize_aux_ch(struct drm_i915_private *i915,
+static void sanitize_aux_ch(struct intel_bios_encoder_data *devdata,
 			    enum port port)
 {
-	struct ddi_vbt_port_info *info = &i915->vbt.ddi_port_info[port];
+	struct drm_i915_private *i915 = devdata->i915;
+	struct ddi_vbt_port_info *info;
 	struct child_device_config *child;
 	enum port p;
 
-	p = get_port_by_aux_ch(i915, info->alternate_aux_channel);
+	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), info->alternate_aux_channel,
+		    port_name(port), devdata->child.aux_channel,
 		    port_name(p), port_name(p));
 
 	/*
@@ -1602,7 +1603,7 @@ static void sanitize_aux_ch(struct drm_i915_private *i915,
 	child = &info->devdata->child;
 
 	child->device_type &= ~DEVICE_TYPE_DISPLAYPORT_OUTPUT;
-	info->alternate_aux_channel = 0;
+	child->aux_channel = 0;
 }
 
 static const u8 cnp_ddc_pin_map[] = {
@@ -1980,11 +1981,8 @@ static void parse_ddi_port(struct drm_i915_private *i915,
 		}
 	}
 
-	if (is_dp) {
-		info->alternate_aux_channel = child->aux_channel;
-
-		sanitize_aux_ch(i915, port);
-	}
+	if (is_dp)
+		sanitize_aux_ch(devdata, port);
 
 	hdmi_level_shift = _intel_bios_hdmi_level_shift(devdata);
 	if (hdmi_level_shift >= 0) {
@@ -2863,7 +2861,7 @@ enum aux_ch intel_bios_port_aux_ch(struct drm_i915_private *i915,
 		&i915->vbt.ddi_port_info[port];
 	enum aux_ch aux_ch;
 
-	if (!info->alternate_aux_channel) {
+	if (!info->devdata->child.aux_channel) {
 		aux_ch = (enum aux_ch)port;
 
 		drm_dbg_kms(&i915->drm,
@@ -2879,7 +2877,7 @@ enum aux_ch intel_bios_port_aux_ch(struct drm_i915_private *i915,
 	 * ADL-S VBT uses PHY based mapping. Combo PHYs A,B,C,D,E
 	 * map to DDI A,TC1,TC2,TC3,TC4 respectively.
 	 */
-	switch (info->alternate_aux_channel) {
+	switch (info->devdata->child.aux_channel) {
 	case DP_AUX_A:
 		aux_ch = AUX_CH_A;
 		break;
@@ -2940,7 +2938,7 @@ enum aux_ch intel_bios_port_aux_ch(struct drm_i915_private *i915,
 			aux_ch = AUX_CH_I;
 		break;
 	default:
-		MISSING_CASE(info->alternate_aux_channel);
+		MISSING_CASE(info->devdata->child.aux_channel);
 		aux_ch = AUX_CH_A;
 		break;
 	}
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index a0dead9f9222..91097526cd96 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -640,7 +640,6 @@ struct ddi_vbt_port_info {
 	/* Non-NULL if port present. */
 	struct intel_bios_encoder_data *devdata;
 
-	u8 alternate_aux_channel;
 	u8 alternate_ddc_pin;
 };
 
-- 
2.20.1

Re: [Intel-gfx] [PATCH 4/7] drm/i915/bios: use alternate aux channel directly from child data

[Nautiyal, Ankit K]

On 8/24/2021 7:04 PM, Jani Nikula wrote:
> Avoid extra caching of the data.
>
> Cc: José Roberto de Souza <jose.souza@intel.com>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>   drivers/gpu/drm/i915/display/intel_bios.c | 26 +++++++++++------------
>   drivers/gpu/drm/i915/i915_drv.h           |  1 -
>   2 files changed, 12 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c
> index 10b2beddc121..674f1424fcc2 100644
> --- a/drivers/gpu/drm/i915/display/intel_bios.c
> +++ b/drivers/gpu/drm/i915/display/intel_bios.c
> @@ -1565,28 +1565,29 @@ static enum port get_port_by_aux_ch(struct drm_i915_private *i915, u8 aux_ch)
>   	for_each_port(port) {
>   		info = &i915->vbt.ddi_port_info[port];
>   
> -		if (info->devdata && aux_ch == info->alternate_aux_channel)
> +		if (info->devdata && aux_ch == info->devdata->child.aux_channel)
>   			return port;
>   	}
>   
>   	return PORT_NONE;
>   }
>   
> -static void sanitize_aux_ch(struct drm_i915_private *i915,
> +static void sanitize_aux_ch(struct intel_bios_encoder_data *devdata,
>   			    enum port port)
>   {
> -	struct ddi_vbt_port_info *info = &i915->vbt.ddi_port_info[port];
> +	struct drm_i915_private *i915 = devdata->i915;
> +	struct ddi_vbt_port_info *info;
>   	struct child_device_config *child;
>   	enum port p;
>   
> -	p = get_port_by_aux_ch(i915, info->alternate_aux_channel);
> +	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), info->alternate_aux_channel,
> +		    port_name(port), devdata->child.aux_channel,
>   		    port_name(p), port_name(p));
>   
>   	/*
> @@ -1602,7 +1603,7 @@ static void sanitize_aux_ch(struct drm_i915_private *i915,
>   	child = &info->devdata->child;
>   
>   	child->device_type &= ~DEVICE_TYPE_DISPLAYPORT_OUTPUT;
> -	info->alternate_aux_channel = 0;
> +	child->aux_channel = 0;
>   }
>   
>   static const u8 cnp_ddc_pin_map[] = {
> @@ -1980,11 +1981,8 @@ static void parse_ddi_port(struct drm_i915_private *i915,
>   		}
>   	}
>   
> -	if (is_dp) {
> -		info->alternate_aux_channel = child->aux_channel;
> -
> -		sanitize_aux_ch(i915, port);
> -	}
> +	if (is_dp)
> +		sanitize_aux_ch(devdata, port);
>   
>   	hdmi_level_shift = _intel_bios_hdmi_level_shift(devdata);
>   	if (hdmi_level_shift >= 0) {
> @@ -2863,7 +2861,7 @@ enum aux_ch intel_bios_port_aux_ch(struct drm_i915_private *i915,
>   		&i915->vbt.ddi_port_info[port];
>   	enum aux_ch aux_ch;
>   
> -	if (!info->alternate_aux_channel) {
> +	if (!info->devdata->child.aux_channel) {

Hi Jani,

The series and the change make sense to me.

 From the CI results it seems that cases with LVDS panel connected are 
getting issues here.

Apparently info->devdata is not set in this case. I guess that, 
parse_ddi_port() returns early before info->devdata gets set.

I think without the patch, this situation is not encountered due to the 
fact that 'info->alternate_aux_channel, is initialized to 0.

With this change, perhaps we should check for 'info->devdata' before 
checking for info->devdata->child.aux_channel.

(This will translate to checking for 'devdata' in the final patch as it 
removes ddi_port_info).

Hope it helps.

Regards,

Ankit


>   		aux_ch = (enum aux_ch)port;
>   
>   		drm_dbg_kms(&i915->drm,
> @@ -2879,7 +2877,7 @@ enum aux_ch intel_bios_port_aux_ch(struct drm_i915_private *i915,
>   	 * ADL-S VBT uses PHY based mapping. Combo PHYs A,B,C,D,E
>   	 * map to DDI A,TC1,TC2,TC3,TC4 respectively.
>   	 */
> -	switch (info->alternate_aux_channel) {
> +	switch (info->devdata->child.aux_channel) {
>   	case DP_AUX_A:
>   		aux_ch = AUX_CH_A;
>   		break;
> @@ -2940,7 +2938,7 @@ enum aux_ch intel_bios_port_aux_ch(struct drm_i915_private *i915,
>   			aux_ch = AUX_CH_I;
>   		break;
>   	default:
> -		MISSING_CASE(info->alternate_aux_channel);
> +		MISSING_CASE(info->devdata->child.aux_channel);
>   		aux_ch = AUX_CH_A;
>   		break;
>   	}
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index a0dead9f9222..91097526cd96 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -640,7 +640,6 @@ struct ddi_vbt_port_info {
>   	/* Non-NULL if port present. */
>   	struct intel_bios_encoder_data *devdata;
>   
> -	u8 alternate_aux_channel;
>   	u8 alternate_ddc_pin;
>   };
>   

Re: [Intel-gfx] [PATCH 4/7] drm/i915/bios: use alternate aux channel directly from child data

[Jani Nikula]

On Thu, 26 Aug 2021, "Nautiyal, Ankit K" <ankit.k.nautiyal@intel.com> wrote:
> On 8/24/2021 7:04 PM, Jani Nikula wrote:
>> Avoid extra caching of the data.
>>
>> Cc: José Roberto de Souza <jose.souza@intel.com>
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> ---
>>   drivers/gpu/drm/i915/display/intel_bios.c | 26 +++++++++++------------
>>   drivers/gpu/drm/i915/i915_drv.h           |  1 -
>>   2 files changed, 12 insertions(+), 15 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c
>> index 10b2beddc121..674f1424fcc2 100644
>> --- a/drivers/gpu/drm/i915/display/intel_bios.c
>> +++ b/drivers/gpu/drm/i915/display/intel_bios.c
>> @@ -1565,28 +1565,29 @@ static enum port get_port_by_aux_ch(struct drm_i915_private *i915, u8 aux_ch)
>>   	for_each_port(port) {
>>   		info = &i915->vbt.ddi_port_info[port];
>>   
>> -		if (info->devdata && aux_ch == info->alternate_aux_channel)
>> +		if (info->devdata && aux_ch == info->devdata->child.aux_channel)
>>   			return port;
>>   	}
>>   
>>   	return PORT_NONE;
>>   }
>>   
>> -static void sanitize_aux_ch(struct drm_i915_private *i915,
>> +static void sanitize_aux_ch(struct intel_bios_encoder_data *devdata,
>>   			    enum port port)
>>   {
>> -	struct ddi_vbt_port_info *info = &i915->vbt.ddi_port_info[port];
>> +	struct drm_i915_private *i915 = devdata->i915;
>> +	struct ddi_vbt_port_info *info;
>>   	struct child_device_config *child;
>>   	enum port p;
>>   
>> -	p = get_port_by_aux_ch(i915, info->alternate_aux_channel);
>> +	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), info->alternate_aux_channel,
>> +		    port_name(port), devdata->child.aux_channel,
>>   		    port_name(p), port_name(p));
>>   
>>   	/*
>> @@ -1602,7 +1603,7 @@ static void sanitize_aux_ch(struct drm_i915_private *i915,
>>   	child = &info->devdata->child;
>>   
>>   	child->device_type &= ~DEVICE_TYPE_DISPLAYPORT_OUTPUT;
>> -	info->alternate_aux_channel = 0;
>> +	child->aux_channel = 0;
>>   }
>>   
>>   static const u8 cnp_ddc_pin_map[] = {
>> @@ -1980,11 +1981,8 @@ static void parse_ddi_port(struct drm_i915_private *i915,
>>   		}
>>   	}
>>   
>> -	if (is_dp) {
>> -		info->alternate_aux_channel = child->aux_channel;
>> -
>> -		sanitize_aux_ch(i915, port);
>> -	}
>> +	if (is_dp)
>> +		sanitize_aux_ch(devdata, port);
>>   
>>   	hdmi_level_shift = _intel_bios_hdmi_level_shift(devdata);
>>   	if (hdmi_level_shift >= 0) {
>> @@ -2863,7 +2861,7 @@ enum aux_ch intel_bios_port_aux_ch(struct drm_i915_private *i915,
>>   		&i915->vbt.ddi_port_info[port];
>>   	enum aux_ch aux_ch;
>>   
>> -	if (!info->alternate_aux_channel) {
>> +	if (!info->devdata->child.aux_channel) {
>
> Hi Jani,
>
> The series and the change make sense to me.
>
>  From the CI results it seems that cases with LVDS panel connected are 
> getting issues here.
>
> Apparently info->devdata is not set in this case. I guess that, 
> parse_ddi_port() returns early before info->devdata gets set.
>
> I think without the patch, this situation is not encountered due to the 
> fact that 'info->alternate_aux_channel, is initialized to 0.
>
> With this change, perhaps we should check for 'info->devdata' before 
> checking for info->devdata->child.aux_channel.
>
> (This will translate to checking for 'devdata' in the final patch as it 
> removes ddi_port_info).
>
> Hope it helps.

Yes, indeed, thanks for figuring this out!

And thanks for the reviews.

BR,
Jani.


>
> Regards,
>
> Ankit
>
>
>>   		aux_ch = (enum aux_ch)port;
>>   
>>   		drm_dbg_kms(&i915->drm,
>> @@ -2879,7 +2877,7 @@ enum aux_ch intel_bios_port_aux_ch(struct drm_i915_private *i915,
>>   	 * ADL-S VBT uses PHY based mapping. Combo PHYs A,B,C,D,E
>>   	 * map to DDI A,TC1,TC2,TC3,TC4 respectively.
>>   	 */
>> -	switch (info->alternate_aux_channel) {
>> +	switch (info->devdata->child.aux_channel) {
>>   	case DP_AUX_A:
>>   		aux_ch = AUX_CH_A;
>>   		break;
>> @@ -2940,7 +2938,7 @@ enum aux_ch intel_bios_port_aux_ch(struct drm_i915_private *i915,
>>   			aux_ch = AUX_CH_I;
>>   		break;
>>   	default:
>> -		MISSING_CASE(info->alternate_aux_channel);
>> +		MISSING_CASE(info->devdata->child.aux_channel);
>>   		aux_ch = AUX_CH_A;
>>   		break;
>>   	}
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> index a0dead9f9222..91097526cd96 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -640,7 +640,6 @@ struct ddi_vbt_port_info {
>>   	/* Non-NULL if port present. */
>>   	struct intel_bios_encoder_data *devdata;
>>   
>> -	u8 alternate_aux_channel;
>>   	u8 alternate_ddc_pin;
>>   };
>>   

-- 
Jani Nikula, Intel Open Source Graphics Center

[Intel-gfx] [PATCH 5/7] drm/i915/bios: move ddc pin mapping code next to ddc pin sanitize

[Jani Nikula]

Move code around to avoid a forward declaration in the future.

Cc: José Roberto de Souza <jose.souza@intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/display/intel_bios.c | 154 +++++++++++-----------
 1 file changed, 77 insertions(+), 77 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c
index 674f1424fcc2..029a10be1586 100644
--- a/drivers/gpu/drm/i915/display/intel_bios.c
+++ b/drivers/gpu/drm/i915/display/intel_bios.c
@@ -1501,6 +1501,83 @@ static u8 translate_iboost(u8 val)
 	return mapping[val];
 }
 
+static const u8 cnp_ddc_pin_map[] = {
+	[0] = 0, /* N/A */
+	[DDC_BUS_DDI_B] = GMBUS_PIN_1_BXT,
+	[DDC_BUS_DDI_C] = GMBUS_PIN_2_BXT,
+	[DDC_BUS_DDI_D] = GMBUS_PIN_4_CNP, /* sic */
+	[DDC_BUS_DDI_F] = GMBUS_PIN_3_BXT, /* sic */
+};
+
+static const u8 icp_ddc_pin_map[] = {
+	[ICL_DDC_BUS_DDI_A] = GMBUS_PIN_1_BXT,
+	[ICL_DDC_BUS_DDI_B] = GMBUS_PIN_2_BXT,
+	[TGL_DDC_BUS_DDI_C] = GMBUS_PIN_3_BXT,
+	[ICL_DDC_BUS_PORT_1] = GMBUS_PIN_9_TC1_ICP,
+	[ICL_DDC_BUS_PORT_2] = GMBUS_PIN_10_TC2_ICP,
+	[ICL_DDC_BUS_PORT_3] = GMBUS_PIN_11_TC3_ICP,
+	[ICL_DDC_BUS_PORT_4] = GMBUS_PIN_12_TC4_ICP,
+	[TGL_DDC_BUS_PORT_5] = GMBUS_PIN_13_TC5_TGP,
+	[TGL_DDC_BUS_PORT_6] = GMBUS_PIN_14_TC6_TGP,
+};
+
+static const u8 rkl_pch_tgp_ddc_pin_map[] = {
+	[ICL_DDC_BUS_DDI_A] = GMBUS_PIN_1_BXT,
+	[ICL_DDC_BUS_DDI_B] = GMBUS_PIN_2_BXT,
+	[RKL_DDC_BUS_DDI_D] = GMBUS_PIN_9_TC1_ICP,
+	[RKL_DDC_BUS_DDI_E] = GMBUS_PIN_10_TC2_ICP,
+};
+
+static const u8 adls_ddc_pin_map[] = {
+	[ICL_DDC_BUS_DDI_A] = GMBUS_PIN_1_BXT,
+	[ADLS_DDC_BUS_PORT_TC1] = GMBUS_PIN_9_TC1_ICP,
+	[ADLS_DDC_BUS_PORT_TC2] = GMBUS_PIN_10_TC2_ICP,
+	[ADLS_DDC_BUS_PORT_TC3] = GMBUS_PIN_11_TC3_ICP,
+	[ADLS_DDC_BUS_PORT_TC4] = GMBUS_PIN_12_TC4_ICP,
+};
+
+static const u8 gen9bc_tgp_ddc_pin_map[] = {
+	[DDC_BUS_DDI_B] = GMBUS_PIN_2_BXT,
+	[DDC_BUS_DDI_C] = GMBUS_PIN_9_TC1_ICP,
+	[DDC_BUS_DDI_D] = GMBUS_PIN_10_TC2_ICP,
+};
+
+static u8 map_ddc_pin(struct drm_i915_private *i915, u8 vbt_pin)
+{
+	const u8 *ddc_pin_map;
+	int n_entries;
+
+	if (IS_ALDERLAKE_S(i915)) {
+		ddc_pin_map = adls_ddc_pin_map;
+		n_entries = ARRAY_SIZE(adls_ddc_pin_map);
+	} else if (INTEL_PCH_TYPE(i915) >= PCH_DG1) {
+		return vbt_pin;
+	} else if (IS_ROCKETLAKE(i915) && INTEL_PCH_TYPE(i915) == PCH_TGP) {
+		ddc_pin_map = rkl_pch_tgp_ddc_pin_map;
+		n_entries = ARRAY_SIZE(rkl_pch_tgp_ddc_pin_map);
+	} else if (HAS_PCH_TGP(i915) && DISPLAY_VER(i915) == 9) {
+		ddc_pin_map = gen9bc_tgp_ddc_pin_map;
+		n_entries = ARRAY_SIZE(gen9bc_tgp_ddc_pin_map);
+	} else if (INTEL_PCH_TYPE(i915) >= PCH_ICP) {
+		ddc_pin_map = icp_ddc_pin_map;
+		n_entries = ARRAY_SIZE(icp_ddc_pin_map);
+	} else if (HAS_PCH_CNP(i915)) {
+		ddc_pin_map = cnp_ddc_pin_map;
+		n_entries = ARRAY_SIZE(cnp_ddc_pin_map);
+	} else {
+		/* Assuming direct map */
+		return vbt_pin;
+	}
+
+	if (vbt_pin < n_entries && ddc_pin_map[vbt_pin] != 0)
+		return ddc_pin_map[vbt_pin];
+
+	drm_dbg_kms(&i915->drm,
+		    "Ignoring alternate pin: VBT claims DDC pin %d, which is not valid for this platform\n",
+		    vbt_pin);
+	return 0;
+}
+
 static enum port get_port_by_ddc_pin(struct drm_i915_private *i915, u8 ddc_pin)
 {
 	const struct ddi_vbt_port_info *info;
@@ -1606,83 +1683,6 @@ static void sanitize_aux_ch(struct intel_bios_encoder_data *devdata,
 	child->aux_channel = 0;
 }
 
-static const u8 cnp_ddc_pin_map[] = {
-	[0] = 0, /* N/A */
-	[DDC_BUS_DDI_B] = GMBUS_PIN_1_BXT,
-	[DDC_BUS_DDI_C] = GMBUS_PIN_2_BXT,
-	[DDC_BUS_DDI_D] = GMBUS_PIN_4_CNP, /* sic */
-	[DDC_BUS_DDI_F] = GMBUS_PIN_3_BXT, /* sic */
-};
-
-static const u8 icp_ddc_pin_map[] = {
-	[ICL_DDC_BUS_DDI_A] = GMBUS_PIN_1_BXT,
-	[ICL_DDC_BUS_DDI_B] = GMBUS_PIN_2_BXT,
-	[TGL_DDC_BUS_DDI_C] = GMBUS_PIN_3_BXT,
-	[ICL_DDC_BUS_PORT_1] = GMBUS_PIN_9_TC1_ICP,
-	[ICL_DDC_BUS_PORT_2] = GMBUS_PIN_10_TC2_ICP,
-	[ICL_DDC_BUS_PORT_3] = GMBUS_PIN_11_TC3_ICP,
-	[ICL_DDC_BUS_PORT_4] = GMBUS_PIN_12_TC4_ICP,
-	[TGL_DDC_BUS_PORT_5] = GMBUS_PIN_13_TC5_TGP,
-	[TGL_DDC_BUS_PORT_6] = GMBUS_PIN_14_TC6_TGP,
-};
-
-static const u8 rkl_pch_tgp_ddc_pin_map[] = {
-	[ICL_DDC_BUS_DDI_A] = GMBUS_PIN_1_BXT,
-	[ICL_DDC_BUS_DDI_B] = GMBUS_PIN_2_BXT,
-	[RKL_DDC_BUS_DDI_D] = GMBUS_PIN_9_TC1_ICP,
-	[RKL_DDC_BUS_DDI_E] = GMBUS_PIN_10_TC2_ICP,
-};
-
-static const u8 adls_ddc_pin_map[] = {
-	[ICL_DDC_BUS_DDI_A] = GMBUS_PIN_1_BXT,
-	[ADLS_DDC_BUS_PORT_TC1] = GMBUS_PIN_9_TC1_ICP,
-	[ADLS_DDC_BUS_PORT_TC2] = GMBUS_PIN_10_TC2_ICP,
-	[ADLS_DDC_BUS_PORT_TC3] = GMBUS_PIN_11_TC3_ICP,
-	[ADLS_DDC_BUS_PORT_TC4] = GMBUS_PIN_12_TC4_ICP,
-};
-
-static const u8 gen9bc_tgp_ddc_pin_map[] = {
-	[DDC_BUS_DDI_B] = GMBUS_PIN_2_BXT,
-	[DDC_BUS_DDI_C] = GMBUS_PIN_9_TC1_ICP,
-	[DDC_BUS_DDI_D] = GMBUS_PIN_10_TC2_ICP,
-};
-
-static u8 map_ddc_pin(struct drm_i915_private *i915, u8 vbt_pin)
-{
-	const u8 *ddc_pin_map;
-	int n_entries;
-
-	if (IS_ALDERLAKE_S(i915)) {
-		ddc_pin_map = adls_ddc_pin_map;
-		n_entries = ARRAY_SIZE(adls_ddc_pin_map);
-	} else if (INTEL_PCH_TYPE(i915) >= PCH_DG1) {
-		return vbt_pin;
-	} else if (IS_ROCKETLAKE(i915) && INTEL_PCH_TYPE(i915) == PCH_TGP) {
-		ddc_pin_map = rkl_pch_tgp_ddc_pin_map;
-		n_entries = ARRAY_SIZE(rkl_pch_tgp_ddc_pin_map);
-	} else if (HAS_PCH_TGP(i915) && DISPLAY_VER(i915) == 9) {
-		ddc_pin_map = gen9bc_tgp_ddc_pin_map;
-		n_entries = ARRAY_SIZE(gen9bc_tgp_ddc_pin_map);
-	} else if (INTEL_PCH_TYPE(i915) >= PCH_ICP) {
-		ddc_pin_map = icp_ddc_pin_map;
-		n_entries = ARRAY_SIZE(icp_ddc_pin_map);
-	} else if (HAS_PCH_CNP(i915)) {
-		ddc_pin_map = cnp_ddc_pin_map;
-		n_entries = ARRAY_SIZE(cnp_ddc_pin_map);
-	} else {
-		/* Assuming direct map */
-		return vbt_pin;
-	}
-
-	if (vbt_pin < n_entries && ddc_pin_map[vbt_pin] != 0)
-		return ddc_pin_map[vbt_pin];
-
-	drm_dbg_kms(&i915->drm,
-		    "Ignoring alternate pin: VBT claims DDC pin %d, which is not valid for this platform\n",
-		    vbt_pin);
-	return 0;
-}
-
 static enum port __dvo_port_to_port(int n_ports, int n_dvo,
 				    const int port_mapping[][3], u8 dvo_port)
 {
-- 
2.20.1

[Intel-gfx] [PATCH 6/7] drm/i915/bios: use ddc pin directly from child data

[Jani Nikula]

Avoid extra caching of the data. This is slightly more subtle than one
would think. For one thing, we explicitly ignore 0 value in child device
ddc pin; this is specified as N/A and does not warrant a warning. For
another, we start looking for ddc pin collisions in sanitize using
unmapped pin numbering.

Cc: José Roberto de Souza <jose.souza@intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/display/intel_bios.c | 49 +++++++++++++----------
 drivers/gpu/drm/i915/i915_drv.h           |  2 -
 2 files changed, 28 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c
index 029a10be1586..d4596be29d7b 100644
--- a/drivers/gpu/drm/i915/display/intel_bios.c
+++ b/drivers/gpu/drm/i915/display/intel_bios.c
@@ -1589,28 +1589,43 @@ static enum port get_port_by_ddc_pin(struct drm_i915_private *i915, u8 ddc_pin)
 	for_each_port(port) {
 		info = &i915->vbt.ddi_port_info[port];
 
-		if (info->devdata && ddc_pin == info->alternate_ddc_pin)
+		if (info->devdata && ddc_pin == info->devdata->child.ddc_pin)
 			return port;
 	}
 
 	return PORT_NONE;
 }
 
-static void sanitize_ddc_pin(struct drm_i915_private *i915,
+static void sanitize_ddc_pin(struct intel_bios_encoder_data *devdata,
 			     enum port port)
 {
-	struct ddi_vbt_port_info *info = &i915->vbt.ddi_port_info[port];
+	struct drm_i915_private *i915 = devdata->i915;
+	struct ddi_vbt_port_info *info;
 	struct child_device_config *child;
+	u8 mapped_ddc_pin;
 	enum port p;
 
-	p = get_port_by_ddc_pin(i915, info->alternate_ddc_pin);
+	if (!devdata->child.ddc_pin)
+		return;
+
+	mapped_ddc_pin = map_ddc_pin(i915, devdata->child.ddc_pin);
+	if (!intel_gmbus_is_valid_pin(i915, mapped_ddc_pin)) {
+		drm_dbg_kms(&i915->drm,
+			    "Port %c has invalid DDC pin %d, "
+			    "sticking to defaults\n",
+			    port_name(port), mapped_ddc_pin);
+		devdata->child.ddc_pin = 0;
+		return;
+	}
+
+	p = get_port_by_ddc_pin(i915, devdata->child.ddc_pin);
 	if (p == PORT_NONE)
 		return;
 
 	drm_dbg_kms(&i915->drm,
 		    "port %c trying to use the same DDC pin (0x%x) as port %c, "
 		    "disabling port %c DVI/HDMI support\n",
-		    port_name(port), info->alternate_ddc_pin,
+		    port_name(port), mapped_ddc_pin,
 		    port_name(p), port_name(p));
 
 	/*
@@ -1628,7 +1643,7 @@ static void sanitize_ddc_pin(struct drm_i915_private *i915,
 	child->device_type &= ~DEVICE_TYPE_TMDS_DVI_SIGNALING;
 	child->device_type |= DEVICE_TYPE_NOT_HDMI_OUTPUT;
 
-	info->alternate_ddc_pin = 0;
+	child->ddc_pin = 0;
 }
 
 static enum port get_port_by_aux_ch(struct drm_i915_private *i915, u8 aux_ch)
@@ -1966,20 +1981,8 @@ static void parse_ddi_port(struct drm_i915_private *i915,
 		    supports_typec_usb, supports_tbt,
 		    devdata->dsc != NULL);
 
-	if (is_dvi) {
-		u8 ddc_pin;
-
-		ddc_pin = map_ddc_pin(i915, child->ddc_pin);
-		if (intel_gmbus_is_valid_pin(i915, ddc_pin)) {
-			info->alternate_ddc_pin = ddc_pin;
-			sanitize_ddc_pin(i915, port);
-		} else {
-			drm_dbg_kms(&i915->drm,
-				    "Port %c has invalid DDC pin %d, "
-				    "sticking to defaults\n",
-				    port_name(port), ddc_pin);
-		}
-	}
+	if (is_dvi)
+		sanitize_ddc_pin(devdata, port);
 
 	if (is_dp)
 		sanitize_aux_ch(devdata, port);
@@ -2993,8 +2996,12 @@ int intel_bios_dp_max_link_rate(struct intel_encoder *encoder)
 int intel_bios_alternate_ddc_pin(struct intel_encoder *encoder)
 {
 	struct drm_i915_private *i915 = to_i915(encoder->base.dev);
+	const struct intel_bios_encoder_data *devdata = i915->vbt.ddi_port_info[encoder->port].devdata;
+
+	if (!devdata->child.ddc_pin)
+		return 0;
 
-	return i915->vbt.ddi_port_info[encoder->port].alternate_ddc_pin;
+	return map_ddc_pin(i915, devdata->child.ddc_pin);
 }
 
 bool intel_bios_encoder_supports_typec_usb(const struct intel_bios_encoder_data *devdata)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 91097526cd96..0ddae9db166b 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -639,8 +639,6 @@ i915_fence_timeout(const struct drm_i915_private *i915)
 struct ddi_vbt_port_info {
 	/* Non-NULL if port present. */
 	struct intel_bios_encoder_data *devdata;
-
-	u8 alternate_ddc_pin;
 };
 
 enum psr_lines_to_wait {
-- 
2.20.1

[Intel-gfx] [PATCH 7/7] drm/i915/bios: get rid of vbt ddi_port_info

[Jani Nikula]

We can finally remove the extra caching in ddi_port_info. Good riddance.

Cc: José Roberto de Souza <jose.souza@intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/display/intel_bios.c | 63 +++++++++--------------
 drivers/gpu/drm/i915/i915_drv.h           |  7 +--
 2 files changed, 25 insertions(+), 45 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c
index d4596be29d7b..184cceb021ea 100644
--- a/drivers/gpu/drm/i915/display/intel_bios.c
+++ b/drivers/gpu/drm/i915/display/intel_bios.c
@@ -1580,16 +1580,16 @@ static u8 map_ddc_pin(struct drm_i915_private *i915, u8 vbt_pin)
 
 static enum port get_port_by_ddc_pin(struct drm_i915_private *i915, u8 ddc_pin)
 {
-	const struct ddi_vbt_port_info *info;
+	const struct intel_bios_encoder_data *devdata;
 	enum port port;
 
 	if (!ddc_pin)
 		return PORT_NONE;
 
 	for_each_port(port) {
-		info = &i915->vbt.ddi_port_info[port];
+		devdata = i915->vbt.ports[port];
 
-		if (info->devdata && ddc_pin == info->devdata->child.ddc_pin)
+		if (devdata && ddc_pin == devdata->child.ddc_pin)
 			return port;
 	}
 
@@ -1600,7 +1600,6 @@ static void sanitize_ddc_pin(struct intel_bios_encoder_data *devdata,
 			     enum port port)
 {
 	struct drm_i915_private *i915 = devdata->i915;
-	struct ddi_vbt_port_info *info;
 	struct child_device_config *child;
 	u8 mapped_ddc_pin;
 	enum port p;
@@ -1637,8 +1636,7 @@ static void sanitize_ddc_pin(struct intel_bios_encoder_data *devdata,
 	 * 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 :(
 	 */
-	info = &i915->vbt.ddi_port_info[p];
-	child = &info->devdata->child;
+	child = &i915->vbt.ports[p]->child;
 
 	child->device_type &= ~DEVICE_TYPE_TMDS_DVI_SIGNALING;
 	child->device_type |= DEVICE_TYPE_NOT_HDMI_OUTPUT;
@@ -1648,16 +1646,16 @@ static void sanitize_ddc_pin(struct intel_bios_encoder_data *devdata,
 
 static enum port get_port_by_aux_ch(struct drm_i915_private *i915, u8 aux_ch)
 {
-	const struct ddi_vbt_port_info *info;
+	const struct intel_bios_encoder_data *devdata;
 	enum port port;
 
 	if (!aux_ch)
 		return PORT_NONE;
 
 	for_each_port(port) {
-		info = &i915->vbt.ddi_port_info[port];
+		devdata = i915->vbt.ports[port];
 
-		if (info->devdata && aux_ch == info->devdata->child.aux_channel)
+		if (devdata && aux_ch == devdata->child.aux_channel)
 			return port;
 	}
 
@@ -1668,7 +1666,6 @@ static void sanitize_aux_ch(struct intel_bios_encoder_data *devdata,
 			    enum port port)
 {
 	struct drm_i915_private *i915 = devdata->i915;
-	struct ddi_vbt_port_info *info;
 	struct child_device_config *child;
 	enum port p;
 
@@ -1691,8 +1688,7 @@ static void sanitize_aux_ch(struct intel_bios_encoder_data *devdata,
 	 * 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 :(
 	 */
-	info = &i915->vbt.ddi_port_info[p];
-	child = &info->devdata->child;
+	child = &i915->vbt.ports[p]->child;
 
 	child->device_type &= ~DEVICE_TYPE_DISPLAYPORT_OUTPUT;
 	child->aux_channel = 0;
@@ -1938,7 +1934,6 @@ static void parse_ddi_port(struct drm_i915_private *i915,
 			   struct intel_bios_encoder_data *devdata)
 {
 	const struct child_device_config *child = &devdata->child;
-	struct ddi_vbt_port_info *info;
 	bool is_dvi, is_hdmi, is_dp, is_edp, is_crt, supports_typec_usb, supports_tbt;
 	int dp_boost_level, dp_max_link_rate, hdmi_boost_level, hdmi_level_shift, max_tmds_clock;
 	enum port port;
@@ -1954,9 +1949,7 @@ static void parse_ddi_port(struct drm_i915_private *i915,
 		return;
 	}
 
-	info = &i915->vbt.ddi_port_info[port];
-
-	if (info->devdata) {
+	if (i915->vbt.ports[port]) {
 		drm_dbg_kms(&i915->drm,
 			    "More than one child device for port %c in VBT, using the first.\n",
 			    port_name(port));
@@ -2019,7 +2012,7 @@ static void parse_ddi_port(struct drm_i915_private *i915,
 			    "Port %c VBT DP max link rate: %d\n",
 			    port_name(port), dp_max_link_rate);
 
-	info->devdata = devdata;
+	i915->vbt.ports[port] = devdata;
 }
 
 static void parse_ddi_ports(struct drm_i915_private *i915)
@@ -2557,12 +2550,8 @@ bool intel_bios_is_port_present(struct drm_i915_private *i915, enum port port)
 		[PORT_F] = { DVO_PORT_DPF, DVO_PORT_HDMIF, },
 	};
 
-	if (HAS_DDI(i915)) {
-		const struct ddi_vbt_port_info *port_info =
-			&i915->vbt.ddi_port_info[port];
-
-		return port_info->devdata;
-	}
+	if (HAS_DDI(i915))
+		return i915->vbt.ports[port];
 
 	/* FIXME maybe deal with port A as well? */
 	if (drm_WARN_ON(&i915->drm,
@@ -2813,8 +2802,7 @@ bool
 intel_bios_is_port_hpd_inverted(const struct drm_i915_private *i915,
 				enum port port)
 {
-	const struct intel_bios_encoder_data *devdata =
-		i915->vbt.ddi_port_info[port].devdata;
+	const struct intel_bios_encoder_data *devdata = i915->vbt.ports[port];
 
 	if (drm_WARN_ON_ONCE(&i915->drm,
 			     !IS_GEMINILAKE(i915) && !IS_BROXTON(i915)))
@@ -2834,8 +2822,7 @@ bool
 intel_bios_is_lspcon_present(const struct drm_i915_private *i915,
 			     enum port port)
 {
-	const struct intel_bios_encoder_data *devdata =
-		i915->vbt.ddi_port_info[port].devdata;
+	const struct intel_bios_encoder_data *devdata = i915->vbt.ports[port];
 
 	return HAS_LSPCON(i915) && devdata && devdata->child.lspcon;
 }
@@ -2851,8 +2838,7 @@ bool
 intel_bios_is_lane_reversal_needed(const struct drm_i915_private *i915,
 				   enum port port)
 {
-	const struct intel_bios_encoder_data *devdata =
-		i915->vbt.ddi_port_info[port].devdata;
+	const struct intel_bios_encoder_data *devdata = i915->vbt.ports[port];
 
 	return devdata && devdata->child.lane_reversal;
 }
@@ -2860,11 +2846,10 @@ intel_bios_is_lane_reversal_needed(const struct drm_i915_private *i915,
 enum aux_ch intel_bios_port_aux_ch(struct drm_i915_private *i915,
 				   enum port port)
 {
-	const struct ddi_vbt_port_info *info =
-		&i915->vbt.ddi_port_info[port];
+	const struct intel_bios_encoder_data *devdata = i915->vbt.ports[port];
 	enum aux_ch aux_ch;
 
-	if (!info->devdata->child.aux_channel) {
+	if (!devdata->child.aux_channel) {
 		aux_ch = (enum aux_ch)port;
 
 		drm_dbg_kms(&i915->drm,
@@ -2880,7 +2865,7 @@ enum aux_ch intel_bios_port_aux_ch(struct drm_i915_private *i915,
 	 * ADL-S VBT uses PHY based mapping. Combo PHYs A,B,C,D,E
 	 * map to DDI A,TC1,TC2,TC3,TC4 respectively.
 	 */
-	switch (info->devdata->child.aux_channel) {
+	switch (devdata->child.aux_channel) {
 	case DP_AUX_A:
 		aux_ch = AUX_CH_A;
 		break;
@@ -2941,7 +2926,7 @@ enum aux_ch intel_bios_port_aux_ch(struct drm_i915_private *i915,
 			aux_ch = AUX_CH_I;
 		break;
 	default:
-		MISSING_CASE(info->devdata->child.aux_channel);
+		MISSING_CASE(devdata->child.aux_channel);
 		aux_ch = AUX_CH_A;
 		break;
 	}
@@ -2955,7 +2940,7 @@ enum aux_ch intel_bios_port_aux_ch(struct drm_i915_private *i915,
 int intel_bios_max_tmds_clock(struct intel_encoder *encoder)
 {
 	struct drm_i915_private *i915 = to_i915(encoder->base.dev);
-	const struct intel_bios_encoder_data *devdata = i915->vbt.ddi_port_info[encoder->port].devdata;
+	const struct intel_bios_encoder_data *devdata = i915->vbt.ports[encoder->port];
 
 	return _intel_bios_max_tmds_clock(devdata);
 }
@@ -2964,7 +2949,7 @@ int intel_bios_max_tmds_clock(struct intel_encoder *encoder)
 int intel_bios_hdmi_level_shift(struct intel_encoder *encoder)
 {
 	struct drm_i915_private *i915 = to_i915(encoder->base.dev);
-	const struct intel_bios_encoder_data *devdata = i915->vbt.ddi_port_info[encoder->port].devdata;
+	const struct intel_bios_encoder_data *devdata = i915->vbt.ports[encoder->port];
 
 	return _intel_bios_hdmi_level_shift(devdata);
 }
@@ -2988,7 +2973,7 @@ int intel_bios_encoder_hdmi_boost_level(const struct intel_bios_encoder_data *de
 int intel_bios_dp_max_link_rate(struct intel_encoder *encoder)
 {
 	struct drm_i915_private *i915 = to_i915(encoder->base.dev);
-	const struct intel_bios_encoder_data *devdata = i915->vbt.ddi_port_info[encoder->port].devdata;
+	const struct intel_bios_encoder_data *devdata = i915->vbt.ports[encoder->port];
 
 	return _intel_bios_dp_max_link_rate(devdata);
 }
@@ -2996,7 +2981,7 @@ int intel_bios_dp_max_link_rate(struct intel_encoder *encoder)
 int intel_bios_alternate_ddc_pin(struct intel_encoder *encoder)
 {
 	struct drm_i915_private *i915 = to_i915(encoder->base.dev);
-	const struct intel_bios_encoder_data *devdata = i915->vbt.ddi_port_info[encoder->port].devdata;
+	const struct intel_bios_encoder_data *devdata = i915->vbt.ports[encoder->port];
 
 	if (!devdata->child.ddc_pin)
 		return 0;
@@ -3017,5 +3002,5 @@ bool intel_bios_encoder_supports_tbt(const struct intel_bios_encoder_data *devda
 const struct intel_bios_encoder_data *
 intel_bios_encoder_data_lookup(struct drm_i915_private *i915, enum port port)
 {
-	return i915->vbt.ddi_port_info[port].devdata;
+	return i915->vbt.ports[port];
 }
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 0ddae9db166b..12b7376ec7ff 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -636,11 +636,6 @@ i915_fence_timeout(const struct drm_i915_private *i915)
 /* Amount of PSF GV points, BSpec precisely defines this */
 #define I915_NUM_PSF_GV_POINTS 3
 
-struct ddi_vbt_port_info {
-	/* Non-NULL if port present. */
-	struct intel_bios_encoder_data *devdata;
-};
-
 enum psr_lines_to_wait {
 	PSR_0_LINES_TO_WAIT = 0,
 	PSR_1_LINE_TO_WAIT,
@@ -721,7 +716,7 @@ struct intel_vbt_data {
 
 	struct list_head display_devices;
 
-	struct ddi_vbt_port_info ddi_port_info[I915_MAX_PORTS];
+	struct intel_bios_encoder_data *ports[I915_MAX_PORTS]; /* Non-NULL if port present. */
 	struct sdvo_device_mapping sdvo_mappings[2];
 };
 
-- 
2.20.1

[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915/bios: remove vbt ddi_port_info caching

[Patchwork]

== Series Details ==

Series: drm/i915/bios: remove vbt ddi_port_info caching
URL   : https://patchwork.freedesktop.org/series/93957/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
b95ad462eb0e drm/i915/bios: use hdmi level shift directly from child data
-:71: WARNING:LONG_LINE: line length of 103 exceeds 100 columns
#71: FILE: drivers/gpu/drm/i915/display/intel_bios.c:2961:
+	const struct intel_bios_encoder_data *devdata = i915->vbt.ddi_port_info[encoder->port].devdata;

total: 0 errors, 1 warnings, 0 checks, 66 lines checked
d1719ebad2f9 drm/i915/bios: use max tmds clock directly from child data
-:92: WARNING:LONG_LINE: line length of 103 exceeds 100 columns
#92: FILE: drivers/gpu/drm/i915/display/intel_bios.c:2952:
+	const struct intel_bios_encoder_data *devdata = i915->vbt.ddi_port_info[encoder->port].devdata;

total: 0 errors, 1 warnings, 0 checks, 85 lines checked
c13124605d17 drm/i915/bios: use dp max link rate directly from child data
-:70: WARNING:LONG_LINE: line length of 103 exceeds 100 columns
#70: FILE: drivers/gpu/drm/i915/display/intel_bios.c:2990:
+	const struct intel_bios_encoder_data *devdata = i915->vbt.ddi_port_info[encoder->port].devdata;

total: 0 errors, 1 warnings, 0 checks, 63 lines checked
d1d79de86edf drm/i915/bios: use alternate aux channel directly from child data
5724045c5163 drm/i915/bios: move ddc pin mapping code next to ddc pin sanitize
356bf413ae7b drm/i915/bios: use ddc pin directly from child data
-:107: WARNING:LONG_LINE: line length of 103 exceeds 100 columns
#107: FILE: drivers/gpu/drm/i915/display/intel_bios.c:2999:
+	const struct intel_bios_encoder_data *devdata = i915->vbt.ddi_port_info[encoder->port].devdata;

total: 0 errors, 1 warnings, 0 checks, 99 lines checked
9e4d240f4472 drm/i915/bios: get rid of vbt ddi_port_info

[Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915/bios: remove vbt ddi_port_info caching

[Patchwork]

[-- Attachment #1: Type: text/plain, Size: 4659 bytes --]

== Series Details ==

Series: drm/i915/bios: remove vbt ddi_port_info caching
URL   : https://patchwork.freedesktop.org/series/93957/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_10514 -> Patchwork_20878
====================================================

Summary
-------

  **FAILURE**

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

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20878/index.html

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

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

### CI changes ###

#### Possible regressions ####

  * boot:
    - fi-snb-2520m:       [PASS][1] -> [FAIL][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10514/fi-snb-2520m/boot.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20878/fi-snb-2520m/boot.html
    - fi-ilk-650:         [PASS][3] -> [FAIL][4]
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10514/fi-ilk-650/boot.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20878/fi-ilk-650/boot.html
    - fi-elk-e7500:       [PASS][5] -> [FAIL][6]
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10514/fi-elk-e7500/boot.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20878/fi-elk-e7500/boot.html
    - fi-ivb-3770:        [PASS][7] -> [FAIL][8]
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10514/fi-ivb-3770/boot.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20878/fi-ivb-3770/boot.html

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

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

### IGT changes ###

#### Issues hit ####

  * igt@i915_module_load@reload:
    - fi-tgl-1115g4:      [PASS][9] -> [DMESG-WARN][10] ([i915#1385] / [i915#4002])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10514/fi-tgl-1115g4/igt@i915_module_load@reload.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20878/fi-tgl-1115g4/igt@i915_module_load@reload.html

  * igt@i915_selftest@live@workarounds:
    - fi-rkl-guc:         [PASS][11] -> [DMESG-FAIL][12] ([i915#3928])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10514/fi-rkl-guc/igt@i915_selftest@live@workarounds.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20878/fi-rkl-guc/igt@i915_selftest@live@workarounds.html

  * igt@runner@aborted:
    - fi-rkl-guc:         NOTRUN -> [FAIL][13] ([i915#3928])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20878/fi-rkl-guc/igt@runner@aborted.html

  
#### Warnings ####

  * igt@kms_psr@primary_page_flip:
    - fi-tgl-1115g4:      [SKIP][14] ([i915#1072]) -> [SKIP][15] ([i915#1072] / [i915#1385])
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10514/fi-tgl-1115g4/igt@kms_psr@primary_page_flip.html
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20878/fi-tgl-1115g4/igt@kms_psr@primary_page_flip.html

  
  [i915#1072]: https://gitlab.freedesktop.org/drm/intel/issues/1072
  [i915#1385]: https://gitlab.freedesktop.org/drm/intel/issues/1385
  [i915#3928]: https://gitlab.freedesktop.org/drm/intel/issues/3928
  [i915#4002]: https://gitlab.freedesktop.org/drm/intel/issues/4002


Participating hosts (40 -> 34)
------------------------------

  Missing    (6): fi-ilk-m540 bat-adls-5 fi-hsw-4200u fi-bsw-cyan fi-bdw-samus bat-jsl-1 


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

  * Linux: CI_DRM_10514 -> Patchwork_20878

  CI-20190529: 20190529
  CI_DRM_10514: cceaf272e7176ed1eeaaf548056efb1c25533cae @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_6184: 84f9bbde1e6156c8b978613d9c85c9043fd3180c @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_20878: 9e4d240f4472e25eb77ebee1bd0dc65432c38489 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

9e4d240f4472 drm/i915/bios: get rid of vbt ddi_port_info
356bf413ae7b drm/i915/bios: use ddc pin directly from child data
5724045c5163 drm/i915/bios: move ddc pin mapping code next to ddc pin sanitize
d1d79de86edf drm/i915/bios: use alternate aux channel directly from child data
c13124605d17 drm/i915/bios: use dp max link rate directly from child data
d1719ebad2f9 drm/i915/bios: use max tmds clock directly from child data
b95ad462eb0e drm/i915/bios: use hdmi level shift directly from child data

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20878/index.html

[-- Attachment #2: Type: text/html, Size: 5672 bytes --]