From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jesse Barnes Subject: Re: New async patch for resume Date: Fri, 6 Jun 2014 10:06:42 -0700 Message-ID: <20140606100642.030ebbb4@jbarnes-desktop> References: <1401983318-6347-1-git-send-email-jbarnes@virtuousgeek.org> <20140606163646.GH7416@phenom.ffwll.local> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail-pd0-f169.google.com (mail-pd0-f169.google.com [209.85.192.169]) by gabe.freedesktop.org (Postfix) with ESMTP id 62E8A6EA9A for ; Fri, 6 Jun 2014 10:06:39 -0700 (PDT) Received: by mail-pd0-f169.google.com with SMTP id w10so2664155pde.0 for ; Fri, 06 Jun 2014 10:06:39 -0700 (PDT) In-Reply-To: <20140606163646.GH7416@phenom.ffwll.local> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Daniel Vetter Cc: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Fri, 6 Jun 2014 18:36:46 +0200 Daniel Vetter 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