diff mbox

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

Message ID 1433895740-13698-2-git-send-email-tprevite@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Todd Previte June 10, 2015, 12:22 a.m. UTC
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(-)

Comments

Daniel Vetter June 15, 2015, 3:37 p.m. UTC | #1
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
Chris Wilson June 15, 2015, 3:50 p.m. UTC | #2
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
diff mbox

Patch

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");