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

[Ville Syrjälä]

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.

VED goes boom due drm_global_mutex deadlock at least if you load
i915 while ipvr is already loaded. I didn't check to hard if there
were any booms on pure unload so far.

Anyways, I was a bit worried about the races so I did a pair of 
small test modules to try out this model, and to me it looked to
work so far. I just unregistered the platform device from the parent
pci driver .remove() hook, and it got blocked until the platform
driver's .remove() hook was done.

In any case if this is broken, then I assume mfd must be broken. And
that thing is at least used quite extensively.

-- 
Ville Syrjälä
Intel OTC