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

Chris Wilson
2015-06-15T15:50:17Z

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