On Mon, 2015-01-05 at 16:35 +0100, Takashi Iwai wrote: > At Mon, 05 Jan 2015 17:29:34 +0200, > Imre Deak wrote: > > > > Hi Mengdong, Takashi, > > > > On Tue, 2014-12-09 at 18:33 +0100, Takashi Iwai wrote: > > > At Tue, 09 Dec 2014 18:56:07 +0200, > > > Imre Deak wrote: > > > > > > > > On Tue, 2014-12-09 at 11:03 +0100, Daniel Vetter wrote: > > > > > On Tue, Dec 09, 2014 at 10:59:54AM +0200, Imre Deak wrote: > > > > > > On Mon, 2014-12-08 at 21:14 +0100, Daniel Vetter wrote: > > > > > > > On Mon, Dec 08, 2014 at 06:42:04PM +0200, Imre Deak wrote: > > > > > > > > The current hda/i915 interface to enable/disable power wells > > > > > > > > and query > > > > > > > > the CD clock rate is based on looking up the relevant i915 > > > > > > > > module > > > > > > > > symbols from the hda driver. By using the component framework > > > > > > > > we can get > > > > > > > > rid of some global state tracking in the i915 driver and pave > > > > > > > > the way to > > > > > > > > fully decouple the two drivers: once support is added to > > > > > > > > enable/disable > > > > > > > > the HDMI functionality dynamically in the hda driver, it can > > > > > > > > bind/unbind > > > > > > > > itself from the i915 component master, without the need to keep > > > > > > > > a > > > > > > > > reference on the i915 module. > > > > > > > > > > > > > > > > This also gets rid of the problem that currently the i915 > > > > > > > > driver exposes > > > > > > > > the interface only on HSW and BDW, while it's also needed at > > > > > > > > least on > > > > > > > > VLV/CHV. > > > > > > > > > > > > > > Awesome that you're tackling this, really happy to see these > > > > > > > hacks go. > > > > > > > Unfortunately I think it's upside down: hda should be the > > > > > > > component master > > > > > > > and i915 should only register a component. > > > > > > > > > > > > > > Longer story: The main reason for the component helpers is to be > > > > > > > able to > > > > > > > magically delay registering the main/master driver until all the > > > > > > > components are loaded. Otherwise -EDEFER doesn't work and also the > > > > > > > suspend/resume ordering this should result in. Master here means > > > > > > > whatever > > > > > > > userspace eventually sees as a device node or similar, component > > > > > > > is > > > > > > > anything really that this userspace interfaces needs to function > > > > > > > correctly. > > > > > > > > > > > > EDEFER doesn't solve the suspend/resume ordering, at least not in > > > > > > the > > > > > > general async s/r case. In any case I don't see a problem in making > > > > > > the > > > > > > hda component the master and I think it's more logical that way as > > > > > > you > > > > > > said, so I changed that. > > > > > > > > > > Yeah for full async s/r we're screwed as-is. But see below for some > > > > > crazy > > > > > ideas. > > > > > > > I think what we need here is then: > > > > > > > - Put the current azx_probe into the master_bind callback. The new > > > > > > > azx_probe will do nothing else than register the master > > > > > > > component and > > > > > > > the component match list. To do so it checks the chip flag and > > > > > > > if it > > > > > > > needs to cooperate with i915 it registers one component match > > > > > > > for that. > > > > > > > The master_bind (old probe) function calls component_bind_all > > > > > > > with the > > > > > > > hda_intel pointer as void *data parameter. > > > > > > > > > > > > I'm not sure this is the best way since by this the i915 module > > > > > > would > > > > > > need to be pinned even when no HDMI functionality is used. I think a > > > > > > better way would be to let the probe function run as now and > > > > > > init/register all the non-HDMI functionality. Then only the HDMI > > > > > > functionality would be inited/registered from the bind/unbind hooks. > > > > > > > > > > Hm, I wasn't sure whether alsa allows this and thought we need i915 > > > > > anyway > > > > > to be able to load the driver. But if we can defer just the hdmi part. > > > > > > > > > > > But we could go either way even as a follow-up to this patchset. > > > > > > > > > > > > > - i915 registers a component for the i915 gfx device. It uses the > > > > > > > component device to get at i915 sturctures and fills the > > > > > > > dev+ops into > > > > > > > the hda_intel pointer it gets as void *data. > > > > > > > > > > > > > > Stuff we then should do on top: > > > > > > > - Add deferred probing to azx_probe: Only when all components are > > > > > > > there > > > > > > > should it actually register. This will take care of all the > > > > > > > module load > > > > > > > order mess. > > > > > > > > > > > > I agree that we should only register user interfaces when > > > > > > everything is > > > > > > in place. But I'm not sure deferred probe is the best, we could do > > > > > > without it by registering HDMI from the component bind hook. > > > > > > > > > > It's mostly a question whether alsa does support delayed addition of > > > > > interface parts. DRM most definitely does not (except for recently > > > > > added > > > > > dp mst connector hotplug). But I guess if the current driver already > > > > > delays registering the hdmi part then we're fine. I'm not sure about > > > > > whether it's really safe - I spotted not a lot of locking really to > > > > > make > > > > > sure there's no races between i915 loading and userspace trying to > > > > > access > > > > > the hdmi side. > > > > > > > > Ok, I'm not sure either. If that's not possible, I agree EDEFER would be > > > > the best and with that we could also get rid of the request_module() we > > > > need now. > > > > > > The probe of HD-audio driver is done in a work anyway, so changing the > > > code to sync with component should be feasible. > > > > > > I'm off today (and yesterday was a Christmas party :), so had little > > > time for review so far. Will check the series tomorrow. > > > > could one of you help reviewing this patchset? Note that there is a v2 > > version with some individual patches having a v3 already. > > Is there any git branch containing the latest patches that is easily > applicable to 3.19-rc (or linux-next)?
I applied them on 3.19-rc2 and pushed the branch to: https://github.com/ideak/linux/commits/audio-component --Imre _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
