Add support for Displayport CTS 4.2.2.7, 4.2.2.8 and 4.2.2.9

Add support for Displayport CTS 4.2.2.7, 4.2.2.8 and 4.2.2.9

[Todd Previte]

Three patches to add support for the above-mentioned compliance tests. The 
first two are branch device tests and the third is a 4-block E-DDC read.     

Todd Previte (3)
   drm/i915: Displayport compliance test 4.2.2.8 support
   drm/i915: Displayport compliance test 4.2.2.7 support
   drm/i915: Displayport compliance test 4.2.2.9 support


drivers/gpu/drm/i915/intel_dp.c  | 30 ++++++++++++++++++++++++++++--
drivers/gpu/drm/i915/intel_drv.h |  4 ++++
2 files changed, 32 insertions(+), 2 deletions(-)

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

[PATCH 1/3] drm/i915: Displayport compliance test 4.2.2.9 support

[Todd Previte]

Implement the fix for test 4.2.2.9. This test is a 4-block E-DDC
read over the AUX channel. For test purposes, the normal 1-block
EDID read is required to write the checksum of that block to the
sink device via the AUX channel. For 4.2.2.9, that checksum must
be the checksum of the last block read. The DRM EDID read code
already does the 4-block read, so the checksum just needs to be
adjusted to the one that we need based on the number of extensions
detected in the block.

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

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index c7cbb67..14147d0 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4020,9 +4020,17 @@ static uint8_t intel_dp_autotest_edid(struct intel_dp *intel_dp)
 				      intel_dp->aux.i2c_defer_count);
 		intel_dp->compliance_test_data = INTEL_DP_RESOLUTION_FAILSAFE;
 	} else {
+		/* Checksum write for EDID reads (DP CTS 1.2 Core r1.1)
+		 *    4.2.2.3: checksum of EDID block 0
+		 *    4.2.2.9: checksum of the last 128-byte block read
+		 */
+		int checksum = *(&intel_connector->detect_edid->checksum +
+				 (intel_connector->detect_edid->extensions *
+				  EDID_LENGTH));
+
 		if (!drm_dp_dpcd_write(&intel_dp->aux,
 					DP_TEST_EDID_CHECKSUM,
-					&intel_connector->detect_edid->checksum,
+					&checksum,
 					1))
 			DRM_DEBUG_KMS("Failed to write EDID checksum\n");
 
-- 
1.9.1

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

Re: [PATCH 1/3] drm/i915: Displayport compliance test 4.2.2.9 support

[Daniel Vetter]

On Tue, Jun 09, 2015 at 05:22:18PM -0700, Todd Previte wrote:
> Implement the fix for test 4.2.2.9. This test is a 4-block E-DDC
> read over the AUX channel. For test purposes, the normal 1-block
> EDID read is required to write the checksum of that block to the
> sink device via the AUX channel. For 4.2.2.9, that checksum must
> be the checksum of the last block read. The DRM EDID read code
> already does the 4-block read, so the checksum just needs to be
> adjusted to the one that we need based on the number of extensions
> detected in the block.
> 
> Signed-off-by: Todd Previte <tprevite@gmail.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index c7cbb67..14147d0 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -4020,9 +4020,17 @@ static uint8_t intel_dp_autotest_edid(struct intel_dp *intel_dp)
>  				      intel_dp->aux.i2c_defer_count);
>  		intel_dp->compliance_test_data = INTEL_DP_RESOLUTION_FAILSAFE;
>  	} else {
> +		/* Checksum write for EDID reads (DP CTS 1.2 Core r1.1)
> +		 *    4.2.2.3: checksum of EDID block 0
> +		 *    4.2.2.9: checksum of the last 128-byte block read
> +		 */
> +		int checksum = *(&intel_connector->detect_edid->checksum +
> +				 (intel_connector->detect_edid->extensions *
> +				  EDID_LENGTH));
> +
>  		if (!drm_dp_dpcd_write(&intel_dp->aux,
>  					DP_TEST_EDID_CHECKSUM,
> -					&intel_connector->detect_edid->checksum,
> +					&checksum,

This is some pretty hairy pointer arithmetic. I think it'd be good to have
a little dp helper function which takes the edid and dp aux pointer and
sends the right checksum to the right dp aux register. Then we could move
the above comment into kerneldoc code.
-Daniel

>  					1))
>  			DRM_DEBUG_KMS("Failed to write EDID checksum\n");
>  
> -- 
> 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

Re: [PATCH 1/3] drm/i915: Displayport compliance test 4.2.2.9 support

[Chris Wilson]

On Mon, Jun 15, 2015 at 05:37:58PM +0200, Daniel Vetter wrote:
> On Tue, Jun 09, 2015 at 05:22:18PM -0700, Todd Previte wrote:
> > Implement the fix for test 4.2.2.9. This test is a 4-block E-DDC
> > read over the AUX channel. For test purposes, the normal 1-block
> > EDID read is required to write the checksum of that block to the
> > sink device via the AUX channel. For 4.2.2.9, that checksum must
> > be the checksum of the last block read. The DRM EDID read code
> > already does the 4-block read, so the checksum just needs to be
> > adjusted to the one that we need based on the number of extensions
> > detected in the block.
> > 
> > Signed-off-by: Todd Previte <tprevite@gmail.com>
> > ---
> >  drivers/gpu/drm/i915/intel_dp.c | 10 +++++++++-
> >  1 file changed, 9 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > index c7cbb67..14147d0 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -4020,9 +4020,17 @@ static uint8_t intel_dp_autotest_edid(struct intel_dp *intel_dp)
> >  				      intel_dp->aux.i2c_defer_count);
> >  		intel_dp->compliance_test_data = INTEL_DP_RESOLUTION_FAILSAFE;
> >  	} else {
> > +		/* Checksum write for EDID reads (DP CTS 1.2 Core r1.1)
> > +		 *    4.2.2.3: checksum of EDID block 0
> > +		 *    4.2.2.9: checksum of the last 128-byte block read
> > +		 */
> > +		int checksum = *(&intel_connector->detect_edid->checksum +
> > +				 (intel_connector->detect_edid->extensions *
> > +				  EDID_LENGTH));
> > +
> >  		if (!drm_dp_dpcd_write(&intel_dp->aux,
> >  					DP_TEST_EDID_CHECKSUM,
> > -					&intel_connector->detect_edid->checksum,
> > +					&checksum,
> 
> This is some pretty hairy pointer arithmetic. I think it'd be good to have
> a little dp helper function which takes the edid and dp aux pointer and
> sends the right checksum to the right dp aux register. Then we could move
> the above comment into kerneldoc code.

struct edid *edid = intel_connector->detect_edid;
u8 *block = (u8 *)edid + EDID_LENGTH*edid->extensions;
u8 *csum = block + offsetof(struct edid, checksum);

not forcing the csum dereference should be a little neater, but making
the actual calculation neater is difficult - but we can offset that with
a little verbosity.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 2/3] drm/i915: Displayport compliance test 4.2.2.7 support

[Todd Previte]

Adds support for complying with the requirements for test 4.2.2.7 Branch
Device Detection upon HPD Plug Event in the Displayport Link CTS 1.2 Core
rev1.1. This test checks to see if the source device can properly detect
a downstream branch device connected to the attached sink. It does so by
advertising the presence of a downstream port through the DPCD. The sink
count and content protection readiness bits are saved in the intel_dp
struct.

Note that as of HDCP 1.3 Displayport amendment, the CP_READY bit (bit 6)
in the SINK_COUNT register (DPCD 0x200) is no longer used. This bit
is stored here for compatibility for pre-HDCP 1.3 devices.

V1:
- Fixed formatting errors
V2:
- Updated for versioning

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

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 14147d0..697857a 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -3856,8 +3856,18 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)
 	intel_dp_print_rates(intel_dp);
 
 	if (!(intel_dp->dpcd[DP_DOWNSTREAMPORT_PRESENT] &
-	      DP_DWN_STRM_PORT_PRESENT))
+	      DP_DWN_STRM_PORT_PRESENT)) {
 		return true; /* native DP sink */
+	} else {
+		uint8_t dpcd_byte;
+		/* Read Sink Count - DP Link CTS 1.2a rev1.1 4.2.2.7 */
+		intel_dp_dpcd_read_wake(&intel_dp->aux, DP_SINK_COUNT,
+					&dpcd_byte, 1);
+		intel_dp->sink_count = DP_GET_SINK_COUNT(dpcd_byte);
+		intel_dp->cp_ready = (dpcd_byte & DP_SINK_CP_READY) >> 6;
+		DRM_DEBUG_KMS("Sink count: %d  CP: %02x\n",
+			      intel_dp->sink_count, intel_dp->cp_ready);
+	}
 
 	if (intel_dp->dpcd[DP_DPCD_REV] == 0x10)
 		return true; /* no per-port downstream info */
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 6c71be9..f9a935c 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -744,6 +744,10 @@ struct intel_dp {
 	unsigned long compliance_test_type;
 	unsigned long compliance_test_data;
 	bool compliance_test_active;
+
+	/* Branch device support */
+	uint8_t sink_count;
+	uint8_t cp_ready;
 };
 
 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 2/3] drm/i915: Displayport compliance test 4.2.2.7 support

[Daniel Vetter]

On Tue, Jun 09, 2015 at 05:22:19PM -0700, Todd Previte wrote:
> Adds support for complying with the requirements for test 4.2.2.7 Branch
> Device Detection upon HPD Plug Event in the Displayport Link CTS 1.2 Core
> rev1.1. This test checks to see if the source device can properly detect
> a downstream branch device connected to the attached sink. It does so by
> advertising the presence of a downstream port through the DPCD. The sink
> count and content protection readiness bits are saved in the intel_dp
> struct.
> 
> Note that as of HDCP 1.3 Displayport amendment, the CP_READY bit (bit 6)
> in the SINK_COUNT register (DPCD 0x200) is no longer used. This bit
> is stored here for compatibility for pre-HDCP 1.3 devices.
> 
> V1:
> - Fixed formatting errors
> V2:
> - Updated for versioning
> 
> Signed-off-by: Todd Previte <tprevite@gmail.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c  | 12 +++++++++++-
>  drivers/gpu/drm/i915/intel_drv.h |  4 ++++
>  2 files changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 14147d0..697857a 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -3856,8 +3856,18 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)
>  	intel_dp_print_rates(intel_dp);
>  
>  	if (!(intel_dp->dpcd[DP_DOWNSTREAMPORT_PRESENT] &
> -	      DP_DWN_STRM_PORT_PRESENT))
> +	      DP_DWN_STRM_PORT_PRESENT)) {
>  		return true; /* native DP sink */
> +	} else {
> +		uint8_t dpcd_byte;
> +		/* Read Sink Count - DP Link CTS 1.2a rev1.1 4.2.2.7 */
> +		intel_dp_dpcd_read_wake(&intel_dp->aux, DP_SINK_COUNT,
> +					&dpcd_byte, 1);
> +		intel_dp->sink_count = DP_GET_SINK_COUNT(dpcd_byte);
> +		intel_dp->cp_ready = (dpcd_byte & DP_SINK_CP_READY) >> 6;
> +		DRM_DEBUG_KMS("Sink count: %d  CP: %02x\n",
> +			      intel_dp->sink_count, intel_dp->cp_ready);
> +	}
>  
>  	if (intel_dp->dpcd[DP_DPCD_REV] == 0x10)
>  		return true; /* no per-port downstream info */
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 6c71be9..f9a935c 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -744,6 +744,10 @@ struct intel_dp {
>  	unsigned long compliance_test_type;
>  	unsigned long compliance_test_data;
>  	bool compliance_test_active;
> +
> +	/* Branch device support */
> +	uint8_t sink_count;
> +	uint8_t cp_ready;

Isn't this something that the dp helpers already maintain? If so then we
should probably use that one. If not I wonder whether we shouldn't do
something with these sink values instead of just passing them around ...
-Daniel

>  };
>  
>  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 3/3] drm/i915: Displayport compliance test 4.2.2.8 support

[Todd Previte]

Adds support for the test 4.2.2.8 EDID read on IRQ_HPD event after
Branch Device Detection in the Displayport Link CTS Core 1.2 rev1.1.
This test checks to see that the source device reads the EDID from
the attached sink device upon detection of a downstream port. A short
pulse is generated by the sink device to indicate a status change in
the downstream ports to which the source device must respond by
reading the EDID from the attached sink.

Since this is a test that occurs during a short pulse instead of a
long pulse, the normal EDID read that occurs during the call to
intel_dp_detect() does happen. Currently this read must be placed here
in order to pass the compliance tests. However, the EDID data from
this read is discarded at this time. In the future, this EDID read
may be used for other purposes and can be stored as necessary at that
time should be need arise.

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

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 697857a..99d2e81 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4220,6 +4220,14 @@ intel_dp_check_link_status(struct intel_dp *intel_dp)
 		if (sink_irq_vector & (DP_CP_IRQ | DP_SINK_SPECIFIC_IRQ))
 			DRM_DEBUG_DRIVER("CP or sink specific irq unhandled\n");
 	}
+	/* Displayport Link CTS 1.2a rev1.1
+	 *      4.2.2.8 : Check for downstream port status change
+	 */
+	if (link_status[2] & DP_DOWNSTREAM_PORT_STATUS_CHANGED) {
+		struct edid *edid_read = NULL;
+		edid_read = drm_get_edid(&intel_dp->attached_connector->base,
+					 &intel_dp->aux.ddc);
+	}
 
 	if (!drm_dp_channel_eq_ok(link_status, intel_dp->lane_count)) {
 		DRM_DEBUG_KMS("%s: channel EQ not ok, retraining\n",
-- 
1.9.1

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

Re: [PATCH 3/3] drm/i915: Displayport compliance test 4.2.2.8 support

[Daniel Vetter]

On Tue, Jun 09, 2015 at 05:22:20PM -0700, Todd Previte wrote:
> Adds support for the test 4.2.2.8 EDID read on IRQ_HPD event after
> Branch Device Detection in the Displayport Link CTS Core 1.2 rev1.1.
> This test checks to see that the source device reads the EDID from
> the attached sink device upon detection of a downstream port. A short
> pulse is generated by the sink device to indicate a status change in
> the downstream ports to which the source device must respond by
> reading the EDID from the attached sink.
> 
> Since this is a test that occurs during a short pulse instead of a
> long pulse, the normal EDID read that occurs during the call to
> intel_dp_detect() does happen. Currently this read must be placed here
> in order to pass the compliance tests. However, the EDID data from
> this read is discarded at this time. In the future, this EDID read
> may be used for other purposes and can be stored as necessary at that
> time should be need arise.
> 
> Signed-off-by: Todd Previte <tprevite@gmail.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 697857a..99d2e81 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -4220,6 +4220,14 @@ intel_dp_check_link_status(struct intel_dp *intel_dp)
>  		if (sink_irq_vector & (DP_CP_IRQ | DP_SINK_SPECIFIC_IRQ))
>  			DRM_DEBUG_DRIVER("CP or sink specific irq unhandled\n");
>  	}
> +	/* Displayport Link CTS 1.2a rev1.1
> +	 *      4.2.2.8 : Check for downstream port status change
> +	 */
> +	if (link_status[2] & DP_DOWNSTREAM_PORT_STATUS_CHANGED) {
> +		struct edid *edid_read = NULL;
> +		edid_read = drm_get_edid(&intel_dp->attached_connector->base,
> +					 &intel_dp->aux.ddc);

This smells like papering over a bug in our dp implementation again.
Assuming we correctly handle the hpd for this case of a downstream port
change then userspace should receive the uevent, which should result in an
edid read.

If that doesn't happen then we have again a bug somewhere in our dp code
that needs to be address properly. Just reading the edid and then throwing
it away isn't it.
-Daniel

> +	}
>  
>  	if (!drm_dp_channel_eq_ok(link_status, intel_dp->lane_count)) {
>  		DRM_DEBUG_KMS("%s: channel EQ not ok, retraining\n",
> -- 
> 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

Re: [PATCH 3/3] drm/i915: Displayport compliance test 4.2.2.8 support

[shuang.he]

Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 6621
-------------------------------------Summary-------------------------------------
Platform          Delta          drm-intel-nightly          Series Applied
ILK                                  302/302              302/302
SNB                                  312/316              312/316
IVB                                  343/343              343/343
BYT                 -2              287/287              285/287
HSW                                  380/380              380/380
-------------------------------------Detailed-------------------------------------
Platform  Test                                drm-intel-nightly          Series Applied
*BYT  igt@gem_partial_pwrite_pread@reads-display      PASS(1)      FAIL(1)
*BYT  igt@gem_partial_pwrite_pread@reads-uncached      PASS(1)      FAIL(1)
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