[Intel-gfx] [PATCH v2] drm/i915/edp: Read link status after exit link training

[Lee, Shawn C]

On 28/04/2017 10:58, Lee, Shawn C wrote:
>> From: "Lee, Shawn C" <shawn.c.lee at intel.com>
>>
>> Display driver read DPCD register 0x202, 0x203 and 0x204 to identify 
>> eDP sink status. If PSR exit is ongoing at eDP sink, and eDP source 
>> read these registers at the same time. Panel will report EQ & symbol 
>> lock not done. It will cause panel display flicking.
>> So driver have to make sure PSR already exit before read link status.
>>
>> Change log:
>> v2:
>> - Use intel_wait_for_register() to replace I915_READ().
>>
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99639
>> TEST=Reboot DUT and no flicking on local display at login screen
>>
>> Cc: Cooper Chiou <cooper.chiou at intel.com>
>> Cc: Gary C Wang <gary.c.wang at intel.com>
>> Cc: Jani Nikula <jani.nikula at intel.com>
>> Cc: Rodrigo Vivi <rodrigo.vivi at intel.com>
>> Cc: Jim Bride <jim.bride at intel.com>
>> Cc: Ryan Lin <ryan.lin at intel.com>
>>
>> Signed-off-by: Shawn Lee <shawn.c.lee at intel.com>
>> ---
>>  drivers/gpu/drm/i915/intel_dp.c |   33 ++++++++++++++++++++++++++++-----
>>  1 file changed, 28 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_dp.c 
>> b/drivers/gpu/drm/i915/intel_dp.c index 08834f74d396..099b6c0605a7 
>> 100644
>> --- a/drivers/gpu/drm/i915/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>> @@ -4252,19 +4252,34 @@ static void 
>> intel_dp_handle_test_request(struct intel_dp *intel_dp)  }
>>
>>  static void
>> +intel_edp_wait_PSR_exit(struct intel_dp *intel_dp) {
>> +	struct drm_device *dev = intel_dp_to_dev(intel_dp);
>> +	struct drm_i915_private *dev_priv = dev->dev_private;
> 
>.... dev_priv = to_i915(dev);
>

I will use to_i915() to replace it at patch v3.

>> +	u16 count = 100;
>> +
>> +	while (count--)
>> +		if (!intel_wait_for_register(dev_priv,
>> +			    EDP_PSR_STATUS_CTL,
>> +			    (EDP_PSR_STATUS_SENDING_TP1 |
>> +			     EDP_PSR_STATUS_SENDING_TP2_TP3 |
>> +			     EDP_PSR_STATUS_SENDING_IDLE |
>> +			     EDP_PSR_STATUS_AUX_SENDING),
>> +			     0,
>> +			     1))
>> +			return;
>> +}
>
>You don't need your own retry loop - intel_wait_for_register will do that for you. But it will only check every millisecond, if the inital busy wait for 2us wasn't successful.
>
>I've  noticed that your initial patch had a 10ms timeout with a 100us period. If you had a good reason for that, then with this conversion you only have a 1ms timeout with a 1ms period, so if you need a shorter period, this solution is not ideal for you.
>
>I've complicated the explanation a bit - but a bottom line is, v1 and v2 are a lot different. Depending on the typical response time one of the two will have a much worse latency.
>

Here is a little tricky to know when sink's PSR state transit to "PSR inactive" and timing re-sync is complete after PSR exit. 
So if SRD_STATUS shows TP1, TP2, TP3 or idle pattern was not sent to sink side. That means Main-Link Training is not ongoing.
But the time for link training is hard to predict. So use a loop to monitor it every 1ms until link training was done.

>> +
>> +static void
>>  intel_dp_check_link_status(struct intel_dp *intel_dp)  {
>>  	struct intel_encoder *intel_encoder = &dp_to_dig_port(intel_dp)->base;
>>  	struct drm_device *dev = intel_dp_to_dev(intel_dp);
>> +	struct drm_i915_private *dev_priv = dev->dev_private;
>
>to_i915 also.
>

I will use to_i915() to replace it at patch v3.

>>  	u8 link_status[DP_LINK_STATUS_SIZE];
>>
>>  	WARN_ON(!drm_modeset_is_locked(&dev->mode_config.connection_mutex));
>>
>> -	if (!intel_dp_get_link_status(intel_dp, link_status)) {
>> -		DRM_ERROR("Failed to get link status\n");
>> -		return;
>> -	}
>> -
>>  	if (!intel_encoder->base.crtc)
>>  		return;
>>
>> @@ -4278,6 +4293,14 @@ static void intel_dp_handle_test_request(struct intel_dp *intel_dp)
>>  	if (!intel_dp_link_params_valid(intel_dp))
>>  		return;
>>
>> +	if (is_edp(intel_dp) && dev_priv->psr.enabled)
>> +		intel_edp_wait_PSR_exit(intel_dp);
>> +
>> +	if (!intel_dp_get_link_status(intel_dp, link_status)) {
>> +		DRM_ERROR("Failed to get link status\n");
>> +		return;
>> +	}
>> +
>>  	/* 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",
>
>
>Regards,
>
>Tvrtko