Re: [RFC 04/15] drm/i915: Add headers for non-HDAudio HDMI interface

[Daniel Vetter <daniel@ffwll.ch>]

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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx