On 3/11/16 1:09 PM, 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.
we don't want to expose this interface when HDAudio is present and
enabled so we would need to add a test for this.
Also it looks like you want the creation of the platform device done in
i915, we were thinking of doing this as part of the audio drivers but in
the end this looks equivalent. In both cases we would end-up ignoring
the HAD022A8 HID and not use acpi for this extension
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.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
