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

From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Cc: mengdong.lin@linux.intel.com, tiwai@suse.de,
	intel-gfx@lists.freedesktop.org, liam.r.girdwood@linux.intel.com,
	vinod.koul@intel.com, broonie@kernel.org,
	rakesh.a.ughreja@intel.com,
	David Henningsson <david.henningsson@canonical.com>
Subject: Re: [RFC 04/15] drm/i915: Add headers for non-HDAudio HDMI interface
Date: Mon, 14 Mar 2016 17:21:54 +0200	[thread overview]
Message-ID: <20160314152154.GR4329@intel.com> (raw)
In-Reply-To: <56E6D536.9020704@linux.intel.com>

On Mon, Mar 14, 2016 at 10:13:58AM -0500, Pierre-Louis Bossart wrote:
> 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

Well, if you have a device you can hang off from then i915 should need
to register it I suppose. Though that would make the interrupt
forwarding perhaps less nice. There's also the suspend/resume order
dependency to deal with if there's no parent/child relationship between
the devices.

> 
> > 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.
> >

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx