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

[Shubhangi Shrivastava <shubhangi.shrivastava@intel.com>]

On Monday 23 May 2016 01:48 PM, 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?
>
>> 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?
>
>>   	}
>>   
>>   	/* 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

For tests:
                  *    4.2.2.4 : Failed EDID read, I2C_NAK
                  *    4.2.2.5 : Failed EDID read, I2C_DEFER
                  *    4.2.2.6 : EDID corruption detected
when edid is detected to be corrupt, we need to fall back to failsafe mode.
This requires change for bpc and status to be connected which is being
performed in the patch: https://patchwork.freedesktop.org/patch/82566/
"drm/i915: Use fail safe mode when edid is corrupt"

Shubhangi


>>   		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