[alsa-devel] HDMI codec, way forward?

[Takashi Iwai]

On Wed, 21 Oct 2015 20:19:38 +0200,
Russell King - ARM Linux wrote:
> 
> On Wed, Oct 21, 2015 at 07:59:06PM +0200, Takashi Iwai wrote:
> > On Wed, 21 Oct 2015 19:34:37 +0200,
> > Russell King - ARM Linux wrote:
> > > 
> > > On Wed, Oct 21, 2015 at 09:49:28PM +0530, Vinod Koul wrote:
> > > > On Wed, Oct 21, 2015 at 03:37:47PM +0100, Russell King - ARM Linux wrote:
> > > > > In any case, this doesn't (and can't) solve the CEC problem, so it's not
> > > > > a solution to the problem at hand.
> > > > 
> > > > Sorry am not sure I follow the reasons for that, wouldn't CEC be another
> > > > slave in such an interface? I though component fwk did allow us to have
> > > > multiple slaves..
> > > 
> > > Not with the way you're using the component helper here.
> > > 
> > > I guess that not all my message is being read, because people keep
> > > replying half-way down my messages...
> > > 
> > > You can only register a struct device _once_ as a slave device.
> > > 
> > > With the way you're using it here for audio, you're registering the
> > > i915 DRM device as a slave component device, and the audio side as
> > > the master.  That means the audio master can bind to the DRM slave
> > > component device.
> > > 
> > > You can't then have a CEC master bind to the i915 DRM slave device
> > > (it's already bound to the audio master device), and you can't
> > > register the i915 DRM device as a second slave component device.
> > > It becomes indistinguishable from the first, and there's no way
> > > to tell which of the two different 'ops' structures should be used
> > > with which master.
> > > 
> > > I said this in my message 20151021140307.GE32532 at n2100.arm.linux.org.uk
> > > which was two of my replies ago in this sub-thread.
> > 
> > Can't the limitation of single slave dev be extended simply?  e.g. add
> > some matching semantics to component_master_add_child() like a shared
> > key in both master and slave, and let assign only the matched slave.
> 
> I've already explained that in the email message I referred to by
> message ID above (which was a reply to one of your previous messages)
> 
> This is why I find email such a poor communication medium - I'm quite
> sure most people only read half an email message before hitting reply,
> and then they stick their reply after where they stopped reading and
> never bother reading the rest of the message.  Normally, at this point,
> I'll start getting grumpy and sending frustrated emails... which would
> eventually deride into flames, but let's _try_ to keep this civil.

Thank you for patience!

> Here's the relevant paragraph from the above referenced message, and
> to make the appropriate bit stand out, I've underlined it with ^.
> 
> > However, there's a lurking problem: you can't register the same struct
> > device as a slave more than once into the component helpers - that's
> > because it's designed to look for devices.  There's intentionally no
>                                               ^^^^^^^^^^^^^^^^^^^^^^^^
> > linkage between the slave ops and master ops to allow for generic
>   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> > slave drivers (like tda998x) to be used with different master drivers
>   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> > (armada, tilcdc, etc).  In theory, you can register the master device
>   ^^^^^^^^^^^^^^^^^^^^^
> > of one componentised system as a slave device of another, but that
> > requires a much more complicated locking setup in component.c (I have
> > patches for that, but I've resisted sending them because it makes the
> > locking very messy.)
> 
> In a subsequent sentence, I also address the issue that would occur
> if any of the already componentised DRM drivers were to attempt what
> you're doing in i915 - although I say it's solvable, I've resisted
> that because it makes the locking in there _much_ more hairy, and I'm
> now not certain that my more complex locking implementation would
> even cope with that scenario.

I understood that the original component design wasn't for cross
subsystems.  OTOH I wondered why it can't be extended for wider use --
that was the simple question; I haven't seen so complex locking in
some upstream drm drivers code through a quick glance, so the mess
about locking you mentioned wasn't clear to me.  Now point taken.

(Still I think it's possible to have multiple components binds for
 such cases, but I won't insist on it :)
 

To be noted, a weak dependency is still necessary for i915 audio case,
and a simple notifier type registration won't work, unfortunately.
GPU power adjustment is mandatory even at probing the audio hardware
to judge whether HDMI codec is present or not.  Meanwhile the
dependency to the graphics must not be tight, either.  The very same
audio driver is for a controller without graphics, too.  That's the
reason we took the component as an alternative implementation, which
is a bit cleaner than the direct symbol resolving in the earlier
code.

So, if any better solution that covers a use case like i915/hda is
present, we're willing to switch.  As repeatedly written, the current
implementation is a stop gap.  Although this doesn't look too bad,
there are still some obvious pitfalls as you pointed out.  We can
paper over them (maybe I'll do for 4.4), but a fundamentally better
solution would be more welcome, of course.


thanks,

Takashi