[PATCH V7] Displayport compliance testing V7

[PATCH V7] Displayport compliance testing V7

[Todd Previte]

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)

Changes for V7:
- Integrates necessary changes from feedback in previous reviews
- Series does not include previous patches that have been accepted for merge
- Eliminates the need to modify intel_dp_hpd_pulse and now sits in the normal
  HPD path through the driver
- Some patch number swizzling to accommodate the merged and removed patches
- Renamed some patches to be more clear about what they do.

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

[PATCH 1/5] drm/i915: Move Displayport test request and sink IRQ logic to intel_dp_detect()

[Todd Previte]

Due to changes in the driver and to support Displayport compliance testing,
the test request and sink IRQ logic has been relocated from
intel_dp_check_link_status to intel_dp_detect. The reason for this is that
this handler should be called during the normal hot plug path for a long
pulse / hot plug event only. The intel_dp_check_link_status function is
called for short pulses from intel_dp_hpd_pulse().

This allows for the proper detection of Displayport compliance test requests
and the invocation of the test handler to support them. Additionally, this
places compliance testing in the normal operational paths, eliminating as
much special case code as possible.

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

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 2ef05d7..55d1f5f 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4123,7 +4123,6 @@ 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;
-	u8 sink_irq_vector;
 	u8 link_status[DP_LINK_STATUS_SIZE];
 
 	WARN_ON(!drm_modeset_is_locked(&dev->mode_config.connection_mutex));
@@ -4147,20 +4146,6 @@ intel_dp_check_link_status(struct intel_dp *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);
@@ -4386,6 +4371,7 @@ intel_dp_detect(struct drm_connector *connector, bool force)
 	enum drm_connector_status status;
 	enum intel_display_power_domain power_domain;
 	bool ret;
+	u8 sink_irq_vector;
 
 	DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n",
 		      connector->base.id, connector->name);
@@ -4428,6 +4414,19 @@ intel_dp_detect(struct drm_connector *connector, bool force)
 		intel_encoder->type = INTEL_OUTPUT_DISPLAYPORT;
 	status = connector_status_connected;
 
+	/* Check for sink IRQ and determine cause */
+	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");
+	}
+
 out:
 	intel_dp_power_put(intel_dp, power_domain);
 	return status;
-- 
1.9.1

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

[PATCH 2/5] drm: Add edid_corrupt flag for Displayport Link CTS 4.2.2.6

[Todd Previte]

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

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

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

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

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

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 53bc7a6..69a5a09 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -1041,13 +1041,15 @@ static bool drm_edid_is_zero(const u8 *in_edid, int length)
  * @raw_edid: pointer to raw EDID block
  * @block: type of block to validate (0 for base, extension otherwise)
  * @print_bad_edid: if true, dump bad EDID blocks to the console
+ * @edid_corrupt: if true, the header or checksum is invalid
  *
  * Validate a base or extension EDID block and optionally dump bad blocks to
  * the console.
  *
  * Return: True if the block is valid, false otherwise.
  */
-bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid)
+bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid,
+			  bool *edid_corrupt)
 {
 	u8 csum;
 	struct edid *edid = (struct edid *)raw_edid;
@@ -1060,11 +1062,23 @@ bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid)
 
 	if (block == 0) {
 		int score = drm_edid_header_is_valid(raw_edid);
-		if (score == 8) ;
-		else if (score >= edid_fixup) {
+		if (score == 8) {
+			if (edid_corrupt)
+				*edid_corrupt = 0;
+		} else if (score >= edid_fixup) {
+			/* Displayport Link CTS Core 1.2 rev1.1 test 4.2.2.6
+			 * The corrupt flag needs to be set here otherwise, the 
+			 * fix-up code here will correct the problem, the 
+			 * checksum is correct and the test fails
+			 */
+			if (edid_corrupt)
+				*edid_corrupt = 1;
 			DRM_DEBUG("Fixing EDID header, your hardware may be failing\n");
 			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/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

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

[PATCH 3/5] drm/i915: Implement the intel_dp_autotest_edid function for DP EDID complaince tests

[Todd Previte]

Updates the EDID compliance test function to perform the analyze and react to
the EDID data read as a result of a hot plug event. The results of this
analysis are handed off to userspace so that the userspace app can set the
display mode appropriately for the test result/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
V10:
- Updated to use actual checksum from the EDID read that occurs during
  normal hot plug path execution
- Removed variables from intel_dp struct that are no longer needed
- Updated the patch subject to more closely match the nature and contents
  of the patch

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

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 55d1f5f..4c1f6a9 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,38 @@ 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;
+	struct intel_connector *intel_connector = intel_dp->attached_connector;
+	struct drm_connector *connector = &intel_connector->base;
+
+	if (intel_connector->detect_edid == NULL ||
+	    connector->edid_corrupt == 1 ||
+	    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_connector->detect_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;
 }
 
@@ -4009,7 +4047,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;
 
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index a4675fa..6c71be9 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -742,6 +742,8 @@ struct intel_dp {
 
 	/* Displayport compliance testing */
 	unsigned long compliance_test_type;
+	unsigned long compliance_test_data;
+	bool compliance_test_active;
 };
 
 struct intel_digital_port {
-- 
1.9.1

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

[PATCH 4/5] drm: Fix for DP CTS test 4.2.2.5 - I2C DEFER handling

[Todd Previte]

For test 4.2.2.5 to pass per the Link CTS Core 1.2 rev1.1 spec, the source
device must attempt at least 7 times to read the EDID when it receives an
I2C defer. The normal DRM code makes only 7 retries, regardless of whether
or not the response is a native defer or an I2C defer. Test 4.2.2.5 fails
since there are native defers interspersed with the I2C defers which
results in less than 7 EDID read attempts.

The solution is to 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>
---
 drivers/gpu/drm/drm_dp_helper.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index 575172e..80a02a4 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -432,7 +432,7 @@ static u32 drm_dp_i2c_functionality(struct i2c_adapter *adapter)
  */
 static int drm_dp_i2c_do_msg(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
 {
-	unsigned int retry;
+	unsigned int retry, defer_i2c;
 	int ret;
 
 	/*
@@ -440,7 +440,7 @@ static int drm_dp_i2c_do_msg(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
 	 * is required to retry at least seven times upon receiving AUX_DEFER
 	 * before giving up the AUX transaction.
 	 */
-	for (retry = 0; retry < 7; retry++) {
+	for (retry = 0, defer_i2c = 0; retry < (7 + defer_i2c); retry++) {
 		mutex_lock(&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

[PATCH 5/5] drm/i915: Add debugfs test control files for Displayport compliance testing

[Todd Previte]

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>
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.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

[PATCH] drm/i915: Move Displayport test request and sink IRQ logic to intel_dp_detect()

[Todd Previte]

Due to changes in the driver and to support Displayport compliance testing,
the test request and sink IRQ logic has been relocated from
intel_dp_check_link_status to intel_dp_detect. The reason for this is that
this handler should be called during the normal hot plug path for a long
pulse / hot plug event only. The intel_dp_check_link_status function is
called for short pulses from intel_dp_hpd_pulse().

This allows for the proper detection of Displayport compliance test requests
and the invocation of the test handler to support them. Additionally, this
places compliance testing in the normal operational paths, eliminating as
much special case code as possible.

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

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 2ef05d7..55d1f5f 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4123,7 +4123,6 @@ 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;
-	u8 sink_irq_vector;
 	u8 link_status[DP_LINK_STATUS_SIZE];
 
 	WARN_ON(!drm_modeset_is_locked(&dev->mode_config.connection_mutex));
@@ -4147,20 +4146,6 @@ intel_dp_check_link_status(struct intel_dp *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);
@@ -4386,6 +4371,7 @@ intel_dp_detect(struct drm_connector *connector, bool force)
 	enum drm_connector_status status;
 	enum intel_display_power_domain power_domain;
 	bool ret;
+	u8 sink_irq_vector;
 
 	DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n",
 		      connector->base.id, connector->name);
@@ -4428,6 +4414,19 @@ intel_dp_detect(struct drm_connector *connector, bool force)
 		intel_encoder->type = INTEL_OUTPUT_DISPLAYPORT;
 	status = connector_status_connected;
 
+	/* Check for sink IRQ and determine cause */
+	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");
+	}
+
 out:
 	intel_dp_power_put(intel_dp, power_domain);
 	return status;
-- 
1.9.1

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

Re: [PATCH] drm/i915: Move Displayport test request and sink IRQ logic to intel_dp_detect()

[shuang.he]

Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 6232
-------------------------------------Summary-------------------------------------
Platform          Delta          drm-intel-nightly          Series Applied
PNV                                  276/276              276/276
ILK                                  302/302              302/302
SNB                                  318/318              318/318
IVB                                  341/341              341/341
BYT                                  287/287              287/287
BDW                                  318/318              318/318
-------------------------------------Detailed-------------------------------------
Platform  Test                                drm-intel-nightly          Series Applied
Note: You need to pay more attention to line start with '*'
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 4/5] drm: Fix for DP CTS test 4.2.2.5 - I2C DEFER handling

[Daniel Vetter]

On Sat, Apr 18, 2015 at 12:04:18AM -0700, Todd Previte wrote:
> 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>

Applied to topic/drm-misc.
-Daniel

> ---
>  drivers/gpu/drm/drm_dp_helper.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
> index 575172e..80a02a4 100644
> --- a/drivers/gpu/drm/drm_dp_helper.c
> +++ b/drivers/gpu/drm/drm_dp_helper.c
> @@ -432,7 +432,7 @@ static u32 drm_dp_i2c_functionality(struct i2c_adapter *adapter)
>   */
>  static int drm_dp_i2c_do_msg(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
>  {
> -	unsigned int retry;
> +	unsigned int retry, defer_i2c;
>  	int ret;
>  
>  	/*
> @@ -440,7 +440,7 @@ static int drm_dp_i2c_do_msg(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
>  	 * is required to retry at least seven times upon receiving AUX_DEFER
>  	 * before giving up the AUX transaction.
>  	 */
> -	for (retry = 0; retry < 7; retry++) {
> +	for (retry = 0, defer_i2c = 0; retry < (7 + defer_i2c); retry++) {
>  		mutex_lock(&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

-- 
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

[PATCH 4/8] drm/i915: Move Displayport test request and sink IRQ logic to intel_dp_detect()

[Todd Previte]

Due to changes in the driver and to support Displayport compliance testing,
the test request and sink IRQ logic has been relocated from
intel_dp_check_link_status to intel_dp_detect. This is because the bulk of the
compliance tests that set the TEST_REQUEST bit in the DEVICE_IRQ field of the
DPCD issue a long pulse / hot plug event to signify the start of the test.
Currently, for a long pulse, intel_dp_check_link_status is not called for a
long HPD pulse, so if test requests come in, they cannot be detected by the
driver.

Once located in the intel_dp_detect, in the regular hot plug event path,
proper detection of Displayport compliance test requests occurs which then
invokes the test handler to support them. Additionally, this places compliance
testing in the normal operational paths, eliminating as much special case code
as possible.

The only change in intel_dp_check_link_status with this patch is that when
the IRQ is the result of a test request from the sink, the test handler is not
invoked during the short pulse path. Short pulse test requests are for a
particular variety of tests (mainly link training) that will be implemented
in the future. Once those tests are available, the test request handler will
be called from here as well.

V1:
- Rewored the commit message to be more clear about the content and intent
  of this patch
- Restore IRQ detection logic to intel_dp_check_link_status(). Continue to
  detect and clear sink IRQs in the short pulse case. Ignore test requests
  in the short pulses for now since they are for future test implementations.

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

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 2ef05d7..b249ee8 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4156,7 +4156,7 @@ intel_dp_check_link_status(struct intel_dp *intel_dp)
 				   sink_irq_vector);
 
 		if (sink_irq_vector & DP_AUTOMATED_TEST_REQUEST)
-			intel_dp_handle_test_request(intel_dp);
+			DRM_DEBUG_DRIVER("Test request in short pulse not handled\n");
 		if (sink_irq_vector & (DP_CP_IRQ | DP_SINK_SPECIFIC_IRQ))
 			DRM_DEBUG_DRIVER("CP or sink specific irq unhandled\n");
 	}
@@ -4386,6 +4386,7 @@ intel_dp_detect(struct drm_connector *connector, bool force)
 	enum drm_connector_status status;
 	enum intel_display_power_domain power_domain;
 	bool ret;
+	u8 sink_irq_vector;
 
 	DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n",
 		      connector->base.id, connector->name);
@@ -4428,6 +4429,20 @@ intel_dp_detect(struct drm_connector *connector, bool force)
 		intel_encoder->type = INTEL_OUTPUT_DISPLAYPORT;
 	status = connector_status_connected;
 
+	/* 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");
+	}
+
 out:
 	intel_dp_power_put(intel_dp, power_domain);
 	return status;
-- 
1.9.1

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

[PATCH 2/5] drm: Add edid_corrupt flag for Displayport Link CTS 4.2.2.6

[Todd Previte]

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

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

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

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

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

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 53bc7a6..be6713c 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -1041,13 +1041,15 @@ static bool drm_edid_is_zero(const u8 *in_edid, int length)
  * @raw_edid: pointer to raw EDID block
  * @block: type of block to validate (0 for base, extension otherwise)
  * @print_bad_edid: if true, dump bad EDID blocks to the console
+ * @edid_corrupt: if true, the header or checksum is invalid
  *
  * Validate a base or extension EDID block and optionally dump bad blocks to
  * the console.
  *
  * Return: True if the block is valid, false otherwise.
  */
-bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid)
+bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid,
+			  bool *edid_corrupt)
 {
 	u8 csum;
 	struct edid *edid = (struct edid *)raw_edid;
@@ -1060,11 +1062,22 @@ bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid)
 
 	if (block == 0) {
 		int score = drm_edid_header_is_valid(raw_edid);
-		if (score == 8) ;
-		else if (score >= edid_fixup) {
+		if (score == 8) {
+			if (edid_corrupt)
+				*edid_corrupt = 0;
+		} else if (score >= edid_fixup) {
+			/* Displayport Link CTS Core 1.2 rev1.1 test 4.2.2.6
+			 * The corrupt flag needs to be set here otherwise, the
+			 * fix-up code here will correct the problem, the
+			 * checksum is correct and the test fails
+			 */
+			if (edid_corrupt)
+				*edid_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;
 		}
 	}
@@ -1075,6 +1088,9 @@ bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid)
 			DRM_ERROR("EDID checksum is invalid, remainder is %d\n", csum);
 		}
 
+		if (edid_corrupt)
+			*edid_corrupt = 1;
+
 		/* allow CEA to slide through, switches mangle this */
 		if (raw_edid[0] != 0x02)
 			goto bad;
@@ -1129,7 +1145,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 +1248,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 +1274,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/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

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

Re: [PATCH 2/5] drm: Add edid_corrupt flag for Displayport Link CTS 4.2.2.6

[Alex Deucher]

On Tue, Apr 21, 2015 at 2:09 PM, Todd Previte <tprevite@gmail.com> wrote:
> Displayport compliance test 4.2.2.6 requires that a source device be capable of
> detecting a corrupt EDID. The test specification states that the sink device
> sets up the EDID with an invalid checksum. To do this, the sink sets up an
> invalid EDID header, expecting the source device to generate the checksum and
> compare it to the value stored in the last byte of the block data.
>
> Unfortunately, the DRM EDID reading and parsing functions are actually too good
> in this case; the header is fixed before the checksum is computed and thus the
> test never sees the invalid checksum. This results in a failure to pass the
> compliance test.
>
> To correct this issue, when the EDID code detects that the header is invalid,
> a flag is set to indicate that the EDID is corrupted. In this case, it sets
> edid_corrupt flag and continues with its fix-up code. This flag is also set in
> the case of a more seriously damaged header (fixup score less than the
> threshold). For consistency, the edid_corrupt flag is also set when the
> checksum is invalid as well.
>
> V2:
> - Removed the static bool global
> - Added a bool to the drm_connector struct to reaplce the static one for
>   holding the status of raw edid header corruption detection
> - Modified the function signature of the is_valid function to take an
>   additional parameter to store the corruption detected value
> - Fixed the other callers of the above is_valid function
> V3:
> - Updated the commit message to be more clear about what and why this
>   patch does what it does.
> - Added comment in code to clarify the operations there
> - Removed compliance variable and check_link_status update; those
>   have been moved to a later patch
> - Removed variable assignment from the bottom of the test handler
> V4:
> - Removed i915 tag from subject line as the patch is not i915-specific
> V5:
> - Moved code causing a compilation error to this patch where the variable
>   is actually declared
> - Maintained blank lines / spacing so as to not contaminate the patch
> V6:
> - Removed extra debug messages
> - Added documentation to for the added parameter on drm_edid_block_valid
> - Fixed more whitespace issues in check_link_status
> - Added a clear of the header_corrupt flag to the end of the test handler
>   in intel_dp.c
> - Changed the usage of the new function prototype in several places to use
>   NULL where it is not needed by compliance testing
> V7:
> - Updated to account for long_pulse flag propagation
> V8:
> - Removed clearing of header_corrupt flag from the test handler in intel_dp.c
> - Added clearing of header_corrupt flag in the drm_edid_block_valid function
> V9:
> - Renamed header_corrupt flag to edid_corrupt to more accurately reflect its
>   value and purpose
> - Updated commit message
> V10:
> - Updated for versioning and patch swizzle
> - Revised the title to more accurately reflect the nature and contents of
>   the patch
> - Fixed formatting/whitespace problems
> - Added set flag when computed checksum is invalid
>
> Signed-off-by: Todd Previte <tprevite@gmail.com>
> Cc: dri-devel@lists.freedesktop.org

Seems reasonable.

Reviewed-by: Alex Deucher <alexander.deucher@amd.com>

> ---
>  drivers/gpu/drm/drm_edid.c      | 32 ++++++++++++++++++++++++++------
>  drivers/gpu/drm/drm_edid_load.c |  7 +++++--
>  include/drm/drm_crtc.h          |  8 +++++++-
>  3 files changed, 38 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 53bc7a6..be6713c 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -1041,13 +1041,15 @@ static bool drm_edid_is_zero(const u8 *in_edid, int length)
>   * @raw_edid: pointer to raw EDID block
>   * @block: type of block to validate (0 for base, extension otherwise)
>   * @print_bad_edid: if true, dump bad EDID blocks to the console
> + * @edid_corrupt: if true, the header or checksum is invalid
>   *
>   * Validate a base or extension EDID block and optionally dump bad blocks to
>   * the console.
>   *
>   * Return: True if the block is valid, false otherwise.
>   */
> -bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid)
> +bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid,
> +                         bool *edid_corrupt)
>  {
>         u8 csum;
>         struct edid *edid = (struct edid *)raw_edid;
> @@ -1060,11 +1062,22 @@ bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid)
>
>         if (block == 0) {
>                 int score = drm_edid_header_is_valid(raw_edid);
> -               if (score == 8) ;
> -               else if (score >= edid_fixup) {
> +               if (score == 8) {
> +                       if (edid_corrupt)
> +                               *edid_corrupt = 0;
> +               } else if (score >= edid_fixup) {
> +                       /* Displayport Link CTS Core 1.2 rev1.1 test 4.2.2.6
> +                        * The corrupt flag needs to be set here otherwise, the
> +                        * fix-up code here will correct the problem, the
> +                        * checksum is correct and the test fails
> +                        */
> +                       if (edid_corrupt)
> +                               *edid_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;
>                 }
>         }
> @@ -1075,6 +1088,9 @@ bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid)
>                         DRM_ERROR("EDID checksum is invalid, remainder is %d\n", csum);
>                 }
>
> +               if (edid_corrupt)
> +                       *edid_corrupt = 1;
> +
>                 /* allow CEA to slide through, switches mangle this */
>                 if (raw_edid[0] != 0x02)
>                         goto bad;
> @@ -1129,7 +1145,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 +1248,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 +1274,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/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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 5/5] drm/i915: Add debugfs test control files for Displayport compliance testing

[shuang.he]

Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 6227
-------------------------------------Summary-------------------------------------
Platform          Delta          drm-intel-nightly          Series Applied
PNV                 -1              276/276              275/276
ILK                                  301/301              301/301
SNB                                  318/318              318/318
IVB                                  345/345              345/345
BYT                 -2              285/285              283/285
BDW                                  321/321              321/321
-------------------------------------Detailed-------------------------------------
Platform  Test                                drm-intel-nightly          Series Applied
*PNV  igt@gen3_render_tiledy_blits      PASS(2)      NO_RESULT(1)PASS(1)
*BYT  igt@gem_reloc_overflow@source-offset-negative-reloc-gtt      PASS(2)      NO_RESULT(1)PASS(1)
*BYT  igt@kms_setmode@invalid-clone-exclusive-crtc      PASS(5)      DMESG_WARN(1)PASS(1)
(dmesg patch applied)drm:check_crtc_state[i915]]*ERROR*mismatch_in_has_infoframe(expected#,found#)@mismatch in has_infoframe .* found
WARNING:at_drivers/gpu/drm/i915/intel_display.c:#check_crtc_state[i915]()@WARNING:.* at .* check_crtc_state+0x
Note: You need to pay more attention to line start with '*'
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 4/8] drm/i915: Move Displayport test request and sink IRQ logic to intel_dp_detect()

[Paulo Zanoni]

2015-04-20 19:27 GMT-03:00 Todd Previte <tprevite@gmail.com>:
> Due to changes in the driver and to support Displayport compliance testing,
> the test request and sink IRQ logic has been relocated from
> intel_dp_check_link_status to intel_dp_detect. This is because the bulk of the
> compliance tests that set the TEST_REQUEST bit in the DEVICE_IRQ field of the
> DPCD issue a long pulse / hot plug event to signify the start of the test.
> Currently, for a long pulse, intel_dp_check_link_status is not called for a
> long HPD pulse, so if test requests come in, they cannot be detected by the
> driver.
>
> Once located in the intel_dp_detect, in the regular hot plug event path,
> proper detection of Displayport compliance test requests occurs which then
> invokes the test handler to support them. Additionally, this places compliance
> testing in the normal operational paths, eliminating as much special case code
> as possible.
>
> The only change in intel_dp_check_link_status with this patch is that when
> the IRQ is the result of a test request from the sink, the test handler is not
> invoked during the short pulse path. Short pulse test requests are for a
> particular variety of tests (mainly link training) that will be implemented
> in the future. Once those tests are available, the test request handler will
> be called from here as well.
>
> V1:
> - Rewored the commit message to be more clear about the content and intent
>   of this patch
> - Restore IRQ detection logic to intel_dp_check_link_status(). Continue to
>   detect and clear sink IRQs in the short pulse case. Ignore test requests
>   in the short pulses for now since they are for future test implementations.

It took me a a while to realize the "duplicate" code paths are not
really duplicate because of the short vs long thing. But this looks
correct so: Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

But, later, we could certainly use a cleanup on intel_dp_hpd_pulse().

>
> Signed-off-by: Todd Previte <tprevite@gmail.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c | 17 ++++++++++++++++-
>  1 file changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 2ef05d7..b249ee8 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -4156,7 +4156,7 @@ intel_dp_check_link_status(struct intel_dp *intel_dp)
>                                    sink_irq_vector);
>
>                 if (sink_irq_vector & DP_AUTOMATED_TEST_REQUEST)
> -                       intel_dp_handle_test_request(intel_dp);
> +                       DRM_DEBUG_DRIVER("Test request in short pulse not handled\n");
>                 if (sink_irq_vector & (DP_CP_IRQ | DP_SINK_SPECIFIC_IRQ))
>                         DRM_DEBUG_DRIVER("CP or sink specific irq unhandled\n");
>         }
> @@ -4386,6 +4386,7 @@ intel_dp_detect(struct drm_connector *connector, bool force)
>         enum drm_connector_status status;
>         enum intel_display_power_domain power_domain;
>         bool ret;
> +       u8 sink_irq_vector;
>
>         DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n",
>                       connector->base.id, connector->name);
> @@ -4428,6 +4429,20 @@ intel_dp_detect(struct drm_connector *connector, bool force)
>                 intel_encoder->type = INTEL_OUTPUT_DISPLAYPORT;
>         status = connector_status_connected;
>
> +       /* 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");
> +       }
> +
>  out:
>         intel_dp_power_put(intel_dp, power_domain);
>         return status;
> --
> 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

Re: [PATCH 3/5] drm/i915: Implement the intel_dp_autotest_edid function for DP EDID complaince tests

[Paulo Zanoni]

2015-04-18 4:04 GMT-03:00 Todd Previte <tprevite@gmail.com>:
> Updates the EDID compliance test function to perform the analyze and react to
> the EDID data read as a result of a hot plug event. The results of this
> analysis are handed off to userspace so that the userspace app can set the
> display mode appropriately for the test result/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
> V10:
> - Updated to use actual checksum from the EDID read that occurs during
>   normal hot plug path execution
> - Removed variables from intel_dp struct that are no longer needed
> - Updated the patch subject to more closely match the nature and contents
>   of the patch
>
> Signed-off-by: Todd Previte <tprevite@gmail.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c  | 41 ++++++++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_drv.h |  2 ++
>  2 files changed, 43 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 55d1f5f..4c1f6a9 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

Now that you cleaned all the other stuff, can't we start at 0 instead
of 4? (of course, this would require a change to the user space tool)


> +#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,38 @@ 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;
> +       struct intel_connector *intel_connector = intel_dp->attached_connector;
> +       struct drm_connector *connector = &intel_connector->base;
> +
> +       if (intel_connector->detect_edid == NULL ||
> +           connector->edid_corrupt == 1 ||
> +           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_connector->detect_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");

You're mixing DRM_DEBUG_DRIVER and DRM_DEBUG_KMS. I'd stick with only
one, and I'd probably pick _KMS.

With or without the above bikesheds: Reviewed-by: Paulo Zanoni
<paulo.r.zanoni@intel.com>

> +
> +               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;
>  }
>
> @@ -4009,7 +4047,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;
>
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index a4675fa..6c71be9 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -742,6 +742,8 @@ struct intel_dp {
>
>         /* Displayport compliance testing */
>         unsigned long compliance_test_type;
> +       unsigned long compliance_test_data;
> +       bool compliance_test_active;
>  };
>
>  struct intel_digital_port {
> --
> 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

Re: [PATCH 2/5] drm: Add edid_corrupt flag for Displayport Link CTS 4.2.2.6

[Paulo Zanoni]

2015-04-22 17:20 GMT-03:00 Alex Deucher <alexdeucher@gmail.com>:
> On Tue, Apr 21, 2015 at 2:09 PM, Todd Previte <tprevite@gmail.com> wrote:
>> Displayport compliance test 4.2.2.6 requires that a source device be capable of
>> detecting a corrupt EDID. The test specification states that the sink device
>> sets up the EDID with an invalid checksum. To do this, the sink sets up an
>> invalid EDID header, expecting the source device to generate the checksum and
>> compare it to the value stored in the last byte of the block data.
>>
>> Unfortunately, the DRM EDID reading and parsing functions are actually too good
>> in this case; the header is fixed before the checksum is computed and thus the
>> test never sees the invalid checksum. This results in a failure to pass the
>> compliance test.
>>
>> To correct this issue, when the EDID code detects that the header is invalid,
>> a flag is set to indicate that the EDID is corrupted. In this case, it sets
>> edid_corrupt flag and continues with its fix-up code. This flag is also set in
>> the case of a more seriously damaged header (fixup score less than the
>> threshold). For consistency, the edid_corrupt flag is also set when the
>> checksum is invalid as well.
>>
>> V2:
>> - Removed the static bool global
>> - Added a bool to the drm_connector struct to reaplce the static one for
>>   holding the status of raw edid header corruption detection
>> - Modified the function signature of the is_valid function to take an
>>   additional parameter to store the corruption detected value
>> - Fixed the other callers of the above is_valid function
>> V3:
>> - Updated the commit message to be more clear about what and why this
>>   patch does what it does.
>> - Added comment in code to clarify the operations there
>> - Removed compliance variable and check_link_status update; those
>>   have been moved to a later patch
>> - Removed variable assignment from the bottom of the test handler
>> V4:
>> - Removed i915 tag from subject line as the patch is not i915-specific
>> V5:
>> - Moved code causing a compilation error to this patch where the variable
>>   is actually declared
>> - Maintained blank lines / spacing so as to not contaminate the patch
>> V6:
>> - Removed extra debug messages
>> - Added documentation to for the added parameter on drm_edid_block_valid
>> - Fixed more whitespace issues in check_link_status
>> - Added a clear of the header_corrupt flag to the end of the test handler
>>   in intel_dp.c
>> - Changed the usage of the new function prototype in several places to use
>>   NULL where it is not needed by compliance testing
>> V7:
>> - Updated to account for long_pulse flag propagation
>> V8:
>> - Removed clearing of header_corrupt flag from the test handler in intel_dp.c
>> - Added clearing of header_corrupt flag in the drm_edid_block_valid function
>> V9:
>> - Renamed header_corrupt flag to edid_corrupt to more accurately reflect its
>>   value and purpose
>> - Updated commit message
>> V10:
>> - Updated for versioning and patch swizzle
>> - Revised the title to more accurately reflect the nature and contents of
>>   the patch
>> - Fixed formatting/whitespace problems
>> - Added set flag when computed checksum is invalid
>>
>> Signed-off-by: Todd Previte <tprevite@gmail.com>
>> Cc: dri-devel@lists.freedesktop.org
>
> Seems reasonable.
>
> Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
(since I made comments on the previous versions, this is my
documentation that the patch looks fine to me now)

>
>> ---
>>  drivers/gpu/drm/drm_edid.c      | 32 ++++++++++++++++++++++++++------
>>  drivers/gpu/drm/drm_edid_load.c |  7 +++++--
>>  include/drm/drm_crtc.h          |  8 +++++++-
>>  3 files changed, 38 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
>> index 53bc7a6..be6713c 100644
>> --- a/drivers/gpu/drm/drm_edid.c
>> +++ b/drivers/gpu/drm/drm_edid.c
>> @@ -1041,13 +1041,15 @@ static bool drm_edid_is_zero(const u8 *in_edid, int length)
>>   * @raw_edid: pointer to raw EDID block
>>   * @block: type of block to validate (0 for base, extension otherwise)
>>   * @print_bad_edid: if true, dump bad EDID blocks to the console
>> + * @edid_corrupt: if true, the header or checksum is invalid
>>   *
>>   * Validate a base or extension EDID block and optionally dump bad blocks to
>>   * the console.
>>   *
>>   * Return: True if the block is valid, false otherwise.
>>   */
>> -bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid)
>> +bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid,
>> +                         bool *edid_corrupt)
>>  {
>>         u8 csum;
>>         struct edid *edid = (struct edid *)raw_edid;
>> @@ -1060,11 +1062,22 @@ bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid)
>>
>>         if (block == 0) {
>>                 int score = drm_edid_header_is_valid(raw_edid);
>> -               if (score == 8) ;
>> -               else if (score >= edid_fixup) {
>> +               if (score == 8) {
>> +                       if (edid_corrupt)
>> +                               *edid_corrupt = 0;
>> +               } else if (score >= edid_fixup) {
>> +                       /* Displayport Link CTS Core 1.2 rev1.1 test 4.2.2.6
>> +                        * The corrupt flag needs to be set here otherwise, the
>> +                        * fix-up code here will correct the problem, the
>> +                        * checksum is correct and the test fails
>> +                        */
>> +                       if (edid_corrupt)
>> +                               *edid_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;
>>                 }
>>         }
>> @@ -1075,6 +1088,9 @@ bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid)
>>                         DRM_ERROR("EDID checksum is invalid, remainder is %d\n", csum);
>>                 }
>>
>> +               if (edid_corrupt)
>> +                       *edid_corrupt = 1;
>> +
>>                 /* allow CEA to slide through, switches mangle this */
>>                 if (raw_edid[0] != 0x02)
>>                         goto bad;
>> @@ -1129,7 +1145,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 +1248,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 +1274,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/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
> _______________________________________________
> 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

Re: [PATCH 3/5] drm/i915: Implement the intel_dp_autotest_edid function for DP EDID complaince tests

[Daniel Vetter]

On Thu, Apr 30, 2015 at 03:30:10PM -0300, Paulo Zanoni wrote:
> 2015-04-18 4:04 GMT-03:00 Todd Previte <tprevite@gmail.com>:
> > Updates the EDID compliance test function to perform the analyze and react to
> > the EDID data read as a result of a hot plug event. The results of this
> > analysis are handed off to userspace so that the userspace app can set the
> > display mode appropriately for the test result/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
> > V10:
> > - Updated to use actual checksum from the EDID read that occurs during
> >   normal hot plug path execution
> > - Removed variables from intel_dp struct that are no longer needed
> > - Updated the patch subject to more closely match the nature and contents
> >   of the patch
> >
> > Signed-off-by: Todd Previte <tprevite@gmail.com>
> > ---
> >  drivers/gpu/drm/i915/intel_dp.c  | 41 ++++++++++++++++++++++++++++++++++++++++
> >  drivers/gpu/drm/i915/intel_drv.h |  2 ++
> >  2 files changed, 43 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > index 55d1f5f..4c1f6a9 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
> 
> Now that you cleaned all the other stuff, can't we start at 0 instead
> of 4? (of course, this would require a change to the user space tool)
> 
> 
> > +#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,38 @@ 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;
> > +       struct intel_connector *intel_connector = intel_dp->attached_connector;
> > +       struct drm_connector *connector = &intel_connector->base;
> > +
> > +       if (intel_connector->detect_edid == NULL ||
> > +           connector->edid_corrupt == 1 ||
> > +           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_connector->detect_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");
> 
> You're mixing DRM_DEBUG_DRIVER and DRM_DEBUG_KMS. I'd stick with only
> one, and I'd probably pick _KMS.
> 
> With or without the above bikesheds: Reviewed-by: Paulo Zanoni
> <paulo.r.zanoni@intel.com>

Yeah I second the two polish requests from Paulo. Merged the previous
patch meanwhile.
-Daniel

> 
> > +
> > +               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;
> >  }
> >
> > @@ -4009,7 +4047,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;
> >
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > index a4675fa..6c71be9 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -742,6 +742,8 @@ struct intel_dp {
> >
> >         /* Displayport compliance testing */
> >         unsigned long compliance_test_type;
> > +       unsigned long compliance_test_data;
> > +       bool compliance_test_active;
> >  };
> >
> >  struct intel_digital_port {
> > --
> > 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

-- 
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

[PATCH 06/10] drm/i915: Implement the intel_dp_autotest_edid function for DP EDID complaince tests

[Todd Previte]

Updates the EDID compliance test function to perform the analyze and react to
the EDID data read as a result of a hot plug event. The results of this
analysis are handed off to userspace so that the userspace app can set the
display mode appropriately for the test result/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
V10:
- Updated to use actual checksum from the EDID read that occurs during
  normal hot plug path execution
- Removed variables from intel_dp struct that are no longer needed
- Updated the patch subject to more closely match the nature and contents
  of the patch
- Fixed formatting problem (long line)
V11:
- Removed extra debug messages
- Updated comments to be more informative
- Removed extra variable
V12:
- Removed the 4 bit offset of the resolution setting in compliance data
- Changed to DRM_DEBUG_KMS instead of DRM_DEBUG_DRIVER

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

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index b249ee8..a59bd75 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	0
+#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,39 @@ 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;
+	struct intel_connector *intel_connector = intel_dp->attached_connector;
+	struct drm_connector *connector = &intel_connector->base;
+
+	if (intel_connector->detect_edid == NULL ||
+	    connector->edid_corrupt == 1 ||
+	    intel_dp->aux.i2c_defer_count > 6) {
+		/* Check EDID read for NACKs, DEFERs and corruption
+		 * (DP CTS 1.2 Core r1.1)
+		 *    4.2.2.4 : Failed EDID read, I2C_NAK
+		 *    4.2.2.5 : Failed EDID read, I2C_DEFER
+		 *    4.2.2.6 : EDID corruption detected
+		 * Use failsafe mode for all cases
+		 */
+		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 {
+		if (!drm_dp_dpcd_write(&intel_dp->aux,
+					DP_TEST_EDID_CHECKSUM,
+					&intel_connector->detect_edid->checksum,
+					1));
+			DRM_DEBUG_KMS("Failed to write EDID checksum\n");
+
+		test_result = DP_TEST_ACK | DP_TEST_EDID_CHECKSUM_WRITE;
+		intel_dp->compliance_test_data = INTEL_DP_RESOLUTION_STANDARD;
+	}
+
+	/* Set test active flag here so userspace doesn't interrupt things */
+	intel_dp->compliance_test_active = 1;
+
 	return test_result;
 }
 
@@ -4009,7 +4048,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;
 
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index a4675fa..6c71be9 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -742,6 +742,8 @@ struct intel_dp {
 
 	/* Displayport compliance testing */
 	unsigned long compliance_test_type;
+	unsigned long compliance_test_data;
+	bool compliance_test_active;
 };
 
 struct intel_digital_port {
-- 
1.9.1

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

Re: [PATCH 3/5] drm/i915: Implement the intel_dp_autotest_edid function for DP EDID complaince tests

[Jani Nikula]

On Mon, 04 May 2015, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Thu, Apr 30, 2015 at 03:30:10PM -0300, Paulo Zanoni wrote:
>> 2015-04-18 4:04 GMT-03:00 Todd Previte <tprevite@gmail.com>:
>> > Updates the EDID compliance test function to perform the analyze and react to
>> > the EDID data read as a result of a hot plug event. The results of this
>> > analysis are handed off to userspace so that the userspace app can set the
>> > display mode appropriately for the test result/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
>> > V10:
>> > - Updated to use actual checksum from the EDID read that occurs during
>> >   normal hot plug path execution
>> > - Removed variables from intel_dp struct that are no longer needed
>> > - Updated the patch subject to more closely match the nature and contents
>> >   of the patch
>> >
>> > Signed-off-by: Todd Previte <tprevite@gmail.com>
>> > ---
>> >  drivers/gpu/drm/i915/intel_dp.c  | 41 ++++++++++++++++++++++++++++++++++++++++
>> >  drivers/gpu/drm/i915/intel_drv.h |  2 ++
>> >  2 files changed, 43 insertions(+)
>> >
>> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>> > index 55d1f5f..4c1f6a9 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
>> 
>> Now that you cleaned all the other stuff, can't we start at 0 instead
>> of 4? (of course, this would require a change to the user space tool)
>> 
>> 
>> > +#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,38 @@ 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;
>> > +       struct intel_connector *intel_connector = intel_dp->attached_connector;
>> > +       struct drm_connector *connector = &intel_connector->base;
>> > +
>> > +       if (intel_connector->detect_edid == NULL ||
>> > +           connector->edid_corrupt == 1 ||
>> > +           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_connector->detect_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");
>> 
>> You're mixing DRM_DEBUG_DRIVER and DRM_DEBUG_KMS. I'd stick with only
>> one, and I'd probably pick _KMS.
>> 
>> With or without the above bikesheds: Reviewed-by: Paulo Zanoni
>> <paulo.r.zanoni@intel.com>
>
> Yeah I second the two polish requests from Paulo. Merged the previous
> patch meanwhile.

I reverted this to unblock people because it breaks the build due to the
dependency on patch 2/5 which was apparently not merged yet. Sorry
Daniel, didn't get hold of you fast enough...

BR,
Jani.




> -Daniel
>
>> 
>> > +
>> > +               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;
>> >  }
>> >
>> > @@ -4009,7 +4047,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;
>> >
>> > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
>> > index a4675fa..6c71be9 100644
>> > --- a/drivers/gpu/drm/i915/intel_drv.h
>> > +++ b/drivers/gpu/drm/i915/intel_drv.h
>> > @@ -742,6 +742,8 @@ struct intel_dp {
>> >
>> >         /* Displayport compliance testing */
>> >         unsigned long compliance_test_type;
>> > +       unsigned long compliance_test_data;
>> > +       bool compliance_test_active;
>> >  };
>> >
>> >  struct intel_digital_port {
>> > --
>> > 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
>
> -- 
> 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

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 06/10] drm/i915: Implement the intel_dp_autotest_edid function for DP EDID complaince tests

[Daniel Vetter]

On Mon, May 04, 2015 at 07:48:20AM -0700, Todd Previte wrote:
> Updates the EDID compliance test function to perform the analyze and react to
> the EDID data read as a result of a hot plug event. The results of this
> analysis are handed off to userspace so that the userspace app can set the
> display mode appropriately for the test result/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
> V10:
> - Updated to use actual checksum from the EDID read that occurs during
>   normal hot plug path execution
> - Removed variables from intel_dp struct that are no longer needed
> - Updated the patch subject to more closely match the nature and contents
>   of the patch
> - Fixed formatting problem (long line)
> V11:
> - Removed extra debug messages
> - Updated comments to be more informative
> - Removed extra variable
> V12:
> - Removed the 4 bit offset of the resolution setting in compliance data
> - Changed to DRM_DEBUG_KMS instead of DRM_DEBUG_DRIVER
> 
> Signed-off-by: Todd Previte <tprevite@gmail.com>
> Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

Pulled in the remaining patches, thanks. Aside: Something seems to be
amiss in your patch number when you resend. This one here is 6/10 in the
original patch series which has only 5 patches ...
-Daniel

> ---
>  drivers/gpu/drm/i915/intel_dp.c  | 42 ++++++++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_drv.h |  2 ++
>  2 files changed, 44 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index b249ee8..a59bd75 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	0
> +#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,39 @@ 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;
> +	struct intel_connector *intel_connector = intel_dp->attached_connector;
> +	struct drm_connector *connector = &intel_connector->base;
> +
> +	if (intel_connector->detect_edid == NULL ||
> +	    connector->edid_corrupt == 1 ||
> +	    intel_dp->aux.i2c_defer_count > 6) {
> +		/* Check EDID read for NACKs, DEFERs and corruption
> +		 * (DP CTS 1.2 Core r1.1)
> +		 *    4.2.2.4 : Failed EDID read, I2C_NAK
> +		 *    4.2.2.5 : Failed EDID read, I2C_DEFER
> +		 *    4.2.2.6 : EDID corruption detected
> +		 * Use failsafe mode for all cases
> +		 */
> +		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 {
> +		if (!drm_dp_dpcd_write(&intel_dp->aux,
> +					DP_TEST_EDID_CHECKSUM,
> +					&intel_connector->detect_edid->checksum,
> +					1));
> +			DRM_DEBUG_KMS("Failed to write EDID checksum\n");
> +
> +		test_result = DP_TEST_ACK | DP_TEST_EDID_CHECKSUM_WRITE;
> +		intel_dp->compliance_test_data = INTEL_DP_RESOLUTION_STANDARD;
> +	}
> +
> +	/* Set test active flag here so userspace doesn't interrupt things */
> +	intel_dp->compliance_test_active = 1;
> +
>  	return test_result;
>  }
>  
> @@ -4009,7 +4048,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;
>  
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index a4675fa..6c71be9 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -742,6 +742,8 @@ struct intel_dp {
>  
>  	/* Displayport compliance testing */
>  	unsigned long compliance_test_type;
> +	unsigned long compliance_test_data;
> +	bool compliance_test_active;
>  };
>  
>  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

[PATCH] drm: Fix missing kerneldoc for the edid_corrupt field in struct drm_connector

[Todd Previte]

The kerneldoc for this newly added parameter was missing from the
original patch. This patch adds the appropriate kerneldoc entry.

Signed-off-by: Todd Previte <tprevite@gmail.com>
---
 include/drm/drm_crtc.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 8bc2724..9cd290c 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -647,6 +647,7 @@ struct drm_encoder {
  * @audio_latency: audio latency info from ELD, if found
  * @null_edid_counter: track sinks that give us all zeros for the EDID
  * @bad_edid_counter: track sinks that give us an EDID with invalid checksum
+ * @edid_corrupt: flag for EDID header corruption detection
  * @debugfs_entry: debugfs directory for this connector
  * @state: current atomic state for this connector
  * @has_tile: is this connector connected to a tiled monitor
-- 
1.9.1

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

Re: [PATCH] drm: Fix missing kerneldoc for the edid_corrupt field in struct drm_connector

[shuang.he]

Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 6359
-------------------------------------Summary-------------------------------------
Platform          Delta          drm-intel-nightly          Series Applied
PNV                                  276/276              276/276
ILK                                  302/302              302/302
SNB                                  316/316              316/316
IVB                                  342/342              342/342
BYT                                  286/286              286/286
BDW                                  321/321              321/321
-------------------------------------Detailed-------------------------------------
Platform  Test                                drm-intel-nightly          Series Applied
Note: You need to pay more attention to line start with '*'
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm: Fix missing kerneldoc for the edid_corrupt field in struct drm_connector

[Daniel Vetter]

On Fri, May 08, 2015 at 07:13:34AM -0700, Todd Previte wrote:
> The kerneldoc for this newly added parameter was missing from the
> original patch. This patch adds the appropriate kerneldoc entry.
> 
> Signed-off-by: Todd Previte <tprevite@gmail.com>

Patch was a bit too late before I've done the -testing tagging, so I went
ahead with my own fixup.
-Daniel
> ---
>  include/drm/drm_crtc.h | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index 8bc2724..9cd290c 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -647,6 +647,7 @@ struct drm_encoder {
>   * @audio_latency: audio latency info from ELD, if found
>   * @null_edid_counter: track sinks that give us all zeros for the EDID
>   * @bad_edid_counter: track sinks that give us an EDID with invalid checksum
> + * @edid_corrupt: flag for EDID header corruption detection
>   * @debugfs_entry: debugfs directory for this connector
>   * @state: current atomic state for this connector
>   * @has_tile: is this connector connected to a tiled monitor
> -- 
> 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