* [PATCH] drm/i915/bios: store child devices in a list
@ 2019-11-06 16:45 ` Jani Nikula
0 siblings, 0 replies; 12+ messages in thread
From: Jani Nikula @ 2019-11-06 16:45 UTC (permalink / raw)
To: intel-gfx; +Cc: jani.nikula
Using the array is getting clumsy. Make things a bit more dynamic.
In code, start migrating towards calling the new struct child_device
"child" and the VBT-originating struct child_device_config "config".
Remove early returns on not having child devices when the end result
after "iterating" the empty list would be the same.
Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
The end goal: allow more meta information to be added to the new
child_device struct, independent of DDI port info being used or not on
the platform, and eventually migrate ddi_port_info to it as well,
unifying the stuff across platforms.
Currently it's not easily possible to associate for example the DSC
compression data to the child device for non-DDI platforms or for DSI
outputs. This lets us add the compression data (pointer) to struct
child_device.
---
drivers/gpu/drm/i915/display/intel_bios.c | 203 ++++++++++------------
drivers/gpu/drm/i915/i915_drv.h | 3 +-
2 files changed, 97 insertions(+), 109 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c
index a03f56b7b4ef..025074862ab0 100644
--- a/drivers/gpu/drm/i915/display/intel_bios.c
+++ b/drivers/gpu/drm/i915/display/intel_bios.c
@@ -58,6 +58,12 @@
* that.
*/
+/* Wrapper for VBT child device config */
+struct child_device {
+ struct child_device_config config;
+ struct list_head node;
+};
+
#define SLAVE_ADDR1 0x70
#define SLAVE_ADDR2 0x72
@@ -449,8 +455,9 @@ static void
parse_sdvo_device_mapping(struct drm_i915_private *dev_priv, u8 bdb_version)
{
struct sdvo_device_mapping *mapping;
- const struct child_device_config *child;
- int i, count = 0;
+ const struct child_device_config *config;
+ const struct child_device *child;
+ int count = 0;
/*
* Only parse SDVO mappings on gens that could have SDVO. This isn't
@@ -461,35 +468,35 @@ parse_sdvo_device_mapping(struct drm_i915_private *dev_priv, u8 bdb_version)
return;
}
- for (i = 0, count = 0; i < dev_priv->vbt.child_dev_num; i++) {
- child = dev_priv->vbt.child_dev + i;
+ list_for_each_entry(child, &dev_priv->vbt.child_devices, node) {
+ config = &child->config;
- if (child->slave_addr != SLAVE_ADDR1 &&
- child->slave_addr != SLAVE_ADDR2) {
+ if (config->slave_addr != SLAVE_ADDR1 &&
+ config->slave_addr != SLAVE_ADDR2) {
/*
* If the slave address is neither 0x70 nor 0x72,
* it is not a SDVO device. Skip it.
*/
continue;
}
- if (child->dvo_port != DEVICE_PORT_DVOB &&
- child->dvo_port != DEVICE_PORT_DVOC) {
+ if (config->dvo_port != DEVICE_PORT_DVOB &&
+ config->dvo_port != DEVICE_PORT_DVOC) {
/* skip the incorrect SDVO port */
DRM_DEBUG_KMS("Incorrect SDVO port. Skip it\n");
continue;
}
DRM_DEBUG_KMS("the SDVO device with slave addr %2x is found on"
" %s port\n",
- child->slave_addr,
- (child->dvo_port == DEVICE_PORT_DVOB) ?
+ config->slave_addr,
+ (config->dvo_port == DEVICE_PORT_DVOB) ?
"SDVOB" : "SDVOC");
- mapping = &dev_priv->vbt.sdvo_mappings[child->dvo_port - 1];
+ mapping = &dev_priv->vbt.sdvo_mappings[config->dvo_port - 1];
if (!mapping->initialized) {
- mapping->dvo_port = child->dvo_port;
- mapping->slave_addr = child->slave_addr;
- mapping->dvo_wiring = child->dvo_wiring;
- mapping->ddc_pin = child->ddc_pin;
- mapping->i2c_pin = child->i2c_pin;
+ mapping->dvo_port = config->dvo_port;
+ mapping->slave_addr = config->slave_addr;
+ mapping->dvo_wiring = config->dvo_wiring;
+ mapping->ddc_pin = config->ddc_pin;
+ mapping->i2c_pin = config->i2c_pin;
mapping->initialized = 1;
DRM_DEBUG_KMS("SDVO device: dvo=%x, addr=%x, wiring=%d, ddc_pin=%d, i2c_pin=%d\n",
mapping->dvo_port,
@@ -501,7 +508,7 @@ parse_sdvo_device_mapping(struct drm_i915_private *dev_priv, u8 bdb_version)
DRM_DEBUG_KMS("Maybe one SDVO port is shared by "
"two SDVO device.\n");
}
- if (child->slave2_addr) {
+ if (config->slave2_addr) {
/* Maybe this is a SDVO device with multiple inputs */
/* And the mapping info is not added */
DRM_DEBUG_KMS("there exists the slave2_addr. Maybe this"
@@ -1571,8 +1578,7 @@ static void parse_ddi_port(struct drm_i915_private *dev_priv,
static void parse_ddi_ports(struct drm_i915_private *dev_priv, u8 bdb_version)
{
- const struct child_device_config *child;
- int i;
+ const struct child_device *child;
if (!HAS_DDI(dev_priv) && !IS_CHERRYVIEW(dev_priv))
return;
@@ -1580,11 +1586,8 @@ static void parse_ddi_ports(struct drm_i915_private *dev_priv, u8 bdb_version)
if (bdb_version < 155)
return;
- 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);
- }
+ list_for_each_entry(child, &dev_priv->vbt.child_devices, node)
+ parse_ddi_port(dev_priv, &child->config, bdb_version);
}
static void
@@ -1592,8 +1595,9 @@ parse_general_definitions(struct drm_i915_private *dev_priv,
const struct bdb_header *bdb)
{
const struct bdb_general_definitions *defs;
- const struct child_device_config *child;
- int i, child_device_num, count;
+ const struct child_device_config *config;
+ struct child_device *child;
+ int i, child_device_num;
u8 expected_size;
u16 block_size;
int bus_pin;
@@ -1629,8 +1633,8 @@ parse_general_definitions(struct drm_i915_private *dev_priv,
} else if (bdb->version <= 229) {
expected_size = 39;
} else {
- expected_size = sizeof(*child);
- BUILD_BUG_ON(sizeof(*child) < 39);
+ expected_size = sizeof(*config);
+ BUILD_BUG_ON(sizeof(*config) < 39);
DRM_DEBUG_DRIVER("Expected child device config size for VBT version %u not known; assuming %u\n",
bdb->version, expected_size);
}
@@ -1649,43 +1653,30 @@ parse_general_definitions(struct drm_i915_private *dev_priv,
/* get the number of child device */
child_device_num = (block_size - sizeof(*defs)) / defs->child_dev_size;
- count = 0;
- /* get the number of child device that is present */
- for (i = 0; i < child_device_num; i++) {
- child = child_device_ptr(defs, i);
- if (!child->device_type)
- continue;
- count++;
- }
- if (!count) {
- DRM_DEBUG_KMS("no child dev is parsed from VBT\n");
- return;
- }
- dev_priv->vbt.child_dev = kcalloc(count, sizeof(*child), GFP_KERNEL);
- if (!dev_priv->vbt.child_dev) {
- DRM_DEBUG_KMS("No memory space for child device\n");
- return;
- }
- dev_priv->vbt.child_dev_num = count;
- count = 0;
for (i = 0; i < child_device_num; i++) {
- child = child_device_ptr(defs, i);
- if (!child->device_type)
+ config = child_device_ptr(defs, i);
+ if (!config->device_type)
continue;
DRM_DEBUG_KMS("Found VBT child device with type 0x%x\n",
- child->device_type);
+ config->device_type);
+
+ child = kmalloc(sizeof(*child), GFP_KERNEL);
/*
* Copy as much as we know (sizeof) and is available
- * (child_dev_size) of the child device. Accessing the data must
- * depend on VBT version.
+ * (child_dev_size) of the child device config. Accessing the
+ * data must depend on VBT version.
*/
- memcpy(dev_priv->vbt.child_dev + count, child,
- min_t(size_t, defs->child_dev_size, sizeof(*child)));
- count++;
+ memcpy(&child->config, config,
+ min_t(size_t, defs->child_dev_size, sizeof(*config)));
+
+ list_add(&child->node, &dev_priv->vbt.child_devices);
}
+
+ if (list_empty(&dev_priv->vbt.child_devices))
+ DRM_DEBUG_KMS("no child dev is parsed from VBT\n");
}
/* Common defaults which may be overridden by VBT. */
@@ -1849,6 +1840,8 @@ void intel_bios_init(struct drm_i915_private *dev_priv)
return;
}
+ INIT_LIST_HEAD(&dev_priv->vbt.child_devices);
+
init_vbt_defaults(dev_priv);
/* If the OpRegion does not have VBT, look in PCI ROM. */
@@ -1903,9 +1896,13 @@ void intel_bios_init(struct drm_i915_private *dev_priv)
*/
void intel_bios_driver_remove(struct drm_i915_private *dev_priv)
{
- kfree(dev_priv->vbt.child_dev);
- dev_priv->vbt.child_dev = NULL;
- dev_priv->vbt.child_dev_num = 0;
+ struct child_device *child, *n;
+
+ list_for_each_entry_safe(child, n, &dev_priv->vbt.child_devices, node) {
+ list_del(&child->node);
+ kfree(child);
+ }
+
kfree(dev_priv->vbt.sdvo_lvds_vbt_mode);
dev_priv->vbt.sdvo_lvds_vbt_mode = NULL;
kfree(dev_priv->vbt.lfp_lvds_vbt_mode);
@@ -1929,21 +1926,22 @@ void intel_bios_driver_remove(struct drm_i915_private *dev_priv)
*/
bool intel_bios_is_tv_present(struct drm_i915_private *dev_priv)
{
- const struct child_device_config *child;
- int i;
+ const struct child_device_config *config;
+ const struct child_device *child;
if (!dev_priv->vbt.int_tv_support)
return false;
- if (!dev_priv->vbt.child_dev_num)
+ if (list_empty(&dev_priv->vbt.child_devices))
return true;
- for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
- child = dev_priv->vbt.child_dev + i;
+ list_for_each_entry(child, &dev_priv->vbt.child_devices, node) {
+ config = &child->config;
+
/*
* If the device type is not TV, continue.
*/
- switch (child->device_type) {
+ switch (config->device_type) {
case DEVICE_TYPE_INT_TV:
case DEVICE_TYPE_TV:
case DEVICE_TYPE_TV_SVIDEO_COMPOSITE:
@@ -1954,7 +1952,7 @@ bool intel_bios_is_tv_present(struct drm_i915_private *dev_priv)
/* Only when the addin_offset is non-zero, it is regarded
* as present.
*/
- if (child->addin_offset)
+ if (config->addin_offset)
return true;
}
@@ -1971,32 +1969,32 @@ bool intel_bios_is_tv_present(struct drm_i915_private *dev_priv)
*/
bool intel_bios_is_lvds_present(struct drm_i915_private *dev_priv, u8 *i2c_pin)
{
- const struct child_device_config *child;
- int i;
+ const struct child_device *child;
+ const struct child_device_config *config;
- if (!dev_priv->vbt.child_dev_num)
+ if (list_empty(&dev_priv->vbt.child_devices))
return true;
- for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
- child = dev_priv->vbt.child_dev + i;
+ list_for_each_entry(child, &dev_priv->vbt.child_devices, node) {
+ config = &child->config;
/* If the device type is not LFP, continue.
* We have to check both the new identifiers as well as the
* old for compatibility with some BIOSes.
*/
- if (child->device_type != DEVICE_TYPE_INT_LFP &&
- child->device_type != DEVICE_TYPE_LFP)
+ if (config->device_type != DEVICE_TYPE_INT_LFP &&
+ config->device_type != DEVICE_TYPE_LFP)
continue;
- if (intel_gmbus_is_valid_pin(dev_priv, child->i2c_pin))
- *i2c_pin = child->i2c_pin;
+ if (intel_gmbus_is_valid_pin(dev_priv, config->i2c_pin))
+ *i2c_pin = config->i2c_pin;
/* However, we cannot trust the BIOS writers to populate
* the VBT correctly. Since LVDS requires additional
* information from AIM blocks, a non-zero addin offset is
* a good indicator that the LVDS is actually present.
*/
- if (child->addin_offset)
+ if (config->addin_offset)
return true;
/* But even then some BIOS writers perform some black magic
@@ -2020,7 +2018,8 @@ bool intel_bios_is_lvds_present(struct drm_i915_private *dev_priv, u8 *i2c_pin)
*/
bool intel_bios_is_port_present(struct drm_i915_private *dev_priv, enum port port)
{
- const struct child_device_config *child;
+ const struct child_device_config *config;
+ const struct child_device *child;
static const struct {
u16 dp, hdmi;
} port_mapping[] = {
@@ -2030,7 +2029,6 @@ bool intel_bios_is_port_present(struct drm_i915_private *dev_priv, enum port por
[PORT_E] = { DVO_PORT_DPE, DVO_PORT_HDMIE, },
[PORT_F] = { DVO_PORT_DPF, DVO_PORT_HDMIF, },
};
- int i;
if (HAS_DDI(dev_priv)) {
const struct ddi_vbt_port_info *port_info =
@@ -2045,16 +2043,13 @@ bool intel_bios_is_port_present(struct drm_i915_private *dev_priv, enum port por
if (WARN_ON(port == PORT_A) || port >= ARRAY_SIZE(port_mapping))
return false;
- if (!dev_priv->vbt.child_dev_num)
- return false;
-
- for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
- child = dev_priv->vbt.child_dev + i;
+ list_for_each_entry(child, &dev_priv->vbt.child_devices, node) {
+ config = &child->config;
- if ((child->dvo_port == port_mapping[port].dp ||
- child->dvo_port == port_mapping[port].hdmi) &&
- (child->device_type & (DEVICE_TYPE_TMDS_DVI_SIGNALING |
- DEVICE_TYPE_DISPLAYPORT_OUTPUT)))
+ if ((config->dvo_port == port_mapping[port].dp ||
+ config->dvo_port == port_mapping[port].hdmi) &&
+ (config->device_type & (DEVICE_TYPE_TMDS_DVI_SIGNALING |
+ DEVICE_TYPE_DISPLAYPORT_OUTPUT)))
return true;
}
@@ -2070,7 +2065,8 @@ bool intel_bios_is_port_present(struct drm_i915_private *dev_priv, enum port por
*/
bool intel_bios_is_port_edp(struct drm_i915_private *dev_priv, enum port port)
{
- const struct child_device_config *child;
+ const struct child_device_config *config;
+ const struct child_device *child;
static const short port_mapping[] = {
[PORT_B] = DVO_PORT_DPB,
[PORT_C] = DVO_PORT_DPC,
@@ -2078,19 +2074,15 @@ bool intel_bios_is_port_edp(struct drm_i915_private *dev_priv, enum port port)
[PORT_E] = DVO_PORT_DPE,
[PORT_F] = DVO_PORT_DPF,
};
- int i;
if (HAS_DDI(dev_priv))
return dev_priv->vbt.ddi_port_info[port].supports_edp;
- if (!dev_priv->vbt.child_dev_num)
- return false;
+ list_for_each_entry(child, &dev_priv->vbt.child_devices, node) {
+ config = &child->config;
- for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
- child = dev_priv->vbt.child_dev + i;
-
- if (child->dvo_port == port_mapping[port] &&
- (child->device_type & DEVICE_TYPE_eDP_BITS) ==
+ if (config->dvo_port == port_mapping[port] &&
+ (config->device_type & DEVICE_TYPE_eDP_BITS) ==
(DEVICE_TYPE_eDP & DEVICE_TYPE_eDP_BITS))
return true;
}
@@ -2136,13 +2128,10 @@ static bool child_dev_is_dp_dual_mode(const struct child_device_config *child,
bool intel_bios_is_port_dp_dual_mode(struct drm_i915_private *dev_priv,
enum port port)
{
- const struct child_device_config *child;
- int i;
+ const struct child_device *child;
- for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
- child = dev_priv->vbt.child_dev + i;
-
- if (child_dev_is_dp_dual_mode(child, port))
+ list_for_each_entry(child, &dev_priv->vbt.child_devices, node) {
+ if (child_dev_is_dp_dual_mode(&child->config, port))
return true;
}
@@ -2159,17 +2148,17 @@ bool intel_bios_is_port_dp_dual_mode(struct drm_i915_private *dev_priv,
bool intel_bios_is_dsi_present(struct drm_i915_private *dev_priv,
enum port *port)
{
- const struct child_device_config *child;
+ const struct child_device_config *config;
+ const struct child_device *child;
u8 dvo_port;
- int i;
- for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
- child = dev_priv->vbt.child_dev + i;
+ list_for_each_entry(child, &dev_priv->vbt.child_devices, node) {
+ config = &child->config;
- if (!(child->device_type & DEVICE_TYPE_MIPI_OUTPUT))
+ if (!(config->device_type & DEVICE_TYPE_MIPI_OUTPUT))
continue;
- dvo_port = child->dvo_port;
+ dvo_port = config->dvo_port;
if (dvo_port == DVO_PORT_MIPIA ||
(dvo_port == DVO_PORT_MIPIB && INTEL_GEN(dev_priv) >= 11) ||
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 7e0f67babe20..ae85ffa08665 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -724,8 +724,7 @@ struct intel_vbt_data {
int crt_ddc_pin;
- int child_dev_num;
- struct child_device_config *child_dev;
+ struct list_head child_devices;
struct ddi_vbt_port_info ddi_port_info[I915_MAX_PORTS];
struct sdvo_device_mapping sdvo_mappings[2];
--
2.20.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [Intel-gfx] [PATCH] drm/i915/bios: store child devices in a list
@ 2019-11-06 16:45 ` Jani Nikula
0 siblings, 0 replies; 12+ messages in thread
From: Jani Nikula @ 2019-11-06 16:45 UTC (permalink / raw)
To: intel-gfx; +Cc: jani.nikula
Using the array is getting clumsy. Make things a bit more dynamic.
In code, start migrating towards calling the new struct child_device
"child" and the VBT-originating struct child_device_config "config".
Remove early returns on not having child devices when the end result
after "iterating" the empty list would be the same.
Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
The end goal: allow more meta information to be added to the new
child_device struct, independent of DDI port info being used or not on
the platform, and eventually migrate ddi_port_info to it as well,
unifying the stuff across platforms.
Currently it's not easily possible to associate for example the DSC
compression data to the child device for non-DDI platforms or for DSI
outputs. This lets us add the compression data (pointer) to struct
child_device.
---
drivers/gpu/drm/i915/display/intel_bios.c | 203 ++++++++++------------
drivers/gpu/drm/i915/i915_drv.h | 3 +-
2 files changed, 97 insertions(+), 109 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c
index a03f56b7b4ef..025074862ab0 100644
--- a/drivers/gpu/drm/i915/display/intel_bios.c
+++ b/drivers/gpu/drm/i915/display/intel_bios.c
@@ -58,6 +58,12 @@
* that.
*/
+/* Wrapper for VBT child device config */
+struct child_device {
+ struct child_device_config config;
+ struct list_head node;
+};
+
#define SLAVE_ADDR1 0x70
#define SLAVE_ADDR2 0x72
@@ -449,8 +455,9 @@ static void
parse_sdvo_device_mapping(struct drm_i915_private *dev_priv, u8 bdb_version)
{
struct sdvo_device_mapping *mapping;
- const struct child_device_config *child;
- int i, count = 0;
+ const struct child_device_config *config;
+ const struct child_device *child;
+ int count = 0;
/*
* Only parse SDVO mappings on gens that could have SDVO. This isn't
@@ -461,35 +468,35 @@ parse_sdvo_device_mapping(struct drm_i915_private *dev_priv, u8 bdb_version)
return;
}
- for (i = 0, count = 0; i < dev_priv->vbt.child_dev_num; i++) {
- child = dev_priv->vbt.child_dev + i;
+ list_for_each_entry(child, &dev_priv->vbt.child_devices, node) {
+ config = &child->config;
- if (child->slave_addr != SLAVE_ADDR1 &&
- child->slave_addr != SLAVE_ADDR2) {
+ if (config->slave_addr != SLAVE_ADDR1 &&
+ config->slave_addr != SLAVE_ADDR2) {
/*
* If the slave address is neither 0x70 nor 0x72,
* it is not a SDVO device. Skip it.
*/
continue;
}
- if (child->dvo_port != DEVICE_PORT_DVOB &&
- child->dvo_port != DEVICE_PORT_DVOC) {
+ if (config->dvo_port != DEVICE_PORT_DVOB &&
+ config->dvo_port != DEVICE_PORT_DVOC) {
/* skip the incorrect SDVO port */
DRM_DEBUG_KMS("Incorrect SDVO port. Skip it\n");
continue;
}
DRM_DEBUG_KMS("the SDVO device with slave addr %2x is found on"
" %s port\n",
- child->slave_addr,
- (child->dvo_port == DEVICE_PORT_DVOB) ?
+ config->slave_addr,
+ (config->dvo_port == DEVICE_PORT_DVOB) ?
"SDVOB" : "SDVOC");
- mapping = &dev_priv->vbt.sdvo_mappings[child->dvo_port - 1];
+ mapping = &dev_priv->vbt.sdvo_mappings[config->dvo_port - 1];
if (!mapping->initialized) {
- mapping->dvo_port = child->dvo_port;
- mapping->slave_addr = child->slave_addr;
- mapping->dvo_wiring = child->dvo_wiring;
- mapping->ddc_pin = child->ddc_pin;
- mapping->i2c_pin = child->i2c_pin;
+ mapping->dvo_port = config->dvo_port;
+ mapping->slave_addr = config->slave_addr;
+ mapping->dvo_wiring = config->dvo_wiring;
+ mapping->ddc_pin = config->ddc_pin;
+ mapping->i2c_pin = config->i2c_pin;
mapping->initialized = 1;
DRM_DEBUG_KMS("SDVO device: dvo=%x, addr=%x, wiring=%d, ddc_pin=%d, i2c_pin=%d\n",
mapping->dvo_port,
@@ -501,7 +508,7 @@ parse_sdvo_device_mapping(struct drm_i915_private *dev_priv, u8 bdb_version)
DRM_DEBUG_KMS("Maybe one SDVO port is shared by "
"two SDVO device.\n");
}
- if (child->slave2_addr) {
+ if (config->slave2_addr) {
/* Maybe this is a SDVO device with multiple inputs */
/* And the mapping info is not added */
DRM_DEBUG_KMS("there exists the slave2_addr. Maybe this"
@@ -1571,8 +1578,7 @@ static void parse_ddi_port(struct drm_i915_private *dev_priv,
static void parse_ddi_ports(struct drm_i915_private *dev_priv, u8 bdb_version)
{
- const struct child_device_config *child;
- int i;
+ const struct child_device *child;
if (!HAS_DDI(dev_priv) && !IS_CHERRYVIEW(dev_priv))
return;
@@ -1580,11 +1586,8 @@ static void parse_ddi_ports(struct drm_i915_private *dev_priv, u8 bdb_version)
if (bdb_version < 155)
return;
- 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);
- }
+ list_for_each_entry(child, &dev_priv->vbt.child_devices, node)
+ parse_ddi_port(dev_priv, &child->config, bdb_version);
}
static void
@@ -1592,8 +1595,9 @@ parse_general_definitions(struct drm_i915_private *dev_priv,
const struct bdb_header *bdb)
{
const struct bdb_general_definitions *defs;
- const struct child_device_config *child;
- int i, child_device_num, count;
+ const struct child_device_config *config;
+ struct child_device *child;
+ int i, child_device_num;
u8 expected_size;
u16 block_size;
int bus_pin;
@@ -1629,8 +1633,8 @@ parse_general_definitions(struct drm_i915_private *dev_priv,
} else if (bdb->version <= 229) {
expected_size = 39;
} else {
- expected_size = sizeof(*child);
- BUILD_BUG_ON(sizeof(*child) < 39);
+ expected_size = sizeof(*config);
+ BUILD_BUG_ON(sizeof(*config) < 39);
DRM_DEBUG_DRIVER("Expected child device config size for VBT version %u not known; assuming %u\n",
bdb->version, expected_size);
}
@@ -1649,43 +1653,30 @@ parse_general_definitions(struct drm_i915_private *dev_priv,
/* get the number of child device */
child_device_num = (block_size - sizeof(*defs)) / defs->child_dev_size;
- count = 0;
- /* get the number of child device that is present */
- for (i = 0; i < child_device_num; i++) {
- child = child_device_ptr(defs, i);
- if (!child->device_type)
- continue;
- count++;
- }
- if (!count) {
- DRM_DEBUG_KMS("no child dev is parsed from VBT\n");
- return;
- }
- dev_priv->vbt.child_dev = kcalloc(count, sizeof(*child), GFP_KERNEL);
- if (!dev_priv->vbt.child_dev) {
- DRM_DEBUG_KMS("No memory space for child device\n");
- return;
- }
- dev_priv->vbt.child_dev_num = count;
- count = 0;
for (i = 0; i < child_device_num; i++) {
- child = child_device_ptr(defs, i);
- if (!child->device_type)
+ config = child_device_ptr(defs, i);
+ if (!config->device_type)
continue;
DRM_DEBUG_KMS("Found VBT child device with type 0x%x\n",
- child->device_type);
+ config->device_type);
+
+ child = kmalloc(sizeof(*child), GFP_KERNEL);
/*
* Copy as much as we know (sizeof) and is available
- * (child_dev_size) of the child device. Accessing the data must
- * depend on VBT version.
+ * (child_dev_size) of the child device config. Accessing the
+ * data must depend on VBT version.
*/
- memcpy(dev_priv->vbt.child_dev + count, child,
- min_t(size_t, defs->child_dev_size, sizeof(*child)));
- count++;
+ memcpy(&child->config, config,
+ min_t(size_t, defs->child_dev_size, sizeof(*config)));
+
+ list_add(&child->node, &dev_priv->vbt.child_devices);
}
+
+ if (list_empty(&dev_priv->vbt.child_devices))
+ DRM_DEBUG_KMS("no child dev is parsed from VBT\n");
}
/* Common defaults which may be overridden by VBT. */
@@ -1849,6 +1840,8 @@ void intel_bios_init(struct drm_i915_private *dev_priv)
return;
}
+ INIT_LIST_HEAD(&dev_priv->vbt.child_devices);
+
init_vbt_defaults(dev_priv);
/* If the OpRegion does not have VBT, look in PCI ROM. */
@@ -1903,9 +1896,13 @@ void intel_bios_init(struct drm_i915_private *dev_priv)
*/
void intel_bios_driver_remove(struct drm_i915_private *dev_priv)
{
- kfree(dev_priv->vbt.child_dev);
- dev_priv->vbt.child_dev = NULL;
- dev_priv->vbt.child_dev_num = 0;
+ struct child_device *child, *n;
+
+ list_for_each_entry_safe(child, n, &dev_priv->vbt.child_devices, node) {
+ list_del(&child->node);
+ kfree(child);
+ }
+
kfree(dev_priv->vbt.sdvo_lvds_vbt_mode);
dev_priv->vbt.sdvo_lvds_vbt_mode = NULL;
kfree(dev_priv->vbt.lfp_lvds_vbt_mode);
@@ -1929,21 +1926,22 @@ void intel_bios_driver_remove(struct drm_i915_private *dev_priv)
*/
bool intel_bios_is_tv_present(struct drm_i915_private *dev_priv)
{
- const struct child_device_config *child;
- int i;
+ const struct child_device_config *config;
+ const struct child_device *child;
if (!dev_priv->vbt.int_tv_support)
return false;
- if (!dev_priv->vbt.child_dev_num)
+ if (list_empty(&dev_priv->vbt.child_devices))
return true;
- for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
- child = dev_priv->vbt.child_dev + i;
+ list_for_each_entry(child, &dev_priv->vbt.child_devices, node) {
+ config = &child->config;
+
/*
* If the device type is not TV, continue.
*/
- switch (child->device_type) {
+ switch (config->device_type) {
case DEVICE_TYPE_INT_TV:
case DEVICE_TYPE_TV:
case DEVICE_TYPE_TV_SVIDEO_COMPOSITE:
@@ -1954,7 +1952,7 @@ bool intel_bios_is_tv_present(struct drm_i915_private *dev_priv)
/* Only when the addin_offset is non-zero, it is regarded
* as present.
*/
- if (child->addin_offset)
+ if (config->addin_offset)
return true;
}
@@ -1971,32 +1969,32 @@ bool intel_bios_is_tv_present(struct drm_i915_private *dev_priv)
*/
bool intel_bios_is_lvds_present(struct drm_i915_private *dev_priv, u8 *i2c_pin)
{
- const struct child_device_config *child;
- int i;
+ const struct child_device *child;
+ const struct child_device_config *config;
- if (!dev_priv->vbt.child_dev_num)
+ if (list_empty(&dev_priv->vbt.child_devices))
return true;
- for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
- child = dev_priv->vbt.child_dev + i;
+ list_for_each_entry(child, &dev_priv->vbt.child_devices, node) {
+ config = &child->config;
/* If the device type is not LFP, continue.
* We have to check both the new identifiers as well as the
* old for compatibility with some BIOSes.
*/
- if (child->device_type != DEVICE_TYPE_INT_LFP &&
- child->device_type != DEVICE_TYPE_LFP)
+ if (config->device_type != DEVICE_TYPE_INT_LFP &&
+ config->device_type != DEVICE_TYPE_LFP)
continue;
- if (intel_gmbus_is_valid_pin(dev_priv, child->i2c_pin))
- *i2c_pin = child->i2c_pin;
+ if (intel_gmbus_is_valid_pin(dev_priv, config->i2c_pin))
+ *i2c_pin = config->i2c_pin;
/* However, we cannot trust the BIOS writers to populate
* the VBT correctly. Since LVDS requires additional
* information from AIM blocks, a non-zero addin offset is
* a good indicator that the LVDS is actually present.
*/
- if (child->addin_offset)
+ if (config->addin_offset)
return true;
/* But even then some BIOS writers perform some black magic
@@ -2020,7 +2018,8 @@ bool intel_bios_is_lvds_present(struct drm_i915_private *dev_priv, u8 *i2c_pin)
*/
bool intel_bios_is_port_present(struct drm_i915_private *dev_priv, enum port port)
{
- const struct child_device_config *child;
+ const struct child_device_config *config;
+ const struct child_device *child;
static const struct {
u16 dp, hdmi;
} port_mapping[] = {
@@ -2030,7 +2029,6 @@ bool intel_bios_is_port_present(struct drm_i915_private *dev_priv, enum port por
[PORT_E] = { DVO_PORT_DPE, DVO_PORT_HDMIE, },
[PORT_F] = { DVO_PORT_DPF, DVO_PORT_HDMIF, },
};
- int i;
if (HAS_DDI(dev_priv)) {
const struct ddi_vbt_port_info *port_info =
@@ -2045,16 +2043,13 @@ bool intel_bios_is_port_present(struct drm_i915_private *dev_priv, enum port por
if (WARN_ON(port == PORT_A) || port >= ARRAY_SIZE(port_mapping))
return false;
- if (!dev_priv->vbt.child_dev_num)
- return false;
-
- for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
- child = dev_priv->vbt.child_dev + i;
+ list_for_each_entry(child, &dev_priv->vbt.child_devices, node) {
+ config = &child->config;
- if ((child->dvo_port == port_mapping[port].dp ||
- child->dvo_port == port_mapping[port].hdmi) &&
- (child->device_type & (DEVICE_TYPE_TMDS_DVI_SIGNALING |
- DEVICE_TYPE_DISPLAYPORT_OUTPUT)))
+ if ((config->dvo_port == port_mapping[port].dp ||
+ config->dvo_port == port_mapping[port].hdmi) &&
+ (config->device_type & (DEVICE_TYPE_TMDS_DVI_SIGNALING |
+ DEVICE_TYPE_DISPLAYPORT_OUTPUT)))
return true;
}
@@ -2070,7 +2065,8 @@ bool intel_bios_is_port_present(struct drm_i915_private *dev_priv, enum port por
*/
bool intel_bios_is_port_edp(struct drm_i915_private *dev_priv, enum port port)
{
- const struct child_device_config *child;
+ const struct child_device_config *config;
+ const struct child_device *child;
static const short port_mapping[] = {
[PORT_B] = DVO_PORT_DPB,
[PORT_C] = DVO_PORT_DPC,
@@ -2078,19 +2074,15 @@ bool intel_bios_is_port_edp(struct drm_i915_private *dev_priv, enum port port)
[PORT_E] = DVO_PORT_DPE,
[PORT_F] = DVO_PORT_DPF,
};
- int i;
if (HAS_DDI(dev_priv))
return dev_priv->vbt.ddi_port_info[port].supports_edp;
- if (!dev_priv->vbt.child_dev_num)
- return false;
+ list_for_each_entry(child, &dev_priv->vbt.child_devices, node) {
+ config = &child->config;
- for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
- child = dev_priv->vbt.child_dev + i;
-
- if (child->dvo_port == port_mapping[port] &&
- (child->device_type & DEVICE_TYPE_eDP_BITS) ==
+ if (config->dvo_port == port_mapping[port] &&
+ (config->device_type & DEVICE_TYPE_eDP_BITS) ==
(DEVICE_TYPE_eDP & DEVICE_TYPE_eDP_BITS))
return true;
}
@@ -2136,13 +2128,10 @@ static bool child_dev_is_dp_dual_mode(const struct child_device_config *child,
bool intel_bios_is_port_dp_dual_mode(struct drm_i915_private *dev_priv,
enum port port)
{
- const struct child_device_config *child;
- int i;
+ const struct child_device *child;
- for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
- child = dev_priv->vbt.child_dev + i;
-
- if (child_dev_is_dp_dual_mode(child, port))
+ list_for_each_entry(child, &dev_priv->vbt.child_devices, node) {
+ if (child_dev_is_dp_dual_mode(&child->config, port))
return true;
}
@@ -2159,17 +2148,17 @@ bool intel_bios_is_port_dp_dual_mode(struct drm_i915_private *dev_priv,
bool intel_bios_is_dsi_present(struct drm_i915_private *dev_priv,
enum port *port)
{
- const struct child_device_config *child;
+ const struct child_device_config *config;
+ const struct child_device *child;
u8 dvo_port;
- int i;
- for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
- child = dev_priv->vbt.child_dev + i;
+ list_for_each_entry(child, &dev_priv->vbt.child_devices, node) {
+ config = &child->config;
- if (!(child->device_type & DEVICE_TYPE_MIPI_OUTPUT))
+ if (!(config->device_type & DEVICE_TYPE_MIPI_OUTPUT))
continue;
- dvo_port = child->dvo_port;
+ dvo_port = config->dvo_port;
if (dvo_port == DVO_PORT_MIPIA ||
(dvo_port == DVO_PORT_MIPIB && INTEL_GEN(dev_priv) >= 11) ||
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 7e0f67babe20..ae85ffa08665 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -724,8 +724,7 @@ struct intel_vbt_data {
int crt_ddc_pin;
- int child_dev_num;
- struct child_device_config *child_dev;
+ struct list_head child_devices;
struct ddi_vbt_port_info ddi_port_info[I915_MAX_PORTS];
struct sdvo_device_mapping sdvo_mappings[2];
--
2.20.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 12+ messages in thread
* ✗ Fi.CI.CHECKPATCH: warning for drm/i915/bios: store child devices in a list
@ 2019-11-06 20:16 ` Patchwork
0 siblings, 0 replies; 12+ messages in thread
From: Patchwork @ 2019-11-06 20:16 UTC (permalink / raw)
To: Jani Nikula; +Cc: intel-gfx
== Series Details ==
Series: drm/i915/bios: store child devices in a list
URL : https://patchwork.freedesktop.org/series/69077/
State : warning
== Summary ==
$ dim checkpatch origin/drm-tip
b08607b14961 drm/i915/bios: store child devices in a list
-:388: CHECK:CAMELCASE: Avoid CamelCase: <DEVICE_TYPE_eDP_BITS>
#388: FILE: drivers/gpu/drm/i915/display/intel_bios.c:2085:
+ (config->device_type & DEVICE_TYPE_eDP_BITS) ==
total: 0 errors, 0 warnings, 1 checks, 400 lines checked
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915/bios: store child devices in a list
@ 2019-11-06 20:16 ` Patchwork
0 siblings, 0 replies; 12+ messages in thread
From: Patchwork @ 2019-11-06 20:16 UTC (permalink / raw)
To: Jani Nikula; +Cc: intel-gfx
== Series Details ==
Series: drm/i915/bios: store child devices in a list
URL : https://patchwork.freedesktop.org/series/69077/
State : warning
== Summary ==
$ dim checkpatch origin/drm-tip
b08607b14961 drm/i915/bios: store child devices in a list
-:388: CHECK:CAMELCASE: Avoid CamelCase: <DEVICE_TYPE_eDP_BITS>
#388: FILE: drivers/gpu/drm/i915/display/intel_bios.c:2085:
+ (config->device_type & DEVICE_TYPE_eDP_BITS) ==
total: 0 errors, 0 warnings, 1 checks, 400 lines checked
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] drm/i915/bios: store child devices in a list
@ 2019-11-06 20:37 ` Ville Syrjälä
0 siblings, 0 replies; 12+ messages in thread
From: Ville Syrjälä @ 2019-11-06 20:37 UTC (permalink / raw)
To: Jani Nikula; +Cc: intel-gfx
On Wed, Nov 06, 2019 at 06:45:31PM +0200, Jani Nikula wrote:
> Using the array is getting clumsy. Make things a bit more dynamic.
>
> In code, start migrating towards calling the new struct child_device
> "child" and the VBT-originating struct child_device_config "config".
>
> Remove early returns on not having child devices when the end result
> after "iterating" the empty list would be the same.
>
> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>
> ---
>
> The end goal: allow more meta information to be added to the new
> child_device struct, independent of DDI port info being used or not on
> the platform, and eventually migrate ddi_port_info to it as well,
> unifying the stuff across platforms.
>
> Currently it's not easily possible to associate for example the DSC
> compression data to the child device for non-DDI platforms or for DSI
> outputs. This lets us add the compression data (pointer) to struct
> child_device.
> ---
> drivers/gpu/drm/i915/display/intel_bios.c | 203 ++++++++++------------
> drivers/gpu/drm/i915/i915_drv.h | 3 +-
> 2 files changed, 97 insertions(+), 109 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c
> index a03f56b7b4ef..025074862ab0 100644
> --- a/drivers/gpu/drm/i915/display/intel_bios.c
> +++ b/drivers/gpu/drm/i915/display/intel_bios.c
> @@ -58,6 +58,12 @@
> * that.
> */
>
> +/* Wrapper for VBT child device config */
> +struct child_device {
> + struct child_device_config config;
> + struct list_head node;
The wrapper is a bit unfortunate. I don't suppose we could just shove
the list head into the existing struct and adjust what needs adjusting?
> +};
> +
> #define SLAVE_ADDR1 0x70
> #define SLAVE_ADDR2 0x72
>
> @@ -449,8 +455,9 @@ static void
> parse_sdvo_device_mapping(struct drm_i915_private *dev_priv, u8 bdb_version)
> {
> struct sdvo_device_mapping *mapping;
> - const struct child_device_config *child;
> - int i, count = 0;
> + const struct child_device_config *config;
This thing could at least can live inside the loop. Though the rename is
also a bit unfortunate, leading to a needlessly large diff. Avoiding the
wrapper struct would also avoid that. I guess another option would
be to select a different name for the wrapper pointer here and keep the
original name for the actual thing.
> + const struct child_device *child;
> + int count = 0;
>
> /*
> * Only parse SDVO mappings on gens that could have SDVO. This isn't
> @@ -461,35 +468,35 @@ parse_sdvo_device_mapping(struct drm_i915_private *dev_priv, u8 bdb_version)
> return;
> }
>
> - for (i = 0, count = 0; i < dev_priv->vbt.child_dev_num; i++) {
> - child = dev_priv->vbt.child_dev + i;
> + list_for_each_entry(child, &dev_priv->vbt.child_devices, node) {
> + config = &child->config;
>
> - if (child->slave_addr != SLAVE_ADDR1 &&
> - child->slave_addr != SLAVE_ADDR2) {
> + if (config->slave_addr != SLAVE_ADDR1 &&
> + config->slave_addr != SLAVE_ADDR2) {
> /*
> * If the slave address is neither 0x70 nor 0x72,
> * it is not a SDVO device. Skip it.
> */
> continue;
> }
> - if (child->dvo_port != DEVICE_PORT_DVOB &&
> - child->dvo_port != DEVICE_PORT_DVOC) {
> + if (config->dvo_port != DEVICE_PORT_DVOB &&
> + config->dvo_port != DEVICE_PORT_DVOC) {
> /* skip the incorrect SDVO port */
> DRM_DEBUG_KMS("Incorrect SDVO port. Skip it\n");
> continue;
> }
> DRM_DEBUG_KMS("the SDVO device with slave addr %2x is found on"
> " %s port\n",
> - child->slave_addr,
> - (child->dvo_port == DEVICE_PORT_DVOB) ?
> + config->slave_addr,
> + (config->dvo_port == DEVICE_PORT_DVOB) ?
> "SDVOB" : "SDVOC");
> - mapping = &dev_priv->vbt.sdvo_mappings[child->dvo_port - 1];
> + mapping = &dev_priv->vbt.sdvo_mappings[config->dvo_port - 1];
> if (!mapping->initialized) {
> - mapping->dvo_port = child->dvo_port;
> - mapping->slave_addr = child->slave_addr;
> - mapping->dvo_wiring = child->dvo_wiring;
> - mapping->ddc_pin = child->ddc_pin;
> - mapping->i2c_pin = child->i2c_pin;
> + mapping->dvo_port = config->dvo_port;
> + mapping->slave_addr = config->slave_addr;
> + mapping->dvo_wiring = config->dvo_wiring;
> + mapping->ddc_pin = config->ddc_pin;
> + mapping->i2c_pin = config->i2c_pin;
> mapping->initialized = 1;
> DRM_DEBUG_KMS("SDVO device: dvo=%x, addr=%x, wiring=%d, ddc_pin=%d, i2c_pin=%d\n",
> mapping->dvo_port,
> @@ -501,7 +508,7 @@ parse_sdvo_device_mapping(struct drm_i915_private *dev_priv, u8 bdb_version)
> DRM_DEBUG_KMS("Maybe one SDVO port is shared by "
> "two SDVO device.\n");
> }
> - if (child->slave2_addr) {
> + if (config->slave2_addr) {
> /* Maybe this is a SDVO device with multiple inputs */
> /* And the mapping info is not added */
> DRM_DEBUG_KMS("there exists the slave2_addr. Maybe this"
> @@ -1571,8 +1578,7 @@ static void parse_ddi_port(struct drm_i915_private *dev_priv,
>
> static void parse_ddi_ports(struct drm_i915_private *dev_priv, u8 bdb_version)
> {
> - const struct child_device_config *child;
> - int i;
> + const struct child_device *child;
>
> if (!HAS_DDI(dev_priv) && !IS_CHERRYVIEW(dev_priv))
> return;
> @@ -1580,11 +1586,8 @@ static void parse_ddi_ports(struct drm_i915_private *dev_priv, u8 bdb_version)
> if (bdb_version < 155)
> return;
>
> - 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);
> - }
> + list_for_each_entry(child, &dev_priv->vbt.child_devices, node)
> + parse_ddi_port(dev_priv, &child->config, bdb_version);
> }
>
> static void
> @@ -1592,8 +1595,9 @@ parse_general_definitions(struct drm_i915_private *dev_priv,
> const struct bdb_header *bdb)
> {
> const struct bdb_general_definitions *defs;
> - const struct child_device_config *child;
> - int i, child_device_num, count;
> + const struct child_device_config *config;
> + struct child_device *child;
> + int i, child_device_num;
> u8 expected_size;
> u16 block_size;
> int bus_pin;
> @@ -1629,8 +1633,8 @@ parse_general_definitions(struct drm_i915_private *dev_priv,
> } else if (bdb->version <= 229) {
> expected_size = 39;
> } else {
> - expected_size = sizeof(*child);
> - BUILD_BUG_ON(sizeof(*child) < 39);
> + expected_size = sizeof(*config);
> + BUILD_BUG_ON(sizeof(*config) < 39);
> DRM_DEBUG_DRIVER("Expected child device config size for VBT version %u not known; assuming %u\n",
> bdb->version, expected_size);
> }
> @@ -1649,43 +1653,30 @@ parse_general_definitions(struct drm_i915_private *dev_priv,
>
> /* get the number of child device */
> child_device_num = (block_size - sizeof(*defs)) / defs->child_dev_size;
> - count = 0;
> - /* get the number of child device that is present */
> - for (i = 0; i < child_device_num; i++) {
> - child = child_device_ptr(defs, i);
> - if (!child->device_type)
> - continue;
> - count++;
> - }
> - if (!count) {
> - DRM_DEBUG_KMS("no child dev is parsed from VBT\n");
> - return;
> - }
> - dev_priv->vbt.child_dev = kcalloc(count, sizeof(*child), GFP_KERNEL);
> - if (!dev_priv->vbt.child_dev) {
> - DRM_DEBUG_KMS("No memory space for child device\n");
> - return;
> - }
>
> - dev_priv->vbt.child_dev_num = count;
> - count = 0;
> for (i = 0; i < child_device_num; i++) {
> - child = child_device_ptr(defs, i);
> - if (!child->device_type)
> + config = child_device_ptr(defs, i);
> + if (!config->device_type)
> continue;
>
> DRM_DEBUG_KMS("Found VBT child device with type 0x%x\n",
> - child->device_type);
> + config->device_type);
> +
> + child = kmalloc(sizeof(*child), GFP_KERNEL);
>
> /*
> * Copy as much as we know (sizeof) and is available
> - * (child_dev_size) of the child device. Accessing the data must
> - * depend on VBT version.
> + * (child_dev_size) of the child device config. Accessing the
> + * data must depend on VBT version.
> */
> - memcpy(dev_priv->vbt.child_dev + count, child,
> - min_t(size_t, defs->child_dev_size, sizeof(*child)));
> - count++;
> + memcpy(&child->config, config,
> + min_t(size_t, defs->child_dev_size, sizeof(*config)));
> +
> + list_add(&child->node, &dev_priv->vbt.child_devices);
> }
> +
> + if (list_empty(&dev_priv->vbt.child_devices))
> + DRM_DEBUG_KMS("no child dev is parsed from VBT\n");
> }
>
> /* Common defaults which may be overridden by VBT. */
> @@ -1849,6 +1840,8 @@ void intel_bios_init(struct drm_i915_private *dev_priv)
> return;
> }
>
> + INIT_LIST_HEAD(&dev_priv->vbt.child_devices);
> +
> init_vbt_defaults(dev_priv);
>
> /* If the OpRegion does not have VBT, look in PCI ROM. */
> @@ -1903,9 +1896,13 @@ void intel_bios_init(struct drm_i915_private *dev_priv)
> */
> void intel_bios_driver_remove(struct drm_i915_private *dev_priv)
> {
> - kfree(dev_priv->vbt.child_dev);
> - dev_priv->vbt.child_dev = NULL;
> - dev_priv->vbt.child_dev_num = 0;
> + struct child_device *child, *n;
> +
> + list_for_each_entry_safe(child, n, &dev_priv->vbt.child_devices, node) {
> + list_del(&child->node);
> + kfree(child);
> + }
> +
> kfree(dev_priv->vbt.sdvo_lvds_vbt_mode);
> dev_priv->vbt.sdvo_lvds_vbt_mode = NULL;
> kfree(dev_priv->vbt.lfp_lvds_vbt_mode);
> @@ -1929,21 +1926,22 @@ void intel_bios_driver_remove(struct drm_i915_private *dev_priv)
> */
> bool intel_bios_is_tv_present(struct drm_i915_private *dev_priv)
> {
> - const struct child_device_config *child;
> - int i;
> + const struct child_device_config *config;
> + const struct child_device *child;
>
> if (!dev_priv->vbt.int_tv_support)
> return false;
>
> - if (!dev_priv->vbt.child_dev_num)
> + if (list_empty(&dev_priv->vbt.child_devices))
> return true;
>
> - for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
> - child = dev_priv->vbt.child_dev + i;
> + list_for_each_entry(child, &dev_priv->vbt.child_devices, node) {
> + config = &child->config;
> +
> /*
> * If the device type is not TV, continue.
> */
> - switch (child->device_type) {
> + switch (config->device_type) {
> case DEVICE_TYPE_INT_TV:
> case DEVICE_TYPE_TV:
> case DEVICE_TYPE_TV_SVIDEO_COMPOSITE:
> @@ -1954,7 +1952,7 @@ bool intel_bios_is_tv_present(struct drm_i915_private *dev_priv)
> /* Only when the addin_offset is non-zero, it is regarded
> * as present.
> */
> - if (child->addin_offset)
> + if (config->addin_offset)
> return true;
> }
>
> @@ -1971,32 +1969,32 @@ bool intel_bios_is_tv_present(struct drm_i915_private *dev_priv)
> */
> bool intel_bios_is_lvds_present(struct drm_i915_private *dev_priv, u8 *i2c_pin)
> {
> - const struct child_device_config *child;
> - int i;
> + const struct child_device *child;
> + const struct child_device_config *config;
>
> - if (!dev_priv->vbt.child_dev_num)
> + if (list_empty(&dev_priv->vbt.child_devices))
> return true;
>
> - for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
> - child = dev_priv->vbt.child_dev + i;
> + list_for_each_entry(child, &dev_priv->vbt.child_devices, node) {
> + config = &child->config;
>
> /* If the device type is not LFP, continue.
> * We have to check both the new identifiers as well as the
> * old for compatibility with some BIOSes.
> */
> - if (child->device_type != DEVICE_TYPE_INT_LFP &&
> - child->device_type != DEVICE_TYPE_LFP)
> + if (config->device_type != DEVICE_TYPE_INT_LFP &&
> + config->device_type != DEVICE_TYPE_LFP)
> continue;
>
> - if (intel_gmbus_is_valid_pin(dev_priv, child->i2c_pin))
> - *i2c_pin = child->i2c_pin;
> + if (intel_gmbus_is_valid_pin(dev_priv, config->i2c_pin))
> + *i2c_pin = config->i2c_pin;
>
> /* However, we cannot trust the BIOS writers to populate
> * the VBT correctly. Since LVDS requires additional
> * information from AIM blocks, a non-zero addin offset is
> * a good indicator that the LVDS is actually present.
> */
> - if (child->addin_offset)
> + if (config->addin_offset)
> return true;
>
> /* But even then some BIOS writers perform some black magic
> @@ -2020,7 +2018,8 @@ bool intel_bios_is_lvds_present(struct drm_i915_private *dev_priv, u8 *i2c_pin)
> */
> bool intel_bios_is_port_present(struct drm_i915_private *dev_priv, enum port port)
> {
> - const struct child_device_config *child;
> + const struct child_device_config *config;
> + const struct child_device *child;
> static const struct {
> u16 dp, hdmi;
> } port_mapping[] = {
> @@ -2030,7 +2029,6 @@ bool intel_bios_is_port_present(struct drm_i915_private *dev_priv, enum port por
> [PORT_E] = { DVO_PORT_DPE, DVO_PORT_HDMIE, },
> [PORT_F] = { DVO_PORT_DPF, DVO_PORT_HDMIF, },
> };
> - int i;
>
> if (HAS_DDI(dev_priv)) {
> const struct ddi_vbt_port_info *port_info =
> @@ -2045,16 +2043,13 @@ bool intel_bios_is_port_present(struct drm_i915_private *dev_priv, enum port por
> if (WARN_ON(port == PORT_A) || port >= ARRAY_SIZE(port_mapping))
> return false;
>
> - if (!dev_priv->vbt.child_dev_num)
> - return false;
> -
> - for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
> - child = dev_priv->vbt.child_dev + i;
> + list_for_each_entry(child, &dev_priv->vbt.child_devices, node) {
> + config = &child->config;
>
> - if ((child->dvo_port == port_mapping[port].dp ||
> - child->dvo_port == port_mapping[port].hdmi) &&
> - (child->device_type & (DEVICE_TYPE_TMDS_DVI_SIGNALING |
> - DEVICE_TYPE_DISPLAYPORT_OUTPUT)))
> + if ((config->dvo_port == port_mapping[port].dp ||
> + config->dvo_port == port_mapping[port].hdmi) &&
> + (config->device_type & (DEVICE_TYPE_TMDS_DVI_SIGNALING |
> + DEVICE_TYPE_DISPLAYPORT_OUTPUT)))
> return true;
> }
>
> @@ -2070,7 +2065,8 @@ bool intel_bios_is_port_present(struct drm_i915_private *dev_priv, enum port por
> */
> bool intel_bios_is_port_edp(struct drm_i915_private *dev_priv, enum port port)
> {
> - const struct child_device_config *child;
> + const struct child_device_config *config;
> + const struct child_device *child;
> static const short port_mapping[] = {
> [PORT_B] = DVO_PORT_DPB,
> [PORT_C] = DVO_PORT_DPC,
> @@ -2078,19 +2074,15 @@ bool intel_bios_is_port_edp(struct drm_i915_private *dev_priv, enum port port)
> [PORT_E] = DVO_PORT_DPE,
> [PORT_F] = DVO_PORT_DPF,
> };
> - int i;
>
> if (HAS_DDI(dev_priv))
> return dev_priv->vbt.ddi_port_info[port].supports_edp;
>
> - if (!dev_priv->vbt.child_dev_num)
> - return false;
> + list_for_each_entry(child, &dev_priv->vbt.child_devices, node) {
> + config = &child->config;
>
> - for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
> - child = dev_priv->vbt.child_dev + i;
> -
> - if (child->dvo_port == port_mapping[port] &&
> - (child->device_type & DEVICE_TYPE_eDP_BITS) ==
> + if (config->dvo_port == port_mapping[port] &&
> + (config->device_type & DEVICE_TYPE_eDP_BITS) ==
> (DEVICE_TYPE_eDP & DEVICE_TYPE_eDP_BITS))
> return true;
> }
> @@ -2136,13 +2128,10 @@ static bool child_dev_is_dp_dual_mode(const struct child_device_config *child,
> bool intel_bios_is_port_dp_dual_mode(struct drm_i915_private *dev_priv,
> enum port port)
> {
> - const struct child_device_config *child;
> - int i;
> + const struct child_device *child;
>
> - for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
> - child = dev_priv->vbt.child_dev + i;
> -
> - if (child_dev_is_dp_dual_mode(child, port))
> + list_for_each_entry(child, &dev_priv->vbt.child_devices, node) {
> + if (child_dev_is_dp_dual_mode(&child->config, port))
> return true;
> }
>
> @@ -2159,17 +2148,17 @@ bool intel_bios_is_port_dp_dual_mode(struct drm_i915_private *dev_priv,
> bool intel_bios_is_dsi_present(struct drm_i915_private *dev_priv,
> enum port *port)
> {
> - const struct child_device_config *child;
> + const struct child_device_config *config;
> + const struct child_device *child;
> u8 dvo_port;
> - int i;
>
> - for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
> - child = dev_priv->vbt.child_dev + i;
> + list_for_each_entry(child, &dev_priv->vbt.child_devices, node) {
> + config = &child->config;
>
> - if (!(child->device_type & DEVICE_TYPE_MIPI_OUTPUT))
> + if (!(config->device_type & DEVICE_TYPE_MIPI_OUTPUT))
> continue;
>
> - dvo_port = child->dvo_port;
> + dvo_port = config->dvo_port;
>
> if (dvo_port == DVO_PORT_MIPIA ||
> (dvo_port == DVO_PORT_MIPIB && INTEL_GEN(dev_priv) >= 11) ||
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 7e0f67babe20..ae85ffa08665 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -724,8 +724,7 @@ struct intel_vbt_data {
>
> int crt_ddc_pin;
>
> - int child_dev_num;
> - struct child_device_config *child_dev;
> + struct list_head child_devices;
>
> struct ddi_vbt_port_info ddi_port_info[I915_MAX_PORTS];
> struct sdvo_device_mapping sdvo_mappings[2];
> --
> 2.20.1
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915/bios: store child devices in a list
@ 2019-11-06 20:37 ` Ville Syrjälä
0 siblings, 0 replies; 12+ messages in thread
From: Ville Syrjälä @ 2019-11-06 20:37 UTC (permalink / raw)
To: Jani Nikula; +Cc: intel-gfx
On Wed, Nov 06, 2019 at 06:45:31PM +0200, Jani Nikula wrote:
> Using the array is getting clumsy. Make things a bit more dynamic.
>
> In code, start migrating towards calling the new struct child_device
> "child" and the VBT-originating struct child_device_config "config".
>
> Remove early returns on not having child devices when the end result
> after "iterating" the empty list would be the same.
>
> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>
> ---
>
> The end goal: allow more meta information to be added to the new
> child_device struct, independent of DDI port info being used or not on
> the platform, and eventually migrate ddi_port_info to it as well,
> unifying the stuff across platforms.
>
> Currently it's not easily possible to associate for example the DSC
> compression data to the child device for non-DDI platforms or for DSI
> outputs. This lets us add the compression data (pointer) to struct
> child_device.
> ---
> drivers/gpu/drm/i915/display/intel_bios.c | 203 ++++++++++------------
> drivers/gpu/drm/i915/i915_drv.h | 3 +-
> 2 files changed, 97 insertions(+), 109 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c
> index a03f56b7b4ef..025074862ab0 100644
> --- a/drivers/gpu/drm/i915/display/intel_bios.c
> +++ b/drivers/gpu/drm/i915/display/intel_bios.c
> @@ -58,6 +58,12 @@
> * that.
> */
>
> +/* Wrapper for VBT child device config */
> +struct child_device {
> + struct child_device_config config;
> + struct list_head node;
The wrapper is a bit unfortunate. I don't suppose we could just shove
the list head into the existing struct and adjust what needs adjusting?
> +};
> +
> #define SLAVE_ADDR1 0x70
> #define SLAVE_ADDR2 0x72
>
> @@ -449,8 +455,9 @@ static void
> parse_sdvo_device_mapping(struct drm_i915_private *dev_priv, u8 bdb_version)
> {
> struct sdvo_device_mapping *mapping;
> - const struct child_device_config *child;
> - int i, count = 0;
> + const struct child_device_config *config;
This thing could at least can live inside the loop. Though the rename is
also a bit unfortunate, leading to a needlessly large diff. Avoiding the
wrapper struct would also avoid that. I guess another option would
be to select a different name for the wrapper pointer here and keep the
original name for the actual thing.
> + const struct child_device *child;
> + int count = 0;
>
> /*
> * Only parse SDVO mappings on gens that could have SDVO. This isn't
> @@ -461,35 +468,35 @@ parse_sdvo_device_mapping(struct drm_i915_private *dev_priv, u8 bdb_version)
> return;
> }
>
> - for (i = 0, count = 0; i < dev_priv->vbt.child_dev_num; i++) {
> - child = dev_priv->vbt.child_dev + i;
> + list_for_each_entry(child, &dev_priv->vbt.child_devices, node) {
> + config = &child->config;
>
> - if (child->slave_addr != SLAVE_ADDR1 &&
> - child->slave_addr != SLAVE_ADDR2) {
> + if (config->slave_addr != SLAVE_ADDR1 &&
> + config->slave_addr != SLAVE_ADDR2) {
> /*
> * If the slave address is neither 0x70 nor 0x72,
> * it is not a SDVO device. Skip it.
> */
> continue;
> }
> - if (child->dvo_port != DEVICE_PORT_DVOB &&
> - child->dvo_port != DEVICE_PORT_DVOC) {
> + if (config->dvo_port != DEVICE_PORT_DVOB &&
> + config->dvo_port != DEVICE_PORT_DVOC) {
> /* skip the incorrect SDVO port */
> DRM_DEBUG_KMS("Incorrect SDVO port. Skip it\n");
> continue;
> }
> DRM_DEBUG_KMS("the SDVO device with slave addr %2x is found on"
> " %s port\n",
> - child->slave_addr,
> - (child->dvo_port == DEVICE_PORT_DVOB) ?
> + config->slave_addr,
> + (config->dvo_port == DEVICE_PORT_DVOB) ?
> "SDVOB" : "SDVOC");
> - mapping = &dev_priv->vbt.sdvo_mappings[child->dvo_port - 1];
> + mapping = &dev_priv->vbt.sdvo_mappings[config->dvo_port - 1];
> if (!mapping->initialized) {
> - mapping->dvo_port = child->dvo_port;
> - mapping->slave_addr = child->slave_addr;
> - mapping->dvo_wiring = child->dvo_wiring;
> - mapping->ddc_pin = child->ddc_pin;
> - mapping->i2c_pin = child->i2c_pin;
> + mapping->dvo_port = config->dvo_port;
> + mapping->slave_addr = config->slave_addr;
> + mapping->dvo_wiring = config->dvo_wiring;
> + mapping->ddc_pin = config->ddc_pin;
> + mapping->i2c_pin = config->i2c_pin;
> mapping->initialized = 1;
> DRM_DEBUG_KMS("SDVO device: dvo=%x, addr=%x, wiring=%d, ddc_pin=%d, i2c_pin=%d\n",
> mapping->dvo_port,
> @@ -501,7 +508,7 @@ parse_sdvo_device_mapping(struct drm_i915_private *dev_priv, u8 bdb_version)
> DRM_DEBUG_KMS("Maybe one SDVO port is shared by "
> "two SDVO device.\n");
> }
> - if (child->slave2_addr) {
> + if (config->slave2_addr) {
> /* Maybe this is a SDVO device with multiple inputs */
> /* And the mapping info is not added */
> DRM_DEBUG_KMS("there exists the slave2_addr. Maybe this"
> @@ -1571,8 +1578,7 @@ static void parse_ddi_port(struct drm_i915_private *dev_priv,
>
> static void parse_ddi_ports(struct drm_i915_private *dev_priv, u8 bdb_version)
> {
> - const struct child_device_config *child;
> - int i;
> + const struct child_device *child;
>
> if (!HAS_DDI(dev_priv) && !IS_CHERRYVIEW(dev_priv))
> return;
> @@ -1580,11 +1586,8 @@ static void parse_ddi_ports(struct drm_i915_private *dev_priv, u8 bdb_version)
> if (bdb_version < 155)
> return;
>
> - 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);
> - }
> + list_for_each_entry(child, &dev_priv->vbt.child_devices, node)
> + parse_ddi_port(dev_priv, &child->config, bdb_version);
> }
>
> static void
> @@ -1592,8 +1595,9 @@ parse_general_definitions(struct drm_i915_private *dev_priv,
> const struct bdb_header *bdb)
> {
> const struct bdb_general_definitions *defs;
> - const struct child_device_config *child;
> - int i, child_device_num, count;
> + const struct child_device_config *config;
> + struct child_device *child;
> + int i, child_device_num;
> u8 expected_size;
> u16 block_size;
> int bus_pin;
> @@ -1629,8 +1633,8 @@ parse_general_definitions(struct drm_i915_private *dev_priv,
> } else if (bdb->version <= 229) {
> expected_size = 39;
> } else {
> - expected_size = sizeof(*child);
> - BUILD_BUG_ON(sizeof(*child) < 39);
> + expected_size = sizeof(*config);
> + BUILD_BUG_ON(sizeof(*config) < 39);
> DRM_DEBUG_DRIVER("Expected child device config size for VBT version %u not known; assuming %u\n",
> bdb->version, expected_size);
> }
> @@ -1649,43 +1653,30 @@ parse_general_definitions(struct drm_i915_private *dev_priv,
>
> /* get the number of child device */
> child_device_num = (block_size - sizeof(*defs)) / defs->child_dev_size;
> - count = 0;
> - /* get the number of child device that is present */
> - for (i = 0; i < child_device_num; i++) {
> - child = child_device_ptr(defs, i);
> - if (!child->device_type)
> - continue;
> - count++;
> - }
> - if (!count) {
> - DRM_DEBUG_KMS("no child dev is parsed from VBT\n");
> - return;
> - }
> - dev_priv->vbt.child_dev = kcalloc(count, sizeof(*child), GFP_KERNEL);
> - if (!dev_priv->vbt.child_dev) {
> - DRM_DEBUG_KMS("No memory space for child device\n");
> - return;
> - }
>
> - dev_priv->vbt.child_dev_num = count;
> - count = 0;
> for (i = 0; i < child_device_num; i++) {
> - child = child_device_ptr(defs, i);
> - if (!child->device_type)
> + config = child_device_ptr(defs, i);
> + if (!config->device_type)
> continue;
>
> DRM_DEBUG_KMS("Found VBT child device with type 0x%x\n",
> - child->device_type);
> + config->device_type);
> +
> + child = kmalloc(sizeof(*child), GFP_KERNEL);
>
> /*
> * Copy as much as we know (sizeof) and is available
> - * (child_dev_size) of the child device. Accessing the data must
> - * depend on VBT version.
> + * (child_dev_size) of the child device config. Accessing the
> + * data must depend on VBT version.
> */
> - memcpy(dev_priv->vbt.child_dev + count, child,
> - min_t(size_t, defs->child_dev_size, sizeof(*child)));
> - count++;
> + memcpy(&child->config, config,
> + min_t(size_t, defs->child_dev_size, sizeof(*config)));
> +
> + list_add(&child->node, &dev_priv->vbt.child_devices);
> }
> +
> + if (list_empty(&dev_priv->vbt.child_devices))
> + DRM_DEBUG_KMS("no child dev is parsed from VBT\n");
> }
>
> /* Common defaults which may be overridden by VBT. */
> @@ -1849,6 +1840,8 @@ void intel_bios_init(struct drm_i915_private *dev_priv)
> return;
> }
>
> + INIT_LIST_HEAD(&dev_priv->vbt.child_devices);
> +
> init_vbt_defaults(dev_priv);
>
> /* If the OpRegion does not have VBT, look in PCI ROM. */
> @@ -1903,9 +1896,13 @@ void intel_bios_init(struct drm_i915_private *dev_priv)
> */
> void intel_bios_driver_remove(struct drm_i915_private *dev_priv)
> {
> - kfree(dev_priv->vbt.child_dev);
> - dev_priv->vbt.child_dev = NULL;
> - dev_priv->vbt.child_dev_num = 0;
> + struct child_device *child, *n;
> +
> + list_for_each_entry_safe(child, n, &dev_priv->vbt.child_devices, node) {
> + list_del(&child->node);
> + kfree(child);
> + }
> +
> kfree(dev_priv->vbt.sdvo_lvds_vbt_mode);
> dev_priv->vbt.sdvo_lvds_vbt_mode = NULL;
> kfree(dev_priv->vbt.lfp_lvds_vbt_mode);
> @@ -1929,21 +1926,22 @@ void intel_bios_driver_remove(struct drm_i915_private *dev_priv)
> */
> bool intel_bios_is_tv_present(struct drm_i915_private *dev_priv)
> {
> - const struct child_device_config *child;
> - int i;
> + const struct child_device_config *config;
> + const struct child_device *child;
>
> if (!dev_priv->vbt.int_tv_support)
> return false;
>
> - if (!dev_priv->vbt.child_dev_num)
> + if (list_empty(&dev_priv->vbt.child_devices))
> return true;
>
> - for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
> - child = dev_priv->vbt.child_dev + i;
> + list_for_each_entry(child, &dev_priv->vbt.child_devices, node) {
> + config = &child->config;
> +
> /*
> * If the device type is not TV, continue.
> */
> - switch (child->device_type) {
> + switch (config->device_type) {
> case DEVICE_TYPE_INT_TV:
> case DEVICE_TYPE_TV:
> case DEVICE_TYPE_TV_SVIDEO_COMPOSITE:
> @@ -1954,7 +1952,7 @@ bool intel_bios_is_tv_present(struct drm_i915_private *dev_priv)
> /* Only when the addin_offset is non-zero, it is regarded
> * as present.
> */
> - if (child->addin_offset)
> + if (config->addin_offset)
> return true;
> }
>
> @@ -1971,32 +1969,32 @@ bool intel_bios_is_tv_present(struct drm_i915_private *dev_priv)
> */
> bool intel_bios_is_lvds_present(struct drm_i915_private *dev_priv, u8 *i2c_pin)
> {
> - const struct child_device_config *child;
> - int i;
> + const struct child_device *child;
> + const struct child_device_config *config;
>
> - if (!dev_priv->vbt.child_dev_num)
> + if (list_empty(&dev_priv->vbt.child_devices))
> return true;
>
> - for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
> - child = dev_priv->vbt.child_dev + i;
> + list_for_each_entry(child, &dev_priv->vbt.child_devices, node) {
> + config = &child->config;
>
> /* If the device type is not LFP, continue.
> * We have to check both the new identifiers as well as the
> * old for compatibility with some BIOSes.
> */
> - if (child->device_type != DEVICE_TYPE_INT_LFP &&
> - child->device_type != DEVICE_TYPE_LFP)
> + if (config->device_type != DEVICE_TYPE_INT_LFP &&
> + config->device_type != DEVICE_TYPE_LFP)
> continue;
>
> - if (intel_gmbus_is_valid_pin(dev_priv, child->i2c_pin))
> - *i2c_pin = child->i2c_pin;
> + if (intel_gmbus_is_valid_pin(dev_priv, config->i2c_pin))
> + *i2c_pin = config->i2c_pin;
>
> /* However, we cannot trust the BIOS writers to populate
> * the VBT correctly. Since LVDS requires additional
> * information from AIM blocks, a non-zero addin offset is
> * a good indicator that the LVDS is actually present.
> */
> - if (child->addin_offset)
> + if (config->addin_offset)
> return true;
>
> /* But even then some BIOS writers perform some black magic
> @@ -2020,7 +2018,8 @@ bool intel_bios_is_lvds_present(struct drm_i915_private *dev_priv, u8 *i2c_pin)
> */
> bool intel_bios_is_port_present(struct drm_i915_private *dev_priv, enum port port)
> {
> - const struct child_device_config *child;
> + const struct child_device_config *config;
> + const struct child_device *child;
> static const struct {
> u16 dp, hdmi;
> } port_mapping[] = {
> @@ -2030,7 +2029,6 @@ bool intel_bios_is_port_present(struct drm_i915_private *dev_priv, enum port por
> [PORT_E] = { DVO_PORT_DPE, DVO_PORT_HDMIE, },
> [PORT_F] = { DVO_PORT_DPF, DVO_PORT_HDMIF, },
> };
> - int i;
>
> if (HAS_DDI(dev_priv)) {
> const struct ddi_vbt_port_info *port_info =
> @@ -2045,16 +2043,13 @@ bool intel_bios_is_port_present(struct drm_i915_private *dev_priv, enum port por
> if (WARN_ON(port == PORT_A) || port >= ARRAY_SIZE(port_mapping))
> return false;
>
> - if (!dev_priv->vbt.child_dev_num)
> - return false;
> -
> - for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
> - child = dev_priv->vbt.child_dev + i;
> + list_for_each_entry(child, &dev_priv->vbt.child_devices, node) {
> + config = &child->config;
>
> - if ((child->dvo_port == port_mapping[port].dp ||
> - child->dvo_port == port_mapping[port].hdmi) &&
> - (child->device_type & (DEVICE_TYPE_TMDS_DVI_SIGNALING |
> - DEVICE_TYPE_DISPLAYPORT_OUTPUT)))
> + if ((config->dvo_port == port_mapping[port].dp ||
> + config->dvo_port == port_mapping[port].hdmi) &&
> + (config->device_type & (DEVICE_TYPE_TMDS_DVI_SIGNALING |
> + DEVICE_TYPE_DISPLAYPORT_OUTPUT)))
> return true;
> }
>
> @@ -2070,7 +2065,8 @@ bool intel_bios_is_port_present(struct drm_i915_private *dev_priv, enum port por
> */
> bool intel_bios_is_port_edp(struct drm_i915_private *dev_priv, enum port port)
> {
> - const struct child_device_config *child;
> + const struct child_device_config *config;
> + const struct child_device *child;
> static const short port_mapping[] = {
> [PORT_B] = DVO_PORT_DPB,
> [PORT_C] = DVO_PORT_DPC,
> @@ -2078,19 +2074,15 @@ bool intel_bios_is_port_edp(struct drm_i915_private *dev_priv, enum port port)
> [PORT_E] = DVO_PORT_DPE,
> [PORT_F] = DVO_PORT_DPF,
> };
> - int i;
>
> if (HAS_DDI(dev_priv))
> return dev_priv->vbt.ddi_port_info[port].supports_edp;
>
> - if (!dev_priv->vbt.child_dev_num)
> - return false;
> + list_for_each_entry(child, &dev_priv->vbt.child_devices, node) {
> + config = &child->config;
>
> - for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
> - child = dev_priv->vbt.child_dev + i;
> -
> - if (child->dvo_port == port_mapping[port] &&
> - (child->device_type & DEVICE_TYPE_eDP_BITS) ==
> + if (config->dvo_port == port_mapping[port] &&
> + (config->device_type & DEVICE_TYPE_eDP_BITS) ==
> (DEVICE_TYPE_eDP & DEVICE_TYPE_eDP_BITS))
> return true;
> }
> @@ -2136,13 +2128,10 @@ static bool child_dev_is_dp_dual_mode(const struct child_device_config *child,
> bool intel_bios_is_port_dp_dual_mode(struct drm_i915_private *dev_priv,
> enum port port)
> {
> - const struct child_device_config *child;
> - int i;
> + const struct child_device *child;
>
> - for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
> - child = dev_priv->vbt.child_dev + i;
> -
> - if (child_dev_is_dp_dual_mode(child, port))
> + list_for_each_entry(child, &dev_priv->vbt.child_devices, node) {
> + if (child_dev_is_dp_dual_mode(&child->config, port))
> return true;
> }
>
> @@ -2159,17 +2148,17 @@ bool intel_bios_is_port_dp_dual_mode(struct drm_i915_private *dev_priv,
> bool intel_bios_is_dsi_present(struct drm_i915_private *dev_priv,
> enum port *port)
> {
> - const struct child_device_config *child;
> + const struct child_device_config *config;
> + const struct child_device *child;
> u8 dvo_port;
> - int i;
>
> - for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
> - child = dev_priv->vbt.child_dev + i;
> + list_for_each_entry(child, &dev_priv->vbt.child_devices, node) {
> + config = &child->config;
>
> - if (!(child->device_type & DEVICE_TYPE_MIPI_OUTPUT))
> + if (!(config->device_type & DEVICE_TYPE_MIPI_OUTPUT))
> continue;
>
> - dvo_port = child->dvo_port;
> + dvo_port = config->dvo_port;
>
> if (dvo_port == DVO_PORT_MIPIA ||
> (dvo_port == DVO_PORT_MIPIB && INTEL_GEN(dev_priv) >= 11) ||
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 7e0f67babe20..ae85ffa08665 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -724,8 +724,7 @@ struct intel_vbt_data {
>
> int crt_ddc_pin;
>
> - int child_dev_num;
> - struct child_device_config *child_dev;
> + struct list_head child_devices;
>
> struct ddi_vbt_port_info ddi_port_info[I915_MAX_PORTS];
> struct sdvo_device_mapping sdvo_mappings[2];
> --
> 2.20.1
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
* ✗ Fi.CI.BAT: failure for drm/i915/bios: store child devices in a list
@ 2019-11-06 20:42 ` Patchwork
0 siblings, 0 replies; 12+ messages in thread
From: Patchwork @ 2019-11-06 20:42 UTC (permalink / raw)
To: Jani Nikula; +Cc: intel-gfx
== Series Details ==
Series: drm/i915/bios: store child devices in a list
URL : https://patchwork.freedesktop.org/series/69077/
State : failure
== Summary ==
CI Bug Log - changes from CI_DRM_7274 -> Patchwork_15159
====================================================
Summary
-------
**FAILURE**
Serious unknown changes coming with Patchwork_15159 absolutely need to be
verified manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_15159, 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_15159/index.html
Possible new issues
-------------------
Here are the unknown changes that may have been introduced in Patchwork_15159:
### IGT changes ###
#### Possible regressions ####
* igt@i915_module_load@reload-no-display:
- fi-skl-6700k2: [PASS][1] -> [INCOMPLETE][2]
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7274/fi-skl-6700k2/igt@i915_module_load@reload-no-display.html
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15159/fi-skl-6700k2/igt@i915_module_load@reload-no-display.html
- fi-bwr-2160: [PASS][3] -> [INCOMPLETE][4]
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7274/fi-bwr-2160/igt@i915_module_load@reload-no-display.html
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15159/fi-bwr-2160/igt@i915_module_load@reload-no-display.html
- fi-kbl-r: [PASS][5] -> [INCOMPLETE][6]
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7274/fi-kbl-r/igt@i915_module_load@reload-no-display.html
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15159/fi-kbl-r/igt@i915_module_load@reload-no-display.html
- fi-blb-e6850: [PASS][7] -> [INCOMPLETE][8]
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7274/fi-blb-e6850/igt@i915_module_load@reload-no-display.html
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15159/fi-blb-e6850/igt@i915_module_load@reload-no-display.html
- fi-hsw-peppy: [PASS][9] -> [INCOMPLETE][10]
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7274/fi-hsw-peppy/igt@i915_module_load@reload-no-display.html
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15159/fi-hsw-peppy/igt@i915_module_load@reload-no-display.html
- fi-kbl-x1275: [PASS][11] -> [INCOMPLETE][12]
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7274/fi-kbl-x1275/igt@i915_module_load@reload-no-display.html
[12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15159/fi-kbl-x1275/igt@i915_module_load@reload-no-display.html
- fi-ilk-650: [PASS][13] -> [INCOMPLETE][14]
[13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7274/fi-ilk-650/igt@i915_module_load@reload-no-display.html
[14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15159/fi-ilk-650/igt@i915_module_load@reload-no-display.html
- fi-skl-6770hq: [PASS][15] -> [INCOMPLETE][16]
[15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7274/fi-skl-6770hq/igt@i915_module_load@reload-no-display.html
[16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15159/fi-skl-6770hq/igt@i915_module_load@reload-no-display.html
- fi-cfl-guc: [PASS][17] -> [INCOMPLETE][18]
[17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7274/fi-cfl-guc/igt@i915_module_load@reload-no-display.html
[18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15159/fi-cfl-guc/igt@i915_module_load@reload-no-display.html
- fi-kbl-soraka: [PASS][19] -> [INCOMPLETE][20]
[19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7274/fi-kbl-soraka/igt@i915_module_load@reload-no-display.html
[20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15159/fi-kbl-soraka/igt@i915_module_load@reload-no-display.html
- fi-skl-lmem: [PASS][21] -> [INCOMPLETE][22]
[21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7274/fi-skl-lmem/igt@i915_module_load@reload-no-display.html
[22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15159/fi-skl-lmem/igt@i915_module_load@reload-no-display.html
- fi-ivb-3770: [PASS][23] -> [INCOMPLETE][24]
[23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7274/fi-ivb-3770/igt@i915_module_load@reload-no-display.html
[24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15159/fi-ivb-3770/igt@i915_module_load@reload-no-display.html
- fi-snb-2520m: [PASS][25] -> [INCOMPLETE][26]
[25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7274/fi-snb-2520m/igt@i915_module_load@reload-no-display.html
[26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15159/fi-snb-2520m/igt@i915_module_load@reload-no-display.html
- fi-cfl-8700k: [PASS][27] -> [INCOMPLETE][28]
[27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7274/fi-cfl-8700k/igt@i915_module_load@reload-no-display.html
[28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15159/fi-cfl-8700k/igt@i915_module_load@reload-no-display.html
- fi-kbl-7500u: [PASS][29] -> [INCOMPLETE][30]
[29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7274/fi-kbl-7500u/igt@i915_module_load@reload-no-display.html
[30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15159/fi-kbl-7500u/igt@i915_module_load@reload-no-display.html
- fi-hsw-4770: [PASS][31] -> [INCOMPLETE][32]
[31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7274/fi-hsw-4770/igt@i915_module_load@reload-no-display.html
[32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15159/fi-hsw-4770/igt@i915_module_load@reload-no-display.html
- fi-kbl-guc: [PASS][33] -> [INCOMPLETE][34]
[33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7274/fi-kbl-guc/igt@i915_module_load@reload-no-display.html
[34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15159/fi-kbl-guc/igt@i915_module_load@reload-no-display.html
- fi-skl-guc: [PASS][35] -> [INCOMPLETE][36]
[35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7274/fi-skl-guc/igt@i915_module_load@reload-no-display.html
[36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15159/fi-skl-guc/igt@i915_module_load@reload-no-display.html
- fi-bdw-5557u: [PASS][37] -> [INCOMPLETE][38]
[37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7274/fi-bdw-5557u/igt@i915_module_load@reload-no-display.html
[38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15159/fi-bdw-5557u/igt@i915_module_load@reload-no-display.html
- fi-hsw-4770r: [PASS][39] -> [INCOMPLETE][40]
[39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7274/fi-hsw-4770r/igt@i915_module_load@reload-no-display.html
[40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15159/fi-hsw-4770r/igt@i915_module_load@reload-no-display.html
* igt@runner@aborted:
- fi-ilk-650: NOTRUN -> [FAIL][41]
[41]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15159/fi-ilk-650/igt@runner@aborted.html
- fi-pnv-d510: NOTRUN -> [FAIL][42]
[42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15159/fi-pnv-d510/igt@runner@aborted.html
- fi-gdg-551: NOTRUN -> [FAIL][43]
[43]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15159/fi-gdg-551/igt@runner@aborted.html
- fi-kbl-soraka: NOTRUN -> [FAIL][44]
[44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15159/fi-kbl-soraka/igt@runner@aborted.html
- fi-kbl-7500u: NOTRUN -> [FAIL][45]
[45]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15159/fi-kbl-7500u/igt@runner@aborted.html
- fi-whl-u: NOTRUN -> [FAIL][46]
[46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15159/fi-whl-u/igt@runner@aborted.html
- fi-bxt-dsi: NOTRUN -> [FAIL][47]
[47]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15159/fi-bxt-dsi/igt@runner@aborted.html
- fi-cfl-guc: NOTRUN -> [FAIL][48]
[48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15159/fi-cfl-guc/igt@runner@aborted.html
- fi-bsw-n3050: NOTRUN -> [FAIL][49]
[49]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15159/fi-bsw-n3050/igt@runner@aborted.html
- fi-blb-e6850: NOTRUN -> [FAIL][50]
[50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15159/fi-blb-e6850/igt@runner@aborted.html
- fi-kbl-x1275: NOTRUN -> [FAIL][51]
[51]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15159/fi-kbl-x1275/igt@runner@aborted.html
- fi-bsw-kefka: NOTRUN -> [FAIL][52]
[52]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15159/fi-bsw-kefka/igt@runner@aborted.html
- fi-cfl-8700k: NOTRUN -> [FAIL][53]
[53]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15159/fi-cfl-8700k/igt@runner@aborted.html
- fi-bsw-nick: NOTRUN -> [FAIL][54]
[54]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15159/fi-bsw-nick/igt@runner@aborted.html
- fi-kbl-8809g: NOTRUN -> [FAIL][55]
[55]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15159/fi-kbl-8809g/igt@runner@aborted.html
- fi-apl-guc: NOTRUN -> [FAIL][56]
[56]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15159/fi-apl-guc/igt@runner@aborted.html
- fi-kbl-r: NOTRUN -> [FAIL][57]
[57]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15159/fi-kbl-r/igt@runner@aborted.html
- fi-kbl-guc: NOTRUN -> [FAIL][58]
[58]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15159/fi-kbl-guc/igt@runner@aborted.html
- fi-elk-e7500: NOTRUN -> [FAIL][59]
[59]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15159/fi-elk-e7500/igt@runner@aborted.html
Known issues
------------
Here are the changes found in Patchwork_15159 that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@i915_module_load@reload-no-display:
- fi-bsw-kefka: [PASS][60] -> [INCOMPLETE][61] ([fdo#105876])
[60]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7274/fi-bsw-kefka/igt@i915_module_load@reload-no-display.html
[61]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15159/fi-bsw-kefka/igt@i915_module_load@reload-no-display.html
- fi-elk-e7500: [PASS][62] -> [INCOMPLETE][63] ([fdo#103989])
[62]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7274/fi-elk-e7500/igt@i915_module_load@reload-no-display.html
[63]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15159/fi-elk-e7500/igt@i915_module_load@reload-no-display.html
- fi-apl-guc: [PASS][64] -> [INCOMPLETE][65] ([fdo#103927])
[64]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7274/fi-apl-guc/igt@i915_module_load@reload-no-display.html
[65]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15159/fi-apl-guc/igt@i915_module_load@reload-no-display.html
- fi-byt-j1900: [PASS][66] -> [INCOMPLETE][67] ([fdo#102657])
[66]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7274/fi-byt-j1900/igt@i915_module_load@reload-no-display.html
[67]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15159/fi-byt-j1900/igt@i915_module_load@reload-no-display.html
- fi-skl-6600u: [PASS][68] -> [INCOMPLETE][69] ([fdo#104108])
[68]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7274/fi-skl-6600u/igt@i915_module_load@reload-no-display.html
[69]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15159/fi-skl-6600u/igt@i915_module_load@reload-no-display.html
- fi-pnv-d510: [PASS][70] -> [INCOMPLETE][71] ([fdo#110740])
[70]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7274/fi-pnv-d510/igt@i915_module_load@reload-no-display.html
[71]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15159/fi-pnv-d510/igt@i915_module_load@reload-no-display.html
- fi-whl-u: [PASS][72] -> [INCOMPLETE][73] ([fdo#111338])
[72]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7274/fi-whl-u/igt@i915_module_load@reload-no-display.html
[73]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15159/fi-whl-u/igt@i915_module_load@reload-no-display.html
- fi-kbl-8809g: [PASS][74] -> [INCOMPLETE][75] ([fdo#103665])
[74]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7274/fi-kbl-8809g/igt@i915_module_load@reload-no-display.html
[75]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15159/fi-kbl-8809g/igt@i915_module_load@reload-no-display.html
- fi-bsw-nick: [PASS][76] -> [INCOMPLETE][77] ([fdo#105876])
[76]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7274/fi-bsw-nick/igt@i915_module_load@reload-no-display.html
[77]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15159/fi-bsw-nick/igt@i915_module_load@reload-no-display.html
- fi-gdg-551: [PASS][78] -> [INCOMPLETE][79] ([fdo#108316])
[78]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7274/fi-gdg-551/igt@i915_module_load@reload-no-display.html
[79]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15159/fi-gdg-551/igt@i915_module_load@reload-no-display.html
- fi-icl-u3: [PASS][80] -> [INCOMPLETE][81] ([fdo#107713])
[80]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7274/fi-icl-u3/igt@i915_module_load@reload-no-display.html
[81]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15159/fi-icl-u3/igt@i915_module_load@reload-no-display.html
- fi-bsw-n3050: [PASS][82] -> [INCOMPLETE][83] ([fdo#105876])
[82]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7274/fi-bsw-n3050/igt@i915_module_load@reload-no-display.html
[83]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15159/fi-bsw-n3050/igt@i915_module_load@reload-no-display.html
- fi-byt-n2820: [PASS][84] -> [INCOMPLETE][85] ([fdo#102657])
[84]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7274/fi-byt-n2820/igt@i915_module_load@reload-no-display.html
[85]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15159/fi-byt-n2820/igt@i915_module_load@reload-no-display.html
- fi-bxt-dsi: [PASS][86] -> [INCOMPLETE][87] ([fdo#103927])
[86]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7274/fi-bxt-dsi/igt@i915_module_load@reload-no-display.html
[87]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15159/fi-bxt-dsi/igt@i915_module_load@reload-no-display.html
- fi-cml-u2: [PASS][88] -> [INCOMPLETE][89] ([fdo#110566])
[88]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7274/fi-cml-u2/igt@i915_module_load@reload-no-display.html
[89]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15159/fi-cml-u2/igt@i915_module_load@reload-no-display.html
- fi-icl-u2: [PASS][90] -> [INCOMPLETE][91] ([fdo#107713])
[90]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7274/fi-icl-u2/igt@i915_module_load@reload-no-display.html
[91]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15159/fi-icl-u2/igt@i915_module_load@reload-no-display.html
- fi-glk-dsi: [PASS][92] -> [INCOMPLETE][93] ([fdo#103359] / [k.org#198133])
[92]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7274/fi-glk-dsi/igt@i915_module_load@reload-no-display.html
[93]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15159/fi-glk-dsi/igt@i915_module_load@reload-no-display.html
#### Possible fixes ####
* igt@gem_exec_suspend@basic:
- {fi-icl-guc}: [FAIL][94] ([fdo#111699]) -> [PASS][95]
[94]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7274/fi-icl-guc/igt@gem_exec_suspend@basic.html
[95]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15159/fi-icl-guc/igt@gem_exec_suspend@basic.html
* igt@kms_chamelium@hdmi-hpd-fast:
- fi-kbl-7500u: [FAIL][96] ([fdo#111045] / [fdo#111096]) -> [PASS][97]
[96]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7274/fi-kbl-7500u/igt@kms_chamelium@hdmi-hpd-fast.html
[97]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15159/fi-kbl-7500u/igt@kms_chamelium@hdmi-hpd-fast.html
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[fdo#102657]: https://bugs.freedesktop.org/show_bug.cgi?id=102657
[fdo#103359]: https://bugs.freedesktop.org/show_bug.cgi?id=103359
[fdo#103665]: https://bugs.freedesktop.org/show_bug.cgi?id=103665
[fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
[fdo#103989]: https://bugs.freedesktop.org/show_bug.cgi?id=103989
[fdo#104108]: https://bugs.freedesktop.org/show_bug.cgi?id=104108
[fdo#105876]: https://bugs.freedesktop.org/show_bug.cgi?id=105876
[fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
[fdo#108316]: https://bugs.freedesktop.org/show_bug.cgi?id=108316
[fdo#110566]: https://bugs.freedesktop.org/show_bug.cgi?id=110566
[fdo#110740]: https://bugs.freedesktop.org/show_bug.cgi?id=110740
[fdo#111045]: https://bugs.freedesktop.org/show_bug.cgi?id=111045
[fdo#111096]: https://bugs.freedesktop.org/show_bug.cgi?id=111096
[fdo#111338]: https://bugs.freedesktop.org/show_bug.cgi?id=111338
[fdo#111699]: https://bugs.freedesktop.org/show_bug.cgi?id=111699
[k.org#198133]: https://bugzilla.kernel.org/show_bug.cgi?id=198133
Participating hosts (51 -> 44)
------------------------------
Missing (7): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-ctg-p8600 fi-byt-clapper fi-bdw-samus fi-snb-2600
Build changes
-------------
* CI: CI-20190529 -> None
* Linux: CI_DRM_7274 -> Patchwork_15159
CI-20190529: 20190529
CI_DRM_7274: bd7bf4f7bb4caede96cd1c086cc6760a584bd721 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_5264: f21213012393bd8041ad93084ce29da088ef8426 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_15159: b08607b1496177ff2a1f0fb05cc717fa5a987816 @ git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
b08607b14961 drm/i915/bios: store child devices in a list
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15159/index.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915/bios: store child devices in a list
@ 2019-11-06 20:42 ` Patchwork
0 siblings, 0 replies; 12+ messages in thread
From: Patchwork @ 2019-11-06 20:42 UTC (permalink / raw)
To: Jani Nikula; +Cc: intel-gfx
== Series Details ==
Series: drm/i915/bios: store child devices in a list
URL : https://patchwork.freedesktop.org/series/69077/
State : failure
== Summary ==
CI Bug Log - changes from CI_DRM_7274 -> Patchwork_15159
====================================================
Summary
-------
**FAILURE**
Serious unknown changes coming with Patchwork_15159 absolutely need to be
verified manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_15159, 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_15159/index.html
Possible new issues
-------------------
Here are the unknown changes that may have been introduced in Patchwork_15159:
### IGT changes ###
#### Possible regressions ####
* igt@i915_module_load@reload-no-display:
- fi-skl-6700k2: [PASS][1] -> [INCOMPLETE][2]
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7274/fi-skl-6700k2/igt@i915_module_load@reload-no-display.html
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15159/fi-skl-6700k2/igt@i915_module_load@reload-no-display.html
- fi-bwr-2160: [PASS][3] -> [INCOMPLETE][4]
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7274/fi-bwr-2160/igt@i915_module_load@reload-no-display.html
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15159/fi-bwr-2160/igt@i915_module_load@reload-no-display.html
- fi-kbl-r: [PASS][5] -> [INCOMPLETE][6]
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7274/fi-kbl-r/igt@i915_module_load@reload-no-display.html
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15159/fi-kbl-r/igt@i915_module_load@reload-no-display.html
- fi-blb-e6850: [PASS][7] -> [INCOMPLETE][8]
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7274/fi-blb-e6850/igt@i915_module_load@reload-no-display.html
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15159/fi-blb-e6850/igt@i915_module_load@reload-no-display.html
- fi-hsw-peppy: [PASS][9] -> [INCOMPLETE][10]
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7274/fi-hsw-peppy/igt@i915_module_load@reload-no-display.html
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15159/fi-hsw-peppy/igt@i915_module_load@reload-no-display.html
- fi-kbl-x1275: [PASS][11] -> [INCOMPLETE][12]
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7274/fi-kbl-x1275/igt@i915_module_load@reload-no-display.html
[12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15159/fi-kbl-x1275/igt@i915_module_load@reload-no-display.html
- fi-ilk-650: [PASS][13] -> [INCOMPLETE][14]
[13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7274/fi-ilk-650/igt@i915_module_load@reload-no-display.html
[14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15159/fi-ilk-650/igt@i915_module_load@reload-no-display.html
- fi-skl-6770hq: [PASS][15] -> [INCOMPLETE][16]
[15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7274/fi-skl-6770hq/igt@i915_module_load@reload-no-display.html
[16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15159/fi-skl-6770hq/igt@i915_module_load@reload-no-display.html
- fi-cfl-guc: [PASS][17] -> [INCOMPLETE][18]
[17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7274/fi-cfl-guc/igt@i915_module_load@reload-no-display.html
[18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15159/fi-cfl-guc/igt@i915_module_load@reload-no-display.html
- fi-kbl-soraka: [PASS][19] -> [INCOMPLETE][20]
[19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7274/fi-kbl-soraka/igt@i915_module_load@reload-no-display.html
[20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15159/fi-kbl-soraka/igt@i915_module_load@reload-no-display.html
- fi-skl-lmem: [PASS][21] -> [INCOMPLETE][22]
[21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7274/fi-skl-lmem/igt@i915_module_load@reload-no-display.html
[22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15159/fi-skl-lmem/igt@i915_module_load@reload-no-display.html
- fi-ivb-3770: [PASS][23] -> [INCOMPLETE][24]
[23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7274/fi-ivb-3770/igt@i915_module_load@reload-no-display.html
[24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15159/fi-ivb-3770/igt@i915_module_load@reload-no-display.html
- fi-snb-2520m: [PASS][25] -> [INCOMPLETE][26]
[25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7274/fi-snb-2520m/igt@i915_module_load@reload-no-display.html
[26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15159/fi-snb-2520m/igt@i915_module_load@reload-no-display.html
- fi-cfl-8700k: [PASS][27] -> [INCOMPLETE][28]
[27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7274/fi-cfl-8700k/igt@i915_module_load@reload-no-display.html
[28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15159/fi-cfl-8700k/igt@i915_module_load@reload-no-display.html
- fi-kbl-7500u: [PASS][29] -> [INCOMPLETE][30]
[29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7274/fi-kbl-7500u/igt@i915_module_load@reload-no-display.html
[30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15159/fi-kbl-7500u/igt@i915_module_load@reload-no-display.html
- fi-hsw-4770: [PASS][31] -> [INCOMPLETE][32]
[31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7274/fi-hsw-4770/igt@i915_module_load@reload-no-display.html
[32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15159/fi-hsw-4770/igt@i915_module_load@reload-no-display.html
- fi-kbl-guc: [PASS][33] -> [INCOMPLETE][34]
[33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7274/fi-kbl-guc/igt@i915_module_load@reload-no-display.html
[34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15159/fi-kbl-guc/igt@i915_module_load@reload-no-display.html
- fi-skl-guc: [PASS][35] -> [INCOMPLETE][36]
[35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7274/fi-skl-guc/igt@i915_module_load@reload-no-display.html
[36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15159/fi-skl-guc/igt@i915_module_load@reload-no-display.html
- fi-bdw-5557u: [PASS][37] -> [INCOMPLETE][38]
[37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7274/fi-bdw-5557u/igt@i915_module_load@reload-no-display.html
[38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15159/fi-bdw-5557u/igt@i915_module_load@reload-no-display.html
- fi-hsw-4770r: [PASS][39] -> [INCOMPLETE][40]
[39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7274/fi-hsw-4770r/igt@i915_module_load@reload-no-display.html
[40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15159/fi-hsw-4770r/igt@i915_module_load@reload-no-display.html
* igt@runner@aborted:
- fi-ilk-650: NOTRUN -> [FAIL][41]
[41]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15159/fi-ilk-650/igt@runner@aborted.html
- fi-pnv-d510: NOTRUN -> [FAIL][42]
[42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15159/fi-pnv-d510/igt@runner@aborted.html
- fi-gdg-551: NOTRUN -> [FAIL][43]
[43]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15159/fi-gdg-551/igt@runner@aborted.html
- fi-kbl-soraka: NOTRUN -> [FAIL][44]
[44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15159/fi-kbl-soraka/igt@runner@aborted.html
- fi-kbl-7500u: NOTRUN -> [FAIL][45]
[45]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15159/fi-kbl-7500u/igt@runner@aborted.html
- fi-whl-u: NOTRUN -> [FAIL][46]
[46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15159/fi-whl-u/igt@runner@aborted.html
- fi-bxt-dsi: NOTRUN -> [FAIL][47]
[47]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15159/fi-bxt-dsi/igt@runner@aborted.html
- fi-cfl-guc: NOTRUN -> [FAIL][48]
[48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15159/fi-cfl-guc/igt@runner@aborted.html
- fi-bsw-n3050: NOTRUN -> [FAIL][49]
[49]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15159/fi-bsw-n3050/igt@runner@aborted.html
- fi-blb-e6850: NOTRUN -> [FAIL][50]
[50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15159/fi-blb-e6850/igt@runner@aborted.html
- fi-kbl-x1275: NOTRUN -> [FAIL][51]
[51]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15159/fi-kbl-x1275/igt@runner@aborted.html
- fi-bsw-kefka: NOTRUN -> [FAIL][52]
[52]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15159/fi-bsw-kefka/igt@runner@aborted.html
- fi-cfl-8700k: NOTRUN -> [FAIL][53]
[53]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15159/fi-cfl-8700k/igt@runner@aborted.html
- fi-bsw-nick: NOTRUN -> [FAIL][54]
[54]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15159/fi-bsw-nick/igt@runner@aborted.html
- fi-kbl-8809g: NOTRUN -> [FAIL][55]
[55]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15159/fi-kbl-8809g/igt@runner@aborted.html
- fi-apl-guc: NOTRUN -> [FAIL][56]
[56]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15159/fi-apl-guc/igt@runner@aborted.html
- fi-kbl-r: NOTRUN -> [FAIL][57]
[57]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15159/fi-kbl-r/igt@runner@aborted.html
- fi-kbl-guc: NOTRUN -> [FAIL][58]
[58]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15159/fi-kbl-guc/igt@runner@aborted.html
- fi-elk-e7500: NOTRUN -> [FAIL][59]
[59]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15159/fi-elk-e7500/igt@runner@aborted.html
Known issues
------------
Here are the changes found in Patchwork_15159 that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@i915_module_load@reload-no-display:
- fi-bsw-kefka: [PASS][60] -> [INCOMPLETE][61] ([fdo#105876])
[60]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7274/fi-bsw-kefka/igt@i915_module_load@reload-no-display.html
[61]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15159/fi-bsw-kefka/igt@i915_module_load@reload-no-display.html
- fi-elk-e7500: [PASS][62] -> [INCOMPLETE][63] ([fdo#103989])
[62]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7274/fi-elk-e7500/igt@i915_module_load@reload-no-display.html
[63]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15159/fi-elk-e7500/igt@i915_module_load@reload-no-display.html
- fi-apl-guc: [PASS][64] -> [INCOMPLETE][65] ([fdo#103927])
[64]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7274/fi-apl-guc/igt@i915_module_load@reload-no-display.html
[65]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15159/fi-apl-guc/igt@i915_module_load@reload-no-display.html
- fi-byt-j1900: [PASS][66] -> [INCOMPLETE][67] ([fdo#102657])
[66]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7274/fi-byt-j1900/igt@i915_module_load@reload-no-display.html
[67]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15159/fi-byt-j1900/igt@i915_module_load@reload-no-display.html
- fi-skl-6600u: [PASS][68] -> [INCOMPLETE][69] ([fdo#104108])
[68]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7274/fi-skl-6600u/igt@i915_module_load@reload-no-display.html
[69]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15159/fi-skl-6600u/igt@i915_module_load@reload-no-display.html
- fi-pnv-d510: [PASS][70] -> [INCOMPLETE][71] ([fdo#110740])
[70]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7274/fi-pnv-d510/igt@i915_module_load@reload-no-display.html
[71]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15159/fi-pnv-d510/igt@i915_module_load@reload-no-display.html
- fi-whl-u: [PASS][72] -> [INCOMPLETE][73] ([fdo#111338])
[72]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7274/fi-whl-u/igt@i915_module_load@reload-no-display.html
[73]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15159/fi-whl-u/igt@i915_module_load@reload-no-display.html
- fi-kbl-8809g: [PASS][74] -> [INCOMPLETE][75] ([fdo#103665])
[74]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7274/fi-kbl-8809g/igt@i915_module_load@reload-no-display.html
[75]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15159/fi-kbl-8809g/igt@i915_module_load@reload-no-display.html
- fi-bsw-nick: [PASS][76] -> [INCOMPLETE][77] ([fdo#105876])
[76]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7274/fi-bsw-nick/igt@i915_module_load@reload-no-display.html
[77]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15159/fi-bsw-nick/igt@i915_module_load@reload-no-display.html
- fi-gdg-551: [PASS][78] -> [INCOMPLETE][79] ([fdo#108316])
[78]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7274/fi-gdg-551/igt@i915_module_load@reload-no-display.html
[79]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15159/fi-gdg-551/igt@i915_module_load@reload-no-display.html
- fi-icl-u3: [PASS][80] -> [INCOMPLETE][81] ([fdo#107713])
[80]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7274/fi-icl-u3/igt@i915_module_load@reload-no-display.html
[81]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15159/fi-icl-u3/igt@i915_module_load@reload-no-display.html
- fi-bsw-n3050: [PASS][82] -> [INCOMPLETE][83] ([fdo#105876])
[82]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7274/fi-bsw-n3050/igt@i915_module_load@reload-no-display.html
[83]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15159/fi-bsw-n3050/igt@i915_module_load@reload-no-display.html
- fi-byt-n2820: [PASS][84] -> [INCOMPLETE][85] ([fdo#102657])
[84]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7274/fi-byt-n2820/igt@i915_module_load@reload-no-display.html
[85]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15159/fi-byt-n2820/igt@i915_module_load@reload-no-display.html
- fi-bxt-dsi: [PASS][86] -> [INCOMPLETE][87] ([fdo#103927])
[86]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7274/fi-bxt-dsi/igt@i915_module_load@reload-no-display.html
[87]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15159/fi-bxt-dsi/igt@i915_module_load@reload-no-display.html
- fi-cml-u2: [PASS][88] -> [INCOMPLETE][89] ([fdo#110566])
[88]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7274/fi-cml-u2/igt@i915_module_load@reload-no-display.html
[89]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15159/fi-cml-u2/igt@i915_module_load@reload-no-display.html
- fi-icl-u2: [PASS][90] -> [INCOMPLETE][91] ([fdo#107713])
[90]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7274/fi-icl-u2/igt@i915_module_load@reload-no-display.html
[91]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15159/fi-icl-u2/igt@i915_module_load@reload-no-display.html
- fi-glk-dsi: [PASS][92] -> [INCOMPLETE][93] ([fdo#103359] / [k.org#198133])
[92]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7274/fi-glk-dsi/igt@i915_module_load@reload-no-display.html
[93]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15159/fi-glk-dsi/igt@i915_module_load@reload-no-display.html
#### Possible fixes ####
* igt@gem_exec_suspend@basic:
- {fi-icl-guc}: [FAIL][94] ([fdo#111699]) -> [PASS][95]
[94]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7274/fi-icl-guc/igt@gem_exec_suspend@basic.html
[95]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15159/fi-icl-guc/igt@gem_exec_suspend@basic.html
* igt@kms_chamelium@hdmi-hpd-fast:
- fi-kbl-7500u: [FAIL][96] ([fdo#111045] / [fdo#111096]) -> [PASS][97]
[96]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7274/fi-kbl-7500u/igt@kms_chamelium@hdmi-hpd-fast.html
[97]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15159/fi-kbl-7500u/igt@kms_chamelium@hdmi-hpd-fast.html
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[fdo#102657]: https://bugs.freedesktop.org/show_bug.cgi?id=102657
[fdo#103359]: https://bugs.freedesktop.org/show_bug.cgi?id=103359
[fdo#103665]: https://bugs.freedesktop.org/show_bug.cgi?id=103665
[fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
[fdo#103989]: https://bugs.freedesktop.org/show_bug.cgi?id=103989
[fdo#104108]: https://bugs.freedesktop.org/show_bug.cgi?id=104108
[fdo#105876]: https://bugs.freedesktop.org/show_bug.cgi?id=105876
[fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
[fdo#108316]: https://bugs.freedesktop.org/show_bug.cgi?id=108316
[fdo#110566]: https://bugs.freedesktop.org/show_bug.cgi?id=110566
[fdo#110740]: https://bugs.freedesktop.org/show_bug.cgi?id=110740
[fdo#111045]: https://bugs.freedesktop.org/show_bug.cgi?id=111045
[fdo#111096]: https://bugs.freedesktop.org/show_bug.cgi?id=111096
[fdo#111338]: https://bugs.freedesktop.org/show_bug.cgi?id=111338
[fdo#111699]: https://bugs.freedesktop.org/show_bug.cgi?id=111699
[k.org#198133]: https://bugzilla.kernel.org/show_bug.cgi?id=198133
Participating hosts (51 -> 44)
------------------------------
Missing (7): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-ctg-p8600 fi-byt-clapper fi-bdw-samus fi-snb-2600
Build changes
-------------
* CI: CI-20190529 -> None
* Linux: CI_DRM_7274 -> Patchwork_15159
CI-20190529: 20190529
CI_DRM_7274: bd7bf4f7bb4caede96cd1c086cc6760a584bd721 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_5264: f21213012393bd8041ad93084ce29da088ef8426 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_15159: b08607b1496177ff2a1f0fb05cc717fa5a987816 @ git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
b08607b14961 drm/i915/bios: store child devices in a list
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15159/index.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] drm/i915/bios: store child devices in a list
@ 2019-11-07 8:22 ` Jani Nikula
0 siblings, 0 replies; 12+ messages in thread
From: Jani Nikula @ 2019-11-07 8:22 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: intel-gfx
On Wed, 06 Nov 2019, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> On Wed, Nov 06, 2019 at 06:45:31PM +0200, Jani Nikula wrote:
>> Using the array is getting clumsy. Make things a bit more dynamic.
>>
>> In code, start migrating towards calling the new struct child_device
>> "child" and the VBT-originating struct child_device_config "config".
>>
>> Remove early returns on not having child devices when the end result
>> after "iterating" the empty list would be the same.
>>
>> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>>
>> ---
>>
>> The end goal: allow more meta information to be added to the new
>> child_device struct, independent of DDI port info being used or not on
>> the platform, and eventually migrate ddi_port_info to it as well,
>> unifying the stuff across platforms.
>>
>> Currently it's not easily possible to associate for example the DSC
>> compression data to the child device for non-DDI platforms or for DSI
>> outputs. This lets us add the compression data (pointer) to struct
>> child_device.
>> ---
>> drivers/gpu/drm/i915/display/intel_bios.c | 203 ++++++++++------------
>> drivers/gpu/drm/i915/i915_drv.h | 3 +-
>> 2 files changed, 97 insertions(+), 109 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c
>> index a03f56b7b4ef..025074862ab0 100644
>> --- a/drivers/gpu/drm/i915/display/intel_bios.c
>> +++ b/drivers/gpu/drm/i915/display/intel_bios.c
>> @@ -58,6 +58,12 @@
>> * that.
>> */
>>
>> +/* Wrapper for VBT child device config */
>> +struct child_device {
>> + struct child_device_config config;
>> + struct list_head node;
>
> The wrapper is a bit unfortunate. I don't suppose we could just shove
> the list head into the existing struct and adjust what needs adjusting?
The existing struct is used for serialization and the size is checked
against what's in vbt etc. I might also add stuff in the wrapper struct,
at least intermediately, so it's kind of useful. I don't really think
the wrapper is all that bad.
>
>> +};
>> +
>> #define SLAVE_ADDR1 0x70
>> #define SLAVE_ADDR2 0x72
>>
>> @@ -449,8 +455,9 @@ static void
>> parse_sdvo_device_mapping(struct drm_i915_private *dev_priv, u8 bdb_version)
>> {
>> struct sdvo_device_mapping *mapping;
>> - const struct child_device_config *child;
>> - int i, count = 0;
>> + const struct child_device_config *config;
>
> This thing could at least can live inside the loop. Though the rename is
> also a bit unfortunate, leading to a needlessly large diff. Avoiding the
> wrapper struct would also avoid that. I guess another option would
> be to select a different name for the wrapper pointer here and keep the
> original name for the actual thing.
The main problem with avoiding the rename is to come up with a better
name for the wrapper structure. :) Child and config seemed apt, but I do
understand the downsides. I'd just like to have names that we can use
througout. Maybe we can stick to child for struct child_device_config,
but what do we call the whole wrapper struct and local vars?
Suggestions?
BR,
Jani.
>
>> + const struct child_device *child;
>> + int count = 0;
>>
>> /*
>> * Only parse SDVO mappings on gens that could have SDVO. This isn't
>> @@ -461,35 +468,35 @@ parse_sdvo_device_mapping(struct drm_i915_private *dev_priv, u8 bdb_version)
>> return;
>> }
>>
>> - for (i = 0, count = 0; i < dev_priv->vbt.child_dev_num; i++) {
>> - child = dev_priv->vbt.child_dev + i;
>> + list_for_each_entry(child, &dev_priv->vbt.child_devices, node) {
>> + config = &child->config;
>>
>> - if (child->slave_addr != SLAVE_ADDR1 &&
>> - child->slave_addr != SLAVE_ADDR2) {
>> + if (config->slave_addr != SLAVE_ADDR1 &&
>> + config->slave_addr != SLAVE_ADDR2) {
>> /*
>> * If the slave address is neither 0x70 nor 0x72,
>> * it is not a SDVO device. Skip it.
>> */
>> continue;
>> }
>> - if (child->dvo_port != DEVICE_PORT_DVOB &&
>> - child->dvo_port != DEVICE_PORT_DVOC) {
>> + if (config->dvo_port != DEVICE_PORT_DVOB &&
>> + config->dvo_port != DEVICE_PORT_DVOC) {
>> /* skip the incorrect SDVO port */
>> DRM_DEBUG_KMS("Incorrect SDVO port. Skip it\n");
>> continue;
>> }
>> DRM_DEBUG_KMS("the SDVO device with slave addr %2x is found on"
>> " %s port\n",
>> - child->slave_addr,
>> - (child->dvo_port == DEVICE_PORT_DVOB) ?
>> + config->slave_addr,
>> + (config->dvo_port == DEVICE_PORT_DVOB) ?
>> "SDVOB" : "SDVOC");
>> - mapping = &dev_priv->vbt.sdvo_mappings[child->dvo_port - 1];
>> + mapping = &dev_priv->vbt.sdvo_mappings[config->dvo_port - 1];
>> if (!mapping->initialized) {
>> - mapping->dvo_port = child->dvo_port;
>> - mapping->slave_addr = child->slave_addr;
>> - mapping->dvo_wiring = child->dvo_wiring;
>> - mapping->ddc_pin = child->ddc_pin;
>> - mapping->i2c_pin = child->i2c_pin;
>> + mapping->dvo_port = config->dvo_port;
>> + mapping->slave_addr = config->slave_addr;
>> + mapping->dvo_wiring = config->dvo_wiring;
>> + mapping->ddc_pin = config->ddc_pin;
>> + mapping->i2c_pin = config->i2c_pin;
>> mapping->initialized = 1;
>> DRM_DEBUG_KMS("SDVO device: dvo=%x, addr=%x, wiring=%d, ddc_pin=%d, i2c_pin=%d\n",
>> mapping->dvo_port,
>> @@ -501,7 +508,7 @@ parse_sdvo_device_mapping(struct drm_i915_private *dev_priv, u8 bdb_version)
>> DRM_DEBUG_KMS("Maybe one SDVO port is shared by "
>> "two SDVO device.\n");
>> }
>> - if (child->slave2_addr) {
>> + if (config->slave2_addr) {
>> /* Maybe this is a SDVO device with multiple inputs */
>> /* And the mapping info is not added */
>> DRM_DEBUG_KMS("there exists the slave2_addr. Maybe this"
>> @@ -1571,8 +1578,7 @@ static void parse_ddi_port(struct drm_i915_private *dev_priv,
>>
>> static void parse_ddi_ports(struct drm_i915_private *dev_priv, u8 bdb_version)
>> {
>> - const struct child_device_config *child;
>> - int i;
>> + const struct child_device *child;
>>
>> if (!HAS_DDI(dev_priv) && !IS_CHERRYVIEW(dev_priv))
>> return;
>> @@ -1580,11 +1586,8 @@ static void parse_ddi_ports(struct drm_i915_private *dev_priv, u8 bdb_version)
>> if (bdb_version < 155)
>> return;
>>
>> - 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);
>> - }
>> + list_for_each_entry(child, &dev_priv->vbt.child_devices, node)
>> + parse_ddi_port(dev_priv, &child->config, bdb_version);
>> }
>>
>> static void
>> @@ -1592,8 +1595,9 @@ parse_general_definitions(struct drm_i915_private *dev_priv,
>> const struct bdb_header *bdb)
>> {
>> const struct bdb_general_definitions *defs;
>> - const struct child_device_config *child;
>> - int i, child_device_num, count;
>> + const struct child_device_config *config;
>> + struct child_device *child;
>> + int i, child_device_num;
>> u8 expected_size;
>> u16 block_size;
>> int bus_pin;
>> @@ -1629,8 +1633,8 @@ parse_general_definitions(struct drm_i915_private *dev_priv,
>> } else if (bdb->version <= 229) {
>> expected_size = 39;
>> } else {
>> - expected_size = sizeof(*child);
>> - BUILD_BUG_ON(sizeof(*child) < 39);
>> + expected_size = sizeof(*config);
>> + BUILD_BUG_ON(sizeof(*config) < 39);
>> DRM_DEBUG_DRIVER("Expected child device config size for VBT version %u not known; assuming %u\n",
>> bdb->version, expected_size);
>> }
>> @@ -1649,43 +1653,30 @@ parse_general_definitions(struct drm_i915_private *dev_priv,
>>
>> /* get the number of child device */
>> child_device_num = (block_size - sizeof(*defs)) / defs->child_dev_size;
>> - count = 0;
>> - /* get the number of child device that is present */
>> - for (i = 0; i < child_device_num; i++) {
>> - child = child_device_ptr(defs, i);
>> - if (!child->device_type)
>> - continue;
>> - count++;
>> - }
>> - if (!count) {
>> - DRM_DEBUG_KMS("no child dev is parsed from VBT\n");
>> - return;
>> - }
>> - dev_priv->vbt.child_dev = kcalloc(count, sizeof(*child), GFP_KERNEL);
>> - if (!dev_priv->vbt.child_dev) {
>> - DRM_DEBUG_KMS("No memory space for child device\n");
>> - return;
>> - }
>>
>> - dev_priv->vbt.child_dev_num = count;
>> - count = 0;
>> for (i = 0; i < child_device_num; i++) {
>> - child = child_device_ptr(defs, i);
>> - if (!child->device_type)
>> + config = child_device_ptr(defs, i);
>> + if (!config->device_type)
>> continue;
>>
>> DRM_DEBUG_KMS("Found VBT child device with type 0x%x\n",
>> - child->device_type);
>> + config->device_type);
>> +
>> + child = kmalloc(sizeof(*child), GFP_KERNEL);
>>
>> /*
>> * Copy as much as we know (sizeof) and is available
>> - * (child_dev_size) of the child device. Accessing the data must
>> - * depend on VBT version.
>> + * (child_dev_size) of the child device config. Accessing the
>> + * data must depend on VBT version.
>> */
>> - memcpy(dev_priv->vbt.child_dev + count, child,
>> - min_t(size_t, defs->child_dev_size, sizeof(*child)));
>> - count++;
>> + memcpy(&child->config, config,
>> + min_t(size_t, defs->child_dev_size, sizeof(*config)));
>> +
>> + list_add(&child->node, &dev_priv->vbt.child_devices);
>> }
>> +
>> + if (list_empty(&dev_priv->vbt.child_devices))
>> + DRM_DEBUG_KMS("no child dev is parsed from VBT\n");
>> }
>>
>> /* Common defaults which may be overridden by VBT. */
>> @@ -1849,6 +1840,8 @@ void intel_bios_init(struct drm_i915_private *dev_priv)
>> return;
>> }
>>
>> + INIT_LIST_HEAD(&dev_priv->vbt.child_devices);
>> +
>> init_vbt_defaults(dev_priv);
>>
>> /* If the OpRegion does not have VBT, look in PCI ROM. */
>> @@ -1903,9 +1896,13 @@ void intel_bios_init(struct drm_i915_private *dev_priv)
>> */
>> void intel_bios_driver_remove(struct drm_i915_private *dev_priv)
>> {
>> - kfree(dev_priv->vbt.child_dev);
>> - dev_priv->vbt.child_dev = NULL;
>> - dev_priv->vbt.child_dev_num = 0;
>> + struct child_device *child, *n;
>> +
>> + list_for_each_entry_safe(child, n, &dev_priv->vbt.child_devices, node) {
>> + list_del(&child->node);
>> + kfree(child);
>> + }
>> +
>> kfree(dev_priv->vbt.sdvo_lvds_vbt_mode);
>> dev_priv->vbt.sdvo_lvds_vbt_mode = NULL;
>> kfree(dev_priv->vbt.lfp_lvds_vbt_mode);
>> @@ -1929,21 +1926,22 @@ void intel_bios_driver_remove(struct drm_i915_private *dev_priv)
>> */
>> bool intel_bios_is_tv_present(struct drm_i915_private *dev_priv)
>> {
>> - const struct child_device_config *child;
>> - int i;
>> + const struct child_device_config *config;
>> + const struct child_device *child;
>>
>> if (!dev_priv->vbt.int_tv_support)
>> return false;
>>
>> - if (!dev_priv->vbt.child_dev_num)
>> + if (list_empty(&dev_priv->vbt.child_devices))
>> return true;
>>
>> - for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
>> - child = dev_priv->vbt.child_dev + i;
>> + list_for_each_entry(child, &dev_priv->vbt.child_devices, node) {
>> + config = &child->config;
>> +
>> /*
>> * If the device type is not TV, continue.
>> */
>> - switch (child->device_type) {
>> + switch (config->device_type) {
>> case DEVICE_TYPE_INT_TV:
>> case DEVICE_TYPE_TV:
>> case DEVICE_TYPE_TV_SVIDEO_COMPOSITE:
>> @@ -1954,7 +1952,7 @@ bool intel_bios_is_tv_present(struct drm_i915_private *dev_priv)
>> /* Only when the addin_offset is non-zero, it is regarded
>> * as present.
>> */
>> - if (child->addin_offset)
>> + if (config->addin_offset)
>> return true;
>> }
>>
>> @@ -1971,32 +1969,32 @@ bool intel_bios_is_tv_present(struct drm_i915_private *dev_priv)
>> */
>> bool intel_bios_is_lvds_present(struct drm_i915_private *dev_priv, u8 *i2c_pin)
>> {
>> - const struct child_device_config *child;
>> - int i;
>> + const struct child_device *child;
>> + const struct child_device_config *config;
>>
>> - if (!dev_priv->vbt.child_dev_num)
>> + if (list_empty(&dev_priv->vbt.child_devices))
>> return true;
>>
>> - for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
>> - child = dev_priv->vbt.child_dev + i;
>> + list_for_each_entry(child, &dev_priv->vbt.child_devices, node) {
>> + config = &child->config;
>>
>> /* If the device type is not LFP, continue.
>> * We have to check both the new identifiers as well as the
>> * old for compatibility with some BIOSes.
>> */
>> - if (child->device_type != DEVICE_TYPE_INT_LFP &&
>> - child->device_type != DEVICE_TYPE_LFP)
>> + if (config->device_type != DEVICE_TYPE_INT_LFP &&
>> + config->device_type != DEVICE_TYPE_LFP)
>> continue;
>>
>> - if (intel_gmbus_is_valid_pin(dev_priv, child->i2c_pin))
>> - *i2c_pin = child->i2c_pin;
>> + if (intel_gmbus_is_valid_pin(dev_priv, config->i2c_pin))
>> + *i2c_pin = config->i2c_pin;
>>
>> /* However, we cannot trust the BIOS writers to populate
>> * the VBT correctly. Since LVDS requires additional
>> * information from AIM blocks, a non-zero addin offset is
>> * a good indicator that the LVDS is actually present.
>> */
>> - if (child->addin_offset)
>> + if (config->addin_offset)
>> return true;
>>
>> /* But even then some BIOS writers perform some black magic
>> @@ -2020,7 +2018,8 @@ bool intel_bios_is_lvds_present(struct drm_i915_private *dev_priv, u8 *i2c_pin)
>> */
>> bool intel_bios_is_port_present(struct drm_i915_private *dev_priv, enum port port)
>> {
>> - const struct child_device_config *child;
>> + const struct child_device_config *config;
>> + const struct child_device *child;
>> static const struct {
>> u16 dp, hdmi;
>> } port_mapping[] = {
>> @@ -2030,7 +2029,6 @@ bool intel_bios_is_port_present(struct drm_i915_private *dev_priv, enum port por
>> [PORT_E] = { DVO_PORT_DPE, DVO_PORT_HDMIE, },
>> [PORT_F] = { DVO_PORT_DPF, DVO_PORT_HDMIF, },
>> };
>> - int i;
>>
>> if (HAS_DDI(dev_priv)) {
>> const struct ddi_vbt_port_info *port_info =
>> @@ -2045,16 +2043,13 @@ bool intel_bios_is_port_present(struct drm_i915_private *dev_priv, enum port por
>> if (WARN_ON(port == PORT_A) || port >= ARRAY_SIZE(port_mapping))
>> return false;
>>
>> - if (!dev_priv->vbt.child_dev_num)
>> - return false;
>> -
>> - for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
>> - child = dev_priv->vbt.child_dev + i;
>> + list_for_each_entry(child, &dev_priv->vbt.child_devices, node) {
>> + config = &child->config;
>>
>> - if ((child->dvo_port == port_mapping[port].dp ||
>> - child->dvo_port == port_mapping[port].hdmi) &&
>> - (child->device_type & (DEVICE_TYPE_TMDS_DVI_SIGNALING |
>> - DEVICE_TYPE_DISPLAYPORT_OUTPUT)))
>> + if ((config->dvo_port == port_mapping[port].dp ||
>> + config->dvo_port == port_mapping[port].hdmi) &&
>> + (config->device_type & (DEVICE_TYPE_TMDS_DVI_SIGNALING |
>> + DEVICE_TYPE_DISPLAYPORT_OUTPUT)))
>> return true;
>> }
>>
>> @@ -2070,7 +2065,8 @@ bool intel_bios_is_port_present(struct drm_i915_private *dev_priv, enum port por
>> */
>> bool intel_bios_is_port_edp(struct drm_i915_private *dev_priv, enum port port)
>> {
>> - const struct child_device_config *child;
>> + const struct child_device_config *config;
>> + const struct child_device *child;
>> static const short port_mapping[] = {
>> [PORT_B] = DVO_PORT_DPB,
>> [PORT_C] = DVO_PORT_DPC,
>> @@ -2078,19 +2074,15 @@ bool intel_bios_is_port_edp(struct drm_i915_private *dev_priv, enum port port)
>> [PORT_E] = DVO_PORT_DPE,
>> [PORT_F] = DVO_PORT_DPF,
>> };
>> - int i;
>>
>> if (HAS_DDI(dev_priv))
>> return dev_priv->vbt.ddi_port_info[port].supports_edp;
>>
>> - if (!dev_priv->vbt.child_dev_num)
>> - return false;
>> + list_for_each_entry(child, &dev_priv->vbt.child_devices, node) {
>> + config = &child->config;
>>
>> - for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
>> - child = dev_priv->vbt.child_dev + i;
>> -
>> - if (child->dvo_port == port_mapping[port] &&
>> - (child->device_type & DEVICE_TYPE_eDP_BITS) ==
>> + if (config->dvo_port == port_mapping[port] &&
>> + (config->device_type & DEVICE_TYPE_eDP_BITS) ==
>> (DEVICE_TYPE_eDP & DEVICE_TYPE_eDP_BITS))
>> return true;
>> }
>> @@ -2136,13 +2128,10 @@ static bool child_dev_is_dp_dual_mode(const struct child_device_config *child,
>> bool intel_bios_is_port_dp_dual_mode(struct drm_i915_private *dev_priv,
>> enum port port)
>> {
>> - const struct child_device_config *child;
>> - int i;
>> + const struct child_device *child;
>>
>> - for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
>> - child = dev_priv->vbt.child_dev + i;
>> -
>> - if (child_dev_is_dp_dual_mode(child, port))
>> + list_for_each_entry(child, &dev_priv->vbt.child_devices, node) {
>> + if (child_dev_is_dp_dual_mode(&child->config, port))
>> return true;
>> }
>>
>> @@ -2159,17 +2148,17 @@ bool intel_bios_is_port_dp_dual_mode(struct drm_i915_private *dev_priv,
>> bool intel_bios_is_dsi_present(struct drm_i915_private *dev_priv,
>> enum port *port)
>> {
>> - const struct child_device_config *child;
>> + const struct child_device_config *config;
>> + const struct child_device *child;
>> u8 dvo_port;
>> - int i;
>>
>> - for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
>> - child = dev_priv->vbt.child_dev + i;
>> + list_for_each_entry(child, &dev_priv->vbt.child_devices, node) {
>> + config = &child->config;
>>
>> - if (!(child->device_type & DEVICE_TYPE_MIPI_OUTPUT))
>> + if (!(config->device_type & DEVICE_TYPE_MIPI_OUTPUT))
>> continue;
>>
>> - dvo_port = child->dvo_port;
>> + dvo_port = config->dvo_port;
>>
>> if (dvo_port == DVO_PORT_MIPIA ||
>> (dvo_port == DVO_PORT_MIPIB && INTEL_GEN(dev_priv) >= 11) ||
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> index 7e0f67babe20..ae85ffa08665 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -724,8 +724,7 @@ struct intel_vbt_data {
>>
>> int crt_ddc_pin;
>>
>> - int child_dev_num;
>> - struct child_device_config *child_dev;
>> + struct list_head child_devices;
>>
>> struct ddi_vbt_port_info ddi_port_info[I915_MAX_PORTS];
>> struct sdvo_device_mapping sdvo_mappings[2];
>> --
>> 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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915/bios: store child devices in a list
@ 2019-11-07 8:22 ` Jani Nikula
0 siblings, 0 replies; 12+ messages in thread
From: Jani Nikula @ 2019-11-07 8:22 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: intel-gfx
On Wed, 06 Nov 2019, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> On Wed, Nov 06, 2019 at 06:45:31PM +0200, Jani Nikula wrote:
>> Using the array is getting clumsy. Make things a bit more dynamic.
>>
>> In code, start migrating towards calling the new struct child_device
>> "child" and the VBT-originating struct child_device_config "config".
>>
>> Remove early returns on not having child devices when the end result
>> after "iterating" the empty list would be the same.
>>
>> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>>
>> ---
>>
>> The end goal: allow more meta information to be added to the new
>> child_device struct, independent of DDI port info being used or not on
>> the platform, and eventually migrate ddi_port_info to it as well,
>> unifying the stuff across platforms.
>>
>> Currently it's not easily possible to associate for example the DSC
>> compression data to the child device for non-DDI platforms or for DSI
>> outputs. This lets us add the compression data (pointer) to struct
>> child_device.
>> ---
>> drivers/gpu/drm/i915/display/intel_bios.c | 203 ++++++++++------------
>> drivers/gpu/drm/i915/i915_drv.h | 3 +-
>> 2 files changed, 97 insertions(+), 109 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c
>> index a03f56b7b4ef..025074862ab0 100644
>> --- a/drivers/gpu/drm/i915/display/intel_bios.c
>> +++ b/drivers/gpu/drm/i915/display/intel_bios.c
>> @@ -58,6 +58,12 @@
>> * that.
>> */
>>
>> +/* Wrapper for VBT child device config */
>> +struct child_device {
>> + struct child_device_config config;
>> + struct list_head node;
>
> The wrapper is a bit unfortunate. I don't suppose we could just shove
> the list head into the existing struct and adjust what needs adjusting?
The existing struct is used for serialization and the size is checked
against what's in vbt etc. I might also add stuff in the wrapper struct,
at least intermediately, so it's kind of useful. I don't really think
the wrapper is all that bad.
>
>> +};
>> +
>> #define SLAVE_ADDR1 0x70
>> #define SLAVE_ADDR2 0x72
>>
>> @@ -449,8 +455,9 @@ static void
>> parse_sdvo_device_mapping(struct drm_i915_private *dev_priv, u8 bdb_version)
>> {
>> struct sdvo_device_mapping *mapping;
>> - const struct child_device_config *child;
>> - int i, count = 0;
>> + const struct child_device_config *config;
>
> This thing could at least can live inside the loop. Though the rename is
> also a bit unfortunate, leading to a needlessly large diff. Avoiding the
> wrapper struct would also avoid that. I guess another option would
> be to select a different name for the wrapper pointer here and keep the
> original name for the actual thing.
The main problem with avoiding the rename is to come up with a better
name for the wrapper structure. :) Child and config seemed apt, but I do
understand the downsides. I'd just like to have names that we can use
througout. Maybe we can stick to child for struct child_device_config,
but what do we call the whole wrapper struct and local vars?
Suggestions?
BR,
Jani.
>
>> + const struct child_device *child;
>> + int count = 0;
>>
>> /*
>> * Only parse SDVO mappings on gens that could have SDVO. This isn't
>> @@ -461,35 +468,35 @@ parse_sdvo_device_mapping(struct drm_i915_private *dev_priv, u8 bdb_version)
>> return;
>> }
>>
>> - for (i = 0, count = 0; i < dev_priv->vbt.child_dev_num; i++) {
>> - child = dev_priv->vbt.child_dev + i;
>> + list_for_each_entry(child, &dev_priv->vbt.child_devices, node) {
>> + config = &child->config;
>>
>> - if (child->slave_addr != SLAVE_ADDR1 &&
>> - child->slave_addr != SLAVE_ADDR2) {
>> + if (config->slave_addr != SLAVE_ADDR1 &&
>> + config->slave_addr != SLAVE_ADDR2) {
>> /*
>> * If the slave address is neither 0x70 nor 0x72,
>> * it is not a SDVO device. Skip it.
>> */
>> continue;
>> }
>> - if (child->dvo_port != DEVICE_PORT_DVOB &&
>> - child->dvo_port != DEVICE_PORT_DVOC) {
>> + if (config->dvo_port != DEVICE_PORT_DVOB &&
>> + config->dvo_port != DEVICE_PORT_DVOC) {
>> /* skip the incorrect SDVO port */
>> DRM_DEBUG_KMS("Incorrect SDVO port. Skip it\n");
>> continue;
>> }
>> DRM_DEBUG_KMS("the SDVO device with slave addr %2x is found on"
>> " %s port\n",
>> - child->slave_addr,
>> - (child->dvo_port == DEVICE_PORT_DVOB) ?
>> + config->slave_addr,
>> + (config->dvo_port == DEVICE_PORT_DVOB) ?
>> "SDVOB" : "SDVOC");
>> - mapping = &dev_priv->vbt.sdvo_mappings[child->dvo_port - 1];
>> + mapping = &dev_priv->vbt.sdvo_mappings[config->dvo_port - 1];
>> if (!mapping->initialized) {
>> - mapping->dvo_port = child->dvo_port;
>> - mapping->slave_addr = child->slave_addr;
>> - mapping->dvo_wiring = child->dvo_wiring;
>> - mapping->ddc_pin = child->ddc_pin;
>> - mapping->i2c_pin = child->i2c_pin;
>> + mapping->dvo_port = config->dvo_port;
>> + mapping->slave_addr = config->slave_addr;
>> + mapping->dvo_wiring = config->dvo_wiring;
>> + mapping->ddc_pin = config->ddc_pin;
>> + mapping->i2c_pin = config->i2c_pin;
>> mapping->initialized = 1;
>> DRM_DEBUG_KMS("SDVO device: dvo=%x, addr=%x, wiring=%d, ddc_pin=%d, i2c_pin=%d\n",
>> mapping->dvo_port,
>> @@ -501,7 +508,7 @@ parse_sdvo_device_mapping(struct drm_i915_private *dev_priv, u8 bdb_version)
>> DRM_DEBUG_KMS("Maybe one SDVO port is shared by "
>> "two SDVO device.\n");
>> }
>> - if (child->slave2_addr) {
>> + if (config->slave2_addr) {
>> /* Maybe this is a SDVO device with multiple inputs */
>> /* And the mapping info is not added */
>> DRM_DEBUG_KMS("there exists the slave2_addr. Maybe this"
>> @@ -1571,8 +1578,7 @@ static void parse_ddi_port(struct drm_i915_private *dev_priv,
>>
>> static void parse_ddi_ports(struct drm_i915_private *dev_priv, u8 bdb_version)
>> {
>> - const struct child_device_config *child;
>> - int i;
>> + const struct child_device *child;
>>
>> if (!HAS_DDI(dev_priv) && !IS_CHERRYVIEW(dev_priv))
>> return;
>> @@ -1580,11 +1586,8 @@ static void parse_ddi_ports(struct drm_i915_private *dev_priv, u8 bdb_version)
>> if (bdb_version < 155)
>> return;
>>
>> - 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);
>> - }
>> + list_for_each_entry(child, &dev_priv->vbt.child_devices, node)
>> + parse_ddi_port(dev_priv, &child->config, bdb_version);
>> }
>>
>> static void
>> @@ -1592,8 +1595,9 @@ parse_general_definitions(struct drm_i915_private *dev_priv,
>> const struct bdb_header *bdb)
>> {
>> const struct bdb_general_definitions *defs;
>> - const struct child_device_config *child;
>> - int i, child_device_num, count;
>> + const struct child_device_config *config;
>> + struct child_device *child;
>> + int i, child_device_num;
>> u8 expected_size;
>> u16 block_size;
>> int bus_pin;
>> @@ -1629,8 +1633,8 @@ parse_general_definitions(struct drm_i915_private *dev_priv,
>> } else if (bdb->version <= 229) {
>> expected_size = 39;
>> } else {
>> - expected_size = sizeof(*child);
>> - BUILD_BUG_ON(sizeof(*child) < 39);
>> + expected_size = sizeof(*config);
>> + BUILD_BUG_ON(sizeof(*config) < 39);
>> DRM_DEBUG_DRIVER("Expected child device config size for VBT version %u not known; assuming %u\n",
>> bdb->version, expected_size);
>> }
>> @@ -1649,43 +1653,30 @@ parse_general_definitions(struct drm_i915_private *dev_priv,
>>
>> /* get the number of child device */
>> child_device_num = (block_size - sizeof(*defs)) / defs->child_dev_size;
>> - count = 0;
>> - /* get the number of child device that is present */
>> - for (i = 0; i < child_device_num; i++) {
>> - child = child_device_ptr(defs, i);
>> - if (!child->device_type)
>> - continue;
>> - count++;
>> - }
>> - if (!count) {
>> - DRM_DEBUG_KMS("no child dev is parsed from VBT\n");
>> - return;
>> - }
>> - dev_priv->vbt.child_dev = kcalloc(count, sizeof(*child), GFP_KERNEL);
>> - if (!dev_priv->vbt.child_dev) {
>> - DRM_DEBUG_KMS("No memory space for child device\n");
>> - return;
>> - }
>>
>> - dev_priv->vbt.child_dev_num = count;
>> - count = 0;
>> for (i = 0; i < child_device_num; i++) {
>> - child = child_device_ptr(defs, i);
>> - if (!child->device_type)
>> + config = child_device_ptr(defs, i);
>> + if (!config->device_type)
>> continue;
>>
>> DRM_DEBUG_KMS("Found VBT child device with type 0x%x\n",
>> - child->device_type);
>> + config->device_type);
>> +
>> + child = kmalloc(sizeof(*child), GFP_KERNEL);
>>
>> /*
>> * Copy as much as we know (sizeof) and is available
>> - * (child_dev_size) of the child device. Accessing the data must
>> - * depend on VBT version.
>> + * (child_dev_size) of the child device config. Accessing the
>> + * data must depend on VBT version.
>> */
>> - memcpy(dev_priv->vbt.child_dev + count, child,
>> - min_t(size_t, defs->child_dev_size, sizeof(*child)));
>> - count++;
>> + memcpy(&child->config, config,
>> + min_t(size_t, defs->child_dev_size, sizeof(*config)));
>> +
>> + list_add(&child->node, &dev_priv->vbt.child_devices);
>> }
>> +
>> + if (list_empty(&dev_priv->vbt.child_devices))
>> + DRM_DEBUG_KMS("no child dev is parsed from VBT\n");
>> }
>>
>> /* Common defaults which may be overridden by VBT. */
>> @@ -1849,6 +1840,8 @@ void intel_bios_init(struct drm_i915_private *dev_priv)
>> return;
>> }
>>
>> + INIT_LIST_HEAD(&dev_priv->vbt.child_devices);
>> +
>> init_vbt_defaults(dev_priv);
>>
>> /* If the OpRegion does not have VBT, look in PCI ROM. */
>> @@ -1903,9 +1896,13 @@ void intel_bios_init(struct drm_i915_private *dev_priv)
>> */
>> void intel_bios_driver_remove(struct drm_i915_private *dev_priv)
>> {
>> - kfree(dev_priv->vbt.child_dev);
>> - dev_priv->vbt.child_dev = NULL;
>> - dev_priv->vbt.child_dev_num = 0;
>> + struct child_device *child, *n;
>> +
>> + list_for_each_entry_safe(child, n, &dev_priv->vbt.child_devices, node) {
>> + list_del(&child->node);
>> + kfree(child);
>> + }
>> +
>> kfree(dev_priv->vbt.sdvo_lvds_vbt_mode);
>> dev_priv->vbt.sdvo_lvds_vbt_mode = NULL;
>> kfree(dev_priv->vbt.lfp_lvds_vbt_mode);
>> @@ -1929,21 +1926,22 @@ void intel_bios_driver_remove(struct drm_i915_private *dev_priv)
>> */
>> bool intel_bios_is_tv_present(struct drm_i915_private *dev_priv)
>> {
>> - const struct child_device_config *child;
>> - int i;
>> + const struct child_device_config *config;
>> + const struct child_device *child;
>>
>> if (!dev_priv->vbt.int_tv_support)
>> return false;
>>
>> - if (!dev_priv->vbt.child_dev_num)
>> + if (list_empty(&dev_priv->vbt.child_devices))
>> return true;
>>
>> - for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
>> - child = dev_priv->vbt.child_dev + i;
>> + list_for_each_entry(child, &dev_priv->vbt.child_devices, node) {
>> + config = &child->config;
>> +
>> /*
>> * If the device type is not TV, continue.
>> */
>> - switch (child->device_type) {
>> + switch (config->device_type) {
>> case DEVICE_TYPE_INT_TV:
>> case DEVICE_TYPE_TV:
>> case DEVICE_TYPE_TV_SVIDEO_COMPOSITE:
>> @@ -1954,7 +1952,7 @@ bool intel_bios_is_tv_present(struct drm_i915_private *dev_priv)
>> /* Only when the addin_offset is non-zero, it is regarded
>> * as present.
>> */
>> - if (child->addin_offset)
>> + if (config->addin_offset)
>> return true;
>> }
>>
>> @@ -1971,32 +1969,32 @@ bool intel_bios_is_tv_present(struct drm_i915_private *dev_priv)
>> */
>> bool intel_bios_is_lvds_present(struct drm_i915_private *dev_priv, u8 *i2c_pin)
>> {
>> - const struct child_device_config *child;
>> - int i;
>> + const struct child_device *child;
>> + const struct child_device_config *config;
>>
>> - if (!dev_priv->vbt.child_dev_num)
>> + if (list_empty(&dev_priv->vbt.child_devices))
>> return true;
>>
>> - for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
>> - child = dev_priv->vbt.child_dev + i;
>> + list_for_each_entry(child, &dev_priv->vbt.child_devices, node) {
>> + config = &child->config;
>>
>> /* If the device type is not LFP, continue.
>> * We have to check both the new identifiers as well as the
>> * old for compatibility with some BIOSes.
>> */
>> - if (child->device_type != DEVICE_TYPE_INT_LFP &&
>> - child->device_type != DEVICE_TYPE_LFP)
>> + if (config->device_type != DEVICE_TYPE_INT_LFP &&
>> + config->device_type != DEVICE_TYPE_LFP)
>> continue;
>>
>> - if (intel_gmbus_is_valid_pin(dev_priv, child->i2c_pin))
>> - *i2c_pin = child->i2c_pin;
>> + if (intel_gmbus_is_valid_pin(dev_priv, config->i2c_pin))
>> + *i2c_pin = config->i2c_pin;
>>
>> /* However, we cannot trust the BIOS writers to populate
>> * the VBT correctly. Since LVDS requires additional
>> * information from AIM blocks, a non-zero addin offset is
>> * a good indicator that the LVDS is actually present.
>> */
>> - if (child->addin_offset)
>> + if (config->addin_offset)
>> return true;
>>
>> /* But even then some BIOS writers perform some black magic
>> @@ -2020,7 +2018,8 @@ bool intel_bios_is_lvds_present(struct drm_i915_private *dev_priv, u8 *i2c_pin)
>> */
>> bool intel_bios_is_port_present(struct drm_i915_private *dev_priv, enum port port)
>> {
>> - const struct child_device_config *child;
>> + const struct child_device_config *config;
>> + const struct child_device *child;
>> static const struct {
>> u16 dp, hdmi;
>> } port_mapping[] = {
>> @@ -2030,7 +2029,6 @@ bool intel_bios_is_port_present(struct drm_i915_private *dev_priv, enum port por
>> [PORT_E] = { DVO_PORT_DPE, DVO_PORT_HDMIE, },
>> [PORT_F] = { DVO_PORT_DPF, DVO_PORT_HDMIF, },
>> };
>> - int i;
>>
>> if (HAS_DDI(dev_priv)) {
>> const struct ddi_vbt_port_info *port_info =
>> @@ -2045,16 +2043,13 @@ bool intel_bios_is_port_present(struct drm_i915_private *dev_priv, enum port por
>> if (WARN_ON(port == PORT_A) || port >= ARRAY_SIZE(port_mapping))
>> return false;
>>
>> - if (!dev_priv->vbt.child_dev_num)
>> - return false;
>> -
>> - for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
>> - child = dev_priv->vbt.child_dev + i;
>> + list_for_each_entry(child, &dev_priv->vbt.child_devices, node) {
>> + config = &child->config;
>>
>> - if ((child->dvo_port == port_mapping[port].dp ||
>> - child->dvo_port == port_mapping[port].hdmi) &&
>> - (child->device_type & (DEVICE_TYPE_TMDS_DVI_SIGNALING |
>> - DEVICE_TYPE_DISPLAYPORT_OUTPUT)))
>> + if ((config->dvo_port == port_mapping[port].dp ||
>> + config->dvo_port == port_mapping[port].hdmi) &&
>> + (config->device_type & (DEVICE_TYPE_TMDS_DVI_SIGNALING |
>> + DEVICE_TYPE_DISPLAYPORT_OUTPUT)))
>> return true;
>> }
>>
>> @@ -2070,7 +2065,8 @@ bool intel_bios_is_port_present(struct drm_i915_private *dev_priv, enum port por
>> */
>> bool intel_bios_is_port_edp(struct drm_i915_private *dev_priv, enum port port)
>> {
>> - const struct child_device_config *child;
>> + const struct child_device_config *config;
>> + const struct child_device *child;
>> static const short port_mapping[] = {
>> [PORT_B] = DVO_PORT_DPB,
>> [PORT_C] = DVO_PORT_DPC,
>> @@ -2078,19 +2074,15 @@ bool intel_bios_is_port_edp(struct drm_i915_private *dev_priv, enum port port)
>> [PORT_E] = DVO_PORT_DPE,
>> [PORT_F] = DVO_PORT_DPF,
>> };
>> - int i;
>>
>> if (HAS_DDI(dev_priv))
>> return dev_priv->vbt.ddi_port_info[port].supports_edp;
>>
>> - if (!dev_priv->vbt.child_dev_num)
>> - return false;
>> + list_for_each_entry(child, &dev_priv->vbt.child_devices, node) {
>> + config = &child->config;
>>
>> - for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
>> - child = dev_priv->vbt.child_dev + i;
>> -
>> - if (child->dvo_port == port_mapping[port] &&
>> - (child->device_type & DEVICE_TYPE_eDP_BITS) ==
>> + if (config->dvo_port == port_mapping[port] &&
>> + (config->device_type & DEVICE_TYPE_eDP_BITS) ==
>> (DEVICE_TYPE_eDP & DEVICE_TYPE_eDP_BITS))
>> return true;
>> }
>> @@ -2136,13 +2128,10 @@ static bool child_dev_is_dp_dual_mode(const struct child_device_config *child,
>> bool intel_bios_is_port_dp_dual_mode(struct drm_i915_private *dev_priv,
>> enum port port)
>> {
>> - const struct child_device_config *child;
>> - int i;
>> + const struct child_device *child;
>>
>> - for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
>> - child = dev_priv->vbt.child_dev + i;
>> -
>> - if (child_dev_is_dp_dual_mode(child, port))
>> + list_for_each_entry(child, &dev_priv->vbt.child_devices, node) {
>> + if (child_dev_is_dp_dual_mode(&child->config, port))
>> return true;
>> }
>>
>> @@ -2159,17 +2148,17 @@ bool intel_bios_is_port_dp_dual_mode(struct drm_i915_private *dev_priv,
>> bool intel_bios_is_dsi_present(struct drm_i915_private *dev_priv,
>> enum port *port)
>> {
>> - const struct child_device_config *child;
>> + const struct child_device_config *config;
>> + const struct child_device *child;
>> u8 dvo_port;
>> - int i;
>>
>> - for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
>> - child = dev_priv->vbt.child_dev + i;
>> + list_for_each_entry(child, &dev_priv->vbt.child_devices, node) {
>> + config = &child->config;
>>
>> - if (!(child->device_type & DEVICE_TYPE_MIPI_OUTPUT))
>> + if (!(config->device_type & DEVICE_TYPE_MIPI_OUTPUT))
>> continue;
>>
>> - dvo_port = child->dvo_port;
>> + dvo_port = config->dvo_port;
>>
>> if (dvo_port == DVO_PORT_MIPIA ||
>> (dvo_port == DVO_PORT_MIPIB && INTEL_GEN(dev_priv) >= 11) ||
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> index 7e0f67babe20..ae85ffa08665 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -724,8 +724,7 @@ struct intel_vbt_data {
>>
>> int crt_ddc_pin;
>>
>> - int child_dev_num;
>> - struct child_device_config *child_dev;
>> + struct list_head child_devices;
>>
>> struct ddi_vbt_port_info ddi_port_info[I915_MAX_PORTS];
>> struct sdvo_device_mapping sdvo_mappings[2];
>> --
>> 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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] drm/i915/bios: store child devices in a list
@ 2019-11-07 10:40 ` Ville Syrjälä
0 siblings, 0 replies; 12+ messages in thread
From: Ville Syrjälä @ 2019-11-07 10:40 UTC (permalink / raw)
To: Jani Nikula; +Cc: intel-gfx
On Thu, Nov 07, 2019 at 10:22:10AM +0200, Jani Nikula wrote:
> On Wed, 06 Nov 2019, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> > On Wed, Nov 06, 2019 at 06:45:31PM +0200, Jani Nikula wrote:
> >> Using the array is getting clumsy. Make things a bit more dynamic.
> >>
> >> In code, start migrating towards calling the new struct child_device
> >> "child" and the VBT-originating struct child_device_config "config".
> >>
> >> Remove early returns on not having child devices when the end result
> >> after "iterating" the empty list would be the same.
> >>
> >> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> >> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> >>
> >> ---
> >>
> >> The end goal: allow more meta information to be added to the new
> >> child_device struct, independent of DDI port info being used or not on
> >> the platform, and eventually migrate ddi_port_info to it as well,
> >> unifying the stuff across platforms.
> >>
> >> Currently it's not easily possible to associate for example the DSC
> >> compression data to the child device for non-DDI platforms or for DSI
> >> outputs. This lets us add the compression data (pointer) to struct
> >> child_device.
> >> ---
> >> drivers/gpu/drm/i915/display/intel_bios.c | 203 ++++++++++------------
> >> drivers/gpu/drm/i915/i915_drv.h | 3 +-
> >> 2 files changed, 97 insertions(+), 109 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c
> >> index a03f56b7b4ef..025074862ab0 100644
> >> --- a/drivers/gpu/drm/i915/display/intel_bios.c
> >> +++ b/drivers/gpu/drm/i915/display/intel_bios.c
> >> @@ -58,6 +58,12 @@
> >> * that.
> >> */
> >>
> >> +/* Wrapper for VBT child device config */
> >> +struct child_device {
> >> + struct child_device_config config;
> >> + struct list_head node;
> >
> > The wrapper is a bit unfortunate. I don't suppose we could just shove
> > the list head into the existing struct and adjust what needs adjusting?
>
> The existing struct is used for serialization and the size is checked
> against what's in vbt etc. I might also add stuff in the wrapper struct,
> at least intermediately, so it's kind of useful. I don't really think
> the wrapper is all that bad.
>
> >
> >> +};
> >> +
> >> #define SLAVE_ADDR1 0x70
> >> #define SLAVE_ADDR2 0x72
> >>
> >> @@ -449,8 +455,9 @@ static void
> >> parse_sdvo_device_mapping(struct drm_i915_private *dev_priv, u8 bdb_version)
> >> {
> >> struct sdvo_device_mapping *mapping;
> >> - const struct child_device_config *child;
> >> - int i, count = 0;
> >> + const struct child_device_config *config;
> >
> > This thing could at least can live inside the loop. Though the rename is
> > also a bit unfortunate, leading to a needlessly large diff. Avoiding the
> > wrapper struct would also avoid that. I guess another option would
> > be to select a different name for the wrapper pointer here and keep the
> > original name for the actual thing.
>
> The main problem with avoiding the rename is to come up with a better
> name for the wrapper structure. :) Child and config seemed apt, but I do
> understand the downsides. I'd just like to have names that we can use
> througout. Maybe we can stick to child for struct child_device_config,
> but what do we call the whole wrapper struct and local vars?
> Suggestions?
I suck at naming things. If no better name comes up I guess we could at
least split the rename to a separate patch. Would be easier to see
what's actually changing with the introduction of the list.
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915/bios: store child devices in a list
@ 2019-11-07 10:40 ` Ville Syrjälä
0 siblings, 0 replies; 12+ messages in thread
From: Ville Syrjälä @ 2019-11-07 10:40 UTC (permalink / raw)
To: Jani Nikula; +Cc: intel-gfx
On Thu, Nov 07, 2019 at 10:22:10AM +0200, Jani Nikula wrote:
> On Wed, 06 Nov 2019, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> > On Wed, Nov 06, 2019 at 06:45:31PM +0200, Jani Nikula wrote:
> >> Using the array is getting clumsy. Make things a bit more dynamic.
> >>
> >> In code, start migrating towards calling the new struct child_device
> >> "child" and the VBT-originating struct child_device_config "config".
> >>
> >> Remove early returns on not having child devices when the end result
> >> after "iterating" the empty list would be the same.
> >>
> >> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> >> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> >>
> >> ---
> >>
> >> The end goal: allow more meta information to be added to the new
> >> child_device struct, independent of DDI port info being used or not on
> >> the platform, and eventually migrate ddi_port_info to it as well,
> >> unifying the stuff across platforms.
> >>
> >> Currently it's not easily possible to associate for example the DSC
> >> compression data to the child device for non-DDI platforms or for DSI
> >> outputs. This lets us add the compression data (pointer) to struct
> >> child_device.
> >> ---
> >> drivers/gpu/drm/i915/display/intel_bios.c | 203 ++++++++++------------
> >> drivers/gpu/drm/i915/i915_drv.h | 3 +-
> >> 2 files changed, 97 insertions(+), 109 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c
> >> index a03f56b7b4ef..025074862ab0 100644
> >> --- a/drivers/gpu/drm/i915/display/intel_bios.c
> >> +++ b/drivers/gpu/drm/i915/display/intel_bios.c
> >> @@ -58,6 +58,12 @@
> >> * that.
> >> */
> >>
> >> +/* Wrapper for VBT child device config */
> >> +struct child_device {
> >> + struct child_device_config config;
> >> + struct list_head node;
> >
> > The wrapper is a bit unfortunate. I don't suppose we could just shove
> > the list head into the existing struct and adjust what needs adjusting?
>
> The existing struct is used for serialization and the size is checked
> against what's in vbt etc. I might also add stuff in the wrapper struct,
> at least intermediately, so it's kind of useful. I don't really think
> the wrapper is all that bad.
>
> >
> >> +};
> >> +
> >> #define SLAVE_ADDR1 0x70
> >> #define SLAVE_ADDR2 0x72
> >>
> >> @@ -449,8 +455,9 @@ static void
> >> parse_sdvo_device_mapping(struct drm_i915_private *dev_priv, u8 bdb_version)
> >> {
> >> struct sdvo_device_mapping *mapping;
> >> - const struct child_device_config *child;
> >> - int i, count = 0;
> >> + const struct child_device_config *config;
> >
> > This thing could at least can live inside the loop. Though the rename is
> > also a bit unfortunate, leading to a needlessly large diff. Avoiding the
> > wrapper struct would also avoid that. I guess another option would
> > be to select a different name for the wrapper pointer here and keep the
> > original name for the actual thing.
>
> The main problem with avoiding the rename is to come up with a better
> name for the wrapper structure. :) Child and config seemed apt, but I do
> understand the downsides. I'd just like to have names that we can use
> througout. Maybe we can stick to child for struct child_device_config,
> but what do we call the whole wrapper struct and local vars?
> Suggestions?
I suck at naming things. If no better name comes up I guess we could at
least split the rename to a separate patch. Would be easier to see
what's actually changing with the introduction of the list.
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2019-11-07 10:40 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-06 16:45 [PATCH] drm/i915/bios: store child devices in a list Jani Nikula
2019-11-06 16:45 ` [Intel-gfx] " Jani Nikula
2019-11-06 20:16 ` ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
2019-11-06 20:16 ` [Intel-gfx] " Patchwork
2019-11-06 20:37 ` [PATCH] " Ville Syrjälä
2019-11-06 20:37 ` [Intel-gfx] " Ville Syrjälä
2019-11-07 8:22 ` Jani Nikula
2019-11-07 8:22 ` [Intel-gfx] " Jani Nikula
2019-11-07 10:40 ` Ville Syrjälä
2019-11-07 10:40 ` [Intel-gfx] " Ville Syrjälä
2019-11-06 20:42 ` ✗ Fi.CI.BAT: failure for " Patchwork
2019-11-06 20:42 ` [Intel-gfx] " Patchwork
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.