diff mbox

[4/8] drm/i915: Move Displayport test request and sink IRQ logic to intel_dp_detect()

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

Commit Message

Todd Previte April 20, 2015, 10:27 p.m. UTC
Due to changes in the driver and to support Displayport compliance testing,
the test request and sink IRQ logic has been relocated from
intel_dp_check_link_status to intel_dp_detect. This is because the bulk of the
compliance tests that set the TEST_REQUEST bit in the DEVICE_IRQ field of the
DPCD issue a long pulse / hot plug event to signify the start of the test.
Currently, for a long pulse, intel_dp_check_link_status is not called for a
long HPD pulse, so if test requests come in, they cannot be detected by the
driver.

Once located in the intel_dp_detect, in the regular hot plug event path,
proper detection of Displayport compliance test requests occurs which then
invokes the test handler to support them. Additionally, this places compliance
testing in the normal operational paths, eliminating as much special case code
as possible.

The only change in intel_dp_check_link_status with this patch is that when
the IRQ is the result of a test request from the sink, the test handler is not
invoked during the short pulse path. Short pulse test requests are for a
particular variety of tests (mainly link training) that will be implemented
in the future. Once those tests are available, the test request handler will
be called from here as well.

V1:
- Rewored the commit message to be more clear about the content and intent
  of this patch
- Restore IRQ detection logic to intel_dp_check_link_status(). Continue to
  detect and clear sink IRQs in the short pulse case. Ignore test requests
  in the short pulses for now since they are for future test implementations.

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

Comments

Paulo Zanoni April 30, 2015, 6:14 p.m. UTC | #1
2015-04-20 19:27 GMT-03:00 Todd Previte <tprevite@gmail.com>:
> Due to changes in the driver and to support Displayport compliance testing,
> the test request and sink IRQ logic has been relocated from
> intel_dp_check_link_status to intel_dp_detect. This is because the bulk of the
> compliance tests that set the TEST_REQUEST bit in the DEVICE_IRQ field of the
> DPCD issue a long pulse / hot plug event to signify the start of the test.
> Currently, for a long pulse, intel_dp_check_link_status is not called for a
> long HPD pulse, so if test requests come in, they cannot be detected by the
> driver.
>
> Once located in the intel_dp_detect, in the regular hot plug event path,
> proper detection of Displayport compliance test requests occurs which then
> invokes the test handler to support them. Additionally, this places compliance
> testing in the normal operational paths, eliminating as much special case code
> as possible.
>
> The only change in intel_dp_check_link_status with this patch is that when
> the IRQ is the result of a test request from the sink, the test handler is not
> invoked during the short pulse path. Short pulse test requests are for a
> particular variety of tests (mainly link training) that will be implemented
> in the future. Once those tests are available, the test request handler will
> be called from here as well.
>
> V1:
> - Rewored the commit message to be more clear about the content and intent
>   of this patch
> - Restore IRQ detection logic to intel_dp_check_link_status(). Continue to
>   detect and clear sink IRQs in the short pulse case. Ignore test requests
>   in the short pulses for now since they are for future test implementations.

It took me a a while to realize the "duplicate" code paths are not
really duplicate because of the short vs long thing. But this looks
correct so: Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

But, later, we could certainly use a cleanup on intel_dp_hpd_pulse().

>
> Signed-off-by: Todd Previte <tprevite@gmail.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c | 17 ++++++++++++++++-
>  1 file changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 2ef05d7..b249ee8 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -4156,7 +4156,7 @@ intel_dp_check_link_status(struct intel_dp *intel_dp)
>                                    sink_irq_vector);
>
>                 if (sink_irq_vector & DP_AUTOMATED_TEST_REQUEST)
> -                       intel_dp_handle_test_request(intel_dp);
> +                       DRM_DEBUG_DRIVER("Test request in short pulse not handled\n");
>                 if (sink_irq_vector & (DP_CP_IRQ | DP_SINK_SPECIFIC_IRQ))
>                         DRM_DEBUG_DRIVER("CP or sink specific irq unhandled\n");
>         }
> @@ -4386,6 +4386,7 @@ intel_dp_detect(struct drm_connector *connector, bool force)
>         enum drm_connector_status status;
>         enum intel_display_power_domain power_domain;
>         bool ret;
> +       u8 sink_irq_vector;
>
>         DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n",
>                       connector->base.id, connector->name);
> @@ -4428,6 +4429,20 @@ intel_dp_detect(struct drm_connector *connector, bool force)
>                 intel_encoder->type = INTEL_OUTPUT_DISPLAYPORT;
>         status = connector_status_connected;
>
> +       /* Try to read the source of the interrupt */
> +       if (intel_dp->dpcd[DP_DPCD_REV] >= 0x11 &&
> +           intel_dp_get_sink_irq(intel_dp, &sink_irq_vector)) {
> +               /* Clear interrupt source */
> +               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");
> +       }
> +
>  out:
>         intel_dp_power_put(intel_dp, power_domain);
>         return status;
> --
> 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 2ef05d7..b249ee8 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4156,7 +4156,7 @@  intel_dp_check_link_status(struct intel_dp *intel_dp)
 				   sink_irq_vector);
 
 		if (sink_irq_vector & DP_AUTOMATED_TEST_REQUEST)
-			intel_dp_handle_test_request(intel_dp);
+			DRM_DEBUG_DRIVER("Test request in short pulse not handled\n");
 		if (sink_irq_vector & (DP_CP_IRQ | DP_SINK_SPECIFIC_IRQ))
 			DRM_DEBUG_DRIVER("CP or sink specific irq unhandled\n");
 	}
@@ -4386,6 +4386,7 @@  intel_dp_detect(struct drm_connector *connector, bool force)
 	enum drm_connector_status status;
 	enum intel_display_power_domain power_domain;
 	bool ret;
+	u8 sink_irq_vector;
 
 	DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n",
 		      connector->base.id, connector->name);
@@ -4428,6 +4429,20 @@  intel_dp_detect(struct drm_connector *connector, bool force)
 		intel_encoder->type = INTEL_OUTPUT_DISPLAYPORT;
 	status = connector_status_connected;
 
+	/* Try to read the source of the interrupt */
+	if (intel_dp->dpcd[DP_DPCD_REV] >= 0x11 &&
+	    intel_dp_get_sink_irq(intel_dp, &sink_irq_vector)) {
+		/* Clear interrupt source */
+		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");
+	}
+
 out:
 	intel_dp_power_put(intel_dp, power_domain);
 	return status;