[PATCH v2 1/2] drm/i915/dp: Generalize intel_dp_link_params function to accept arguments to be validated

[PATCH v2 1/2] drm/i915/dp: Generalize intel_dp_link_params function to accept arguments to be validated

[Manasi Navare]

This function now takes the link rate and lane ocunt to be validated
as an argument so that this can be used for validating even the
compliance test link parameters.

Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Reviewed-by: Jani Nikula <jani.nikula@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index db51338..dd01ab8 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -322,19 +322,20 @@ static int intel_dp_common_len_rate_limit(struct intel_dp *intel_dp,
 	return 0;
 }
 
-static bool intel_dp_link_params_valid(struct intel_dp *intel_dp)
+static bool intel_dp_link_params_valid(struct intel_dp *intel_dp, int link_rate,
+				       uint8_t lane_count)
 {
 	/*
 	 * FIXME: we need to synchronize the current link parameters with
 	 * hardware readout. Currently fast link training doesn't work on
 	 * boot-up.
 	 */
-	if (intel_dp->link_rate == 0 ||
-	    intel_dp->link_rate > intel_dp->max_link_rate)
+	if (link_rate == 0 ||
+	    link_rate > intel_dp->max_link_rate)
 		return false;
 
-	if (intel_dp->lane_count == 0 ||
-	    intel_dp->lane_count > intel_dp_max_lane_count(intel_dp))
+	if (lane_count == 0 ||
+	    lane_count > intel_dp_max_lane_count(intel_dp))
 		return false;
 
 	return true;
@@ -4260,7 +4261,8 @@ intel_dp_check_link_status(struct intel_dp *intel_dp)
 	 * Validate the cached values of intel_dp->link_rate and
 	 * intel_dp->lane_count before attempting to retrain.
 	 */
-	if (!intel_dp_link_params_valid(intel_dp))
+	if (!intel_dp_link_params_valid(intel_dp, intel_dp->link_rate,
+					intel_dp->lane_count))
 		return;
 
 	/* Retrain if Channel EQ or CR not ok */
-- 
2.1.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2 2/2] drm/i915/dp: Validate the compliance test link parameters

[Manasi Navare]

Validate the compliance test link parameters when the compliance
test dpcd registers are read. Also validate them in compute_config
before using them since the max values might have been reduced
due to link training fallback.

If either the link rate or lane count is invalid, we still bail
from using the test parameters since the combination would not work
and instead use the fallback values.

v2:
* Added commit message to explain why we still bail when either of
of the params is invalid (Ville Syrjala)
* Add reason for validating in the comment (Jani Nikula)
* Also check if index >= 0 after validating (Jani Nikula)

Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 34 +++++++++++++++++-----------------
 1 file changed, 17 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index dd01ab8..23dbdef 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1678,12 +1678,18 @@ intel_dp_compute_config(struct intel_encoder *encoder,
 	if (intel_dp->compliance.test_type == DP_TEST_LINK_TRAINING) {
 		int index;
 
-		index = intel_dp_rate_index(intel_dp->common_rates,
-					    intel_dp->num_common_rates,
-					    intel_dp->compliance.test_link_rate);
-		if (index >= 0)
-			min_clock = max_clock = index;
-		min_lane_count = max_lane_count = intel_dp->compliance.test_lane_count;
+		/* Validate the compliance test data since max values
+		 * might have changed due to link train fallback.
+		 */
+		if (intel_dp_link_params_valid(intel_dp, intel_dp->compliance.test_link_rate,
+					       intel_dp->compliance.test_lane_count)) {
+			index = intel_dp_rate_index(intel_dp->common_rates,
+						    intel_dp->num_common_rates,
+						    intel_dp->compliance.test_link_rate);
+			if (index >= 0)
+				min_clock = max_clock = index;
+			min_lane_count = max_lane_count = intel_dp->compliance.test_lane_count;
+		}
 	}
 	DRM_DEBUG_KMS("DP link computation with max lane count %i "
 		      "max bw %d pixel clock %iKHz\n",
@@ -3961,8 +3967,7 @@ intel_dp_get_sink_irq_esi(struct intel_dp *intel_dp, u8 *sink_irq_vector)
 static uint8_t intel_dp_autotest_link_training(struct intel_dp *intel_dp)
 {
 	int status = 0;
-	int min_lane_count = 1;
-	int link_rate_index, test_link_rate;
+	int test_link_rate;
 	uint8_t test_lane_count, test_link_bw;
 	/* (DP CTS 1.2)
 	 * 4.3.1.11
@@ -3976,10 +3981,6 @@ static uint8_t intel_dp_autotest_link_training(struct intel_dp *intel_dp)
 		return DP_TEST_NAK;
 	}
 	test_lane_count &= DP_MAX_LANE_COUNT_MASK;
-	/* Validate the requested lane count */
-	if (test_lane_count < min_lane_count ||
-	    test_lane_count > intel_dp->max_link_lane_count)
-		return DP_TEST_NAK;
 
 	status = drm_dp_dpcd_readb(&intel_dp->aux, DP_TEST_LINK_RATE,
 				   &test_link_bw);
@@ -3987,12 +3988,11 @@ static uint8_t intel_dp_autotest_link_training(struct intel_dp *intel_dp)
 		DRM_DEBUG_KMS("Link Rate read failed\n");
 		return DP_TEST_NAK;
 	}
-	/* Validate the requested link rate */
 	test_link_rate = drm_dp_bw_code_to_link_rate(test_link_bw);
-	link_rate_index = intel_dp_rate_index(intel_dp->common_rates,
-					      intel_dp->num_common_rates,
-					      test_link_rate);
-	if (link_rate_index < 0)
+
+	/* Validate the requested link rate and lane count */
+	if (!intel_dp_link_params_valid(intel_dp, test_link_rate,
+					test_lane_count))
 		return DP_TEST_NAK;
 
 	intel_dp->compliance.test_lane_count = test_lane_count;
-- 
2.1.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for series starting with [v2,1/2] drm/i915/dp: Generalize intel_dp_link_params function to accept arguments to be validated

[Patchwork]

== Series Details ==

Series: series starting with [v2,1/2] drm/i915/dp: Generalize intel_dp_link_params function to accept arguments to be validated
URL   : https://patchwork.freedesktop.org/series/25506/
State : success

== Summary ==

Series 25506v1 Series without cover letter
https://patchwork.freedesktop.org/api/1.0/series/25506/revisions/1/mbox/

fi-bdw-5557u     total:278  pass:267  dwarn:0   dfail:0   fail:0   skip:11  time:444s
fi-bdw-gvtdvm    total:278  pass:256  dwarn:8   dfail:0   fail:0   skip:14  time:438s
fi-bsw-n3050     total:278  pass:242  dwarn:0   dfail:0   fail:0   skip:36  time:578s
fi-bxt-j4205     total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19  time:518s
fi-byt-j1900     total:278  pass:254  dwarn:0   dfail:0   fail:0   skip:24  time:487s
fi-byt-n2820     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time:485s
fi-glk-2a        total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19  time:594s
fi-hsw-4770      total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time:435s
fi-hsw-4770r     total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time:408s
fi-ilk-650       total:278  pass:228  dwarn:0   dfail:0   fail:0   skip:50  time:422s
fi-ivb-3520m     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:496s
fi-ivb-3770      total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:464s
fi-kbl-7500u     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:468s
fi-kbl-7560u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time:562s
fi-kbl-r         total:278  pass:259  dwarn:1   dfail:0   fail:0   skip:18  time:572s
fi-skl-6260u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time:459s
fi-skl-6700hq    total:278  pass:228  dwarn:1   dfail:0   fail:27  skip:22  time:409s
fi-skl-6700k     total:278  pass:256  dwarn:4   dfail:0   fail:0   skip:18  time:470s
fi-skl-6770hq    total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time:494s
fi-skl-gvtdvm    total:278  pass:265  dwarn:0   dfail:0   fail:0   skip:13  time:443s
fi-snb-2520m     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time:539s
fi-snb-2600      total:278  pass:249  dwarn:0   dfail:0   fail:0   skip:29  time:407s

5b363842b49a66879d4c67d652923fb04e44b271 drm-tip: 2017y-06m-08d-20h-34m-37s UTC integration manifest
5dc27c5 drm/i915/dp: Validate the compliance test link parameters
1d08555 drm/i915/dp: Generalize intel_dp_link_params function to accept arguments to be validated

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4916/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 1/2] drm/i915/dp: Generalize intel_dp_link_params function to accept arguments to be validated

[Manasi Navare]

Can this be merged? It has a r-b from  Jani Nikula.


On Thu, Jun 08, 2017 at 01:41:02PM -0700, Manasi Navare wrote:
> This function now takes the link rate and lane ocunt to be validated
> as an argument so that this can be used for validating even the
> compliance test link parameters.
> 
> Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Reviewed-by: Jani Nikula <jani.nikula@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c | 14 ++++++++------
>  1 file changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index db51338..dd01ab8 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -322,19 +322,20 @@ static int intel_dp_common_len_rate_limit(struct intel_dp *intel_dp,
>  	return 0;
>  }
>  
> -static bool intel_dp_link_params_valid(struct intel_dp *intel_dp)
> +static bool intel_dp_link_params_valid(struct intel_dp *intel_dp, int link_rate,
> +				       uint8_t lane_count)
>  {
>  	/*
>  	 * FIXME: we need to synchronize the current link parameters with
>  	 * hardware readout. Currently fast link training doesn't work on
>  	 * boot-up.
>  	 */
> -	if (intel_dp->link_rate == 0 ||
> -	    intel_dp->link_rate > intel_dp->max_link_rate)
> +	if (link_rate == 0 ||
> +	    link_rate > intel_dp->max_link_rate)
>  		return false;
>  
> -	if (intel_dp->lane_count == 0 ||
> -	    intel_dp->lane_count > intel_dp_max_lane_count(intel_dp))
> +	if (lane_count == 0 ||
> +	    lane_count > intel_dp_max_lane_count(intel_dp))
>  		return false;
>  
>  	return true;
> @@ -4260,7 +4261,8 @@ intel_dp_check_link_status(struct intel_dp *intel_dp)
>  	 * Validate the cached values of intel_dp->link_rate and
>  	 * intel_dp->lane_count before attempting to retrain.
>  	 */
> -	if (!intel_dp_link_params_valid(intel_dp))
> +	if (!intel_dp_link_params_valid(intel_dp, intel_dp->link_rate,
> +					intel_dp->lane_count))
>  		return;
>  
>  	/* Retrain if Channel EQ or CR not ok */
> -- 
> 2.1.4
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 1/2] drm/i915/dp: Generalize intel_dp_link_params function to accept arguments to be validated

[Jani Nikula]

On Mon, 12 Jun 2017, Manasi Navare <manasi.d.navare@intel.com> wrote:
> Can this be merged? It has a r-b from  Jani Nikula.

Both pushed, sorry for the delay.

BR,
Jani.

>
>
> On Thu, Jun 08, 2017 at 01:41:02PM -0700, Manasi Navare wrote:
>> This function now takes the link rate and lane ocunt to be validated
>> as an argument so that this can be used for validating even the
>> compliance test link parameters.
>> 
>> Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
>> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
>> Cc: Jani Nikula <jani.nikula@linux.intel.com>
>> Reviewed-by: Jani Nikula <jani.nikula@linux.intel.com>
>> ---
>>  drivers/gpu/drm/i915/intel_dp.c | 14 ++++++++------
>>  1 file changed, 8 insertions(+), 6 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>> index db51338..dd01ab8 100644
>> --- a/drivers/gpu/drm/i915/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>> @@ -322,19 +322,20 @@ static int intel_dp_common_len_rate_limit(struct intel_dp *intel_dp,
>>  	return 0;
>>  }
>>  
>> -static bool intel_dp_link_params_valid(struct intel_dp *intel_dp)
>> +static bool intel_dp_link_params_valid(struct intel_dp *intel_dp, int link_rate,
>> +				       uint8_t lane_count)
>>  {
>>  	/*
>>  	 * FIXME: we need to synchronize the current link parameters with
>>  	 * hardware readout. Currently fast link training doesn't work on
>>  	 * boot-up.
>>  	 */
>> -	if (intel_dp->link_rate == 0 ||
>> -	    intel_dp->link_rate > intel_dp->max_link_rate)
>> +	if (link_rate == 0 ||
>> +	    link_rate > intel_dp->max_link_rate)
>>  		return false;
>>  
>> -	if (intel_dp->lane_count == 0 ||
>> -	    intel_dp->lane_count > intel_dp_max_lane_count(intel_dp))
>> +	if (lane_count == 0 ||
>> +	    lane_count > intel_dp_max_lane_count(intel_dp))
>>  		return false;
>>  
>>  	return true;
>> @@ -4260,7 +4261,8 @@ intel_dp_check_link_status(struct intel_dp *intel_dp)
>>  	 * Validate the cached values of intel_dp->link_rate and
>>  	 * intel_dp->lane_count before attempting to retrain.
>>  	 */
>> -	if (!intel_dp_link_params_valid(intel_dp))
>> +	if (!intel_dp_link_params_valid(intel_dp, intel_dp->link_rate,
>> +					intel_dp->lane_count))
>>  		return;
>>  
>>  	/* Retrain if Channel EQ or CR not ok */
>> -- 
>> 2.1.4
>> 

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