Re: [PATCH 2/7] drm/i915: Add support for audio driver notifications

[Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>]

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.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx