Re: [PATCH v2 1/2] drm: Add support for DP 1.4 Compliance edid corruption test 4.2.2.6
On Fri, 01 Nov 2019, "Jerry (Fangzhi) Zuo" wrote:
> DP 1.4 edid corruption test requires source DUT to write calculated
> CRC, not the corrupted CRC from reference sink.
>
> Return the calculated CRC back, and initiate the required sequence.
>
> -v2: Have separate routine for returning real CRC
>
> Signed-off-by: Jerry (Fangzhi) Zuo
> ---
> drivers/gpu/drm/drm_dp_helper.c | 36
> drivers/gpu/drm/drm_edid.c | 14 ++
> include/drm/drm_connector.h | 7 +++
> include/drm/drm_dp_helper.h | 3 +++
> 4 files changed, 60 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
> index ffc68d305afe..75dbd30c62a7 100644
> --- a/drivers/gpu/drm/drm_dp_helper.c
> +++ b/drivers/gpu/drm/drm_dp_helper.c
> @@ -336,6 +336,42 @@ int drm_dp_dpcd_read_link_status(struct drm_dp_aux *aux,
> }
> EXPORT_SYMBOL(drm_dp_dpcd_read_link_status);
>
> +/**
> + * drm_dp_send_bad_edid_checksum() - send back real edid checksum value
> + * @aux: DisplayPort AUX channel
> + * @bad_edid_checksum: real edid checksum for the last block
> + *
> + * Returns true on success
> + */
> +bool drm_dp_send_bad_edid_checksum(struct drm_dp_aux *aux,
> +u8 bad_edid_checksum)
> +{
> +u8 link_edid_read = 0, auto_test_req = 0;
> +u8 test_resp = 0;
> +
> +drm_dp_dpcd_read(aux, DP_DEVICE_SERVICE_IRQ_VECTOR, _test_req,
> 1);
> +auto_test_req &= DP_AUTOMATED_TEST_REQUEST;
> +
> +drm_dp_dpcd_read(aux, DP_TEST_REQUEST, _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, _test_req,
> 1);
> +
> +/* send back checksum for the last edid extension block data */
> +drm_dp_dpcd_write(aux, DP_TEST_EDID_CHECKSUM, _edid_checksum, 1);
> +
> +test_resp |= DP_TEST_EDID_CHECKSUM_WRITE;
> +drm_dp_dpcd_write(aux, DP_TEST_RESPONSE, _resp, 1);
> +
> +return true;
> +}
> +EXPORT_SYMBOL(drm_dp_send_bad_edid_checksum);
> +
> /**
> * drm_dp_link_probe() - probe a DisplayPort link for capabilities
> * @aux: DisplayPort AUX channel
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 82a4ceed3fcf..0598314e3f46 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -1354,6 +1354,17 @@ static int drm_edid_block_checksum(const u8 *raw_edid)
> return csum;
> }
>
> +static int drm_edid_block_real_checksum(const u8 *raw_edid)
> +{
> + int i;
> + u8 csum = 0;
> +
> + for (i = 0; i < EDID_LENGTH - 1; i++)
> + csum += raw_edid[i];
> +
> + return (0x100 - csum);
Now you have two functions that have the loop to calculate checksums,
which is not at all what I tried to tell you to do.
I tried to suggest something like this:
static int drm_edid_block_checksum(const u8 *raw_edid)
{
int i;
u8 csum = 0;
for (i = 0; i < EDID_LENGTH - 1; i++)
csum += raw_edid[i];
return 0x100 - csum;
}
static int drm_edid_block_checksum_diff(const u8 *raw_edid)
{
u8 csum = drm_edid_block_checksum(raw_edid) + raw_edid[EDID_LENGTH - 1];
return csum;
}
Alternatively, you could have just the function to calculate the
checksum, and then the check is comparing the calculated checksum
against the checksum in the EDID.
BR,
Jani.
> +}
> +
> static bool drm_edid_is_zero(const u8 *in_edid, int length)
> {
> if (memchr_inv(in_edid, 0, length))
> @@ -1572,6 +1583,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->bad_edid_checksum = drm_edid_block_real_checksum(edid +
> edid[0x7e] * EDID_LENGTH);
> }
>
> /* Get override or firmware EDID */
> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> index 681cb590f952..8442461542b9 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -1345,6 +1345,13 @@ struct drm_connector {
>* rev1.1 4.2.2.6
>*/
> bool edid_corrupt;
> + /**
> + * @bad_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 bad_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 5a795075d5da..2a7e54bebb18 100644
> ---
Re: [PATCH v2 1/2] drm: Add support for DP 1.4 Compliance edid corruption test 4.2.2.6
On 2019-11-01 3:38 p.m., Jerry (Fangzhi) Zuo wrote:
> DP 1.4 edid corruption test requires source DUT to write calculated
> CRC, not the corrupted CRC from reference sink.
>
> Return the calculated CRC back, and initiate the required sequence.
>
> -v2: Have separate routine for returning real CRC
>
> Signed-off-by: Jerry (Fangzhi) Zuo
> ---
> drivers/gpu/drm/drm_dp_helper.c | 36
> drivers/gpu/drm/drm_edid.c | 14 ++
> include/drm/drm_connector.h | 7 +++
> include/drm/drm_dp_helper.h | 3 +++
> 4 files changed, 60 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
> index ffc68d305afe..75dbd30c62a7 100644
> --- a/drivers/gpu/drm/drm_dp_helper.c
> +++ b/drivers/gpu/drm/drm_dp_helper.c
> @@ -336,6 +336,42 @@ int drm_dp_dpcd_read_link_status(struct drm_dp_aux *aux,
> }
> EXPORT_SYMBOL(drm_dp_dpcd_read_link_status);
>
> +/**
> + * drm_dp_send_bad_edid_checksum() - send back real edid checksum value
> + * @aux: DisplayPort AUX channel
> + * @bad_edid_checksum: real edid checksum for the last block
> + *
> + * Returns true on success
> + */
> +bool drm_dp_send_bad_edid_checksum(struct drm_dp_aux *aux,
> +u8 bad_edid_checksum)
> +{
> +u8 link_edid_read = 0, auto_test_req = 0;
> +u8 test_resp = 0;
> +
> +drm_dp_dpcd_read(aux, DP_DEVICE_SERVICE_IRQ_VECTOR, _test_req,
> 1);
> +auto_test_req &= DP_AUTOMATED_TEST_REQUEST;
> +
> +drm_dp_dpcd_read(aux, DP_TEST_REQUEST, _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, _test_req,
> 1);
> +
> +/* send back checksum for the last edid extension block data */
> +drm_dp_dpcd_write(aux, DP_TEST_EDID_CHECKSUM, _edid_checksum, 1);
> +
> +test_resp |= DP_TEST_EDID_CHECKSUM_WRITE;
> +drm_dp_dpcd_write(aux, DP_TEST_RESPONSE, _resp, 1);
> +
> +return true;
> +}
> +EXPORT_SYMBOL(drm_dp_send_bad_edid_checksum);
> +
> /**
> * drm_dp_link_probe() - probe a DisplayPort link for capabilities
> * @aux: DisplayPort AUX channel
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 82a4ceed3fcf..0598314e3f46 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -1354,6 +1354,17 @@ static int drm_edid_block_checksum(const u8 *raw_edid)
> return csum;
> }
>
> +static int drm_edid_block_real_checksum(const u8 *raw_edid)
> +{
> + int i;
> + u8 csum = 0;
> +
> + for (i = 0; i < EDID_LENGTH - 1; i++)
> + csum += raw_edid[i];
> +
> + return (0x100 - csum);
> +}
> +
> static bool drm_edid_is_zero(const u8 *in_edid, int length)
> {
> if (memchr_inv(in_edid, 0, length))
> @@ -1572,6 +1583,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->bad_edid_checksum = drm_edid_block_real_checksum(edid +
> edid[0x7e] * EDID_LENGTH);
> }
>
> /* Get override or firmware EDID */
> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> index 681cb590f952..8442461542b9 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -1345,6 +1345,13 @@ struct drm_connector {
>* rev1.1 4.2.2.6
>*/
> bool edid_corrupt;
> + /**
> + * @bad_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 bad_edid_checksum;
This variable name confused me a bit. Maybe name this
"computed_edid_checksum" to clarify that this is the EDID checksum that
we've computed for the EDID, i.e. the correct one.
> +
>
> /** @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 5a795075d5da..2a7e54bebb18 100644
> --- a/include/drm/drm_dp_helper.h
> +++ b/include/drm/drm_dp_helper.h
> @@ -1383,6 +1383,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_bad_edid_checksum(struct drm_dp_aux *aux,
Same as before, might be good to name this
drm_dp_send_computed_edid_checksum or something similar.
Harry
> + u8 bad_edid_checksum);
> +
> /*
> * DisplayPort
[PATCH v2 1/2] drm: Add support for DP 1.4 Compliance edid corruption test 4.2.2.6
DP 1.4 edid corruption test requires source DUT to write calculated
CRC, not the corrupted CRC from reference sink.
Return the calculated CRC back, and initiate the required sequence.
-v2: Have separate routine for returning real CRC
Signed-off-by: Jerry (Fangzhi) Zuo
---
drivers/gpu/drm/drm_dp_helper.c | 36
drivers/gpu/drm/drm_edid.c | 14 ++
include/drm/drm_connector.h | 7 +++
include/drm/drm_dp_helper.h | 3 +++
4 files changed, 60 insertions(+)
diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index ffc68d305afe..75dbd30c62a7 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -336,6 +336,42 @@ int drm_dp_dpcd_read_link_status(struct drm_dp_aux *aux,
}
EXPORT_SYMBOL(drm_dp_dpcd_read_link_status);
+/**
+ * drm_dp_send_bad_edid_checksum() - send back real edid checksum value
+ * @aux: DisplayPort AUX channel
+ * @bad_edid_checksum: real edid checksum for the last block
+ *
+ * Returns true on success
+ */
+bool drm_dp_send_bad_edid_checksum(struct drm_dp_aux *aux,
+u8 bad_edid_checksum)
+{
+u8 link_edid_read = 0, auto_test_req = 0;
+u8 test_resp = 0;
+
+drm_dp_dpcd_read(aux, DP_DEVICE_SERVICE_IRQ_VECTOR, _test_req, 1);
+auto_test_req &= DP_AUTOMATED_TEST_REQUEST;
+
+drm_dp_dpcd_read(aux, DP_TEST_REQUEST, _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, _test_req,
1);
+
+/* send back checksum for the last edid extension block data */
+drm_dp_dpcd_write(aux, DP_TEST_EDID_CHECKSUM, _edid_checksum, 1);
+
+test_resp |= DP_TEST_EDID_CHECKSUM_WRITE;
+drm_dp_dpcd_write(aux, DP_TEST_RESPONSE, _resp, 1);
+
+return true;
+}
+EXPORT_SYMBOL(drm_dp_send_bad_edid_checksum);
+
/**
* drm_dp_link_probe() - probe a DisplayPort link for capabilities
* @aux: DisplayPort AUX channel
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 82a4ceed3fcf..0598314e3f46 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -1354,6 +1354,17 @@ static int drm_edid_block_checksum(const u8 *raw_edid)
return csum;
}
+static int drm_edid_block_real_checksum(const u8 *raw_edid)
+{
+ int i;
+ u8 csum = 0;
+
+ for (i = 0; i < EDID_LENGTH - 1; i++)
+ csum += raw_edid[i];
+
+ return (0x100 - csum);
+}
+
static bool drm_edid_is_zero(const u8 *in_edid, int length)
{
if (memchr_inv(in_edid, 0, length))
@@ -1572,6 +1583,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->bad_edid_checksum = drm_edid_block_real_checksum(edid +
edid[0x7e] * EDID_LENGTH);
}
/* Get override or firmware EDID */
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index 681cb590f952..8442461542b9 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -1345,6 +1345,13 @@ struct drm_connector {
* rev1.1 4.2.2.6
*/
bool edid_corrupt;
+ /**
+ * @bad_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 bad_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 5a795075d5da..2a7e54bebb18 100644
--- a/include/drm/drm_dp_helper.h
+++ b/include/drm/drm_dp_helper.h
@@ -1383,6 +1383,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_bad_edid_checksum(struct drm_dp_aux *aux,
+ u8 bad_edid_checksum);
+
/*
* DisplayPort link
*/
--
2.14.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
