Re: [PATCH] drm/i915: Add a bit of locking to intel_dp_hpd_pulse()

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Thu, Jul 31, 2014 at 08:59:08PM +1000, Dave Airlie wrote:
> On 31 July 2014 17:37, Daniel Vetter <daniel@ffwll.ch> wrote:
> > On Thu, Jul 31, 2014 at 1:49 AM, Dave Airlie <airlied@gmail.com> wrote:
> >> Daniel, the only way intel_dp->is_mst can get reset is inside this path.
> >
> > Ok, so that one should be safe. Then I guess we can just push the
> > locking down into the respective non-mst leafs (since atm we do
> > link-retraining without any locking, which isn't good). And it needs
> > to be dev->mode_config.mutex, not connection mutex.

Why that? We can't be doing a modeset w/o connection_mutex so that
seems like it should be enough. Well, there's also dpms which leaves
the crtc<->encoder<->connector links intact but that too takes
connection_mutex currently.

> 
> I'd like to know why we do link training at this point though as well,
> adding locking is required of course, I was just going to wrap the
> short irq call to the link status check with the lock, but I think it
> should be possible to push it down further,

I don't really know why the sink generates the hpd when we turn off the
port, but that doesn't really matter I think. We need to be prepared for
hpds at any time.

intel_dp_check_link_status() just checks if there's a crtc, which there
is (either the old one or the new one, depending on how far along the
modeset path we are I guess), and then it just checks
drm_dp_channel_eq_ok() which says false since the link was turned off,
and then it proceeds to retrain the link.

Maybe it should also check crtc->active? Though that itself won't 
eliminate the problem unless the locking gets fixed somehow.

> 
> I'm not sure how vague the spec is on what should happen on HPDs, but
> if we drop the port clock we obviously will lose the link, but we
> should also know not to be retraining it at that point anyways.
> 
> Dave.

-- 
Ville Syrjälä
Intel OTC