Re: [PATCH 1/4] drm/i915: Add support for DP link training compliance

[Jani Nikula <jani.nikula@linux.intel.com>]

On Wed, 18 Jan 2017, Manasi Navare <manasi.d.navare@intel.com> wrote:
> This patch adds support to handle automated DP compliance
> link training test requests. This patch has been tested with
> Unigraf DPR-120 DP Compliance device for testing Link
> Training Compliance.
> After we get a short pulse Compliance test request, test
> request values are read and hotplug uevent is sent in order
> to trigger another modeset during which the pipe is configured
> and link is retrained and enabled for link parameters requested
> by the test.
>
> v2:
> * Validate the test lane count before using it in
> intel_dp_compute_config (Jani Nikula)
> Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Cc: Daniel Vetter <daniel.vetter@intel.com>
> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c  | 46 ++++++++++++++++++++++++++++++++++++----
>  drivers/gpu/drm/i915/intel_drv.h |  2 ++
>  2 files changed, 44 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index e80d620..7a1d3c4 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1613,6 +1613,7 @@ static int intel_dp_compute_bpp(struct intel_dp *intel_dp,
>  	/* Conveniently, the link BW constants become indices with a shift...*/
>  	int min_clock = 0;
>  	int max_clock;
> +	int link_rate_index;
>  	int bpp, mode_rate;
>  	int link_avail, link_clock;
>  	int common_rates[DP_MAX_SUPPORTED_RATES] = {};
> @@ -1654,6 +1655,17 @@ static int intel_dp_compute_bpp(struct intel_dp *intel_dp,
>  	if (adjusted_mode->flags & DRM_MODE_FLAG_DBLCLK)
>  		return false;
>  
> +	/* Use values requested by Compliance Test Request */
> +	if (intel_dp->compliance.test_type == DP_TEST_LINK_TRAINING) {
> +		link_rate_index = intel_dp_link_rate_index(intel_dp,
> +							   common_rates,
> +							   drm_dp_bw_code_to_link_rate(intel_dp->compliance.test_link_rate));
> +		if (link_rate_index >= 0)
> +			min_clock = max_clock = link_rate_index;
> +		if (min_lane_count <= intel_dp->compliance.test_lane_count
> +		    && intel_dp->compliance.test_lane_count >= max_lane_count)
> +			min_lane_count = max_lane_count = intel_dp->compliance.test_lane_count;
> +	}

Why don't we do these checks and conversions when we get the test
request, so we can NAK the request on errors?

>  	DRM_DEBUG_KMS("DP link computation with max lane count %i "
>  		      "max bw %d pixel clock %iKHz\n",
>  		      max_lane_count, common_rates[max_clock],
> @@ -3921,6 +3933,27 @@ int intel_dp_sink_crc(struct intel_dp *intel_dp, u8 *crc)
>  static uint8_t intel_dp_autotest_link_training(struct intel_dp *intel_dp)
>  {
>  	uint8_t test_result = DP_TEST_ACK;
> +	int status = 0;
> +	/* (DP CTS 1.2)
> +	 * 4.3.1.11
> +	 */
> +	/* Read the TEST_LANE_COUNT and TEST_LINK_RTAE fields (DP CTS 3.1.4) */
> +	status = drm_dp_dpcd_readb(&intel_dp->aux, DP_TEST_LANE_COUNT,
> +				  &intel_dp->compliance.test_lane_count);
> +
> +	if (status <= 0) {
> +		DRM_DEBUG_KMS("Lane count read failed\n");
> +		return 0;

return DP_TEST_NAK;

> +	}
> +	intel_dp->compliance.test_lane_count &= DP_MAX_LANE_COUNT_MASK;
> +
> +	status = drm_dp_dpcd_readb(&intel_dp->aux, DP_TEST_LINK_RATE,
> +				   &intel_dp->compliance.test_link_rate);
> +	if (status <= 0) {
> +		DRM_DEBUG_KMS("Link Rate read failed\n");
> +		return 0;

return DP_TEST_NAK;

> +	}
> +
>  	return test_result;

Could replace this with a direct return DP_TEST_ACK without the temp
variable.

>  }
>  
> @@ -4135,9 +4168,8 @@ static void intel_dp_handle_test_request(struct intel_dp *intel_dp)
>  	if (!intel_dp->lane_count)
>  		return;
>  
> -	/* if link training is requested we should perform it always */
> -	if ((intel_dp->compliance.test_type == DP_TEST_LINK_TRAINING) ||
> -	    (!drm_dp_channel_eq_ok(link_status, intel_dp->lane_count))) {
> +	/* Retrain if Channel EQ or CR not ok */
> +	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);
>  
> @@ -4162,6 +4194,7 @@ static void intel_dp_handle_test_request(struct intel_dp *intel_dp)
>  intel_dp_short_pulse(struct intel_dp *intel_dp)
>  {
>  	struct drm_device *dev = intel_dp_to_dev(intel_dp);
> +	struct intel_encoder *intel_encoder = &dp_to_dig_port(intel_dp)->base;
>  	u8 sink_irq_vector = 0;
>  	u8 old_sink_count = intel_dp->sink_count;
>  	bool ret;
> @@ -4195,7 +4228,7 @@ static void intel_dp_handle_test_request(struct intel_dp *intel_dp)
>  				   sink_irq_vector);
>  
>  		if (sink_irq_vector & DP_AUTOMATED_TEST_REQUEST)
> -			DRM_DEBUG_DRIVER("Test request in short pulse not handled\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");
>  	}
> @@ -4203,6 +4236,11 @@ static void intel_dp_handle_test_request(struct intel_dp *intel_dp)
>  	drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
>  	intel_dp_check_link_status(intel_dp);
>  	drm_modeset_unlock(&dev->mode_config.connection_mutex);
> +	if (intel_dp->compliance.test_type == DP_TEST_LINK_TRAINING) {
> +		DRM_DEBUG_KMS("Link Training Compliance Test requested\n");
> +		/* Send a Hotplug Uevent to userspace to start modeset */
> +		drm_kms_helper_hotplug_event(intel_encoder->base.dev);
> +	}
>  
>  	return true;
>  }
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 84258df..7a2eb76 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -893,6 +893,8 @@ struct intel_dp_compliance {
>  	unsigned long test_type;
>  	struct intel_dp_compliance_data test_data;
>  	bool test_active;
> +	u8 test_link_rate;
> +	u8 test_lane_count;
>  };
>  
>  struct intel_dp {

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx