drm: handle override/firmware edid at the lowest level

Handle override edid and firmware edid at the low level to transparently and completely replace the real edid. This also prevents actual edid reads for them, but retains ddc probe.

Make the function useful for resetting, not just setting, the ELD.

Signed-off-by Jani Nikula <[email protected]>

---
 drivers/gpu/drm/drm_edid.c | 3 +++
 1 file changed, 3 insertions(+)

drivers/gpu/drm/drm_edid.c +0 -0

index 24e7b282f16c..362036360724 100644

Handle debugfs override edid and firmware edid at the low level to transparently and completely replace the real edid. Previously, we practically only used the modes from the override EDID, and none of the other data. This also prevents actual EDID reads when the EDID is to be overridden, but retains the DDC probe.

FIXME: validate override edid, deduplicate firmware edid validation.

Signed-off-by Jani Nikula <[email protected]>

---
 drivers/gpu/drm/drm_edid.c         | 15 +++++++++++++++
 drivers/gpu/drm/drm_probe_helper.c | 19 +------------------
 2 files changed, 16 insertions(+), 18 deletions(-)

drivers/gpu/drm/drm_edid.c +15 -0

index 362036360724..054e2d74eafc 100644
@@ -1309,6 +1309,10 @@ static void connector_bad_edid(struct drm_connector *connector,
  * level, drivers must make all reasonable efforts to expose it as an I2C
  * adapter and use drm_get_edid() instead of abusing this function.
  *
+ * The EDID may be overridden using debugfs override_edid or firmare EDID
+ * (drm_load_edid_firmware()), in this priority order. Having either of them
+ * bypasses actual EDID reads.
+ *
  * Return: Pointer to valid EDID or NULL if we couldn't find any.
  */
 struct edid *drm_do_get_edid(struct drm_connector *connector,

@@ -1318,6 +1322,17 @@ struct edid *drm_do_get_edid(struct drm_connector *connector,
 {
 	int i, j = 0, valid_extensions = 0;
 	u8 *edid, *new;
+	struct edid *override = NULL;
+
+	if (connector->override_edid)
+		override = drm_edid_duplicate((const struct edid *)
+					      connector->edid_blob_ptr->data);
+
+	if (!override)
+		override = drm_load_edid_firmware(connector);
+
+	if (!IS_ERR_OR_NULL(override))
+		return override;
 
 	if ((edid = kmalloc(EDID_LENGTH, GFP_KERNEL)) == NULL)
 		return NULL;

drivers/gpu/drm/drm_probe_helper.c +1 -18

index 358957118ca9..871326cbc465 100644
@@ -199,8 +199,6 @@ drm_connector_detect(struct drm_connector *connector, bool force)
  *    drm_mode_probed_add(). New modes start their life with status as OK.
  *    Modes are added from a single source using the following priority order.
  *
- *    - debugfs 'override_edid' (used for testing only)
- *    - firmware EDID (drm_load_edid_firmware())
  *    - &drm_connector_helper_funcs.get_modes vfunc
  *    - if the connector status is connector_status_connected, standard
  *      VESA DMT modes up to 1024x768 are automatically added

@@ -305,22 +303,7 @@ int drm_helper_probe_single_connector_modes(struct drm_connector *connector,
 		goto prune;
 	}
 
-	if (connector->override_edid) {
-		struct edid *edid = (struct edid *) connector->edid_blob_ptr->data;
-
-		count = drm_add_edid_modes(connector, edid);
-		drm_edid_to_eld(connector, edid);
-	} else {
-		struct edid *edid = drm_load_edid_firmware(connector);
-		if (!IS_ERR_OR_NULL(edid)) {
-			drm_mode_connector_update_edid_property(connector, edid);
-			count = drm_add_edid_modes(connector, edid);
-			drm_edid_to_eld(connector, edid);
-			kfree(edid);
-		}
-		if (count == 0)
-			count = (*connector_funcs->get_modes)(connector);
-	}
+	count = (*connector_funcs->get_modes)(connector);
 
 	if (count == 0 && connector->status == connector_status_connected)
 		count = drm_add_modes_noedid(connector, 1024, 768);

Signature · 1 line

2.1.4

Quoted 5 lines · Jani Nikula

Handle debugfs override edid and firmware edid at the low level to transparently and completely replace the real edid. Previously, we practically only used the modes from the override EDID, and none of the other data. This also prevents actual EDID reads when the EDID is to be overridden, but retains the DDC probe.

Hmm. Isn't that a bad thing? If someone has broken DDC on their cable or somethign then the override EDID wouldn't be returned by drm_get_edid().

Quoted 82 lines

FIXME: validate override edid, deduplicate firmware edid validation.

Signed-off-by Jani Nikula <[email protected]>

---
 drivers/gpu/drm/drm_edid.c         | 15 +++++++++++++++
 drivers/gpu/drm/drm_probe_helper.c | 19 +------------------
 2 files changed, 16 insertions(+), 18 deletions(-)

drivers/gpu/drm/drm_edid.c +15 -0

index 362036360724..054e2d74eafc 100644
@@ -1309,6 +1309,10 @@ static void connector_bad_edid(struct drm_connector *connector,
  * level, drivers must make all reasonable efforts to expose it as an I2C
  * adapter and use drm_get_edid() instead of abusing this function.
  *
+ * The EDID may be overridden using debugfs override_edid or firmare EDID
+ * (drm_load_edid_firmware()), in this priority order. Having either of them
+ * bypasses actual EDID reads.
+ *
  * Return: Pointer to valid EDID or NULL if we couldn't find any.
  */
 struct edid *drm_do_get_edid(struct drm_connector *connector,

@@ -1318,6 +1322,17 @@ struct edid *drm_do_get_edid(struct drm_connector *connector,
 {
 	int i, j = 0, valid_extensions = 0;
 	u8 *edid, *new;
+	struct edid *override = NULL;
+
+	if (connector->override_edid)
+		override = drm_edid_duplicate((const struct edid *)
+					      connector->edid_blob_ptr->data);
+
+	if (!override)
+		override = drm_load_edid_firmware(connector);
+
+	if (!IS_ERR_OR_NULL(override))
+		return override;
 
 	if ((edid = kmalloc(EDID_LENGTH, GFP_KERNEL)) == NULL)
 		return NULL;

drivers/gpu/drm/drm_probe_helper.c +1 -18

index 358957118ca9..871326cbc465 100644
@@ -199,8 +199,6 @@ drm_connector_detect(struct drm_connector *connector, bool force)
  *    drm_mode_probed_add(). New modes start their life with status as OK.
  *    Modes are added from a single source using the following priority order.
  *
- *    - debugfs 'override_edid' (used for testing only)
- *    - firmware EDID (drm_load_edid_firmware())
  *    - &drm_connector_helper_funcs.get_modes vfunc
  *    - if the connector status is connector_status_connected, standard
  *      VESA DMT modes up to 1024x768 are automatically added

@@ -305,22 +303,7 @@ int drm_helper_probe_single_connector_modes(struct drm_connector *connector,
 		goto prune;
 	}
 
-	if (connector->override_edid) {
-		struct edid *edid = (struct edid *) connector->edid_blob_ptr->data;
-
-		count = drm_add_edid_modes(connector, edid);
-		drm_edid_to_eld(connector, edid);
-	} else {
-		struct edid *edid = drm_load_edid_firmware(connector);
-		if (!IS_ERR_OR_NULL(edid)) {
-			drm_mode_connector_update_edid_property(connector, edid);
-			count = drm_add_edid_modes(connector, edid);
-			drm_edid_to_eld(connector, edid);
-			kfree(edid);
-		}
-		if (count == 0)
-			count = (*connector_funcs->get_modes)(connector);
-	}
+	count = (*connector_funcs->get_modes)(connector);
 
 	if (count == 0 && connector->status == connector_status_connected)
 		count = drm_add_modes_noedid(connector, 1024, 768);

Signature · 1 line

2.1.4

Quoted 7 lines · Ville Syrjälä

Quoted 5 lines · Jani Nikula

Handle debugfs override edid and firmware edid at the low level to transparently and completely replace the real edid. Previously, we practically only used the modes from the override EDID, and none of the other data. This also prevents actual EDID reads when the EDID is to be overridden, but retains the DDC probe.

Quoted 2 lines

Hmm. Isn't that a bad thing? If someone has broken DDC on their cable or somethign then the override EDID wouldn't be returned by drm_get_edid().

Isn't this in line with how this currently works?

BR,
Jani.

Quoted 82 lines

Quoted 82 lines

FIXME: validate override edid, deduplicate firmware edid validation.

Signed-off-by Jani Nikula <[email protected]>

---
 drivers/gpu/drm/drm_edid.c         | 15 +++++++++++++++
 drivers/gpu/drm/drm_probe_helper.c | 19 +------------------
 2 files changed, 16 insertions(+), 18 deletions(-)

drivers/gpu/drm/drm_edid.c +15 -0

index 362036360724..054e2d74eafc 100644
@@ -1309,6 +1309,10 @@ static void connector_bad_edid(struct drm_connector *connector,
  * level, drivers must make all reasonable efforts to expose it as an I2C
  * adapter and use drm_get_edid() instead of abusing this function.
  *
+ * The EDID may be overridden using debugfs override_edid or firmare EDID
+ * (drm_load_edid_firmware()), in this priority order. Having either of them
+ * bypasses actual EDID reads.
+ *
  * Return: Pointer to valid EDID or NULL if we couldn't find any.
  */
 struct edid *drm_do_get_edid(struct drm_connector *connector,

@@ -1318,6 +1322,17 @@ struct edid *drm_do_get_edid(struct drm_connector *connector,
 {
 	int i, j = 0, valid_extensions = 0;
 	u8 *edid, *new;
+	struct edid *override = NULL;
+
+	if (connector->override_edid)
+		override = drm_edid_duplicate((const struct edid *)
+					      connector->edid_blob_ptr->data);
+
+	if (!override)
+		override = drm_load_edid_firmware(connector);
+
+	if (!IS_ERR_OR_NULL(override))
+		return override;
 
 	if ((edid = kmalloc(EDID_LENGTH, GFP_KERNEL)) == NULL)
 		return NULL;

drivers/gpu/drm/drm_probe_helper.c +1 -18

index 358957118ca9..871326cbc465 100644
@@ -199,8 +199,6 @@ drm_connector_detect(struct drm_connector *connector, bool force)
  *    drm_mode_probed_add(). New modes start their life with status as OK.
  *    Modes are added from a single source using the following priority order.
  *
- *    - debugfs 'override_edid' (used for testing only)
- *    - firmware EDID (drm_load_edid_firmware())
  *    - &drm_connector_helper_funcs.get_modes vfunc
  *    - if the connector status is connector_status_connected, standard
  *      VESA DMT modes up to 1024x768 are automatically added

@@ -305,22 +303,7 @@ int drm_helper_probe_single_connector_modes(struct drm_connector *connector,
 		goto prune;
 	}
 
-	if (connector->override_edid) {
-		struct edid *edid = (struct edid *) connector->edid_blob_ptr->data;
-
-		count = drm_add_edid_modes(connector, edid);
-		drm_edid_to_eld(connector, edid);
-	} else {
-		struct edid *edid = drm_load_edid_firmware(connector);
-		if (!IS_ERR_OR_NULL(edid)) {
-			drm_mode_connector_update_edid_property(connector, edid);
-			count = drm_add_edid_modes(connector, edid);
-			drm_edid_to_eld(connector, edid);
-			kfree(edid);
-		}
-		if (count == 0)
-			count = (*connector_funcs->get_modes)(connector);
-	}
+	count = (*connector_funcs->get_modes)(connector);
 
 	if (count == 0 && connector->status == connector_status_connected)
 		count = drm_add_modes_noedid(connector, 1024, 768);

Signature · 1 line

2.1.4

Signature · 1 line

Jani Nikula, Intel Open Source Technology Center

Quoted 11 lines · Jani Nikula

Quoted 9 lines · Ville Syrjälä

Quoted 5 lines · Jani Nikula

Handle debugfs override edid and firmware edid at the low level to transparently and completely replace the real edid. Previously, we practically only used the modes from the override EDID, and none of the other data. This also prevents actual EDID reads when the EDID is to be overridden, but retains the DDC probe.

Hmm. Isn't that a bad thing? If someone has broken DDC on their cable or somethign then the override EDID wouldn't be returned by drm_get_edid().

Quoted 1 line

Isn't this in line with how this currently works?

Not really. If connector->force is used and an override EDID is provided, then drm_get_edid() likely never gets called, and the modes come directly from the overide EDID. With your change forcing the connector state wouldn't because we still wouldn't get any modes from the override EDID due to the DDC check.

I guess one way around this would be to skip the DDC check if the connector state has been forced.

Quoted 91 lines

BR,
Jani.

Quoted 82 lines

Quoted 82 lines

FIXME: validate override edid, deduplicate firmware edid validation.

Signed-off-by Jani Nikula <[email protected]>

---
 drivers/gpu/drm/drm_edid.c         | 15 +++++++++++++++
 drivers/gpu/drm/drm_probe_helper.c | 19 +------------------
 2 files changed, 16 insertions(+), 18 deletions(-)

drivers/gpu/drm/drm_edid.c +15 -0

index 362036360724..054e2d74eafc 100644
@@ -1309,6 +1309,10 @@ static void connector_bad_edid(struct drm_connector *connector,
  * level, drivers must make all reasonable efforts to expose it as an I2C
  * adapter and use drm_get_edid() instead of abusing this function.
  *
+ * The EDID may be overridden using debugfs override_edid or firmare EDID
+ * (drm_load_edid_firmware()), in this priority order. Having either of them
+ * bypasses actual EDID reads.
+ *
  * Return: Pointer to valid EDID or NULL if we couldn't find any.
  */
 struct edid *drm_do_get_edid(struct drm_connector *connector,

@@ -1318,6 +1322,17 @@ struct edid *drm_do_get_edid(struct drm_connector *connector,
 {
 	int i, j = 0, valid_extensions = 0;
 	u8 *edid, *new;
+	struct edid *override = NULL;
+
+	if (connector->override_edid)
+		override = drm_edid_duplicate((const struct edid *)
+					      connector->edid_blob_ptr->data);
+
+	if (!override)
+		override = drm_load_edid_firmware(connector);
+
+	if (!IS_ERR_OR_NULL(override))
+		return override;
 
 	if ((edid = kmalloc(EDID_LENGTH, GFP_KERNEL)) == NULL)
 		return NULL;

drivers/gpu/drm/drm_probe_helper.c +1 -18

index 358957118ca9..871326cbc465 100644
@@ -199,8 +199,6 @@ drm_connector_detect(struct drm_connector *connector, bool force)
  *    drm_mode_probed_add(). New modes start their life with status as OK.
  *    Modes are added from a single source using the following priority order.
  *
- *    - debugfs 'override_edid' (used for testing only)
- *    - firmware EDID (drm_load_edid_firmware())
  *    - &drm_connector_helper_funcs.get_modes vfunc
  *    - if the connector status is connector_status_connected, standard
  *      VESA DMT modes up to 1024x768 are automatically added

@@ -305,22 +303,7 @@ int drm_helper_probe_single_connector_modes(struct drm_connector *connector,
 		goto prune;
 	}
 
-	if (connector->override_edid) {
-		struct edid *edid = (struct edid *) connector->edid_blob_ptr->data;
-
-		count = drm_add_edid_modes(connector, edid);
-		drm_edid_to_eld(connector, edid);
-	} else {
-		struct edid *edid = drm_load_edid_firmware(connector);
-		if (!IS_ERR_OR_NULL(edid)) {
-			drm_mode_connector_update_edid_property(connector, edid);
-			count = drm_add_edid_modes(connector, edid);
-			drm_edid_to_eld(connector, edid);
-			kfree(edid);
-		}
-		if (count == 0)
-			count = (*connector_funcs->get_modes)(connector);
-	}
+	count = (*connector_funcs->get_modes)(connector);
 
 	if (count == 0 && connector->status == connector_status_connected)
 		count = drm_add_modes_noedid(connector, 1024, 768);

Signature · 1 line

2.1.4

Signature · 1 line

Jani Nikula, Intel Open Source Technology Center

Quoted 12 lines · Ville Syrjälä

Quoted 11 lines · Jani Nikula

Quoted 9 lines · Ville Syrjälä

Quoted 5 lines · Jani Nikula

Handle debugfs override edid and firmware edid at the low level to transparently and completely replace the real edid. Previously, we practically only used the modes from the override EDID, and none of the other data. This also prevents actual EDID reads when the EDID is to be overridden, but retains the DDC probe.

Hmm. Isn't that a bad thing? If someone has broken DDC on their cable or somethign then the override EDID wouldn't be returned by drm_get_edid().

Quoted 7 lines

Quoted 1 line

Isn't this in line with how this currently works?

Not really. If connector->force is used and an override EDID is provided, then drm_get_edid() likely never gets called, and the modes come directly from the overide EDID. With your change forcing the connector state wouldn't because we still wouldn't get any modes from the override EDID due to the DDC check.

Right.

Quoted 2 lines

I guess one way around this would be to skip the DDC check if the connector state has been forced.

I guess I'm asking if you think the whole approach here is workable. What we currently have is so flaky.

BR,
Jani.

Quoted 91 lines

Quoted 91 lines

BR,
Jani.

Quoted 82 lines

Quoted 82 lines

FIXME: validate override edid, deduplicate firmware edid validation.

Signed-off-by Jani Nikula <[email protected]>

---
 drivers/gpu/drm/drm_edid.c         | 15 +++++++++++++++
 drivers/gpu/drm/drm_probe_helper.c | 19 +------------------
 2 files changed, 16 insertions(+), 18 deletions(-)

drivers/gpu/drm/drm_edid.c +15 -0

index 362036360724..054e2d74eafc 100644
@@ -1309,6 +1309,10 @@ static void connector_bad_edid(struct drm_connector *connector,
  * level, drivers must make all reasonable efforts to expose it as an I2C
  * adapter and use drm_get_edid() instead of abusing this function.
  *
+ * The EDID may be overridden using debugfs override_edid or firmare EDID
+ * (drm_load_edid_firmware()), in this priority order. Having either of them
+ * bypasses actual EDID reads.
+ *
  * Return: Pointer to valid EDID or NULL if we couldn't find any.
  */
 struct edid *drm_do_get_edid(struct drm_connector *connector,

@@ -1318,6 +1322,17 @@ struct edid *drm_do_get_edid(struct drm_connector *connector,
 {
 	int i, j = 0, valid_extensions = 0;
 	u8 *edid, *new;
+	struct edid *override = NULL;
+
+	if (connector->override_edid)
+		override = drm_edid_duplicate((const struct edid *)
+					      connector->edid_blob_ptr->data);
+
+	if (!override)
+		override = drm_load_edid_firmware(connector);
+
+	if (!IS_ERR_OR_NULL(override))
+		return override;
 
 	if ((edid = kmalloc(EDID_LENGTH, GFP_KERNEL)) == NULL)
 		return NULL;

drivers/gpu/drm/drm_probe_helper.c +1 -18

index 358957118ca9..871326cbc465 100644
@@ -199,8 +199,6 @@ drm_connector_detect(struct drm_connector *connector, bool force)
  *    drm_mode_probed_add(). New modes start their life with status as OK.
  *    Modes are added from a single source using the following priority order.
  *
- *    - debugfs 'override_edid' (used for testing only)
- *    - firmware EDID (drm_load_edid_firmware())
  *    - &drm_connector_helper_funcs.get_modes vfunc
  *    - if the connector status is connector_status_connected, standard
  *      VESA DMT modes up to 1024x768 are automatically added

@@ -305,22 +303,7 @@ int drm_helper_probe_single_connector_modes(struct drm_connector *connector,
 		goto prune;
 	}
 
-	if (connector->override_edid) {
-		struct edid *edid = (struct edid *) connector->edid_blob_ptr->data;
-
-		count = drm_add_edid_modes(connector, edid);
-		drm_edid_to_eld(connector, edid);
-	} else {
-		struct edid *edid = drm_load_edid_firmware(connector);
-		if (!IS_ERR_OR_NULL(edid)) {
-			drm_mode_connector_update_edid_property(connector, edid);
-			count = drm_add_edid_modes(connector, edid);
-			drm_edid_to_eld(connector, edid);
-			kfree(edid);
-		}
-		if (count == 0)
-			count = (*connector_funcs->get_modes)(connector);
-	}
+	count = (*connector_funcs->get_modes)(connector);
 
 	if (count == 0 && connector->status == connector_status_connected)
 		count = drm_add_modes_noedid(connector, 1024, 768);

Signature · 1 line

2.1.4

Signature · 1 line

Jani Nikula, Intel Open Source Technology Center

Signature · 1 line

Jani Nikula, Intel Open Source Technology Center

Quoted 15 lines · Jani Nikula

Make the function useful for resetting, not just setting, the ELD.

Signed-off-by Jani Nikula <[email protected]>

---
 drivers/gpu/drm/drm_edid.c | 3 +++
 1 file changed, 3 insertions(+)

drivers/gpu/drm/drm_edid.c +0 -0

index 24e7b282f16c..362036360724 100644
@@ -3430,6 +3430,9 @@ void drm_edid_to_eld(struct drm_connector *connector, struct edid *edid)
 
 	memset(eld, 0, sizeof(connector->eld));
 

drivers/gpu/drm/drm_edid.c void drm_edid_to_eld(struct drm_connector *connector, struct edid *edid) · line 3438

Quoted 6 lines

drivers/gpu/drm/drm_edid.c +3 -0

+	if (!edid)
+		return;
+
 	connector->latency_present[0] = false;
 	connector->latency_present[1] = false;
 	connector->video_latency[0] = 0;

/me thinks the check should be after all these.

Hmm. Actually the cea ext block check below should be safe wrt. edid==NULL, so not sure we need this at all.

Quoted 16 lines · Ville Syrjälä

Quoted 15 lines · Jani Nikula

Make the function useful for resetting, not just setting, the ELD.

Signed-off-by Jani Nikula <[email protected]>

---
 drivers/gpu/drm/drm_edid.c | 3 +++
 1 file changed, 3 insertions(+)

drivers/gpu/drm/drm_edid.c +0 -0

index 24e7b282f16c..362036360724 100644
@@ -3430,6 +3430,9 @@ void drm_edid_to_eld(struct drm_connector *connector, struct edid *edid)
 
 	memset(eld, 0, sizeof(connector->eld));
 

Quoted 8 lines

drivers/gpu/drm/drm_edid.c void drm_edid_to_eld(struct drm_connector *connector, struct edid *edid) · line 3438

Quoted 6 lines

drivers/gpu/drm/drm_edid.c +3 -0

+	if (!edid)
+		return;
+
 	connector->latency_present[0] = false;
 	connector->latency_present[1] = false;
 	connector->video_latency[0] = 0;

/me thinks the check should be after all these.

D'oh!

Quoted 2 lines

Hmm. Actually the cea ext block check below should be safe wrt. edid==NULL, so not sure we need this at all.

I'd just like to be explicit and avoid the debug message on missing CEA extensions if the whole EDID is missing.

Quoted 21 lines · Jani Nikula

Quoted 12 lines · Ville Syrjälä

Quoted 11 lines · Jani Nikula

Quoted 9 lines · Ville Syrjälä

Quoted 5 lines · Jani Nikula

Handle debugfs override edid and firmware edid at the low level to transparently and completely replace the real edid. Previously, we practically only used the modes from the override EDID, and none of the other data. This also prevents actual EDID reads when the EDID is to be overridden, but retains the DDC probe.

Hmm. Isn't that a bad thing? If someone has broken DDC on their cable or somethign then the override EDID wouldn't be returned by drm_get_edid().

Quoted 7 lines

Quoted 1 line

Isn't this in line with how this currently works?

Not really. If connector->force is used and an override EDID is provided, then drm_get_edid() likely never gets called, and the modes come directly from the overide EDID. With your change forcing the connector state wouldn't because we still wouldn't get any modes from the override EDID due to the DDC check.

Quoted 7 lines

Right.

Quoted 2 lines

I guess one way around this would be to skip the DDC check if the connector state has been forced.

I guess I'm asking if you think the whole approach here is workable. What we currently have is so flaky.

I'm 100% with you on that. So yes, I do like the idea.

I think the last time the override EDIDs were discussed someone was suggesting that the EDID override itself should force the connector state. IIRC Daniel didn't agree. I don't have a particularly strong opinion either way, but I guess trying to preserve the current semantics (ie. EDID override and connector status forcing are mostly separate) would be the thing to aim for, at least initially.

Quoted 99 lines

BR,
Jani.

Quoted 91 lines

Quoted 91 lines

BR,
Jani.

Quoted 82 lines

Quoted 82 lines

FIXME: validate override edid, deduplicate firmware edid validation.

Signed-off-by Jani Nikula <[email protected]>

---
 drivers/gpu/drm/drm_edid.c         | 15 +++++++++++++++
 drivers/gpu/drm/drm_probe_helper.c | 19 +------------------
 2 files changed, 16 insertions(+), 18 deletions(-)

drivers/gpu/drm/drm_edid.c +15 -0

index 362036360724..054e2d74eafc 100644
@@ -1309,6 +1309,10 @@ static void connector_bad_edid(struct drm_connector *connector,
  * level, drivers must make all reasonable efforts to expose it as an I2C
  * adapter and use drm_get_edid() instead of abusing this function.
  *
+ * The EDID may be overridden using debugfs override_edid or firmare EDID
+ * (drm_load_edid_firmware()), in this priority order. Having either of them
+ * bypasses actual EDID reads.
+ *
  * Return: Pointer to valid EDID or NULL if we couldn't find any.
  */
 struct edid *drm_do_get_edid(struct drm_connector *connector,

@@ -1318,6 +1322,17 @@ struct edid *drm_do_get_edid(struct drm_connector *connector,
 {
 	int i, j = 0, valid_extensions = 0;
 	u8 *edid, *new;
+	struct edid *override = NULL;
+
+	if (connector->override_edid)
+		override = drm_edid_duplicate((const struct edid *)
+					      connector->edid_blob_ptr->data);
+
+	if (!override)
+		override = drm_load_edid_firmware(connector);
+
+	if (!IS_ERR_OR_NULL(override))
+		return override;
 
 	if ((edid = kmalloc(EDID_LENGTH, GFP_KERNEL)) == NULL)
 		return NULL;

drivers/gpu/drm/drm_probe_helper.c +1 -18

index 358957118ca9..871326cbc465 100644
@@ -199,8 +199,6 @@ drm_connector_detect(struct drm_connector *connector, bool force)
  *    drm_mode_probed_add(). New modes start their life with status as OK.
  *    Modes are added from a single source using the following priority order.
  *
- *    - debugfs 'override_edid' (used for testing only)
- *    - firmware EDID (drm_load_edid_firmware())
  *    - &drm_connector_helper_funcs.get_modes vfunc
  *    - if the connector status is connector_status_connected, standard
  *      VESA DMT modes up to 1024x768 are automatically added

@@ -305,22 +303,7 @@ int drm_helper_probe_single_connector_modes(struct drm_connector *connector,
 		goto prune;
 	}
 
-	if (connector->override_edid) {
-		struct edid *edid = (struct edid *) connector->edid_blob_ptr->data;
-
-		count = drm_add_edid_modes(connector, edid);
-		drm_edid_to_eld(connector, edid);
-	} else {
-		struct edid *edid = drm_load_edid_firmware(connector);
-		if (!IS_ERR_OR_NULL(edid)) {
-			drm_mode_connector_update_edid_property(connector, edid);
-			count = drm_add_edid_modes(connector, edid);
-			drm_edid_to_eld(connector, edid);
-			kfree(edid);
-		}
-		if (count == 0)
-			count = (*connector_funcs->get_modes)(connector);
-	}
+	count = (*connector_funcs->get_modes)(connector);
 
 	if (count == 0 && connector->status == connector_status_connected)
 		count = drm_add_modes_noedid(connector, 1024, 768);

Signature · 1 line

2.1.4

Signature · 1 line

Jani Nikula, Intel Open Source Technology Center

Signature · 1 line

Jani Nikula, Intel Open Source Technology Center

Signature · 2 lines

Ville Syrjälä
Intel OTC