On Monday 23 May 2016 01:48 PM, Ander Conselvan De Oliveira wrote:
On Fri, 2016-04-29 at 18:28 -0700, Manasi Navare wrote:
This patch addresses a few issues from the original patch for
DP Compliance EDID test support submitted by
Todd Previte<todd.previte@xxxxxxxxx>
Do you mean commit 559be30cb74d ("drm/i915: Implement the intel_dp_autotest_edid
function for DP EDID complaince tests")? Please see the link below on how to
refer to other commits in the commit message and how to add a Fixes: tag.
https://www.kernel.org/doc/Documentation/SubmittingPatches
Video Mode requested in the EDID test handler for the EDID Read
test (CTS 4.2.2.3) should be set to PREFERRED as per the CTS spec.
Intel connector status should be connected even if detect_edid is
NULL when compliance_test flag is set. This is required to handle
the corrupt EDID (CTS 4.2.2.6) or EDID Read Failure I2C NACK/I2C
DEFER (CTS 4.2.2.4 and 4.2.2.5) tests from CTS spec.
What exactly do those tests test? It sounds like this patch adds a separate code
path to implement the right behavior only when running the CTS. Shouldn't the
driver handle those failures during normal operation in the same way?
Signed-off-by: Manasi Navare<manasi.d.navare@xxxxxxxxx>
---
drivers/gpu/drm/i915/intel_dp.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 0961f22..456fc17 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4023,7 +4023,7 @@ static uint8_t intel_dp_autotest_video_pattern(struct
intel_dp *intel_dp)
static uint8_t intel_dp_autotest_edid(struct intel_dp *intel_dp)
{
- uint8_t test_result = DP_TEST_NAK;
+ uint8_t test_result = DP_TEST_ACK;
struct intel_connector *intel_connector = intel_dp-
attached_connector;
struct drm_connector *connector = &intel_connector->base;
@@ -4058,7 +4058,7 @@ static uint8_t intel_dp_autotest_edid(struct intel_dp
*intel_dp)
DRM_DEBUG_KMS("Failed to write EDID checksum\n");
test_result = DP_TEST_ACK | DP_TEST_EDID_CHECKSUM_WRITE;
- intel_dp->compliance_test_data =
INTEL_DP_RESOLUTION_STANDARD;
+ intel_dp->compliance_test_data =
INTEL_DP_RESOLUTION_PREFERRED;
Is this used for anything else than logging?
}
/* Set test active flag here so userspace doesn't interrupt things */
@@ -4650,7 +4650,7 @@ intel_dp_detect(struct drm_connector *connector, bool
force)
intel_dp->detect_done = false;
- if (intel_connector->detect_edid)
+ if (intel_connector->detect_edid || intel_dp->compliance_test_active)
Should this check connector->edid_corrupt instead? I guess that would require
some logic to fallback to fail safe mode and bpc too.
I think Shubhangi had a patch for this same problem, but it also seems to create
a separate path for compliance.
Ander
For tests:
* 4.2.2.4 : Failed EDID read, I2C_NAK
* 4.2.2.5 : Failed EDID read, I2C_DEFER
* 4.2.2.6 : EDID corruption detected
when edid is detected to be corrupt, we need to fall back to failsafe mode.
This requires change for bpc and status to be connected which is being
performed in the patch: https://patchwork.freedesktop.org/patch/82566/
"drm/i915: Use fail safe mode when edid is corrupt"
Shubhangi
return connector_status_connected;
else
return connector_status_disconnected;
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
