Re: [PATCH 08/17] drm/i915: Wait for PHY port ready before link training on VLV/CHV

Todd Previte
2014-11-04T21:58:18Z

On 10/28/2014 1:15 AM, Daniel Vetter wrote:
> On Wed, Oct 22, 2014 at 08:10:40AM -0700, Todd Previte wrote:
>> On 10/16/2014 11:27 AM, ville.syrjala@linux.intel.com wrote:
>>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>>
>>> There's no point in checking if the data lanes came out of reset after
>>> link training. If the data lanes aren't ready link training will fail
>>> anyway.
>>>
>>> Suggested-by: Todd Previte <tprevite@gmail.com>
>>> Cc: Todd Previte <tprevite@gmail.com>
>>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>> ---
>>> If Todd has a better patch with better description we can use that one
>>> instead of my version. But at least I think the spot where I put the
>>> vlv_wait_port_ready() is the right one. We could perhaps skip the link
>>> training attempt entirely if the port is already stuck.
>>>
>>> drivers/gpu/drm/i915/intel_dp.c | 7 +++----
>>> 1 file changed, 3 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>>> index 4c8f169..6f568b4 100644
>>> --- a/drivers/gpu/drm/i915/intel_dp.c
>>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>>> @@ -2550,6 +2550,9 @@ static void intel_enable_dp(struct intel_encoder *encoder)
>>> pps_unlock(intel_dp);
>>> + if (IS_VALLEYVIEW(dev))
>>> + vlv_wait_port_ready(dev_priv, dp_to_dig_port(intel_dp));
>>> +
>>> intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON);
>>> intel_dp_start_link_train(intel_dp);
>>> intel_dp_complete_link_train(intel_dp);
>>> @@ -2689,8 +2692,6 @@ static void vlv_pre_enable_dp(struct intel_encoder *encoder)
>>> mutex_unlock(&dev_priv->dpio_lock);
>>> intel_enable_dp(encoder);
>>> -
>>> - vlv_wait_port_ready(dev_priv, dport);
>>> }
>>> static void vlv_dp_pre_pll_enable(struct intel_encoder *encoder)
>>> @@ -2783,8 +2784,6 @@ static void chv_pre_enable_dp(struct intel_encoder *encoder)
>>> mutex_unlock(&dev_priv->dpio_lock);
>>> intel_enable_dp(encoder);
>>> -
>>> - vlv_wait_port_ready(dev_priv, dport);
>>> }
>>> static void chv_dp_pre_pll_enable(struct intel_encoder *encoder)
>> We should definitely skip link training if the PHYs are down. There's going
>> to be a WARN when wait_port_ready() fails so we'll be well aware that
>> something went wrong. Spamming dmesg with errors/WARNs from trying to train
>> the link after that is really counterproductive, since we already know
>> there's no way link training could succeed.
> I'm taking this as an ack for Ville's patch, especially since the
> discussion around your patch to avoid link-training at other places is
> still ongoing.
> -Daniel
Sounds good. Once the avoiding link training part is resolved, it's a
minor follow-up patch to implement it.

-T