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?
>
> 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?
> }
>
> /* 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
> return connector_status_connected;
> else
> return connector_status_disconnected;
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
