Re: [Intel-gfx] Need your advice: Add a new communication inteface between HD-Audio and Gfx drivers for hotplug notification/ELD update

Rob Clark
2014-01-22T18:15:12Z

On Wed, Jan 22, 2014 at 12:23 PM, Takashi Iwai <tiwai@suse.de> wrote:
> At Wed, 22 Jan 2014 10:45:26 -0500,
> Rob Clark wrote:
>>
>> On Wed, Jan 22, 2014 at 10:20 AM, Daniel Vetter <daniel@ffwll.ch> wrote:
>> > On Wed, Jan 22, 2014 at 10:04:14AM -0500, Rob Clark wrote:
>> >> sorry to jump into this a bit late, so maybe this was covered already earlier..
>> >
>> > It just started, I've quoted everything when cc'ing dri-devel. But good to
>> > have examples outside of x86 (where things are mostly standardized by the
>> > Eye of Redmond).
>>
>> perhaps the arm/SoC stuff was standardized by the Eye of Cthulhu
>>
>> btw, added a few other SoC drm types who might be interested in the topic
>>
>> >> For drm/msm the hdmi code needs something along these lines from audio:
>> >>
>> >> int hdmi_audio_info_setup(struct platform_device *pdev, bool enabled,
>> >> uint32_t num_of_channels, uint32_t channel_allocation,
>> >> uint32_t level_shift, bool down_mix);
>> >> void hdmi_audio_set_sample_rate(struct platform_device *pdev, int rate);
>> >>
>> >> (former is mainly to setup avi audio infoframe, latter for clks)
>> >>
>> >> in addition to hotplug and ELD stuff
>> >
>> > Can you elaborate a bit on what you need for msm? On intel (and I think
>> > the other x86 platforms are similar) we have separate buffers in the hw
>> > for avi infoframes and audio infoframes, so the drm and alsa driver can
>> > bash the right stuff into the hardware on their own. I'm mostly confused
>> > since you say _AVI_ infoframe here, which is purely generated by the drm
>> > side of the code here. Or at least that's been my understanding.
>>
>> Sorry, typo, meant audio infoframe
>>
>> We could have the API such that the audio driver constructs the
>> infoframe.. that probably makes more sense and simplifies things.
>
> The HD-audio has a code for doing that, so if needed, it can be copied
> from there. But, even AMD HD-audio codec doesn't use this scheme but
> just sets up a few verbs for channel-allocations, etc, so the complete
> audio infoframe isn't necessary in most cases, as it seems.
>
>> But
>> the drm driver is the one that needs to bash that constructed buffer
>> into the hw. Or, well, either that or both drivers ioremap the same
>> block of registers, but that seems somewhat lame.
>>
>> But I do need to know some basic things, like # of channels.. and
>> would kinda prefer not to have to parse the audio infoframe to get
>> that info.
>
> Yes, it makes things complex. It's one of the reasons we'd like to
> have a more straightforward interface.
>
>> > The clocks are also funny really, but I'm not sure whether this fits. I'd
>> > have expected some DT-based generic clock source which the audio driver
>> > just grabs as part of his multi-device beast (maybe using Russell's new
>> > driver core code) and used directly. What's the reason that this has to go
>> > through the drm driverr?
>>
>> possibly it could be exposed to the audio driver as a 'struct clk'
>> that is implemented/registered/exported by the drm driver, I guess?
>
> Hrm, but I guess this is also purely optional?
> I'd like to start rather from a minimum set.

well, for anything where the video side of things does not need to
know (which would incl audio clock if this doesn't need to be setup by
display on some hw) would be optional, I think..

if you did need to configure audio clock on the display side of
things, it might be simpler just to have a set_rate() fxn ptr in the
struct, but the 'struct clk' approach seems more proper. That said,
I'm not really sure how it works for the audio driver to find the clk
in a non-DT kernel.. if that was a problem, I'm not one to let
perfection be the enemy of improvement, so would be ok with the
simpler interface.

BR,
-R

>> fwiw, if curious, what I have on msm so far is at:
>>
>> https://github.com/freedreno/kernel-msm/commit/6ffd278d39a3ff8712c70b5fd98dc135bd6c7b8a
>>
>> It works on downstream qcom android kernel.. the API exported by drm
>> driver, called by audio driver, is basically just a clone of the hack
>> that was already there between fbdev and alsa. I haven't tried to
>> clean that up at all yet. It was enough work just untangling ION (!!)
>> from alsa in that kernel :-/
>
> Thanks for the pointer!
>
>
> Takashi
>
>
>>
>> BR,
>> -R
>>
>> > Cheers, Daniel
>> > --
>> > Daniel Vetter
>> > Software Engineer, Intel Corporation
>> > +41 (0) 79 365 57 48 - http://blog.ffwll.ch
>>