* [PATCH] drm/i915/bios: store child devices in a list
@ 2019-11-06 16:45 Jani Nikula
2019-11-06 16:45 ` [Intel-gfx] " Jani Nikula
` (3 more replies)
0 siblings, 4 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 [PATCH] drm/i915/bios: store child devices in a list Jani Nikula @ 2019-11-06 16:45 ` Jani Nikula 2019-11-06 20:16 ` ✗ Fi.CI.CHECKPATCH: warning for " Patchwork ` (2 subsequent siblings) 3 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 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 ` Patchwork 2019-11-06 20:16 ` [Intel-gfx] " Patchwork 2019-11-06 20:37 ` [PATCH] " Ville Syrjälä 2019-11-06 20:42 ` ✗ Fi.CI.BAT: failure for " Patchwork 3 siblings, 1 reply; 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 ` ✗ Fi.CI.CHECKPATCH: warning for " Patchwork @ 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 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:37 ` Ville Syrjälä 2019-11-06 20:37 ` [Intel-gfx] " Ville Syrjälä 2019-11-07 8:22 ` Jani Nikula 2019-11-06 20:42 ` ✗ Fi.CI.BAT: failure for " Patchwork 3 siblings, 2 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 ` [PATCH] " Ville Syrjälä @ 2019-11-06 20:37 ` Ville Syrjälä 2019-11-07 8:22 ` Jani Nikula 1 sibling, 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: [PATCH] drm/i915/bios: store child devices in a list 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ä 1 sibling, 2 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 @ 2019-11-07 8:22 ` Jani Nikula 2019-11-07 10:40 ` Ville Syrjälä 1 sibling, 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 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ä 1 sibling, 1 reply; 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ä @ 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
* ✗ Fi.CI.BAT: failure for drm/i915/bios: store child devices in a list 2019-11-06 16:45 [PATCH] drm/i915/bios: store child devices in a list Jani Nikula ` (2 preceding siblings ...) 2019-11-06 20:37 ` [PATCH] " Ville Syrjälä @ 2019-11-06 20:42 ` Patchwork 2019-11-06 20:42 ` [Intel-gfx] " Patchwork 3 siblings, 1 reply; 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 ` ✗ Fi.CI.BAT: failure for " Patchwork @ 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
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 a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).