[Intel-gfx] [PATCH] drm/edid: Fix DDC probe for passive DP dongles
On 5/21/2015 1:28 AM, Jani Nikula wrote:
> On Thu, 21 May 2015, Todd Previte wrote:
>> Passive DP->DVI/HDMI dongles show up to the system as HDMI devices, as they
>> do not have a sink device in them to respond to any AUX traffic. When
>> probing these dongles over the DDC, sometimes they will NAK the first attempt
>> even though the transaction is valid and they support the DDC protocol. The
>> retry loop inside of drm_do_probe_ddc_edid() would normally catch this case
>> and try the transaction again, resulting in success.
>>
>> That, however, was thwarted by the fix for fdo.org bug #41059. The patch is:
>> commit 9292f37e1f5c79400254dca46f83313488093825
>> Author: Eugeni Dodonov
>> Date: Thu Jan 5 09:34:28 2012 -0200
>>
>> drm: give up on edid retries when i2c bus is not responding
> Some extra background:
>
> That commit refers to the i2c bit banging code, while i915 now prefers
> gmbus, and only falls back to big banging on certain failures. (See
> gmbux_xfer() in i915/intel_i2c.c). This means that in most cases i915 is
> no longer susceptible to the 5*3 timeout loops, but it also means we
> don't have the i2c bit banging retry at all on -ENXIO, like Todd notes.
>
> The questions are, is one retry after -ENXIO in drm_do_probe_ddc_edid
> enough now? Should we revert the original commit instead since the
> underlying algorithm has changed? Or should we return something other
> than -ENXIO from our gmbus code to not hit this exit with no retries
> path?
Question #1:
During development and testing, all of the passive dongles used would
NAK once before operating correctly. I had an additional dongle here
that I tested as well and it exhibited the exact same behavior. While I
believe that more testing certainly needs to be done (as you mention
below) at this point I would say allowing a single NAK is sufficient to
solve this problem.
Question #2:
While I agree to some extent that this patch is dated with respect to
the underlying algorithm changes, there still appears to be some value
in keeping it since it does allow for more efficient processing of
disconnected ports. So I would recommend against reverting this commit
right now.
Question #3:
I'm looking into this now. I'll have more information by the end of the day.
>> This added code to exit immediately if the return code from the
>> i2c_transfer function was -ENXIO in order to reduce the amount of time spent
>> in waiting for unresponsive or disconnected devices. For the DP dongles,
>> this means that the second retry never happens which results in a failed
>> EDID probe and a black screen.
>>
>> To work around this problem without undoing the fix for bug #41059, the
>> number of retries is checked along with the return code. This allows for a
>> device to NAK once and still continue operations. A second NAK will result
>> in breaking the loop as it would have before and stopping the DDC probe.
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=85924
>
> Maybe throw this at other dongle bugs you can find too?
>
> We're going to need Tested-bys though.
>
> BR,
> Jani.
Definitely - I'll go wrangle up the dongle bugs I can find on kernel.org
and FDO and see if this affects them.
As for testing, the more the merrier on this front. If anyone has
passive DP->DVI/HDMI dongles and the ability to test them, please see
the bug Jani referenced above and report your findings there.
-T
>
>> Signed-off-by: Todd Previte
>> Cc: intel-gfx at lists.freedesktop.org
>> ---
>> drivers/gpu/drm/drm_edid.c | 5 -
>> 1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
>> index 7087da3..e8047bd 100644
>> --- a/drivers/gpu/drm/drm_edid.c
>> +++ b/drivers/gpu/drm/drm_edid.c
>> @@ -1238,7 +1238,10 @@ drm_do_probe_ddc_edid(void *data, u8 *buf, unsigned
>> int block, size_t len)
>> */
>> ret = i2c_transfer(adapter, [3 - xfers], xfers);
>>
>> -if (ret == -ENXIO) {
>> +/* Passive DP->DVI/HDMI dongles sometimes NAK the first probe
>> + * Try to probe again but if it NAKs, stop trying
>> + */
>> +if (ret == -ENXIO && retries < 5) {
>> DRM_DEBUG_KMS("drm: skipping non-existent adapter %s\n",
>> adapter->name);
>> break;
>> --
>> 1.9.1
>>
>> ___
>> Intel-gfx mailing list
>> Intel-gfx at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[PATCH] drm/edid: Fix DDC probe for passive DP dongles
Passive DP->DVI/HDMI dongles show up to the system as HDMI devices, as they
do not have a sink device in them to respond to any AUX traffic. When
probing these dongles over the DDC, sometimes they will NAK the first attempt
even though the transaction is valid and they support the DDC protocol. The
retry loop inside of drm_do_probe_ddc_edid() would normally catch this case
and try the transaction again, resulting in success.
That, however, was thwarted by the fix for fdo.org bug #41059. The patch is:
commit 9292f37e1f5c79400254dca46f83313488093825
Author: Eugeni Dodonov
Date: Thu Jan 5 09:34:28 2012 -0200
drm: give up on edid retries when i2c bus is not responding
This added code to exit immediately if the return code from the
i2c_transfer function was -ENXIO in order to reduce the amount of time spent
in waiting for unresponsive or disconnected devices. For the DP dongles,
this means that the second retry never happens which results in a failed
EDID probe and a black screen.
To work around this problem without undoing the fix for bug #41059, the
number of retries is checked along with the return code. This allows for a
device to NAK once and still continue operations. A second NAK will result
in breaking the loop as it would have before and stopping the DDC probe.
Signed-off-by: Todd Previte
Cc: intel-gfx at lists.freedesktop.org
---
drivers/gpu/drm/drm_edid.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 7087da3..e8047bd 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -1238,7 +1238,10 @@ drm_do_probe_ddc_edid(void *data, u8 *buf, unsigned int
block, size_t len)
*/
ret = i2c_transfer(adapter, [3 - xfers], xfers);
- if (ret == -ENXIO) {
+ /* Passive DP->DVI/HDMI dongles sometimes NAK the first probe
+* Try to probe again but if it NAKs, stop trying
+*/
+ if (ret == -ENXIO && retries < 5) {
DRM_DEBUG_KMS("drm: skipping non-existent adapter %s\n",
adapter->name);
break;
--
1.9.1
[PATCH 2/5] drm: Add edid_corrupt flag for Displayport Link CTS 4.2.2.6
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. This flag is also set in
the case of a more seriously damaged header (fixup score less than the
threshold). For consistency, the edid_corrupt flag is also set when the
checksum is invalid as well.
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
V10:
- Updated for versioning and patch swizzle
- Revised the title to more accurately reflect the nature and contents of
the patch
- Fixed formatting/whitespace problems
- Added set flag when computed checksum is invalid
Signed-off-by: Todd Previte
Cc: dri-devel at lists.freedesktop.org
---
drivers/gpu/drm/drm_edid.c | 32 ++--
drivers/gpu/drm/drm_edid_load.c | 7 +--
include/drm/drm_crtc.h | 8 +++-
3 files changed, 38 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 53bc7a6..be6713c 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,22 @@ 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_corru
[PATCH 4/5] drm: Fix for DP CTS test 4.2.2.5 - I2C DEFER handling
For test 4.2.2.5 to pass per the Link CTS Core 1.2 rev1.1 spec, the source
device must attempt at least 7 times to read the EDID when it receives an
I2C defer. The normal DRM code makes only 7 retries, regardless of whether
or not the response is a native defer or an I2C defer. Test 4.2.2.5 fails
since there are native defers interspersed with the I2C defers which
results in less than 7 EDID read attempts.
The solution is to add the numer of defers to the retry counter when an I2C
DEFER is returned such that another read attempt will be made. This situation
should normally only occur in compliance testing, however, as a worse case
real-world scenario, it would result in 13 attempts ( 6 native defers, 7 I2C
defers) for a single transaction to complete. The net result is a slightly
slower response to an EDID read that shouldn't significantly impact overall
performance.
V2:
- Added a check on the number of I2C Defers to limit the number
of times that the retries variable will be decremented. This
is to address review feedback regarding possible infinite loops
from misbehaving sink devices.
V3:
- Fixed the limit value to 7 instead of 8 to get the correct retry
count.
- Combined the increment of the defer count into the if-statement
V4:
- Removed i915 tag from subject as the patch is not i915-specific
V5:
- Updated the for-loop to add the number of i2c defers to the retry
counter such that the correct number of retry attempts will be
made
Signed-off-by: Todd Previte
Cc: dri-devel at lists.freedesktop.org
Reviewed-by: Paulo Zanoni
---
drivers/gpu/drm/drm_dp_helper.c | 10 --
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index 575172e..80a02a4 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -432,7 +432,7 @@ static u32 drm_dp_i2c_functionality(struct i2c_adapter
*adapter)
*/
static int drm_dp_i2c_do_msg(struct drm_dp_aux *aux, struct drm_dp_aux_msg
*msg)
{
- unsigned int retry;
+ unsigned int retry, defer_i2c;
int ret;
/*
@@ -440,7 +440,7 @@ static int drm_dp_i2c_do_msg(struct drm_dp_aux *aux, struct
drm_dp_aux_msg *msg)
* is required to retry at least seven times upon receiving AUX_DEFER
* before giving up the AUX transaction.
*/
- for (retry = 0; retry < 7; retry++) {
+ for (retry = 0, defer_i2c = 0; retry < (7 + defer_i2c); retry++) {
mutex_lock(>hw_mutex);
ret = aux->transfer(aux, msg);
mutex_unlock(>hw_mutex);
@@ -499,7 +499,13 @@ static int drm_dp_i2c_do_msg(struct drm_dp_aux *aux,
struct drm_dp_aux_msg *msg)
case DP_AUX_I2C_REPLY_DEFER:
DRM_DEBUG_KMS("I2C defer\n");
+ /* DP Compliance Test 4.2.2.5 Requirement:
+* Must have at least 7 retries for I2C defers on the
+* transaction to pass this test
+*/
aux->i2c_defer_count++;
+ if (defer_i2c < 7)
+ defer_i2c++;
usleep_range(400, 500);
continue;
--
1.9.1
[PATCH 2/5] drm: Add edid_corrupt flag for Displayport Link CTS 4.2.2.6
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
V10:
- Updated for versioning and patch swizzle
- Revised the title to more accurately reflect the nature and contents of
the patch
Signed-off-by: Todd Previte
Cc: dri-devel at lists.freedesktop.org
---
drivers/gpu/drm/drm_edid.c | 30 --
drivers/gpu/drm/drm_edid_load.c | 7 +--
include/drm/drm_crtc.h | 8 +++-
3 files changed, 36 insertions(+), 9 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");
[PATCH 4/5] drm: Fix for DP CTS test 4.2.2.5 - I2C DEFER handling
For test 4.2.2.5 to pass per the Link CTS Core 1.2 rev1.1 spec, the source
device must attempt at least 7 times to read the EDID when it receives an
I2C defer. The normal DRM code makes only 7 retries, regardless of whether
or not the response is a native defer or an I2C defer. Test 4.2.2.5 fails
since there are native defers interspersed with the I2C defers which
results in less than 7 EDID read attempts.
The solution is to add the numer of defers to the retry counter when an I2C
DEFER is returned such that another read attempt will be made. This situation
should normally only occur in compliance testing, however, as a worse case
real-world scenario, it would result in 13 attempts ( 6 native defers, 7 I2C
defers) for a single transaction to complete. The net result is a slightly
slower response to an EDID read that shouldn't significantly impact overall
performance.
V2:
- Added a check on the number of I2C Defers to limit the number
of times that the retries variable will be decremented. This
is to address review feedback regarding possible infinite loops
from misbehaving sink devices.
V3:
- Fixed the limit value to 7 instead of 8 to get the correct retry
count.
- Combined the increment of the defer count into the if-statement
V4:
- Removed i915 tag from subject as the patch is not i915-specific
V5:
- Updated the for-loop to add the number of i2c defers to the retry
counter such that the correct number of retry attempts will be
made
Signed-off-by: Todd Previte
Cc: dri-devel at lists.freedesktop.org
Reviewed-by: Paulo Zanoni
---
drivers/gpu/drm/drm_dp_helper.c | 10 --
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index 575172e..80a02a4 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -432,7 +432,7 @@ static u32 drm_dp_i2c_functionality(struct i2c_adapter
*adapter)
*/
static int drm_dp_i2c_do_msg(struct drm_dp_aux *aux, struct drm_dp_aux_msg
*msg)
{
- unsigned int retry;
+ unsigned int retry, defer_i2c;
int ret;
/*
@@ -440,7 +440,7 @@ static int drm_dp_i2c_do_msg(struct drm_dp_aux *aux, struct
drm_dp_aux_msg *msg)
* is required to retry at least seven times upon receiving AUX_DEFER
* before giving up the AUX transaction.
*/
- for (retry = 0; retry < 7; retry++) {
+ for (retry = 0, defer_i2c = 0; retry < (7 + defer_i2c); retry++) {
mutex_lock(>hw_mutex);
ret = aux->transfer(aux, msg);
mutex_unlock(>hw_mutex);
@@ -499,7 +499,13 @@ static int drm_dp_i2c_do_msg(struct drm_dp_aux *aux,
struct drm_dp_aux_msg *msg)
case DP_AUX_I2C_REPLY_DEFER:
DRM_DEBUG_KMS("I2C defer\n");
+ /* DP Compliance Test 4.2.2.5 Requirement:
+* Must have at least 7 retries for I2C defers on the
+* transaction to pass this test
+*/
aux->i2c_defer_count++;
+ if (defer_i2c < 7)
+ defer_i2c++;
usleep_range(400, 500);
continue;
--
1.9.1
[PATCH 2/5] drm: Add edid_corrupt flag for Displayport Link CTS 4.2.2.6
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
V10:
- Updated for versioning and patch swizzle
- Revised the title to more accurately reflect the nature and contents of
the patch
Signed-off-by: Todd Previte
Cc: dri-devel at lists.freedesktop.org
---
drivers/gpu/drm/drm_edid.c | 30 --
drivers/gpu/drm/drm_edid_load.c | 7 +--
include/drm/drm_crtc.h | 8 +++-
3 files changed, 36 insertions(+), 9 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");
[PATCH 05/12] drm: Add supporting structure for Displayport Link CTS test 4.2.2.6
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
Cc: dri-devel at 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));
[PATCH 05/12] drm: Add supporting structure for Displayport Link CTS test 4.2.2.6
On 4/16/2015 6:34 AM, Paulo Zanoni wrote:
> 2015-04-15 19:03 GMT-03:00 Todd Previte :
>> 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
>> Cc: dri-devel at 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) {
[PATCH 05/12] drm: Add supporting structure for Displayport Link CTS test 4.2.2.6
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
Cc: dri-devel at 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
[Intel-gfx] [PATCH 05/12] drm: Add supporting structure for Displayport Link CTS test 4.2.2.6
On 4/15/2015 1:25 PM, Paulo Zanoni wrote:
> 2015-04-15 14:15 GMT-03:00 Todd Previte :
>> 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
>> Cc: dri-devel at 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
[PATCH 05/12] drm: Add supporting structure for Displayport Link CTS test 4.2.2.6
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
Cc: dri-devel at 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;
+ }
[PATCH 10/10] drm: Fix the 'native defer' message in drm_dp_i2c_do_msg()
The debug message is missing a newline at the end and it makes the
logs hard to read when a device defers a lot. Simple 2-character fix
adds the newline at the end.
Signed-off-by: Todd Previte
Cc: dri-devel at lists.freedesktop.org
Reviewed-by: Paulo Zanoni
Reviewed-by: Alex Deucher
---
drivers/gpu/drm/drm_dp_helper.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index 7f0356e..80a02a4 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -466,7 +466,7 @@ static int drm_dp_i2c_do_msg(struct drm_dp_aux *aux, struct
drm_dp_aux_msg *msg)
return -EREMOTEIO;
case DP_AUX_NATIVE_REPLY_DEFER:
- DRM_DEBUG_KMS("native defer");
+ DRM_DEBUG_KMS("native defer\n");
/*
* We could check for I2C bit rate capabilities and if
* available adjust this interval. We could also be
--
1.9.1
[PATCH 08/10] drm: Fix for DP CTS test 4.2.2.5 - I2C DEFER handling
For test 4.2.2.5 to pass per the Link CTS Core 1.2 rev1.1 spec, the source
device must attempt at least 7 times to read the EDID when it receives an
I2C defer. The normal DRM code makes only 7 retries, regardless of whether
or not the response is a native defer or an I2C defer. Test 4.2.2.5 fails
since there are native defers interspersed with the I2C defers which
results in less than 7 EDID read attempts.
The solution is to add the numer of defers to the retry counter when an I2C
DEFER is returned such that another read attempt will be made. This situation
should normally only occur in compliance testing, however, as a worse case
real-world scenario, it would result in 13 attempts ( 6 native defers, 7 I2C
defers) for a single transaction to complete. The net result is a slightly
slower response to an EDID read that shouldn't significantly impact overall
performance.
V2:
- Added a check on the number of I2C Defers to limit the number
of times that the retries variable will be decremented. This
is to address review feedback regarding possible infinite loops
from misbehaving sink devices.
V3:
- Fixed the limit value to 7 instead of 8 to get the correct retry
count.
- Combined the increment of the defer count into the if-statement
V4:
- Removed i915 tag from subject as the patch is not i915-specific
V5:
- Updated the for-loop to add the number of i2c defers to the retry
counter such that the correct number of retry attempts will be
made
Signed-off-by: Todd Previte
Cc: dri-devel at lists.freedesktop.org
---
drivers/gpu/drm/drm_dp_helper.c | 10 --
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index 71dcbc6..7f0356e 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -432,7 +432,7 @@ static u32 drm_dp_i2c_functionality(struct i2c_adapter
*adapter)
*/
static int drm_dp_i2c_do_msg(struct drm_dp_aux *aux, struct drm_dp_aux_msg
*msg)
{
- unsigned int retry;
+ unsigned int retry, defer_i2c;
int ret;
/*
@@ -440,7 +440,7 @@ static int drm_dp_i2c_do_msg(struct drm_dp_aux *aux, struct
drm_dp_aux_msg *msg)
* is required to retry at least seven times upon receiving AUX_DEFER
* before giving up the AUX transaction.
*/
- for (retry = 0; retry < 7; retry++) {
+ for (retry = 0, defer_i2c = 0; retry < (7 + defer_i2c); retry++) {
mutex_lock(>hw_mutex);
ret = aux->transfer(aux, msg);
mutex_unlock(>hw_mutex);
@@ -499,7 +499,13 @@ static int drm_dp_i2c_do_msg(struct drm_dp_aux *aux,
struct drm_dp_aux_msg *msg)
case DP_AUX_I2C_REPLY_DEFER:
DRM_DEBUG_KMS("I2C defer\n");
+ /* DP Compliance Test 4.2.2.5 Requirement:
+* Must have at least 7 retries for I2C defers on the
+* transaction to pass this test
+*/
aux->i2c_defer_count++;
+ if (defer_i2c < 7)
+ defer_i2c++;
usleep_range(400, 500);
continue;
--
1.9.1
[PATCH 05/10] drm: Add supporting structure for Displayport Link CTS test 4.2.2.6
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
Signed-off-by: Todd Previte
Cc: dri-devel at 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_
[PATCH 06/13] drm: Add supporting structure for Displayport Link CTS test 4.2.2.6
On 4/13/15 3:18 PM, Paulo Zanoni wrote:
> 2015-04-13 11:53 GMT-03:00 Todd Previte :
>> 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
>>
>> Signed-off-by: Todd Previte
>> Cc: dri-devel at lists.freedesktop.org
>> ---
>> drivers/gpu/drm/drm_edid.c| 31 ++-
>> drivers/gpu/drm/drm_edid_load.c | 7 +--
>> drivers/gpu/drm/i2c/tda998x_drv.c | 4 ++--
>> drivers/gpu/drm/i915/intel_dp.c | 2 +-
>> include/drm/drm_crtc.h| 8 +++-
>> 5 files changed, 41 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
>> index 53bc7a6..12e5be7 100644
>> --- a/drivers/gpu/drm/drm_edid.c
>> +++ b/drivers/gpu/drm/drm_edid.c
>> @@ -1005,7 +1005,6 @@ int drm_edid_header_is_valid(const u8 *raw_edid)
>> for (i = 0; i < sizeof(edid_header); i++)
>> if (raw_edid[i] == edid_header[i])
>> score++;
>> -
>> return score;
>> }
>> EXPORT_SYMBOL(drm_edid_header_is_valid);
> Bad chunk...
Fixed
>
>> @@ -1047,7 +1046,8 @@ static bool drm_edid_is_zero(const u8 *in_edid, int
>> length)
>>*
>>* 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)
> Need to add the new parameter description to the documentation above.
Done.
>
>> {
>> u8 csum;
>> struct edid *edid = (struct edid *)raw_edid;
>> @@ -1062,9 +1062,27 @@ 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 then
>> correct
>> +* and the test fails
>> +*/
>> + csum = drm_edid_block_checksum(raw_edid);
>> + if (csum) {
>> + DRM_DEBUG_DRIVER("Invali
[PATCH 06/13] drm: Add supporting structure for Displayport Link CTS test 4.2.2.6
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
Signed-off-by: Todd Previte
Cc: dri-devel at lists.freedesktop.org
---
drivers/gpu/drm/drm_edid.c| 31 ++-
drivers/gpu/drm/drm_edid_load.c | 7 +--
drivers/gpu/drm/i2c/tda998x_drv.c | 4 ++--
drivers/gpu/drm/i915/intel_dp.c | 2 +-
include/drm/drm_crtc.h| 8 +++-
5 files changed, 41 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 53bc7a6..12e5be7 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -1005,7 +1005,6 @@ int drm_edid_header_is_valid(const u8 *raw_edid)
for (i = 0; i < sizeof(edid_header); i++)
if (raw_edid[i] == edid_header[i])
score++;
-
return score;
}
EXPORT_SYMBOL(drm_edid_header_is_valid);
@@ -1047,7 +1046,8 @@ static bool drm_edid_is_zero(const u8 *in_edid, int
length)
*
* 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 +1062,27 @@ 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 then
correct
+* and the test fails
+*/
+ csum = drm_edid_block_checksum(raw_edid);
+ if (csum) {
+ DRM_DEBUG_DRIVER("Invalid EDID header, score =
%d\n", score);
+ DRM_DEBUG_DRIVER("Invalid EDID checksum %d\n",
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,
[PATCH 09/13] drm: Fix for DP CTS test 4.2.2.5 - I2C DEFER handling
For test 4.2.2.5 to pass per the Link CTS Core 1.2 rev1.1 spec, the source
device must attempt at least 7 times to read the EDID when it receives an
I2C defer. The normal DRM code makes only 7 retries, regardless of whether
or not the response is a native defer or an I2C defer. Test 4.2.2.5 fails
since there are native defers interspersed with the I2C defers which
results in less than 7 EDID read attempts.
The solution is to decrement the retry counter when an I2C DEFER is returned
such that another read attempt will be made. This situation should normally
only occur in compliance testing, however, as a worse case real-world
scenario, it would result in 13 attempts ( 6 native defers, 7 I2C defers)
for a single transaction to complete. The net result is a slightly slower
response to an EDID read that shouldn't significantly impact overall
performance.
V2:
- Added a check on the number of I2C Defers to limit the number
of times that the retries variable will be decremented. This
is to address review feedback regarding possible infinite loops
from misbehaving sink devices.
V3:
- Fixed the limit value to 7 instead of 8 to get the correct retry
count.
- Combined the increment of the defer count into the if-statement
V4:
- Removed i915 tag from subject as the patch is not i915-specific
Signed-off-by: Todd Previte
Cc: dri-devel at lists.freedesktop.org
---
drivers/gpu/drm/drm_dp_helper.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index 79968e3..9ecfd27 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -468,7 +468,12 @@ static int drm_dp_i2c_do_msg(struct drm_dp_aux *aux,
struct drm_dp_aux_msg *msg)
case DP_AUX_I2C_REPLY_DEFER:
DRM_DEBUG_KMS("I2C defer\n");
- aux->i2c_defer_count++;
+ /* DP Compliance Test 4.2.2.5 Requirement:
+* Must have at least 7 retries for I2C defers on the
+* transaction to pass this test
+*/
+ if (++aux->i2c_defer_count < 7)
+ retry = 0;
usleep_range(400, 500);
continue;
--
1.9.1
[PATCH 06/13] drm: Add supporting structure for Displayport Link CTS test 4.2.2.6
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
Signed-off-by: Todd Previte
Cc: dri-devel at lists.freedesktop.org
---
drivers/gpu/drm/drm_edid.c| 31 ++-
drivers/gpu/drm/drm_edid_load.c | 7 +--
drivers/gpu/drm/i2c/tda998x_drv.c | 4 ++--
include/drm/drm_crtc.h| 8 +++-
4 files changed, 40 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 53bc7a6..12e5be7 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -1005,7 +1005,6 @@ int drm_edid_header_is_valid(const u8 *raw_edid)
for (i = 0; i < sizeof(edid_header); i++)
if (raw_edid[i] == edid_header[i])
score++;
-
return score;
}
EXPORT_SYMBOL(drm_edid_header_is_valid);
@@ -1047,7 +1046,8 @@ static bool drm_edid_is_zero(const u8 *in_edid, int
length)
*
* 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 +1062,27 @@ 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 then
correct
+* and the test fails
+*/
+ csum = drm_edid_block_checksum(raw_edid);
+ if (csum) {
+ DRM_DEBUG_DRIVER("Invalid EDID header, score =
%d\n", score);
+ DRM_DEBUG_DRIVER("Invalid EDID checksum %d\n",
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
[PATCH 06/11] drm/i915: Add supporting structure for Displayport Link CTS test 4.2.2.6
Easy enough to do. Tag removed and updated patch posted. Thanks Emil! On 4/10/2015 10:45 AM, Emil Velikov wrote: > Hi Todd > > On 10/04/15 16:12, Todd Previte wrote: >> 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 >> >> Signed-off-by: Todd Previte >> Cc: dri-devel at lists.freedesktop.org >> --- >> drivers/gpu/drm/drm_edid.c| 31 ++- >> drivers/gpu/drm/drm_edid_load.c | 7 +-- >> drivers/gpu/drm/i2c/tda998x_drv.c | 4 ++-- >> include/drm/drm_crtc.h| 8 +++- >> 4 files changed, 40 insertions(+), 10 deletions(-) >> > Neither this nor patch 09/11 seems to be i915 specific. If you're doing > another revision you might want to use just "drm:". > > Cheers, > Emil
[PATCH 11/11] drm: Fix the 'native defer' message in drm_dp_i2c_do_msg()
The debug message is missing a newline at the end and it makes the
logs hard to read when a device defers a lot. Simple 2-character fix
adds the newline at the end.
Signed-off-by: Todd Previte
Cc: dri-devel at lists.freedesktop.org
---
drivers/gpu/drm/drm_dp_helper.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index 9ecfd27..4ac416e 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -433,7 +433,7 @@ static int drm_dp_i2c_do_msg(struct drm_dp_aux *aux, struct
drm_dp_aux_msg *msg)
return -EREMOTEIO;
case DP_AUX_NATIVE_REPLY_DEFER:
- DRM_DEBUG_KMS("native defer");
+ DRM_DEBUG_KMS("native defer\n");
/*
* We could check for I2C bit rate capabilities and if
* available adjust this interval. We could also be
--
1.9.1
[PATCH 09/11] drm/i915: Fix for DP CTS test 4.2.2.5 - I2C DEFER handling
For test 4.2.2.5 to pass per the Link CTS Core 1.2 rev1.1 spec, the source
device must attempt at least 7 times to read the EDID when it receives an
I2C defer. The normal DRM code makes only 7 retries, regardless of whether
or not the response is a native defer or an I2C defer. Test 4.2.2.5 fails
since there are native defers interspersed with the I2C defers which
results in less than 7 EDID read attempts.
The solution is to decrement the retry counter when an I2C DEFER is returned
such that another read attempt will be made. This situation should normally
only occur in compliance testing, however, as a worse case real-world
scenario, it would result in 13 attempts ( 6 native defers, 7 I2C defers)
for a single transaction to complete. The net result is a slightly slower
response to an EDID read that shouldn't significantly impact overall
performance.
V2:
- Added a check on the number of I2C Defers to limit the number
of times that the retries variable will be decremented. This
is to address review feedback regarding possible infinite loops
from misbehaving sink devices.
V3:
- Fixed the limit value to 7 instead of 8 to get the correct retry
count.
- Combined the increment of the defer count into the if-statement
Signed-off-by: Todd Previte
Cc: dri-devel at lists.freedesktop.org
---
drivers/gpu/drm/drm_dp_helper.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index 79968e3..9ecfd27 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -468,7 +468,12 @@ static int drm_dp_i2c_do_msg(struct drm_dp_aux *aux,
struct drm_dp_aux_msg *msg)
case DP_AUX_I2C_REPLY_DEFER:
DRM_DEBUG_KMS("I2C defer\n");
- aux->i2c_defer_count++;
+ /* DP Compliance Test 4.2.2.5 Requirement:
+* Must have at least 7 retries for I2C defers on the
+* transaction to pass this test
+*/
+ if (++aux->i2c_defer_count < 7)
+ retry = 0;
usleep_range(400, 500);
continue;
--
1.9.1
[PATCH 06/11] drm/i915: Add supporting structure for Displayport Link CTS test 4.2.2.6
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
Signed-off-by: Todd Previte
Cc: dri-devel at lists.freedesktop.org
---
drivers/gpu/drm/drm_edid.c| 31 ++-
drivers/gpu/drm/drm_edid_load.c | 7 +--
drivers/gpu/drm/i2c/tda998x_drv.c | 4 ++--
include/drm/drm_crtc.h| 8 +++-
4 files changed, 40 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 53bc7a6..12e5be7 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -1005,7 +1005,6 @@ int drm_edid_header_is_valid(const u8 *raw_edid)
for (i = 0; i < sizeof(edid_header); i++)
if (raw_edid[i] == edid_header[i])
score++;
-
return score;
}
EXPORT_SYMBOL(drm_edid_header_is_valid);
@@ -1047,7 +1046,8 @@ static bool drm_edid_is_zero(const u8 *in_edid, int
length)
*
* 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 +1062,27 @@ 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 then
correct
+* and the test fails
+*/
+ csum = drm_edid_block_checksum(raw_edid);
+ if (csum) {
+ DRM_DEBUG_DRIVER("Invalid EDID header, score =
%d\n", score);
+ DRM_DEBUG_DRIVER("Invalid EDID checksum %d\n",
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++) {
[Intel-gfx] [PATCH 4/9] drm/i915: Add check for corrupt raw EDID header for Displayport compliance testing
On 4/8/2015 3:37 PM, Paulo Zanoni wrote:
> 2015-04-08 18:43 GMT-03:00 Todd Previte:
>> On 4/8/2015 9:51 AM, Paulo Zanoni wrote:
>>> 2015-03-31 14:15 GMT-03:00 Todd Previte:
>>>> Displayport compliance test 4.2.2.6 requires that a source device be
>>>> capable of detecting
>>>> a corrupt EDID. To do this, the test sets up an invalid EDID header to be
>>>> read by the source
>>>> device. Unfortunately, the DRM EDID reading and parsing functions are
>>>> actually too good in
>>>> this case and prevent the source from reading the corrupted EDID. The
>>>> result is a failed
>>>> compliance test.
>>>>
>>>> In order to successfully pass the test, the raw EDID header must be
>>>> checked on each read
>>>> to see if has been "corrupted". If an invalid raw header is detected, a
>>>> flag is set that
>>>> allows the compliance testing code to acknowledge that fact and react
>>>> appropriately. The
>>>> flag is automatically cleared on read.
>>>>
>>>> This code is designed to expressly work for compliance testing without
>>>> disrupting normal
>>>> operations for EDID reading and parsing.
>>>>
>>>> Signed-off-by: Todd Previte
>>>> Cc:dri-devel at lists.freedesktop.org
>>>> ---
>>>>drivers/gpu/drm/drm_edid.c | 33 +
>>>>drivers/gpu/drm/i915/intel_dp.c | 17 +
>>>>drivers/gpu/drm/i915/intel_drv.h | 1 +
>>>>include/drm/drm_edid.h | 5 +
>>>>4 files changed, 56 insertions(+)
>>>>
>>>> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
>>>> index 53bc7a6..3d4f473 100644
>>>> --- a/drivers/gpu/drm/drm_edid.c
>>>> +++ b/drivers/gpu/drm/drm_edid.c
>>>> @@ -990,6 +990,32 @@ static const u8 edid_header[] = {
>>>> 0x00, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x00
>>>>};
>>>>
>>>> +
>>>> +/* Flag for EDID corruption testing
>>>> + * Displayport Link CTS Core 1.2 rev1.1 - 4.2.2.6
>>>> + */
>>>> +static bool raw_edid_header_corrupted;
>>> A static variable like this is not a good design, especially for a
>>> module like drm.ko. If you really need this, please store it inside
>>> some struct. But see below first.
>> Per our discussion this morning, I concur. This has been removed in favor of
>> a different solution that uses a new boolean flag in the drm_connector
>> struct.
>>
>> Capturing more of the discussion here, the static boolean was a bad idea to
>> begin with and needed to be removed. One solution was to make the flag
>> non-static and non-clear-on-read, then add a separate clear() function. But
>> it still had the problem of potential misuse other places in the code. The
>> current solution (which will be posted with V5) modifies the is_valid()
>> function and adds a flag in the drm_connector struct that can be used to
>> detect this low-level header corruption.
>>
>>
>>>> +
>>>> +/**
>>>> + * drm_raw_edid_header_valid - check to see if the raw header is
>>>> + * corrupt or not. Used solely for Displayport compliance
>>>> + * testing and required by Link CTS Core 1.2 rev1.1 4.2.2.6.
>>>> + * @raw_edid: pointer to raw base EDID block
>>>> + *
>>>> + * Indicates whether the original EDID header as read from the
>>>> + * device was corrupt or not. Clears on read.
>>>> + *
>>>> + * Return: true if the raw header was corrupt, otherwise false
>>>> + */
>>>> +bool drm_raw_edid_header_corrupt(void)
>>>> +{
>>>> + bool corrupted = raw_edid_header_corrupted;
>>>> +
>>>> + raw_edid_header_corrupted = 0;
>>>> + return corrupted;
>>>> +}
>>>> +EXPORT_SYMBOL(drm_raw_edid_header_corrupt);
>>>> +
>>>>/**
>>>> * drm_edid_header_is_valid - sanity check the header of the base EDID
>>>> block
>>>> * @raw_edid: pointer to raw base EDID block
>>>> @@ -1006,6 +1032,13 @@ int drm_edid_header_is_valid(const u8 *raw_edid)
>>>> if (raw_edid[i] == edid_header[i])
>>>> score++;
>>>>
>>>> + if (score != 8) {
>>>> + /* Log and se
[Intel-gfx] [PATCH 4/9] drm/i915: Add check for corrupt raw EDID header for Displayport compliance testing
On 4/8/2015 9:51 AM, Paulo Zanoni wrote:
> 2015-03-31 14:15 GMT-03:00 Todd Previte :
>> Displayport compliance test 4.2.2.6 requires that a source device be capable
>> of detecting
>> a corrupt EDID. To do this, the test sets up an invalid EDID header to be
>> read by the source
>> device. Unfortunately, the DRM EDID reading and parsing functions are
>> actually too good in
>> this case and prevent the source from reading the corrupted EDID. The result
>> is a failed
>> compliance test.
>>
>> In order to successfully pass the test, the raw EDID header must be checked
>> on each read
>> to see if has been "corrupted". If an invalid raw header is detected, a flag
>> is set that
>> allows the compliance testing code to acknowledge that fact and react
>> appropriately. The
>> flag is automatically cleared on read.
>>
>> This code is designed to expressly work for compliance testing without
>> disrupting normal
>> operations for EDID reading and parsing.
>>
>> Signed-off-by: Todd Previte
>> Cc: dri-devel at lists.freedesktop.org
>> ---
>> drivers/gpu/drm/drm_edid.c | 33 +
>> drivers/gpu/drm/i915/intel_dp.c | 17 +
>> drivers/gpu/drm/i915/intel_drv.h | 1 +
>> include/drm/drm_edid.h | 5 +
>> 4 files changed, 56 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
>> index 53bc7a6..3d4f473 100644
>> --- a/drivers/gpu/drm/drm_edid.c
>> +++ b/drivers/gpu/drm/drm_edid.c
>> @@ -990,6 +990,32 @@ static const u8 edid_header[] = {
>> 0x00, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x00
>> };
>>
>> +
>> +/* Flag for EDID corruption testing
>> + * Displayport Link CTS Core 1.2 rev1.1 - 4.2.2.6
>> + */
>> +static bool raw_edid_header_corrupted;
> A static variable like this is not a good design, especially for a
> module like drm.ko. If you really need this, please store it inside
> some struct. But see below first.
Per our discussion this morning, I concur. This has been removed in
favor of a different solution that uses a new boolean flag in the
drm_connector struct.
Capturing more of the discussion here, the static boolean was a bad idea
to begin with and needed to be removed. One solution was to make the
flag non-static and non-clear-on-read, then add a separate clear()
function. But it still had the problem of potential misuse other places
in the code. The current solution (which will be posted with V5)
modifies the is_valid() function and adds a flag in the drm_connector
struct that can be used to detect this low-level header corruption.
>
>> +
>> +/**
>> + * drm_raw_edid_header_valid - check to see if the raw header is
>> + * corrupt or not. Used solely for Displayport compliance
>> + * testing and required by Link CTS Core 1.2 rev1.1 4.2.2.6.
>> + * @raw_edid: pointer to raw base EDID block
>> + *
>> + * Indicates whether the original EDID header as read from the
>> + * device was corrupt or not. Clears on read.
>> + *
>> + * Return: true if the raw header was corrupt, otherwise false
>> + */
>> +bool drm_raw_edid_header_corrupt(void)
>> +{
>> + bool corrupted = raw_edid_header_corrupted;
>> +
>> + raw_edid_header_corrupted = 0;
>> + return corrupted;
>> +}
>> +EXPORT_SYMBOL(drm_raw_edid_header_corrupt);
>> +
>> /**
>>* drm_edid_header_is_valid - sanity check the header of the base EDID
>> block
>>* @raw_edid: pointer to raw base EDID block
>> @@ -1006,6 +1032,13 @@ int drm_edid_header_is_valid(const u8 *raw_edid)
>> if (raw_edid[i] == edid_header[i])
>> score++;
>>
>> + if (score != 8) {
>> + /* Log and set flag here for EDID corruption testing
>> +* Displayport Link CTS Core 1.2 rev1.1 - 4.2.2.6
>> +*/
>> + DRM_DEBUG_DRIVER("Raw EDID header invalid\n");
>> + raw_edid_header_corrupted = 1;
>> + }
> The problem is that here we're limiting ourselves to just a bad edid
> header, not a bad edid in general, so there are many things which we
> might not get - such as a simple wrong checksum edid value. I remember
> that on the previous patch you calculated the whole checksum manually,
> but I don't see that code anymore. What was the reason for the change?
So this code is specifically for the 4.2.2.6 compliance test that is
looking for nothing more than an invalid EDID header. In fact, the test
unit onl
[PATCH 07/11] drm/i915: Fix for DP CTS test 4.2.2.5 - I2C DEFER handling
On 4/7/15 7:29 AM, Paulo Zanoni wrote:
> 2015-04-06 23:11 GMT-03:00 Todd Previte :
>> For test 4.2.2.5 to pass per the Link CTS Core 1.2 rev1.1 spec, the source
>> device must attempt at least 7 times to read the EDID when it receives an
>> I2C defer. The normal DRM code makes only 7 retries, regardless of whether
>> or not the response is a native defer or an I2C defer. Test 4.2.2.5 fails
>> since there are native defers interspersed with the I2C defers which
>> results in less than 7 EDID read attempts.
>>
>> The solution is to decrement the retry counter when an I2C DEFER is returned
>> such that another read attempt will be made. This situation should normally
>> only occur in compliance testing, however, as a worse case real-world
>> scenario, it would result in 13 attempts ( 6 native defers, 7 I2C defers)
>> for a single transaction to complete. The net result is a slightly slower
>> response to an EDID read that shouldn't significantly impact overall
>> performance.
>>
>> V2:
>> - Added a check on the number of I2C Defers to limit the number
>>of times that the retries variable will be decremented. This
>>is to address review feedback regarding possible infinite loops
>>from misbehaving sink devices.
>>
>> Signed-off-by: Todd Previte
>> Cc: dri-devel at lists.freedesktop.org
>> ---
>> drivers/gpu/drm/drm_dp_helper.c | 6 ++
>> 1 file changed, 6 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_dp_helper.c
>> b/drivers/gpu/drm/drm_dp_helper.c
>> index 79968e3..23025cf 100644
>> --- a/drivers/gpu/drm/drm_dp_helper.c
>> +++ b/drivers/gpu/drm/drm_dp_helper.c
>> @@ -469,6 +469,12 @@ static int drm_dp_i2c_do_msg(struct drm_dp_aux *aux,
>> struct drm_dp_aux_msg *msg)
>> case DP_AUX_I2C_REPLY_DEFER:
>> DRM_DEBUG_KMS("I2C defer\n");
>> aux->i2c_defer_count++;
>> + /* DP Compliance Test 4.2.2.5 Requirement:
>> +* Must have at least 7 retries for I2C defers on the
>> +* transaction to pass this test
>> +*/
>> + if (aux->i2c_defer_count < 8)
> I don't think this is the way to go. During normal
> (non-compliance-testing) operation we never zero i2c_defer_count, so
> we can't expect this to work, since we may start drm_dp_i2c_do_msg
> with a i2c_defer_count value different than zero. Also, during i915.ko
> DP compliance we only zero i2c_defer_count at the very beginning of
> each test, not at every aux transaction, and I really think we need a
> solution that is not specific to compliance testing.
To capture the discussion from IRC:
The primary issue previously was the potential for an infinite loop when
decrementing the retry count. That is clearly addressed by this code, by
only decrementing the loop while the defer count is below 8. In
actuality, that needs to be 7, so a followup patch with that change will
be posted shortly. There are other solutions (Ville has one in his
reply), but this one works correctly, is minimally invasive and doesn't
require changes to the loop structure.
The defer counter isn't used anywhere outside of Displayport compliance
testing. In fact, the counters in the aux struct didn't even exist until
I put them in there in a previous patch to support this exact test case.
So there really isn't a non-DP compliance test specific solution to be
had here. It's also unlikely that this has any significant effect on
normal operations. In the event that a device misbehaves and begins
issuing loads of defers, this code would only delay it by at most 7
retries before the counter exceeded its value.
Since this value is not used outside of compliance testing, a reset
per-transaction is unnecessary. Additionally, that would break the EDID
compliance testing code as it would have no way of detecting that the
read had failed due to continuous defers. As long as the counter is
reset for each test request (which it is), this code will function
properly for both real world and compliance operations.
As indicated in the comments, this code exists to ensure compliance with
test 4.2.2.5 from the Displayport Link CTS Core 1.2 rev1.1. This test
verifies that the source device is capable of responding to a failure to
read the EDID when the sink device continuously defers the transaction.
It ensures that at least 7 I2C defers are received before calling it
quits, versus simply retrying the transaction 7 times regardless of the
failure mode.
Since I'm going to post an updated patch anyways, I will likely roll the
defer count increment into the if-statement to remove one line of cod
[PATCH 07/11] drm/i915: Fix for DP CTS test 4.2.2.5 - I2C DEFER handling
For test 4.2.2.5 to pass per the Link CTS Core 1.2 rev1.1 spec, the source
device must attempt at least 7 times to read the EDID when it receives an
I2C defer. The normal DRM code makes only 7 retries, regardless of whether
or not the response is a native defer or an I2C defer. Test 4.2.2.5 fails
since there are native defers interspersed with the I2C defers which
results in less than 7 EDID read attempts.
The solution is to decrement the retry counter when an I2C DEFER is returned
such that another read attempt will be made. This situation should normally
only occur in compliance testing, however, as a worse case real-world
scenario, it would result in 13 attempts ( 6 native defers, 7 I2C defers)
for a single transaction to complete. The net result is a slightly slower
response to an EDID read that shouldn't significantly impact overall
performance.
V2:
- Added a check on the number of I2C Defers to limit the number
of times that the retries variable will be decremented. This
is to address review feedback regarding possible infinite loops
from misbehaving sink devices.
Signed-off-by: Todd Previte
Cc: dri-devel at lists.freedesktop.org
---
drivers/gpu/drm/drm_dp_helper.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index 79968e3..23025cf 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -469,6 +469,12 @@ static int drm_dp_i2c_do_msg(struct drm_dp_aux *aux,
struct drm_dp_aux_msg *msg)
case DP_AUX_I2C_REPLY_DEFER:
DRM_DEBUG_KMS("I2C defer\n");
aux->i2c_defer_count++;
+ /* DP Compliance Test 4.2.2.5 Requirement:
+* Must have at least 7 retries for I2C defers on the
+* transaction to pass this test
+*/
+ if (aux->i2c_defer_count < 8)
+ retry--;
usleep_range(400, 500);
continue;
--
1.9.1
[PATCH 7/9] drm/i915: Fix for DP CTS test 4.2.2.5 - I2C DEFER handling
On 4/6/15 5:05 PM, Paulo Zanoni wrote:
> 2015-03-31 14:15 GMT-03:00 Todd Previte :
>> For test 4.2.2.5 to pass per the Link CTS Core 1.2 rev1.1 spec, the source
>> device must attempt at least 7 times to read the EDID when it receives an
>> I2C defer. The normal DRM code makes only 7 retries, regardless of whether
>> or not the response is a native defer or an I2C defer. Test 4.2.2.5 fails
>> since there are native defers interspersed with the I2C defers which
>> results in less than 7 EDID read attempts.
>>
>> The solution is to decrement the retry counter when an I2C DEFER is returned
>> such that another read attempt will be made. This situation should normally
>> only occur in compliance testing, however, as a worse case real-world
>> scenario, it would result in 13 attempts ( 6 native defers, 7 I2C defers)
>> for a single transaction to complete. The net result is a slightly slower
>> response to an EDID read that shouldn't significantly impact overall
>> performance.
>>
>> Signed-off-by: Todd Previte
>> Cc: dri-devel at lists.freedesktop.org
>> ---
>> drivers/gpu/drm/drm_dp_helper.c | 5 +
>> 1 file changed, 5 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_dp_helper.c
>> b/drivers/gpu/drm/drm_dp_helper.c
>> index 79968e3..0539758 100644
>> --- a/drivers/gpu/drm/drm_dp_helper.c
>> +++ b/drivers/gpu/drm/drm_dp_helper.c
>> @@ -469,6 +469,11 @@ static int drm_dp_i2c_do_msg(struct drm_dp_aux *aux,
>> struct drm_dp_aux_msg *msg)
>> case DP_AUX_I2C_REPLY_DEFER:
>> DRM_DEBUG_KMS("I2C defer\n");
>> aux->i2c_defer_count++;
>> + /* DP Compliance Test 4.2.2.5 Requirement:
>> +* Must have at least 7 retries for I2C defers on the
>> +* transaction to pass this test
>> +*/
>> + retry--;
> I wouldn't be surprised if someone discovers a monitor or some sort of
> dongle that keeps sending I2C defer errors forever, keeping us in an
> infinite loop. Shouldn't we count each error in separate? Or maybe
> just loop up to 14 times, in case that doesn't violate any spec (I
> didn't check)?
I think the safest thing to do would be to put a failsafe on the
i2c_defer_counter. That would ensure that the compliance test gets its 7
retries and that if we do encounter a misbehaving device, the driver
won't let the unending defers cause an infinite loop. Updated patch
shortly.
>> usleep_range(400, 500);
>> continue;
>>
>> --
>> 1.9.1
>>
>> ___
>> dri-devel mailing list
>> dri-devel at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>
>
[PATCH 1/5] drm/i915: Fix for DP CTS test 4.2.2.5 - I2C DEFER handling
For test 4.2.2.5 to pass per the Link CTS Core 1.2 rev1.1 spec, the source
device must attempt at least 7 times to read the EDID when it receives an
I2C defer. The normal DRM code makes only 7 retries, regardless of whether
or not the response is a native defer or an I2C defer. Test 4.2.2.5 fails
since there are native defers interspersed with the I2C defers which
results in less than 7 EDID read attempts.
The solution is to decrement the retry counter when an I2C DEFER is returned
such that another read attempt will be made. This situation should normally
only occur in compliance testing, however, as a worse case real-world
scenario, it would result in 13 attempts ( 6 native defers, 7 I2C defers)
for a single transaction to complete. The net result is a slightly slower
response to an EDID read that shouldn't significantly impact overall
performance.
Signed-off-by: Todd Previte
Cc: dri-devel at lists.freedesktop.org
---
drivers/gpu/drm/drm_dp_helper.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index 79968e3..0539758 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -469,6 +469,11 @@ static int drm_dp_i2c_do_msg(struct drm_dp_aux *aux,
struct drm_dp_aux_msg *msg)
case DP_AUX_I2C_REPLY_DEFER:
DRM_DEBUG_KMS("I2C defer\n");
aux->i2c_defer_count++;
+ /* DP Compliance Test 4.2.2.5 Requirement:
+* Must have at least 7 retries for I2C defers on the
+* transaction to pass this test
+*/
+ retry--;
usleep_range(400, 500);
continue;
--
1.9.1
[PATCH 9/9] drm: Fix the 'native defer' message in drm_dp_i2c_do_msg()
The debug message is missing a newline at the end and it makes the
logs hard to read when a device defers a lot. Simple 2-character fix
adds the newline at the end.
Signed-off-by: Todd Previte
Cc: dri-devel at lists.freedesktop.org
---
drivers/gpu/drm/drm_dp_helper.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index 0539758..281bb67 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -433,7 +433,7 @@ static int drm_dp_i2c_do_msg(struct drm_dp_aux *aux, struct
drm_dp_aux_msg *msg)
return -EREMOTEIO;
case DP_AUX_NATIVE_REPLY_DEFER:
- DRM_DEBUG_KMS("native defer");
+ DRM_DEBUG_KMS("native defer\n");
/*
* We could check for I2C bit rate capabilities and if
* available adjust this interval. We could also be
--
1.9.1
[PATCH 7/9] drm/i915: Fix for DP CTS test 4.2.2.5 - I2C DEFER handling
For test 4.2.2.5 to pass per the Link CTS Core 1.2 rev1.1 spec, the source
device must attempt at least 7 times to read the EDID when it receives an
I2C defer. The normal DRM code makes only 7 retries, regardless of whether
or not the response is a native defer or an I2C defer. Test 4.2.2.5 fails
since there are native defers interspersed with the I2C defers which
results in less than 7 EDID read attempts.
The solution is to decrement the retry counter when an I2C DEFER is returned
such that another read attempt will be made. This situation should normally
only occur in compliance testing, however, as a worse case real-world
scenario, it would result in 13 attempts ( 6 native defers, 7 I2C defers)
for a single transaction to complete. The net result is a slightly slower
response to an EDID read that shouldn't significantly impact overall
performance.
Signed-off-by: Todd Previte
Cc: dri-devel at lists.freedesktop.org
---
drivers/gpu/drm/drm_dp_helper.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index 79968e3..0539758 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -469,6 +469,11 @@ static int drm_dp_i2c_do_msg(struct drm_dp_aux *aux,
struct drm_dp_aux_msg *msg)
case DP_AUX_I2C_REPLY_DEFER:
DRM_DEBUG_KMS("I2C defer\n");
aux->i2c_defer_count++;
+ /* DP Compliance Test 4.2.2.5 Requirement:
+* Must have at least 7 retries for I2C defers on the
+* transaction to pass this test
+*/
+ retry--;
usleep_range(400, 500);
continue;
--
1.9.1
[PATCH 4/9] drm/i915: Add check for corrupt raw EDID header for Displayport compliance testing
Displayport compliance test 4.2.2.6 requires that a source device be capable of
detecting
a corrupt EDID. To do this, the test sets up an invalid EDID header to be read
by the source
device. Unfortunately, the DRM EDID reading and parsing functions are actually
too good in
this case and prevent the source from reading the corrupted EDID. The result is
a failed
compliance test.
In order to successfully pass the test, the raw EDID header must be checked on
each read
to see if has been "corrupted". If an invalid raw header is detected, a flag is
set that
allows the compliance testing code to acknowledge that fact and react
appropriately. The
flag is automatically cleared on read.
This code is designed to expressly work for compliance testing without
disrupting normal
operations for EDID reading and parsing.
Signed-off-by: Todd Previte
Cc: dri-devel at lists.freedesktop.org
---
drivers/gpu/drm/drm_edid.c | 33 +
drivers/gpu/drm/i915/intel_dp.c | 17 +
drivers/gpu/drm/i915/intel_drv.h | 1 +
include/drm/drm_edid.h | 5 +
4 files changed, 56 insertions(+)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 53bc7a6..3d4f473 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -990,6 +990,32 @@ static const u8 edid_header[] = {
0x00, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x00
};
+
+/* Flag for EDID corruption testing
+ * Displayport Link CTS Core 1.2 rev1.1 - 4.2.2.6
+ */
+static bool raw_edid_header_corrupted;
+
+/**
+ * drm_raw_edid_header_valid - check to see if the raw header is
+ * corrupt or not. Used solely for Displayport compliance
+ * testing and required by Link CTS Core 1.2 rev1.1 4.2.2.6.
+ * @raw_edid: pointer to raw base EDID block
+ *
+ * Indicates whether the original EDID header as read from the
+ * device was corrupt or not. Clears on read.
+ *
+ * Return: true if the raw header was corrupt, otherwise false
+ */
+bool drm_raw_edid_header_corrupt(void)
+{
+ bool corrupted = raw_edid_header_corrupted;
+
+ raw_edid_header_corrupted = 0;
+ return corrupted;
+}
+EXPORT_SYMBOL(drm_raw_edid_header_corrupt);
+
/**
* drm_edid_header_is_valid - sanity check the header of the base EDID block
* @raw_edid: pointer to raw base EDID block
@@ -1006,6 +1032,13 @@ int drm_edid_header_is_valid(const u8 *raw_edid)
if (raw_edid[i] == edid_header[i])
score++;
+ if (score != 8) {
+ /* Log and set flag here for EDID corruption testing
+* Displayport Link CTS Core 1.2 rev1.1 - 4.2.2.6
+*/
+ DRM_DEBUG_DRIVER("Raw EDID header invalid\n");
+ raw_edid_header_corrupted = 1;
+ }
return score;
}
EXPORT_SYMBOL(drm_edid_header_is_valid);
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index dc87276..57f8e43 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -3824,6 +3824,9 @@ update_status:
, 1);
if (status <= 0)
DRM_DEBUG_KMS("Could not write test response to sink\n");
+
+ /* Clear flag here, after testing is complete*/
+ intel_dp->compliance_edid_invalid = 0;
}
static int
@@ -3896,6 +3899,10 @@ intel_dp_check_link_status(struct intel_dp *intel_dp)
{
struct drm_device *dev = intel_dp_to_dev(intel_dp);
struct intel_encoder *intel_encoder = _to_dig_port(intel_dp)->base;
+ struct drm_connector *connector = _dp->attached_connector->base;
+ struct i2c_adapter *adapter = _dp->aux.ddc;
+ struct edid *edid_read = NULL;
+
u8 sink_irq_vector;
u8 link_status[DP_LINK_STATUS_SIZE];
@@ -3912,6 +3919,16 @@ intel_dp_check_link_status(struct intel_dp *intel_dp)
return;
}
+ /* Compliance testing requires an EDID read for all HPD events
+* Link CTS Core 1.2 rev 1.1: Test 4.2.2.1
+* Flag set here will be handled in the EDID test function
+*/
+ edid_read = drm_get_edid(connector, adapter);
+ if (!edid_read || drm_raw_edid_header_corrupt() == 1) {
+ DRM_DEBUG_DRIVER("EDID invalid, setting flag\n");
+ intel_dp->compliance_edid_invalid = 1;
+ }
+
/* Try to read the source of the interrupt */
if (intel_dp->dpcd[DP_DPCD_REV] >= 0x11 &&
intel_dp_get_sink_irq(intel_dp, _irq_vector)) {
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index e7b62be..42e4251 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -651,6 +651,7 @@ struct intel_dp {
/* Displayport compliance testing */
unsigned long compliance_test_type;
bool compliance_testing_active;
+ bool compliance_edid_inval
[PATCH 9/9] drm: Fix the 'native defer' message in drm_dp_i2c_do_msg()
The debug message is missing a newline at the end and it makes the
logs hard to read when a device defers a lot. Simple 2-character fix
adds the newline at the end.
Signed-off-by: Todd Previte
Cc: dri-devel at lists.freedesktop.org
---
drivers/gpu/drm/drm_dp_helper.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index 0539758..281bb67 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -433,7 +433,7 @@ static int drm_dp_i2c_do_msg(struct drm_dp_aux *aux, struct
drm_dp_aux_msg *msg)
return -EREMOTEIO;
case DP_AUX_NATIVE_REPLY_DEFER:
- DRM_DEBUG_KMS("native defer");
+ DRM_DEBUG_KMS("native defer\n");
/*
* We could check for I2C bit rate capabilities and if
* available adjust this interval. We could also be
--
1.9.1
[PATCH 7/9] drm/i915: Fix for DP CTS test 4.2.2.5 - I2C DEFER handling
For test 4.2.2.5 to pass per the Link CTS Core 1.2 rev1.1 spec, the source
device must attempt at least 7 times to read the EDID when it receives an
I2C defer. The normal DRM code makes only 7 retries, regardless of whether
or not the response is a native defer or an I2C defer. Test 4.2.2.5 fails
since there are native defers interspersed with the I2C defers which
results in less than 7 EDID read attempts.
The solution is to decrement the retry counter when an I2C DEFER is returned
such that another read attempt will be made. This situation should normally
only occur in compliance testing, however, as a worse case real-world
scenario, it would result in 13 attempts ( 6 native defers, 7 I2C defers)
for a single transaction to complete. The net result is a slightly slower
response to an EDID read that shouldn't significantly impact overall
performance.
Signed-off-by: Todd Previte
Cc: dri-devel at lists.freedesktop.org
---
drivers/gpu/drm/drm_dp_helper.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index 79968e3..0539758 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -469,6 +469,11 @@ static int drm_dp_i2c_do_msg(struct drm_dp_aux *aux,
struct drm_dp_aux_msg *msg)
case DP_AUX_I2C_REPLY_DEFER:
DRM_DEBUG_KMS("I2C defer\n");
aux->i2c_defer_count++;
+ /* DP Compliance Test 4.2.2.5 Requirement:
+* Must have at least 7 retries for I2C defers on the
+* transaction to pass this test
+*/
+ retry--;
usleep_range(400, 500);
continue;
--
1.9.1
[PATCH 4/9] drm/i915: Add check for corrupt raw EDID header for Displayport compliance testing
Displayport compliance test 4.2.2.6 requires that a source device be capable of
detecting
a corrupt EDID. To do this, the test sets up an invalid EDID header to be read
by the source
device. Unfortunately, the DRM EDID reading and parsing functions are actually
too good in
this case and prevent the source from reading the corrupted EDID. The result is
a failed
compliance test.
In order to successfully pass the test, the raw EDID header must be checked on
each read
to see if has been "corrupted". If an invalid raw header is detected, a flag is
set that
allows the compliance testing code to acknowledge that fact and react
appropriately. The
flag is automatically cleared on read.
This code is designed to expressly work for compliance testing without
disrupting normal
operations for EDID reading and parsing.
Signed-off-by: Todd Previte
Cc: dri-devel at lists.freedesktop.org
---
drivers/gpu/drm/drm_edid.c | 33 +
drivers/gpu/drm/i915/intel_dp.c | 17 +
drivers/gpu/drm/i915/intel_drv.h | 1 +
include/drm/drm_edid.h | 5 +
4 files changed, 56 insertions(+)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 53bc7a6..3d4f473 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -990,6 +990,32 @@ static const u8 edid_header[] = {
0x00, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x00
};
+
+/* Flag for EDID corruption testing
+ * Displayport Link CTS Core 1.2 rev1.1 - 4.2.2.6
+ */
+static bool raw_edid_header_corrupted;
+
+/**
+ * drm_raw_edid_header_valid - check to see if the raw header is
+ * corrupt or not. Used solely for Displayport compliance
+ * testing and required by Link CTS Core 1.2 rev1.1 4.2.2.6.
+ * @raw_edid: pointer to raw base EDID block
+ *
+ * Indicates whether the original EDID header as read from the
+ * device was corrupt or not. Clears on read.
+ *
+ * Return: true if the raw header was corrupt, otherwise false
+ */
+bool drm_raw_edid_header_corrupt(void)
+{
+ bool corrupted = raw_edid_header_corrupted;
+
+ raw_edid_header_corrupted = 0;
+ return corrupted;
+}
+EXPORT_SYMBOL(drm_raw_edid_header_corrupt);
+
/**
* drm_edid_header_is_valid - sanity check the header of the base EDID block
* @raw_edid: pointer to raw base EDID block
@@ -1006,6 +1032,13 @@ int drm_edid_header_is_valid(const u8 *raw_edid)
if (raw_edid[i] == edid_header[i])
score++;
+ if (score != 8) {
+ /* Log and set flag here for EDID corruption testing
+* Displayport Link CTS Core 1.2 rev1.1 - 4.2.2.6
+*/
+ DRM_DEBUG_DRIVER("Raw EDID header invalid\n");
+ raw_edid_header_corrupted = 1;
+ }
return score;
}
EXPORT_SYMBOL(drm_edid_header_is_valid);
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index dc87276..57f8e43 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -3824,6 +3824,9 @@ update_status:
, 1);
if (status <= 0)
DRM_DEBUG_KMS("Could not write test response to sink\n");
+
+ /* Clear flag here, after testing is complete*/
+ intel_dp->compliance_edid_invalid = 0;
}
static int
@@ -3896,6 +3899,10 @@ intel_dp_check_link_status(struct intel_dp *intel_dp)
{
struct drm_device *dev = intel_dp_to_dev(intel_dp);
struct intel_encoder *intel_encoder = _to_dig_port(intel_dp)->base;
+ struct drm_connector *connector = _dp->attached_connector->base;
+ struct i2c_adapter *adapter = _dp->aux.ddc;
+ struct edid *edid_read = NULL;
+
u8 sink_irq_vector;
u8 link_status[DP_LINK_STATUS_SIZE];
@@ -3912,6 +3919,16 @@ intel_dp_check_link_status(struct intel_dp *intel_dp)
return;
}
+ /* Compliance testing requires an EDID read for all HPD events
+* Link CTS Core 1.2 rev 1.1: Test 4.2.2.1
+* Flag set here will be handled in the EDID test function
+*/
+ edid_read = drm_get_edid(connector, adapter);
+ if (!edid_read || drm_raw_edid_header_corrupt() == 1) {
+ DRM_DEBUG_DRIVER("EDID invalid, setting flag\n");
+ intel_dp->compliance_edid_invalid = 1;
+ }
+
/* Try to read the source of the interrupt */
if (intel_dp->dpcd[DP_DPCD_REV] >= 0x11 &&
intel_dp_get_sink_irq(intel_dp, _irq_vector)) {
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index e7b62be..42e4251 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -651,6 +651,7 @@ struct intel_dp {
/* Displayport compliance testing */
unsigned long compliance_test_type;
bool compliance_testing_active;
+ bool compliance_edid_inval
[PATCH 02/10] drm/i915: Add counters in the drm_dp_aux struct for I2C NACKs and DEFERs
These counters are used for Displayort compliance testing to detect error
conditions when executing tests 4.2.2.4 and 4.2.2.5 in the Displayport Link
CTS specificaiton. They determine whether to use the preferred/requested
mode or the failsafe mode during these tests.
V2:
- Addressed previous review feedback
- Updated commit message
- Changed from uint8_t to uint32_t
Cc: dri-devel at lists.freedesktop.org
Signed-off-by: Todd Previte
---
drivers/gpu/drm/drm_dp_helper.c | 2 ++
include/drm/drm_dp_helper.h | 1 +
2 files changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index 08e33b8..8353051 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -654,10 +654,12 @@ static int drm_dp_i2c_do_msg(struct drm_dp_aux *aux,
struct drm_dp_aux_msg *msg)
case DP_AUX_I2C_REPLY_NACK:
DRM_DEBUG_KMS("I2C nack\n");
+ aux->i2c_nack_count++;
return -EREMOTEIO;
case DP_AUX_I2C_REPLY_DEFER:
DRM_DEBUG_KMS("I2C defer\n");
+ aux->i2c_defer_count++;
usleep_range(400, 500);
continue;
diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
index 8edeed0..23082ce 100644
--- a/include/drm/drm_dp_helper.h
+++ b/include/drm/drm_dp_helper.h
@@ -551,6 +551,7 @@ struct drm_dp_aux {
struct mutex hw_mutex;
ssize_t (*transfer)(struct drm_dp_aux *aux,
struct drm_dp_aux_msg *msg);
+ uint32_t i2c_nack_count, i2c_defer_count;
};
ssize_t drm_dp_dpcd_read(struct drm_dp_aux *aux, unsigned int offset,
--
1.9.1
[Intel-gfx] [PATCH 02/10] drm/i915: Add counters in the drm_dp_aux struct for I2C NACKs and DEFERs
To address previous feedback, I'll quote below and answer.
>It would be nice if you could cite on the commit message the name of
>the specification and the name of the test(s) that use it.
Done. For reference, in the Displayport Link CTS spec, tests 4.2.2.4 and
4.2.2.5 are the ones that use these, specifically.
>Does it really need to be uint8_t? I see on patch 7 that you don't
>really write this value to a place that only accepts uint8_t-sized
>arguments, so I fear that if we get 256 NACKs or DEFERs we may end up
>doing the wrong thing.
There are no compliance tests that are concerned with the number of
native DEFERs or NACKs received, hence why they are not addressed here.
There's no requirement for the size of this value and I selected uint8_t
as the smallest reasonable size for it. It's only used to count the
number of NACKs and DEFERs during compliance testing and it gets reset
to 0 at the beginning of the test block where it's used in a later
patch. It's unlikely that a case would occur during this compliance test
where exactly 256 NACKs or DEFERs occur, but your point is well-taken.
I'll make them uint32s and be done with it. I don't think 6 extra bytes
is going to run the kernel out of memory. :)
>Also, why don't we need to count the native NACKs and DEFERs?
There are no compliance tests that are concerned with the number of
native DEFERs or NACKs received.
New patch will be here shortly.
-T
On 10/21/2014 10:10 AM, Paulo Zanoni wrote:
> 2014-10-09 12:38 GMT-03:00 Todd Previte :
>> These counters are used for Displayort complinace testing to detect error
>> conditions
>> when executing certain compliance tests. Currently these are used in the
>> EDID tests
>> to determine if the video mode needs to be set to the preferred mode or the
>> failsafe
>> mode.
>>
>> Cc: dri-devel at lists.freedesktop.org
> I see that this patch and a few others in your series still have
> unaddressed/unanswered review comments, given on the first time you
> sent the patches. Please take a look at them.
>
>> Signed-off-by: Todd Previte
>> ---
>> drivers/gpu/drm/drm_dp_helper.c | 2 ++
>> include/drm/drm_dp_helper.h | 1 +
>> 2 files changed, 3 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_dp_helper.c
>> b/drivers/gpu/drm/drm_dp_helper.c
>> index 08e33b8..8353051 100644
>> --- a/drivers/gpu/drm/drm_dp_helper.c
>> +++ b/drivers/gpu/drm/drm_dp_helper.c
>> @@ -654,10 +654,12 @@ static int drm_dp_i2c_do_msg(struct drm_dp_aux *aux,
>> struct drm_dp_aux_msg *msg)
>>
>> case DP_AUX_I2C_REPLY_NACK:
>> DRM_DEBUG_KMS("I2C nack\n");
>> + aux->i2c_nack_count++;
>> return -EREMOTEIO;
>>
>> case DP_AUX_I2C_REPLY_DEFER:
>> DRM_DEBUG_KMS("I2C defer\n");
>> + aux->i2c_defer_count++;
>> usleep_range(400, 500);
>> continue;
>>
>> diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
>> index 8edeed0..45f3ee8 100644
>> --- a/include/drm/drm_dp_helper.h
>> +++ b/include/drm/drm_dp_helper.h
>> @@ -551,6 +551,7 @@ struct drm_dp_aux {
>> struct mutex hw_mutex;
>> ssize_t (*transfer)(struct drm_dp_aux *aux,
>> struct drm_dp_aux_msg *msg);
>> + uint8_t i2c_nack_count, i2c_defer_count;
>> };
>>
>> ssize_t drm_dp_dpcd_read(struct drm_dp_aux *aux, unsigned int offset,
>> --
>> 1.9.1
>>
>> ___
>> Intel-gfx mailing list
>> Intel-gfx at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
>
[PATCH 02/10] drm/i915: Add counters in the drm_dp_aux struct for I2C NACKs and DEFERs
These counters are used for Displayort complinace testing to detect error
conditions
when executing certain compliance tests. Currently these are used in the EDID
tests
to determine if the video mode needs to be set to the preferred mode or the
failsafe
mode.
Cc: dri-devel at lists.freedesktop.org
Signed-off-by: Todd Previte
---
drivers/gpu/drm/drm_dp_helper.c | 2 ++
include/drm/drm_dp_helper.h | 1 +
2 files changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index 08e33b8..8353051 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -654,10 +654,12 @@ static int drm_dp_i2c_do_msg(struct drm_dp_aux *aux,
struct drm_dp_aux_msg *msg)
case DP_AUX_I2C_REPLY_NACK:
DRM_DEBUG_KMS("I2C nack\n");
+ aux->i2c_nack_count++;
return -EREMOTEIO;
case DP_AUX_I2C_REPLY_DEFER:
DRM_DEBUG_KMS("I2C defer\n");
+ aux->i2c_defer_count++;
usleep_range(400, 500);
continue;
diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
index 8edeed0..45f3ee8 100644
--- a/include/drm/drm_dp_helper.h
+++ b/include/drm/drm_dp_helper.h
@@ -551,6 +551,7 @@ struct drm_dp_aux {
struct mutex hw_mutex;
ssize_t (*transfer)(struct drm_dp_aux *aux,
struct drm_dp_aux_msg *msg);
+ uint8_t i2c_nack_count, i2c_defer_count;
};
ssize_t drm_dp_dpcd_read(struct drm_dp_aux *aux, unsigned int offset,
--
1.9.1
[PATCH 02/10] drm/i915: Add counters in the drm_dp_aux struct for I2C NACKs and DEFERs
Sorry for the spam - Ignore the duplicates. There will be one more coming
when I post this series to intel-gfx.
-T
-Original Message-
From: Todd Previte [mailto:tprev...@gmail.com]
Sent: Thursday, October 09, 2014 8:33 AM
To: tprevite at gmail.com
Cc: dri-devel at lists.freedesktop.org
Subject: [PATCH 02/10] drm/i915: Add counters in the drm_dp_aux struct for
I2C NACKs and DEFERs
These counters are used for Displayort complinace testing to detect error
conditions when executing certain compliance tests. Currently these are used
in the EDID tests to determine if the video mode needs to be set to the
preferred mode or the failsafe mode.
Cc: dri-devel at lists.freedesktop.org
Signed-off-by: Todd Previte
---
drivers/gpu/drm/drm_dp_helper.c | 2 ++
include/drm/drm_dp_helper.h | 1 +
2 files changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/drm_dp_helper.c
b/drivers/gpu/drm/drm_dp_helper.c index 08e33b8..8353051 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -654,10 +654,12 @@ static int drm_dp_i2c_do_msg(struct drm_dp_aux *aux,
struct drm_dp_aux_msg *msg)
case DP_AUX_I2C_REPLY_NACK:
DRM_DEBUG_KMS("I2C nack\n");
+ aux->i2c_nack_count++;
return -EREMOTEIO;
case DP_AUX_I2C_REPLY_DEFER:
DRM_DEBUG_KMS("I2C defer\n");
+ aux->i2c_defer_count++;
usleep_range(400, 500);
continue;
diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h index
8edeed0..45f3ee8 100644
--- a/include/drm/drm_dp_helper.h
+++ b/include/drm/drm_dp_helper.h
@@ -551,6 +551,7 @@ struct drm_dp_aux {
struct mutex hw_mutex;
ssize_t (*transfer)(struct drm_dp_aux *aux,
struct drm_dp_aux_msg *msg);
+ uint8_t i2c_nack_count, i2c_defer_count;
};
ssize_t drm_dp_dpcd_read(struct drm_dp_aux *aux, unsigned int offset,
--
1.9.1
[PATCH 02/10] drm/i915: Add counters in the drm_dp_aux struct for I2C NACKs and DEFERs
These counters are used for Displayort complinace testing to detect error
conditions
when executing certain compliance tests. Currently these are used in the EDID
tests
to determine if the video mode needs to be set to the preferred mode or the
failsafe
mode.
Cc: dri-devel at lists.freedesktop.org
Signed-off-by: Todd Previte
---
drivers/gpu/drm/drm_dp_helper.c | 2 ++
include/drm/drm_dp_helper.h | 1 +
2 files changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index 08e33b8..8353051 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -654,10 +654,12 @@ static int drm_dp_i2c_do_msg(struct drm_dp_aux *aux,
struct drm_dp_aux_msg *msg)
case DP_AUX_I2C_REPLY_NACK:
DRM_DEBUG_KMS("I2C nack\n");
+ aux->i2c_nack_count++;
return -EREMOTEIO;
case DP_AUX_I2C_REPLY_DEFER:
DRM_DEBUG_KMS("I2C defer\n");
+ aux->i2c_defer_count++;
usleep_range(400, 500);
continue;
diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
index 8edeed0..45f3ee8 100644
--- a/include/drm/drm_dp_helper.h
+++ b/include/drm/drm_dp_helper.h
@@ -551,6 +551,7 @@ struct drm_dp_aux {
struct mutex hw_mutex;
ssize_t (*transfer)(struct drm_dp_aux *aux,
struct drm_dp_aux_msg *msg);
+ uint8_t i2c_nack_count, i2c_defer_count;
};
ssize_t drm_dp_dpcd_read(struct drm_dp_aux *aux, unsigned int offset,
--
1.9.1
[PATCH 02/10] drm/i915: Add counters in the drm_dp_aux struct for I2C NACKs and DEFERs
These counters are used for Displayort complinace testing to detect error
conditions
when executing certain compliance tests. Currently these are used in the EDID
tests
to determine if the video mode needs to be set to the preferred mode or the
failsafe
mode.
Cc: dri-devel at lists.freedesktop.org
Signed-off-by: Todd Previte
---
drivers/gpu/drm/drm_dp_helper.c | 2 ++
include/drm/drm_dp_helper.h | 1 +
2 files changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index 08e33b8..8353051 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -654,10 +654,12 @@ static int drm_dp_i2c_do_msg(struct drm_dp_aux *aux,
struct drm_dp_aux_msg *msg)
case DP_AUX_I2C_REPLY_NACK:
DRM_DEBUG_KMS("I2C nack\n");
+ aux->i2c_nack_count++;
return -EREMOTEIO;
case DP_AUX_I2C_REPLY_DEFER:
DRM_DEBUG_KMS("I2C defer\n");
+ aux->i2c_defer_count++;
usleep_range(400, 500);
continue;
diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
index 8edeed0..45f3ee8 100644
--- a/include/drm/drm_dp_helper.h
+++ b/include/drm/drm_dp_helper.h
@@ -551,6 +551,7 @@ struct drm_dp_aux {
struct mutex hw_mutex;
ssize_t (*transfer)(struct drm_dp_aux *aux,
struct drm_dp_aux_msg *msg);
+ uint8_t i2c_nack_count, i2c_defer_count;
};
ssize_t drm_dp_dpcd_read(struct drm_dp_aux *aux, unsigned int offset,
--
1.9.1
[PATCH] drm/i915: Fix Sink CRC
Hi Rodrigo,
This patch looks good.
Reviewed-by: Todd Previte
-T
-Original Message-
From: Rodrigo Vivi [mailto:rodrigo.v...@gmail.com]
Sent: Tuesday, September 16, 2014 4:18 PM
To: intel-gfx at lists.freedesktop.org
Cc: dri-devel at lists.freedesktop.org; Rodrigo Vivi; Todd Previte; Daniel
Vetter; Jani Nikula
Subject: [PATCH] drm/i915: Fix Sink CRC
In some cases like when PSR just got enabled the panel need more vblank
times to calculate CRC. I figured that out with the new PSR test cases
facing some cases that I had a green screen but a blank CRC. Even with
2 vblank waits on kernel + 2 vblank waits on test case.
So let's give up to 6 vblank wait time. However we now check for
TEST_CRC_COUNT that shows when panel finished to calculate CRC and has it
ready.
v2: Jani pointed out attempts decrements was wrong and should never reach
the error condition. And Daniel pointed out that EIO is more appropriated
than EGAIN. Also I realized that I have to read test_crc_count after setting
test_sink
v3: Rebase and adding error message
Cc: Todd Previte
Cc: Daniel Vetter
Cc: Jani Nikula
Signed-off-by: Rodrigo Vivi
---
drivers/gpu/drm/i915/intel_dp.c | 23 +--
include/drm/drm_dp_helper.h | 5 +++--
2 files changed, 20 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c
b/drivers/gpu/drm/i915/intel_dp.c index e4d0367..d9091dc7 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -3468,21 +3468,32 @@ int intel_dp_sink_crc(struct intel_dp *intel_dp, u8
*crc)
struct drm_device *dev = intel_dig_port->base.base.dev;
struct intel_crtc *intel_crtc =
to_intel_crtc(intel_dig_port->base.base.crtc);
- u8 buf[1];
+ u8 buf;
+ int test_crc_count;
+ int attempts = 6;
- if (drm_dp_dpcd_readb(_dp->aux, DP_TEST_SINK_MISC, buf) < 0)
+ if (drm_dp_dpcd_readb(_dp->aux, DP_TEST_SINK_MISC, ) < 0)
return -EIO;
- if (!(buf[0] & DP_TEST_CRC_SUPPORTED))
+ if (!(buf & DP_TEST_CRC_SUPPORTED))
return -ENOTTY;
if (drm_dp_dpcd_writeb(_dp->aux, DP_TEST_SINK,
DP_TEST_SINK_START) < 0)
return -EIO;
- /* Wait 2 vblanks to be sure we will have the correct CRC value */
- intel_wait_for_vblank(dev, intel_crtc->pipe);
- intel_wait_for_vblank(dev, intel_crtc->pipe);
+ drm_dp_dpcd_readb(_dp->aux, DP_TEST_SINK_MISC, );
+ test_crc_count = buf & DP_TEST_COUNT_MASK;
+
+ do {
+ drm_dp_dpcd_readb(_dp->aux, DP_TEST_SINK_MISC, );
+ intel_wait_for_vblank(dev, intel_crtc->pipe);
+ } while (--attempts && (buf & DP_TEST_COUNT_MASK) ==
test_crc_count);
+
+ if (attempts == 0) {
+ DRM_ERROR("Panel is unable to calculate CRC after 6
vblanks\n");
+ return -EIO;
+ }
if (drm_dp_dpcd_read(_dp->aux, DP_TEST_CRC_R_CR, crc, 6) < 0)
return -EIO;
diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h index
9305c71..8edeed0 100644
--- a/include/drm/drm_dp_helper.h
+++ b/include/drm/drm_dp_helper.h
@@ -303,7 +303,8 @@
#define DP_TEST_CRC_B_CB 0x244
#define DP_TEST_SINK_MISC 0x246
-#define DP_TEST_CRC_SUPPORTED (1 << 5)
+# define DP_TEST_CRC_SUPPORTED (1 << 5)
+# define DP_TEST_COUNT_MASK0x7
#define DP_TEST_RESPONSE 0x260
# define DP_TEST_ACK (1 << 0)
@@ -313,7 +314,7 @@
#define DP_TEST_EDID_CHECKSUM 0x261
#define DP_TEST_SINK 0x270
-#define DP_TEST_SINK_START (1 << 0)
+# define DP_TEST_SINK_START(1 << 0)
#define DP_PAYLOAD_TABLE_UPDATE_STATUS 0x2c0 /* 1.2 MST */
# define DP_PAYLOAD_TABLE_UPDATED (1 << 0)
--
1.9.3
[Intel-gfx] [PATCH 2/2] drm/i915: rework digital port IRQ handling (v2)
This looks like it's good to go.
As an aside, I don't *think* any of the compliance testing stuff I'm
working on cares whether it's short of long pulse (1.1a compliance), but
it will be interesting to see if/when/where it might have an effect.
Reviewed-by: Todd Previte
> Dave Airlie <mailto:airlied at gmail.com>
> Tuesday, June 17, 2014 6:29 PM
> From: Dave Airlie
>
> The digital ports from Ironlake and up have the ability to distinguish
> between long and short HPD pulses. Displayport 1.1 only uses the short
> form to request link retraining usually, so we haven't really needed
> support for it until now.
>
> However with DP 1.2 MST we need to handle the short irqs on their
> own outside the modesetting locking the long hpd's involve. This
> patch adds the framework to distinguish between short/long to the
> current code base, to lay the basis for future DP 1.2 MST work.
>
> This should mean we get better bisectability in case of regression
> due to the new irq handling.
>
> v2: add GM45 support (untested, due to lack of hw)
>
> Signed-off-by: Dave Airlie
> ---
> drivers/gpu/drm/i915/i915_drv.h | 5 ++
> drivers/gpu/drm/i915/i915_irq.c | 160
> +--
> drivers/gpu/drm/i915/intel_ddi.c | 3 +
> drivers/gpu/drm/i915/intel_dp.c | 20 +
> drivers/gpu/drm/i915/intel_drv.h | 2 +
> 5 files changed, 182 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h
> b/drivers/gpu/drm/i915/i915_drv.h
> index 8f68678..5fd5bf3 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1551,6 +1551,11 @@ struct drm_i915_private {
>
> struct i915_runtime_pm pm;
>
> + struct intel_digital_port *hpd_irq_port[I915_MAX_PORTS];
> + u32 long_hpd_port_mask;
> + u32 short_hpd_port_mask;
> + struct work_struct dig_port_work;
> +
> /* Old dri1 support infrastructure, beware the dragons ya fools entering
> * here! */
> struct i915_dri1_state dri1;
> diff --git a/drivers/gpu/drm/i915/i915_irq.c
> b/drivers/gpu/drm/i915/i915_irq.c
> index cbf31cb..9913c08 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -1096,6 +1096,53 @@ static bool intel_hpd_irq_event(struct
> drm_device *dev,
> return true;
> }
>
> +static void i915_digport_work_func(struct work_struct *work)
> +{
> + struct drm_i915_private *dev_priv =
> + container_of(work, struct drm_i915_private, dig_port_work);
> + unsigned long irqflags;
> + u32 long_port_mask, short_port_mask;
> + struct intel_digital_port *intel_dig_port;
> + int i, ret;
> + u32 old_bits = 0;
> +
> + spin_lock_irqsave(_priv->irq_lock, irqflags);
> + long_port_mask = dev_priv->long_hpd_port_mask;
> + dev_priv->long_hpd_port_mask = 0;
> + short_port_mask = dev_priv->short_hpd_port_mask;
> + dev_priv->short_hpd_port_mask = 0;
> + spin_unlock_irqrestore(_priv->irq_lock, irqflags);
> +
> + for (i = 0; i < I915_MAX_PORTS; i++) {
> + bool valid = false;
> + bool long_hpd = false;
> + intel_dig_port = dev_priv->hpd_irq_port[i];
> + if (!intel_dig_port || !intel_dig_port->hpd_pulse)
> + continue;
> +
> + if (long_port_mask & (1 << i)) {
> + valid = true;
> + long_hpd = true;
> + } else if (short_port_mask & (1 << i))
> + valid = true;
> +
> + if (valid) {
> + ret = intel_dig_port->hpd_pulse(intel_dig_port, long_hpd);
> + if (ret == true) {
> + /* if we get true fallback to old school hpd */
> + old_bits |= (1 << intel_dig_port->base.hpd_pin);
> + }
> + }
> + }
> +
> + if (old_bits) {
> + spin_lock_irqsave(_priv->irq_lock, irqflags);
> + dev_priv->hpd_event_bits |= old_bits;
> + spin_unlock_irqrestore(_priv->irq_lock, irqflags);
> + schedule_work(_priv->hotplug_work);
> + }
> +}
> +
> /*
> * Handle hotplug events outside the interrupt handler proper.
> */
> @@ -1520,23 +1567,104 @@ static irqreturn_t gen8_gt_irq_handler(struct
> drm_device *dev,
> #define HPD_STORM_DETECT_PERIOD 1000
> #define HPD_STORM_THRESHOLD 5
>
> +static int ilk_port_to_hotplug_shift(enum port port)
> +{
> + switch (port) {
> + case PORT_A:
> + case PORT_E:
> + default:
> + return -1;
> + case PORT_B:
> + return 0;
> + case PORT_C:
> + return 8;
> + case PORT_D:
> + return 16;
> + }
> +}
> +
> +static int g4x_port_to_hotplug_shift(enum port port)
> +{
> + switch (port) {
> + case PORT_A:
> + case PORT_E:
> + default:
> + return -1;
> + case PORT_B:
> + return 17;
> + case PORT_C:
> + return 19;
> + case PORT_D:
> + return 21;
> + }
> +}
> +
> +static inline enum port get_port_from_pin(enum hpd_p
[PATCH 1/2] drm/i915: Add #defines for short/long pulse on gmch platforms
These look like they're already integrated into -nightly? But for the
record...
Reviewed-by: Todd Previte
-T
> Dave Airlie <mailto:airlied at gmail.com>
> Tuesday, June 17, 2014 6:29 PM
> From: Daniel Vetter
>
> For no reason at all the public docs lack them, and Dave needs them
> for his hpd interrupt rework.
>
> Cc: Dave Airlie
> Signed-off-by: Daniel Vetter
> Signed-off-by: Dave Airlie
> ---
> drivers/gpu/drm/i915/i915_reg.h | 6 ++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h
> b/drivers/gpu/drm/i915/i915_reg.h
> index 5122254..5d8ba0c 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -2526,8 +2526,14 @@ enum punit_power_well {
> #define PORTC_HOTPLUG_LIVE_STATUS_VLV (1 << 28)
> #define PORTB_HOTPLUG_LIVE_STATUS_VLV (1 << 29)
> #define PORTD_HOTPLUG_INT_STATUS (3 << 21)
> +#define PORTD_HOTPLUG_INT_LONG_PULSE (2 << 21)
> +#define PORTD_HOTPLUG_INT_SHORT_PULSE (1 << 21)
> #define PORTC_HOTPLUG_INT_STATUS (3 << 19)
> +#define PORTC_HOTPLUG_INT_LONG_PULSE (2 << 19)
> +#define PORTC_HOTPLUG_INT_SHORT_PULSE (1 << 19)
> #define PORTB_HOTPLUG_INT_STATUS (3 << 17)
> +#define PORTB_HOTPLUG_INT_LONG_PULSE (2 << 17)
> +#define PORTB_HOTPLUG_INT_SHORT_PLUSE (1 << 17)
> /* CRT/TV common between gen3+ */
> #define CRT_HOTPLUG_INT_STATUS (1 << 11)
> #define TV_HOTPLUG_INT_STATUS (1 << 10)
> Dave Airlie <mailto:airlied at gmail.com>
> Tuesday, June 17, 2014 6:29 PM
> Can we get these merged or even looked at?, they are blocking the
> whole MST progress,
> and I don't have any insight to secret Intel review process. :-)
>
> Dave.
>
> ___
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Sent with Postbox <http://www.getpostbox.com>
-- next part --
An HTML attachment was scrubbed...
URL:
<http://lists.freedesktop.org/archives/dri-devel/attachments/20140624/a913c093/attachment.html>
-- next part --
A non-text attachment was scrubbed...
Name: postbox-contact.jpg
Type: image/jpeg
Size: 1291 bytes
Desc: not available
URL:
<http://lists.freedesktop.org/archives/dri-devel/attachments/20140624/a913c093/attachment.jpg>
[Intel-gfx] [PATCH 09/11] drm/i915: check connector->encoder before using it.
Looks good.
Reviewed-by: Todd Previte
> Dave Airlie <mailto:airlied at gmail.com>
> Tuesday, May 20, 2014 7:55 PM
> From: Dave Airlie
>
> DP MST will need connectors that aren't connected to specific
> encoders, add some checks in advance to avoid oopses.
>
> Signed-off-by: Dave Airlie
> ---
> drivers/gpu/drm/i915/i915_debugfs.c | 16 +---
> drivers/gpu/drm/i915/i915_irq.c | 4
> drivers/gpu/drm/i915/intel_display.c | 25 ++---
> 3 files changed, 27 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> b/drivers/gpu/drm/i915/i915_debugfs.c
> index 1e83ae4..88e944f 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -2279,13 +2279,15 @@ static void intel_connector_info(struct
> seq_file *m,
> seq_printf(m, "\tCEA rev: %d\n",
> connector->display_info.cea_rev);
> }
> - if (intel_encoder->type == INTEL_OUTPUT_DISPLAYPORT ||
> - intel_encoder->type == INTEL_OUTPUT_EDP)
> - intel_dp_info(m, intel_connector);
> - else if (intel_encoder->type == INTEL_OUTPUT_HDMI)
> - intel_hdmi_info(m, intel_connector);
> - else if (intel_encoder->type == INTEL_OUTPUT_LVDS)
> - intel_lvds_info(m, intel_connector);
> + if (intel_encoder) {
> + if (intel_encoder->type == INTEL_OUTPUT_DISPLAYPORT ||
> + intel_encoder->type == INTEL_OUTPUT_EDP)
> + intel_dp_info(m, intel_connector);
> + else if (intel_encoder->type == INTEL_OUTPUT_HDMI)
> + intel_hdmi_info(m, intel_connector);
> + else if (intel_encoder->type == INTEL_OUTPUT_LVDS)
> + intel_lvds_info(m, intel_connector);
> + }
>
> seq_printf(m, "\tmodes:\n");
> list_for_each_entry(mode, >modes, head)
> diff --git a/drivers/gpu/drm/i915/i915_irq.c
> b/drivers/gpu/drm/i915/i915_irq.c
> index afa5519..5852dee 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -1016,6 +1016,8 @@ static void i915_hotplug_work_func(struct
> work_struct *work)
> dev_priv->hpd_event_bits = 0;
> list_for_each_entry(connector, _config->connector_list, head) {
> intel_connector = to_intel_connector(connector);
> + if (!intel_connector->encoder)
> + continue;
> intel_encoder = intel_connector->encoder;
> if (intel_encoder->hpd_pin > HPD_NONE &&
> dev_priv->hpd_stats[intel_encoder->hpd_pin].hpd_mark ==
> HPD_MARK_DISABLED &&
> @@ -1046,6 +1048,8 @@ static void i915_hotplug_work_func(struct
> work_struct *work)
>
> list_for_each_entry(connector, _config->connector_list, head) {
> intel_connector = to_intel_connector(connector);
> + if (!intel_connector->encoder)
> + continue;
> intel_encoder = intel_connector->encoder;
> if (hpd_event_bits & (1 << intel_encoder->hpd_pin)) {
> if (intel_encoder->hot_plug)
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index b39d036..75b2aaf 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4600,20 +4600,23 @@ static void intel_connector_check_state(struct
> intel_connector *connector)
> "wrong connector dpms state\n");
> WARN(connector->base.encoder != >base,
> "active connector not linked to encoder\n");
> - WARN(!encoder->connectors_active,
> - "encoder->connectors_active not set\n");
>
> - encoder_enabled = encoder->get_hw_state(encoder, );
> - WARN(!encoder_enabled, "encoder not enabled\n");
> - if (WARN_ON(!encoder->base.crtc))
> - return;
> + if (encoder) {
> + WARN(!encoder->connectors_active,
> + "encoder->connectors_active not set\n");
> +
> + encoder_enabled = encoder->get_hw_state(encoder, );
> + WARN(!encoder_enabled, "encoder not enabled\n");
> + if (WARN_ON(!encoder->base.crtc))
> + return;
>
> - crtc = encoder->base.crtc;
> + crtc = encoder->base.crtc;
>
> - WARN(!crtc->enabled, "crtc not enabled\n");
> - WARN(!to_intel_crtc(crtc)->active, "crtc not active\n");
> - WARN(pipe != to_intel_crtc(crtc)->pipe,
> - "encoder active on the wrong pipe\n");
> + WARN(!crtc->enabled, "crtc not enabled\n");
> + WARN(!to_intel_crtc(crtc)->active, "crtc not active\n");
> + WARN(pipe != to_intel_crtc(crtc)->pipe,
> + "encoder active on the wrong pipe\n");
> + }
> }
> }
>
> Dave Airlie <mailto:airlied at gmail.com>
> Tuesday, May 20, 2014 7:54 PM
> Hey,
>
> So this set is pretty close to what I think we should be merging
> initially,
>
> Since the last set, it makes f
[PATCH 08/11] i915: split some DP modesetting code into a separate function
Looks good to me.
Reviewed-by: Todd Previte
> Dave Airlie <mailto:airlied at gmail.com>
> Tuesday, May 20, 2014 7:55 PM
> From: Dave Airlie
>
> this is just prep work for mst support.
>
> Signed-off-by: Dave Airlie
> ---
> drivers/gpu/drm/i915/intel_ddi.c | 20 +---
> drivers/gpu/drm/i915/intel_drv.h | 1 +
> 2 files changed, 14 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c
> b/drivers/gpu/drm/i915/intel_ddi.c
> index 0ad4e96..a5b8b76 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -364,6 +364,18 @@ void hsw_fdi_link_train(struct drm_crtc *crtc)
> DRM_ERROR("FDI link training failed!\n");
> }
>
> +void intel_ddi_mode_set_dp(struct intel_encoder *encoder)
> +{
> + struct intel_dp *intel_dp = enc_to_intel_dp(>base);
> + struct intel_digital_port *intel_dig_port =
> + enc_to_dig_port(>base);
> +
> + intel_dp->DP = intel_dig_port->saved_port_bits |
> + DDI_BUF_CTL_ENABLE | DDI_BUF_EMP_400MV_0DB_HSW;
> + intel_dp->DP |= DDI_PORT_WIDTH(intel_dp->lane_count);
> +
> +}
> +
> static void intel_ddi_mode_set(struct intel_encoder *encoder)
> {
> struct intel_crtc *crtc = to_intel_crtc(encoder->base.crtc);
> @@ -378,13 +390,7 @@ static void intel_ddi_mode_set(struct
> intel_encoder *encoder)
> crtc->eld_vld = false;
> if (type == INTEL_OUTPUT_DISPLAYPORT || type == INTEL_OUTPUT_EDP) {
> struct intel_dp *intel_dp = enc_to_intel_dp(>base);
> - struct intel_digital_port *intel_dig_port =
> - enc_to_dig_port(>base);
> -
> - intel_dp->DP = intel_dig_port->saved_port_bits |
> - DDI_BUF_CTL_ENABLE | DDI_BUF_EMP_400MV_0DB_HSW;
> - intel_dp->DP |= DDI_PORT_WIDTH(intel_dp->lane_count);
> -
> + intel_ddi_mode_set_dp(encoder);
> if (intel_dp->has_audio) {
> DRM_DEBUG_DRIVER("DP audio on pipe %c on DDI\n",
> pipe_name(crtc->pipe));
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h
> index b885df1..8e41cdc 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -683,6 +683,7 @@ void intel_ddi_fdi_disable(struct drm_crtc *crtc);
> void intel_ddi_get_config(struct intel_encoder *encoder,
> struct intel_crtc_config *pipe_config);
>
> +void intel_ddi_mode_set_dp(struct intel_encoder *encoder);
>
> /* intel_display.c */
> const char *intel_output_name(int output);
> Dave Airlie <mailto:airlied at gmail.com>
> Tuesday, May 20, 2014 7:54 PM
> Hey,
>
> So this set is pretty close to what I think we should be merging
> initially,
>
> Since the last set, it makes fbcon and suspend/resume work a lot better,
>
> I've also fixed a couple of bugs in -intel that make things work a lot
> better.
>
> I've bashed on this a bit using kms-flip from intel-gpu-tools, hacked
> to add 3 monitor support.
>
> It still generates a fair few i915 state checker backtraces, and some
> of them are fairly hard to work out, it might be we should just tone
> down the state checker for encoders/connectors with no actual hw backing
> them.
>
> Dave.
>
> ___
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Sent using Postbox:
http://www.getpostbox.com
-- next part --
An HTML attachment was scrubbed...
URL:
<http://lists.freedesktop.org/archives/dri-devel/attachments/20140617/93973305/attachment.html>
-- next part --
A non-text attachment was scrubbed...
Name: postbox-contact.jpg
Type: image/jpeg
Size: 1291 bytes
Desc: not available
URL:
<http://lists.freedesktop.org/archives/dri-devel/attachments/20140617/93973305/attachment.jpg>
[Intel-gfx] [PATCH 07/11] drm/helper: add Displayport multi-stream helper (v0.5)
This patch is a monster, but that's to be expected with MST, I suppose. :) It has some formatting issues (lines over 80 characters in length) but that can be cleaned up later (as far as I'm concerned). Otherwise I don't see anything glaring here, so... Reviewed-by: Todd Previte > Dave Airlie <mailto:airlied at gmail.com> > Tuesday, May 20, 2014 7:55 PM > From: Dave Airlie > > This is the initial import of the helper for displayport multistream. > > It consists of a topology manager, init/destroy/set mst state > > It supports DP 1.2 MST sideband msg protocol handler - via hpd irqs > > connector detect and edid retrieval interface. > > It supports i2c device over DP 1.2 sideband msg protocol (EDID reads only) > > bandwidth manager API via vcpi allocation and payload updating, > along with a helper to check the ACT status. > > Objects: > MST topology manager - one per toplevel MST capable GPU port - not > sure if this should be higher level again > MST branch unit - one instance per plugged branching unit - one at top > of hierarchy - others hanging from ports > MST port - one port per port reported by branching units, can have MST > units hanging from them as well. > > Changes since initial posting: > a) add a mutex responsbile for the queues, it locks the sideband and > msg slots, and msgs to transmit state > b) add worker to handle connection state change events, for MST device > chaining and hotplug > c) add a payload spinlock > d) add path sideband msg support > e) fixup enum path resources transmit > f) reduce max dpcd msg to 16, as per DP1.2 spec. > g) separate tx queue kicking from irq processing and move irq acking > back to drivers. > > Changes since v0.2: > a) reorganise code, > b) drop ACT forcing code > c) add connector naming interface using path property > d) add topology dumper helper > e) proper reference counting and lookup for ports and mstbs. > f) move tx kicking into a workq > g) add aux locking - this should be redone > h) split teardown into two parts > i) start working on documentation on interface. > > Changes since v0.3: > a) vc payload locking and tracking fixes > b) add hotplug callback into driver - replaces crazy return 1 scheme > c) txmsg + mst branch device refcount fixes > d) don't bail on mst shutdown if device is gone > e) change irq handler to take all 4 bytes of SINK_COUNT + ESI vectors > f) make DP payload updates timeout longer - observed on docking > station redock > g) add more info to debugfs dumper > > Changes since v0.4: > a) suspend/resume support > b) more debugging in debugfs > > TODO: > misc features > > Signed-off-by: Dave Airlie > --- > Documentation/DocBook/drm.tmpl | 6 + > drivers/gpu/drm/Makefile | 2 +- > drivers/gpu/drm/drm_dp_mst_topology.c | 2739 > + > include/drm/drm_dp_mst_helper.h | 507 ++ > 4 files changed, 3253 insertions(+), 1 deletion(-) > create mode 100644 drivers/gpu/drm/drm_dp_mst_topology.c > create mode 100644 include/drm/drm_dp_mst_helper.h > > diff --git a/Documentation/DocBook/drm.tmpl > b/Documentation/DocBook/drm.tmpl > index 83dd0b0..1883976 100644 > --- a/Documentation/DocBook/drm.tmpl > +++ b/Documentation/DocBook/drm.tmpl > @@ -2296,6 +2296,12 @@ void intel_crt_init(struct drm_device *dev) > !Edrivers/gpu/drm/drm_dp_helper.c > > > + Display Port MST Helper Functions Reference > +!Pdrivers/gpu/drm/drm_dp_mst_topology.c dp mst helper > +!Iinclude/drm/drm_dp_mst_helper.h > +!Edrivers/gpu/drm/drm_dp_mst_topology.c > + > + > EDID Helper Functions Reference > !Edrivers/gpu/drm/drm_edid.c > > diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile > index 48e38ba..712b73e 100644 > --- a/drivers/gpu/drm/Makefile > +++ b/drivers/gpu/drm/Makefile > @@ -23,7 +23,7 @@ drm-$(CONFIG_DRM_PANEL) += drm_panel.o > > drm-usb-y := drm_usb.o > > -drm_kms_helper-y := drm_crtc_helper.o drm_dp_helper.o drm_probe_helper.o > +drm_kms_helper-y := drm_crtc_helper.o drm_dp_helper.o > drm_probe_helper.o drm_dp_mst_topology.o > drm_kms_helper-$(CONFIG_DRM_LOAD_EDID_FIRMWARE) += drm_edid_load.o > drm_kms_helper-$(CONFIG_DRM_KMS_FB_HELPER) += drm_fb_helper.o > drm_kms_helper-$(CONFIG_DRM_KMS_CMA_HELPER) += drm_fb_cma_helper.o > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c > b/drivers/gpu/drm/drm_dp_mst_topology.c > new file mode 100644 > index 000..ebd9292 > --- /dev/null > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c > @@ -0,0 +1,2739 @@ > +/* > + * Copyright ? 2014 Red Hat > + * > + * Permission to use, copy, modify, distribute, and sell this > software and its > + * documentation for any purpose is hereby granted w
[Intel-gfx] [PATCH 06/11] drm: add a path blob property
This one looks fine to me.
Reviewed-by: Todd Previte
> Dave Airlie <mailto:airlied at gmail.com>
> Tuesday, May 20, 2014 7:54 PM
> From: Dave Airlie
>
> This property will be used by the MST code to provide userspace
> with a path to parse so it can recognise connectors around hotplugs.
>
> Signed-off-by: Dave Airlie
> ---
> drivers/gpu/drm/drm_crtc.c | 26 ++
> include/drm/drm_crtc.h | 5 +
> 2 files changed, 31 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index 8bf87a6..06b9255 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -1165,6 +1165,7 @@ static int
> drm_mode_create_standard_connector_properties(struct drm_device *dev)
> {
> struct drm_property *edid;
> struct drm_property *dpms;
> + struct drm_property *dev_path;
>
> /*
> * Standard properties (apply to all connectors)
> @@ -1179,6 +1180,12 @@ static int
> drm_mode_create_standard_connector_properties(struct drm_device *dev)
> ARRAY_SIZE(drm_dpms_enum_list));
> dev->mode_config.dpms_property = dpms;
>
> + dev_path = drm_property_create(dev,
> + DRM_MODE_PROP_BLOB |
> + DRM_MODE_PROP_IMMUTABLE,
> + "PATH", 0);
> + dev->mode_config.path_property = dev_path;
> +
> return 0;
> }
>
> @@ -3637,6 +3644,25 @@ done:
> return ret;
> }
>
> +int drm_mode_connector_set_path_property(struct drm_connector *connector,
> + char *path)
> +{
> + struct drm_device *dev = connector->dev;
> + int ret, size;
> + size = strlen(path) + 1;
> +
> + connector->path_blob_ptr = drm_property_create_blob(connector->dev,
> + size, path);
> + if (!connector->path_blob_ptr)
> + return -EINVAL;
> +
> + ret = drm_object_property_set_value(>base,
> + dev->mode_config.path_property,
> + connector->path_blob_ptr->base.id);
> + return ret;
> +}
> +EXPORT_SYMBOL(drm_mode_connector_set_path_property);
> +
> /**
> * drm_mode_connector_update_edid_property - update the edid property
> of a connector
> * @connector: drm connector
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index 55bc523..e33959b 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -500,6 +500,8 @@ struct drm_connector {
> struct drm_property_blob *edid_blob_ptr;
> struct drm_object_properties properties;
>
> + struct drm_property_blob *path_blob_ptr;
> +
> uint8_t polled; /* DRM_CONNECTOR_POLL_* */
>
> /* requested DPMS state */
> @@ -774,6 +776,7 @@ struct drm_mode_config {
> struct list_head property_blob_list;
> struct drm_property *edid_property;
> struct drm_property *dpms_property;
> + struct drm_property *path_property;
> struct drm_property *plane_type_property;
>
> /* DVI-I properties */
> @@ -926,6 +929,8 @@ extern void drm_mode_config_init(struct drm_device
> *dev);
> extern void drm_mode_config_reset(struct drm_device *dev);
> extern void drm_mode_config_cleanup(struct drm_device *dev);
>
> +extern int drm_mode_connector_set_path_property(struct drm_connector
> *connector,
> + char *path);
> extern int drm_mode_connector_update_edid_property(struct
> drm_connector *connector,
> struct edid *edid);
> extern int drm_object_property_set_value(struct drm_mode_object *obj,
> Dave Airlie <mailto:airlied at gmail.com>
> Tuesday, May 20, 2014 7:54 PM
> Hey,
>
> So this set is pretty close to what I think we should be merging
> initially,
>
> Since the last set, it makes fbcon and suspend/resume work a lot better,
>
> I've also fixed a couple of bugs in -intel that make things work a lot
> better.
>
> I've bashed on this a bit using kms-flip from intel-gpu-tools, hacked
> to add 3 monitor support.
>
> It still generates a fair few i915 state checker backtraces, and some
> of them are fairly hard to work out, it might be we should just tone
> down the state checker for encoders/connectors with no actual hw backing
> them.
>
> Dave.
>
> ___
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Sent using Postbox:
http://www.getpostbox.com
-- next part --
An HTML attachment was scrubbed...
URL:
<http://lists.freedesktop.org/archives/dri-devel/attachments/20140617/92b836b3/attachment-0001.html>
-- next part --
A non-text attachment was scrubbed...
Name: postbox-contact.jpg
Type: image/jpeg
Size: 1291 bytes
Desc: not available
URL:
<http://lists.freedesktop.org/archives/dri-devel/attachments/20140617/92b836b3/attachment-0001.jpg>
[Intel-gfx] [PATCH 05/11] drm/fb_helper: allow adding/removing connectors later
Minor formatting issues - there's a number of lines that exceed 80
characters in length. One other comment inline below.
Reviewed-by: Todd Previte
> Dave Airlie <mailto:airlied at gmail.com>
> Tuesday, May 20, 2014 7:54 PM
> From: Dave Airlie
>
> This is required to get fbcon probing to work on new connectors,
> callers should acquire the mode config lock before calling these.
>
> Signed-off-by: Dave Airlie
> ---
> drivers/gpu/drm/drm_fb_helper.c | 53
> +
> include/drm/drm_fb_helper.h | 4
> 2 files changed, 57 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_fb_helper.c
> b/drivers/gpu/drm/drm_fb_helper.c
> index 04d3fd3..a184204 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -105,6 +105,58 @@ fail:
> }
> EXPORT_SYMBOL(drm_fb_helper_single_add_all_connectors);
>
> +int drm_fb_helper_add_one_connector(struct drm_fb_helper *fb_helper,
> struct drm_connector *connector)
> +{
> + struct drm_fb_helper_connector **temp;
> + struct drm_fb_helper_connector *fb_helper_connector;
> +
> + WARN_ON(!mutex_is_locked(_helper->dev->mode_config.mutex));
> + if (fb_helper->connector_count + 1 >
> fb_helper->connector_info_alloc_count) {
> + temp = krealloc(fb_helper->connector_info, sizeof(struct
> drm_fb_helper_connector) * (fb_helper->connector_count + 1), GFP_KERNEL);
> + if (!temp)
> + return -ENOMEM;
> +
> + fb_helper->connector_info_alloc_count = fb_helper->connector_count + 1;
> + fb_helper->connector_info = temp;
> + }
> +
> +
> + fb_helper_connector = kzalloc(sizeof(struct
> drm_fb_helper_connector), GFP_KERNEL);
> + if (!fb_helper_connector)
> + return -ENOMEM;
> +
> + fb_helper_connector->connector = connector;
> + fb_helper->connector_info[fb_helper->connector_count++] =
> fb_helper_connector;
> + return 0;
> +}
> +EXPORT_SYMBOL(drm_fb_helper_add_one_connector);
> +
> +int drm_fb_helper_remove_one_connector(struct drm_fb_helper *fb_helper,
> + struct drm_connector *connector)
> +{
> + struct drm_fb_helper_connector *fb_helper_connector;
> + int i, j;
> +
> + WARN_ON(!mutex_is_locked(_helper->dev->mode_config.mutex));
> +
> + for (i = 0; i < fb_helper->connector_count; i++) {
> + if (fb_helper->connector_info[i]->connector == connector)
> + break;
> + }
> +
> + if (i == fb_helper->connector_count)
> + return -EINVAL;
> + fb_helper_connector = fb_helper->connector_info[i];
> +
> + for (j = i + 1; j < fb_helper->connector_count; j++) {
> + fb_helper->connector_info[j - 1] = fb_helper->connector_info[j];
> + }
Why switch to using a different index variable here? Seems like you
could just increment i and keep going...
> + fb_helper->connector_count--;
> + kfree(fb_helper_connector);
> + return 0;
> +}
> +EXPORT_SYMBOL(drm_fb_helper_remove_one_connector);
> +
> static int drm_fb_helper_parse_command_line(struct drm_fb_helper
> *fb_helper)
> {
> struct drm_fb_helper_connector *fb_helper_conn;
> @@ -534,6 +586,7 @@ int drm_fb_helper_init(struct drm_device *dev,
> kfree(fb_helper->crtc_info);
> return -ENOMEM;
> }
> + fb_helper->connector_info_alloc_count = dev->mode_config.num_connector;
> fb_helper->connector_count = 0;
>
> for (i = 0; i < crtc_count; i++) {
> diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h
> index 6e622f7..4abb415 100644
> --- a/include/drm/drm_fb_helper.h
> +++ b/include/drm/drm_fb_helper.h
> @@ -86,6 +86,7 @@ struct drm_fb_helper {
> int crtc_count;
> struct drm_fb_helper_crtc *crtc_info;
> int connector_count;
> + int connector_info_alloc_count;
> struct drm_fb_helper_connector **connector_info;
> struct drm_fb_helper_funcs *funcs;
> struct fb_info *fbdev;
> @@ -128,4 +129,7 @@ struct drm_display_mode *
> drm_pick_cmdline_mode(struct drm_fb_helper_connector *fb_helper_conn,
> int width, int height);
>
> +int drm_fb_helper_add_one_connector(struct drm_fb_helper *fb_helper,
> struct drm_connector *connector);
> +int drm_fb_helper_remove_one_connector(struct drm_fb_helper *fb_helper,
> + struct drm_connector *connector);
> #endif
> Dave Airlie <mailto:airlied at gmail.com>
> Tuesday, May 20, 2014 7:54 PM
> Hey,
>
> So this set is pretty close to what I think we should be merging
> initially,
>
> Since the last set, it makes fbcon and suspend/resume work a lot better,
>
> I've also fixed a couple of bugs in -intel that make things work a lot
> better.
>
> I've bashed on this a bit using kms-flip from intel-gpu-tools, hacked
> to add 3 monitor support.
>
> It still generates a fa
[PATCH 04/11] drm/crtc: add interface to reinitialise the legacy mode group
> Dave Airlie <mailto:airlied at gmail.com>
> Tuesday, May 20, 2014 7:54 PM
> From: Dave Airlie
>
> This can be called to update things after dynamic connectors/encoders
> are created/deleted.
>
> Signed-off-by: Dave Airlie
> ---
> drivers/gpu/drm/drm_crtc.c | 9 +
> include/drm/drm_crtc.h | 1 +
> 2 files changed, 10 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index f1753e6..8bf87a6 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -1421,6 +1421,15 @@ int drm_mode_group_init_legacy_group(struct
> drm_device *dev,
> }
> EXPORT_SYMBOL(drm_mode_group_init_legacy_group);
>
> +void drm_reinit_primary_mode_group(struct drm_device *dev)
> +{
> + drm_modeset_lock_all(dev);
> + drm_mode_group_destroy(>primary->mode_group);
> + drm_mode_group_init_legacy_group(dev, >primary->mode_group);
> + drm_modeset_unlock_all(dev);
> +}
> +EXPORT_SYMBOL(drm_reinit_primary_mode_group);
> +
> /**
> * drm_crtc_convert_to_umode - convert a drm_display_mode into a modeinfo
> * @out: drm_mode_modeinfo struct to return to the user
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index c6b9e8a..55bc523 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -916,6 +916,7 @@ extern const char *drm_get_tv_select_name(int val);
> extern void drm_fb_release(struct drm_file *file_priv);
> extern int drm_mode_group_init_legacy_group(struct drm_device *dev,
> struct drm_mode_group *group);
> extern void drm_mode_group_destroy(struct drm_mode_group *group);
> +extern void drm_reinit_primary_mode_group(struct drm_device *dev);
> extern bool drm_probe_ddc(struct i2c_adapter *adapter);
> extern struct edid *drm_get_edid(struct drm_connector *connector,
> struct i2c_adapter *adapter);
Reviewed-by: Todd Previte
> Dave Airlie <mailto:airlied at gmail.com>
> Tuesday, May 20, 2014 7:54 PM
> Hey,
>
> So this set is pretty close to what I think we should be merging
> initially,
>
> Since the last set, it makes fbcon and suspend/resume work a lot better,
>
> I've also fixed a couple of bugs in -intel that make things work a lot
> better.
>
> I've bashed on this a bit using kms-flip from intel-gpu-tools, hacked
> to add 3 monitor support.
>
> It still generates a fair few i915 state checker backtraces, and some
> of them are fairly hard to work out, it might be we should just tone
> down the state checker for encoders/connectors with no actual hw backing
> them.
>
> Dave.
>
> ___
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
-- next part --
An HTML attachment was scrubbed...
URL:
<http://lists.freedesktop.org/archives/dri-devel/attachments/20140522/4c9c0c23/attachment-0001.html>
-- next part --
A non-text attachment was scrubbed...
Name: postbox-contact.jpg
Type: image/jpeg
Size: 1291 bytes
Desc: not available
URL:
<http://lists.freedesktop.org/archives/dri-devel/attachments/20140522/4c9c0c23/attachment-0001.jpg>
[Intel-gfx] [PATCH 03/11] drm/i915: add some registers need for displayport MST support.
> Dave Airlie <mailto:airlied at gmail.com>
> Tuesday, May 20, 2014 7:54 PM
> From: Dave Airlie
>
> These are just from the Haswell spec.
>
> Signed-off-by: Dave Airlie
> ---
> drivers/gpu/drm/i915/i915_reg.h | 11 +--
> 1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h
> b/drivers/gpu/drm/i915/i915_reg.h
> index 8f84555..557b37a 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -5386,6 +5386,7 @@ enum punit_power_well {
> #define TRANS_DDI_EDP_INPUT_A_ONOFF (4<<12)
> #define TRANS_DDI_EDP_INPUT_B_ONOFF (5<<12)
> #define TRANS_DDI_EDP_INPUT_C_ONOFF (6<<12)
> +#define TRANS_DDI_DP_VC_PAYLOAD_ALLOC (1<<8)
> #define TRANS_DDI_BFI_ENABLE (1<<4)
>
> /* DisplayPort Transport Control */
> @@ -5395,6 +5396,7 @@ enum punit_power_well {
> #define DP_TP_CTL_ENABLE (1<<31)
> #define DP_TP_CTL_MODE_SST (0<<27)
> #define DP_TP_CTL_MODE_MST (1<<27)
> +#define DP_TP_CTL_FORCE_ACT (1<<25)
> #define DP_TP_CTL_ENHANCED_FRAME_ENABLE (1<<18)
> #define DP_TP_CTL_FDI_AUTOTRAIN (1<<15)
> #define DP_TP_CTL_LINK_TRAIN_MASK (7<<8)
> @@ -5409,8 +5411,13 @@ enum punit_power_well {
> #define DP_TP_STATUS_A 0x64044
> #define DP_TP_STATUS_B 0x64144
> #define DP_TP_STATUS(port) _PORT(port, DP_TP_STATUS_A, DP_TP_STATUS_B)
> -#define DP_TP_STATUS_IDLE_DONE (1<<25)
> -#define DP_TP_STATUS_AUTOTRAIN_DONE (1<<12)
> +#define DP_TP_STATUS_IDLE_DONE (1<<25)
> +#define DP_TP_STATUS_ACT_SENT (1<<24)
> +#define DP_TP_STATUS_MODE_STATUS_MST (1<<23)
> +#define DP_TP_STATUS_AUTOTRAIN_DONE (1<<12)
> +#define DP_TP_STATUS_PAYLOAD_MAPPING_VC2 (3 << 8)
> +#define DP_TP_STATUS_PAYLOAD_MAPPING_VC1 (3 << 4)
> +#define DP_TP_STATUS_PAYLOAD_MAPPING_VC0 (3 << 0)
>
> /* DDI Buffer Control */
> #define DDI_BUF_CTL_A 0x64000
Definitions look correct. If I noticed any discrepancies during testing,
I'll flag it.
Reviewed-by: Todd Previte
> Dave Airlie <mailto:airlied at gmail.com>
> Tuesday, May 20, 2014 7:54 PM
> Hey,
>
> So this set is pretty close to what I think we should be merging
> initially,
>
> Since the last set, it makes fbcon and suspend/resume work a lot better,
>
> I've also fixed a couple of bugs in -intel that make things work a lot
> better.
>
> I've bashed on this a bit using kms-flip from intel-gpu-tools, hacked
> to add 3 monitor support.
>
> It still generates a fair few i915 state checker backtraces, and some
> of them are fairly hard to work out, it might be we should just tone
> down the state checker for encoders/connectors with no actual hw backing
> them.
>
> Dave.
>
> ___
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
-- next part --
An HTML attachment was scrubbed...
URL:
<http://lists.freedesktop.org/archives/dri-devel/attachments/20140522/096b429d/attachment.html>
-- next part --
A non-text attachment was scrubbed...
Name: postbox-contact.jpg
Type: image/jpeg
Size: 1291 bytes
Desc: not available
URL:
<http://lists.freedesktop.org/archives/dri-devel/attachments/20140522/096b429d/attachment.jpg>
[Intel-gfx] [PATCH 02/11] drm: add DP MST encoder type
> Dave Airlie <mailto:airlied at gmail.com>
> Tuesday, May 20, 2014 7:54 PM
> From: Dave Airlie
>
> This adds an encoder type for DP MST encoders.
>
> Signed-off-by: Dave Airlie
> ---
> drivers/gpu/drm/drm_crtc.c | 1 +
> include/uapi/drm/drm_mode.h | 1 +
> 2 files changed, 2 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index a3fe324..f1753e6 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -227,6 +227,7 @@ static const struct drm_prop_enum_list
> drm_encoder_enum_list[] =
> { DRM_MODE_ENCODER_TVDAC, "TV" },
> { DRM_MODE_ENCODER_VIRTUAL, "Virtual" },
> { DRM_MODE_ENCODER_DSI, "DSI" },
> + { DRM_MODE_ENCODER_DPMST, "DP MST" },
> };
>
> static const struct drm_prop_enum_list drm_subpixel_enum_list[] =
> diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> index f104c26..719add4 100644
> --- a/include/uapi/drm/drm_mode.h
> +++ b/include/uapi/drm/drm_mode.h
> @@ -181,6 +181,7 @@ struct drm_mode_get_plane_res {
> #define DRM_MODE_ENCODER_TVDAC 4
> #define DRM_MODE_ENCODER_VIRTUAL 5
> #define DRM_MODE_ENCODER_DSI 6
> +#define DRM_MODE_ENCODER_DPMST 7
>
> struct drm_mode_get_encoder {
> __u32 encoder_id;
Reviewed-by: Todd Previte
> Dave Airlie <mailto:airlied at gmail.com>
> Tuesday, May 20, 2014 7:54 PM
> Hey,
>
> So this set is pretty close to what I think we should be merging
> initially,
>
> Since the last set, it makes fbcon and suspend/resume work a lot better,
>
> I've also fixed a couple of bugs in -intel that make things work a lot
> better.
>
> I've bashed on this a bit using kms-flip from intel-gpu-tools, hacked
> to add 3 monitor support.
>
> It still generates a fair few i915 state checker backtraces, and some
> of them are fairly hard to work out, it might be we should just tone
> down the state checker for encoders/connectors with no actual hw backing
> them.
>
> Dave.
>
> ___
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
-- next part --
An HTML attachment was scrubbed...
URL:
<http://lists.freedesktop.org/archives/dri-devel/attachments/20140522/1174bfbb/attachment.html>
-- next part --
A non-text attachment was scrubbed...
Name: postbox-contact.jpg
Type: image/jpeg
Size: 1291 bytes
Desc: not available
URL:
<http://lists.freedesktop.org/archives/dri-devel/attachments/20140522/1174bfbb/attachment.jpg>
[PATCH 01/11] drm/dp_helper: add defines for DP 1.2 and MST support.
> Dave Airlie <mailto:airlied at gmail.com> > Tuesday, May 20, 2014 7:54 PM > From: Dave Airlie > > This just adds the defines from the DP 1.2 spec, which we > will use later. > > Signed-off-by: Dave Airlie > --- > include/drm/drm_dp_helper.h | 78 > + > 1 file changed, 78 insertions(+) > > diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h > index cfcacec..879836d 100644 > --- a/include/drm/drm_dp_helper.h > +++ b/include/drm/drm_dp_helper.h > @@ -37,6 +37,7 @@ > * eDP: Embedded DisplayPort version 1 > * DPI: DisplayPort Interoperability Guideline v1.1a > * 1.2: DisplayPort 1.2 > + * MST: Multistream Transport - part of DP 1.2a > * > * 1.2 formally includes both eDP and DPI definitions. > */ > @@ -103,9 +104,14 @@ > #define DP_TRAINING_AUX_RD_INTERVAL 0x00e /* XXX 1.2? */ > > /* Multiple stream transport */ > +#define DP_FAUX_CAP 0x020 /* 1.2 */ > +# define DP_FAUX_CAP_1 (1 << 0) > + > #define DP_MSTM_CAP 0x021 /* 1.2 */ > # define DP_MST_CAP (1 << 0) > > +#define DP_GUID 0x030 /* 1.2 */ > + > #define DP_PSR_SUPPORT 0x070 /* XXX 1.2? */ > # define DP_PSR_IS_SUPPORTED 1 > #define DP_PSR_CAPS 0x071 /* XXX 1.2? */ > @@ -221,6 +227,16 @@ > # define DP_PSR_CRC_VERIFICATION (1 << 2) > # define DP_PSR_FRAME_CAPTURE (1 << 3) > > +#define DP_ADAPTER_CTRL 0x1a0 > +# define DP_ADAPTER_CTRL_FORCE_LOAD_SENSE (1 << 0) > + > +#define DP_BRANCH_DEVICE_CTRL 0x1a1 > +# define DP_BRANCH_DEVICE_IRQ_HPD (1 << 0) > + > +#define DP_PAYLOAD_ALLOCATE_SET 0x1c0 > +#define DP_PAYLOAD_ALLOCATE_START_TIME_SLOT 0x1c1 > +#define DP_PAYLOAD_ALLOCATE_TIME_SLOT_COUNT 0x1c2 > + > #define DP_SINK_COUNT 0x200 > /* prior to 1.2 bit 7 was reserved mbz */ > # define DP_GET_SINK_COUNT(x) x) & 0x80) >> 1) | ((x) & 0x3f)) > @@ -230,6 +246,9 @@ > # define DP_REMOTE_CONTROL_COMMAND_PENDING (1 << 0) > # define DP_AUTOMATED_TEST_REQUEST (1 << 1) > # define DP_CP_IRQ (1 << 2) > +# define DP_MCCS_IRQ (1 << 3) > +# define DP_DOWN_REP_MSG_RDY (1 << 4) /* DP MST */ > +# define DP_UP_REQ_MSG_RDY (1 << 5) /* DP MST */ > # define DP_SINK_SPECIFIC_IRQ (1 << 6) > > #define DP_LANE0_1_STATUS 0x202 > @@ -294,6 +313,13 @@ > #define DP_TEST_SINK 0x270 > #define DP_TEST_SINK_START (1 << 0) > > +#define DP_PAYLOAD_TABLE_UPDATE_STATUS 0x2c0 /* 1.2 MST */ > +# define DP_PAYLOAD_TABLE_UPDATED (1 << 0) > +# define DP_PAYLOAD_ACT_HANDLED (1 << 1) > + > +#define DP_VC_PAYLOAD_ID_SLOT_1 0x2c1 /* 1.2 MST */ > +/* up to ID_SLOT_63 at 0x2ff */ > + > #define DP_SOURCE_OUI 0x300 > #define DP_SINK_OUI 0x400 > #define DP_BRANCH_OUI 0x500 > @@ -303,6 +329,21 @@ > # define DP_SET_POWER_D3 0x2 > # define DP_SET_POWER_MASK 0x3 > > +#define DP_SIDEBAND_MSG_DOWN_REQ_BASE 0x1000 /* 1.2 MST */ > +#define DP_SIDEBAND_MSG_UP_REP_BASE 0x1200 /* 1.2 MST */ > +#define DP_SIDEBAND_MSG_DOWN_REP_BASE 0x1400 /* 1.2 MST */ > +#define DP_SIDEBAND_MSG_UP_REQ_BASE 0x1600 /* 1.2 MST */ > + > +#define DP_SINK_COUNT_ESI 0x2002 /* 1.2 */ > +/* 0-5 sink count */ > +# define DP_SINK_COUNT_CP_READY (1 << 6) > + > +#define DP_DEVICE_SERVICE_IRQ_VECTOR_ESI0 0x2003 /* 1.2 */ > + > +#define DP_DEVICE_SERVICE_IRQ_VECTOR_ESI1 0x2004 /* 1.2 */ > + > +#define DP_LINK_SERVICE_IRQ_VECTOR_ESI0 0x2005 /* 1.2 */ > + > #define DP_PSR_ERROR_STATUS 0x2006 /* XXX 1.2? */ > # define DP_PSR_LINK_CRC_ERROR (1 << 0) > # define DP_PSR_RFB_STORAGE_ERROR (1 << 1) > @@ -319,6 +360,43 @@ > # define DP_PSR_SINK_INTERNAL_ERROR 7 > # define DP_PSR_SINK_STATE_MASK 0x07 > > +/* DP 1.2 Sideband message defines */ > +/* peer device type - DP 1.2a Table 2-92 */ > +#define DP_PEER_DEVICE_NONE 0x0 > +#define DP_PEER_DEVICE_SOURCE_OR_SST 0x1 > +#define DP_PEER_DEVICE_MST_BRANCHING 0x2 > +#define DP_PEER_DEVICE_SST_SINK 0x3 > +#define DP_PEER_DEVICE_DP_LEGACY_CONV 0x4 > + > +/* DP 1.2 MST sideband request names DP 1.2a Table 2-80 */ > +#define DP_LINK_ADDRESS 0x01 > +#define DP_CONNECTION_STATUS_NOTIFY 0x02 > +#define DP_ENUM_PATH_RESOURCES 0x10 > +#define DP_ALLOCATE_PAYLOAD 0x11 > +#define DP_QUERY_PAYLOAD 0x12 > +#define DP_RESOURCE_STATUS_NOTIFY 0x13 > +#define DP_CLEAR_PAYLOAD_ID_TABLE 0x14 > +#define DP_REMOTE_DPCD_READ 0x20 > +#define DP_REMOTE_DPCD_WRITE 0x21 > +#define DP_REMOTE_I2C_READ 0x22 > +#define DP_REMOTE_I2C_WRITE 0x23 > +#define DP_POWER_UP_PHY 0x24 > +#define DP_POWER_DOWN_PHY 0x25 > +#define DP_SINK_EVENT_NOTIFY 0x30 > +#define DP_QUERY_STREAM_ENC_STATUS 0x38 > + > +/* DP 1.2 MST sideband nak reasons - table 2.84 */ > +#define
[PATCH] drm/dp: Clarify automated test constant and add constant for FAUX test pattern
- DP_TEST_LINK_PATTERN is ambiguous, rename to DP_TEST_LINK_VIDEO_PATTERN to clarify - Added DP_TEST_LINK_FAUX_PATTERN to support automated testing of Fast AUX Signed-off-by: Todd Previte tprev...@gmail.com --- include/drm/drm_dp_helper.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h index ae8dbfb..34c8202 100644 --- a/include/drm/drm_dp_helper.h +++ b/include/drm/drm_dp_helper.h @@ -266,9 +266,10 @@ #define DP_TEST_REQUEST0x218 # define DP_TEST_LINK_TRAINING (1 0) -# define DP_TEST_LINK_PATTERN (1 1) +# define DP_TEST_LINK_VIDEO_PATTERN(1 1) # define DP_TEST_LINK_EDID_READ(1 2) # define DP_TEST_LINK_PHY_TEST_PATTERN (1 3) /* DPCD = 1.1 */ +# define DP_TEST_LINK_FAUX_PATTERN (1 4) /* DPCD = 1.2 */ #define DP_TEST_LINK_RATE 0x219 # define DP_LINK_RATE_162 (0x6) -- 1.8.1.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/2] drm/dp: add defines for downstream port types
Looks good. Reviewed-by: Todd Previte tprev...@gmail.com On Fri, Sep 27, 2013 at 4:48 AM, Jani Nikula jani.nik...@intel.com wrote: Detailed cap info at address 80h is not available with DPCD ver 1.0. Whether such devices exist in the wild I don't know, but there should be no harm done in having the defines for downstream port 0 in address 05h. Signed-off-by: Jani Nikula jani.nik...@intel.com --- include/drm/drm_dp_helper.h |8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h index ae8dbfb..83da4eb 100644 --- a/include/drm/drm_dp_helper.h +++ b/include/drm/drm_dp_helper.h @@ -77,10 +77,10 @@ #define DP_DOWNSTREAMPORT_PRESENT 0x005 # define DP_DWN_STRM_PORT_PRESENT (1 0) # define DP_DWN_STRM_PORT_TYPE_MASK 0x06 -/* 00b = DisplayPort */ -/* 01b = Analog */ -/* 10b = TMDS or HDMI */ -/* 11b = Other */ +# define DP_DWN_STRM_PORT_TYPE_DP (0 1) +# define DP_DWN_STRM_PORT_TYPE_ANALOG (1 1) +# define DP_DWN_STRM_PORT_TYPE_TMDS (2 1) +# define DP_DWN_STRM_PORT_TYPE_OTHER(3 1) # define DP_FORMAT_CONVERSION (1 3) # define DP_DETAILED_CAP_INFO_AVAILABLE(1 4) /* DPI */ -- 1.7.9.5 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
