From owner-svn-src-projects@FreeBSD.ORG Fri May 24 15:04:36 2013 Return-Path: Delivered-To: svn-src-projects@freebsd.org Received: from mx1.freebsd.org (mx1.FreeBSD.org [8.8.178.115]) by hub.freebsd.org (Postfix) with ESMTP id 0BA3698E; Fri, 24 May 2013 15:04:36 +0000 (UTC) (envelope-from chmeeedalf@gmail.com) Received: from mail-bk0-x235.google.com (mail-bk0-x235.google.com [IPv6:2a00:1450:4008:c01::235]) by mx1.freebsd.org (Postfix) with ESMTP id 410637E8; Fri, 24 May 2013 15:04:35 +0000 (UTC) Received: by mail-bk0-f53.google.com with SMTP id mx1so2609513bkb.12 for ; Fri, 24 May 2013 08:04:34 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:sender:in-reply-to:references:date :x-google-sender-auth:message-id:subject:from:to:cc:content-type; bh=4/SBpLcFQqHhXZQ0kQA1TRP6la8tONv/b+cV5MQPbqI=; b=ZS4objQQfIg1dACjyJac8jxVBH7dZaE/drpkai5xPrnYIhg9hJp8cGLwzMEDOvlbBQ tW71vCxRLTLxxqAlAKSk00kLBaAO8P/cxOrmOxmHnay/+EMy5Wxc9FyI72WI5aIN07P8 FgxU8SeOJo3wTy7y1Tca9W4E516HS1wVRDiSgCqEkBhBN4/hqN4+PPew/FyzGTnJna4p uj5PEUuA78wO6c0t6smYnqBfFI5ULCwiqI4usaANkJ4F0DldLWZafyz/nH08usXRcLXY 9AjzAOlx+HXN4QdbgEK9zGctIYUrPNjiAn8ZnzTNrNWsdKMRsr+quqMyS8g6s932ONEU MEwQ== MIME-Version: 1.0 X-Received: by 10.204.173.9 with SMTP id n9mr9350970bkz.47.1369407874235; Fri, 24 May 2013 08:04:34 -0700 (PDT) Sender: chmeeedalf@gmail.com Received: by 10.204.38.144 with HTTP; Fri, 24 May 2013 08:04:34 -0700 (PDT) In-Reply-To: <201305240849.27877.jhb@freebsd.org> References: <201305240358.r4O3wVw5026183@svn.freebsd.org> <201305240849.27877.jhb@freebsd.org> Date: Fri, 24 May 2013 08:04:34 -0700 X-Google-Sender-Auth: UC-o2ZwT9uWI-_yEV9e92J6uAOY Message-ID: Subject: Re: svn commit: r250956 - projects/pmac_pmu/sys/kern From: Justin Hibbits To: John Baldwin Content-Type: text/plain; charset=ISO-8859-1 X-Content-Filtered-By: Mailman/MimeDel 2.1.14 Cc: svn-src-projects@freebsd.org, src-committers@freebsd.org X-BeenThere: svn-src-projects@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: "SVN commit messages for the src " projects" tree" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 24 May 2013 15:04:36 -0000 On Fri, May 24, 2013 at 5:49 AM, John Baldwin wrote: > On Thursday, May 23, 2013 11:58:31 pm Justin Hibbits wrote: > > Author: jhibbits > > Date: Fri May 24 03:58:31 2013 > > New Revision: 250956 > > URL: http://svnweb.freebsd.org/changeset/base/250956 > > > > Log: > > Add multipass for suspend/resume. This allows some devices to be > resumed before > > others, even their peers, as required by PowerPC Mac hardware. > > I think this is a good start. One question I have is why not reuse the > pass number > from the driver instead of stuffing it in the devclass? (It is > conceivable that > different drivers with different passes might share a devclass even, > though unlikely). > > That is, can't you use child->driver->dl_pass directly? (If a child > doesn't have a > driver it can't suspend or resume anyway). > I was going to do this, but I don't see a dl_pass in the driver_t structure, the pass number currently goes into the driverlink list, so this was the quickest way to add and test this, as well as the most nonintrusive. I can change the driver_t definition to add a pass, though, it's not difficult, but would change the module ABI, since the driver definition is public. > Also, can you explain the EAGAIN logic? It's not obvious. Is this how > you are > enforcing that already resumed drivers keep traversing their tree in > subsequent > passes (if not you need to deal with that in some fashion). > Yes, I use EAGAIN to mean "Make another pass at this". I discussed it with Warner at BSDCan and that seemed a simple solution. Drivers that are pass-aware can check bus_current_pass for the pass they want to resume themselves on, and just call bus_generic_resume() if it's not their pass. Drivers that are not pass aware would behave as they do now. > I think we might want to be more explicit about forcing suspend and resume > to > traverse the tree for each pass. The boot-time probe has a BUS_NEW_PASS > callback > it uses for this, and bus_generic_new_pass() is what it uses for that. I > think > you might want to create BUS_SUSPEND_PASS and BUS_RESUME_PASS with generic > versions similar to bus_generic_new_pass. You would check the DF_SUSPENDED > flag there to decide whether or not you wanted to call > BUS_SUSPEND/RESUME_PASS > or DEVICE_SUSPEND(). > I did consider this as well. However, I'm not certain what that would add over this way. I found this to be relatively nonintrusive and mostly straight forward. The other thing that makes this more complicated is that unlike > probe/attach, > we might need to actually ask the bus to suspend and resume the device so > that > the bus can do power management. Right now this works because the bus > devices > suspend everything in one pass so they can order things correctly (e.g. > pci_suspend()). If we want to shut down some devices early we might > consider > having a new bus_if method which takes a child and handles suspending that > specific child (it would call DEVICE_SUSPEND). For PCI you might get > something > like this: > > int > pci_suspend_child(device_t bus, device_t child) > { > struct pci_devinfo *dinfo; > int error; > > dinfo = device_get_ivars(child); > pci_cfg_save(child, dinfo, 0); > error = DEVICE_SUSPEND(child); > if (error) > return (error); > if (pci_do_power_suspend) > /* set power state for just this child to D3 */ > return (error); > } > > I need to think a bit more, but I think this is more of a correct model to > handle > passes, and will also be cleaner for suspend/resume in general. I agree that adding the ability to suspend children independently is a good idea. I'm interested in any more thoughts you have on this, so we could get it working for everything. I do want to get this correct before merging into HEAD. There are still other bugs I need to fix, such as suspend/resume of the video (might need to wait until the DRM2 branch is merged for the video reset, so this will add some time, too). - Justin