diff mbox

[09/12] drm/i915: Update intel_dp_check_link_status() for Displayport compliance testing

Message ID 1405365047-6866-10-git-send-email-tprevite@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Todd Previte July 14, 2014, 7:10 p.m. UTC
Move the DPCD read to the top and check for an interrupt from the sink to catch
Displayport automated testing requests necessary to support Displayport compliance
testing. The checks for active connectors and link status are moved below the
check for the interrupt.

Adds a check at the top to verify that the device is connected. This is necessary
for DP compliance testing to ensure that test requests are captured and acknowledged.
If a test request is present during a connected->disconnected transition,
the test code will attempt to execute even though the connection has been disabled,
resulting in a faied test.

Signed-off-by: Todd Previte <tprevite@gmail.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 32 ++++++++++++++++++--------------
 1 file changed, 18 insertions(+), 14 deletions(-)

Comments

Paulo Zanoni July 30, 2014, 2:29 p.m. UTC | #1
2014-07-14 16:10 GMT-03:00 Todd Previte <tprevite@gmail.com>:
> Move the DPCD read to the top and check for an interrupt from the sink to catch
> Displayport automated testing requests necessary to support Displayport compliance
> testing. The checks for active connectors and link status are moved below the
> check for the interrupt.
>
> Adds a check at the top to verify that the device is connected. This is necessary
> for DP compliance testing to ensure that test requests are captured and acknowledged.
> If a test request is present during a connected->disconnected transition,
> the test code will attempt to execute even though the connection has been disabled,
> resulting in a faied test.
>

The problem here we that we just got an interrupt, so I am not sure if
we should believe what's in intel_dp->attached_connector->base.status:
doesn't it reflect the old state that we had before we got the
interrupt? I'm not sure if this information could be wrong. Can you
please elaborate on that?

The rest of the patch seems correct.

> Signed-off-by: Todd Previte <tprevite@gmail.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c | 32 ++++++++++++++++++--------------
>  1 file changed, 18 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 1c6ee34..65830e9 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -3600,22 +3600,13 @@ intel_dp_check_link_status(struct intel_dp *intel_dp)
>         u8 sink_irq_vector;
>         u8 link_status[DP_LINK_STATUS_SIZE];
>
> -       /* FIXME: This access isn't protected by any locks. */
> -       if (!intel_encoder->connectors_active)
> -               return;
> -
> -       if (WARN_ON(!intel_encoder->base.crtc))
> +       /* Bail if not connected */
> +       if (intel_dp->attached_connector->base.status != connector_status_connected)
>                 return;
>
> -       /* Try to read receiver status if the link appears to be up */
> -       if (!intel_dp_get_link_status(intel_dp, link_status)) {
> -               return;
> -       }
> -
> -       /* Now read the DPCD to see if it's actually running */
> -       if (!intel_dp_get_dpcd(intel_dp)) {
> +       /* Attempt to read the DPCD */
> +       if (!intel_dp_get_dpcd(intel_dp))
>                 return;
> -       }
>
>         /* Try to read the source of the interrupt */
>         if (intel_dp->dpcd[DP_DPCD_REV] >= 0x11 &&
> @@ -3625,12 +3616,25 @@ intel_dp_check_link_status(struct intel_dp *intel_dp)
>                                    DP_DEVICE_SERVICE_IRQ_VECTOR,
>                                    sink_irq_vector);
>
> -               if (sink_irq_vector & DP_AUTOMATED_TEST_REQUEST)
> +               if (sink_irq_vector & DP_AUTOMATED_TEST_REQUEST) {
> +                       DRM_DEBUG_KMS("Displayport: Received automated test request\n");
>                         intel_dp_handle_test_request(intel_dp);
> +               }
>                 if (sink_irq_vector & (DP_CP_IRQ | DP_SINK_SPECIFIC_IRQ))
>                         DRM_DEBUG_DRIVER("CP or sink specific irq unhandled\n");
>         }
>
> +       /* FIXME: This access isn't protected by any locks. */
> +       if (!intel_encoder->connectors_active)
> +               return;
> +
> +       if (WARN_ON(!intel_encoder->base.crtc))
> +               return;
> +
> +       /* Try to read receiver status if the link appears to be up */
> +       if (!intel_dp_get_link_status(intel_dp, link_status))
> +               return;
> +
>         if (!drm_dp_channel_eq_ok(link_status, intel_dp->lane_count)) {
>                 DRM_DEBUG_KMS("%s: channel EQ not ok, retraining\n",
>                               intel_encoder->base.name);
> --
> 1.9.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 1c6ee34..65830e9 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -3600,22 +3600,13 @@  intel_dp_check_link_status(struct intel_dp *intel_dp)
 	u8 sink_irq_vector;
 	u8 link_status[DP_LINK_STATUS_SIZE];
 
-	/* FIXME: This access isn't protected by any locks. */
-	if (!intel_encoder->connectors_active)
-		return;
-
-	if (WARN_ON(!intel_encoder->base.crtc))
+	/* Bail if not connected */
+	if (intel_dp->attached_connector->base.status != connector_status_connected)
 		return;
 
-	/* Try to read receiver status if the link appears to be up */
-	if (!intel_dp_get_link_status(intel_dp, link_status)) {
-		return;
-	}
-
-	/* Now read the DPCD to see if it's actually running */
-	if (!intel_dp_get_dpcd(intel_dp)) {
+	/* Attempt to read the DPCD */
+	if (!intel_dp_get_dpcd(intel_dp))
 		return;
-	}
 
 	/* Try to read the source of the interrupt */
 	if (intel_dp->dpcd[DP_DPCD_REV] >= 0x11 &&
@@ -3625,12 +3616,25 @@  intel_dp_check_link_status(struct intel_dp *intel_dp)
 				   DP_DEVICE_SERVICE_IRQ_VECTOR,
 				   sink_irq_vector);
 
-		if (sink_irq_vector & DP_AUTOMATED_TEST_REQUEST)
+		if (sink_irq_vector & DP_AUTOMATED_TEST_REQUEST) {
+			DRM_DEBUG_KMS("Displayport: Received automated test request\n");
 			intel_dp_handle_test_request(intel_dp);
+		}
 		if (sink_irq_vector & (DP_CP_IRQ | DP_SINK_SPECIFIC_IRQ))
 			DRM_DEBUG_DRIVER("CP or sink specific irq unhandled\n");
 	}
 
+	/* FIXME: This access isn't protected by any locks. */
+	if (!intel_encoder->connectors_active)
+		return;
+
+	if (WARN_ON(!intel_encoder->base.crtc))
+		return;
+
+	/* Try to read receiver status if the link appears to be up */
+	if (!intel_dp_get_link_status(intel_dp, link_status))
+		return;
+
 	if (!drm_dp_channel_eq_ok(link_status, intel_dp->lane_count)) {
 		DRM_DEBUG_KMS("%s: channel EQ not ok, retraining\n",
 			      intel_encoder->base.name);