All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH V6] Displayport compliance testing V6
@ 2015-04-15 15:38 Todd Previte
  2015-04-15 15:38 ` [PATCH 01/10] drm/i915: Add automated testing support for Displayport compliance testing Todd Previte
                   ` (9 more replies)
  0 siblings, 10 replies; 31+ messages in thread
From: Todd Previte @ 2015-04-15 15:38 UTC (permalink / raw)
  To: intel-gfx

This is the 6th iteration of the Displayport compliance testing patch set for 
performing compliance testing operations of the i915 driver. High level changes 
are listed below, with the specifics for each patch listed in the commit messages.

Kernel:

Changes for V4:
- Removed the code for link configuration in debugfs. It wasn’t used in this 
  patch set so there was no need to add 500+ lines of code to the kernel. It may  
  be reintroduced in a future patch if it becomes necessary to support link 
  configuration testing for Displayport compliance
- Merged working changes in to the kernel code to keep the patches smaller and
  as discrete, testable units. This included moving variables around between 
  patches such that their declaration and use appears in the same patch. One 
  variable was removed entirely as it was no longer necessary.
- Changed the debugfs interface for test control. Previously test control was 
  handled by a single file that contained tags and values that were parsed and 
  used by both the user app and the kernel. This required a lot of parsing code    
  on both sides of the equation. That has been eliminated in favor of 3 separate,
  single value files for test type, test data and testing active. This reduces 
  the overhead of polling on test_active in the user app as well as eliminating 
  the need to parse a single monolithic file every time it checks the flag 
  (currently 1ms intervals). The net result is a more responsible app and a lot 
  less code on both sides.

Changes for V5:
- Removed a duplicate EDID read from the EDID auto test function
- Added a failsafe check in the I2C DEFER check to make sure a misbhaving
  device wouldn’t cause an infinite loop
- Shuffled around some variable declarations and assignments to put them in
  the correct patches and places
- Fixed a commit message that was no longer accurate
- Removed the main stream disable code from check_link_status as this doesn’t
  play nice with a number of systems. A replacement for this will be done in
  the user app when necessary.

Changes for V6:
- Addressed all the review feedback from V5 and made the necessary changes
- Added a new flag for detecting EDID header corruption for compliance testing
- Cleaned up checkpatch.pl issues
- Fixed whitespace/formatting problems
- Made substantial adjustments to the hpd_pulse code to make sure it works for
  all permutations of SST/MST and short/long pulses
- Reintegrated patches 2 and 3 into a single patch again
- Removed several instances of duplicate code
- Propagate the long_hpd flag into check_link_status to only perform the EDID
  read for a long pulse (hot plug event)

Userspace app:

The userspace app can be found here:

https://github.com/tprevite/intel-gpu-tools/tree/dp_compliance

The user app has the following requirements:
- Must be executed as root from the command line
- No other display managers can be running. Must be in console mode.
- Only the test device should be connected to one of the external Displayport 
  ports. No other displays should be connected via Displayport. eDP displays are 
  ignored by the compliance code so they should operate normally.

Previous versions of the user app disabled all displays on the DUT. That is no 
longer necessary in order to execute Displayport compliance testing. The app can 
be run remotely via an SSH login or directly on the DUT at the preference of the 
operator. Aside from starting and stopping the app itself, no direct interaction 
is necessary at this time on the part of the test operator to perform compliance 
testing.

Some of the tests will still pass if the user app isn't running. The primary   
purpose of the user space application is to handle the heavy lifting of the
mode sets required to set the specific display resolutions for the tests. 

Changes for V2:
	- Removed unnecessary memcpy()s and replaced with assignments
	- Updated the README
	- Moved the menu code around and reworded it to be more accurate
	- Added a “NONE” display mode to disable the main link

Changes for V3:
	- Rewrote the input handling code
	- Cleaned up some minor issues / extraneous tags
	- Fixed a minor bug in the setup of the failsafe video mode
	- Removed the delay on startup and updated the output messages
	  accordingly

The Github version is the most up to date, with several patches having been
applied recently which are encapsulated in the high level notes for V3 above.

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCH 01/10] drm/i915: Add automated testing support for Displayport compliance testing
  2015-04-15 15:38 [PATCH V6] Displayport compliance testing V6 Todd Previte
@ 2015-04-15 15:38 ` Todd Previte
  2015-04-16 11:32   ` Daniel Vetter
  2015-04-15 15:38 ` [PATCH 02/10] drm/i915: Update intel_dp_check_link_status() " Todd Previte
                   ` (8 subsequent siblings)
  9 siblings, 1 reply; 31+ messages in thread
From: Todd Previte @ 2015-04-15 15:38 UTC (permalink / raw)
  To: intel-gfx

Add the skeleton framework for supporting automation for Displayport compliance
testing. This patch adds the necessary framework for the source device to
appropriately respond to test automation requests from a sink device.

V2:
- Addressed previous mailing list feedback
- Fixed compilation issue (struct members declared in a later patch)
- Updated debug messages to be more accurate
- Added status checks for the DPCD read/write calls
- Removed excess comments and debug messages
- Fixed debug message compilation warnings
- Fixed compilation issue with missing variables
- Updated link training autotest to ACK

V3:
- Fixed the checks on the DPCD return code to be <= 0
  rather than != 0
- Removed extraneous assignment of a NAK return code in the
  DPCD read failure case
- Changed the return in the DPCD read failure case to a goto
  to the exit point where the status code is written to the sink
- Removed FAUX test case since it's deprecated now
- Removed the compliance flag assignment in handle_test_request

V4:
- Moved declaration of type_type here
- Removed declaration of test_data (moved to a later patch)
- Added reset to 0 for compliance test variables

V5:
- Moved test_active variable declaration and initialization out of
  this patch and into the patch where it's used
- Changed variable name compliance_testing_active to
  compliance_test_active to unify the naming convention
- Added initialization for compliance_test_type variable

Signed-off-by: Todd Previte <tprevite@gmail.com>
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c  | 75 +++++++++++++++++++++++++++++++++++++---
 drivers/gpu/drm/i915/intel_drv.h |  3 ++
 2 files changed, 74 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 14cdd00..263eff3 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -3970,11 +3970,78 @@ intel_dp_get_sink_irq_esi(struct intel_dp *intel_dp, u8 *sink_irq_vector)
 	return true;
 }
 
-static void
-intel_dp_handle_test_request(struct intel_dp *intel_dp)
+static uint8_t intel_dp_autotest_link_training(struct intel_dp *intel_dp)
+{
+	uint8_t test_result = DP_TEST_ACK;
+	return test_result;
+}
+
+static uint8_t intel_dp_autotest_video_pattern(struct intel_dp *intel_dp)
+{
+	uint8_t test_result = DP_TEST_NAK;
+	return test_result;
+}
+
+static uint8_t intel_dp_autotest_edid(struct intel_dp *intel_dp)
+{
+	uint8_t test_result = DP_TEST_NAK;
+	return test_result;
+}
+
+static uint8_t intel_dp_autotest_phy_pattern(struct intel_dp *intel_dp)
+{
+	uint8_t test_result = DP_TEST_NAK;
+	return test_result;
+}
+
+static void intel_dp_handle_test_request(struct intel_dp *intel_dp)
 {
-	/* NAK by default */
-	drm_dp_dpcd_writeb(&intel_dp->aux, DP_TEST_RESPONSE, DP_TEST_NAK);
+	uint8_t response = DP_TEST_NAK;
+	uint8_t rxdata = 0;
+	int status = 0;
+
+	intel_dp->compliance_test_type = 0;
+	intel_dp->aux.i2c_nack_count = 0;
+	intel_dp->aux.i2c_defer_count = 0;
+
+	status = drm_dp_dpcd_read(&intel_dp->aux, DP_TEST_REQUEST, &rxdata, 1);
+	if (status <= 0) {
+		DRM_DEBUG_KMS("Could not read test request from sink\n");
+		goto update_status;
+	}
+
+	switch (rxdata) {
+	case DP_TEST_LINK_TRAINING:
+		DRM_DEBUG_KMS("LINK_TRAINING test requested\n");
+		intel_dp->compliance_test_type = DP_TEST_LINK_TRAINING;
+		response = intel_dp_autotest_link_training(intel_dp);
+		break;
+	case DP_TEST_LINK_VIDEO_PATTERN:
+		DRM_DEBUG_KMS("TEST_PATTERN test requested\n");
+		intel_dp->compliance_test_type = DP_TEST_LINK_VIDEO_PATTERN;
+		response = intel_dp_autotest_video_pattern(intel_dp);
+		break;
+	case DP_TEST_LINK_EDID_READ:
+		DRM_DEBUG_KMS("EDID test requested\n");
+		intel_dp->compliance_test_type = DP_TEST_LINK_EDID_READ;
+		response = intel_dp_autotest_edid(intel_dp);
+		break;
+	case DP_TEST_LINK_PHY_TEST_PATTERN:
+		DRM_DEBUG_KMS("PHY_PATTERN test requested\n");
+		intel_dp->compliance_test_type = DP_TEST_LINK_PHY_TEST_PATTERN;
+		response = intel_dp_autotest_phy_pattern(intel_dp);
+		break;
+	default:
+		DRM_DEBUG_KMS("Invalid test request '%02x'\n", rxdata);
+		break;
+	}
+
+update_status:
+	status = drm_dp_dpcd_write(&intel_dp->aux,
+				   DP_TEST_RESPONSE,
+				   &response, 1);
+	if (status <= 0)
+		DRM_DEBUG_KMS("Could not write test response to sink\n");
 }
 
 static int
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 6a2ee0c..a4675fa 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -739,6 +739,9 @@ struct intel_dp {
 				     bool has_aux_irq,
 				     int send_bytes,
 				     uint32_t aux_clock_divider);
+
+	/* Displayport compliance testing */
+	unsigned long compliance_test_type;
 };
 
 struct intel_digital_port {
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 02/10] drm/i915: Update intel_dp_check_link_status() for Displayport compliance testing
  2015-04-15 15:38 [PATCH V6] Displayport compliance testing V6 Todd Previte
  2015-04-15 15:38 ` [PATCH 01/10] drm/i915: Add automated testing support for Displayport compliance testing Todd Previte
@ 2015-04-15 15:38 ` Todd Previte
  2015-04-15 19:28   ` [PATCH 02/12] " Todd Previte
  2015-04-15 15:38 ` [PATCH 03/10] drm/i915: Add EDID read in intel_dp_check_link_status() for Link CTS 4.2.2.1 Todd Previte
                   ` (7 subsequent siblings)
  9 siblings, 1 reply; 31+ messages in thread
From: Todd Previte @ 2015-04-15 15:38 UTC (permalink / raw)
  To: intel-gfx

This patch is a combination of changes that does the following:

	- Ignore disconnected Displayport connectors in check_link_status
	- Move the DPCD read further up in intel_dp_check_link_status()
	- Adds a new function that checks the HW HPD pin status
	- Replace the check for SW connected status with the new function

Adds a check at the top to verify that the device is connected. This is
necessary for DP compliance testing to ensure that test requests are captured
and acknowledged. If a test request is present during a connected->disconnected
transition, the test code will attempt to execute even though the device is
disconnected, resulting in a faied test.

This patch is actually both a bug fix and a component of compliance testing.
Because HPD events are received both on connect and disconnect actions, it's
vital that we don't try and train the link when we're transitioning from
connected->disconnected. That results in errors and warning in the logs from
failed AUX transactions and can trigger the WARN for the check of !base.crtc.
By making the check at the beginning to see if the connection is truly active,
those problems are avoided and testing / link training will only be attempted
when there is a valid Displayport connection.

Move the DPCD read to the top and check for an interrupt from the sink to catch
Displayport automated testing requests necessary to support Displayport
compliance testing. The checks for active connectors and link status are moved
below the check for the interrupt.

The main reason for doing this is to make sure that a test request isn't missed.
Checking for the status of the encoder/crtc isn't necessary for some test cases
(AUX channel tests are one example) and without moving the check for the
interrupt, these tests may not execute if one of those checks fails.
Additionally, if reading the DPCD fails, regardless of whether or not testing is
happening, there's no way to train the link since configurations and status
can't be read, nor can link training parameters be written.

V1:
- This is the second part of the single-patch split previously
  mentioned.

V2:
- Remerge the two split patches into one and update the commit message
  accordingly.
- Replace the SW connected status check with a HW HPD pin status check
- Adds a new function that examines the status of the HPD pin to
  determine if a sink device is connected
V3:
- Removed duplicate code from the hpd_pulse -> check_link_status path

Signed-off-by: Todd Previte <tprevite@gmail.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 32 +++++++++++++-------------------
 1 file changed, 13 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 263eff3..1352c00 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4119,6 +4119,19 @@ intel_dp_check_link_status(struct intel_dp *intel_dp)
 
 	WARN_ON(!drm_modeset_is_locked(&dev->mode_config.connection_mutex));
 
+	/* Try to read the source of the interrupt */
+	if (intel_dp->dpcd[DP_DPCD_REV] >= 0x11 &&
+	    intel_dp_get_sink_irq(intel_dp, &sink_irq_vector)) {
+		/* Clear interrupt source */
+		drm_dp_dpcd_writeb(&intel_dp->aux,
+				   DP_DEVICE_SERVICE_IRQ_VECTOR,
+				   sink_irq_vector);
+		if (sink_irq_vector & DP_AUTOMATED_TEST_REQUEST)
+			intel_dp_handle_test_request(intel_dp);
+		if (sink_irq_vector & (DP_CP_IRQ | DP_SINK_SPECIFIC_IRQ))
+			DRM_DEBUG_DRIVER("CP or sink specific irq unhandled\n");
+	}
+
 	if (!intel_encoder->connectors_active)
 		return;
 
@@ -4133,25 +4146,6 @@ intel_dp_check_link_status(struct intel_dp *intel_dp)
 		return;
 	}
 
-	/* Now read the DPCD to see if it's actually running */
-	if (!intel_dp_get_dpcd(intel_dp)) {
-		return;
-	}
-
-	/* Try to read the source of the interrupt */
-	if (intel_dp->dpcd[DP_DPCD_REV] >= 0x11 &&
-	    intel_dp_get_sink_irq(intel_dp, &sink_irq_vector)) {
-		/* Clear interrupt source */
-		drm_dp_dpcd_writeb(&intel_dp->aux,
-				   DP_DEVICE_SERVICE_IRQ_VECTOR,
-				   sink_irq_vector);
-
-		if (sink_irq_vector & DP_AUTOMATED_TEST_REQUEST)
-			intel_dp_handle_test_request(intel_dp);
-		if (sink_irq_vector & (DP_CP_IRQ | DP_SINK_SPECIFIC_IRQ))
-			DRM_DEBUG_DRIVER("CP or sink specific irq unhandled\n");
-	}
-
 	if (!drm_dp_channel_eq_ok(link_status, intel_dp->lane_count)) {
 		DRM_DEBUG_KMS("%s: channel EQ not ok, retraining\n",
 			      intel_encoder->base.name);
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 03/10] drm/i915: Add EDID read in intel_dp_check_link_status() for Link CTS 4.2.2.1
  2015-04-15 15:38 [PATCH V6] Displayport compliance testing V6 Todd Previte
  2015-04-15 15:38 ` [PATCH 01/10] drm/i915: Add automated testing support for Displayport compliance testing Todd Previte
  2015-04-15 15:38 ` [PATCH 02/10] drm/i915: Update intel_dp_check_link_status() " Todd Previte
@ 2015-04-15 15:38 ` Todd Previte
  2015-04-15 17:15   ` [PATCH 03/12] " Todd Previte
  2015-04-15 19:29   ` Todd Previte
  2015-04-15 15:38 ` [PATCH 04/10] drm/i915: Add a delay in Displayport AUX transactions for compliance testing Todd Previte
                   ` (6 subsequent siblings)
  9 siblings, 2 replies; 31+ messages in thread
From: Todd Previte @ 2015-04-15 15:38 UTC (permalink / raw)
  To: intel-gfx

Adds in an EDID read after the DPCD read to accommodate test 4.2.2.1 in the
Displayport Link CTS Core 1.2 rev1.1. This test requires an EDID read for
all HPD plug events. To reduce the amount of code, this EDID read is also
used for Link CTS tests 4.2.2.3, 4.2.2.4, 4.2.2.5 and 4.2.2.6. Actual
support for these tests is implemented in later patches in this series.

V2:
- Fixed compilation error introduced during rework
V3:
- Plugged a memory leak where the EDID data wasn't being freed
  after allocation in this function
V4:
- Fixed whitespace problems
- Cleaned up formatting

Signed-off-by: Todd Previte <tprevite@gmail.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 1352c00..c112359 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4114,11 +4114,24 @@ 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 = &dp_to_dig_port(intel_dp)->base;
+	struct drm_connector *connector = &intel_dp->attached_connector->base;
+	struct i2c_adapter *adapter = &intel_dp->aux.ddc;
+	struct edid *edid_read = NULL;
 	u8 sink_irq_vector;
 	u8 link_status[DP_LINK_STATUS_SIZE];
 
 	WARN_ON(!drm_modeset_is_locked(&dev->mode_config.connection_mutex));
 
+	/* Displayport Link CTS Core 1.2 rev1.1 EDID testing
+	 * 4.2.2.1 - EDID read required for all HPD events
+	 */
+	edid_read = drm_get_edid(connector, adapter);
+	if (!edid_read) {
+		DRM_DEBUG_DRIVER("Invalid EDID detected\n");
+	} else {
+		kfree(edid_read);
+	}
+
 	/* Try to read the source of the interrupt */
 	if (intel_dp->dpcd[DP_DPCD_REV] >= 0x11 &&
 	    intel_dp_get_sink_irq(intel_dp, &sink_irq_vector)) {
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 04/10] drm/i915: Add a delay in Displayport AUX transactions for compliance testing
  2015-04-15 15:38 [PATCH V6] Displayport compliance testing V6 Todd Previte
                   ` (2 preceding siblings ...)
  2015-04-15 15:38 ` [PATCH 03/10] drm/i915: Add EDID read in intel_dp_check_link_status() for Link CTS 4.2.2.1 Todd Previte
@ 2015-04-15 15:38 ` Todd Previte
  2015-04-16 12:37   ` Daniel Vetter
  2015-04-15 15:38 ` [PATCH 05/10] drm: Add supporting structure for Displayport Link CTS test 4.2.2.6 Todd Previte
                   ` (5 subsequent siblings)
  9 siblings, 1 reply; 31+ messages in thread
From: Todd Previte @ 2015-04-15 15:38 UTC (permalink / raw)
  To: intel-gfx

The Displayport Link Layer Compliance Testing Specification 1.2 rev 1.1
specifies that repeated AUX transactions after a failure (no response /
invalid response) must have a minimum delay of 400us before the resend can
occur. Tests 4.2.1.1 and 4.2.1.2 are two tests that require this specifically.

Also, the check for DP_AUX_CH_CTL_TIME_OUT_ERROR has been moved out into a
separate case. This case just continues with the next iteration of the loop
as the HW has already waited the required amount of time.

V2:
- Changed udelay() to usleep_range()
V3:
- Removed extraneous check for timeout
- Updated comment to reflect this change
V4:
- Reformatted a comment
V5:
- Added separate check for HW timeout on AUX transactions. A message
  is logged upon detection of this case.
V6:
- Add continue statement to HW timeout detect case
- Remove the log message indicating a timeout has been
  detected (review feedback)
V7:
- Updated the commit message to remove verbage about the HW timeout
  case that is no longer valid.

Signed-off-by: Todd Previte <tprevite@gmail.com>
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index c112359..dae5c9a 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -874,9 +874,18 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
 				   DP_AUX_CH_CTL_TIME_OUT_ERROR |
 				   DP_AUX_CH_CTL_RECEIVE_ERROR);
 
-			if (status & (DP_AUX_CH_CTL_TIME_OUT_ERROR |
-				      DP_AUX_CH_CTL_RECEIVE_ERROR))
+			if (status & DP_AUX_CH_CTL_TIME_OUT_ERROR)
 				continue;
+
+			/* DP CTS 1.2 Core Rev 1.1, 4.2.1.1 & 4.2.1.2
+			 *   400us delay required for errors and timeouts
+			 *   Timeout errors from the HW already meet this
+			 *   requirement so skip to next iteration
+			 */
+			if (status & DP_AUX_CH_CTL_RECEIVE_ERROR) {
+				usleep_range(400, 500);
+				continue;
+			}
 			if (status & DP_AUX_CH_CTL_DONE)
 				break;
 		}
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 05/10] drm: Add supporting structure for Displayport Link CTS test 4.2.2.6
  2015-04-15 15:38 [PATCH V6] Displayport compliance testing V6 Todd Previte
                   ` (3 preceding siblings ...)
  2015-04-15 15:38 ` [PATCH 04/10] drm/i915: Add a delay in Displayport AUX transactions for compliance testing Todd Previte
@ 2015-04-15 15:38 ` Todd Previte
  2015-04-15 17:15   ` [PATCH 05/12] " Todd Previte
                     ` (2 more replies)
  2015-04-15 15:38 ` [PATCH 06/10] drm/i915: Update intel_dp_hpd_pulse() for non-MST operation Todd Previte
                   ` (4 subsequent siblings)
  9 siblings, 3 replies; 31+ messages in thread
From: Todd Previte @ 2015-04-15 15:38 UTC (permalink / raw)
  To: intel-gfx; +Cc: dri-devel

Displayport compliance test 4.2.2.6 requires that a source device be capable of
detecting a corrupt EDID. The test specification states that the sink device
sets up the EDID with an invalid checksum. To do this, the sink sets up an
invalid EDID header, expecting the source device to generate the checksum and
compare it to the value stored in the last byte of the block data.

Unfortunately, the DRM EDID reading and parsing functions are actually too good
in this case; the header is fixed before the checksum is computed and thus the
code never sees the invalid checksum. This results in a failure to pass the
compliance test.

To correct this issue, a checksum is generated when the EDID header is detected
as corrupted. If the checksum is invalid, it sets the header_corrupt flag and
logs the errors. In the case of a more seriously damaged header (fixup score
less than the threshold) the code does not generate the checksum but does set
the header_corrupt flag.

V2:
- Removed the static bool global
- Added a bool to the drm_connector struct to reaplce the static one for
  holding the status of raw edid header corruption detection
- Modified the function signature of the is_valid function to take an
  additional parameter to store the corruption detected value
- Fixed the other callers of the above is_valid function
V3:
- Updated the commit message to be more clear about what and why this
  patch does what it does.
- Added comment in code to clarify the operations there
- Removed compliance variable and check_link_status update; those
  have been moved to a later patch
- Removed variable assignment from the bottom of the test handler
V4:
- Removed i915 tag from subject line as the patch is not i915-specific
V5:
- Moved code causing a compilation error to this patch where the variable
  is actually declared
- Maintained blank lines / spacing so as to not contaminate the patch
V6:
- Removed extra debug messages
- Added documentation to for the added parameter on drm_edid_block_valid
- Fixed more whitespace issues in check_link_status
- Added a clear of the header_corrupt flag to the end of the test handler
  in intel_dp.c
- Changed the usage of the new function prototype in several places to use
  NULL where it is not needed by compliance testing

Signed-off-by: Todd Previte <tprevite@gmail.com>
Cc: dri-devel@lists.freedesktop.org
---
 drivers/gpu/drm/drm_edid.c      | 30 ++++++++++++++++++++++++++----
 drivers/gpu/drm/drm_edid_load.c |  7 +++++--
 drivers/gpu/drm/i915/intel_dp.c |  6 +++++-
 include/drm/drm_crtc.h          |  8 +++++++-
 4 files changed, 43 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 53bc7a6..1ed18f5 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -1041,13 +1041,15 @@ static bool drm_edid_is_zero(const u8 *in_edid, int length)
  * @raw_edid: pointer to raw EDID block
  * @block: type of block to validate (0 for base, extension otherwise)
  * @print_bad_edid: if true, dump bad EDID blocks to the console
+ * @header_corrupt: if true, the header or checksum is invalid
  *
  * Validate a base or extension EDID block and optionally dump bad blocks to
  * the console.
  *
  * Return: True if the block is valid, false otherwise.
  */
-bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid)
+bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid,
+			  bool *header_corrupt)
 {
 	u8 csum;
 	struct edid *edid = (struct edid *)raw_edid;
@@ -1062,9 +1064,25 @@ bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid)
 		int score = drm_edid_header_is_valid(raw_edid);
 		if (score == 8) ;
 		else if (score >= edid_fixup) {
+			/* Displayport Link CTS Core 1.2 rev1.1 test 4.2.2.6
+			 * In order to properly generate the invalid checksum
+			 * required for this test, it must be generated using
+			 * the raw EDID data. Otherwise, the fix-up code here
+			 * will correct the problem, the checksum is correct
+			 * and the test fails
+			 */
+			csum = drm_edid_block_checksum(raw_edid);
+			if (csum) {
+				if (header_corrupt)
+					*header_corrupt = 1;
+			}
 			DRM_DEBUG("Fixing EDID header, your hardware may be failing\n");
 			memcpy(raw_edid, edid_header, sizeof(edid_header));
 		} else {
+			if (header_corrupt) {
+				DRM_DEBUG_DRIVER("Invalid EDID header\n");
+				*header_corrupt = 1;
+			}
 			goto bad;
 		}
 	}
@@ -1129,7 +1147,7 @@ bool drm_edid_is_valid(struct edid *edid)
 		return false;
 
 	for (i = 0; i <= edid->extensions; i++)
-		if (!drm_edid_block_valid(raw + i * EDID_LENGTH, i, true))
+		if (!drm_edid_block_valid(raw + i * EDID_LENGTH, i, true, NULL))
 			return false;
 
 	return true;
@@ -1232,7 +1250,8 @@ struct edid *drm_do_get_edid(struct drm_connector *connector,
 	for (i = 0; i < 4; i++) {
 		if (get_edid_block(data, block, 0, EDID_LENGTH))
 			goto out;
-		if (drm_edid_block_valid(block, 0, print_bad_edid))
+		if (drm_edid_block_valid(block, 0, print_bad_edid,
+					 &connector->edid_header_corrupt))
 			break;
 		if (i == 0 && drm_edid_is_zero(block, EDID_LENGTH)) {
 			connector->null_edid_counter++;
@@ -1257,7 +1276,10 @@ struct edid *drm_do_get_edid(struct drm_connector *connector,
 				  block + (valid_extensions + 1) * EDID_LENGTH,
 				  j, EDID_LENGTH))
 				goto out;
-			if (drm_edid_block_valid(block + (valid_extensions + 1) * EDID_LENGTH, j, print_bad_edid)) {
+			if (drm_edid_block_valid(block + (valid_extensions + 1)
+						 * EDID_LENGTH, j,
+						 print_bad_edid,
+						 NULL)) {
 				valid_extensions++;
 				break;
 			}
diff --git a/drivers/gpu/drm/drm_edid_load.c b/drivers/gpu/drm/drm_edid_load.c
index 4c0aa97..48b48e8 100644
--- a/drivers/gpu/drm/drm_edid_load.c
+++ b/drivers/gpu/drm/drm_edid_load.c
@@ -216,7 +216,8 @@ static void *edid_load(struct drm_connector *connector, const char *name,
 		goto out;
 	}
 
-	if (!drm_edid_block_valid(edid, 0, print_bad_edid)) {
+	if (!drm_edid_block_valid(edid, 0, print_bad_edid,
+				  &connector->edid_header_corrupt)) {
 		connector->bad_edid_counter++;
 		DRM_ERROR("Base block of EDID firmware \"%s\" is invalid ",
 		    name);
@@ -229,7 +230,9 @@ static void *edid_load(struct drm_connector *connector, const char *name,
 		if (i != valid_extensions + 1)
 			memcpy(edid + (valid_extensions + 1) * EDID_LENGTH,
 			    edid + i * EDID_LENGTH, EDID_LENGTH);
-		if (drm_edid_block_valid(edid + i * EDID_LENGTH, i, print_bad_edid))
+		if (drm_edid_block_valid(edid + i * EDID_LENGTH, i,
+					 print_bad_edid,
+					 NULL))
 			valid_extensions++;
 	}
 
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index dae5c9a..9181483 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4005,6 +4005,7 @@ static uint8_t intel_dp_autotest_phy_pattern(struct intel_dp *intel_dp)
 
 static void intel_dp_handle_test_request(struct intel_dp *intel_dp)
 {
+	struct drm_connector *connector = &intel_dp->attached_connector->base;
 	uint8_t response = DP_TEST_NAK;
 	uint8_t rxdata = 0;
 	int status = 0;
@@ -4051,6 +4052,9 @@ update_status:
 				   &response, 1);
 	if (status <= 0)
 		DRM_DEBUG_KMS("Could not write test response to sink\n");
+
+	/* Clear EDID corruption flag now */
+	connector->edid_header_corrupt = 0;
 }
 
 static int
@@ -4135,7 +4139,7 @@ intel_dp_check_link_status(struct intel_dp *intel_dp)
 	 * 4.2.2.1 - EDID read required for all HPD events
 	 */
 	edid_read = drm_get_edid(connector, adapter);
-	if (!edid_read) {
+	if (!edid_read || connector->edid_header_corrupt == 1) {
 		DRM_DEBUG_DRIVER("Invalid EDID detected\n");
 	} else {
 		kfree(edid_read);
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index d4e4b82..f7a4a92 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -719,6 +719,11 @@ struct drm_connector {
 	int null_edid_counter; /* needed to workaround some HW bugs where we get all 0s */
 	unsigned bad_edid_counter;
 
+	/* Flag for raw EDID header corruption - used in Displayport
+	 * compliance testing - * Displayport Link CTS Core 1.2 rev1.1 4.2.2.6
+	 */
+	bool edid_header_corrupt;
+
 	struct dentry *debugfs_entry;
 
 	struct drm_connector_state *state;
@@ -1443,7 +1448,8 @@ extern void drm_set_preferred_mode(struct drm_connector *connector,
 				   int hpref, int vpref);
 
 extern int drm_edid_header_is_valid(const u8 *raw_edid);
-extern bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid);
+extern bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid,
+				 bool *header_corrupt);
 extern bool drm_edid_is_valid(struct edid *edid);
 
 extern struct drm_tile_group *drm_mode_create_tile_group(struct drm_device *dev,
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 06/10] drm/i915: Update intel_dp_hpd_pulse() for non-MST operation
  2015-04-15 15:38 [PATCH V6] Displayport compliance testing V6 Todd Previte
                   ` (4 preceding siblings ...)
  2015-04-15 15:38 ` [PATCH 05/10] drm: Add supporting structure for Displayport Link CTS test 4.2.2.6 Todd Previte
@ 2015-04-15 15:38 ` Todd Previte
  2015-04-15 17:15   ` [PATCH 06/12] " Todd Previte
  2015-04-16  7:22   ` Todd Previte
  2015-04-15 15:38 ` [PATCH 07/10] drm/i915: Support EDID compliance tests with the intel_dp_autotest_edid() function Todd Previte
                   ` (3 subsequent siblings)
  9 siblings, 2 replies; 31+ messages in thread
From: Todd Previte @ 2015-04-15 15:38 UTC (permalink / raw)
  To: intel-gfx

Update the hot plug function to handle the SST case. Instead of placing
the SST case within the long/short pulse block, it is now handled after
determining that MST mode is not in use. This way, the topology management
layer can handle any MST-related operations while SST operations are still
correctly handled afterwards.

This patch also corrects the problem of SST mode only being handled in the
case of a short (0.5ms - 1.0ms) HPD pulse. For compliance testing purposes
both short and long pulses are used by the different tests, thus both cases
need to be addressed for SST.

This patch replaces [PATCH 10/10] drm/i915: Fix intel_dp_hot_plug() in the
previous compliance testing patch sequence. Review feedback on that patch
indicated that updating intel_dp_hot_plug() was not the correct place for
the test handler.

For the SST case, the main stream is disabled for long HPD pulses as this
generally indicates either a connect/disconnect event or link failure. For
a number of case in compliance testing, the source is required to disable
the main link upon detection of a long HPD.

V2:
- N/A
V3:
- Place the SST mode link status check into the mst_fail case
- Remove obsolete comment regarding SST mode operation
- Removed an erroneous line of code that snuck in during rebasing
V4:
- Added a disable of the main stream (DP transport) for the long pulse case
  for SST to support compliance testing
V5:
- Reworked SST handling to support tests 4.2.2.7 and 4.2.2.8
V6:
- Reformatted a comment
V7:
- Moved a comment again that was inadvertently moved
- Updated the code to properly handle all permutations of MST/SST and
  short/long pulse.
- Adds a new 'connected' flag that prevents unnecessary operations when
  the link is disconnected.
- Added a function to encapsulate detection of the HPD pin status in
  in order to determine connected status
- Reformatted the if-statements in the function so the braces are
  consistent for those with single statements after the if-statement

Signed-off-by: Todd Previte <tprevite@gmail.com>
---
 drivers/gpu/drm/i915/intel_dp.c  | 55 +++++++++++++++++++++++++---------------
 drivers/gpu/drm/i915/intel_drv.h |  1 +
 2 files changed, 35 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 9181483..f70d20e 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4745,6 +4745,7 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	enum intel_display_power_domain power_domain;
 	enum irqreturn ret = IRQ_NONE;
+	bool connected = false;
 
 	if (intel_dig_port->base.type != INTEL_OUTPUT_EDP)
 		intel_dig_port->base.type = INTEL_OUTPUT_DISPLAYPORT;
@@ -4768,19 +4769,15 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
 	power_domain = intel_display_port_power_domain(intel_encoder);
 	intel_display_power_get(dev_priv, power_domain);
 
+	connected = intel_dp_digital_port_connected(intel_dp);
+
 	if (long_hpd) {
 
-		if (HAS_PCH_SPLIT(dev)) {
-			if (!ibx_digital_port_connected(dev_priv, intel_dig_port))
-				goto mst_fail;
-		} else {
-			if (g4x_digital_port_connected(dev, intel_dig_port) != 1)
-				goto mst_fail;
-		}
+		if (!connected)
+			goto mst_fail;
 
-		if (!intel_dp_get_dpcd(intel_dp)) {
+		if (!intel_dp_get_dpcd(intel_dp))
 			goto mst_fail;
-		}
 
 		intel_dp_probe_oui(intel_dp);
 
@@ -4788,20 +4785,9 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
 			goto mst_fail;
 
 	} else {
-		if (intel_dp->is_mst) {
+		if (intel_dp->is_mst)
 			if (intel_dp_check_mst_status(intel_dp) == -EINVAL)
 				goto mst_fail;
-		}
-
-		if (!intel_dp->is_mst) {
-			/*
-			 * we'll check the link status via the normal hot plug path later -
-			 * but for short hpds we should check it now
-			 */
-			drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
-			intel_dp_check_link_status(intel_dp);
-			drm_modeset_unlock(&dev->mode_config.connection_mutex);
-		}
 	}
 
 	ret = IRQ_HANDLED;
@@ -4815,6 +4801,15 @@ mst_fail:
 		drm_dp_mst_topology_mgr_set_mst(&intel_dp->mst_mgr, intel_dp->is_mst);
 	}
 put_power:
+	/* SST mode - handle short/long pulses here */
+	if (!intel_dp->is_mst) {
+
+		drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
+		if (connected)
+			intel_dp_check_link_status(intel_dp);
+		drm_modeset_unlock(&dev->mode_config.connection_mutex);
+		ret = IRQ_HANDLED;
+	}
 	intel_display_power_put(dev_priv, power_domain);
 
 	return ret;
@@ -5801,3 +5796,21 @@ void intel_dp_mst_resume(struct drm_device *dev)
 		}
 	}
 }
+
+bool intel_dp_digital_port_connected(struct intel_dp *intel_dp)
+{
+	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
+	struct drm_device *dev = intel_dig_port->base.base.dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	bool connected = true;
+
+	if (HAS_PCH_SPLIT(dev)) {
+		if (!ibx_digital_port_connected(dev_priv, intel_dig_port))
+			connected = false;
+	} else {
+		if (g4x_digital_port_connected(dev, intel_dig_port) != 1)
+			connected = false;
+	}
+
+	return connected;
+}
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index a4675fa..accf08b 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1173,6 +1173,7 @@ void intel_edp_drrs_disable(struct intel_dp *intel_dp);
 void intel_edp_drrs_invalidate(struct drm_device *dev,
 		unsigned frontbuffer_bits);
 void intel_edp_drrs_flush(struct drm_device *dev, unsigned frontbuffer_bits);
+bool intel_dp_digital_port_connected(struct intel_dp *intel_dp);
 
 /* intel_dp_mst.c */
 int intel_dp_mst_encoder_init(struct intel_digital_port *intel_dig_port, int conn_id);
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 07/10] drm/i915: Support EDID compliance tests with the intel_dp_autotest_edid() function
  2015-04-15 15:38 [PATCH V6] Displayport compliance testing V6 Todd Previte
                   ` (5 preceding siblings ...)
  2015-04-15 15:38 ` [PATCH 06/10] drm/i915: Update intel_dp_hpd_pulse() for non-MST operation Todd Previte
@ 2015-04-15 15:38 ` Todd Previte
  2015-04-15 17:16   ` [PATCH 07/12] " Todd Previte
  2015-04-15 15:38 ` [PATCH 08/10] drm: Fix for DP CTS test 4.2.2.5 - I2C DEFER handling Todd Previte
                   ` (2 subsequent siblings)
  9 siblings, 1 reply; 31+ messages in thread
From: Todd Previte @ 2015-04-15 15:38 UTC (permalink / raw)
  To: intel-gfx

Updates the EDID compliance test function to perform the EDID read as
required by the tests. This read needs to take place in the kernel for
reasons of speed and efficiency. The results of the EDID read operations
are handed off to userspace so that the userspace app can set the display
mode appropriately for the test response.

The compliance_test_active flag now appears at the end of the individual
test handling functions. This is so that the kernel-side operations can
be completed without the risk of interruption from the userspace app
that is polling on that flag.

V2:
- Addressed mailing list feedback
- Removed excess debug messages
- Removed extraneous comments
- Fixed formatting issues (line length > 80)
- Updated the debug message in compute_edid_checksum to output hex values
  instead of decimal
V3:
- Addressed more list feedback
- Added the test_active flag to the autotest function
- Removed test_active flag from handler
- Added failsafe check on the compliance test active flag
  at the end of the test handler
- Fixed checkpatch.pl issues
V4:
- Removed the checksum computation function and its use as it has been
  rendered superfluous by changes to the core DRM EDID functions
- Updated to use the raw header corruption detection mechanism
- Moved the declaration of the test_data variable here
V5:
- Update test active flag variable name to match the change in the
  first patch of the series.
- Relocated the test active flag declaration and initialization
  to this patch
V6:
- Updated to use the new flag for raw EDID header corruption
- Removed the extra EDID read from the autotest function
- Added the edid_checksum variable to struct intel_dp so that the
  autotest function can write it to the sink device
- Moved the update to the hpd_pulse function to another patch
- Removed extraneous constants
V7:
- Fixed erroneous placement of the checksum assignment. In some cases
  such as when the EDID read fails and is NULL, this causes a NULL ptr
  dereference in the kernel. Bad news. Fixed now.
V8:
- Updated to support the kfree() on the EDID data added previously

Signed-off-by: Todd Previte <tprevite@gmail.com>
---
 drivers/gpu/drm/i915/intel_dp.c  | 47 +++++++++++++++++++++++++++++++++++++++-
 drivers/gpu/drm/i915/intel_drv.h |  4 ++++
 2 files changed, 50 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index f70d20e..033b0ef 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -41,6 +41,12 @@
 
 #define DP_LINK_CHECK_TIMEOUT	(10 * 1000)
 
+/* Compliance test status bits  */
+#define INTEL_DP_RESOLUTION_SHIFT_MASK	4
+#define INTEL_DP_RESOLUTION_PREFERRED	(1 << INTEL_DP_RESOLUTION_SHIFT_MASK)
+#define INTEL_DP_RESOLUTION_STANDARD	(2 << INTEL_DP_RESOLUTION_SHIFT_MASK)
+#define INTEL_DP_RESOLUTION_FAILSAFE	(3 << INTEL_DP_RESOLUTION_SHIFT_MASK)
+
 struct dp_link_dpll {
 	int link_bw;
 	struct dpll dpll;
@@ -3994,6 +4000,35 @@ 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;
+	uint32_t ret = 0;
+
+	if (intel_dp->compliance_edid_invalid ||
+	    intel_dp->aux.i2c_defer_count > 6) {
+		/* Check for NACKs/DEFERs, use failsafe if detected
+		 *  (DP CTS 1.2 Core Rev 1.1, 4.2.2.4, 4.2.2.5)
+		 */
+		if (intel_dp->aux.i2c_nack_count > 0 ||
+			intel_dp->aux.i2c_defer_count > 0)
+			DRM_DEBUG_KMS("EDID read had %d NACKs, %d DEFERs\n",
+				      intel_dp->aux.i2c_nack_count,
+				      intel_dp->aux.i2c_defer_count);
+		intel_dp->compliance_test_data = INTEL_DP_RESOLUTION_FAILSAFE;
+	} else {
+		ret = drm_dp_dpcd_write(&intel_dp->aux,
+					DP_TEST_EDID_CHECKSUM,
+					&intel_dp->compliance_edid_checksum, 1);
+		if (ret <= 0)
+			DRM_DEBUG_DRIVER("Failed to write EDID checksum\n");
+		else
+			DRM_DEBUG_DRIVER("EDID checksum written to sink\n");
+
+		test_result = DP_TEST_ACK | DP_TEST_EDID_CHECKSUM_WRITE;
+		intel_dp->compliance_test_data = INTEL_DP_RESOLUTION_STANDARD;
+	}
+
+	/* Set test active flag here so userspace doesn't interrupt things */
+	intel_dp->compliance_test_active = 1;
+
 	return test_result;
 }
 
@@ -4010,7 +4045,10 @@ static void intel_dp_handle_test_request(struct intel_dp *intel_dp)
 	uint8_t rxdata = 0;
 	int status = 0;
 
+	intel_dp->compliance_test_active = 0;
 	intel_dp->compliance_test_type = 0;
+	intel_dp->compliance_test_data = 0;
+
 	intel_dp->aux.i2c_nack_count = 0;
 	intel_dp->aux.i2c_defer_count = 0;
 
@@ -4137,11 +4175,18 @@ intel_dp_check_link_status(struct intel_dp *intel_dp)
 
 	/* Displayport Link CTS Core 1.2 rev1.1 EDID testing
 	 * 4.2.2.1 - EDID read required for all HPD events
-	 */
+	 * Also used for 4.2.2.3, 4.2.2.4 4.2.2.5 and 4.2.2.6
+	 *   - compliance_edid_invalid flag used for 4.2.2.6
+	 *   - compliance_edid_checksum used for 4.2.2.3
+	*/
+	intel_dp->compliance_edid_invalid = 0;
 	edid_read = drm_get_edid(connector, adapter);
 	if (!edid_read || connector->edid_header_corrupt == 1) {
 		DRM_DEBUG_DRIVER("Invalid EDID detected\n");
+		intel_dp->compliance_edid_invalid = 1;
+		intel_dp->compliance_edid_checksum = 0;
 	} else {
+		intel_dp->compliance_edid_checksum = edid_read->checksum;
 		kfree(edid_read);
 	}
 
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index accf08b..73ced46 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -742,6 +742,10 @@ struct intel_dp {
 
 	/* Displayport compliance testing */
 	unsigned long compliance_test_type;
+	unsigned long compliance_test_data;
+	bool compliance_test_active;
+	bool compliance_edid_invalid;
+	uint8_t compliance_edid_checksum;
 };
 
 struct intel_digital_port {
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 08/10] drm: Fix for DP CTS test 4.2.2.5 - I2C DEFER handling
  2015-04-15 15:38 [PATCH V6] Displayport compliance testing V6 Todd Previte
                   ` (6 preceding siblings ...)
  2015-04-15 15:38 ` [PATCH 07/10] drm/i915: Support EDID compliance tests with the intel_dp_autotest_edid() function Todd Previte
@ 2015-04-15 15:38 ` Todd Previte
  2015-04-16 20:23   ` Paulo Zanoni
  2015-04-15 15:38 ` [PATCH 09/10] drm/i915: Add debugfs test control files for Displayport compliance testing Todd Previte
  2015-04-15 15:38 ` [PATCH 10/10] drm: Fix the 'native defer' message in drm_dp_i2c_do_msg() Todd Previte
  9 siblings, 1 reply; 31+ messages in thread
From: Todd Previte @ 2015-04-15 15:38 UTC (permalink / raw)
  To: intel-gfx; +Cc: dri-devel

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 <tprevite@gmail.com>
Cc: dri-devel@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(&aux->hw_mutex);
 		ret = aux->transfer(aux, msg);
 		mutex_unlock(&aux->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

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 09/10] drm/i915: Add debugfs test control files for Displayport compliance testing
  2015-04-15 15:38 [PATCH V6] Displayport compliance testing V6 Todd Previte
                   ` (7 preceding siblings ...)
  2015-04-15 15:38 ` [PATCH 08/10] drm: Fix for DP CTS test 4.2.2.5 - I2C DEFER handling Todd Previte
@ 2015-04-15 15:38 ` Todd Previte
  2015-04-16 21:25   ` Paulo Zanoni
  2015-04-15 15:38 ` [PATCH 10/10] drm: Fix the 'native defer' message in drm_dp_i2c_do_msg() Todd Previte
  9 siblings, 1 reply; 31+ messages in thread
From: Todd Previte @ 2015-04-15 15:38 UTC (permalink / raw)
  To: intel-gfx

This patch adds 3 debugfs files for handling Displayport compliance testing
and supercedes the previous patches that implemented debugfs support for
compliance testing. Those patches were:

- [PATCH 04/17] drm/i915: Add debugfs functions for Displayport
                          compliance testing
- [PATCH 08/17] drm/i915: Add new debugfs file for Displayport
                          compliance test control
- [PATCH 09/17] drm/i915: Add debugfs write and test param parsing
                          functions for DP test control

This new patch simplifies the debugfs implementation by places a single
test control value into an individual file. Each file is readable by
the usersapce application and the test_active file is writable to
indicate to the kernel when userspace has completed its portion of the
test sequence.

Replacing the previous files simplifies operation and speeds response
time for the user app, as it is required to poll on the test_active file
in order to determine when it needs to begin its operations.

V2:
- Updated the test active variable name to match the change in
  the initial patch of the series
V3:
- Added a fix in the test_active_write function to prevent a NULL pointer
  dereference if the encoder on the connector is invalid

Signed-off-by: Todd Previte <tprevite@gmail.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c | 209 ++++++++++++++++++++++++++++++++++++
 1 file changed, 209 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 2394924..c33d390 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -3937,6 +3937,212 @@ static const struct file_operations i915_display_crc_ctl_fops = {
 	.write = display_crc_ctl_write
 };
 
+static ssize_t i915_displayport_test_active_write(struct file *file,
+					    const char __user *ubuf,
+					    size_t len, loff_t *offp)
+{
+	char *input_buffer;
+	int status = 0;
+	struct seq_file *m;
+	struct drm_device *dev;
+	struct drm_connector *connector;
+	struct list_head *connector_list;
+	struct intel_dp *intel_dp;
+	int val = 0;
+
+	m = file->private_data;
+	if (!m) {
+		status = -ENODEV;
+		return status;
+	}
+	dev = m->private;
+
+	if (!dev) {
+		status = -ENODEV;
+		return status;
+	}
+	connector_list = &dev->mode_config.connector_list;
+
+	if (len == 0)
+		return 0;
+
+	input_buffer = kmalloc(len + 1, GFP_KERNEL);
+	if (!input_buffer)
+		return -ENOMEM;
+
+	if (copy_from_user(input_buffer, ubuf, len)) {
+		status = -EFAULT;
+		goto out;
+	}
+
+	input_buffer[len] = '\0';
+	DRM_DEBUG_DRIVER("Copied %d bytes from user\n", (unsigned int)len);
+
+	list_for_each_entry(connector, connector_list, head) {
+
+		if (connector->connector_type !=
+		    DRM_MODE_CONNECTOR_DisplayPort)
+			continue;
+
+		if (connector->connector_type ==
+		    DRM_MODE_CONNECTOR_DisplayPort &&
+		    connector->status == connector_status_connected &&
+		    connector->encoder != NULL) {
+			intel_dp = enc_to_intel_dp(connector->encoder);
+			status = kstrtoint(input_buffer, 10, &val);
+			if (status < 0)
+				goto out;
+			DRM_DEBUG_DRIVER("Got %d for test active\n", val);
+			/* To prevent erroneous activation of the compliance
+			 * testing code, only accept an actual value of 1 here
+			 */
+			if (val == 1)
+				intel_dp->compliance_test_active = 1;
+			else
+				intel_dp->compliance_test_active = 0;
+		}
+	}
+out:
+	kfree(input_buffer);
+	if (status < 0)
+		return status;
+
+	*offp += len;
+	return len;
+}
+
+static int i915_displayport_test_active_show(struct seq_file *m, void *data)
+{
+	struct drm_device *dev = m->private;
+	struct drm_connector *connector;
+	struct list_head *connector_list = &dev->mode_config.connector_list;
+	struct intel_dp *intel_dp;
+
+	if (!dev)
+		return -ENODEV;
+
+	list_for_each_entry(connector, connector_list, head) {
+
+		if (connector->connector_type !=
+		    DRM_MODE_CONNECTOR_DisplayPort)
+			continue;
+
+		if (connector->status == connector_status_connected &&
+		    connector->encoder != NULL) {
+			intel_dp = enc_to_intel_dp(connector->encoder);
+			if (intel_dp->compliance_test_active)
+				seq_puts(m, "1");
+			else
+				seq_puts(m, "0");
+		} else
+			seq_puts(m, "0");
+	}
+
+	return 0;
+}
+
+static int i915_displayport_test_active_open(struct inode *inode,
+				       struct file *file)
+{
+	struct drm_device *dev = inode->i_private;
+
+	return single_open(file, i915_displayport_test_active_show, dev);
+}
+
+static const struct file_operations i915_displayport_test_active_fops = {
+	.owner = THIS_MODULE,
+	.open = i915_displayport_test_active_open,
+	.read = seq_read,
+	.llseek = seq_lseek,
+	.release = single_release,
+	.write = i915_displayport_test_active_write
+};
+
+static int i915_displayport_test_data_show(struct seq_file *m, void *data)
+{
+	struct drm_device *dev = m->private;
+	struct drm_connector *connector;
+	struct list_head *connector_list = &dev->mode_config.connector_list;
+	struct intel_dp *intel_dp;
+
+	if (!dev)
+		return -ENODEV;
+
+	list_for_each_entry(connector, connector_list, head) {
+
+		if (connector->connector_type !=
+		    DRM_MODE_CONNECTOR_DisplayPort)
+			continue;
+
+		if (connector->status == connector_status_connected &&
+		    connector->encoder != NULL) {
+			intel_dp = enc_to_intel_dp(connector->encoder);
+			seq_printf(m, "%lx", intel_dp->compliance_test_data);
+		} else
+			seq_puts(m, "0");
+	}
+
+	return 0;
+}
+static int i915_displayport_test_data_open(struct inode *inode,
+				       struct file *file)
+{
+	struct drm_device *dev = inode->i_private;
+
+	return single_open(file, i915_displayport_test_data_show, dev);
+}
+
+static const struct file_operations i915_displayport_test_data_fops = {
+	.owner = THIS_MODULE,
+	.open = i915_displayport_test_data_open,
+	.read = seq_read,
+	.llseek = seq_lseek,
+	.release = single_release
+};
+
+static int i915_displayport_test_type_show(struct seq_file *m, void *data)
+{
+	struct drm_device *dev = m->private;
+	struct drm_connector *connector;
+	struct list_head *connector_list = &dev->mode_config.connector_list;
+	struct intel_dp *intel_dp;
+
+	if (!dev)
+		return -ENODEV;
+
+	list_for_each_entry(connector, connector_list, head) {
+
+		if (connector->connector_type !=
+		    DRM_MODE_CONNECTOR_DisplayPort)
+			continue;
+
+		if (connector->status == connector_status_connected &&
+		    connector->encoder != NULL) {
+			intel_dp = enc_to_intel_dp(connector->encoder);
+			seq_printf(m, "%02lx", intel_dp->compliance_test_type);
+		} else
+			seq_puts(m, "0");
+	}
+
+	return 0;
+}
+
+static int i915_displayport_test_type_open(struct inode *inode,
+				       struct file *file)
+{
+	struct drm_device *dev = inode->i_private;
+
+	return single_open(file, i915_displayport_test_type_show, dev);
+}
+
+static const struct file_operations i915_displayport_test_type_fops = {
+	.owner = THIS_MODULE,
+	.open = i915_displayport_test_type_open,
+	.read = seq_read,
+	.llseek = seq_lseek,
+	.release = single_release
+};
+
 static void wm_latency_show(struct seq_file *m, const uint16_t wm[8])
 {
 	struct drm_device *dev = m->private;
@@ -4832,6 +5038,9 @@ static const struct i915_debugfs_files {
 	{"i915_spr_wm_latency", &i915_spr_wm_latency_fops},
 	{"i915_cur_wm_latency", &i915_cur_wm_latency_fops},
 	{"i915_fbc_false_color", &i915_fbc_fc_fops},
+	{"i915_dp_test_data", &i915_displayport_test_data_fops},
+	{"i915_dp_test_type", &i915_displayport_test_type_fops},
+	{"i915_dp_test_active", &i915_displayport_test_active_fops}
 };
 
 void intel_display_crc_init(struct drm_device *dev)
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 10/10] drm: Fix the 'native defer' message in drm_dp_i2c_do_msg()
  2015-04-15 15:38 [PATCH V6] Displayport compliance testing V6 Todd Previte
                   ` (8 preceding siblings ...)
  2015-04-15 15:38 ` [PATCH 09/10] drm/i915: Add debugfs test control files for Displayport compliance testing Todd Previte
@ 2015-04-15 15:38 ` Todd Previte
  2015-04-16 12:42   ` Daniel Vetter
  2015-04-16 12:44   ` Daniel Vetter
  9 siblings, 2 replies; 31+ messages in thread
From: Todd Previte @ 2015-04-15 15:38 UTC (permalink / raw)
  To: intel-gfx; +Cc: dri-devel

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 <tprevite@gmail.com>
Cc: dri-devel@lists.freedesktop.org
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
---
 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

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 03/12] drm/i915: Add EDID read in intel_dp_check_link_status() for Link CTS 4.2.2.1
  2015-04-15 15:38 ` [PATCH 03/10] drm/i915: Add EDID read in intel_dp_check_link_status() for Link CTS 4.2.2.1 Todd Previte
@ 2015-04-15 17:15   ` Todd Previte
  2015-04-15 19:29   ` Todd Previte
  1 sibling, 0 replies; 31+ messages in thread
From: Todd Previte @ 2015-04-15 17:15 UTC (permalink / raw)
  To: intel-gfx

Adds in an EDID read after the DPCD read to accommodate test 4.2.2.1 in the
Displayport Link CTS Core 1.2 rev1.1. This test requires an EDID read for
all HPD plug events. To reduce the amount of code, this EDID read is also
used for Link CTS tests 4.2.2.3, 4.2.2.4, 4.2.2.5 and 4.2.2.6. Actual
support for these tests is implemented in later patches in this series.

V2:
- Fixed compilation error introduced during rework
V3:
- Plugged a memory leak where the EDID data wasn't being freed
  after allocation in this function
V4:
- Fixed whitespace problems
- Cleaned up formatting
V5:
- Added propagation of the long_hpd flag from the hot_pulse function

Signed-off-by: Todd Previte <tprevite@gmail.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 1352c00..23586f6 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4110,15 +4110,30 @@ go_again:
  *  4. Check link status on receipt of hot-plug interrupt
  */
 static void
-intel_dp_check_link_status(struct intel_dp *intel_dp)
+intel_dp_check_link_status(struct intel_dp *intel_dp, bool long_hpd)
 {
 	struct drm_device *dev = intel_dp_to_dev(intel_dp);
 	struct intel_encoder *intel_encoder = &dp_to_dig_port(intel_dp)->base;
+	struct drm_connector *connector = &intel_dp->attached_connector->base;
+	struct i2c_adapter *adapter = &intel_dp->aux.ddc;
+	struct edid *edid_read = NULL;
 	u8 sink_irq_vector;
 	u8 link_status[DP_LINK_STATUS_SIZE];
 
 	WARN_ON(!drm_modeset_is_locked(&dev->mode_config.connection_mutex));
 
+	/* Displayport Link CTS Core 1.2 rev1.1 EDID testing
+	 * 4.2.2.1 - EDID read required for all HPD events
+	 */
+	if (long_hpd) {
+		edid_read = drm_get_edid(connector, adapter);
+		if (!edid_read) {
+			DRM_DEBUG_DRIVER("Invalid EDID detected\n");
+		} else {
+			kfree(edid_read);
+		}
+	}
+
 	/* Try to read the source of the interrupt */
 	if (intel_dp->dpcd[DP_DPCD_REV] >= 0x11 &&
 	    intel_dp_get_sink_irq(intel_dp, &sink_irq_vector)) {
@@ -4773,7 +4788,7 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
 			 * but for short hpds we should check it now
 			 */
 			drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
-			intel_dp_check_link_status(intel_dp);
+			intel_dp_check_link_status(intel_dp, long_hpd);
 			drm_modeset_unlock(&dev->mode_config.connection_mutex);
 		}
 	}
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 05/12] drm: Add supporting structure for Displayport Link CTS test 4.2.2.6
  2015-04-15 15:38 ` [PATCH 05/10] drm: Add supporting structure for Displayport Link CTS test 4.2.2.6 Todd Previte
@ 2015-04-15 17:15   ` Todd Previte
  2015-04-15 20:25     ` [Intel-gfx] " Paulo Zanoni
  2015-04-15 22:03   ` Todd Previte
  2015-04-16 15:47   ` Todd Previte
  2 siblings, 1 reply; 31+ messages in thread
From: Todd Previte @ 2015-04-15 17:15 UTC (permalink / raw)
  To: intel-gfx; +Cc: dri-devel

Displayport compliance test 4.2.2.6 requires that a source device be capable of
detecting a corrupt EDID. The test specification states that the sink device
sets up the EDID with an invalid checksum. To do this, the sink sets up an
invalid EDID header, expecting the source device to generate the checksum and
compare it to the value stored in the last byte of the block data.

Unfortunately, the DRM EDID reading and parsing functions are actually too good
in this case; the header is fixed before the checksum is computed and thus the
code never sees the invalid checksum. This results in a failure to pass the
compliance test.

To correct this issue, a checksum is generated when the EDID header is detected
as corrupted. If the checksum is invalid, it sets the header_corrupt flag and
logs the errors. In the case of a more seriously damaged header (fixup score
less than the threshold) the code does not generate the checksum but does set
the header_corrupt flag.

V2:
- Removed the static bool global
- Added a bool to the drm_connector struct to reaplce the static one for
  holding the status of raw edid header corruption detection
- Modified the function signature of the is_valid function to take an
  additional parameter to store the corruption detected value
- Fixed the other callers of the above is_valid function
V3:
- Updated the commit message to be more clear about what and why this
  patch does what it does.
- Added comment in code to clarify the operations there
- Removed compliance variable and check_link_status update; those
  have been moved to a later patch
- Removed variable assignment from the bottom of the test handler
V4:
- Removed i915 tag from subject line as the patch is not i915-specific
V5:
- Moved code causing a compilation error to this patch where the variable
  is actually declared
- Maintained blank lines / spacing so as to not contaminate the patch
V6:
- Removed extra debug messages
- Added documentation to for the added parameter on drm_edid_block_valid
- Fixed more whitespace issues in check_link_status
- Added a clear of the header_corrupt flag to the end of the test handler
  in intel_dp.c
- Changed the usage of the new function prototype in several places to use
  NULL where it is not needed by compliance testing
V7:
- Updated to account for long_pulse flag propagation

Signed-off-by: Todd Previte <tprevite@gmail.com>
Cc: dri-devel@lists.freedesktop.org
---
 drivers/gpu/drm/drm_edid.c      | 30 ++++++++++++++++++++++++++----
 drivers/gpu/drm/drm_edid_load.c |  7 +++++--
 drivers/gpu/drm/i915/intel_dp.c |  6 +++++-
 include/drm/drm_crtc.h          |  8 +++++++-
 4 files changed, 43 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 53bc7a6..1ed18f5 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -1041,13 +1041,15 @@ static bool drm_edid_is_zero(const u8 *in_edid, int length)
  * @raw_edid: pointer to raw EDID block
  * @block: type of block to validate (0 for base, extension otherwise)
  * @print_bad_edid: if true, dump bad EDID blocks to the console
+ * @header_corrupt: if true, the header or checksum is invalid
  *
  * Validate a base or extension EDID block and optionally dump bad blocks to
  * the console.
  *
  * Return: True if the block is valid, false otherwise.
  */
-bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid)
+bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid,
+			  bool *header_corrupt)
 {
 	u8 csum;
 	struct edid *edid = (struct edid *)raw_edid;
@@ -1062,9 +1064,25 @@ bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid)
 		int score = drm_edid_header_is_valid(raw_edid);
 		if (score == 8) ;
 		else if (score >= edid_fixup) {
+			/* Displayport Link CTS Core 1.2 rev1.1 test 4.2.2.6
+			 * In order to properly generate the invalid checksum
+			 * required for this test, it must be generated using
+			 * the raw EDID data. Otherwise, the fix-up code here
+			 * will correct the problem, the checksum is correct
+			 * and the test fails
+			 */
+			csum = drm_edid_block_checksum(raw_edid);
+			if (csum) {
+				if (header_corrupt)
+					*header_corrupt = 1;
+			}
 			DRM_DEBUG("Fixing EDID header, your hardware may be failing\n");
 			memcpy(raw_edid, edid_header, sizeof(edid_header));
 		} else {
+			if (header_corrupt) {
+				DRM_DEBUG_DRIVER("Invalid EDID header\n");
+				*header_corrupt = 1;
+			}
 			goto bad;
 		}
 	}
@@ -1129,7 +1147,7 @@ bool drm_edid_is_valid(struct edid *edid)
 		return false;
 
 	for (i = 0; i <= edid->extensions; i++)
-		if (!drm_edid_block_valid(raw + i * EDID_LENGTH, i, true))
+		if (!drm_edid_block_valid(raw + i * EDID_LENGTH, i, true, NULL))
 			return false;
 
 	return true;
@@ -1232,7 +1250,8 @@ struct edid *drm_do_get_edid(struct drm_connector *connector,
 	for (i = 0; i < 4; i++) {
 		if (get_edid_block(data, block, 0, EDID_LENGTH))
 			goto out;
-		if (drm_edid_block_valid(block, 0, print_bad_edid))
+		if (drm_edid_block_valid(block, 0, print_bad_edid,
+					 &connector->edid_header_corrupt))
 			break;
 		if (i == 0 && drm_edid_is_zero(block, EDID_LENGTH)) {
 			connector->null_edid_counter++;
@@ -1257,7 +1276,10 @@ struct edid *drm_do_get_edid(struct drm_connector *connector,
 				  block + (valid_extensions + 1) * EDID_LENGTH,
 				  j, EDID_LENGTH))
 				goto out;
-			if (drm_edid_block_valid(block + (valid_extensions + 1) * EDID_LENGTH, j, print_bad_edid)) {
+			if (drm_edid_block_valid(block + (valid_extensions + 1)
+						 * EDID_LENGTH, j,
+						 print_bad_edid,
+						 NULL)) {
 				valid_extensions++;
 				break;
 			}
diff --git a/drivers/gpu/drm/drm_edid_load.c b/drivers/gpu/drm/drm_edid_load.c
index 4c0aa97..48b48e8 100644
--- a/drivers/gpu/drm/drm_edid_load.c
+++ b/drivers/gpu/drm/drm_edid_load.c
@@ -216,7 +216,8 @@ static void *edid_load(struct drm_connector *connector, const char *name,
 		goto out;
 	}
 
-	if (!drm_edid_block_valid(edid, 0, print_bad_edid)) {
+	if (!drm_edid_block_valid(edid, 0, print_bad_edid,
+				  &connector->edid_header_corrupt)) {
 		connector->bad_edid_counter++;
 		DRM_ERROR("Base block of EDID firmware \"%s\" is invalid ",
 		    name);
@@ -229,7 +230,9 @@ static void *edid_load(struct drm_connector *connector, const char *name,
 		if (i != valid_extensions + 1)
 			memcpy(edid + (valid_extensions + 1) * EDID_LENGTH,
 			    edid + i * EDID_LENGTH, EDID_LENGTH);
-		if (drm_edid_block_valid(edid + i * EDID_LENGTH, i, print_bad_edid))
+		if (drm_edid_block_valid(edid + i * EDID_LENGTH, i,
+					 print_bad_edid,
+					 NULL))
 			valid_extensions++;
 	}
 
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index b120d59..ee41e10 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4005,6 +4005,7 @@ static uint8_t intel_dp_autotest_phy_pattern(struct intel_dp *intel_dp)
 
 static void intel_dp_handle_test_request(struct intel_dp *intel_dp)
 {
+	struct drm_connector *connector = &intel_dp->attached_connector->base;
 	uint8_t response = DP_TEST_NAK;
 	uint8_t rxdata = 0;
 	int status = 0;
@@ -4051,6 +4052,9 @@ update_status:
 				   &response, 1);
 	if (status <= 0)
 		DRM_DEBUG_KMS("Could not write test response to sink\n");
+
+	/* Clear EDID corruption flag now */
+	connector->edid_header_corrupt = 0;
 }
 
 static int
@@ -4136,7 +4140,7 @@ intel_dp_check_link_status(struct intel_dp *intel_dp, bool long_hpd)
 	 */
 	if (long_hpd) {
 		edid_read = drm_get_edid(connector, adapter);
-		if (!edid_read) {
+		if (!edid_read || connector->edid_header_corrupt == 1) {
 			DRM_DEBUG_DRIVER("Invalid EDID detected\n");
 		} else {
 			kfree(edid_read);
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index d4e4b82..f7a4a92 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -719,6 +719,11 @@ struct drm_connector {
 	int null_edid_counter; /* needed to workaround some HW bugs where we get all 0s */
 	unsigned bad_edid_counter;
 
+	/* Flag for raw EDID header corruption - used in Displayport
+	 * compliance testing - * Displayport Link CTS Core 1.2 rev1.1 4.2.2.6
+	 */
+	bool edid_header_corrupt;
+
 	struct dentry *debugfs_entry;
 
 	struct drm_connector_state *state;
@@ -1443,7 +1448,8 @@ extern void drm_set_preferred_mode(struct drm_connector *connector,
 				   int hpref, int vpref);
 
 extern int drm_edid_header_is_valid(const u8 *raw_edid);
-extern bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid);
+extern bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid,
+				 bool *header_corrupt);
 extern bool drm_edid_is_valid(struct edid *edid);
 
 extern struct drm_tile_group *drm_mode_create_tile_group(struct drm_device *dev,
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 06/12] drm/i915: Update intel_dp_hpd_pulse() for non-MST operation
  2015-04-15 15:38 ` [PATCH 06/10] drm/i915: Update intel_dp_hpd_pulse() for non-MST operation Todd Previte
@ 2015-04-15 17:15   ` Todd Previte
  2015-04-16  7:22   ` Todd Previte
  1 sibling, 0 replies; 31+ messages in thread
From: Todd Previte @ 2015-04-15 17:15 UTC (permalink / raw)
  To: intel-gfx

Update the hot plug function to handle the SST case. Instead of placing
the SST case within the long/short pulse block, it is now handled after
determining that MST mode is not in use. This way, the topology management
layer can handle any MST-related operations while SST operations are still
correctly handled afterwards.

This patch also corrects the problem of SST mode only being handled in the
case of a short (0.5ms - 1.0ms) HPD pulse. For compliance testing purposes
both short and long pulses are used by the different tests, thus both cases
need to be addressed for SST.

This patch replaces [PATCH 10/10] drm/i915: Fix intel_dp_hot_plug() in the
previous compliance testing patch sequence. Review feedback on that patch
indicated that updating intel_dp_hot_plug() was not the correct place for
the test handler.

For the SST case, the main stream is disabled for long HPD pulses as this
generally indicates either a connect/disconnect event or link failure. For
a number of case in compliance testing, the source is required to disable
the main link upon detection of a long HPD.

V2:
- N/A
V3:
- Place the SST mode link status check into the mst_fail case
- Remove obsolete comment regarding SST mode operation
- Removed an erroneous line of code that snuck in during rebasing
V4:
- Added a disable of the main stream (DP transport) for the long pulse case
  for SST to support compliance testing
V5:
- Reworked SST handling to support tests 4.2.2.7 and 4.2.2.8
V6:
- Reformatted a comment
V7:
- Moved a comment again that was inadvertently moved
- Updated the code to properly handle all permutations of MST/SST and
  short/long pulse.
- Adds a new 'connected' flag that prevents unnecessary operations when
  the link is disconnected.
- Added a function to encapsulate detection of the HPD pin status in
  in order to determine connected status
- Reformatted the if-statements in the function so the braces are
  consistent for those with single statements after the if-statement
V8:
- Updated to account for long_hpd flag propagation

Signed-off-by: Todd Previte <tprevite@gmail.com>
---
 drivers/gpu/drm/i915/intel_dp.c  | 55 +++++++++++++++++++++++++---------------
 drivers/gpu/drm/i915/intel_drv.h |  1 +
 2 files changed, 35 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index ee41e10..aa3b89d 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4747,6 +4747,7 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	enum intel_display_power_domain power_domain;
 	enum irqreturn ret = IRQ_NONE;
+	bool connected = false;
 
 	if (intel_dig_port->base.type != INTEL_OUTPUT_EDP)
 		intel_dig_port->base.type = INTEL_OUTPUT_DISPLAYPORT;
@@ -4770,19 +4771,15 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
 	power_domain = intel_display_port_power_domain(intel_encoder);
 	intel_display_power_get(dev_priv, power_domain);
 
+	connected = intel_dp_digital_port_connected(intel_dp);
+
 	if (long_hpd) {
 
-		if (HAS_PCH_SPLIT(dev)) {
-			if (!ibx_digital_port_connected(dev_priv, intel_dig_port))
-				goto mst_fail;
-		} else {
-			if (g4x_digital_port_connected(dev, intel_dig_port) != 1)
-				goto mst_fail;
-		}
+		if (!connected)
+			goto mst_fail;
 
-		if (!intel_dp_get_dpcd(intel_dp)) {
+		if (!intel_dp_get_dpcd(intel_dp))
 			goto mst_fail;
-		}
 
 		intel_dp_probe_oui(intel_dp);
 
@@ -4790,20 +4787,9 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
 			goto mst_fail;
 
 	} else {
-		if (intel_dp->is_mst) {
+		if (intel_dp->is_mst)
 			if (intel_dp_check_mst_status(intel_dp) == -EINVAL)
 				goto mst_fail;
-		}
-
-		if (!intel_dp->is_mst) {
-			/*
-			 * we'll check the link status via the normal hot plug path later -
-			 * but for short hpds we should check it now
-			 */
-			drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
-			intel_dp_check_link_status(intel_dp, long_hpd);
-			drm_modeset_unlock(&dev->mode_config.connection_mutex);
-		}
 	}
 
 	ret = IRQ_HANDLED;
@@ -4817,6 +4803,15 @@ mst_fail:
 		drm_dp_mst_topology_mgr_set_mst(&intel_dp->mst_mgr, intel_dp->is_mst);
 	}
 put_power:
+	/* SST mode - handle short/long pulses here */
+	if (!intel_dp->is_mst) {
+
+		drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
+		if (connected)
+			intel_dp_check_link_status(intel_dp, long_hpd);
+		drm_modeset_unlock(&dev->mode_config.connection_mutex);
+		ret = IRQ_HANDLED;
+	}
 	intel_display_power_put(dev_priv, power_domain);
 
 	return ret;
@@ -5803,3 +5798,21 @@ void intel_dp_mst_resume(struct drm_device *dev)
 		}
 	}
 }
+
+bool intel_dp_digital_port_connected(struct intel_dp *intel_dp)
+{
+	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
+	struct drm_device *dev = intel_dig_port->base.base.dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	bool connected = true;
+
+	if (HAS_PCH_SPLIT(dev)) {
+		if (!ibx_digital_port_connected(dev_priv, intel_dig_port))
+			connected = false;
+	} else {
+		if (g4x_digital_port_connected(dev, intel_dig_port) != 1)
+			connected = false;
+	}
+
+	return connected;
+}
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index a4675fa..accf08b 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1173,6 +1173,7 @@ void intel_edp_drrs_disable(struct intel_dp *intel_dp);
 void intel_edp_drrs_invalidate(struct drm_device *dev,
 		unsigned frontbuffer_bits);
 void intel_edp_drrs_flush(struct drm_device *dev, unsigned frontbuffer_bits);
+bool intel_dp_digital_port_connected(struct intel_dp *intel_dp);
 
 /* intel_dp_mst.c */
 int intel_dp_mst_encoder_init(struct intel_digital_port *intel_dig_port, int conn_id);
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 07/12] drm/i915: Support EDID compliance tests with the intel_dp_autotest_edid() function
  2015-04-15 15:38 ` [PATCH 07/10] drm/i915: Support EDID compliance tests with the intel_dp_autotest_edid() function Todd Previte
@ 2015-04-15 17:16   ` Todd Previte
  0 siblings, 0 replies; 31+ messages in thread
From: Todd Previte @ 2015-04-15 17:16 UTC (permalink / raw)
  To: intel-gfx

Updates the EDID compliance test function to perform the EDID read as
required by the tests. This read needs to take place in the kernel for
reasons of speed and efficiency. The results of the EDID read operations
are handed off to userspace so that the userspace app can set the display
mode appropriately for the test response.

The compliance_test_active flag now appears at the end of the individual
test handling functions. This is so that the kernel-side operations can
be completed without the risk of interruption from the userspace app
that is polling on that flag.

V2:
- Addressed mailing list feedback
- Removed excess debug messages
- Removed extraneous comments
- Fixed formatting issues (line length > 80)
- Updated the debug message in compute_edid_checksum to output hex values
  instead of decimal
V3:
- Addressed more list feedback
- Added the test_active flag to the autotest function
- Removed test_active flag from handler
- Added failsafe check on the compliance test active flag
  at the end of the test handler
- Fixed checkpatch.pl issues
V4:
- Removed the checksum computation function and its use as it has been
  rendered superfluous by changes to the core DRM EDID functions
- Updated to use the raw header corruption detection mechanism
- Moved the declaration of the test_data variable here
V5:
- Update test active flag variable name to match the change in the
  first patch of the series.
- Relocated the test active flag declaration and initialization
  to this patch
V6:
- Updated to use the new flag for raw EDID header corruption
- Removed the extra EDID read from the autotest function
- Added the edid_checksum variable to struct intel_dp so that the
  autotest function can write it to the sink device
- Moved the update to the hpd_pulse function to another patch
- Removed extraneous constants
V7:
- Fixed erroneous placement of the checksum assignment. In some cases
  such as when the EDID read fails and is NULL, this causes a NULL ptr
  dereference in the kernel. Bad news. Fixed now.
V8:
- Updated to support the kfree() on the EDID data added previously
V9:
- Updated for the long_hpd flag propagation

Signed-off-by: Todd Previte <tprevite@gmail.com>
---
 drivers/gpu/drm/i915/intel_dp.c  | 47 +++++++++++++++++++++++++++++++++++++++-
 drivers/gpu/drm/i915/intel_drv.h |  4 ++++
 2 files changed, 50 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index aa3b89d..714d48b 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -41,6 +41,12 @@
 
 #define DP_LINK_CHECK_TIMEOUT	(10 * 1000)
 
+/* Compliance test status bits  */
+#define INTEL_DP_RESOLUTION_SHIFT_MASK	4
+#define INTEL_DP_RESOLUTION_PREFERRED	(1 << INTEL_DP_RESOLUTION_SHIFT_MASK)
+#define INTEL_DP_RESOLUTION_STANDARD	(2 << INTEL_DP_RESOLUTION_SHIFT_MASK)
+#define INTEL_DP_RESOLUTION_FAILSAFE	(3 << INTEL_DP_RESOLUTION_SHIFT_MASK)
+
 struct dp_link_dpll {
 	int link_bw;
 	struct dpll dpll;
@@ -3994,6 +4000,35 @@ 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;
+	uint32_t ret = 0;
+
+	if (intel_dp->compliance_edid_invalid ||
+	    intel_dp->aux.i2c_defer_count > 6) {
+		/* Check for NACKs/DEFERs, use failsafe if detected
+		 *  (DP CTS 1.2 Core Rev 1.1, 4.2.2.4, 4.2.2.5)
+		 */
+		if (intel_dp->aux.i2c_nack_count > 0 ||
+			intel_dp->aux.i2c_defer_count > 0)
+			DRM_DEBUG_KMS("EDID read had %d NACKs, %d DEFERs\n",
+				      intel_dp->aux.i2c_nack_count,
+				      intel_dp->aux.i2c_defer_count);
+		intel_dp->compliance_test_data = INTEL_DP_RESOLUTION_FAILSAFE;
+	} else {
+		ret = drm_dp_dpcd_write(&intel_dp->aux,
+					DP_TEST_EDID_CHECKSUM,
+					&intel_dp->compliance_edid_checksum, 1);
+		if (ret <= 0)
+			DRM_DEBUG_DRIVER("Failed to write EDID checksum\n");
+		else
+			DRM_DEBUG_DRIVER("EDID checksum written to sink\n");
+
+		test_result = DP_TEST_ACK | DP_TEST_EDID_CHECKSUM_WRITE;
+		intel_dp->compliance_test_data = INTEL_DP_RESOLUTION_STANDARD;
+	}
+
+	/* Set test active flag here so userspace doesn't interrupt things */
+	intel_dp->compliance_test_active = 1;
+
 	return test_result;
 }
 
@@ -4010,7 +4045,10 @@ static void intel_dp_handle_test_request(struct intel_dp *intel_dp)
 	uint8_t rxdata = 0;
 	int status = 0;
 
+	intel_dp->compliance_test_active = 0;
 	intel_dp->compliance_test_type = 0;
+	intel_dp->compliance_test_data = 0;
+
 	intel_dp->aux.i2c_nack_count = 0;
 	intel_dp->aux.i2c_defer_count = 0;
 
@@ -4137,12 +4175,19 @@ intel_dp_check_link_status(struct intel_dp *intel_dp, bool long_hpd)
 
 	/* Displayport Link CTS Core 1.2 rev1.1 EDID testing
 	 * 4.2.2.1 - EDID read required for all HPD events
-	 */
+	 * Also used for 4.2.2.3, 4.2.2.4 4.2.2.5 and 4.2.2.6
+	 *   - compliance_edid_invalid flag used for 4.2.2.6
+	 *   - compliance_edid_checksum used for 4.2.2.3
+	*/
+	intel_dp->compliance_edid_invalid = 0;
 	if (long_hpd) {
 		edid_read = drm_get_edid(connector, adapter);
 		if (!edid_read || connector->edid_header_corrupt == 1) {
 			DRM_DEBUG_DRIVER("Invalid EDID detected\n");
+			intel_dp->compliance_edid_invalid = 1;
+			intel_dp->compliance_edid_checksum = 0;
 		} else {
+			intel_dp->compliance_edid_checksum = edid_read->checksum;
 			kfree(edid_read);
 		}
 	}
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index accf08b..73ced46 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -742,6 +742,10 @@ struct intel_dp {
 
 	/* Displayport compliance testing */
 	unsigned long compliance_test_type;
+	unsigned long compliance_test_data;
+	bool compliance_test_active;
+	bool compliance_edid_invalid;
+	uint8_t compliance_edid_checksum;
 };
 
 struct intel_digital_port {
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 02/12] drm/i915: Update intel_dp_check_link_status() for Displayport compliance testing
  2015-04-15 15:38 ` [PATCH 02/10] drm/i915: Update intel_dp_check_link_status() " Todd Previte
@ 2015-04-15 19:28   ` Todd Previte
  2015-04-15 19:45     ` Paulo Zanoni
  0 siblings, 1 reply; 31+ messages in thread
From: Todd Previte @ 2015-04-15 19:28 UTC (permalink / raw)
  To: intel-gfx

Move the DPCD read to the top and check for an interrupt from the sink to catch
Displayport automated testing requests necessary to support Displayport
compliance testing. The checks for active connectors and link status are moved
below the check for the interrupt.

The main reason for doing this is to make sure that a test request isn't missed.
Checking for the status of the encoder/crtc isn't necessary for some test cases
(AUX channel tests are one example) and without moving the check for the
interrupt, these tests may not execute if one of those checks fails.
Additionally, if reading the DPCD fails, regardless of whether or not testing is
happening, there's no way to train the link since configurations and status
can't be read, nor can link training parameters be written.

V1:
- This is the second part of the single-patch split previously
  mentioned.
V2:
- Remerge the two split patches into one and update the commit message
  accordingly.
- Replace the SW connected status check with a HW HPD pin status check
- Adds a new function that examines the status of the HPD pin to
  determine if a sink device is connected
V3:
- CLean up of the patch merge from previous split
- Updated the commit message

Signed-off-by: Todd Previte <tprevite@gmail.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 33 +++++++++++++++------------------
 1 file changed, 15 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 263eff3..9c38986 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4119,24 +4119,8 @@ intel_dp_check_link_status(struct intel_dp *intel_dp)
 
 	WARN_ON(!drm_modeset_is_locked(&dev->mode_config.connection_mutex));
 
-	if (!intel_encoder->connectors_active)
-		return;
-
-	if (WARN_ON(!intel_encoder->base.crtc))
-		return;
-
-	if (!to_intel_crtc(intel_encoder->base.crtc)->active)
-		return;
-
-	/* Try to read receiver status if the link appears to be up */
-	if (!intel_dp_get_link_status(intel_dp, link_status)) {
-		return;
-	}
-
-	/* Now read the DPCD to see if it's actually running */
-	if (!intel_dp_get_dpcd(intel_dp)) {
+	if (!intel_dp_get_dpcd(intel_dp))
 		return;
-	}
 
 	/* Try to read the source of the interrupt */
 	if (intel_dp->dpcd[DP_DPCD_REV] >= 0x11 &&
@@ -4145,13 +4129,26 @@ intel_dp_check_link_status(struct intel_dp *intel_dp)
 		drm_dp_dpcd_writeb(&intel_dp->aux,
 				   DP_DEVICE_SERVICE_IRQ_VECTOR,
 				   sink_irq_vector);
-
 		if (sink_irq_vector & DP_AUTOMATED_TEST_REQUEST)
 			intel_dp_handle_test_request(intel_dp);
 		if (sink_irq_vector & (DP_CP_IRQ | DP_SINK_SPECIFIC_IRQ))
 			DRM_DEBUG_DRIVER("CP or sink specific irq unhandled\n");
 	}
 
+	if (!intel_encoder->connectors_active)
+		return;
+
+	if (WARN_ON(!intel_encoder->base.crtc))
+		return;
+
+	if (!to_intel_crtc(intel_encoder->base.crtc)->active)
+		return;
+
+	/* Try to read receiver status if the link appears to be up */
+	if (!intel_dp_get_link_status(intel_dp, link_status)) {
+		return;
+	}
+
 	if (!drm_dp_channel_eq_ok(link_status, intel_dp->lane_count)) {
 		DRM_DEBUG_KMS("%s: channel EQ not ok, retraining\n",
 			      intel_encoder->base.name);
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 03/12] drm/i915: Add EDID read in intel_dp_check_link_status() for Link CTS 4.2.2.1
  2015-04-15 15:38 ` [PATCH 03/10] drm/i915: Add EDID read in intel_dp_check_link_status() for Link CTS 4.2.2.1 Todd Previte
  2015-04-15 17:15   ` [PATCH 03/12] " Todd Previte
@ 2015-04-15 19:29   ` Todd Previte
  1 sibling, 0 replies; 31+ messages in thread
From: Todd Previte @ 2015-04-15 19:29 UTC (permalink / raw)
  To: intel-gfx

Adds in an EDID read after the DPCD read to accommodate test 4.2.2.1 in the
Displayport Link CTS Core 1.2 rev1.1. This test requires an EDID read for
all HPD plug events. To reduce the amount of code, this EDID read is also
used for Link CTS tests 4.2.2.3, 4.2.2.4, 4.2.2.5 and 4.2.2.6. Actual
support for these tests is implemented in later patches in this series.

V2:
- Fixed compilation error introduced during rework
V3:
- Plugged a memory leak where the EDID data wasn't being freed
  after allocation in this function
V4:
- Fixed whitespace problems
- Cleaned up formatting
V5:
- Added propagation of the long_hpd flag from the hot_pulse function
V6:
- Versioning, accommodating changes from previous patch

Signed-off-by: Todd Previte <tprevite@gmail.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 9c38986..a875b44 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4110,10 +4110,13 @@ go_again:
  *  4. Check link status on receipt of hot-plug interrupt
  */
 static void
-intel_dp_check_link_status(struct intel_dp *intel_dp)
+intel_dp_check_link_status(struct intel_dp *intel_dp, bool long_hpd)
 {
 	struct drm_device *dev = intel_dp_to_dev(intel_dp);
 	struct intel_encoder *intel_encoder = &dp_to_dig_port(intel_dp)->base;
+	struct drm_connector *connector = &intel_dp->attached_connector->base;
+	struct i2c_adapter *adapter = &intel_dp->aux.ddc;
+	struct edid *edid_read = NULL;
 	u8 sink_irq_vector;
 	u8 link_status[DP_LINK_STATUS_SIZE];
 
@@ -4122,6 +4125,18 @@ intel_dp_check_link_status(struct intel_dp *intel_dp)
 	if (!intel_dp_get_dpcd(intel_dp))
 		return;
 
+	/* Displayport Link CTS Core 1.2 rev1.1 EDID testing
+	 * 4.2.2.1 - EDID read required for all HPD events
+	 */
+	if (long_hpd) {
+		edid_read = drm_get_edid(connector, adapter);
+		if (!edid_read) {
+			DRM_DEBUG_DRIVER("Invalid EDID detected\n");
+		} else {
+			kfree(edid_read);
+		}
+	}
+
 	/* Try to read the source of the interrupt */
 	if (intel_dp->dpcd[DP_DPCD_REV] >= 0x11 &&
 	    intel_dp_get_sink_irq(intel_dp, &sink_irq_vector)) {
@@ -4776,7 +4791,7 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
 			 * but for short hpds we should check it now
 			 */
 			drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
-			intel_dp_check_link_status(intel_dp);
+			intel_dp_check_link_status(intel_dp, long_hpd);
 			drm_modeset_unlock(&dev->mode_config.connection_mutex);
 		}
 	}
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* Re: [PATCH 02/12] drm/i915: Update intel_dp_check_link_status() for Displayport compliance testing
  2015-04-15 19:28   ` [PATCH 02/12] " Todd Previte
@ 2015-04-15 19:45     ` Paulo Zanoni
  0 siblings, 0 replies; 31+ messages in thread
From: Paulo Zanoni @ 2015-04-15 19:45 UTC (permalink / raw)
  To: Todd Previte; +Cc: Intel Graphics Development

2015-04-15 16:28 GMT-03:00 Todd Previte <tprevite@gmail.com>:
> Move the DPCD read to the top and check for an interrupt from the sink to catch
> Displayport automated testing requests necessary to support Displayport
> compliance testing. The checks for active connectors and link status are moved
> below the check for the interrupt.
>
> The main reason for doing this is to make sure that a test request isn't missed.
> Checking for the status of the encoder/crtc isn't necessary for some test cases
> (AUX channel tests are one example) and without moving the check for the
> interrupt, these tests may not execute if one of those checks fails.
> Additionally, if reading the DPCD fails, regardless of whether or not testing is
> happening, there's no way to train the link since configurations and status
> can't be read, nor can link training parameters be written.
>
> V1:
> - This is the second part of the single-patch split previously
>   mentioned.
> V2:
> - Remerge the two split patches into one and update the commit message
>   accordingly.
> - Replace the SW connected status check with a HW HPD pin status check
> - Adds a new function that examines the status of the HPD pin to
>   determine if a sink device is connected
> V3:
> - CLean up of the patch merge from previous split
> - Updated the commit message
>
> Signed-off-by: Todd Previte <tprevite@gmail.com>

Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

> ---
>  drivers/gpu/drm/i915/intel_dp.c | 33 +++++++++++++++------------------
>  1 file changed, 15 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 263eff3..9c38986 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -4119,24 +4119,8 @@ intel_dp_check_link_status(struct intel_dp *intel_dp)
>
>         WARN_ON(!drm_modeset_is_locked(&dev->mode_config.connection_mutex));
>
> -       if (!intel_encoder->connectors_active)
> -               return;
> -
> -       if (WARN_ON(!intel_encoder->base.crtc))
> -               return;
> -
> -       if (!to_intel_crtc(intel_encoder->base.crtc)->active)
> -               return;
> -
> -       /* Try to read receiver status if the link appears to be up */
> -       if (!intel_dp_get_link_status(intel_dp, link_status)) {
> -               return;
> -       }
> -
> -       /* Now read the DPCD to see if it's actually running */
> -       if (!intel_dp_get_dpcd(intel_dp)) {
> +       if (!intel_dp_get_dpcd(intel_dp))
>                 return;
> -       }
>
>         /* Try to read the source of the interrupt */
>         if (intel_dp->dpcd[DP_DPCD_REV] >= 0x11 &&
> @@ -4145,13 +4129,26 @@ intel_dp_check_link_status(struct intel_dp *intel_dp)
>                 drm_dp_dpcd_writeb(&intel_dp->aux,
>                                    DP_DEVICE_SERVICE_IRQ_VECTOR,
>                                    sink_irq_vector);
> -
>                 if (sink_irq_vector & DP_AUTOMATED_TEST_REQUEST)
>                         intel_dp_handle_test_request(intel_dp);
>                 if (sink_irq_vector & (DP_CP_IRQ | DP_SINK_SPECIFIC_IRQ))
>                         DRM_DEBUG_DRIVER("CP or sink specific irq unhandled\n");
>         }
>
> +       if (!intel_encoder->connectors_active)
> +               return;
> +
> +       if (WARN_ON(!intel_encoder->base.crtc))
> +               return;
> +
> +       if (!to_intel_crtc(intel_encoder->base.crtc)->active)
> +               return;
> +
> +       /* Try to read receiver status if the link appears to be up */
> +       if (!intel_dp_get_link_status(intel_dp, link_status)) {
> +               return;
> +       }
> +
>         if (!drm_dp_channel_eq_ok(link_status, intel_dp->lane_count)) {
>                 DRM_DEBUG_KMS("%s: channel EQ not ok, retraining\n",
>                               intel_encoder->base.name);
> --
> 1.9.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Paulo Zanoni
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [Intel-gfx] [PATCH 05/12] drm: Add supporting structure for Displayport Link CTS test 4.2.2.6
  2015-04-15 17:15   ` [PATCH 05/12] " Todd Previte
@ 2015-04-15 20:25     ` Paulo Zanoni
  2015-04-15 21:59       ` Todd Previte
  0 siblings, 1 reply; 31+ messages in thread
From: Paulo Zanoni @ 2015-04-15 20:25 UTC (permalink / raw)
  To: Todd Previte; +Cc: Intel Graphics Development, DRI Development

2015-04-15 14:15 GMT-03:00 Todd Previte <tprevite@gmail.com>:
> Displayport compliance test 4.2.2.6 requires that a source device be capable of
> detecting a corrupt EDID. The test specification states that the sink device
> sets up the EDID with an invalid checksum. To do this, the sink sets up an
> invalid EDID header, expecting the source device to generate the checksum and
> compare it to the value stored in the last byte of the block data.
>
> Unfortunately, the DRM EDID reading and parsing functions are actually too good
> in this case; the header is fixed before the checksum is computed and thus the
> code never sees the invalid checksum. This results in a failure to pass the
> compliance test.
>
> To correct this issue, a checksum is generated when the EDID header is detected
> as corrupted. If the checksum is invalid, it sets the header_corrupt flag and
> logs the errors. In the case of a more seriously damaged header (fixup score
> less than the threshold) the code does not generate the checksum but does set
> the header_corrupt flag.
>
> V2:
> - Removed the static bool global
> - Added a bool to the drm_connector struct to reaplce the static one for
>   holding the status of raw edid header corruption detection
> - Modified the function signature of the is_valid function to take an
>   additional parameter to store the corruption detected value
> - Fixed the other callers of the above is_valid function
> V3:
> - Updated the commit message to be more clear about what and why this
>   patch does what it does.
> - Added comment in code to clarify the operations there
> - Removed compliance variable and check_link_status update; those
>   have been moved to a later patch
> - Removed variable assignment from the bottom of the test handler
> V4:
> - Removed i915 tag from subject line as the patch is not i915-specific
> V5:
> - Moved code causing a compilation error to this patch where the variable
>   is actually declared
> - Maintained blank lines / spacing so as to not contaminate the patch
> V6:
> - Removed extra debug messages
> - Added documentation to for the added parameter on drm_edid_block_valid
> - Fixed more whitespace issues in check_link_status
> - Added a clear of the header_corrupt flag to the end of the test handler
>   in intel_dp.c
> - Changed the usage of the new function prototype in several places to use
>   NULL where it is not needed by compliance testing
> V7:
> - Updated to account for long_pulse flag propagation
>
> Signed-off-by: Todd Previte <tprevite@gmail.com>
> Cc: dri-devel@lists.freedesktop.org
> ---
>  drivers/gpu/drm/drm_edid.c      | 30 ++++++++++++++++++++++++++----
>  drivers/gpu/drm/drm_edid_load.c |  7 +++++--
>  drivers/gpu/drm/i915/intel_dp.c |  6 +++++-
>  include/drm/drm_crtc.h          |  8 +++++++-
>  4 files changed, 43 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 53bc7a6..1ed18f5 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -1041,13 +1041,15 @@ static bool drm_edid_is_zero(const u8 *in_edid, int length)
>   * @raw_edid: pointer to raw EDID block
>   * @block: type of block to validate (0 for base, extension otherwise)
>   * @print_bad_edid: if true, dump bad EDID blocks to the console
> + * @header_corrupt: if true, the header or checksum is invalid
>   *
>   * Validate a base or extension EDID block and optionally dump bad blocks to
>   * the console.
>   *
>   * Return: True if the block is valid, false otherwise.
>   */
> -bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid)
> +bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid,
> +                         bool *header_corrupt)
>  {
>         u8 csum;
>         struct edid *edid = (struct edid *)raw_edid;
> @@ -1062,9 +1064,25 @@ bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid)
>                 int score = drm_edid_header_is_valid(raw_edid);
>                 if (score == 8) ;
>                 else if (score >= edid_fixup) {
> +                       /* Displayport Link CTS Core 1.2 rev1.1 test 4.2.2.6
> +                        * In order to properly generate the invalid checksum
> +                        * required for this test, it must be generated using
> +                        * the raw EDID data. Otherwise, the fix-up code here
> +                        * will correct the problem, the checksum is correct
> +                        * and the test fails
> +                        */
> +                       csum = drm_edid_block_checksum(raw_edid);
> +                       if (csum) {
> +                               if (header_corrupt)
> +                                       *header_corrupt = 1;
> +                       }
>                         DRM_DEBUG("Fixing EDID header, your hardware may be failing\n");
>                         memcpy(raw_edid, edid_header, sizeof(edid_header));
>                 } else {
> +                       if (header_corrupt) {
> +                               DRM_DEBUG_DRIVER("Invalid EDID header\n");

Still using DRM_DEBUG_DRIVER. See include/drm/drmP.h for a description
of each macro. BTW, since we're just going to dump the whole EDID
anyway, the message is also not very informative.


> +                               *header_corrupt = 1;
> +                       }
>                         goto bad;
>                 }
>         }
> @@ -1129,7 +1147,7 @@ bool drm_edid_is_valid(struct edid *edid)
>                 return false;
>
>         for (i = 0; i <= edid->extensions; i++)
> -               if (!drm_edid_block_valid(raw + i * EDID_LENGTH, i, true))
> +               if (!drm_edid_block_valid(raw + i * EDID_LENGTH, i, true, NULL))
>                         return false;
>
>         return true;
> @@ -1232,7 +1250,8 @@ struct edid *drm_do_get_edid(struct drm_connector *connector,
>         for (i = 0; i < 4; i++) {
>                 if (get_edid_block(data, block, 0, EDID_LENGTH))
>                         goto out;
> -               if (drm_edid_block_valid(block, 0, print_bad_edid))
> +               if (drm_edid_block_valid(block, 0, print_bad_edid,
> +                                        &connector->edid_header_corrupt))
>                         break;
>                 if (i == 0 && drm_edid_is_zero(block, EDID_LENGTH)) {
>                         connector->null_edid_counter++;
> @@ -1257,7 +1276,10 @@ struct edid *drm_do_get_edid(struct drm_connector *connector,
>                                   block + (valid_extensions + 1) * EDID_LENGTH,
>                                   j, EDID_LENGTH))
>                                 goto out;
> -                       if (drm_edid_block_valid(block + (valid_extensions + 1) * EDID_LENGTH, j, print_bad_edid)) {
> +                       if (drm_edid_block_valid(block + (valid_extensions + 1)
> +                                                * EDID_LENGTH, j,
> +                                                print_bad_edid,
> +                                                NULL)) {
>                                 valid_extensions++;
>                                 break;
>                         }
> diff --git a/drivers/gpu/drm/drm_edid_load.c b/drivers/gpu/drm/drm_edid_load.c
> index 4c0aa97..48b48e8 100644
> --- a/drivers/gpu/drm/drm_edid_load.c
> +++ b/drivers/gpu/drm/drm_edid_load.c
> @@ -216,7 +216,8 @@ static void *edid_load(struct drm_connector *connector, const char *name,
>                 goto out;
>         }
>
> -       if (!drm_edid_block_valid(edid, 0, print_bad_edid)) {
> +       if (!drm_edid_block_valid(edid, 0, print_bad_edid,
> +                                 &connector->edid_header_corrupt)) {
>                 connector->bad_edid_counter++;
>                 DRM_ERROR("Base block of EDID firmware \"%s\" is invalid ",
>                     name);
> @@ -229,7 +230,9 @@ static void *edid_load(struct drm_connector *connector, const char *name,
>                 if (i != valid_extensions + 1)
>                         memcpy(edid + (valid_extensions + 1) * EDID_LENGTH,
>                             edid + i * EDID_LENGTH, EDID_LENGTH);
> -               if (drm_edid_block_valid(edid + i * EDID_LENGTH, i, print_bad_edid))
> +               if (drm_edid_block_valid(edid + i * EDID_LENGTH, i,
> +                                        print_bad_edid,
> +                                        NULL))
>                         valid_extensions++;
>         }
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index b120d59..ee41e10 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -4005,6 +4005,7 @@ static uint8_t intel_dp_autotest_phy_pattern(struct intel_dp *intel_dp)
>
>  static void intel_dp_handle_test_request(struct intel_dp *intel_dp)
>  {
> +       struct drm_connector *connector = &intel_dp->attached_connector->base;
>         uint8_t response = DP_TEST_NAK;
>         uint8_t rxdata = 0;
>         int status = 0;
> @@ -4051,6 +4052,9 @@ update_status:
>                                    &response, 1);
>         if (status <= 0)
>                 DRM_DEBUG_KMS("Could not write test response to sink\n");
> +
> +       /* Clear EDID corruption flag now */
> +       connector->edid_header_corrupt = 0;

I don't think it makes sense to clear this inside i915.ko. We're
adding the feature to drm.ko, so It makes sense to clear it there.

Isn't it possible to just clear it inside drm_edid_block_valid()?
Something like:

if (score == 8) {
    if (header_corrupt)
        *header_corrupt = 0;
} else if (score >= edid_fixup) {
        ... etc ...
    }
}

If not, why?


>  }
>
>  static int
> @@ -4136,7 +4140,7 @@ intel_dp_check_link_status(struct intel_dp *intel_dp, bool long_hpd)
>          */
>         if (long_hpd) {
>                 edid_read = drm_get_edid(connector, adapter);
> -               if (!edid_read) {
> +               if (!edid_read || connector->edid_header_corrupt == 1) {
>                         DRM_DEBUG_DRIVER("Invalid EDID detected\n");
>                 } else {
>                         kfree(edid_read);
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index d4e4b82..f7a4a92 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -719,6 +719,11 @@ struct drm_connector {
>         int null_edid_counter; /* needed to workaround some HW bugs where we get all 0s */
>         unsigned bad_edid_counter;
>
> +       /* Flag for raw EDID header corruption - used in Displayport
> +        * compliance testing - * Displayport Link CTS Core 1.2 rev1.1 4.2.2.6
> +        */
> +       bool edid_header_corrupt;
> +
>         struct dentry *debugfs_entry;
>
>         struct drm_connector_state *state;
> @@ -1443,7 +1448,8 @@ extern void drm_set_preferred_mode(struct drm_connector *connector,
>                                    int hpref, int vpref);
>
>  extern int drm_edid_header_is_valid(const u8 *raw_edid);
> -extern bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid);
> +extern bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid,
> +                                bool *header_corrupt);
>  extern bool drm_edid_is_valid(struct edid *edid);
>
>  extern struct drm_tile_group *drm_mode_create_tile_group(struct drm_device *dev,
> --
> 1.9.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Paulo Zanoni
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 05/12] drm: Add supporting structure for Displayport Link CTS test 4.2.2.6
  2015-04-15 20:25     ` [Intel-gfx] " Paulo Zanoni
@ 2015-04-15 21:59       ` Todd Previte
  0 siblings, 0 replies; 31+ messages in thread
From: Todd Previte @ 2015-04-15 21:59 UTC (permalink / raw)
  To: Paulo Zanoni; +Cc: Intel Graphics Development, DRI Development



On 4/15/2015 1:25 PM, Paulo Zanoni wrote:
> 2015-04-15 14:15 GMT-03:00 Todd Previte <tprevite@gmail.com>:
>> Displayport compliance test 4.2.2.6 requires that a source device be capable of
>> detecting a corrupt EDID. The test specification states that the sink device
>> sets up the EDID with an invalid checksum. To do this, the sink sets up an
>> invalid EDID header, expecting the source device to generate the checksum and
>> compare it to the value stored in the last byte of the block data.
>>
>> Unfortunately, the DRM EDID reading and parsing functions are actually too good
>> in this case; the header is fixed before the checksum is computed and thus the
>> code never sees the invalid checksum. This results in a failure to pass the
>> compliance test.
>>
>> To correct this issue, a checksum is generated when the EDID header is detected
>> as corrupted. If the checksum is invalid, it sets the header_corrupt flag and
>> logs the errors. In the case of a more seriously damaged header (fixup score
>> less than the threshold) the code does not generate the checksum but does set
>> the header_corrupt flag.
>>
>> V2:
>> - Removed the static bool global
>> - Added a bool to the drm_connector struct to reaplce the static one for
>>    holding the status of raw edid header corruption detection
>> - Modified the function signature of the is_valid function to take an
>>    additional parameter to store the corruption detected value
>> - Fixed the other callers of the above is_valid function
>> V3:
>> - Updated the commit message to be more clear about what and why this
>>    patch does what it does.
>> - Added comment in code to clarify the operations there
>> - Removed compliance variable and check_link_status update; those
>>    have been moved to a later patch
>> - Removed variable assignment from the bottom of the test handler
>> V4:
>> - Removed i915 tag from subject line as the patch is not i915-specific
>> V5:
>> - Moved code causing a compilation error to this patch where the variable
>>    is actually declared
>> - Maintained blank lines / spacing so as to not contaminate the patch
>> V6:
>> - Removed extra debug messages
>> - Added documentation to for the added parameter on drm_edid_block_valid
>> - Fixed more whitespace issues in check_link_status
>> - Added a clear of the header_corrupt flag to the end of the test handler
>>    in intel_dp.c
>> - Changed the usage of the new function prototype in several places to use
>>    NULL where it is not needed by compliance testing
>> V7:
>> - Updated to account for long_pulse flag propagation
>>
>> Signed-off-by: Todd Previte <tprevite@gmail.com>
>> Cc: dri-devel@lists.freedesktop.org
>> ---
>>   drivers/gpu/drm/drm_edid.c      | 30 ++++++++++++++++++++++++++----
>>   drivers/gpu/drm/drm_edid_load.c |  7 +++++--
>>   drivers/gpu/drm/i915/intel_dp.c |  6 +++++-
>>   include/drm/drm_crtc.h          |  8 +++++++-
>>   4 files changed, 43 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
>> index 53bc7a6..1ed18f5 100644
>> --- a/drivers/gpu/drm/drm_edid.c
>> +++ b/drivers/gpu/drm/drm_edid.c
>> @@ -1041,13 +1041,15 @@ static bool drm_edid_is_zero(const u8 *in_edid, int length)
>>    * @raw_edid: pointer to raw EDID block
>>    * @block: type of block to validate (0 for base, extension otherwise)
>>    * @print_bad_edid: if true, dump bad EDID blocks to the console
>> + * @header_corrupt: if true, the header or checksum is invalid
>>    *
>>    * Validate a base or extension EDID block and optionally dump bad blocks to
>>    * the console.
>>    *
>>    * Return: True if the block is valid, false otherwise.
>>    */
>> -bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid)
>> +bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid,
>> +                         bool *header_corrupt)
>>   {
>>          u8 csum;
>>          struct edid *edid = (struct edid *)raw_edid;
>> @@ -1062,9 +1064,25 @@ bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid)
>>                  int score = drm_edid_header_is_valid(raw_edid);
>>                  if (score == 8) ;
>>                  else if (score >= edid_fixup) {
>> +                       /* Displayport Link CTS Core 1.2 rev1.1 test 4.2.2.6
>> +                        * In order to properly generate the invalid checksum
>> +                        * required for this test, it must be generated using
>> +                        * the raw EDID data. Otherwise, the fix-up code here
>> +                        * will correct the problem, the checksum is correct
>> +                        * and the test fails
>> +                        */
>> +                       csum = drm_edid_block_checksum(raw_edid);
>> +                       if (csum) {
>> +                               if (header_corrupt)
>> +                                       *header_corrupt = 1;
>> +                       }
>>                          DRM_DEBUG("Fixing EDID header, your hardware may be failing\n");
>>                          memcpy(raw_edid, edid_header, sizeof(edid_header));
>>                  } else {
>> +                       if (header_corrupt) {
>> +                               DRM_DEBUG_DRIVER("Invalid EDID header\n");
> Still using DRM_DEBUG_DRIVER. See include/drm/drmP.h for a description
> of each macro. BTW, since we're just going to dump the whole EDID
> anyway, the message is also not very informative.
Fair enough. Removed for the next patch.
>
>> +                               *header_corrupt = 1;
>> +                       }
>>                          goto bad;
>>                  }
>>          }
>> @@ -1129,7 +1147,7 @@ bool drm_edid_is_valid(struct edid *edid)
>>                  return false;
>>
>>          for (i = 0; i <= edid->extensions; i++)
>> -               if (!drm_edid_block_valid(raw + i * EDID_LENGTH, i, true))
>> +               if (!drm_edid_block_valid(raw + i * EDID_LENGTH, i, true, NULL))
>>                          return false;
>>
>>          return true;
>> @@ -1232,7 +1250,8 @@ struct edid *drm_do_get_edid(struct drm_connector *connector,
>>          for (i = 0; i < 4; i++) {
>>                  if (get_edid_block(data, block, 0, EDID_LENGTH))
>>                          goto out;
>> -               if (drm_edid_block_valid(block, 0, print_bad_edid))
>> +               if (drm_edid_block_valid(block, 0, print_bad_edid,
>> +                                        &connector->edid_header_corrupt))
>>                          break;
>>                  if (i == 0 && drm_edid_is_zero(block, EDID_LENGTH)) {
>>                          connector->null_edid_counter++;
>> @@ -1257,7 +1276,10 @@ struct edid *drm_do_get_edid(struct drm_connector *connector,
>>                                    block + (valid_extensions + 1) * EDID_LENGTH,
>>                                    j, EDID_LENGTH))
>>                                  goto out;
>> -                       if (drm_edid_block_valid(block + (valid_extensions + 1) * EDID_LENGTH, j, print_bad_edid)) {
>> +                       if (drm_edid_block_valid(block + (valid_extensions + 1)
>> +                                                * EDID_LENGTH, j,
>> +                                                print_bad_edid,
>> +                                                NULL)) {
>>                                  valid_extensions++;
>>                                  break;
>>                          }
>> diff --git a/drivers/gpu/drm/drm_edid_load.c b/drivers/gpu/drm/drm_edid_load.c
>> index 4c0aa97..48b48e8 100644
>> --- a/drivers/gpu/drm/drm_edid_load.c
>> +++ b/drivers/gpu/drm/drm_edid_load.c
>> @@ -216,7 +216,8 @@ static void *edid_load(struct drm_connector *connector, const char *name,
>>                  goto out;
>>          }
>>
>> -       if (!drm_edid_block_valid(edid, 0, print_bad_edid)) {
>> +       if (!drm_edid_block_valid(edid, 0, print_bad_edid,
>> +                                 &connector->edid_header_corrupt)) {
>>                  connector->bad_edid_counter++;
>>                  DRM_ERROR("Base block of EDID firmware \"%s\" is invalid ",
>>                      name);
>> @@ -229,7 +230,9 @@ static void *edid_load(struct drm_connector *connector, const char *name,
>>                  if (i != valid_extensions + 1)
>>                          memcpy(edid + (valid_extensions + 1) * EDID_LENGTH,
>>                              edid + i * EDID_LENGTH, EDID_LENGTH);
>> -               if (drm_edid_block_valid(edid + i * EDID_LENGTH, i, print_bad_edid))
>> +               if (drm_edid_block_valid(edid + i * EDID_LENGTH, i,
>> +                                        print_bad_edid,
>> +                                        NULL))
>>                          valid_extensions++;
>>          }
>>
>> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>> index b120d59..ee41e10 100644
>> --- a/drivers/gpu/drm/i915/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>> @@ -4005,6 +4005,7 @@ static uint8_t intel_dp_autotest_phy_pattern(struct intel_dp *intel_dp)
>>
>>   static void intel_dp_handle_test_request(struct intel_dp *intel_dp)
>>   {
>> +       struct drm_connector *connector = &intel_dp->attached_connector->base;
>>          uint8_t response = DP_TEST_NAK;
>>          uint8_t rxdata = 0;
>>          int status = 0;
>> @@ -4051,6 +4052,9 @@ update_status:
>>                                     &response, 1);
>>          if (status <= 0)
>>                  DRM_DEBUG_KMS("Could not write test response to sink\n");
>> +
>> +       /* Clear EDID corruption flag now */
>> +       connector->edid_header_corrupt = 0;
> I don't think it makes sense to clear this inside i915.ko. We're
> adding the feature to drm.ko, so It makes sense to clear it there.
Good point, that makes a lot of sense. Clearing the flag in the DRM 
module is the best place for it. Thanks Paulo.
> Isn't it possible to just clear it inside drm_edid_block_valid()?
> Something like:
>
> if (score == 8) {
>      if (header_corrupt)
>          *header_corrupt = 0;
> } else if (score >= edid_fixup) {
>          ... etc ...
>      }
> }
That seems the most logical place for it. Doesn't seem to cause any 
issues with the tests so this works for me. Updated patch will be out 
shortly.

> If not, why?
As I said above, the only issue would be if it gets cleared before the 
test handlers see it, since they're the ones that care about it. But it 
doesn't so it's a non-issue. :)

>
>>   }
>>
>>   static int
>> @@ -4136,7 +4140,7 @@ intel_dp_check_link_status(struct intel_dp *intel_dp, bool long_hpd)
>>           */
>>          if (long_hpd) {
>>                  edid_read = drm_get_edid(connector, adapter);
>> -               if (!edid_read) {
>> +               if (!edid_read || connector->edid_header_corrupt == 1) {
>>                          DRM_DEBUG_DRIVER("Invalid EDID detected\n");
>>                  } else {
>>                          kfree(edid_read);
>> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
>> index d4e4b82..f7a4a92 100644
>> --- a/include/drm/drm_crtc.h
>> +++ b/include/drm/drm_crtc.h
>> @@ -719,6 +719,11 @@ struct drm_connector {
>>          int null_edid_counter; /* needed to workaround some HW bugs where we get all 0s */
>>          unsigned bad_edid_counter;
>>
>> +       /* Flag for raw EDID header corruption - used in Displayport
>> +        * compliance testing - * Displayport Link CTS Core 1.2 rev1.1 4.2.2.6
>> +        */
>> +       bool edid_header_corrupt;
>> +
>>          struct dentry *debugfs_entry;
>>
>>          struct drm_connector_state *state;
>> @@ -1443,7 +1448,8 @@ extern void drm_set_preferred_mode(struct drm_connector *connector,
>>                                     int hpref, int vpref);
>>
>>   extern int drm_edid_header_is_valid(const u8 *raw_edid);
>> -extern bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid);
>> +extern bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid,
>> +                                bool *header_corrupt);
>>   extern bool drm_edid_is_valid(struct edid *edid);
>>
>>   extern struct drm_tile_group *drm_mode_create_tile_group(struct drm_device *dev,
>> --
>> 1.9.1
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
>

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCH 05/12] drm: Add supporting structure for Displayport Link CTS test 4.2.2.6
  2015-04-15 15:38 ` [PATCH 05/10] drm: Add supporting structure for Displayport Link CTS test 4.2.2.6 Todd Previte
  2015-04-15 17:15   ` [PATCH 05/12] " Todd Previte
@ 2015-04-15 22:03   ` Todd Previte
  2015-04-16 13:34     ` Paulo Zanoni
  2015-04-16 15:47   ` Todd Previte
  2 siblings, 1 reply; 31+ messages in thread
From: Todd Previte @ 2015-04-15 22:03 UTC (permalink / raw)
  To: intel-gfx; +Cc: dri-devel

Displayport compliance test 4.2.2.6 requires that a source device be capable of
detecting a corrupt EDID. The test specification states that the sink device
sets up the EDID with an invalid checksum. To do this, the sink sets up an
invalid EDID header, expecting the source device to generate the checksum and
compare it to the value stored in the last byte of the block data.

Unfortunately, the DRM EDID reading and parsing functions are actually too good
in this case; the header is fixed before the checksum is computed and thus the
code never sees the invalid checksum. This results in a failure to pass the
compliance test.

To correct this issue, a checksum is generated when the EDID header is detected
as corrupted. If the checksum is invalid, it sets the header_corrupt flag and
logs the errors. In the case of a more seriously damaged header (fixup score
less than the threshold) the code does not generate the checksum but does set
the header_corrupt flag.

V2:
- Removed the static bool global
- Added a bool to the drm_connector struct to reaplce the static one for
  holding the status of raw edid header corruption detection
- Modified the function signature of the is_valid function to take an
  additional parameter to store the corruption detected value
- Fixed the other callers of the above is_valid function
V3:
- Updated the commit message to be more clear about what and why this
  patch does what it does.
- Added comment in code to clarify the operations there
- Removed compliance variable and check_link_status update; those
  have been moved to a later patch
- Removed variable assignment from the bottom of the test handler
V4:
- Removed i915 tag from subject line as the patch is not i915-specific
V5:
- Moved code causing a compilation error to this patch where the variable
  is actually declared
- Maintained blank lines / spacing so as to not contaminate the patch
V6:
- Removed extra debug messages
- Added documentation to for the added parameter on drm_edid_block_valid
- Fixed more whitespace issues in check_link_status
- Added a clear of the header_corrupt flag to the end of the test handler
  in intel_dp.c
- Changed the usage of the new function prototype in several places to use
  NULL where it is not needed by compliance testing
V7:
- Updated to account for long_pulse flag propagation
V8:
- Removed clearing of header_corrupt flag from the test handler in intel_dp.c
- Added clearing of header_corrupt flag in the drm_edid_block_valid function

Signed-off-by: Todd Previte <tprevite@gmail.com>
Cc: dri-devel@lists.freedesktop.org
---
 drivers/gpu/drm/drm_edid.c      | 35 +++++++++++++++++++++++++++++------
 drivers/gpu/drm/drm_edid_load.c |  7 +++++--
 drivers/gpu/drm/i915/intel_dp.c |  2 +-
 include/drm/drm_crtc.h          |  8 +++++++-
 4 files changed, 42 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 53bc7a6..6d037f9 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -1041,13 +1041,15 @@ static bool drm_edid_is_zero(const u8 *in_edid, int length)
  * @raw_edid: pointer to raw EDID block
  * @block: type of block to validate (0 for base, extension otherwise)
  * @print_bad_edid: if true, dump bad EDID blocks to the console
+ * @header_corrupt: if true, the header or checksum is invalid
  *
  * Validate a base or extension EDID block and optionally dump bad blocks to
  * the console.
  *
  * Return: True if the block is valid, false otherwise.
  */
-bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid)
+bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid,
+			  bool *header_corrupt)
 {
 	u8 csum;
 	struct edid *edid = (struct edid *)raw_edid;
@@ -1060,11 +1062,28 @@ bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid)
 
 	if (block == 0) {
 		int score = drm_edid_header_is_valid(raw_edid);
-		if (score == 8) ;
-		else if (score >= edid_fixup) {
+		if (score == 8) {
+			if (header_corrupt)
+				*header_corrupt = 0;
+		} else if (score >= edid_fixup) {
+			/* Displayport Link CTS Core 1.2 rev1.1 test 4.2.2.6
+			 * In order to properly generate the invalid checksum
+			 * required for this test, it must be generated using
+			 * the raw EDID data. Otherwise, the fix-up code here
+			 * will correct the problem, the checksum is correct
+			 * and the test fails
+			 */
+			csum = drm_edid_block_checksum(raw_edid);
+			if (csum) {
+				if (header_corrupt)
+					*header_corrupt = 1;
+			}
 			DRM_DEBUG("Fixing EDID header, your hardware may be failing\n");
 			memcpy(raw_edid, edid_header, sizeof(edid_header));
 		} else {
+			if (header_corrupt) {
+				*header_corrupt = 1;
+			}
 			goto bad;
 		}
 	}
@@ -1129,7 +1148,7 @@ bool drm_edid_is_valid(struct edid *edid)
 		return false;
 
 	for (i = 0; i <= edid->extensions; i++)
-		if (!drm_edid_block_valid(raw + i * EDID_LENGTH, i, true))
+		if (!drm_edid_block_valid(raw + i * EDID_LENGTH, i, true, NULL))
 			return false;
 
 	return true;
@@ -1232,7 +1251,8 @@ struct edid *drm_do_get_edid(struct drm_connector *connector,
 	for (i = 0; i < 4; i++) {
 		if (get_edid_block(data, block, 0, EDID_LENGTH))
 			goto out;
-		if (drm_edid_block_valid(block, 0, print_bad_edid))
+		if (drm_edid_block_valid(block, 0, print_bad_edid,
+					 &connector->edid_header_corrupt))
 			break;
 		if (i == 0 && drm_edid_is_zero(block, EDID_LENGTH)) {
 			connector->null_edid_counter++;
@@ -1257,7 +1277,10 @@ struct edid *drm_do_get_edid(struct drm_connector *connector,
 				  block + (valid_extensions + 1) * EDID_LENGTH,
 				  j, EDID_LENGTH))
 				goto out;
-			if (drm_edid_block_valid(block + (valid_extensions + 1) * EDID_LENGTH, j, print_bad_edid)) {
+			if (drm_edid_block_valid(block + (valid_extensions + 1)
+						 * EDID_LENGTH, j,
+						 print_bad_edid,
+						 NULL)) {
 				valid_extensions++;
 				break;
 			}
diff --git a/drivers/gpu/drm/drm_edid_load.c b/drivers/gpu/drm/drm_edid_load.c
index 4c0aa97..48b48e8 100644
--- a/drivers/gpu/drm/drm_edid_load.c
+++ b/drivers/gpu/drm/drm_edid_load.c
@@ -216,7 +216,8 @@ static void *edid_load(struct drm_connector *connector, const char *name,
 		goto out;
 	}
 
-	if (!drm_edid_block_valid(edid, 0, print_bad_edid)) {
+	if (!drm_edid_block_valid(edid, 0, print_bad_edid,
+				  &connector->edid_header_corrupt)) {
 		connector->bad_edid_counter++;
 		DRM_ERROR("Base block of EDID firmware \"%s\" is invalid ",
 		    name);
@@ -229,7 +230,9 @@ static void *edid_load(struct drm_connector *connector, const char *name,
 		if (i != valid_extensions + 1)
 			memcpy(edid + (valid_extensions + 1) * EDID_LENGTH,
 			    edid + i * EDID_LENGTH, EDID_LENGTH);
-		if (drm_edid_block_valid(edid + i * EDID_LENGTH, i, print_bad_edid))
+		if (drm_edid_block_valid(edid + i * EDID_LENGTH, i,
+					 print_bad_edid,
+					 NULL))
 			valid_extensions++;
 	}
 
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 7ca0e18..97224ff 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4139,7 +4139,7 @@ intel_dp_check_link_status(struct intel_dp *intel_dp, bool long_hpd)
 	 */
 	if (long_hpd) {
 		edid_read = drm_get_edid(connector, adapter);
-		if (!edid_read) {
+		if (!edid_read || connector->edid_header_corrupt == 1) {
 			DRM_DEBUG_DRIVER("Invalid EDID detected\n");
 		} else {
 			kfree(edid_read);
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index d4e4b82..f7a4a92 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -719,6 +719,11 @@ struct drm_connector {
 	int null_edid_counter; /* needed to workaround some HW bugs where we get all 0s */
 	unsigned bad_edid_counter;
 
+	/* Flag for raw EDID header corruption - used in Displayport
+	 * compliance testing - * Displayport Link CTS Core 1.2 rev1.1 4.2.2.6
+	 */
+	bool edid_header_corrupt;
+
 	struct dentry *debugfs_entry;
 
 	struct drm_connector_state *state;
@@ -1443,7 +1448,8 @@ extern void drm_set_preferred_mode(struct drm_connector *connector,
 				   int hpref, int vpref);
 
 extern int drm_edid_header_is_valid(const u8 *raw_edid);
-extern bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid);
+extern bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid,
+				 bool *header_corrupt);
 extern bool drm_edid_is_valid(struct edid *edid);
 
 extern struct drm_tile_group *drm_mode_create_tile_group(struct drm_device *dev,
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 06/12] drm/i915: Update intel_dp_hpd_pulse() for non-MST operation
  2015-04-15 15:38 ` [PATCH 06/10] drm/i915: Update intel_dp_hpd_pulse() for non-MST operation Todd Previte
  2015-04-15 17:15   ` [PATCH 06/12] " Todd Previte
@ 2015-04-16  7:22   ` Todd Previte
  1 sibling, 0 replies; 31+ messages in thread
From: Todd Previte @ 2015-04-16  7:22 UTC (permalink / raw)
  To: intel-gfx

Update the hot plug function to handle the SST case. Instead of placing
the SST case within the long/short pulse block, it is now handled after
determining that MST mode is not in use. This way, the topology management
layer can handle any MST-related operations while SST operations are still
correctly handled afterwards.

This patch also corrects the problem of SST mode only being handled in the
case of a short (0.5ms - 1.0ms) HPD pulse. For compliance testing purposes
both short and long pulses are used by the different tests, thus both cases
need to be addressed for SST.

One additional change is the removal of the return of IRQ_HANDLED for the
SST case. This corrects an issue where the normal hot plug functions were
not being called under certain circumstances while implementing the
compliance testing.

This patch replaces [PATCH 10/10] drm/i915: Fix intel_dp_hot_plug() in the
previous compliance testing patch sequence. Review feedback on that patch
indicated that updating intel_dp_hot_plug() was not the correct place for
the test handler.

V2:
- N/A
V3:
- Place the SST mode link status check into the mst_fail case
- Remove obsolete comment regarding SST mode operation
- Removed an erroneous line of code that snuck in during rebasing
V4:
- Added a disable of the main stream (DP transport) for the long pulse case
  for SST to support compliance testing
V5:
- Reworked SST handling to support tests 4.2.2.7 and 4.2.2.8
V6:
- Reformatted a comment
V7:
- Moved a comment again that was inadvertently moved
- Updated the code to properly handle all permutations of MST/SST and
  short/long pulse.
- Adds a new 'connected' flag that prevents unnecessary operations when
  the link is disconnected.
- Added a function to encapsulate detection of the HPD pin status in
  in order to determine connected status
- Reformatted the if-statements in the function so the braces are
  consistent for those with single statements after the if-statement
- Main link disable that was added in V4 has been removed
V8:
- Updated to account for long_hpd flag propagation
V9:
- Change the return type for the SST case to IRQ_NONE to allow for
  proper invocation of other hot plug functions

Signed-off-by: Todd Previte <tprevite@gmail.com>
---
 drivers/gpu/drm/i915/intel_dp.c  | 54 ++++++++++++++++++++++++----------------
 drivers/gpu/drm/i915/intel_drv.h |  1 +
 2 files changed, 34 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 97224ff..1ba5247 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4746,6 +4746,7 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	enum intel_display_power_domain power_domain;
 	enum irqreturn ret = IRQ_NONE;
+	bool connected = false;
 
 	if (intel_dig_port->base.type != INTEL_OUTPUT_EDP)
 		intel_dig_port->base.type = INTEL_OUTPUT_DISPLAYPORT;
@@ -4769,19 +4770,15 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
 	power_domain = intel_display_port_power_domain(intel_encoder);
 	intel_display_power_get(dev_priv, power_domain);
 
+	connected = intel_dp_digital_port_connected(intel_dp);
+
 	if (long_hpd) {
 
-		if (HAS_PCH_SPLIT(dev)) {
-			if (!ibx_digital_port_connected(dev_priv, intel_dig_port))
-				goto mst_fail;
-		} else {
-			if (g4x_digital_port_connected(dev, intel_dig_port) != 1)
-				goto mst_fail;
-		}
+		if (!connected)
+			goto mst_fail;
 
-		if (!intel_dp_get_dpcd(intel_dp)) {
+		if (!intel_dp_get_dpcd(intel_dp))
 			goto mst_fail;
-		}
 
 		intel_dp_probe_oui(intel_dp);
 
@@ -4789,20 +4786,9 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
 			goto mst_fail;
 
 	} else {
-		if (intel_dp->is_mst) {
+		if (intel_dp->is_mst)
 			if (intel_dp_check_mst_status(intel_dp) == -EINVAL)
 				goto mst_fail;
-		}
-
-		if (!intel_dp->is_mst) {
-			/*
-			 * we'll check the link status via the normal hot plug path later -
-			 * but for short hpds we should check it now
-			 */
-			drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
-			intel_dp_check_link_status(intel_dp, long_hpd);
-			drm_modeset_unlock(&dev->mode_config.connection_mutex);
-		}
 	}
 
 	ret = IRQ_HANDLED;
@@ -4816,6 +4802,14 @@ mst_fail:
 		drm_dp_mst_topology_mgr_set_mst(&intel_dp->mst_mgr, intel_dp->is_mst);
 	}
 put_power:
+	/* SST mode - handle short/long pulses here */
+	if (!intel_dp->is_mst) {
+
+		drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
+		if (connected)
+			intel_dp_check_link_status(intel_dp, long_hpd);
+		drm_modeset_unlock(&dev->mode_config.connection_mutex);
+	}
 	intel_display_power_put(dev_priv, power_domain);
 
 	return ret;
@@ -5802,3 +5796,21 @@ void intel_dp_mst_resume(struct drm_device *dev)
 		}
 	}
 }
+
+bool intel_dp_digital_port_connected(struct intel_dp *intel_dp)
+{
+	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
+	struct drm_device *dev = intel_dig_port->base.base.dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	bool connected = true;
+
+	if (HAS_PCH_SPLIT(dev)) {
+		if (!ibx_digital_port_connected(dev_priv, intel_dig_port))
+			connected = false;
+	} else {
+		if (g4x_digital_port_connected(dev, intel_dig_port) != 1)
+			connected = false;
+	}
+
+	return connected;
+}
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index a4675fa..accf08b 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1173,6 +1173,7 @@ void intel_edp_drrs_disable(struct intel_dp *intel_dp);
 void intel_edp_drrs_invalidate(struct drm_device *dev,
 		unsigned frontbuffer_bits);
 void intel_edp_drrs_flush(struct drm_device *dev, unsigned frontbuffer_bits);
+bool intel_dp_digital_port_connected(struct intel_dp *intel_dp);
 
 /* intel_dp_mst.c */
 int intel_dp_mst_encoder_init(struct intel_digital_port *intel_dig_port, int conn_id);
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* Re: [PATCH 01/10] drm/i915: Add automated testing support for Displayport compliance testing
  2015-04-15 15:38 ` [PATCH 01/10] drm/i915: Add automated testing support for Displayport compliance testing Todd Previte
@ 2015-04-16 11:32   ` Daniel Vetter
  0 siblings, 0 replies; 31+ messages in thread
From: Daniel Vetter @ 2015-04-16 11:32 UTC (permalink / raw)
  To: Todd Previte; +Cc: intel-gfx

On Wed, Apr 15, 2015 at 08:38:38AM -0700, Todd Previte wrote:
> Add the skeleton framework for supporting automation for Displayport compliance
> testing. This patch adds the necessary framework for the source device to
> appropriately respond to test automation requests from a sink device.
> 
> V2:
> - Addressed previous mailing list feedback
> - Fixed compilation issue (struct members declared in a later patch)
> - Updated debug messages to be more accurate
> - Added status checks for the DPCD read/write calls
> - Removed excess comments and debug messages
> - Fixed debug message compilation warnings
> - Fixed compilation issue with missing variables
> - Updated link training autotest to ACK
> 
> V3:
> - Fixed the checks on the DPCD return code to be <= 0
>   rather than != 0
> - Removed extraneous assignment of a NAK return code in the
>   DPCD read failure case
> - Changed the return in the DPCD read failure case to a goto
>   to the exit point where the status code is written to the sink
> - Removed FAUX test case since it's deprecated now
> - Removed the compliance flag assignment in handle_test_request
> 
> V4:
> - Moved declaration of type_type here
> - Removed declaration of test_data (moved to a later patch)
> - Added reset to 0 for compliance test variables
> 
> V5:
> - Moved test_active variable declaration and initialization out of
>   this patch and into the patch where it's used
> - Changed variable name compliance_testing_active to
>   compliance_test_active to unify the naming convention
> - Added initialization for compliance_test_type variable
> 
> Signed-off-by: Todd Previte <tprevite@gmail.com>
> Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

Queued for -next, thanks for the patch.
-Daniel

> ---
>  drivers/gpu/drm/i915/intel_dp.c  | 75 +++++++++++++++++++++++++++++++++++++---
>  drivers/gpu/drm/i915/intel_drv.h |  3 ++
>  2 files changed, 74 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 14cdd00..263eff3 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -3970,11 +3970,78 @@ intel_dp_get_sink_irq_esi(struct intel_dp *intel_dp, u8 *sink_irq_vector)
>  	return true;
>  }
>  
> -static void
> -intel_dp_handle_test_request(struct intel_dp *intel_dp)
> +static uint8_t intel_dp_autotest_link_training(struct intel_dp *intel_dp)
> +{
> +	uint8_t test_result = DP_TEST_ACK;
> +	return test_result;
> +}
> +
> +static uint8_t intel_dp_autotest_video_pattern(struct intel_dp *intel_dp)
> +{
> +	uint8_t test_result = DP_TEST_NAK;
> +	return test_result;
> +}
> +
> +static uint8_t intel_dp_autotest_edid(struct intel_dp *intel_dp)
> +{
> +	uint8_t test_result = DP_TEST_NAK;
> +	return test_result;
> +}
> +
> +static uint8_t intel_dp_autotest_phy_pattern(struct intel_dp *intel_dp)
> +{
> +	uint8_t test_result = DP_TEST_NAK;
> +	return test_result;
> +}
> +
> +static void intel_dp_handle_test_request(struct intel_dp *intel_dp)
>  {
> -	/* NAK by default */
> -	drm_dp_dpcd_writeb(&intel_dp->aux, DP_TEST_RESPONSE, DP_TEST_NAK);
> +	uint8_t response = DP_TEST_NAK;
> +	uint8_t rxdata = 0;
> +	int status = 0;
> +
> +	intel_dp->compliance_test_type = 0;
> +	intel_dp->aux.i2c_nack_count = 0;
> +	intel_dp->aux.i2c_defer_count = 0;
> +
> +	status = drm_dp_dpcd_read(&intel_dp->aux, DP_TEST_REQUEST, &rxdata, 1);
> +	if (status <= 0) {
> +		DRM_DEBUG_KMS("Could not read test request from sink\n");
> +		goto update_status;
> +	}
> +
> +	switch (rxdata) {
> +	case DP_TEST_LINK_TRAINING:
> +		DRM_DEBUG_KMS("LINK_TRAINING test requested\n");
> +		intel_dp->compliance_test_type = DP_TEST_LINK_TRAINING;
> +		response = intel_dp_autotest_link_training(intel_dp);
> +		break;
> +	case DP_TEST_LINK_VIDEO_PATTERN:
> +		DRM_DEBUG_KMS("TEST_PATTERN test requested\n");
> +		intel_dp->compliance_test_type = DP_TEST_LINK_VIDEO_PATTERN;
> +		response = intel_dp_autotest_video_pattern(intel_dp);
> +		break;
> +	case DP_TEST_LINK_EDID_READ:
> +		DRM_DEBUG_KMS("EDID test requested\n");
> +		intel_dp->compliance_test_type = DP_TEST_LINK_EDID_READ;
> +		response = intel_dp_autotest_edid(intel_dp);
> +		break;
> +	case DP_TEST_LINK_PHY_TEST_PATTERN:
> +		DRM_DEBUG_KMS("PHY_PATTERN test requested\n");
> +		intel_dp->compliance_test_type = DP_TEST_LINK_PHY_TEST_PATTERN;
> +		response = intel_dp_autotest_phy_pattern(intel_dp);
> +		break;
> +	default:
> +		DRM_DEBUG_KMS("Invalid test request '%02x'\n", rxdata);
> +		break;
> +	}
> +
> +update_status:
> +	status = drm_dp_dpcd_write(&intel_dp->aux,
> +				   DP_TEST_RESPONSE,
> +				   &response, 1);
> +	if (status <= 0)
> +		DRM_DEBUG_KMS("Could not write test response to sink\n");
>  }
>  
>  static int
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 6a2ee0c..a4675fa 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -739,6 +739,9 @@ struct intel_dp {
>  				     bool has_aux_irq,
>  				     int send_bytes,
>  				     uint32_t aux_clock_divider);
> +
> +	/* Displayport compliance testing */
> +	unsigned long compliance_test_type;
>  };
>  
>  struct intel_digital_port {
> -- 
> 1.9.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 04/10] drm/i915: Add a delay in Displayport AUX transactions for compliance testing
  2015-04-15 15:38 ` [PATCH 04/10] drm/i915: Add a delay in Displayport AUX transactions for compliance testing Todd Previte
@ 2015-04-16 12:37   ` Daniel Vetter
  0 siblings, 0 replies; 31+ messages in thread
From: Daniel Vetter @ 2015-04-16 12:37 UTC (permalink / raw)
  To: Todd Previte; +Cc: intel-gfx

On Wed, Apr 15, 2015 at 08:38:41AM -0700, Todd Previte wrote:
> The Displayport Link Layer Compliance Testing Specification 1.2 rev 1.1
> specifies that repeated AUX transactions after a failure (no response /
> invalid response) must have a minimum delay of 400us before the resend can
> occur. Tests 4.2.1.1 and 4.2.1.2 are two tests that require this specifically.
> 
> Also, the check for DP_AUX_CH_CTL_TIME_OUT_ERROR has been moved out into a
> separate case. This case just continues with the next iteration of the loop
> as the HW has already waited the required amount of time.
> 
> V2:
> - Changed udelay() to usleep_range()
> V3:
> - Removed extraneous check for timeout
> - Updated comment to reflect this change
> V4:
> - Reformatted a comment
> V5:
> - Added separate check for HW timeout on AUX transactions. A message
>   is logged upon detection of this case.
> V6:
> - Add continue statement to HW timeout detect case
> - Remove the log message indicating a timeout has been
>   detected (review feedback)
> V7:
> - Updated the commit message to remove verbage about the HW timeout
>   case that is no longer valid.
> 
> Signed-off-by: Todd Previte <tprevite@gmail.com>
> Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

Queued for -next, thanks for the patch.
-Daniel

> ---
>  drivers/gpu/drm/i915/intel_dp.c | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index c112359..dae5c9a 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -874,9 +874,18 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
>  				   DP_AUX_CH_CTL_TIME_OUT_ERROR |
>  				   DP_AUX_CH_CTL_RECEIVE_ERROR);
>  
> -			if (status & (DP_AUX_CH_CTL_TIME_OUT_ERROR |
> -				      DP_AUX_CH_CTL_RECEIVE_ERROR))
> +			if (status & DP_AUX_CH_CTL_TIME_OUT_ERROR)
>  				continue;
> +
> +			/* DP CTS 1.2 Core Rev 1.1, 4.2.1.1 & 4.2.1.2
> +			 *   400us delay required for errors and timeouts
> +			 *   Timeout errors from the HW already meet this
> +			 *   requirement so skip to next iteration
> +			 */
> +			if (status & DP_AUX_CH_CTL_RECEIVE_ERROR) {
> +				usleep_range(400, 500);
> +				continue;
> +			}
>  			if (status & DP_AUX_CH_CTL_DONE)
>  				break;
>  		}
> -- 
> 1.9.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 10/10] drm: Fix the 'native defer' message in drm_dp_i2c_do_msg()
  2015-04-15 15:38 ` [PATCH 10/10] drm: Fix the 'native defer' message in drm_dp_i2c_do_msg() Todd Previte
@ 2015-04-16 12:42   ` Daniel Vetter
  2015-04-16 12:44   ` Daniel Vetter
  1 sibling, 0 replies; 31+ messages in thread
From: Daniel Vetter @ 2015-04-16 12:42 UTC (permalink / raw)
  To: Todd Previte; +Cc: intel-gfx, dri-devel

On Wed, Apr 15, 2015 at 08:38:47AM -0700, Todd Previte wrote:
> 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 <tprevite@gmail.com>
> Cc: dri-devel@lists.freedesktop.org
> Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Reviewed-by: Alex Deucher <alexander.deucher@amd.com>

Applied to drm-misc.
-Daniel

> ---
>  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
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 10/10] drm: Fix the 'native defer' message in drm_dp_i2c_do_msg()
  2015-04-15 15:38 ` [PATCH 10/10] drm: Fix the 'native defer' message in drm_dp_i2c_do_msg() Todd Previte
  2015-04-16 12:42   ` Daniel Vetter
@ 2015-04-16 12:44   ` Daniel Vetter
  1 sibling, 0 replies; 31+ messages in thread
From: Daniel Vetter @ 2015-04-16 12:44 UTC (permalink / raw)
  To: Todd Previte; +Cc: intel-gfx, dri-devel

On Wed, Apr 15, 2015 at 08:38:47AM -0700, Todd Previte wrote:
> 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 <tprevite@gmail.com>
> Cc: dri-devel@lists.freedesktop.org
> Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Reviewed-by: Alex Deucher <alexander.deucher@amd.com>

Applied to topic/drm-misc.
-Daniel

> ---
>  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
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 05/12] drm: Add supporting structure for Displayport Link CTS test 4.2.2.6
  2015-04-15 22:03   ` Todd Previte
@ 2015-04-16 13:34     ` Paulo Zanoni
  2015-04-16 14:02       ` Todd Previte
  0 siblings, 1 reply; 31+ messages in thread
From: Paulo Zanoni @ 2015-04-16 13:34 UTC (permalink / raw)
  To: Todd Previte; +Cc: Intel Graphics Development, DRI Development

2015-04-15 19:03 GMT-03:00 Todd Previte <tprevite@gmail.com>:
> Displayport compliance test 4.2.2.6 requires that a source device be capable of
> detecting a corrupt EDID. The test specification states that the sink device
> sets up the EDID with an invalid checksum. To do this, the sink sets up an
> invalid EDID header, expecting the source device to generate the checksum and
> compare it to the value stored in the last byte of the block data.
>
> Unfortunately, the DRM EDID reading and parsing functions are actually too good
> in this case; the header is fixed before the checksum is computed and thus the
> code never sees the invalid checksum. This results in a failure to pass the
> compliance test.
>
> To correct this issue, a checksum is generated when the EDID header is detected
> as corrupted. If the checksum is invalid, it sets the header_corrupt flag and
> logs the errors. In the case of a more seriously damaged header (fixup score
> less than the threshold) the code does not generate the checksum but does set
> the header_corrupt flag.
>
> V2:
> - Removed the static bool global
> - Added a bool to the drm_connector struct to reaplce the static one for
>   holding the status of raw edid header corruption detection
> - Modified the function signature of the is_valid function to take an
>   additional parameter to store the corruption detected value
> - Fixed the other callers of the above is_valid function
> V3:
> - Updated the commit message to be more clear about what and why this
>   patch does what it does.
> - Added comment in code to clarify the operations there
> - Removed compliance variable and check_link_status update; those
>   have been moved to a later patch
> - Removed variable assignment from the bottom of the test handler
> V4:
> - Removed i915 tag from subject line as the patch is not i915-specific
> V5:
> - Moved code causing a compilation error to this patch where the variable
>   is actually declared
> - Maintained blank lines / spacing so as to not contaminate the patch
> V6:
> - Removed extra debug messages
> - Added documentation to for the added parameter on drm_edid_block_valid
> - Fixed more whitespace issues in check_link_status
> - Added a clear of the header_corrupt flag to the end of the test handler
>   in intel_dp.c
> - Changed the usage of the new function prototype in several places to use
>   NULL where it is not needed by compliance testing
> V7:
> - Updated to account for long_pulse flag propagation
> V8:
> - Removed clearing of header_corrupt flag from the test handler in intel_dp.c
> - Added clearing of header_corrupt flag in the drm_edid_block_valid function
>
> Signed-off-by: Todd Previte <tprevite@gmail.com>
> Cc: dri-devel@lists.freedesktop.org
> ---
>  drivers/gpu/drm/drm_edid.c      | 35 +++++++++++++++++++++++++++++------
>  drivers/gpu/drm/drm_edid_load.c |  7 +++++--
>  drivers/gpu/drm/i915/intel_dp.c |  2 +-
>  include/drm/drm_crtc.h          |  8 +++++++-
>  4 files changed, 42 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 53bc7a6..6d037f9 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -1041,13 +1041,15 @@ static bool drm_edid_is_zero(const u8 *in_edid, int length)
>   * @raw_edid: pointer to raw EDID block
>   * @block: type of block to validate (0 for base, extension otherwise)
>   * @print_bad_edid: if true, dump bad EDID blocks to the console
> + * @header_corrupt: if true, the header or checksum is invalid
>   *
>   * Validate a base or extension EDID block and optionally dump bad blocks to
>   * the console.
>   *
>   * Return: True if the block is valid, false otherwise.
>   */
> -bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid)
> +bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid,
> +                         bool *header_corrupt)
>  {
>         u8 csum;
>         struct edid *edid = (struct edid *)raw_edid;
> @@ -1060,11 +1062,28 @@ bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid)
>
>         if (block == 0) {
>                 int score = drm_edid_header_is_valid(raw_edid);
> -               if (score == 8) ;
> -               else if (score >= edid_fixup) {
> +               if (score == 8) {
> +                       if (header_corrupt)
> +                               *header_corrupt = 0;
> +               } else if (score >= edid_fixup) {
> +                       /* Displayport Link CTS Core 1.2 rev1.1 test 4.2.2.6
> +                        * In order to properly generate the invalid checksum
> +                        * required for this test, it must be generated using
> +                        * the raw EDID data. Otherwise, the fix-up code here
> +                        * will correct the problem, the checksum is correct
> +                        * and the test fails
> +                        */
> +                       csum = drm_edid_block_checksum(raw_edid);
> +                       if (csum) {
> +                               if (header_corrupt)
> +                                       *header_corrupt = 1;
> +                       }


I was ready to give the R-B tag, but then I realized this. Shouldn't
we be setting header_corrupt here regardless of the checksum value? I
mean, score is not 8, so the header is indeed corrupt.

Also, setting "header_corrupt" to 1 in case the checksum is invalid is
somewhat a distortion of meaning in the variable. If we really need
the checksum, we should probably have something like
connector->raw_checksum, or maybe rename header_corrupt to
edid_corrupt, where "edid_corrupt" means "either invalid header or
invalid checksum"?

And for compliance testing specifically, as far as I understand, I see
on patch 7 that you only care about the checksum if the header is
valid, so we don't seem to need the csum value here, so it seems my
suggestion to just assign header_corrupt=1 regardless of the checksum
value seems to be enough to solve your problem.

Sorry for not realizing this earlier, the back-and-forth between
patches royally confused me.


>                         DRM_DEBUG("Fixing EDID header, your hardware may be failing\n");
>                         memcpy(raw_edid, edid_header, sizeof(edid_header));
>                 } else {
> +                       if (header_corrupt) {
> +                               *header_corrupt = 1;
> +                       }
>                         goto bad;
>                 }
>         }
> @@ -1129,7 +1148,7 @@ bool drm_edid_is_valid(struct edid *edid)
>                 return false;
>
>         for (i = 0; i <= edid->extensions; i++)
> -               if (!drm_edid_block_valid(raw + i * EDID_LENGTH, i, true))
> +               if (!drm_edid_block_valid(raw + i * EDID_LENGTH, i, true, NULL))
>                         return false;
>
>         return true;
> @@ -1232,7 +1251,8 @@ struct edid *drm_do_get_edid(struct drm_connector *connector,
>         for (i = 0; i < 4; i++) {
>                 if (get_edid_block(data, block, 0, EDID_LENGTH))
>                         goto out;
> -               if (drm_edid_block_valid(block, 0, print_bad_edid))
> +               if (drm_edid_block_valid(block, 0, print_bad_edid,
> +                                        &connector->edid_header_corrupt))
>                         break;
>                 if (i == 0 && drm_edid_is_zero(block, EDID_LENGTH)) {
>                         connector->null_edid_counter++;
> @@ -1257,7 +1277,10 @@ struct edid *drm_do_get_edid(struct drm_connector *connector,
>                                   block + (valid_extensions + 1) * EDID_LENGTH,
>                                   j, EDID_LENGTH))
>                                 goto out;
> -                       if (drm_edid_block_valid(block + (valid_extensions + 1) * EDID_LENGTH, j, print_bad_edid)) {
> +                       if (drm_edid_block_valid(block + (valid_extensions + 1)
> +                                                * EDID_LENGTH, j,
> +                                                print_bad_edid,
> +                                                NULL)) {
>                                 valid_extensions++;
>                                 break;
>                         }
> diff --git a/drivers/gpu/drm/drm_edid_load.c b/drivers/gpu/drm/drm_edid_load.c
> index 4c0aa97..48b48e8 100644
> --- a/drivers/gpu/drm/drm_edid_load.c
> +++ b/drivers/gpu/drm/drm_edid_load.c
> @@ -216,7 +216,8 @@ static void *edid_load(struct drm_connector *connector, const char *name,
>                 goto out;
>         }
>
> -       if (!drm_edid_block_valid(edid, 0, print_bad_edid)) {
> +       if (!drm_edid_block_valid(edid, 0, print_bad_edid,
> +                                 &connector->edid_header_corrupt)) {
>                 connector->bad_edid_counter++;
>                 DRM_ERROR("Base block of EDID firmware \"%s\" is invalid ",
>                     name);
> @@ -229,7 +230,9 @@ static void *edid_load(struct drm_connector *connector, const char *name,
>                 if (i != valid_extensions + 1)
>                         memcpy(edid + (valid_extensions + 1) * EDID_LENGTH,
>                             edid + i * EDID_LENGTH, EDID_LENGTH);
> -               if (drm_edid_block_valid(edid + i * EDID_LENGTH, i, print_bad_edid))
> +               if (drm_edid_block_valid(edid + i * EDID_LENGTH, i,
> +                                        print_bad_edid,
> +                                        NULL))
>                         valid_extensions++;
>         }
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 7ca0e18..97224ff 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -4139,7 +4139,7 @@ intel_dp_check_link_status(struct intel_dp *intel_dp, bool long_hpd)
>          */
>         if (long_hpd) {
>                 edid_read = drm_get_edid(connector, adapter);
> -               if (!edid_read) {
> +               if (!edid_read || connector->edid_header_corrupt == 1) {
>                         DRM_DEBUG_DRIVER("Invalid EDID detected\n");
>                 } else {
>                         kfree(edid_read);
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index d4e4b82..f7a4a92 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -719,6 +719,11 @@ struct drm_connector {
>         int null_edid_counter; /* needed to workaround some HW bugs where we get all 0s */
>         unsigned bad_edid_counter;
>
> +       /* Flag for raw EDID header corruption - used in Displayport
> +        * compliance testing - * Displayport Link CTS Core 1.2 rev1.1 4.2.2.6
> +        */
> +       bool edid_header_corrupt;
> +
>         struct dentry *debugfs_entry;
>
>         struct drm_connector_state *state;
> @@ -1443,7 +1448,8 @@ extern void drm_set_preferred_mode(struct drm_connector *connector,
>                                    int hpref, int vpref);
>
>  extern int drm_edid_header_is_valid(const u8 *raw_edid);
> -extern bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid);
> +extern bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid,
> +                                bool *header_corrupt);
>  extern bool drm_edid_is_valid(struct edid *edid);
>
>  extern struct drm_tile_group *drm_mode_create_tile_group(struct drm_device *dev,
> --
> 1.9.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel



-- 
Paulo Zanoni
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 05/12] drm: Add supporting structure for Displayport Link CTS test 4.2.2.6
  2015-04-16 13:34     ` Paulo Zanoni
@ 2015-04-16 14:02       ` Todd Previte
  0 siblings, 0 replies; 31+ messages in thread
From: Todd Previte @ 2015-04-16 14:02 UTC (permalink / raw)
  To: Paulo Zanoni; +Cc: Intel Graphics Development, DRI Development



On 4/16/2015 6:34 AM, Paulo Zanoni wrote:
> 2015-04-15 19:03 GMT-03:00 Todd Previte <tprevite@gmail.com>:
>> Displayport compliance test 4.2.2.6 requires that a source device be capable of
>> detecting a corrupt EDID. The test specification states that the sink device
>> sets up the EDID with an invalid checksum. To do this, the sink sets up an
>> invalid EDID header, expecting the source device to generate the checksum and
>> compare it to the value stored in the last byte of the block data.
>>
>> Unfortunately, the DRM EDID reading and parsing functions are actually too good
>> in this case; the header is fixed before the checksum is computed and thus the
>> code never sees the invalid checksum. This results in a failure to pass the
>> compliance test.
>>
>> To correct this issue, a checksum is generated when the EDID header is detected
>> as corrupted. If the checksum is invalid, it sets the header_corrupt flag and
>> logs the errors. In the case of a more seriously damaged header (fixup score
>> less than the threshold) the code does not generate the checksum but does set
>> the header_corrupt flag.
>>
>> V2:
>> - Removed the static bool global
>> - Added a bool to the drm_connector struct to reaplce the static one for
>>    holding the status of raw edid header corruption detection
>> - Modified the function signature of the is_valid function to take an
>>    additional parameter to store the corruption detected value
>> - Fixed the other callers of the above is_valid function
>> V3:
>> - Updated the commit message to be more clear about what and why this
>>    patch does what it does.
>> - Added comment in code to clarify the operations there
>> - Removed compliance variable and check_link_status update; those
>>    have been moved to a later patch
>> - Removed variable assignment from the bottom of the test handler
>> V4:
>> - Removed i915 tag from subject line as the patch is not i915-specific
>> V5:
>> - Moved code causing a compilation error to this patch where the variable
>>    is actually declared
>> - Maintained blank lines / spacing so as to not contaminate the patch
>> V6:
>> - Removed extra debug messages
>> - Added documentation to for the added parameter on drm_edid_block_valid
>> - Fixed more whitespace issues in check_link_status
>> - Added a clear of the header_corrupt flag to the end of the test handler
>>    in intel_dp.c
>> - Changed the usage of the new function prototype in several places to use
>>    NULL where it is not needed by compliance testing
>> V7:
>> - Updated to account for long_pulse flag propagation
>> V8:
>> - Removed clearing of header_corrupt flag from the test handler in intel_dp.c
>> - Added clearing of header_corrupt flag in the drm_edid_block_valid function
>>
>> Signed-off-by: Todd Previte <tprevite@gmail.com>
>> Cc: dri-devel@lists.freedesktop.org
>> ---
>>   drivers/gpu/drm/drm_edid.c      | 35 +++++++++++++++++++++++++++++------
>>   drivers/gpu/drm/drm_edid_load.c |  7 +++++--
>>   drivers/gpu/drm/i915/intel_dp.c |  2 +-
>>   include/drm/drm_crtc.h          |  8 +++++++-
>>   4 files changed, 42 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
>> index 53bc7a6..6d037f9 100644
>> --- a/drivers/gpu/drm/drm_edid.c
>> +++ b/drivers/gpu/drm/drm_edid.c
>> @@ -1041,13 +1041,15 @@ static bool drm_edid_is_zero(const u8 *in_edid, int length)
>>    * @raw_edid: pointer to raw EDID block
>>    * @block: type of block to validate (0 for base, extension otherwise)
>>    * @print_bad_edid: if true, dump bad EDID blocks to the console
>> + * @header_corrupt: if true, the header or checksum is invalid
>>    *
>>    * Validate a base or extension EDID block and optionally dump bad blocks to
>>    * the console.
>>    *
>>    * Return: True if the block is valid, false otherwise.
>>    */
>> -bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid)
>> +bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid,
>> +                         bool *header_corrupt)
>>   {
>>          u8 csum;
>>          struct edid *edid = (struct edid *)raw_edid;
>> @@ -1060,11 +1062,28 @@ bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid)
>>
>>          if (block == 0) {
>>                  int score = drm_edid_header_is_valid(raw_edid);
>> -               if (score == 8) ;
>> -               else if (score >= edid_fixup) {
>> +               if (score == 8) {
>> +                       if (header_corrupt)
>> +                               *header_corrupt = 0;
>> +               } else if (score >= edid_fixup) {
>> +                       /* Displayport Link CTS Core 1.2 rev1.1 test 4.2.2.6
>> +                        * In order to properly generate the invalid checksum
>> +                        * required for this test, it must be generated using
>> +                        * the raw EDID data. Otherwise, the fix-up code here
>> +                        * will correct the problem, the checksum is correct
>> +                        * and the test fails
>> +                        */
>> +                       csum = drm_edid_block_checksum(raw_edid);
>> +                       if (csum) {
>> +                               if (header_corrupt)
>> +                                       *header_corrupt = 1;
>> +                       }
>
> I was ready to give the R-B tag, but then I realized this. Shouldn't
> we be setting header_corrupt here regardless of the checksum value? I
> mean, score is not 8, so the header is indeed corrupt.
I added the checksum computation as an extra validation that the header 
was indeed corrupt. However, that checksum is discarded so the 
additional checksum generation isn't strictly necessary. I've removed it 
in the updated patch.
> Also, setting "header_corrupt" to 1 in case the checksum is invalid is
> somewhat a distortion of meaning in the variable. If we really need
> the checksum, we should probably have something like
> connector->raw_checksum, or maybe rename header_corrupt to
> edid_corrupt, where "edid_corrupt" means "either invalid header or
> invalid checksum"?
I was thinking the same thing as I've been working with this. I think 
renaming it to "edid_corrupt" is more accurate. Fixed in the update.
> And for compliance testing specifically, as far as I understand, I see
> on patch 7 that you only care about the checksum if the header is
> valid, so we don't seem to need the csum value here, so it seems my
> suggestion to just assign header_corrupt=1 regardless of the checksum
> value seems to be enough to solve your problem.
Agreed.
> Sorry for not realizing this earlier, the back-and-forth between
> patches royally confused me.
Indeed, that's an issue for me as well.
>
>>                          DRM_DEBUG("Fixing EDID header, your hardware may be failing\n");
>>                          memcpy(raw_edid, edid_header, sizeof(edid_header));
>>                  } else {
>> +                       if (header_corrupt) {
>> +                               *header_corrupt = 1;
>> +                       }
>>                          goto bad;
>>                  }
>>          }
>> @@ -1129,7 +1148,7 @@ bool drm_edid_is_valid(struct edid *edid)
>>                  return false;
>>
>>          for (i = 0; i <= edid->extensions; i++)
>> -               if (!drm_edid_block_valid(raw + i * EDID_LENGTH, i, true))
>> +               if (!drm_edid_block_valid(raw + i * EDID_LENGTH, i, true, NULL))
>>                          return false;
>>
>>          return true;
>> @@ -1232,7 +1251,8 @@ struct edid *drm_do_get_edid(struct drm_connector *connector,
>>          for (i = 0; i < 4; i++) {
>>                  if (get_edid_block(data, block, 0, EDID_LENGTH))
>>                          goto out;
>> -               if (drm_edid_block_valid(block, 0, print_bad_edid))
>> +               if (drm_edid_block_valid(block, 0, print_bad_edid,
>> +                                        &connector->edid_header_corrupt))
>>                          break;
>>                  if (i == 0 && drm_edid_is_zero(block, EDID_LENGTH)) {
>>                          connector->null_edid_counter++;
>> @@ -1257,7 +1277,10 @@ struct edid *drm_do_get_edid(struct drm_connector *connector,
>>                                    block + (valid_extensions + 1) * EDID_LENGTH,
>>                                    j, EDID_LENGTH))
>>                                  goto out;
>> -                       if (drm_edid_block_valid(block + (valid_extensions + 1) * EDID_LENGTH, j, print_bad_edid)) {
>> +                       if (drm_edid_block_valid(block + (valid_extensions + 1)
>> +                                                * EDID_LENGTH, j,
>> +                                                print_bad_edid,
>> +                                                NULL)) {
>>                                  valid_extensions++;
>>                                  break;
>>                          }
>> diff --git a/drivers/gpu/drm/drm_edid_load.c b/drivers/gpu/drm/drm_edid_load.c
>> index 4c0aa97..48b48e8 100644
>> --- a/drivers/gpu/drm/drm_edid_load.c
>> +++ b/drivers/gpu/drm/drm_edid_load.c
>> @@ -216,7 +216,8 @@ static void *edid_load(struct drm_connector *connector, const char *name,
>>                  goto out;
>>          }
>>
>> -       if (!drm_edid_block_valid(edid, 0, print_bad_edid)) {
>> +       if (!drm_edid_block_valid(edid, 0, print_bad_edid,
>> +                                 &connector->edid_header_corrupt)) {
>>                  connector->bad_edid_counter++;
>>                  DRM_ERROR("Base block of EDID firmware \"%s\" is invalid ",
>>                      name);
>> @@ -229,7 +230,9 @@ static void *edid_load(struct drm_connector *connector, const char *name,
>>                  if (i != valid_extensions + 1)
>>                          memcpy(edid + (valid_extensions + 1) * EDID_LENGTH,
>>                              edid + i * EDID_LENGTH, EDID_LENGTH);
>> -               if (drm_edid_block_valid(edid + i * EDID_LENGTH, i, print_bad_edid))
>> +               if (drm_edid_block_valid(edid + i * EDID_LENGTH, i,
>> +                                        print_bad_edid,
>> +                                        NULL))
>>                          valid_extensions++;
>>          }
>>
>> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>> index 7ca0e18..97224ff 100644
>> --- a/drivers/gpu/drm/i915/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>> @@ -4139,7 +4139,7 @@ intel_dp_check_link_status(struct intel_dp *intel_dp, bool long_hpd)
>>           */
>>          if (long_hpd) {
>>                  edid_read = drm_get_edid(connector, adapter);
>> -               if (!edid_read) {
>> +               if (!edid_read || connector->edid_header_corrupt == 1) {
>>                          DRM_DEBUG_DRIVER("Invalid EDID detected\n");
>>                  } else {
>>                          kfree(edid_read);
>> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
>> index d4e4b82..f7a4a92 100644
>> --- a/include/drm/drm_crtc.h
>> +++ b/include/drm/drm_crtc.h
>> @@ -719,6 +719,11 @@ struct drm_connector {
>>          int null_edid_counter; /* needed to workaround some HW bugs where we get all 0s */
>>          unsigned bad_edid_counter;
>>
>> +       /* Flag for raw EDID header corruption - used in Displayport
>> +        * compliance testing - * Displayport Link CTS Core 1.2 rev1.1 4.2.2.6
>> +        */
>> +       bool edid_header_corrupt;
>> +
>>          struct dentry *debugfs_entry;
>>
>>          struct drm_connector_state *state;
>> @@ -1443,7 +1448,8 @@ extern void drm_set_preferred_mode(struct drm_connector *connector,
>>                                     int hpref, int vpref);
>>
>>   extern int drm_edid_header_is_valid(const u8 *raw_edid);
>> -extern bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid);
>> +extern bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid,
>> +                                bool *header_corrupt);
>>   extern bool drm_edid_is_valid(struct edid *edid);
>>
>>   extern struct drm_tile_group *drm_mode_create_tile_group(struct drm_device *dev,
>> --
>> 1.9.1
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>
>

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCH 05/12] drm: Add supporting structure for Displayport Link CTS test 4.2.2.6
  2015-04-15 15:38 ` [PATCH 05/10] drm: Add supporting structure for Displayport Link CTS test 4.2.2.6 Todd Previte
  2015-04-15 17:15   ` [PATCH 05/12] " Todd Previte
  2015-04-15 22:03   ` Todd Previte
@ 2015-04-16 15:47   ` Todd Previte
  2 siblings, 0 replies; 31+ messages in thread
From: Todd Previte @ 2015-04-16 15:47 UTC (permalink / raw)
  To: intel-gfx; +Cc: dri-devel

Displayport compliance test 4.2.2.6 requires that a source device be capable of
detecting a corrupt EDID. The test specification states that the sink device
sets up the EDID with an invalid checksum. To do this, the sink sets up an
invalid EDID header, expecting the source device to generate the checksum and
compare it to the value stored in the last byte of the block data.

Unfortunately, the DRM EDID reading and parsing functions are actually too good
in this case; the header is fixed before the checksum is computed and thus the
test never sees the invalid checksum. This results in a failure to pass the
compliance test.

To correct this issue, when the EDID code detects that the header is invalid,
a flag is set to indicate that the EDID is corrupted. In this case, it sets
edid_corrupt flag and continues with its fix-up code. In the case of a more
seriously damaged header (fixup score less than the threshold) the code does
 not generate the checksum but does set the edid_corrupt flag.

V2:
- Removed the static bool global
- Added a bool to the drm_connector struct to reaplce the static one for
  holding the status of raw edid header corruption detection
- Modified the function signature of the is_valid function to take an
  additional parameter to store the corruption detected value
- Fixed the other callers of the above is_valid function
V3:
- Updated the commit message to be more clear about what and why this
  patch does what it does.
- Added comment in code to clarify the operations there
- Removed compliance variable and check_link_status update; those
  have been moved to a later patch
- Removed variable assignment from the bottom of the test handler
V4:
- Removed i915 tag from subject line as the patch is not i915-specific
V5:
- Moved code causing a compilation error to this patch where the variable
  is actually declared
- Maintained blank lines / spacing so as to not contaminate the patch
V6:
- Removed extra debug messages
- Added documentation to for the added parameter on drm_edid_block_valid
- Fixed more whitespace issues in check_link_status
- Added a clear of the header_corrupt flag to the end of the test handler
  in intel_dp.c
- Changed the usage of the new function prototype in several places to use
  NULL where it is not needed by compliance testing
V7:
- Updated to account for long_pulse flag propagation
V8:
- Removed clearing of header_corrupt flag from the test handler in intel_dp.c
- Added clearing of header_corrupt flag in the drm_edid_block_valid function
V9:
- Renamed header_corrupt flag to edid_corrupt to more accurately reflect its
  value and purpose
- Updated commit message

Signed-off-by: Todd Previte <tprevite@gmail.com>
Cc: dri-devel@lists.freedesktop.org
---
 drivers/gpu/drm/drm_edid.c      | 30 ++++++++++++++++++++++++------
 drivers/gpu/drm/drm_edid_load.c |  7 +++++--
 drivers/gpu/drm/i915/intel_dp.c |  2 +-
 include/drm/drm_crtc.h          |  8 +++++++-
 4 files changed, 37 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 53bc7a6..69a5a09 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -1041,13 +1041,15 @@ static bool drm_edid_is_zero(const u8 *in_edid, int length)
  * @raw_edid: pointer to raw EDID block
  * @block: type of block to validate (0 for base, extension otherwise)
  * @print_bad_edid: if true, dump bad EDID blocks to the console
+ * @edid_corrupt: if true, the header or checksum is invalid
  *
  * Validate a base or extension EDID block and optionally dump bad blocks to
  * the console.
  *
  * Return: True if the block is valid, false otherwise.
  */
-bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid)
+bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid,
+			  bool *edid_corrupt)
 {
 	u8 csum;
 	struct edid *edid = (struct edid *)raw_edid;
@@ -1060,11 +1062,23 @@ bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid)
 
 	if (block == 0) {
 		int score = drm_edid_header_is_valid(raw_edid);
-		if (score == 8) ;
-		else if (score >= edid_fixup) {
+		if (score == 8) {
+			if (edid_corrupt)
+				*edid_corrupt = 0;
+		} else if (score >= edid_fixup) {
+			/* Displayport Link CTS Core 1.2 rev1.1 test 4.2.2.6
+			 * The corrupt flag needs to be set here otherwise, the 
+			 * fix-up code here will correct the problem, the 
+			 * checksum is correct and the test fails
+			 */
+			if (edid_corrupt)
+				*edid_corrupt = 1;
 			DRM_DEBUG("Fixing EDID header, your hardware may be failing\n");
 			memcpy(raw_edid, edid_header, sizeof(edid_header));
 		} else {
+			if (edid_corrupt) {
+				*edid_corrupt = 1;
+			}
 			goto bad;
 		}
 	}
@@ -1129,7 +1143,7 @@ bool drm_edid_is_valid(struct edid *edid)
 		return false;
 
 	for (i = 0; i <= edid->extensions; i++)
-		if (!drm_edid_block_valid(raw + i * EDID_LENGTH, i, true))
+		if (!drm_edid_block_valid(raw + i * EDID_LENGTH, i, true, NULL))
 			return false;
 
 	return true;
@@ -1232,7 +1246,8 @@ struct edid *drm_do_get_edid(struct drm_connector *connector,
 	for (i = 0; i < 4; i++) {
 		if (get_edid_block(data, block, 0, EDID_LENGTH))
 			goto out;
-		if (drm_edid_block_valid(block, 0, print_bad_edid))
+		if (drm_edid_block_valid(block, 0, print_bad_edid,
+					 &connector->edid_corrupt))
 			break;
 		if (i == 0 && drm_edid_is_zero(block, EDID_LENGTH)) {
 			connector->null_edid_counter++;
@@ -1257,7 +1272,10 @@ struct edid *drm_do_get_edid(struct drm_connector *connector,
 				  block + (valid_extensions + 1) * EDID_LENGTH,
 				  j, EDID_LENGTH))
 				goto out;
-			if (drm_edid_block_valid(block + (valid_extensions + 1) * EDID_LENGTH, j, print_bad_edid)) {
+			if (drm_edid_block_valid(block + (valid_extensions + 1)
+						 * EDID_LENGTH, j,
+						 print_bad_edid,
+						 NULL)) {
 				valid_extensions++;
 				break;
 			}
diff --git a/drivers/gpu/drm/drm_edid_load.c b/drivers/gpu/drm/drm_edid_load.c
index 4c0aa97..c5605fe 100644
--- a/drivers/gpu/drm/drm_edid_load.c
+++ b/drivers/gpu/drm/drm_edid_load.c
@@ -216,7 +216,8 @@ static void *edid_load(struct drm_connector *connector, const char *name,
 		goto out;
 	}
 
-	if (!drm_edid_block_valid(edid, 0, print_bad_edid)) {
+	if (!drm_edid_block_valid(edid, 0, print_bad_edid,
+				  &connector->edid_corrupt)) {
 		connector->bad_edid_counter++;
 		DRM_ERROR("Base block of EDID firmware \"%s\" is invalid ",
 		    name);
@@ -229,7 +230,9 @@ static void *edid_load(struct drm_connector *connector, const char *name,
 		if (i != valid_extensions + 1)
 			memcpy(edid + (valid_extensions + 1) * EDID_LENGTH,
 			    edid + i * EDID_LENGTH, EDID_LENGTH);
-		if (drm_edid_block_valid(edid + i * EDID_LENGTH, i, print_bad_edid))
+		if (drm_edid_block_valid(edid + i * EDID_LENGTH, i,
+					 print_bad_edid,
+					 NULL))
 			valid_extensions++;
 	}
 
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 7ca0e18..d4c0fd3 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4139,7 +4139,7 @@ intel_dp_check_link_status(struct intel_dp *intel_dp, bool long_hpd)
 	 */
 	if (long_hpd) {
 		edid_read = drm_get_edid(connector, adapter);
-		if (!edid_read) {
+		if (!edid_read || connector->edid_corrupt == 1) {
 			DRM_DEBUG_DRIVER("Invalid EDID detected\n");
 		} else {
 			kfree(edid_read);
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index d4e4b82..8bc2724 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -719,6 +719,11 @@ struct drm_connector {
 	int null_edid_counter; /* needed to workaround some HW bugs where we get all 0s */
 	unsigned bad_edid_counter;
 
+	/* Flag for raw EDID header corruption - used in Displayport
+	 * compliance testing - * Displayport Link CTS Core 1.2 rev1.1 4.2.2.6
+	 */
+	bool edid_corrupt;
+
 	struct dentry *debugfs_entry;
 
 	struct drm_connector_state *state;
@@ -1443,7 +1448,8 @@ extern void drm_set_preferred_mode(struct drm_connector *connector,
 				   int hpref, int vpref);
 
 extern int drm_edid_header_is_valid(const u8 *raw_edid);
-extern bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid);
+extern bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid,
+				 bool *edid_corrupt);
 extern bool drm_edid_is_valid(struct edid *edid);
 
 extern struct drm_tile_group *drm_mode_create_tile_group(struct drm_device *dev,
-- 
1.9.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* Re: [PATCH 08/10] drm: Fix for DP CTS test 4.2.2.5 - I2C DEFER handling
  2015-04-15 15:38 ` [PATCH 08/10] drm: Fix for DP CTS test 4.2.2.5 - I2C DEFER handling Todd Previte
@ 2015-04-16 20:23   ` Paulo Zanoni
  0 siblings, 0 replies; 31+ messages in thread
From: Paulo Zanoni @ 2015-04-16 20:23 UTC (permalink / raw)
  To: Todd Previte; +Cc: Intel Graphics Development, DRI Development

2015-04-15 12:38 GMT-03:00 Todd Previte <tprevite@gmail.com>:
> 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 <tprevite@gmail.com>
> Cc: dri-devel@lists.freedesktop.org

Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
(although just counting to 14 probably wouldn't hurt and would make
things simpler)

> ---
>  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(&aux->hw_mutex);
>                 ret = aux->transfer(aux, msg);
>                 mutex_unlock(&aux->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
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel



-- 
Paulo Zanoni
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 09/10] drm/i915: Add debugfs test control files for Displayport compliance testing
  2015-04-15 15:38 ` [PATCH 09/10] drm/i915: Add debugfs test control files for Displayport compliance testing Todd Previte
@ 2015-04-16 21:25   ` Paulo Zanoni
  0 siblings, 0 replies; 31+ messages in thread
From: Paulo Zanoni @ 2015-04-16 21:25 UTC (permalink / raw)
  To: Todd Previte; +Cc: Intel Graphics Development

2015-04-15 12:38 GMT-03:00 Todd Previte <tprevite@gmail.com>:
> This patch adds 3 debugfs files for handling Displayport compliance testing
> and supercedes the previous patches that implemented debugfs support for
> compliance testing. Those patches were:
>
> - [PATCH 04/17] drm/i915: Add debugfs functions for Displayport
>                           compliance testing
> - [PATCH 08/17] drm/i915: Add new debugfs file for Displayport
>                           compliance test control
> - [PATCH 09/17] drm/i915: Add debugfs write and test param parsing
>                           functions for DP test control
>
> This new patch simplifies the debugfs implementation by places a single
> test control value into an individual file. Each file is readable by
> the usersapce application and the test_active file is writable to
> indicate to the kernel when userspace has completed its portion of the
> test sequence.
>
> Replacing the previous files simplifies operation and speeds response
> time for the user app, as it is required to poll on the test_active file
> in order to determine when it needs to begin its operations.
>
> V2:
> - Updated the test active variable name to match the change in
>   the initial patch of the series
> V3:
> - Added a fix in the test_active_write function to prevent a NULL pointer
>   dereference if the encoder on the connector is invalid
>
> Signed-off-by: Todd Previte <tprevite@gmail.com>
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c | 209 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 209 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 2394924..c33d390 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -3937,6 +3937,212 @@ static const struct file_operations i915_display_crc_ctl_fops = {
>         .write = display_crc_ctl_write
>  };
>
> +static ssize_t i915_displayport_test_active_write(struct file *file,
> +                                           const char __user *ubuf,
> +                                           size_t len, loff_t *offp)
> +{
> +       char *input_buffer;
> +       int status = 0;
> +       struct seq_file *m;
> +       struct drm_device *dev;
> +       struct drm_connector *connector;
> +       struct list_head *connector_list;
> +       struct intel_dp *intel_dp;
> +       int val = 0;
> +
> +       m = file->private_data;
> +       if (!m) {
> +               status = -ENODEV;
> +               return status;
> +       }
> +       dev = m->private;
> +
> +       if (!dev) {
> +               status = -ENODEV;
> +               return status;
> +       }
> +       connector_list = &dev->mode_config.connector_list;
> +
> +       if (len == 0)
> +               return 0;
> +
> +       input_buffer = kmalloc(len + 1, GFP_KERNEL);
> +       if (!input_buffer)
> +               return -ENOMEM;

In 2 spots above you use "status = -ENODEV; return status;", but here
you just "return -ENOMEM". I'd be consistent, possibly using the
shorter form in the 3 cases.


> +
> +       if (copy_from_user(input_buffer, ubuf, len)) {
> +               status = -EFAULT;
> +               goto out;
> +       }
> +
> +       input_buffer[len] = '\0';
> +       DRM_DEBUG_DRIVER("Copied %d bytes from user\n", (unsigned int)len);
> +
> +       list_for_each_entry(connector, connector_list, head) {
> +
> +               if (connector->connector_type !=
> +                   DRM_MODE_CONNECTOR_DisplayPort)
> +                       continue;
> +
> +               if (connector->connector_type ==
> +                   DRM_MODE_CONNECTOR_DisplayPort &&
> +                   connector->status == connector_status_connected &&
> +                   connector->encoder != NULL) {
> +                       intel_dp = enc_to_intel_dp(connector->encoder);
> +                       status = kstrtoint(input_buffer, 10, &val);
> +                       if (status < 0)
> +                               goto out;
> +                       DRM_DEBUG_DRIVER("Got %d for test active\n", val);
> +                       /* To prevent erroneous activation of the compliance
> +                        * testing code, only accept an actual value of 1 here
> +                        */
> +                       if (val == 1)
> +                               intel_dp->compliance_test_active = 1;
> +                       else
> +                               intel_dp->compliance_test_active = 0;
> +               }
> +       }
> +out:
> +       kfree(input_buffer);
> +       if (status < 0)
> +               return status;
> +
> +       *offp += len;
> +       return len;
> +}
> +
> +static int i915_displayport_test_active_show(struct seq_file *m, void *data)
> +{
> +       struct drm_device *dev = m->private;
> +       struct drm_connector *connector;
> +       struct list_head *connector_list = &dev->mode_config.connector_list;
> +       struct intel_dp *intel_dp;
> +
> +       if (!dev)
> +               return -ENODEV;
> +
> +       list_for_each_entry(connector, connector_list, head) {
> +
> +               if (connector->connector_type !=
> +                   DRM_MODE_CONNECTOR_DisplayPort)
> +                       continue;
> +
> +               if (connector->status == connector_status_connected &&
> +                   connector->encoder != NULL) {
> +                       intel_dp = enc_to_intel_dp(connector->encoder);
> +                       if (intel_dp->compliance_test_active)
> +                               seq_puts(m, "1");
> +                       else
> +                               seq_puts(m, "0");
> +               } else
> +                       seq_puts(m, "0");
> +       }
> +
> +       return 0;
> +}
> +
> +static int i915_displayport_test_active_open(struct inode *inode,
> +                                      struct file *file)
> +{
> +       struct drm_device *dev = inode->i_private;
> +
> +       return single_open(file, i915_displayport_test_active_show, dev);
> +}
> +
> +static const struct file_operations i915_displayport_test_active_fops = {
> +       .owner = THIS_MODULE,
> +       .open = i915_displayport_test_active_open,
> +       .read = seq_read,
> +       .llseek = seq_lseek,
> +       .release = single_release,
> +       .write = i915_displayport_test_active_write
> +};
> +
> +static int i915_displayport_test_data_show(struct seq_file *m, void *data)
> +{
> +       struct drm_device *dev = m->private;
> +       struct drm_connector *connector;
> +       struct list_head *connector_list = &dev->mode_config.connector_list;
> +       struct intel_dp *intel_dp;
> +
> +       if (!dev)
> +               return -ENODEV;
> +
> +       list_for_each_entry(connector, connector_list, head) {
> +
> +               if (connector->connector_type !=
> +                   DRM_MODE_CONNECTOR_DisplayPort)
> +                       continue;
> +
> +               if (connector->status == connector_status_connected &&
> +                   connector->encoder != NULL) {
> +                       intel_dp = enc_to_intel_dp(connector->encoder);
> +                       seq_printf(m, "%lx", intel_dp->compliance_test_data);
> +               } else
> +                       seq_puts(m, "0");
> +       }
> +
> +       return 0;
> +}
> +static int i915_displayport_test_data_open(struct inode *inode,
> +                                      struct file *file)
> +{
> +       struct drm_device *dev = inode->i_private;
> +
> +       return single_open(file, i915_displayport_test_data_show, dev);
> +}
> +
> +static const struct file_operations i915_displayport_test_data_fops = {
> +       .owner = THIS_MODULE,
> +       .open = i915_displayport_test_data_open,
> +       .read = seq_read,
> +       .llseek = seq_lseek,
> +       .release = single_release
> +};
> +
> +static int i915_displayport_test_type_show(struct seq_file *m, void *data)
> +{
> +       struct drm_device *dev = m->private;
> +       struct drm_connector *connector;
> +       struct list_head *connector_list = &dev->mode_config.connector_list;
> +       struct intel_dp *intel_dp;
> +
> +       if (!dev)
> +               return -ENODEV;
> +
> +       list_for_each_entry(connector, connector_list, head) {
> +
> +               if (connector->connector_type !=
> +                   DRM_MODE_CONNECTOR_DisplayPort)
> +                       continue;
> +
> +               if (connector->status == connector_status_connected &&
> +                   connector->encoder != NULL) {
> +                       intel_dp = enc_to_intel_dp(connector->encoder);
> +                       seq_printf(m, "%02lx", intel_dp->compliance_test_type);
> +               } else
> +                       seq_puts(m, "0");
> +       }
> +
> +       return 0;
> +}
> +
> +static int i915_displayport_test_type_open(struct inode *inode,
> +                                      struct file *file)
> +{
> +       struct drm_device *dev = inode->i_private;
> +
> +       return single_open(file, i915_displayport_test_type_show, dev);
> +}
> +
> +static const struct file_operations i915_displayport_test_type_fops = {
> +       .owner = THIS_MODULE,
> +       .open = i915_displayport_test_type_open,
> +       .read = seq_read,
> +       .llseek = seq_lseek,
> +       .release = single_release
> +};
> +
>  static void wm_latency_show(struct seq_file *m, const uint16_t wm[8])
>  {
>         struct drm_device *dev = m->private;
> @@ -4832,6 +5038,9 @@ static const struct i915_debugfs_files {
>         {"i915_spr_wm_latency", &i915_spr_wm_latency_fops},
>         {"i915_cur_wm_latency", &i915_cur_wm_latency_fops},
>         {"i915_fbc_false_color", &i915_fbc_fc_fops},
> +       {"i915_dp_test_data", &i915_displayport_test_data_fops},
> +       {"i915_dp_test_type", &i915_displayport_test_type_fops},

Since both test_data and test_type are simpler, you can put them on
i915_debugfs_list instead of i915_debugfs_files. This will allow the
removal of the 2 _open functions and the 2 _fops structs, making your
patch ~30 lines smaller.

Since the stuff above is not required for the files to work:
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

> +       {"i915_dp_test_active", &i915_displayport_test_active_fops}
>  };
>
>  void intel_display_crc_init(struct drm_device *dev)
> --
> 1.9.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Paulo Zanoni
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 31+ messages in thread

end of thread, other threads:[~2015-04-16 21:25 UTC | newest]

Thread overview: 31+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-04-15 15:38 [PATCH V6] Displayport compliance testing V6 Todd Previte
2015-04-15 15:38 ` [PATCH 01/10] drm/i915: Add automated testing support for Displayport compliance testing Todd Previte
2015-04-16 11:32   ` Daniel Vetter
2015-04-15 15:38 ` [PATCH 02/10] drm/i915: Update intel_dp_check_link_status() " Todd Previte
2015-04-15 19:28   ` [PATCH 02/12] " Todd Previte
2015-04-15 19:45     ` Paulo Zanoni
2015-04-15 15:38 ` [PATCH 03/10] drm/i915: Add EDID read in intel_dp_check_link_status() for Link CTS 4.2.2.1 Todd Previte
2015-04-15 17:15   ` [PATCH 03/12] " Todd Previte
2015-04-15 19:29   ` Todd Previte
2015-04-15 15:38 ` [PATCH 04/10] drm/i915: Add a delay in Displayport AUX transactions for compliance testing Todd Previte
2015-04-16 12:37   ` Daniel Vetter
2015-04-15 15:38 ` [PATCH 05/10] drm: Add supporting structure for Displayport Link CTS test 4.2.2.6 Todd Previte
2015-04-15 17:15   ` [PATCH 05/12] " Todd Previte
2015-04-15 20:25     ` [Intel-gfx] " Paulo Zanoni
2015-04-15 21:59       ` Todd Previte
2015-04-15 22:03   ` Todd Previte
2015-04-16 13:34     ` Paulo Zanoni
2015-04-16 14:02       ` Todd Previte
2015-04-16 15:47   ` Todd Previte
2015-04-15 15:38 ` [PATCH 06/10] drm/i915: Update intel_dp_hpd_pulse() for non-MST operation Todd Previte
2015-04-15 17:15   ` [PATCH 06/12] " Todd Previte
2015-04-16  7:22   ` Todd Previte
2015-04-15 15:38 ` [PATCH 07/10] drm/i915: Support EDID compliance tests with the intel_dp_autotest_edid() function Todd Previte
2015-04-15 17:16   ` [PATCH 07/12] " Todd Previte
2015-04-15 15:38 ` [PATCH 08/10] drm: Fix for DP CTS test 4.2.2.5 - I2C DEFER handling Todd Previte
2015-04-16 20:23   ` Paulo Zanoni
2015-04-15 15:38 ` [PATCH 09/10] drm/i915: Add debugfs test control files for Displayport compliance testing Todd Previte
2015-04-16 21:25   ` Paulo Zanoni
2015-04-15 15:38 ` [PATCH 10/10] drm: Fix the 'native defer' message in drm_dp_i2c_do_msg() Todd Previte
2015-04-16 12:42   ` Daniel Vetter
2015-04-16 12:44   ` Daniel Vetter

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.