Re: [PATCH 1/2] drm: Add support for DP 1.4 Compliance edid corruption test 4.2.2.6

2019-11-01 Thread Jani Nikula
On Wed, 30 Oct 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.
>
> Signed-off-by: Jerry (Fangzhi) Zuo 
> ---
>  drivers/gpu/drm/drm_dp_helper.c | 36 
>  drivers/gpu/drm/drm_edid.c  | 15 ---
>  include/drm/drm_connector.h |  7 +++
>  include/drm/drm_dp_helper.h |  3 +++
>  4 files changed, 58 insertions(+), 3 deletions(-)
>
> 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..400064dcc010 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -1344,13 +1344,19 @@ static void drm_get_displayid(struct drm_connector 
> *connector,
> struct edid *edid);
>  static int validate_displayid(u8 *displayid, int length, int idx);
>  
> -static int drm_edid_block_checksum(const u8 *raw_edid)
> +static int drm_edid_block_checksum(const u8 *raw_edid, bool c)
>  {
>   int i;
>   u8 csum = 0;
> - for (i = 0; i < EDID_LENGTH; i++)
> + u8 len;
> +
> + len = c ? EDID_LENGTH : (EDID_LENGTH - 1);
> +
> + for (i = 0; i < len; i++)
>   csum += raw_edid[i];
>  
> + csum = c ? csum : (0x100 - csum);
> +
>   return csum;
>  }

I think a mysterious boolean 'c' argument just makes this more
confusing.

Please split the function to two, one that calculates the checksum (c ==
false case) and another that uses the function (c == true case). The
latter would return the same value as the above.

Please do this as a prep patch without any of the other modifications.

BR,
Jani.



>  
> @@ -1408,7 +1414,7 @@ bool drm_edid_block_valid(u8 *raw_edid, int block, bool 
> print_bad_edid,
>   }
>   }
>  
> - csum = drm_edid_block_checksum(raw_edid);
> + csum = drm_edid_block_checksum(raw_edid, true);
>   if (csum) {
>   if (edid_corrupt)
>   *edid_corrupt = true;
> @@ -1572,6 +1578,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_checksum(edid + 
> edid[0x7e] * EDID_LENGTH, false);
>  }
>  
>  /* 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;
> 

[PATCH 1/2] drm: Add support for DP 1.4 Compliance edid corruption test 4.2.2.6

2019-10-30 Thread Jerry (Fangzhi) Zuo
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.

Signed-off-by: Jerry (Fangzhi) Zuo 
---
 drivers/gpu/drm/drm_dp_helper.c | 36 
 drivers/gpu/drm/drm_edid.c  | 15 ---
 include/drm/drm_connector.h |  7 +++
 include/drm/drm_dp_helper.h |  3 +++
 4 files changed, 58 insertions(+), 3 deletions(-)

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..400064dcc010 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -1344,13 +1344,19 @@ static void drm_get_displayid(struct drm_connector 
*connector,
  struct edid *edid);
 static int validate_displayid(u8 *displayid, int length, int idx);
 
-static int drm_edid_block_checksum(const u8 *raw_edid)
+static int drm_edid_block_checksum(const u8 *raw_edid, bool c)
 {
int i;
u8 csum = 0;
-   for (i = 0; i < EDID_LENGTH; i++)
+   u8 len;
+
+   len = c ? EDID_LENGTH : (EDID_LENGTH - 1);
+
+   for (i = 0; i < len; i++)
csum += raw_edid[i];
 
+   csum = c ? csum : (0x100 - csum);
+
return csum;
 }
 
@@ -1408,7 +1414,7 @@ bool drm_edid_block_valid(u8 *raw_edid, int block, bool 
print_bad_edid,
}
}
 
-   csum = drm_edid_block_checksum(raw_edid);
+   csum = drm_edid_block_checksum(raw_edid, true);
if (csum) {
if (edid_corrupt)
*edid_corrupt = true;
@@ -1572,6 +1578,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_checksum(edid + 
edid[0x7e] * EDID_LENGTH, false);
 }
 
 /* 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