diff mbox

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

Message ID 1429126125-1320-1-git-send-email-tprevite@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Todd Previte April 15, 2015, 7:28 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.

The main reason for doing this is to make sure that a test request isn't missed.
Checking for the status of the encoder/crtc isn't necessary for some test cases
(AUX channel tests are one example) and without moving the check for the
interrupt, these tests may not execute if one of those checks fails.
Additionally, if reading the DPCD fails, regardless of whether or not testing is
happening, there's no way to train the link since configurations and status
can't be read, nor can link training parameters be written.

V1:
- This is the second part of the single-patch split previously
  mentioned.
V2:
- Remerge the two split patches into one and update the commit message
  accordingly.
- Replace the SW connected status check with a HW HPD pin status check
- Adds a new function that examines the status of the HPD pin to
  determine if a sink device is connected
V3:
- CLean up of the patch merge from previous split
- Updated the commit message

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

Comments

Paulo Zanoni April 15, 2015, 7:45 p.m. UTC | #1
2015-04-15 16:28 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.
>
> The main reason for doing this is to make sure that a test request isn't missed.
> Checking for the status of the encoder/crtc isn't necessary for some test cases
> (AUX channel tests are one example) and without moving the check for the
> interrupt, these tests may not execute if one of those checks fails.
> Additionally, if reading the DPCD fails, regardless of whether or not testing is
> happening, there's no way to train the link since configurations and status
> can't be read, nor can link training parameters be written.
>
> V1:
> - This is the second part of the single-patch split previously
>   mentioned.
> V2:
> - Remerge the two split patches into one and update the commit message
>   accordingly.
> - Replace the SW connected status check with a HW HPD pin status check
> - Adds a new function that examines the status of the HPD pin to
>   determine if a sink device is connected
> V3:
> - CLean up of the patch merge from previous split
> - Updated the commit message
>
> Signed-off-by: Todd Previte <tprevite@gmail.com>

Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

> ---
>  drivers/gpu/drm/i915/intel_dp.c | 33 +++++++++++++++------------------
>  1 file changed, 15 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 263eff3..9c38986 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -4119,24 +4119,8 @@ intel_dp_check_link_status(struct intel_dp *intel_dp)
>
>         WARN_ON(!drm_modeset_is_locked(&dev->mode_config.connection_mutex));
>
> -       if (!intel_encoder->connectors_active)
> -               return;
> -
> -       if (WARN_ON(!intel_encoder->base.crtc))
> -               return;
> -
> -       if (!to_intel_crtc(intel_encoder->base.crtc)->active)
> -               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)) {
> +       if (!intel_dp_get_dpcd(intel_dp))
>                 return;
> -       }
>
>         /* Try to read the source of the interrupt */
>         if (intel_dp->dpcd[DP_DPCD_REV] >= 0x11 &&
> @@ -4145,13 +4129,26 @@ intel_dp_check_link_status(struct intel_dp *intel_dp)
>                 drm_dp_dpcd_writeb(&intel_dp->aux,
>                                    DP_DEVICE_SERVICE_IRQ_VECTOR,
>                                    sink_irq_vector);
> -
>                 if (sink_irq_vector & DP_AUTOMATED_TEST_REQUEST)
>                         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");
>         }
>
> +       if (!intel_encoder->connectors_active)
> +               return;
> +
> +       if (WARN_ON(!intel_encoder->base.crtc))
> +               return;
> +
> +       if (!to_intel_crtc(intel_encoder->base.crtc)->active)
> +               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 263eff3..9c38986 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4119,24 +4119,8 @@  intel_dp_check_link_status(struct intel_dp *intel_dp)
 
 	WARN_ON(!drm_modeset_is_locked(&dev->mode_config.connection_mutex));
 
-	if (!intel_encoder->connectors_active)
-		return;
-
-	if (WARN_ON(!intel_encoder->base.crtc))
-		return;
-
-	if (!to_intel_crtc(intel_encoder->base.crtc)->active)
-		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)) {
+	if (!intel_dp_get_dpcd(intel_dp))
 		return;
-	}
 
 	/* Try to read the source of the interrupt */
 	if (intel_dp->dpcd[DP_DPCD_REV] >= 0x11 &&
@@ -4145,13 +4129,26 @@  intel_dp_check_link_status(struct intel_dp *intel_dp)
 		drm_dp_dpcd_writeb(&intel_dp->aux,
 				   DP_DEVICE_SERVICE_IRQ_VECTOR,
 				   sink_irq_vector);
-
 		if (sink_irq_vector & DP_AUTOMATED_TEST_REQUEST)
 			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");
 	}
 
+	if (!intel_encoder->connectors_active)
+		return;
+
+	if (WARN_ON(!intel_encoder->base.crtc))
+		return;
+
+	if (!to_intel_crtc(intel_encoder->base.crtc)->active)
+		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);