On Wed, Mar 28, 2012 at 03:59:17PM -0700, Ben Widawsky wrote: > On Thu, 29 Mar 2012 00:43:00 +0200 > Daniel Vetter <daniel at ffwll.ch> wrote: > > > On Sun, Mar 18, 2012 at 01:39:42PM -0700, Ben Widawsky wrote: > > > Very basic code for context setup/destruction in the driver. > > > > > > There are 4 entry points into the contexts, load, unload, open, > > > close. The names are self-explanatory except that load can be > > > called during reset, and also during pm thaw/resume. As we expect > > > our context to be preserved across these events, we do not > > > reinitialize in this case. > > > > > > Also an important note, as I intend to use contexts for ILK RC6, the > > > context initialization must always come before RC6 initialization. > > > > > > As Adam Jackson pointed out, I picked an arbitrary cutoff of 1MB > > > where I decide the HW context is too big. The reason for this is > > > even though context sizes are increasing with every generation, > > > they are still measured in pages. If we somehow read back way more > > > than that, it probably means BIOS has done something strange, or > > > we're running on a platform that wasn't designed for this. > > > > > > The 1MB was just a nice round number. I'm open to changing it to > > > something sensible if someone has a better idea. > > > > > > Signed-off-by: Ben Widawsky <ben at bwidawsk.net> > > > > <bikeshed> I see not that much precedence for _load and _unload for > > setup/teardown ... > > > > Also this patch is imo way too early in the series - you just add > > empty functions so I have no idea what they're doing. And hence can't > > check whether you add them at the right place. Whereas if this comes > > later I already know what they're doing and can check without > > applying whether they're all called at the right place. > > </bikeshed> > > I understand that you get no greater pleasure than bikeshedding my > patches until I want to cry... but seriously with the precedent, it's > in our driver already. So what do you want instead, setup()/teardown() > - init/fini? > > Anyway, here is the precedent: > i915_driver_UNLOAD()->i915_gem_context_unload() > i915_driver_LOAD()->i915_gem_LOAD() // which used to call my function > i915_driver_LOAD()->...->i915)gem_context_load() Well, I've fixed up gem_load, that's now also called init ;-) And driver_load and _unload are remnants from the stoneage, when these two functions could essentially only be called a moduel load/unload time. Now we have hotplug and drm drivers don't use stealth attach any more ... Anyway, I've seen a few things while reading your patches yesterday that looked puzzling and strange, but I didn't really know what to do with them. So I just added some easy bikeshed comments. After a nights worth of sleep I think I'm seeing clearer, so expect some real feedback soon. Cheers, Daniel -- Daniel Vetter Mail: daniel at ffwll.ch Mobile: +41 (0)79 365 57 48
