All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jani Nikula <jani.nikula@intel.com>
To: dri-devel@lists.freedesktop.org
Cc: jani.nikula@intel.com, intel-gfx@lists.freedesktop.org
Subject: [PATCH 07/12] drm/edid: abstract an EDID block read helper
Date: Thu,  7 Apr 2022 12:14:33 +0300	[thread overview]
Message-ID: <f9fa9811b2a6a9bbdd03809805b3d18fb21956cf.1649322799.git.jani.nikula@intel.com> (raw)
In-Reply-To: <cover.1649322799.git.jani.nikula@intel.com>

We have an abstraction for the EDID base block read, yet duplicating the
retries and error handling for extension block reads. Introduce a more
generic EDID block read helper.

Switch to the helper piecemeal, starting with drm_edid_get_panel_id(),
which doesn't need or have access to the connector anyway.

The subtle change is switching from drm_edid_block_valid() to
edid_block_check(). We also status print once, not for every attempt.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/drm_edid.c | 58 +++++++++++++++++++++++++++++++++-----
 1 file changed, 51 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 926ffe5cd97e..162f0a959376 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -1670,6 +1670,7 @@ EXPORT_SYMBOL(drm_edid_are_equal);
 
 enum edid_block_status {
 	EDID_BLOCK_OK = 0,
+	EDID_BLOCK_READ_FAIL,
 	EDID_BLOCK_NULL,
 	EDID_BLOCK_ZERO,
 	EDID_BLOCK_HEADER_CORRUPT,
@@ -1736,6 +1737,9 @@ static void edid_block_status_print(enum edid_block_status status,
 	switch (status) {
 	case EDID_BLOCK_OK:
 		break;
+	case EDID_BLOCK_READ_FAIL:
+		pr_debug("EDID block %d read failed\n", block_num);
+		break;
 	case EDID_BLOCK_NULL:
 		pr_debug("EDID block %d pointer is NULL\n", block_num);
 		break;
@@ -2039,6 +2043,39 @@ EXPORT_SYMBOL(drm_add_override_edid_modes);
 
 typedef int read_block_fn(void *context, u8 *buf, unsigned int block, size_t len);
 
+static enum edid_block_status edid_block_read(void *block, unsigned int block_num,
+					      read_block_fn read_block,
+					      void *context)
+{
+	enum edid_block_status status;
+	bool is_base_block = block_num == 0;
+	int try;
+
+	for (try = 0; try < 4; try++) {
+		if (read_block(context, block, block_num, EDID_LENGTH))
+			return EDID_BLOCK_READ_FAIL;
+
+		status = edid_block_check(block, is_base_block);
+		if (status == EDID_BLOCK_HEADER_REPAIR) {
+			edid_header_fix(block);
+
+			/* Retry with fixed header, update status if that worked. */
+			status = edid_block_check(block, is_base_block);
+			if (status == EDID_BLOCK_OK)
+				status = EDID_BLOCK_HEADER_FIXED;
+		}
+
+		if (edid_block_status_valid(status, edid_block_tag(block)))
+			break;
+
+		/* Fail early for unrepairable base block all zeros. */
+		if (try == 0 && is_base_block && status == EDID_BLOCK_ZERO)
+			break;
+	}
+
+	return status;
+}
+
 static struct edid *drm_do_get_edid_base_block(struct drm_connector *connector,
 					       read_block_fn read_block,
 					       void *context)
@@ -2237,20 +2274,27 @@ static u32 edid_extract_panel_id(const struct edid *edid)
 
 u32 drm_edid_get_panel_id(struct i2c_adapter *adapter)
 {
-	const struct edid *edid;
-	u32 panel_id;
-
-	edid = drm_do_get_edid_base_block(NULL, drm_do_probe_ddc_edid, adapter);
+	enum edid_block_status status;
+	void *base_block;
+	u32 panel_id = 0;
 
 	/*
 	 * There are no manufacturer IDs of 0, so if there is a problem reading
 	 * the EDID then we'll just return 0.
 	 */
-	if (!edid)
+
+	base_block = kmalloc(EDID_LENGTH, GFP_KERNEL);
+	if (!base_block)
 		return 0;
 
-	panel_id = edid_extract_panel_id(edid);
-	kfree(edid);
+	status = edid_block_read(base_block, 0, drm_do_probe_ddc_edid, adapter);
+
+	edid_block_status_print(status, base_block, 0);
+
+	if (edid_block_status_valid(status, edid_block_tag(base_block)))
+		panel_id = edid_extract_panel_id(base_block);
+
+	kfree(base_block);
 
 	return panel_id;
 }
-- 
2.30.2


WARNING: multiple messages have this Message-ID (diff)
From: Jani Nikula <jani.nikula@intel.com>
To: dri-devel@lists.freedesktop.org
Cc: jani.nikula@intel.com, intel-gfx@lists.freedesktop.org
Subject: [Intel-gfx] [PATCH 07/12] drm/edid: abstract an EDID block read helper
Date: Thu,  7 Apr 2022 12:14:33 +0300	[thread overview]
Message-ID: <f9fa9811b2a6a9bbdd03809805b3d18fb21956cf.1649322799.git.jani.nikula@intel.com> (raw)
In-Reply-To: <cover.1649322799.git.jani.nikula@intel.com>

We have an abstraction for the EDID base block read, yet duplicating the
retries and error handling for extension block reads. Introduce a more
generic EDID block read helper.

Switch to the helper piecemeal, starting with drm_edid_get_panel_id(),
which doesn't need or have access to the connector anyway.

The subtle change is switching from drm_edid_block_valid() to
edid_block_check(). We also status print once, not for every attempt.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/drm_edid.c | 58 +++++++++++++++++++++++++++++++++-----
 1 file changed, 51 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 926ffe5cd97e..162f0a959376 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -1670,6 +1670,7 @@ EXPORT_SYMBOL(drm_edid_are_equal);
 
 enum edid_block_status {
 	EDID_BLOCK_OK = 0,
+	EDID_BLOCK_READ_FAIL,
 	EDID_BLOCK_NULL,
 	EDID_BLOCK_ZERO,
 	EDID_BLOCK_HEADER_CORRUPT,
@@ -1736,6 +1737,9 @@ static void edid_block_status_print(enum edid_block_status status,
 	switch (status) {
 	case EDID_BLOCK_OK:
 		break;
+	case EDID_BLOCK_READ_FAIL:
+		pr_debug("EDID block %d read failed\n", block_num);
+		break;
 	case EDID_BLOCK_NULL:
 		pr_debug("EDID block %d pointer is NULL\n", block_num);
 		break;
@@ -2039,6 +2043,39 @@ EXPORT_SYMBOL(drm_add_override_edid_modes);
 
 typedef int read_block_fn(void *context, u8 *buf, unsigned int block, size_t len);
 
+static enum edid_block_status edid_block_read(void *block, unsigned int block_num,
+					      read_block_fn read_block,
+					      void *context)
+{
+	enum edid_block_status status;
+	bool is_base_block = block_num == 0;
+	int try;
+
+	for (try = 0; try < 4; try++) {
+		if (read_block(context, block, block_num, EDID_LENGTH))
+			return EDID_BLOCK_READ_FAIL;
+
+		status = edid_block_check(block, is_base_block);
+		if (status == EDID_BLOCK_HEADER_REPAIR) {
+			edid_header_fix(block);
+
+			/* Retry with fixed header, update status if that worked. */
+			status = edid_block_check(block, is_base_block);
+			if (status == EDID_BLOCK_OK)
+				status = EDID_BLOCK_HEADER_FIXED;
+		}
+
+		if (edid_block_status_valid(status, edid_block_tag(block)))
+			break;
+
+		/* Fail early for unrepairable base block all zeros. */
+		if (try == 0 && is_base_block && status == EDID_BLOCK_ZERO)
+			break;
+	}
+
+	return status;
+}
+
 static struct edid *drm_do_get_edid_base_block(struct drm_connector *connector,
 					       read_block_fn read_block,
 					       void *context)
@@ -2237,20 +2274,27 @@ static u32 edid_extract_panel_id(const struct edid *edid)
 
 u32 drm_edid_get_panel_id(struct i2c_adapter *adapter)
 {
-	const struct edid *edid;
-	u32 panel_id;
-
-	edid = drm_do_get_edid_base_block(NULL, drm_do_probe_ddc_edid, adapter);
+	enum edid_block_status status;
+	void *base_block;
+	u32 panel_id = 0;
 
 	/*
 	 * There are no manufacturer IDs of 0, so if there is a problem reading
 	 * the EDID then we'll just return 0.
 	 */
-	if (!edid)
+
+	base_block = kmalloc(EDID_LENGTH, GFP_KERNEL);
+	if (!base_block)
 		return 0;
 
-	panel_id = edid_extract_panel_id(edid);
-	kfree(edid);
+	status = edid_block_read(base_block, 0, drm_do_probe_ddc_edid, adapter);
+
+	edid_block_status_print(status, base_block, 0);
+
+	if (edid_block_status_valid(status, edid_block_tag(base_block)))
+		panel_id = edid_extract_panel_id(base_block);
+
+	kfree(base_block);
 
 	return panel_id;
 }
-- 
2.30.2


  parent reply	other threads:[~2022-04-07  9:15 UTC|newest]

Thread overview: 46+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-04-07  9:14 [PATCH 00/12] drm/edid: low level EDID block read refactoring etc Jani Nikula
2022-04-07  9:14 ` [Intel-gfx] " Jani Nikula
2022-04-07  9:14 ` [PATCH 01/12] drm/edid: convert edid_is_zero() to edid_block_is_zero() for blocks Jani Nikula
2022-04-07  9:14   ` [Intel-gfx] " Jani Nikula
2022-04-07  9:14 ` [PATCH 02/12] drm/edid: have edid_block_check() detect blocks that are all zero Jani Nikula
2022-04-07  9:14   ` [Intel-gfx] " Jani Nikula
2022-04-07  9:14 ` [PATCH 03/12] drm/edid: refactor EDID block status printing Jani Nikula
2022-04-07  9:14   ` [Intel-gfx] " Jani Nikula
2022-04-07 11:54   ` Ville Syrjälä
2022-04-07 11:54     ` [Intel-gfx] " Ville Syrjälä
2022-04-11  9:49     ` Jani Nikula
2022-04-11  9:49       ` [Intel-gfx] " Jani Nikula
2022-04-07  9:14 ` [PATCH 04/12] drm/edid: add a helper to log dump an EDID block Jani Nikula
2022-04-07  9:14   ` [Intel-gfx] " Jani Nikula
2022-04-07  9:14 ` [PATCH 05/12] drm/edid: pass struct edid to connector_bad_edid() Jani Nikula
2022-04-07  9:14   ` [Intel-gfx] " Jani Nikula
2022-04-07 11:56   ` Ville Syrjälä
2022-04-07 11:56     ` [Intel-gfx] " Ville Syrjälä
2022-04-07  9:14 ` [PATCH 06/12] drm/edid: add typedef for block read function Jani Nikula
2022-04-07  9:14   ` [Intel-gfx] " Jani Nikula
2022-04-07 12:06   ` Ville Syrjälä
2022-04-07 12:06     ` [Intel-gfx] " Ville Syrjälä
2022-04-11  9:48     ` Jani Nikula
2022-04-11  9:48       ` [Intel-gfx] " Jani Nikula
2022-04-07  9:14 ` Jani Nikula [this message]
2022-04-07  9:14   ` [Intel-gfx] [PATCH 07/12] drm/edid: abstract an EDID block read helper Jani Nikula
2022-04-07  9:14 ` [PATCH 08/12] drm/edid: use EDID block read helper in drm_do_get_edid() Jani Nikula
2022-04-07  9:14   ` [Intel-gfx] " Jani Nikula
2022-04-07  9:14 ` [PATCH 09/12] drm/edid: convert extension block read to EDID block read helper Jani Nikula
2022-04-07  9:14   ` [Intel-gfx] " Jani Nikula
2022-04-07  9:14 ` [PATCH 10/12] drm/edid: drop extra local var Jani Nikula
2022-04-07  9:14   ` [Intel-gfx] " Jani Nikula
2022-04-07  9:14 ` [PATCH 11/12] drm/edid: add single point of return to drm_do_get_edid() Jani Nikula
2022-04-07  9:14   ` [Intel-gfx] " Jani Nikula
2022-04-07  9:14 ` [PATCH 12/12] drm/edid: add EDID block count and size helpers Jani Nikula
2022-04-07  9:14   ` [Intel-gfx] " Jani Nikula
2022-04-07 15:07   ` [PATCH v2] " Jani Nikula
2022-04-07 15:07     ` [Intel-gfx] " Jani Nikula
2022-04-07 12:24 ` [Intel-gfx] ✗ Fi.CI.BUILD: failure for drm/edid: low level EDID block read refactoring etc Patchwork
2022-04-07 12:41   ` Jani Nikula
2022-04-07 12:44 ` [PATCH 00/12] " Ville Syrjälä
2022-04-07 12:44   ` [Intel-gfx] " Ville Syrjälä
2022-04-11  9:51   ` Jani Nikula
2022-04-11  9:51     ` [Intel-gfx] " Jani Nikula
2022-04-07 17:43 ` [Intel-gfx] ✗ Fi.CI.BAT: failure for drm/edid: low level EDID block read refactoring etc. (rev2) Patchwork
2022-04-07 23:17 ` [Intel-gfx] ✗ Fi.CI.BAT: failure for drm/edid: low level EDID block read refactoring etc. (rev3) Patchwork

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=f9fa9811b2a6a9bbdd03809805b3d18fb21956cf.1649322799.git.jani.nikula@intel.com \
    --to=jani.nikula@intel.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=intel-gfx@lists.freedesktop.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.