Re: New async patch for resume

[Jesse Barnes <jbarnes@virtuousgeek.org>]

On Fri, 6 Jun 2014 18:36:46 +0200
Daniel Vetter <daniel@ffwll.ch> wrote:

> On Thu, Jun 05, 2014 at 08:48:37AM -0700, Jesse Barnes wrote:
> > In digging into the async crtc stuff, I found it was going to be really
> > difficult to prevent other functions from getting clobbered by a delayed
> > crtc enable or disable.  Daniel's work to remove a bunch of the
> > ->mode_set callbacks is a good start, but we still end up doing some reg
> > reads and writes in the mode set path today.  Until those are cleared up
> > and we somehow enforce a rule that all hw changes go through the crtc
> > enable/disable paths with everything else staged in multithread safe structs,
> > I don't think the async crtc approach will be solid.
> 
> Just a quick note: Once the runtim pm for dpms code is in, there's 0 hw
> touching going on in mode_set callbacks. The only thing we still do in
> there is computing the dpll config.
> 
> So if this is somehow annoying your feature work I think we should wait a
> bit until that's landed.

I still worry about synchronizing multiple delayed CRTC enable/disable
calls vs. mode set and struct synchronization... I need to think about
it harder.

> 
> > So, since resume is the biggest issue here anyway, I've tried making
> > just the resume mode set asynchronous.  Even this is a bit tricky, since
> > we need to apply any pending mode set at certain points, then check
> > whether the crtc we're operating on in any given path is still active.
> > I think I've caught those cases here, but if we have more we can use the
> > intel_sync_mode_set() call with appropriate post-call checks (after we
> > re-acquire the corresponding crtc mutex).
> > 
> > Feedback welcome.  This has seen light testing on my BYT and really
> > reduces the time spend in the i915 _thaw function, letting userspace
> > start running much sooner than before.
> 
> I think we've very quickly discussed this on irc, so here's the recap. Rob
> Clark's atomic modeset also has an async mode. And we have an awful lot of
> code to port over to atomic modeset, and I'm vary of adding more i915
> custom solutions which some poor schlock (Ville or me, most likely) needs
> to convert.
> 
> So from that pov I'd really prefer we'd pour this effort into pushing
> Rob's and Ville's patches forward. Of course the delaying probing can go
> in, and also the other stuff you're working on I think.

I was dreaming about this last night and I think that's really the only
safe way to do it.  I was trying to avoid touching the top level DRM
code, but really I think that's the only way to avoid races or
introduce all sorts of trickery.

I don't want to wait long though... I guess I'll pull up Rob's latest
stuff and see if there's anything I can to do help move things along.

-- 
Jesse Barnes, Intel Open Source Technology Center