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