On Mon, Mar 14, 2016 at 10:04:00AM +0100, Daniel Vetter wrote: > On Fri, Mar 11, 2016 at 09:09:12PM +0200, Ville Syrjälä wrote: > > On Fri, Mar 11, 2016 at 11:27:13AM -0600, Pierre-Louis Bossart wrote: > > > Thanks for the review Ville > > > > > > [snip] > > > > > > > Kinda hard to see where everything gets used due to the way the patches > > > > are split up. > > > > > > Yes, it's far from great... > > > > > > > At least the hotplug/mode change events are not needed. We only have the > > > > two points where i915 should inform the audio driver about this stuff, > > > > and those are the intel_audio_code_enable/disable(). For that we > > > > already have the .pin_eld_notify() hook. > > > > > > > > The interrupt stuff should mostly vanish from i915 with the subdevice > > > > approach. As in i915 would just call the interrupt handler of the audio > > > > driver based on the LPE bits in IIR, and the audio driver can then do > > > > whatever it wants based on its own status register. > > > > > > Are you saying that the subdevice would provide a read/write interface > > > for the audio driver to look at display registers, and the i915 driver > > > would only provide a notification interface (EDID and interrupts) to the > > > audio driver? > > > > The audio driver would simply ioremap the appropriate range of > > registers itself. > > > > > If yes, would there be two component framework links, one between > > > i915/audio driver and one between subdevice/audio driver. > > > > Yeah sort of. i915 registers the platform device for the audio, the > > audio driver can then bind to the device via the platform driver .probe > > callback. It can then register with the audio component stuff at some > > point to get the relevant notifications on the display state. When > > i915 gets unloaded we remove the platform device, at which point the > > audio driver's platform driver .remove() callback gets invoked and > > it should unregister/cleanup everything. > > > > I just tried to frob around with the VED code a bit, and got it to load > > at least. It's not quite happy about reloading i915 while the ipvr > > driver was loaded though. Not sure what's going on there, but I do > > think this approach should work. So the VED patch could serve as a > > decent enough model to follow. > > platform devices registerd by modules are apparently inherently racy and > in an unfixable way. At least I remember something like that from VED > discussion. > > In short you _must_ unload VED manually before unloading i915, or it all > goes boom. If this is the only thing that went boom it's acceptable. > > Another bit we didn't fully do for VED is abstracting away the dma mapping > stuff, because x86 dma abstraction sucks (compared to arm). Not sure, but > this might have been fixed meanwhile - if we can set up a dma_ops that the > subdevice would use, we should do so (instead of the page_to_pfn hacks VED > used). This one might be a bit a problem - on byt we got away with pfn_to_page because no iommu at all, but that's not a good idea really. Definitely need to reevaluate this again. Iirc there's been some talk of just walking up a chain of platform devices until the core x86 dma code finds something with dma support, then use that. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
