On 12/14/16 7:13 AM, Takashi Iwai wrote:
On Wed, 14 Dec 2016 13:55:52 +0100, Daniel Vetter wrote:
Only noticed it here, but why again do we need to re-roll our intel-only hdmi/eld notification? The one we have for hda is somewhat justified since it went in at roughly the same time as the new shared one across a bunch of soc. But this one here is just a reinvented wheel.
And yes this code has been hanging around internally for years, but that's kinda not a good excuse.
Obviously this comment applies to patch 1 too.
Yeah, a unified common interface would be better, really. I'm basically OK with the current implementation, though, as long as it works. But if we can sort it out quickly, it's better.
That said, we may reuse i915_audio_component stuff -- or at least, reuse the object used there without actual component binding (the lpe driver doesn't need the component because it's a strong dependency unlike HD-audio case), and just add a few more fields there. Instead of exposing the resource, we can provide the register accessor there, too.
It's just an idea, so not necessarily serious taken. But we can think of unification more easily now than later.
BTW, now one thing came to my mind: don't we need the power control (pm and power well domain) when accessing from the sound driver side? The HD-audio component object has the gfx side callback points for such.
The code hasn't actually been around for years. We threw away the legacy driver and restarted the i915 integration pretty much from scratch using the feedback on the intel-gfx mailing list, specifically Jesse Barnes and Ville Syrjala, with only basic programming sequences and register definitions kept on the audio side. The volume of code required on the i915 side was reduced by probably 50%, with useless stuff taken out left and right. We're down to ~500 lines changed on the i915 side with about 400 just for the interface in the new intel_lpe_audio.c file.
The initial idea for the rework was to use the component framework. Then during the initial review it was suggested that component framework wasn't that great and that the design should follow the example of the VED integration with a subdevice created and pdata used to communicate between the i915 and audio sides. I threw the initial component framework patches away and we started in this direction.
There is no power domain here and in general no issue with probe happening independently at different times, the subdevice/pdata is simple enough to model the hardware. If there is an agreement to push for a unified interface, that's fine and I will commit to port the code as needed. We can modify the interface, all that's needed is to provide the ELD and let the audio side program a window of register space on the i915 side. But quite frankly I'd rather see the code being merged first to expand the userbase, today HDMI can only be enabled with out-of-tree patches pulled from my github ported on the last 6 kernel versions. I also plan to work an update on DisplayPort support since there are CHT devices on the market now.