* [PATCH 05/12] drm: Add supporting structure for Displayport Link CTS test 4.2.2.6
2015-04-15 15:38 ` [PATCH 05/10] drm: Add supporting structure for Displayport Link CTS test 4.2.2.6 Todd Previte
@ 2015-04-15 17:15 ` Todd Previte
2015-04-15 20:25 ` [Intel-gfx] " Paulo Zanoni
2015-04-15 22:03 ` Todd Previte
2015-04-16 15:47 ` Todd Previte
2 siblings, 1 reply; 13+ messages in thread
From: Todd Previte @ 2015-04-15 17:15 UTC (permalink / raw)
To: intel-gfx; +Cc: dri-devel
Displayport compliance test 4.2.2.6 requires that a source device be capable of
detecting a corrupt EDID. The test specification states that the sink device
sets up the EDID with an invalid checksum. To do this, the sink sets up an
invalid EDID header, expecting the source device to generate the checksum and
compare it to the value stored in the last byte of the block data.
Unfortunately, the DRM EDID reading and parsing functions are actually too good
in this case; the header is fixed before the checksum is computed and thus the
code never sees the invalid checksum. This results in a failure to pass the
compliance test.
To correct this issue, a checksum is generated when the EDID header is detected
as corrupted. If the checksum is invalid, it sets the header_corrupt flag and
logs the errors. In the case of a more seriously damaged header (fixup score
less than the threshold) the code does not generate the checksum but does set
the header_corrupt flag.
V2:
- Removed the static bool global
- Added a bool to the drm_connector struct to reaplce the static one for
holding the status of raw edid header corruption detection
- Modified the function signature of the is_valid function to take an
additional parameter to store the corruption detected value
- Fixed the other callers of the above is_valid function
V3:
- Updated the commit message to be more clear about what and why this
patch does what it does.
- Added comment in code to clarify the operations there
- Removed compliance variable and check_link_status update; those
have been moved to a later patch
- Removed variable assignment from the bottom of the test handler
V4:
- Removed i915 tag from subject line as the patch is not i915-specific
V5:
- Moved code causing a compilation error to this patch where the variable
is actually declared
- Maintained blank lines / spacing so as to not contaminate the patch
V6:
- Removed extra debug messages
- Added documentation to for the added parameter on drm_edid_block_valid
- Fixed more whitespace issues in check_link_status
- Added a clear of the header_corrupt flag to the end of the test handler
in intel_dp.c
- Changed the usage of the new function prototype in several places to use
NULL where it is not needed by compliance testing
V7:
- Updated to account for long_pulse flag propagation
Signed-off-by: Todd Previte <tprevite@gmail.com>
Cc: dri-devel@lists.freedesktop.org
---
drivers/gpu/drm/drm_edid.c | 30 ++++++++++++++++++++++++++----
drivers/gpu/drm/drm_edid_load.c | 7 +++++--
drivers/gpu/drm/i915/intel_dp.c | 6 +++++-
include/drm/drm_crtc.h | 8 +++++++-
4 files changed, 43 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 53bc7a6..1ed18f5 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -1041,13 +1041,15 @@ static bool drm_edid_is_zero(const u8 *in_edid, int length)
* @raw_edid: pointer to raw EDID block
* @block: type of block to validate (0 for base, extension otherwise)
* @print_bad_edid: if true, dump bad EDID blocks to the console
+ * @header_corrupt: if true, the header or checksum is invalid
*
* Validate a base or extension EDID block and optionally dump bad blocks to
* the console.
*
* Return: True if the block is valid, false otherwise.
*/
-bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid)
+bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid,
+ bool *header_corrupt)
{
u8 csum;
struct edid *edid = (struct edid *)raw_edid;
@@ -1062,9 +1064,25 @@ bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid)
int score = drm_edid_header_is_valid(raw_edid);
if (score == 8) ;
else if (score >= edid_fixup) {
+ /* Displayport Link CTS Core 1.2 rev1.1 test 4.2.2.6
+ * In order to properly generate the invalid checksum
+ * required for this test, it must be generated using
+ * the raw EDID data. Otherwise, the fix-up code here
+ * will correct the problem, the checksum is correct
+ * and the test fails
+ */
+ csum = drm_edid_block_checksum(raw_edid);
+ if (csum) {
+ if (header_corrupt)
+ *header_corrupt = 1;
+ }
DRM_DEBUG("Fixing EDID header, your hardware may be failing\n");
memcpy(raw_edid, edid_header, sizeof(edid_header));
} else {
+ if (header_corrupt) {
+ DRM_DEBUG_DRIVER("Invalid EDID header\n");
+ *header_corrupt = 1;
+ }
goto bad;
}
}
@@ -1129,7 +1147,7 @@ bool drm_edid_is_valid(struct edid *edid)
return false;
for (i = 0; i <= edid->extensions; i++)
- if (!drm_edid_block_valid(raw + i * EDID_LENGTH, i, true))
+ if (!drm_edid_block_valid(raw + i * EDID_LENGTH, i, true, NULL))
return false;
return true;
@@ -1232,7 +1250,8 @@ struct edid *drm_do_get_edid(struct drm_connector *connector,
for (i = 0; i < 4; i++) {
if (get_edid_block(data, block, 0, EDID_LENGTH))
goto out;
- if (drm_edid_block_valid(block, 0, print_bad_edid))
+ if (drm_edid_block_valid(block, 0, print_bad_edid,
+ &connector->edid_header_corrupt))
break;
if (i == 0 && drm_edid_is_zero(block, EDID_LENGTH)) {
connector->null_edid_counter++;
@@ -1257,7 +1276,10 @@ struct edid *drm_do_get_edid(struct drm_connector *connector,
block + (valid_extensions + 1) * EDID_LENGTH,
j, EDID_LENGTH))
goto out;
- if (drm_edid_block_valid(block + (valid_extensions + 1) * EDID_LENGTH, j, print_bad_edid)) {
+ if (drm_edid_block_valid(block + (valid_extensions + 1)
+ * EDID_LENGTH, j,
+ print_bad_edid,
+ NULL)) {
valid_extensions++;
break;
}
diff --git a/drivers/gpu/drm/drm_edid_load.c b/drivers/gpu/drm/drm_edid_load.c
index 4c0aa97..48b48e8 100644
--- a/drivers/gpu/drm/drm_edid_load.c
+++ b/drivers/gpu/drm/drm_edid_load.c
@@ -216,7 +216,8 @@ static void *edid_load(struct drm_connector *connector, const char *name,
goto out;
}
- if (!drm_edid_block_valid(edid, 0, print_bad_edid)) {
+ if (!drm_edid_block_valid(edid, 0, print_bad_edid,
+ &connector->edid_header_corrupt)) {
connector->bad_edid_counter++;
DRM_ERROR("Base block of EDID firmware \"%s\" is invalid ",
name);
@@ -229,7 +230,9 @@ static void *edid_load(struct drm_connector *connector, const char *name,
if (i != valid_extensions + 1)
memcpy(edid + (valid_extensions + 1) * EDID_LENGTH,
edid + i * EDID_LENGTH, EDID_LENGTH);
- if (drm_edid_block_valid(edid + i * EDID_LENGTH, i, print_bad_edid))
+ if (drm_edid_block_valid(edid + i * EDID_LENGTH, i,
+ print_bad_edid,
+ NULL))
valid_extensions++;
}
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index b120d59..ee41e10 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4005,6 +4005,7 @@ static uint8_t intel_dp_autotest_phy_pattern(struct intel_dp *intel_dp)
static void intel_dp_handle_test_request(struct intel_dp *intel_dp)
{
+ struct drm_connector *connector = &intel_dp->attached_connector->base;
uint8_t response = DP_TEST_NAK;
uint8_t rxdata = 0;
int status = 0;
@@ -4051,6 +4052,9 @@ update_status:
&response, 1);
if (status <= 0)
DRM_DEBUG_KMS("Could not write test response to sink\n");
+
+ /* Clear EDID corruption flag now */
+ connector->edid_header_corrupt = 0;
}
static int
@@ -4136,7 +4140,7 @@ intel_dp_check_link_status(struct intel_dp *intel_dp, bool long_hpd)
*/
if (long_hpd) {
edid_read = drm_get_edid(connector, adapter);
- if (!edid_read) {
+ if (!edid_read || connector->edid_header_corrupt == 1) {
DRM_DEBUG_DRIVER("Invalid EDID detected\n");
} else {
kfree(edid_read);
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index d4e4b82..f7a4a92 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -719,6 +719,11 @@ struct drm_connector {
int null_edid_counter; /* needed to workaround some HW bugs where we get all 0s */
unsigned bad_edid_counter;
+ /* Flag for raw EDID header corruption - used in Displayport
+ * compliance testing - * Displayport Link CTS Core 1.2 rev1.1 4.2.2.6
+ */
+ bool edid_header_corrupt;
+
struct dentry *debugfs_entry;
struct drm_connector_state *state;
@@ -1443,7 +1448,8 @@ extern void drm_set_preferred_mode(struct drm_connector *connector,
int hpref, int vpref);
extern int drm_edid_header_is_valid(const u8 *raw_edid);
-extern bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid);
+extern bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid,
+ bool *header_corrupt);
extern bool drm_edid_is_valid(struct edid *edid);
extern struct drm_tile_group *drm_mode_create_tile_group(struct drm_device *dev,
--
1.9.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [Intel-gfx] [PATCH 05/12] drm: Add supporting structure for Displayport Link CTS test 4.2.2.6
2015-04-15 17:15 ` [PATCH 05/12] " Todd Previte
@ 2015-04-15 20:25 ` Paulo Zanoni
2015-04-15 21:59 ` Todd Previte
0 siblings, 1 reply; 13+ messages in thread
From: Paulo Zanoni @ 2015-04-15 20:25 UTC (permalink / raw)
To: Todd Previte; +Cc: Intel Graphics Development, DRI Development
2015-04-15 14:15 GMT-03:00 Todd Previte <tprevite@gmail.com>:
> Displayport compliance test 4.2.2.6 requires that a source device be capable of
> detecting a corrupt EDID. The test specification states that the sink device
> sets up the EDID with an invalid checksum. To do this, the sink sets up an
> invalid EDID header, expecting the source device to generate the checksum and
> compare it to the value stored in the last byte of the block data.
>
> Unfortunately, the DRM EDID reading and parsing functions are actually too good
> in this case; the header is fixed before the checksum is computed and thus the
> code never sees the invalid checksum. This results in a failure to pass the
> compliance test.
>
> To correct this issue, a checksum is generated when the EDID header is detected
> as corrupted. If the checksum is invalid, it sets the header_corrupt flag and
> logs the errors. In the case of a more seriously damaged header (fixup score
> less than the threshold) the code does not generate the checksum but does set
> the header_corrupt flag.
>
> V2:
> - Removed the static bool global
> - Added a bool to the drm_connector struct to reaplce the static one for
> holding the status of raw edid header corruption detection
> - Modified the function signature of the is_valid function to take an
> additional parameter to store the corruption detected value
> - Fixed the other callers of the above is_valid function
> V3:
> - Updated the commit message to be more clear about what and why this
> patch does what it does.
> - Added comment in code to clarify the operations there
> - Removed compliance variable and check_link_status update; those
> have been moved to a later patch
> - Removed variable assignment from the bottom of the test handler
> V4:
> - Removed i915 tag from subject line as the patch is not i915-specific
> V5:
> - Moved code causing a compilation error to this patch where the variable
> is actually declared
> - Maintained blank lines / spacing so as to not contaminate the patch
> V6:
> - Removed extra debug messages
> - Added documentation to for the added parameter on drm_edid_block_valid
> - Fixed more whitespace issues in check_link_status
> - Added a clear of the header_corrupt flag to the end of the test handler
> in intel_dp.c
> - Changed the usage of the new function prototype in several places to use
> NULL where it is not needed by compliance testing
> V7:
> - Updated to account for long_pulse flag propagation
>
> Signed-off-by: Todd Previte <tprevite@gmail.com>
> Cc: dri-devel@lists.freedesktop.org
> ---
> drivers/gpu/drm/drm_edid.c | 30 ++++++++++++++++++++++++++----
> drivers/gpu/drm/drm_edid_load.c | 7 +++++--
> drivers/gpu/drm/i915/intel_dp.c | 6 +++++-
> include/drm/drm_crtc.h | 8 +++++++-
> 4 files changed, 43 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 53bc7a6..1ed18f5 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -1041,13 +1041,15 @@ static bool drm_edid_is_zero(const u8 *in_edid, int length)
> * @raw_edid: pointer to raw EDID block
> * @block: type of block to validate (0 for base, extension otherwise)
> * @print_bad_edid: if true, dump bad EDID blocks to the console
> + * @header_corrupt: if true, the header or checksum is invalid
> *
> * Validate a base or extension EDID block and optionally dump bad blocks to
> * the console.
> *
> * Return: True if the block is valid, false otherwise.
> */
> -bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid)
> +bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid,
> + bool *header_corrupt)
> {
> u8 csum;
> struct edid *edid = (struct edid *)raw_edid;
> @@ -1062,9 +1064,25 @@ bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid)
> int score = drm_edid_header_is_valid(raw_edid);
> if (score == 8) ;
> else if (score >= edid_fixup) {
> + /* Displayport Link CTS Core 1.2 rev1.1 test 4.2.2.6
> + * In order to properly generate the invalid checksum
> + * required for this test, it must be generated using
> + * the raw EDID data. Otherwise, the fix-up code here
> + * will correct the problem, the checksum is correct
> + * and the test fails
> + */
> + csum = drm_edid_block_checksum(raw_edid);
> + if (csum) {
> + if (header_corrupt)
> + *header_corrupt = 1;
> + }
> DRM_DEBUG("Fixing EDID header, your hardware may be failing\n");
> memcpy(raw_edid, edid_header, sizeof(edid_header));
> } else {
> + if (header_corrupt) {
> + DRM_DEBUG_DRIVER("Invalid EDID header\n");
Still using DRM_DEBUG_DRIVER. See include/drm/drmP.h for a description
of each macro. BTW, since we're just going to dump the whole EDID
anyway, the message is also not very informative.
> + *header_corrupt = 1;
> + }
> goto bad;
> }
> }
> @@ -1129,7 +1147,7 @@ bool drm_edid_is_valid(struct edid *edid)
> return false;
>
> for (i = 0; i <= edid->extensions; i++)
> - if (!drm_edid_block_valid(raw + i * EDID_LENGTH, i, true))
> + if (!drm_edid_block_valid(raw + i * EDID_LENGTH, i, true, NULL))
> return false;
>
> return true;
> @@ -1232,7 +1250,8 @@ struct edid *drm_do_get_edid(struct drm_connector *connector,
> for (i = 0; i < 4; i++) {
> if (get_edid_block(data, block, 0, EDID_LENGTH))
> goto out;
> - if (drm_edid_block_valid(block, 0, print_bad_edid))
> + if (drm_edid_block_valid(block, 0, print_bad_edid,
> + &connector->edid_header_corrupt))
> break;
> if (i == 0 && drm_edid_is_zero(block, EDID_LENGTH)) {
> connector->null_edid_counter++;
> @@ -1257,7 +1276,10 @@ struct edid *drm_do_get_edid(struct drm_connector *connector,
> block + (valid_extensions + 1) * EDID_LENGTH,
> j, EDID_LENGTH))
> goto out;
> - if (drm_edid_block_valid(block + (valid_extensions + 1) * EDID_LENGTH, j, print_bad_edid)) {
> + if (drm_edid_block_valid(block + (valid_extensions + 1)
> + * EDID_LENGTH, j,
> + print_bad_edid,
> + NULL)) {
> valid_extensions++;
> break;
> }
> diff --git a/drivers/gpu/drm/drm_edid_load.c b/drivers/gpu/drm/drm_edid_load.c
> index 4c0aa97..48b48e8 100644
> --- a/drivers/gpu/drm/drm_edid_load.c
> +++ b/drivers/gpu/drm/drm_edid_load.c
> @@ -216,7 +216,8 @@ static void *edid_load(struct drm_connector *connector, const char *name,
> goto out;
> }
>
> - if (!drm_edid_block_valid(edid, 0, print_bad_edid)) {
> + if (!drm_edid_block_valid(edid, 0, print_bad_edid,
> + &connector->edid_header_corrupt)) {
> connector->bad_edid_counter++;
> DRM_ERROR("Base block of EDID firmware \"%s\" is invalid ",
> name);
> @@ -229,7 +230,9 @@ static void *edid_load(struct drm_connector *connector, const char *name,
> if (i != valid_extensions + 1)
> memcpy(edid + (valid_extensions + 1) * EDID_LENGTH,
> edid + i * EDID_LENGTH, EDID_LENGTH);
> - if (drm_edid_block_valid(edid + i * EDID_LENGTH, i, print_bad_edid))
> + if (drm_edid_block_valid(edid + i * EDID_LENGTH, i,
> + print_bad_edid,
> + NULL))
> valid_extensions++;
> }
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index b120d59..ee41e10 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -4005,6 +4005,7 @@ static uint8_t intel_dp_autotest_phy_pattern(struct intel_dp *intel_dp)
>
> static void intel_dp_handle_test_request(struct intel_dp *intel_dp)
> {
> + struct drm_connector *connector = &intel_dp->attached_connector->base;
> uint8_t response = DP_TEST_NAK;
> uint8_t rxdata = 0;
> int status = 0;
> @@ -4051,6 +4052,9 @@ update_status:
> &response, 1);
> if (status <= 0)
> DRM_DEBUG_KMS("Could not write test response to sink\n");
> +
> + /* Clear EDID corruption flag now */
> + connector->edid_header_corrupt = 0;
I don't think it makes sense to clear this inside i915.ko. We're
adding the feature to drm.ko, so It makes sense to clear it there.
Isn't it possible to just clear it inside drm_edid_block_valid()?
Something like:
if (score == 8) {
if (header_corrupt)
*header_corrupt = 0;
} else if (score >= edid_fixup) {
... etc ...
}
}
If not, why?
> }
>
> static int
> @@ -4136,7 +4140,7 @@ intel_dp_check_link_status(struct intel_dp *intel_dp, bool long_hpd)
> */
> if (long_hpd) {
> edid_read = drm_get_edid(connector, adapter);
> - if (!edid_read) {
> + if (!edid_read || connector->edid_header_corrupt == 1) {
> DRM_DEBUG_DRIVER("Invalid EDID detected\n");
> } else {
> kfree(edid_read);
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index d4e4b82..f7a4a92 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -719,6 +719,11 @@ struct drm_connector {
> int null_edid_counter; /* needed to workaround some HW bugs where we get all 0s */
> unsigned bad_edid_counter;
>
> + /* Flag for raw EDID header corruption - used in Displayport
> + * compliance testing - * Displayport Link CTS Core 1.2 rev1.1 4.2.2.6
> + */
> + bool edid_header_corrupt;
> +
> struct dentry *debugfs_entry;
>
> struct drm_connector_state *state;
> @@ -1443,7 +1448,8 @@ extern void drm_set_preferred_mode(struct drm_connector *connector,
> int hpref, int vpref);
>
> extern int drm_edid_header_is_valid(const u8 *raw_edid);
> -extern bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid);
> +extern bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid,
> + bool *header_corrupt);
> extern bool drm_edid_is_valid(struct edid *edid);
>
> extern struct drm_tile_group *drm_mode_create_tile_group(struct drm_device *dev,
> --
> 1.9.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Paulo Zanoni
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 05/12] drm: Add supporting structure for Displayport Link CTS test 4.2.2.6
2015-04-15 20:25 ` [Intel-gfx] " Paulo Zanoni
@ 2015-04-15 21:59 ` Todd Previte
0 siblings, 0 replies; 13+ messages in thread
From: Todd Previte @ 2015-04-15 21:59 UTC (permalink / raw)
To: Paulo Zanoni; +Cc: Intel Graphics Development, DRI Development
On 4/15/2015 1:25 PM, Paulo Zanoni wrote:
> 2015-04-15 14:15 GMT-03:00 Todd Previte <tprevite@gmail.com>:
>> Displayport compliance test 4.2.2.6 requires that a source device be capable of
>> detecting a corrupt EDID. The test specification states that the sink device
>> sets up the EDID with an invalid checksum. To do this, the sink sets up an
>> invalid EDID header, expecting the source device to generate the checksum and
>> compare it to the value stored in the last byte of the block data.
>>
>> Unfortunately, the DRM EDID reading and parsing functions are actually too good
>> in this case; the header is fixed before the checksum is computed and thus the
>> code never sees the invalid checksum. This results in a failure to pass the
>> compliance test.
>>
>> To correct this issue, a checksum is generated when the EDID header is detected
>> as corrupted. If the checksum is invalid, it sets the header_corrupt flag and
>> logs the errors. In the case of a more seriously damaged header (fixup score
>> less than the threshold) the code does not generate the checksum but does set
>> the header_corrupt flag.
>>
>> V2:
>> - Removed the static bool global
>> - Added a bool to the drm_connector struct to reaplce the static one for
>> holding the status of raw edid header corruption detection
>> - Modified the function signature of the is_valid function to take an
>> additional parameter to store the corruption detected value
>> - Fixed the other callers of the above is_valid function
>> V3:
>> - Updated the commit message to be more clear about what and why this
>> patch does what it does.
>> - Added comment in code to clarify the operations there
>> - Removed compliance variable and check_link_status update; those
>> have been moved to a later patch
>> - Removed variable assignment from the bottom of the test handler
>> V4:
>> - Removed i915 tag from subject line as the patch is not i915-specific
>> V5:
>> - Moved code causing a compilation error to this patch where the variable
>> is actually declared
>> - Maintained blank lines / spacing so as to not contaminate the patch
>> V6:
>> - Removed extra debug messages
>> - Added documentation to for the added parameter on drm_edid_block_valid
>> - Fixed more whitespace issues in check_link_status
>> - Added a clear of the header_corrupt flag to the end of the test handler
>> in intel_dp.c
>> - Changed the usage of the new function prototype in several places to use
>> NULL where it is not needed by compliance testing
>> V7:
>> - Updated to account for long_pulse flag propagation
>>
>> Signed-off-by: Todd Previte <tprevite@gmail.com>
>> Cc: dri-devel@lists.freedesktop.org
>> ---
>> drivers/gpu/drm/drm_edid.c | 30 ++++++++++++++++++++++++++----
>> drivers/gpu/drm/drm_edid_load.c | 7 +++++--
>> drivers/gpu/drm/i915/intel_dp.c | 6 +++++-
>> include/drm/drm_crtc.h | 8 +++++++-
>> 4 files changed, 43 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
>> index 53bc7a6..1ed18f5 100644
>> --- a/drivers/gpu/drm/drm_edid.c
>> +++ b/drivers/gpu/drm/drm_edid.c
>> @@ -1041,13 +1041,15 @@ static bool drm_edid_is_zero(const u8 *in_edid, int length)
>> * @raw_edid: pointer to raw EDID block
>> * @block: type of block to validate (0 for base, extension otherwise)
>> * @print_bad_edid: if true, dump bad EDID blocks to the console
>> + * @header_corrupt: if true, the header or checksum is invalid
>> *
>> * Validate a base or extension EDID block and optionally dump bad blocks to
>> * the console.
>> *
>> * Return: True if the block is valid, false otherwise.
>> */
>> -bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid)
>> +bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid,
>> + bool *header_corrupt)
>> {
>> u8 csum;
>> struct edid *edid = (struct edid *)raw_edid;
>> @@ -1062,9 +1064,25 @@ bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid)
>> int score = drm_edid_header_is_valid(raw_edid);
>> if (score == 8) ;
>> else if (score >= edid_fixup) {
>> + /* Displayport Link CTS Core 1.2 rev1.1 test 4.2.2.6
>> + * In order to properly generate the invalid checksum
>> + * required for this test, it must be generated using
>> + * the raw EDID data. Otherwise, the fix-up code here
>> + * will correct the problem, the checksum is correct
>> + * and the test fails
>> + */
>> + csum = drm_edid_block_checksum(raw_edid);
>> + if (csum) {
>> + if (header_corrupt)
>> + *header_corrupt = 1;
>> + }
>> DRM_DEBUG("Fixing EDID header, your hardware may be failing\n");
>> memcpy(raw_edid, edid_header, sizeof(edid_header));
>> } else {
>> + if (header_corrupt) {
>> + DRM_DEBUG_DRIVER("Invalid EDID header\n");
> Still using DRM_DEBUG_DRIVER. See include/drm/drmP.h for a description
> of each macro. BTW, since we're just going to dump the whole EDID
> anyway, the message is also not very informative.
Fair enough. Removed for the next patch.
>
>> + *header_corrupt = 1;
>> + }
>> goto bad;
>> }
>> }
>> @@ -1129,7 +1147,7 @@ bool drm_edid_is_valid(struct edid *edid)
>> return false;
>>
>> for (i = 0; i <= edid->extensions; i++)
>> - if (!drm_edid_block_valid(raw + i * EDID_LENGTH, i, true))
>> + if (!drm_edid_block_valid(raw + i * EDID_LENGTH, i, true, NULL))
>> return false;
>>
>> return true;
>> @@ -1232,7 +1250,8 @@ struct edid *drm_do_get_edid(struct drm_connector *connector,
>> for (i = 0; i < 4; i++) {
>> if (get_edid_block(data, block, 0, EDID_LENGTH))
>> goto out;
>> - if (drm_edid_block_valid(block, 0, print_bad_edid))
>> + if (drm_edid_block_valid(block, 0, print_bad_edid,
>> + &connector->edid_header_corrupt))
>> break;
>> if (i == 0 && drm_edid_is_zero(block, EDID_LENGTH)) {
>> connector->null_edid_counter++;
>> @@ -1257,7 +1276,10 @@ struct edid *drm_do_get_edid(struct drm_connector *connector,
>> block + (valid_extensions + 1) * EDID_LENGTH,
>> j, EDID_LENGTH))
>> goto out;
>> - if (drm_edid_block_valid(block + (valid_extensions + 1) * EDID_LENGTH, j, print_bad_edid)) {
>> + if (drm_edid_block_valid(block + (valid_extensions + 1)
>> + * EDID_LENGTH, j,
>> + print_bad_edid,
>> + NULL)) {
>> valid_extensions++;
>> break;
>> }
>> diff --git a/drivers/gpu/drm/drm_edid_load.c b/drivers/gpu/drm/drm_edid_load.c
>> index 4c0aa97..48b48e8 100644
>> --- a/drivers/gpu/drm/drm_edid_load.c
>> +++ b/drivers/gpu/drm/drm_edid_load.c
>> @@ -216,7 +216,8 @@ static void *edid_load(struct drm_connector *connector, const char *name,
>> goto out;
>> }
>>
>> - if (!drm_edid_block_valid(edid, 0, print_bad_edid)) {
>> + if (!drm_edid_block_valid(edid, 0, print_bad_edid,
>> + &connector->edid_header_corrupt)) {
>> connector->bad_edid_counter++;
>> DRM_ERROR("Base block of EDID firmware \"%s\" is invalid ",
>> name);
>> @@ -229,7 +230,9 @@ static void *edid_load(struct drm_connector *connector, const char *name,
>> if (i != valid_extensions + 1)
>> memcpy(edid + (valid_extensions + 1) * EDID_LENGTH,
>> edid + i * EDID_LENGTH, EDID_LENGTH);
>> - if (drm_edid_block_valid(edid + i * EDID_LENGTH, i, print_bad_edid))
>> + if (drm_edid_block_valid(edid + i * EDID_LENGTH, i,
>> + print_bad_edid,
>> + NULL))
>> valid_extensions++;
>> }
>>
>> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>> index b120d59..ee41e10 100644
>> --- a/drivers/gpu/drm/i915/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>> @@ -4005,6 +4005,7 @@ static uint8_t intel_dp_autotest_phy_pattern(struct intel_dp *intel_dp)
>>
>> static void intel_dp_handle_test_request(struct intel_dp *intel_dp)
>> {
>> + struct drm_connector *connector = &intel_dp->attached_connector->base;
>> uint8_t response = DP_TEST_NAK;
>> uint8_t rxdata = 0;
>> int status = 0;
>> @@ -4051,6 +4052,9 @@ update_status:
>> &response, 1);
>> if (status <= 0)
>> DRM_DEBUG_KMS("Could not write test response to sink\n");
>> +
>> + /* Clear EDID corruption flag now */
>> + connector->edid_header_corrupt = 0;
> I don't think it makes sense to clear this inside i915.ko. We're
> adding the feature to drm.ko, so It makes sense to clear it there.
Good point, that makes a lot of sense. Clearing the flag in the DRM
module is the best place for it. Thanks Paulo.
> Isn't it possible to just clear it inside drm_edid_block_valid()?
> Something like:
>
> if (score == 8) {
> if (header_corrupt)
> *header_corrupt = 0;
> } else if (score >= edid_fixup) {
> ... etc ...
> }
> }
That seems the most logical place for it. Doesn't seem to cause any
issues with the tests so this works for me. Updated patch will be out
shortly.
> If not, why?
As I said above, the only issue would be if it gets cleared before the
test handlers see it, since they're the ones that care about it. But it
doesn't so it's a non-issue. :)
>
>> }
>>
>> static int
>> @@ -4136,7 +4140,7 @@ intel_dp_check_link_status(struct intel_dp *intel_dp, bool long_hpd)
>> */
>> if (long_hpd) {
>> edid_read = drm_get_edid(connector, adapter);
>> - if (!edid_read) {
>> + if (!edid_read || connector->edid_header_corrupt == 1) {
>> DRM_DEBUG_DRIVER("Invalid EDID detected\n");
>> } else {
>> kfree(edid_read);
>> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
>> index d4e4b82..f7a4a92 100644
>> --- a/include/drm/drm_crtc.h
>> +++ b/include/drm/drm_crtc.h
>> @@ -719,6 +719,11 @@ struct drm_connector {
>> int null_edid_counter; /* needed to workaround some HW bugs where we get all 0s */
>> unsigned bad_edid_counter;
>>
>> + /* Flag for raw EDID header corruption - used in Displayport
>> + * compliance testing - * Displayport Link CTS Core 1.2 rev1.1 4.2.2.6
>> + */
>> + bool edid_header_corrupt;
>> +
>> struct dentry *debugfs_entry;
>>
>> struct drm_connector_state *state;
>> @@ -1443,7 +1448,8 @@ extern void drm_set_preferred_mode(struct drm_connector *connector,
>> int hpref, int vpref);
>>
>> extern int drm_edid_header_is_valid(const u8 *raw_edid);
>> -extern bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid);
>> +extern bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid,
>> + bool *header_corrupt);
>> extern bool drm_edid_is_valid(struct edid *edid);
>>
>> extern struct drm_tile_group *drm_mode_create_tile_group(struct drm_device *dev,
>> --
>> 1.9.1
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 05/12] drm: Add supporting structure for Displayport Link CTS test 4.2.2.6
2015-04-15 15:38 ` [PATCH 05/10] drm: Add supporting structure for Displayport Link CTS test 4.2.2.6 Todd Previte
2015-04-15 17:15 ` [PATCH 05/12] " Todd Previte
@ 2015-04-15 22:03 ` Todd Previte
2015-04-16 13:34 ` Paulo Zanoni
2015-04-16 15:47 ` Todd Previte
2 siblings, 1 reply; 13+ messages in thread
From: Todd Previte @ 2015-04-15 22:03 UTC (permalink / raw)
To: intel-gfx; +Cc: dri-devel
Displayport compliance test 4.2.2.6 requires that a source device be capable of
detecting a corrupt EDID. The test specification states that the sink device
sets up the EDID with an invalid checksum. To do this, the sink sets up an
invalid EDID header, expecting the source device to generate the checksum and
compare it to the value stored in the last byte of the block data.
Unfortunately, the DRM EDID reading and parsing functions are actually too good
in this case; the header is fixed before the checksum is computed and thus the
code never sees the invalid checksum. This results in a failure to pass the
compliance test.
To correct this issue, a checksum is generated when the EDID header is detected
as corrupted. If the checksum is invalid, it sets the header_corrupt flag and
logs the errors. In the case of a more seriously damaged header (fixup score
less than the threshold) the code does not generate the checksum but does set
the header_corrupt flag.
V2:
- Removed the static bool global
- Added a bool to the drm_connector struct to reaplce the static one for
holding the status of raw edid header corruption detection
- Modified the function signature of the is_valid function to take an
additional parameter to store the corruption detected value
- Fixed the other callers of the above is_valid function
V3:
- Updated the commit message to be more clear about what and why this
patch does what it does.
- Added comment in code to clarify the operations there
- Removed compliance variable and check_link_status update; those
have been moved to a later patch
- Removed variable assignment from the bottom of the test handler
V4:
- Removed i915 tag from subject line as the patch is not i915-specific
V5:
- Moved code causing a compilation error to this patch where the variable
is actually declared
- Maintained blank lines / spacing so as to not contaminate the patch
V6:
- Removed extra debug messages
- Added documentation to for the added parameter on drm_edid_block_valid
- Fixed more whitespace issues in check_link_status
- Added a clear of the header_corrupt flag to the end of the test handler
in intel_dp.c
- Changed the usage of the new function prototype in several places to use
NULL where it is not needed by compliance testing
V7:
- Updated to account for long_pulse flag propagation
V8:
- Removed clearing of header_corrupt flag from the test handler in intel_dp.c
- Added clearing of header_corrupt flag in the drm_edid_block_valid function
Signed-off-by: Todd Previte <tprevite@gmail.com>
Cc: dri-devel@lists.freedesktop.org
---
drivers/gpu/drm/drm_edid.c | 35 +++++++++++++++++++++++++++++------
drivers/gpu/drm/drm_edid_load.c | 7 +++++--
drivers/gpu/drm/i915/intel_dp.c | 2 +-
include/drm/drm_crtc.h | 8 +++++++-
4 files changed, 42 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 53bc7a6..6d037f9 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -1041,13 +1041,15 @@ static bool drm_edid_is_zero(const u8 *in_edid, int length)
* @raw_edid: pointer to raw EDID block
* @block: type of block to validate (0 for base, extension otherwise)
* @print_bad_edid: if true, dump bad EDID blocks to the console
+ * @header_corrupt: if true, the header or checksum is invalid
*
* Validate a base or extension EDID block and optionally dump bad blocks to
* the console.
*
* Return: True if the block is valid, false otherwise.
*/
-bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid)
+bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid,
+ bool *header_corrupt)
{
u8 csum;
struct edid *edid = (struct edid *)raw_edid;
@@ -1060,11 +1062,28 @@ bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid)
if (block == 0) {
int score = drm_edid_header_is_valid(raw_edid);
- if (score == 8) ;
- else if (score >= edid_fixup) {
+ if (score == 8) {
+ if (header_corrupt)
+ *header_corrupt = 0;
+ } else if (score >= edid_fixup) {
+ /* Displayport Link CTS Core 1.2 rev1.1 test 4.2.2.6
+ * In order to properly generate the invalid checksum
+ * required for this test, it must be generated using
+ * the raw EDID data. Otherwise, the fix-up code here
+ * will correct the problem, the checksum is correct
+ * and the test fails
+ */
+ csum = drm_edid_block_checksum(raw_edid);
+ if (csum) {
+ if (header_corrupt)
+ *header_corrupt = 1;
+ }
DRM_DEBUG("Fixing EDID header, your hardware may be failing\n");
memcpy(raw_edid, edid_header, sizeof(edid_header));
} else {
+ if (header_corrupt) {
+ *header_corrupt = 1;
+ }
goto bad;
}
}
@@ -1129,7 +1148,7 @@ bool drm_edid_is_valid(struct edid *edid)
return false;
for (i = 0; i <= edid->extensions; i++)
- if (!drm_edid_block_valid(raw + i * EDID_LENGTH, i, true))
+ if (!drm_edid_block_valid(raw + i * EDID_LENGTH, i, true, NULL))
return false;
return true;
@@ -1232,7 +1251,8 @@ struct edid *drm_do_get_edid(struct drm_connector *connector,
for (i = 0; i < 4; i++) {
if (get_edid_block(data, block, 0, EDID_LENGTH))
goto out;
- if (drm_edid_block_valid(block, 0, print_bad_edid))
+ if (drm_edid_block_valid(block, 0, print_bad_edid,
+ &connector->edid_header_corrupt))
break;
if (i == 0 && drm_edid_is_zero(block, EDID_LENGTH)) {
connector->null_edid_counter++;
@@ -1257,7 +1277,10 @@ struct edid *drm_do_get_edid(struct drm_connector *connector,
block + (valid_extensions + 1) * EDID_LENGTH,
j, EDID_LENGTH))
goto out;
- if (drm_edid_block_valid(block + (valid_extensions + 1) * EDID_LENGTH, j, print_bad_edid)) {
+ if (drm_edid_block_valid(block + (valid_extensions + 1)
+ * EDID_LENGTH, j,
+ print_bad_edid,
+ NULL)) {
valid_extensions++;
break;
}
diff --git a/drivers/gpu/drm/drm_edid_load.c b/drivers/gpu/drm/drm_edid_load.c
index 4c0aa97..48b48e8 100644
--- a/drivers/gpu/drm/drm_edid_load.c
+++ b/drivers/gpu/drm/drm_edid_load.c
@@ -216,7 +216,8 @@ static void *edid_load(struct drm_connector *connector, const char *name,
goto out;
}
- if (!drm_edid_block_valid(edid, 0, print_bad_edid)) {
+ if (!drm_edid_block_valid(edid, 0, print_bad_edid,
+ &connector->edid_header_corrupt)) {
connector->bad_edid_counter++;
DRM_ERROR("Base block of EDID firmware \"%s\" is invalid ",
name);
@@ -229,7 +230,9 @@ static void *edid_load(struct drm_connector *connector, const char *name,
if (i != valid_extensions + 1)
memcpy(edid + (valid_extensions + 1) * EDID_LENGTH,
edid + i * EDID_LENGTH, EDID_LENGTH);
- if (drm_edid_block_valid(edid + i * EDID_LENGTH, i, print_bad_edid))
+ if (drm_edid_block_valid(edid + i * EDID_LENGTH, i,
+ print_bad_edid,
+ NULL))
valid_extensions++;
}
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 7ca0e18..97224ff 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4139,7 +4139,7 @@ intel_dp_check_link_status(struct intel_dp *intel_dp, bool long_hpd)
*/
if (long_hpd) {
edid_read = drm_get_edid(connector, adapter);
- if (!edid_read) {
+ if (!edid_read || connector->edid_header_corrupt == 1) {
DRM_DEBUG_DRIVER("Invalid EDID detected\n");
} else {
kfree(edid_read);
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index d4e4b82..f7a4a92 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -719,6 +719,11 @@ struct drm_connector {
int null_edid_counter; /* needed to workaround some HW bugs where we get all 0s */
unsigned bad_edid_counter;
+ /* Flag for raw EDID header corruption - used in Displayport
+ * compliance testing - * Displayport Link CTS Core 1.2 rev1.1 4.2.2.6
+ */
+ bool edid_header_corrupt;
+
struct dentry *debugfs_entry;
struct drm_connector_state *state;
@@ -1443,7 +1448,8 @@ extern void drm_set_preferred_mode(struct drm_connector *connector,
int hpref, int vpref);
extern int drm_edid_header_is_valid(const u8 *raw_edid);
-extern bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid);
+extern bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid,
+ bool *header_corrupt);
extern bool drm_edid_is_valid(struct edid *edid);
extern struct drm_tile_group *drm_mode_create_tile_group(struct drm_device *dev,
--
1.9.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH 05/12] drm: Add supporting structure for Displayport Link CTS test 4.2.2.6
2015-04-15 22:03 ` Todd Previte
@ 2015-04-16 13:34 ` Paulo Zanoni
2015-04-16 14:02 ` Todd Previte
0 siblings, 1 reply; 13+ messages in thread
From: Paulo Zanoni @ 2015-04-16 13:34 UTC (permalink / raw)
To: Todd Previte; +Cc: Intel Graphics Development, DRI Development
2015-04-15 19:03 GMT-03:00 Todd Previte <tprevite@gmail.com>:
> Displayport compliance test 4.2.2.6 requires that a source device be capable of
> detecting a corrupt EDID. The test specification states that the sink device
> sets up the EDID with an invalid checksum. To do this, the sink sets up an
> invalid EDID header, expecting the source device to generate the checksum and
> compare it to the value stored in the last byte of the block data.
>
> Unfortunately, the DRM EDID reading and parsing functions are actually too good
> in this case; the header is fixed before the checksum is computed and thus the
> code never sees the invalid checksum. This results in a failure to pass the
> compliance test.
>
> To correct this issue, a checksum is generated when the EDID header is detected
> as corrupted. If the checksum is invalid, it sets the header_corrupt flag and
> logs the errors. In the case of a more seriously damaged header (fixup score
> less than the threshold) the code does not generate the checksum but does set
> the header_corrupt flag.
>
> V2:
> - Removed the static bool global
> - Added a bool to the drm_connector struct to reaplce the static one for
> holding the status of raw edid header corruption detection
> - Modified the function signature of the is_valid function to take an
> additional parameter to store the corruption detected value
> - Fixed the other callers of the above is_valid function
> V3:
> - Updated the commit message to be more clear about what and why this
> patch does what it does.
> - Added comment in code to clarify the operations there
> - Removed compliance variable and check_link_status update; those
> have been moved to a later patch
> - Removed variable assignment from the bottom of the test handler
> V4:
> - Removed i915 tag from subject line as the patch is not i915-specific
> V5:
> - Moved code causing a compilation error to this patch where the variable
> is actually declared
> - Maintained blank lines / spacing so as to not contaminate the patch
> V6:
> - Removed extra debug messages
> - Added documentation to for the added parameter on drm_edid_block_valid
> - Fixed more whitespace issues in check_link_status
> - Added a clear of the header_corrupt flag to the end of the test handler
> in intel_dp.c
> - Changed the usage of the new function prototype in several places to use
> NULL where it is not needed by compliance testing
> V7:
> - Updated to account for long_pulse flag propagation
> V8:
> - Removed clearing of header_corrupt flag from the test handler in intel_dp.c
> - Added clearing of header_corrupt flag in the drm_edid_block_valid function
>
> Signed-off-by: Todd Previte <tprevite@gmail.com>
> Cc: dri-devel@lists.freedesktop.org
> ---
> drivers/gpu/drm/drm_edid.c | 35 +++++++++++++++++++++++++++++------
> drivers/gpu/drm/drm_edid_load.c | 7 +++++--
> drivers/gpu/drm/i915/intel_dp.c | 2 +-
> include/drm/drm_crtc.h | 8 +++++++-
> 4 files changed, 42 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 53bc7a6..6d037f9 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -1041,13 +1041,15 @@ static bool drm_edid_is_zero(const u8 *in_edid, int length)
> * @raw_edid: pointer to raw EDID block
> * @block: type of block to validate (0 for base, extension otherwise)
> * @print_bad_edid: if true, dump bad EDID blocks to the console
> + * @header_corrupt: if true, the header or checksum is invalid
> *
> * Validate a base or extension EDID block and optionally dump bad blocks to
> * the console.
> *
> * Return: True if the block is valid, false otherwise.
> */
> -bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid)
> +bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid,
> + bool *header_corrupt)
> {
> u8 csum;
> struct edid *edid = (struct edid *)raw_edid;
> @@ -1060,11 +1062,28 @@ bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid)
>
> if (block == 0) {
> int score = drm_edid_header_is_valid(raw_edid);
> - if (score == 8) ;
> - else if (score >= edid_fixup) {
> + if (score == 8) {
> + if (header_corrupt)
> + *header_corrupt = 0;
> + } else if (score >= edid_fixup) {
> + /* Displayport Link CTS Core 1.2 rev1.1 test 4.2.2.6
> + * In order to properly generate the invalid checksum
> + * required for this test, it must be generated using
> + * the raw EDID data. Otherwise, the fix-up code here
> + * will correct the problem, the checksum is correct
> + * and the test fails
> + */
> + csum = drm_edid_block_checksum(raw_edid);
> + if (csum) {
> + if (header_corrupt)
> + *header_corrupt = 1;
> + }
I was ready to give the R-B tag, but then I realized this. Shouldn't
we be setting header_corrupt here regardless of the checksum value? I
mean, score is not 8, so the header is indeed corrupt.
Also, setting "header_corrupt" to 1 in case the checksum is invalid is
somewhat a distortion of meaning in the variable. If we really need
the checksum, we should probably have something like
connector->raw_checksum, or maybe rename header_corrupt to
edid_corrupt, where "edid_corrupt" means "either invalid header or
invalid checksum"?
And for compliance testing specifically, as far as I understand, I see
on patch 7 that you only care about the checksum if the header is
valid, so we don't seem to need the csum value here, so it seems my
suggestion to just assign header_corrupt=1 regardless of the checksum
value seems to be enough to solve your problem.
Sorry for not realizing this earlier, the back-and-forth between
patches royally confused me.
> DRM_DEBUG("Fixing EDID header, your hardware may be failing\n");
> memcpy(raw_edid, edid_header, sizeof(edid_header));
> } else {
> + if (header_corrupt) {
> + *header_corrupt = 1;
> + }
> goto bad;
> }
> }
> @@ -1129,7 +1148,7 @@ bool drm_edid_is_valid(struct edid *edid)
> return false;
>
> for (i = 0; i <= edid->extensions; i++)
> - if (!drm_edid_block_valid(raw + i * EDID_LENGTH, i, true))
> + if (!drm_edid_block_valid(raw + i * EDID_LENGTH, i, true, NULL))
> return false;
>
> return true;
> @@ -1232,7 +1251,8 @@ struct edid *drm_do_get_edid(struct drm_connector *connector,
> for (i = 0; i < 4; i++) {
> if (get_edid_block(data, block, 0, EDID_LENGTH))
> goto out;
> - if (drm_edid_block_valid(block, 0, print_bad_edid))
> + if (drm_edid_block_valid(block, 0, print_bad_edid,
> + &connector->edid_header_corrupt))
> break;
> if (i == 0 && drm_edid_is_zero(block, EDID_LENGTH)) {
> connector->null_edid_counter++;
> @@ -1257,7 +1277,10 @@ struct edid *drm_do_get_edid(struct drm_connector *connector,
> block + (valid_extensions + 1) * EDID_LENGTH,
> j, EDID_LENGTH))
> goto out;
> - if (drm_edid_block_valid(block + (valid_extensions + 1) * EDID_LENGTH, j, print_bad_edid)) {
> + if (drm_edid_block_valid(block + (valid_extensions + 1)
> + * EDID_LENGTH, j,
> + print_bad_edid,
> + NULL)) {
> valid_extensions++;
> break;
> }
> diff --git a/drivers/gpu/drm/drm_edid_load.c b/drivers/gpu/drm/drm_edid_load.c
> index 4c0aa97..48b48e8 100644
> --- a/drivers/gpu/drm/drm_edid_load.c
> +++ b/drivers/gpu/drm/drm_edid_load.c
> @@ -216,7 +216,8 @@ static void *edid_load(struct drm_connector *connector, const char *name,
> goto out;
> }
>
> - if (!drm_edid_block_valid(edid, 0, print_bad_edid)) {
> + if (!drm_edid_block_valid(edid, 0, print_bad_edid,
> + &connector->edid_header_corrupt)) {
> connector->bad_edid_counter++;
> DRM_ERROR("Base block of EDID firmware \"%s\" is invalid ",
> name);
> @@ -229,7 +230,9 @@ static void *edid_load(struct drm_connector *connector, const char *name,
> if (i != valid_extensions + 1)
> memcpy(edid + (valid_extensions + 1) * EDID_LENGTH,
> edid + i * EDID_LENGTH, EDID_LENGTH);
> - if (drm_edid_block_valid(edid + i * EDID_LENGTH, i, print_bad_edid))
> + if (drm_edid_block_valid(edid + i * EDID_LENGTH, i,
> + print_bad_edid,
> + NULL))
> valid_extensions++;
> }
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 7ca0e18..97224ff 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -4139,7 +4139,7 @@ intel_dp_check_link_status(struct intel_dp *intel_dp, bool long_hpd)
> */
> if (long_hpd) {
> edid_read = drm_get_edid(connector, adapter);
> - if (!edid_read) {
> + if (!edid_read || connector->edid_header_corrupt == 1) {
> DRM_DEBUG_DRIVER("Invalid EDID detected\n");
> } else {
> kfree(edid_read);
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index d4e4b82..f7a4a92 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -719,6 +719,11 @@ struct drm_connector {
> int null_edid_counter; /* needed to workaround some HW bugs where we get all 0s */
> unsigned bad_edid_counter;
>
> + /* Flag for raw EDID header corruption - used in Displayport
> + * compliance testing - * Displayport Link CTS Core 1.2 rev1.1 4.2.2.6
> + */
> + bool edid_header_corrupt;
> +
> struct dentry *debugfs_entry;
>
> struct drm_connector_state *state;
> @@ -1443,7 +1448,8 @@ extern void drm_set_preferred_mode(struct drm_connector *connector,
> int hpref, int vpref);
>
> extern int drm_edid_header_is_valid(const u8 *raw_edid);
> -extern bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid);
> +extern bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid,
> + bool *header_corrupt);
> extern bool drm_edid_is_valid(struct edid *edid);
>
> extern struct drm_tile_group *drm_mode_create_tile_group(struct drm_device *dev,
> --
> 1.9.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Paulo Zanoni
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 05/12] drm: Add supporting structure for Displayport Link CTS test 4.2.2.6
2015-04-16 13:34 ` Paulo Zanoni
@ 2015-04-16 14:02 ` Todd Previte
0 siblings, 0 replies; 13+ messages in thread
From: Todd Previte @ 2015-04-16 14:02 UTC (permalink / raw)
To: Paulo Zanoni; +Cc: Intel Graphics Development, DRI Development
On 4/16/2015 6:34 AM, Paulo Zanoni wrote:
> 2015-04-15 19:03 GMT-03:00 Todd Previte <tprevite@gmail.com>:
>> Displayport compliance test 4.2.2.6 requires that a source device be capable of
>> detecting a corrupt EDID. The test specification states that the sink device
>> sets up the EDID with an invalid checksum. To do this, the sink sets up an
>> invalid EDID header, expecting the source device to generate the checksum and
>> compare it to the value stored in the last byte of the block data.
>>
>> Unfortunately, the DRM EDID reading and parsing functions are actually too good
>> in this case; the header is fixed before the checksum is computed and thus the
>> code never sees the invalid checksum. This results in a failure to pass the
>> compliance test.
>>
>> To correct this issue, a checksum is generated when the EDID header is detected
>> as corrupted. If the checksum is invalid, it sets the header_corrupt flag and
>> logs the errors. In the case of a more seriously damaged header (fixup score
>> less than the threshold) the code does not generate the checksum but does set
>> the header_corrupt flag.
>>
>> V2:
>> - Removed the static bool global
>> - Added a bool to the drm_connector struct to reaplce the static one for
>> holding the status of raw edid header corruption detection
>> - Modified the function signature of the is_valid function to take an
>> additional parameter to store the corruption detected value
>> - Fixed the other callers of the above is_valid function
>> V3:
>> - Updated the commit message to be more clear about what and why this
>> patch does what it does.
>> - Added comment in code to clarify the operations there
>> - Removed compliance variable and check_link_status update; those
>> have been moved to a later patch
>> - Removed variable assignment from the bottom of the test handler
>> V4:
>> - Removed i915 tag from subject line as the patch is not i915-specific
>> V5:
>> - Moved code causing a compilation error to this patch where the variable
>> is actually declared
>> - Maintained blank lines / spacing so as to not contaminate the patch
>> V6:
>> - Removed extra debug messages
>> - Added documentation to for the added parameter on drm_edid_block_valid
>> - Fixed more whitespace issues in check_link_status
>> - Added a clear of the header_corrupt flag to the end of the test handler
>> in intel_dp.c
>> - Changed the usage of the new function prototype in several places to use
>> NULL where it is not needed by compliance testing
>> V7:
>> - Updated to account for long_pulse flag propagation
>> V8:
>> - Removed clearing of header_corrupt flag from the test handler in intel_dp.c
>> - Added clearing of header_corrupt flag in the drm_edid_block_valid function
>>
>> Signed-off-by: Todd Previte <tprevite@gmail.com>
>> Cc: dri-devel@lists.freedesktop.org
>> ---
>> drivers/gpu/drm/drm_edid.c | 35 +++++++++++++++++++++++++++++------
>> drivers/gpu/drm/drm_edid_load.c | 7 +++++--
>> drivers/gpu/drm/i915/intel_dp.c | 2 +-
>> include/drm/drm_crtc.h | 8 +++++++-
>> 4 files changed, 42 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
>> index 53bc7a6..6d037f9 100644
>> --- a/drivers/gpu/drm/drm_edid.c
>> +++ b/drivers/gpu/drm/drm_edid.c
>> @@ -1041,13 +1041,15 @@ static bool drm_edid_is_zero(const u8 *in_edid, int length)
>> * @raw_edid: pointer to raw EDID block
>> * @block: type of block to validate (0 for base, extension otherwise)
>> * @print_bad_edid: if true, dump bad EDID blocks to the console
>> + * @header_corrupt: if true, the header or checksum is invalid
>> *
>> * Validate a base or extension EDID block and optionally dump bad blocks to
>> * the console.
>> *
>> * Return: True if the block is valid, false otherwise.
>> */
>> -bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid)
>> +bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid,
>> + bool *header_corrupt)
>> {
>> u8 csum;
>> struct edid *edid = (struct edid *)raw_edid;
>> @@ -1060,11 +1062,28 @@ bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid)
>>
>> if (block == 0) {
>> int score = drm_edid_header_is_valid(raw_edid);
>> - if (score == 8) ;
>> - else if (score >= edid_fixup) {
>> + if (score == 8) {
>> + if (header_corrupt)
>> + *header_corrupt = 0;
>> + } else if (score >= edid_fixup) {
>> + /* Displayport Link CTS Core 1.2 rev1.1 test 4.2.2.6
>> + * In order to properly generate the invalid checksum
>> + * required for this test, it must be generated using
>> + * the raw EDID data. Otherwise, the fix-up code here
>> + * will correct the problem, the checksum is correct
>> + * and the test fails
>> + */
>> + csum = drm_edid_block_checksum(raw_edid);
>> + if (csum) {
>> + if (header_corrupt)
>> + *header_corrupt = 1;
>> + }
>
> I was ready to give the R-B tag, but then I realized this. Shouldn't
> we be setting header_corrupt here regardless of the checksum value? I
> mean, score is not 8, so the header is indeed corrupt.
I added the checksum computation as an extra validation that the header
was indeed corrupt. However, that checksum is discarded so the
additional checksum generation isn't strictly necessary. I've removed it
in the updated patch.
> Also, setting "header_corrupt" to 1 in case the checksum is invalid is
> somewhat a distortion of meaning in the variable. If we really need
> the checksum, we should probably have something like
> connector->raw_checksum, or maybe rename header_corrupt to
> edid_corrupt, where "edid_corrupt" means "either invalid header or
> invalid checksum"?
I was thinking the same thing as I've been working with this. I think
renaming it to "edid_corrupt" is more accurate. Fixed in the update.
> And for compliance testing specifically, as far as I understand, I see
> on patch 7 that you only care about the checksum if the header is
> valid, so we don't seem to need the csum value here, so it seems my
> suggestion to just assign header_corrupt=1 regardless of the checksum
> value seems to be enough to solve your problem.
Agreed.
> Sorry for not realizing this earlier, the back-and-forth between
> patches royally confused me.
Indeed, that's an issue for me as well.
>
>> DRM_DEBUG("Fixing EDID header, your hardware may be failing\n");
>> memcpy(raw_edid, edid_header, sizeof(edid_header));
>> } else {
>> + if (header_corrupt) {
>> + *header_corrupt = 1;
>> + }
>> goto bad;
>> }
>> }
>> @@ -1129,7 +1148,7 @@ bool drm_edid_is_valid(struct edid *edid)
>> return false;
>>
>> for (i = 0; i <= edid->extensions; i++)
>> - if (!drm_edid_block_valid(raw + i * EDID_LENGTH, i, true))
>> + if (!drm_edid_block_valid(raw + i * EDID_LENGTH, i, true, NULL))
>> return false;
>>
>> return true;
>> @@ -1232,7 +1251,8 @@ struct edid *drm_do_get_edid(struct drm_connector *connector,
>> for (i = 0; i < 4; i++) {
>> if (get_edid_block(data, block, 0, EDID_LENGTH))
>> goto out;
>> - if (drm_edid_block_valid(block, 0, print_bad_edid))
>> + if (drm_edid_block_valid(block, 0, print_bad_edid,
>> + &connector->edid_header_corrupt))
>> break;
>> if (i == 0 && drm_edid_is_zero(block, EDID_LENGTH)) {
>> connector->null_edid_counter++;
>> @@ -1257,7 +1277,10 @@ struct edid *drm_do_get_edid(struct drm_connector *connector,
>> block + (valid_extensions + 1) * EDID_LENGTH,
>> j, EDID_LENGTH))
>> goto out;
>> - if (drm_edid_block_valid(block + (valid_extensions + 1) * EDID_LENGTH, j, print_bad_edid)) {
>> + if (drm_edid_block_valid(block + (valid_extensions + 1)
>> + * EDID_LENGTH, j,
>> + print_bad_edid,
>> + NULL)) {
>> valid_extensions++;
>> break;
>> }
>> diff --git a/drivers/gpu/drm/drm_edid_load.c b/drivers/gpu/drm/drm_edid_load.c
>> index 4c0aa97..48b48e8 100644
>> --- a/drivers/gpu/drm/drm_edid_load.c
>> +++ b/drivers/gpu/drm/drm_edid_load.c
>> @@ -216,7 +216,8 @@ static void *edid_load(struct drm_connector *connector, const char *name,
>> goto out;
>> }
>>
>> - if (!drm_edid_block_valid(edid, 0, print_bad_edid)) {
>> + if (!drm_edid_block_valid(edid, 0, print_bad_edid,
>> + &connector->edid_header_corrupt)) {
>> connector->bad_edid_counter++;
>> DRM_ERROR("Base block of EDID firmware \"%s\" is invalid ",
>> name);
>> @@ -229,7 +230,9 @@ static void *edid_load(struct drm_connector *connector, const char *name,
>> if (i != valid_extensions + 1)
>> memcpy(edid + (valid_extensions + 1) * EDID_LENGTH,
>> edid + i * EDID_LENGTH, EDID_LENGTH);
>> - if (drm_edid_block_valid(edid + i * EDID_LENGTH, i, print_bad_edid))
>> + if (drm_edid_block_valid(edid + i * EDID_LENGTH, i,
>> + print_bad_edid,
>> + NULL))
>> valid_extensions++;
>> }
>>
>> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>> index 7ca0e18..97224ff 100644
>> --- a/drivers/gpu/drm/i915/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>> @@ -4139,7 +4139,7 @@ intel_dp_check_link_status(struct intel_dp *intel_dp, bool long_hpd)
>> */
>> if (long_hpd) {
>> edid_read = drm_get_edid(connector, adapter);
>> - if (!edid_read) {
>> + if (!edid_read || connector->edid_header_corrupt == 1) {
>> DRM_DEBUG_DRIVER("Invalid EDID detected\n");
>> } else {
>> kfree(edid_read);
>> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
>> index d4e4b82..f7a4a92 100644
>> --- a/include/drm/drm_crtc.h
>> +++ b/include/drm/drm_crtc.h
>> @@ -719,6 +719,11 @@ struct drm_connector {
>> int null_edid_counter; /* needed to workaround some HW bugs where we get all 0s */
>> unsigned bad_edid_counter;
>>
>> + /* Flag for raw EDID header corruption - used in Displayport
>> + * compliance testing - * Displayport Link CTS Core 1.2 rev1.1 4.2.2.6
>> + */
>> + bool edid_header_corrupt;
>> +
>> struct dentry *debugfs_entry;
>>
>> struct drm_connector_state *state;
>> @@ -1443,7 +1448,8 @@ extern void drm_set_preferred_mode(struct drm_connector *connector,
>> int hpref, int vpref);
>>
>> extern int drm_edid_header_is_valid(const u8 *raw_edid);
>> -extern bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid);
>> +extern bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid,
>> + bool *header_corrupt);
>> extern bool drm_edid_is_valid(struct edid *edid);
>>
>> extern struct drm_tile_group *drm_mode_create_tile_group(struct drm_device *dev,
>> --
>> 1.9.1
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 05/12] drm: Add supporting structure for Displayport Link CTS test 4.2.2.6
2015-04-15 15:38 ` [PATCH 05/10] drm: Add supporting structure for Displayport Link CTS test 4.2.2.6 Todd Previte
2015-04-15 17:15 ` [PATCH 05/12] " Todd Previte
2015-04-15 22:03 ` Todd Previte
@ 2015-04-16 15:47 ` Todd Previte
2 siblings, 0 replies; 13+ messages in thread
From: Todd Previte @ 2015-04-16 15:47 UTC (permalink / raw)
To: intel-gfx; +Cc: dri-devel
Displayport compliance test 4.2.2.6 requires that a source device be capable of
detecting a corrupt EDID. The test specification states that the sink device
sets up the EDID with an invalid checksum. To do this, the sink sets up an
invalid EDID header, expecting the source device to generate the checksum and
compare it to the value stored in the last byte of the block data.
Unfortunately, the DRM EDID reading and parsing functions are actually too good
in this case; the header is fixed before the checksum is computed and thus the
test never sees the invalid checksum. This results in a failure to pass the
compliance test.
To correct this issue, when the EDID code detects that the header is invalid,
a flag is set to indicate that the EDID is corrupted. In this case, it sets
edid_corrupt flag and continues with its fix-up code. In the case of a more
seriously damaged header (fixup score less than the threshold) the code does
not generate the checksum but does set the edid_corrupt flag.
V2:
- Removed the static bool global
- Added a bool to the drm_connector struct to reaplce the static one for
holding the status of raw edid header corruption detection
- Modified the function signature of the is_valid function to take an
additional parameter to store the corruption detected value
- Fixed the other callers of the above is_valid function
V3:
- Updated the commit message to be more clear about what and why this
patch does what it does.
- Added comment in code to clarify the operations there
- Removed compliance variable and check_link_status update; those
have been moved to a later patch
- Removed variable assignment from the bottom of the test handler
V4:
- Removed i915 tag from subject line as the patch is not i915-specific
V5:
- Moved code causing a compilation error to this patch where the variable
is actually declared
- Maintained blank lines / spacing so as to not contaminate the patch
V6:
- Removed extra debug messages
- Added documentation to for the added parameter on drm_edid_block_valid
- Fixed more whitespace issues in check_link_status
- Added a clear of the header_corrupt flag to the end of the test handler
in intel_dp.c
- Changed the usage of the new function prototype in several places to use
NULL where it is not needed by compliance testing
V7:
- Updated to account for long_pulse flag propagation
V8:
- Removed clearing of header_corrupt flag from the test handler in intel_dp.c
- Added clearing of header_corrupt flag in the drm_edid_block_valid function
V9:
- Renamed header_corrupt flag to edid_corrupt to more accurately reflect its
value and purpose
- Updated commit message
Signed-off-by: Todd Previte <tprevite@gmail.com>
Cc: dri-devel@lists.freedesktop.org
---
drivers/gpu/drm/drm_edid.c | 30 ++++++++++++++++++++++++------
drivers/gpu/drm/drm_edid_load.c | 7 +++++--
drivers/gpu/drm/i915/intel_dp.c | 2 +-
include/drm/drm_crtc.h | 8 +++++++-
4 files changed, 37 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 53bc7a6..69a5a09 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -1041,13 +1041,15 @@ static bool drm_edid_is_zero(const u8 *in_edid, int length)
* @raw_edid: pointer to raw EDID block
* @block: type of block to validate (0 for base, extension otherwise)
* @print_bad_edid: if true, dump bad EDID blocks to the console
+ * @edid_corrupt: if true, the header or checksum is invalid
*
* Validate a base or extension EDID block and optionally dump bad blocks to
* the console.
*
* Return: True if the block is valid, false otherwise.
*/
-bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid)
+bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid,
+ bool *edid_corrupt)
{
u8 csum;
struct edid *edid = (struct edid *)raw_edid;
@@ -1060,11 +1062,23 @@ bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid)
if (block == 0) {
int score = drm_edid_header_is_valid(raw_edid);
- if (score == 8) ;
- else if (score >= edid_fixup) {
+ if (score == 8) {
+ if (edid_corrupt)
+ *edid_corrupt = 0;
+ } else if (score >= edid_fixup) {
+ /* Displayport Link CTS Core 1.2 rev1.1 test 4.2.2.6
+ * The corrupt flag needs to be set here otherwise, the
+ * fix-up code here will correct the problem, the
+ * checksum is correct and the test fails
+ */
+ if (edid_corrupt)
+ *edid_corrupt = 1;
DRM_DEBUG("Fixing EDID header, your hardware may be failing\n");
memcpy(raw_edid, edid_header, sizeof(edid_header));
} else {
+ if (edid_corrupt) {
+ *edid_corrupt = 1;
+ }
goto bad;
}
}
@@ -1129,7 +1143,7 @@ bool drm_edid_is_valid(struct edid *edid)
return false;
for (i = 0; i <= edid->extensions; i++)
- if (!drm_edid_block_valid(raw + i * EDID_LENGTH, i, true))
+ if (!drm_edid_block_valid(raw + i * EDID_LENGTH, i, true, NULL))
return false;
return true;
@@ -1232,7 +1246,8 @@ struct edid *drm_do_get_edid(struct drm_connector *connector,
for (i = 0; i < 4; i++) {
if (get_edid_block(data, block, 0, EDID_LENGTH))
goto out;
- if (drm_edid_block_valid(block, 0, print_bad_edid))
+ if (drm_edid_block_valid(block, 0, print_bad_edid,
+ &connector->edid_corrupt))
break;
if (i == 0 && drm_edid_is_zero(block, EDID_LENGTH)) {
connector->null_edid_counter++;
@@ -1257,7 +1272,10 @@ struct edid *drm_do_get_edid(struct drm_connector *connector,
block + (valid_extensions + 1) * EDID_LENGTH,
j, EDID_LENGTH))
goto out;
- if (drm_edid_block_valid(block + (valid_extensions + 1) * EDID_LENGTH, j, print_bad_edid)) {
+ if (drm_edid_block_valid(block + (valid_extensions + 1)
+ * EDID_LENGTH, j,
+ print_bad_edid,
+ NULL)) {
valid_extensions++;
break;
}
diff --git a/drivers/gpu/drm/drm_edid_load.c b/drivers/gpu/drm/drm_edid_load.c
index 4c0aa97..c5605fe 100644
--- a/drivers/gpu/drm/drm_edid_load.c
+++ b/drivers/gpu/drm/drm_edid_load.c
@@ -216,7 +216,8 @@ static void *edid_load(struct drm_connector *connector, const char *name,
goto out;
}
- if (!drm_edid_block_valid(edid, 0, print_bad_edid)) {
+ if (!drm_edid_block_valid(edid, 0, print_bad_edid,
+ &connector->edid_corrupt)) {
connector->bad_edid_counter++;
DRM_ERROR("Base block of EDID firmware \"%s\" is invalid ",
name);
@@ -229,7 +230,9 @@ static void *edid_load(struct drm_connector *connector, const char *name,
if (i != valid_extensions + 1)
memcpy(edid + (valid_extensions + 1) * EDID_LENGTH,
edid + i * EDID_LENGTH, EDID_LENGTH);
- if (drm_edid_block_valid(edid + i * EDID_LENGTH, i, print_bad_edid))
+ if (drm_edid_block_valid(edid + i * EDID_LENGTH, i,
+ print_bad_edid,
+ NULL))
valid_extensions++;
}
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 7ca0e18..d4c0fd3 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4139,7 +4139,7 @@ intel_dp_check_link_status(struct intel_dp *intel_dp, bool long_hpd)
*/
if (long_hpd) {
edid_read = drm_get_edid(connector, adapter);
- if (!edid_read) {
+ if (!edid_read || connector->edid_corrupt == 1) {
DRM_DEBUG_DRIVER("Invalid EDID detected\n");
} else {
kfree(edid_read);
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index d4e4b82..8bc2724 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -719,6 +719,11 @@ struct drm_connector {
int null_edid_counter; /* needed to workaround some HW bugs where we get all 0s */
unsigned bad_edid_counter;
+ /* Flag for raw EDID header corruption - used in Displayport
+ * compliance testing - * Displayport Link CTS Core 1.2 rev1.1 4.2.2.6
+ */
+ bool edid_corrupt;
+
struct dentry *debugfs_entry;
struct drm_connector_state *state;
@@ -1443,7 +1448,8 @@ extern void drm_set_preferred_mode(struct drm_connector *connector,
int hpref, int vpref);
extern int drm_edid_header_is_valid(const u8 *raw_edid);
-extern bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid);
+extern bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid,
+ bool *edid_corrupt);
extern bool drm_edid_is_valid(struct edid *edid);
extern struct drm_tile_group *drm_mode_create_tile_group(struct drm_device *dev,
--
1.9.1
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 13+ messages in thread