Re: [PATCH] i915: Always register as a PCI driver (was: Re: [PATCH] DRM / i915: Fix resume regression on MSI Wind U100 w/o KMS)

["Rafael J. Wysocki" <rjw@sisk.pl>]

On Saturday 09 January 2010, Dave Airlie wrote:
> On Sat, Jan 9, 2010 at 11:35 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> > On Saturday 09 January 2010, Jesse Barnes wrote:
> >> On Fri, 8 Jan 2010 16:50:57 -0800 (PST)
> >> Linus Torvalds <torvalds@linux-foundation.org> wrote:
> >>
> >> >
> >> >
> >> > On Sat, 9 Jan 2010, Rafael J. Wysocki wrote:
> >> > >
> >> > > Which is functionally equivalent to my patch, because
> >> > > i915_suspend/resume() won't be called by drm_class_suspend/resume()
> >> > > in the KMS case anyway.
> >> >
> >> > Ahh, right you are - that class suspend function does a check for
> >> > DRIVER_MODESET, and only does the suspend/resume if it's not a
> >> > MODESET driver.
> >> >
> >> > Ok, so I withdraw my objections to your original patch - it's
> >> > confusing, but that's just because DRM is such a horrible mess with
> >> > subtle things.
> >>
> >> Yeah the non-KMS paths just suck.
> >>
> >> Acked-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> >>
> >> Though hopefully you can get the PCI driver registration working w/o
> >> too much trouble; that would be even better.
> >
> > Actually, I have a working patch, with one tiny detail I'm not sure of.
> >
> > Namely, I need to call pci_set_drvdata(pdev, dev) unconditionally in drm_stub.c
> > for the things to work, but I _think_ it won't hurt even if we're not going to
> > use the pdev's private data.
> >
> > The benefit of this is having just one code path for suspend/resume instead of
> > two different code paths depending on whether the driver is using the KMS or
> > not, which is well worth it IMO.
> >
> > The patch is appended.
> 
> NAK
> 
> for the reasons I explained in the previous email. This conflicts with systems
> where intelfb and intel drm are used together, this is something that ppl do use
> prior to KMS happening.
> 
> We just need to document in the headers why the hooks are needed,
> and maybe a bit of patch review to make sure nobody removes them again.

OK, so my original patch is the right one in that case.

Rafael