| Message ID | 20191204180258.3906-1-Jerry.Zuo@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v4] drm: Add support for DP 1.4 Compliance edid corruption test | expand |
[AMD Official Use Only - Internal Distribution Only]
Hi All:
I just checked the CI report https://patchwork.freedesktop.org/series/70530/. The failures described in there are not quite related to my patch. Seems it is a false-positive. Does anyone know something about the issue described in the report?
In addition, I'll resend a new version that fixes the checkpatch issues.
Thanks a lot.
Regards,
Jerry
-----Original Message-----
From: Jerry (Fangzhi) Zuo <Jerry.Zuo@amd.com>
Sent: December 4, 2019 1:03 PM
To: intel-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org; amd-gfx@lists.freedesktop.org
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>; Jani Nikula <jani.nikula@linux.intel.com>; manasi.d.navare@intel.com; Wentland, Harry <Harry.Wentland@amd.com>; Kazlauskas, Nicholas <Nicholas.Kazlauskas@amd.com>; Siqueira, Rodrigo <Rodrigo.Siqueira@amd.com>; Deucher, Alexander <Alexander.Deucher@amd.com>; Zuo, Jerry <Jerry.Zuo@amd.com>
Subject: [PATCH v4] drm: Add support for DP 1.4 Compliance edid corruption test
Unlike DP 1.2 edid corruption test, DP 1.4 requires to calculate real CRC value of the last edid data block, and write it back.
Current edid CRC calculates routine adds the last CRC byte, and check if non-zero.
This behavior is not accurate; actually, we need to return the actual CRC value when corruption is detected.
This commit changes this issue by returning the calculated CRC, and initiate the required sequence.
Change since v3
- Fix a minor typo.
Change since v2
- Rewrite checksum computation routine to avoid duplicated code.
- Rename to avoid confusion.
Change since v1
- Have separate routine for returning real CRC.
Signed-off-by: Jerry (Fangzhi) Zuo <Jerry.Zuo@amd.com>
---
drivers/gpu/drm/drm_dp_helper.c | 35 +++++++++++++++++++++++++++++++++++
drivers/gpu/drm/drm_edid.c | 23 +++++++++++++++++++----
include/drm/drm_connector.h | 6 ++++++
include/drm/drm_dp_helper.h | 3 +++
4 files changed, 63 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c index 2c7870aef469..c59f7c94ebf1 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -351,6 +351,41 @@ int drm_dp_dpcd_read_link_status(struct drm_dp_aux *aux, } EXPORT_SYMBOL(drm_dp_dpcd_read_link_status);
+/**
+ * drm_dp_send_real_edid_checksum() - send back real edid checksum
+value
+ * @aux: DisplayPort AUX channel
+ * @real_edid_checksum: real edid checksum for the last block
+ *
+ * Returns true on success
+ */
+bool drm_dp_send_real_edid_checksum(struct drm_dp_aux *aux,
+ u8 real_edid_checksum)
+{
+ u8 link_edid_read = 0, auto_test_req = 0, test_resp = 0;
+
+ drm_dp_dpcd_read(aux, DP_DEVICE_SERVICE_IRQ_VECTOR, &auto_test_req, 1);
+ auto_test_req &= DP_AUTOMATED_TEST_REQUEST;
+
+ drm_dp_dpcd_read(aux, DP_TEST_REQUEST, &link_edid_read, 1);
+ link_edid_read &= DP_TEST_LINK_EDID_READ;
+
+ if (!auto_test_req || !link_edid_read) {
+ DRM_DEBUG_KMS("Source DUT does not support TEST_EDID_READ\n");
+ return false;
+ }
+
+ drm_dp_dpcd_write(aux, DP_DEVICE_SERVICE_IRQ_VECTOR, &auto_test_req,
+1);
+
+ /* send back checksum for the last edid extension block data */
+ drm_dp_dpcd_write(aux, DP_TEST_EDID_CHECKSUM, &real_edid_checksum, 1);
+
+ test_resp |= DP_TEST_EDID_CHECKSUM_WRITE;
+ drm_dp_dpcd_write(aux, DP_TEST_RESPONSE, &test_resp, 1);
+
+ return true;
+}
+EXPORT_SYMBOL(drm_dp_send_real_edid_checksum);
+
/**
* drm_dp_downstream_max_clock() - extract branch device max
* pixel rate for legacy VGA
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 5b33b7cfd645..0e35405ecc74 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -1450,11 +1450,22 @@ static int validate_displayid(u8 *displayid, int length, int idx); static int drm_edid_block_checksum(const u8 *raw_edid) {
int i;
- u8 csum = 0;
- for (i = 0; i < EDID_LENGTH; i++)
+ u8 csum = 0, crc = 0;
+
+ for (i = 0; i < EDID_LENGTH - 1; i++)
csum += raw_edid[i];
- return csum;
+ crc = 0x100 - csum;
+
+ return crc;
+}
+
+static bool drm_edid_block_checksum_diff(const u8 *raw_edid, u8
+real_checksum) {
+ if (raw_edid[EDID_LENGTH - 1] != real_checksum)
+ return true;
+ else
+ return false;
}
static bool drm_edid_is_zero(const u8 *in_edid, int length) @@ -1512,7 +1523,7 @@ bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid,
}
csum = drm_edid_block_checksum(raw_edid);
- if (csum) {
+ if (drm_edid_block_checksum_diff(raw_edid, csum)) {
if (edid_corrupt)
*edid_corrupt = true;
@@ -1653,6 +1664,7 @@ static void connector_bad_edid(struct drm_connector *connector,
u8 *edid, int num_blocks)
{
int i;
+ u8 num_of_ext = edid[0x7e];
if (connector->bad_edid_counter++ && !drm_debug_enabled(DRM_UT_KMS))
return;
@@ -1675,6 +1687,9 @@ static void connector_bad_edid(struct drm_connector *connector,
prefix, DUMP_PREFIX_NONE, 16, 1,
block, EDID_LENGTH, false);
}
+
+ /* Calculate real checksum for the last edid extension block data */
+ connector->real_edid_checksum = drm_edid_block_checksum(edid +
+num_of_ext * EDID_LENGTH);
}
/* Get override or firmware EDID */
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h index 5f8c3389d46f..86fc9f603fbc 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -1349,6 +1349,12 @@ struct drm_connector {
* rev1.1 4.2.2.6
*/
bool edid_corrupt;
+ /**
+ * @real_edid_checksum: real edid checksum value for corrupted edid block.
+ * Required in Displayport 1.4 compliance testing
+ * rev1.1 4.2.2.6
+ */
+ uint8_t real_edid_checksum;
/** @debugfs_entry: debugfs directory for this connector */
struct dentry *debugfs_entry;
diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h index 51ecb5112ef8..1f9bd688f90e 100644
--- a/include/drm/drm_dp_helper.h
+++ b/include/drm/drm_dp_helper.h
@@ -1455,6 +1455,9 @@ static inline ssize_t drm_dp_dpcd_writeb(struct drm_dp_aux *aux, int drm_dp_dpcd_read_link_status(struct drm_dp_aux *aux,
u8 status[DP_LINK_STATUS_SIZE]);
+bool drm_dp_send_real_edid_checksum(struct drm_dp_aux *aux,
+ u8 real_edid_checksum);
+
int drm_dp_downstream_max_clock(const u8 dpcd[DP_RECEIVER_CAP_SIZE],
const u8 port_cap[4]);
int drm_dp_downstream_max_bpc(const u8 dpcd[DP_RECEIVER_CAP_SIZE],
--
2.14.1
diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c index 2c7870aef469..c59f7c94ebf1 100644 --- a/drivers/gpu/drm/drm_dp_helper.c +++ b/drivers/gpu/drm/drm_dp_helper.c @@ -351,6 +351,41 @@ int drm_dp_dpcd_read_link_status(struct drm_dp_aux *aux, } EXPORT_SYMBOL(drm_dp_dpcd_read_link_status); +/** + * drm_dp_send_real_edid_checksum() - send back real edid checksum value + * @aux: DisplayPort AUX channel + * @real_edid_checksum: real edid checksum for the last block + * + * Returns true on success + */ +bool drm_dp_send_real_edid_checksum(struct drm_dp_aux *aux, + u8 real_edid_checksum) +{ + u8 link_edid_read = 0, auto_test_req = 0, test_resp = 0; + + drm_dp_dpcd_read(aux, DP_DEVICE_SERVICE_IRQ_VECTOR, &auto_test_req, 1); + auto_test_req &= DP_AUTOMATED_TEST_REQUEST; + + drm_dp_dpcd_read(aux, DP_TEST_REQUEST, &link_edid_read, 1); + link_edid_read &= DP_TEST_LINK_EDID_READ; + + if (!auto_test_req || !link_edid_read) { + DRM_DEBUG_KMS("Source DUT does not support TEST_EDID_READ\n"); + return false; + } + + drm_dp_dpcd_write(aux, DP_DEVICE_SERVICE_IRQ_VECTOR, &auto_test_req, 1); + + /* send back checksum for the last edid extension block data */ + drm_dp_dpcd_write(aux, DP_TEST_EDID_CHECKSUM, &real_edid_checksum, 1); + + test_resp |= DP_TEST_EDID_CHECKSUM_WRITE; + drm_dp_dpcd_write(aux, DP_TEST_RESPONSE, &test_resp, 1); + + return true; +} +EXPORT_SYMBOL(drm_dp_send_real_edid_checksum); + /** * drm_dp_downstream_max_clock() - extract branch device max * pixel rate for legacy VGA diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 5b33b7cfd645..0e35405ecc74 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -1450,11 +1450,22 @@ static int validate_displayid(u8 *displayid, int length, int idx); static int drm_edid_block_checksum(const u8 *raw_edid) { int i; - u8 csum = 0; - for (i = 0; i < EDID_LENGTH; i++) + u8 csum = 0, crc = 0; + + for (i = 0; i < EDID_LENGTH - 1; i++) csum += raw_edid[i]; - return csum; + crc = 0x100 - csum; + + return crc; +} + +static bool drm_edid_block_checksum_diff(const u8 *raw_edid, u8 real_checksum) +{ + if (raw_edid[EDID_LENGTH - 1] != real_checksum) + return true; + else + return false; } static bool drm_edid_is_zero(const u8 *in_edid, int length) @@ -1512,7 +1523,7 @@ bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid, } csum = drm_edid_block_checksum(raw_edid); - if (csum) { + if (drm_edid_block_checksum_diff(raw_edid, csum)) { if (edid_corrupt) *edid_corrupt = true; @@ -1653,6 +1664,7 @@ static void connector_bad_edid(struct drm_connector *connector, u8 *edid, int num_blocks) { int i; + u8 num_of_ext = edid[0x7e]; if (connector->bad_edid_counter++ && !drm_debug_enabled(DRM_UT_KMS)) return; @@ -1675,6 +1687,9 @@ static void connector_bad_edid(struct drm_connector *connector, prefix, DUMP_PREFIX_NONE, 16, 1, block, EDID_LENGTH, false); } + + /* Calculate real checksum for the last edid extension block data */ + connector->real_edid_checksum = drm_edid_block_checksum(edid + num_of_ext * EDID_LENGTH); } /* Get override or firmware EDID */ diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h index 5f8c3389d46f..86fc9f603fbc 100644 --- a/include/drm/drm_connector.h +++ b/include/drm/drm_connector.h @@ -1349,6 +1349,12 @@ struct drm_connector { * rev1.1 4.2.2.6 */ bool edid_corrupt; + /** + * @real_edid_checksum: real edid checksum value for corrupted edid block. + * Required in Displayport 1.4 compliance testing + * rev1.1 4.2.2.6 + */ + uint8_t real_edid_checksum; /** @debugfs_entry: debugfs directory for this connector */ struct dentry *debugfs_entry; diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h index 51ecb5112ef8..1f9bd688f90e 100644 --- a/include/drm/drm_dp_helper.h +++ b/include/drm/drm_dp_helper.h @@ -1455,6 +1455,9 @@ static inline ssize_t drm_dp_dpcd_writeb(struct drm_dp_aux *aux, int drm_dp_dpcd_read_link_status(struct drm_dp_aux *aux, u8 status[DP_LINK_STATUS_SIZE]); +bool drm_dp_send_real_edid_checksum(struct drm_dp_aux *aux, + u8 real_edid_checksum); + int drm_dp_downstream_max_clock(const u8 dpcd[DP_RECEIVER_CAP_SIZE], const u8 port_cap[4]); int drm_dp_downstream_max_bpc(const u8 dpcd[DP_RECEIVER_CAP_SIZE],
Unlike DP 1.2 edid corruption test, DP 1.4 requires to calculate real CRC value of the last edid data block, and write it back. Current edid CRC calculates routine adds the last CRC byte, and check if non-zero. This behavior is not accurate; actually, we need to return the actual CRC value when corruption is detected. This commit changes this issue by returning the calculated CRC, and initiate the required sequence. Change since v3 - Fix a minor typo. Change since v2 - Rewrite checksum computation routine to avoid duplicated code. - Rename to avoid confusion. Change since v1 - Have separate routine for returning real CRC. Signed-off-by: Jerry (Fangzhi) Zuo <Jerry.Zuo@amd.com> --- drivers/gpu/drm/drm_dp_helper.c | 35 +++++++++++++++++++++++++++++++++++ drivers/gpu/drm/drm_edid.c | 23 +++++++++++++++++++---- include/drm/drm_connector.h | 6 ++++++ include/drm/drm_dp_helper.h | 3 +++ 4 files changed, 63 insertions(+), 4 deletions(-)