[Intel-gfx] [PATCH] drm/i915: Don't recheck link status for eDP display.

[Puthikorn Voravootivat]

I tried  https://patchwork.freedesktop.org/series/30670/ but it doesn't work.

> When you see the display blinking, do you know for sure that drm_dp_channel_eq_ok() returns false?
Yes. Repro is running "modetest" while PSR is on (easier to repro with
PSR2 but PSR1 is fine too)

> lane_align = dp_link_status(link_status,
>    DP_LANE_ALIGN_STATUS_UPDATED);
> if ((lane_align & DP_INTERLANE_ALIGN_DONE) == 0)
>    return false;

I always got lane_align = 128 (DP_LINK_STATUS_UPDATED)
So drm_dp_channel_eq_ok() always return false.

If I disable PSR, drm_dp_channel_eq_ok() will return true and no
blinking occurs.

On Tue, Oct 17, 2017 at 2:21 PM, Manasi Navare
<manasi.d.navare at intel.com> wrote:
> On Tue, Oct 17, 2017 at 02:01:56PM -0700, Puthikorn Voravootivat wrote:
>> intel_dp_long_pulse() is always checking link status because
>> there has been known issues of link loss triggerring long pulse.
>>
>> However this is not needed for eDP display since we won't have
>> link loss for internal display. Also there are reports that
>> screens are flickering during link status check. (repro by
>> running modetest command repeatedly)
>>
>> Signed-off-by: Puthikorn Voravootivat <puthik at chromium.org>
>> ---
>>  drivers/gpu/drm/i915/intel_dp.c | 7 ++++++-
>>  1 file changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>> index 4b65cf137f79..75a77ef257e2 100644
>> --- a/drivers/gpu/drm/i915/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>> @@ -4763,7 +4763,8 @@ intel_dp_long_pulse(struct intel_connector *intel_connector)
>>                */
>>               status = connector_status_disconnected;
>>               goto out;
>> -     } else {
>> +     } else if (status != connector_status_connected ||
>> +                intel_encoder->type != INTEL_OUTPUT_EDP) {
>>               /*
>>                * If display is now connected check links status,
>>                * there has been known issues of link loss triggerring
>> @@ -4775,6 +4776,10 @@ intel_dp_long_pulse(struct intel_connector *intel_connector)
>>                * going back up soon after. And once that happens we must
>>                * retrain the link to get a picture. That's in case no
>>                * userspace component reacted to intermittent HPD dip.
>> +              *
>> +              * Skip checking links status for connected eDP display.
>> +              * There are known issues of display blinking during checking
>> +              * link status and we don't have link loss for internal display.
>>                */
>
> Inside intel_dp_check_link_status(), it actually checks if link status is good by
> checking both clock recovery and Channel EQ bits in DPCD, only then it will retrain.
> So in case of eDP panel, if there is no link loss then it should always return link
> status as good and not retrain.
> So IMHO I dont think we need to explicitly avoid this for eDP.
>
> When you see the display blinking, do you know for sure that drm_dp_channel_eq_ok() returns false?
> Also look at this patch: https://patchwork.freedesktop.org/series/30670/
> This makes sure none of the eDP training parameters change unless link is bad. So with this patch, if the
> link is good then it should never retrain even in intel_dp_check_link_status() for eDP.
>
> Manasi
>>               intel_dp_check_link_status(intel_dp);
>>       }
>> --
>> 2.15.0.rc0.271.g36b669edcc-goog
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx