2014-12-10 21:53 GMT-02:00 Todd Previte <tprev...@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.
Why exactly is this needed?
>
> 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.
Why exactly? Can you please describe in terms of how the code is
executed in each case and what is missing?
Since there appears to be 2 different changes, shouldn't this patch be
split into 2 different patches?
> 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 <tprev...@gmail.com>
> ---
> drivers/gpu/drm/i915/intel_dp.c | 32 +++++++++++++++++++-------------
> 1 file changed, 19 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 3dc92a3..1b452cc 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -3890,21 +3890,14 @@ 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)) {
> + /* Bail if not connected */
Bikeshed: I don't see the reason for the comment above :)
Other than that, the patch looks fine. Let's see what PRTS will say about it.
> + if (intel_dp->attached_connector->base.status !=
> + connector_status_connected) {
> + DRM_DEBUG_KMS("Not connected\n");
> return;
> }
>
> - /* Now read the DPCD to see if it's actually running */
> + /* Attempt to read the DPCD */
> if (!intel_dp_get_dpcd(intel_dp)) {
> return;
> }
> @@ -3916,13 +3909,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
--
Paulo Zanoni
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
