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@xxxxxxxxx> > > 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@xxxxxxxxx> > > > --- > > > 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@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx