On Fri, 2016-03-11 at 20:38 +0000, Chris Wilson wrote: > On Fri, Mar 11, 2016 at 10:11:20PM +0200, Imre Deak wrote: > > On Fri, 2016-03-11 at 19:55 +0000, Chris Wilson wrote: > > > On Fri, Mar 11, 2016 at 06:31:42PM +0200, Imre Deak wrote: > > > > According to the new init phases scheme we should register the > > > > device > > > > making it available via some kernel internal or user space > > > > interface as > > > > the last step in the init sequence, so move the corresponding > > > > code > > > > to a > > > > separate function. > > > > > > > > Also add a TODO comment about code that still needs to be moved > > > > around > > > > to one of the init phases functions depending on what the role > > > > and > > > > effect > > > > of that code is. > > > > > > > > No functional change, except for the reordering of the unload > > > > time > > > > unregistration steps of sysfs wrt. acpi and opregion. > > > > > > > > Suggested by Chris. > > > > > > > > CC: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> > > > > Signed-off-by: Imre Deak <imre.deak@xxxxxxxxx> > > > > --- > > > > drivers/gpu/drm/i915/i915_dma.c | 83 > > > > +++++++++++++++++++++++++++ > > > > -------------- > > > > 1 file changed, 54 insertions(+), 29 deletions(-) > > > > > > > > diff --git a/drivers/gpu/drm/i915/i915_dma.c > > > > b/drivers/gpu/drm/i915/i915_dma.c > > > > index aaf1b17..43dcb5a 100644 > > > > --- a/drivers/gpu/drm/i915/i915_dma.c > > > > +++ b/drivers/gpu/drm/i915/i915_dma.c > > > > @@ -1209,6 +1209,53 @@ static void > > > > i915_driver_cleanup_hw(struct > > > > drm_i915_private *dev_priv) > > > > } > > > > > > > > /** > > > > + * i915_driver_init_register - register the driver with the > > > > rest > > > > of the system > > > > + * @dev_priv: device private > > > > + * > > > > + * Perform any steps necessary to make the driver available > > > > via > > > > kernel > > > > + * internal or userspace interfaces. > > > > + */ > > > > +static void i915_driver_init_register(struct drm_i915_private > > > > *dev_priv) > > > > > > This is the only one I didn't like. The problem with _register is > > > that > > > it makes me think of mmio register (yes, I know that's too > > > driver-centric and the use of register_callback_interface is > > > widespread.) A compromise would be > > > > > > i915_driver_init_frameworks() > > > > Ok, can rename it. > > > > > Other than I got to here without spotting anything obnoxious or > > > troublematic wrt to mmio/gem. The only thing we lack is fault > > > injection > > > into igt/drv_module_reload, maybe we can do > > > i915.inject_fault = LOAD_MMIO | LOAD_FRAMEWORK etc > > > The other tricky part is deciding on what the failure should be. > > > For > > > critical faults we just expect the module to fail to load, but > > > for > > > aspects like GEM, we just want to the GPU to be disabled but > > > modesetting > > > still work. > > > > Yep sounds useful, but can we do it as a follow-up? > > I'd rather not. The question is how much of this churn is covered by > igt? I think the answer is scarily low, since half of this is error > path > during init. Adding a module paramter and then checking bits in each > of > the new phase functions is going to be a relatively simple job and > lets > us have a little more confidence that the changes + fixes are solid. Yes not much coverage, but that means that probably there are already existing issues. I specifically avoided touching any of the problematic parts (in the modesetting init function) and I don't see anything in this patchset that would make things worse (as you also pointed out). Realistically, fixing up all the error path handling in the modesetting path would be quite a bit of work. Otoh, this patchset is a dependency on other PM related stuff that is quite late already, so I'd like to progress with that. That's why I suggested to fix up the other parts later. --Imre > The tricker part would be adding the loop over insmod i915.ko into > drv_module_reload_basic, but well worth the bugs it is likely to > uncover. > -Chris > _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
