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

[Ander Conselvan De Oliveira <conselvan2@gmail.com>]

On Wed, 2016-05-25 at 17:22 -0700, Manasi Navare wrote:
> 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@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.

So my question is, in those failure scenarios when not in the middle of
compliance testing, does the system behaves as expected?

Ander

> 
>  
> > 
> > > 
> > > 
> > > Signed-off-by: Manasi Navare <manasi.d.navare@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;
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx