[Intel-gfx] [PATCH 3/5] drm/i915: Fixes to support the DP Compliance EDID tests.

[Manasi Navare]

On Mon, May 23, 2016 at 11:18:20AM +0300, Ander Conselvan De Oliveira wrote:
> On Fri, 2016-04-29 at 18:28 -0700, Manasi Navare wrote:
> > This patch addresses a few issues from the original patch for
> > DP Compliance EDID test support submitted by
> > Todd Previte<todd.previte at gmail.com>
> 
> Do you mean commit 559be30cb74d ("drm/i915: Implement the intel_dp_autotest_edid
> function for DP EDID complaince tests")? Please see the link below on how to
> refer to other commits in the commit message and how to add a Fixes: tag.
> 
> https://www.kernel.org/doc/Documentation/SubmittingPatches
> 
> > 
> > Video Mode requested in the EDID test handler for the EDID Read
> > test (CTS 4.2.2.3) should be set to PREFERRED as per the CTS spec.
> > Intel connector status should be connected even if detect_edid is
> > NULL when compliance_test flag is set. This is required to handle
> > the corrupt EDID (CTS 4.2.2.6) or EDID Read Failure I2C NACK/I2C
> > DEFER (CTS 4.2.2.4 and 4.2.2.5) tests from CTS spec.
> 
> What exactly do those tests test? It sounds like this patch adds a separate code
> path to implement the right behavior only when running the CTS. Shouldn't the
> driver handle those failures during normal operation in the same way?
>

These tests see if the system behaves as expected in case of currupt EDID 
or I2C NACK or I2C DEFER and validates if in all these cases it displays
the failsafe mode. This test gets triggered on a long pulse sent by DPR 120.

 
> > 
> > Signed-off-by: Manasi Navare <manasi.d.navare at intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_dp.c | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > index 0961f22..456fc17 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -4023,7 +4023,7 @@ 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;
> > +	uint8_t test_result = DP_TEST_ACK;
> >  	struct intel_connector *intel_connector = intel_dp-
> > >attached_connector;
> >  	struct drm_connector *connector = &intel_connector->base;
> >  
> > @@ -4058,7 +4058,7 @@ static uint8_t intel_dp_autotest_edid(struct intel_dp
> > *intel_dp)
> >  			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;
> > +		intel_dp->compliance_test_data =
> > INTEL_DP_RESOLUTION_PREFERRED;
> 
> Is this used for anything else than logging?
> 

This is used to tell the userspace app to display the Preferred mode
or failsafe mode for that test scenario.
This compliance test data gets read in userspace IGT App.


> >  	}
> >  
> >  	/* Set test active flag here so userspace doesn't interrupt things */
> > @@ -4650,7 +4650,7 @@ intel_dp_detect(struct drm_connector *connector, bool
> > force)
> >  
> >  	intel_dp->detect_done = false;
> >  
> > -	if (intel_connector->detect_edid)
> > +	if (intel_connector->detect_edid || intel_dp->compliance_test_active)
> 
> Should this check connector->edid_corrupt instead? I guess that would require
> some logic to fallback to fail safe mode and bpc too.
> 
> I think Shubhangi had a patch for this same problem, but it also seems to create
> a separate path for compliance.
> 
> Ander

This check only makes sure that if the compliance test is in progress then that means
it is testing for cases like corrupt edid and NACK/I2C defer and hence its a fake or
purposely created corrupt EDID or I2C failure scenario so report the connector
as connected. Otherwise, it reports it out as disconnected and treats this test scenario
as a real failure and the test does not complete.

Manasi

> >  		return connector_status_connected;
> >  	else
> >  		return connector_status_disconnected;