Alsa-Devel Archive on lore.kernel.org
 help / color / Atom feed
From: Anssi Hannula <anssi.hannula@iki.fi>
To: Russell King - ARM Linux <linux@arm.linux.org.uk>
Cc: Fabio Estevam <fabio.estevam@freescale.com>,
	alsa-devel@alsa-project.org, dri-devel@lists.freedesktop.org,
	Mark Brown <broonie@kernel.org>, Yakir Yang <ykk@rock-chips.com>,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [alsa-devel] [PATCH 12/13] drm: bridge/dw_hdmi-ahb-audio: add audio driver
Date: Sun, 10 May 2015 23:47:07 +0300
Message-ID: <554FC3CB.3030807@iki.fi> (raw)
In-Reply-To: <20150510193320.GZ2067@n2100.arm.linux.org.uk>

10.05.2015, 22:33, Russell King - ARM Linux kirjoitti:
> On Sun, May 10, 2015 at 09:59:42PM +0300, Anssi Hannula wrote:
>> What I'd like to see is arrive at some sort of general consensus on how
>> the AES bits should be handled (i.e. should the driver always set them
>> themselves and disallow/allow the userspace to override the rate bits),
>> which could then be applied to other drivers as well.
>>
>> But maybe that is for another time, or just a futile effort altogether...
> 
> My personal view is that where we're dealing with PCM audio, the driver
> needs to set these bits correctly as there is nothing in userspace to
> do this.  This provides an identical interface between each audio device
> which accepts PCM samples - whether it's a SPDIF or non-SPDIF based
> device.
>
> For non-audio data sent via an audio device, the AES bits need to be
> conveyed from userspace, and we should respect what userspace gives us.
> (If it's wrong, it's a userspace bug, and userspace should be fixed,
> rather than trying to work around the bug by patching the kernel.)

Just to make sure I didn't misunderstand. You propose looking at the
"non-pcm" (aka "non-audio") bit in the AES to see whether driver/kernel
should force rate (and maybe other) AES bits?
Because I think that is currently the only way for the kernel/driver to
know if the data is "non-audio".


>> Indeed. I did notice there is a SND(RV)_PCM_FORMAT_SPECIAL but I guess
>> it might not be easily used for this purpose since it doesn't have a
>> specific sample width etc (but I am not familiar enough with this to say
>> whether it could work or not)...
> 
> I spent quite a while looking at alsa-lib, wondering whether I could
> move all the conversions out to userspace, but I couldn't without
> building them _into_ alsa-lib.  This was a while back now, but from
> what I remember, plugins to alsa-lib which aren't built as part of
> alsa-lib are not able to do format conversions.

Sounds a bit strange (assuming one'd plainly reference the plugin from
<hw>.conf directly), but OK. I'm not suggesting to look into it again
unless you really want to yourself :)

>>> However, in the case of VLC, if it wants to send non-audio, it will
>>> open the IEC958 device, which will use the iec958 plugin to configure
>>> the AES bits for non-audio, and pass IEC958 data to the kernel (which
>>> still needs to be reformatted to the hardware's special format.)
>>
>> Ah, so the AES bits are actually overridable by userspace, which is what
>> I was initially concerned with :)
>>
>> Of course, this means that applications opening "iec958" but not setting
>> rate bits (which is common) will get the default 48kHz bits from
>> /usr/share/alsa/pcm/(iec958|hdmi).conf). Not sure how big an issue that
>> is, though. The "iec958" ALSA plugin does seem to have a FIXME comment
>> about setting AES bits according to sample rate.
> 
> Note that VLC does set the "sample" rate appropriately:
> 
>         switch (aout->format.i_rate)
>         {
> #define FS(freq) \
>             case freq: aes3 = IEC958_AES3_CON_FS_ ## freq; break;
>             FS( 44100) /* def. */ FS( 48000) FS( 32000)
>             FS( 22050)            FS( 24000)
>             FS( 88200) FS(768000) FS( 96000)
>             FS(176400)            FS(192000)
> #undef FS
>             default:
>                 aes3 = IEC958_AES3_CON_FS_NOTID;
>                 break;

Yep, one of the few that do.

>>> <confdir:pcm/iec958.conf>
>>>
>>> dw-hdmi-ahb-aud.pcm.iec958.0 {
>>
>> I think you should s/iec958/hdmi/ for the above two lines. HDMI devices
>> should be using "hdmi" instead of "iec958" by convention (the latter is
>> used for optical/coaxial S/PDIF).
> 
> Except doing that kills VLC's passthrough option (denoted by "spdif"),
> which explicitly wants the iec958 device:
> 
>     /* Choose the IEC device for S/PDIF output:
>        if the device is overridden by the user then it will be the one.
>        Otherwise we compute the default device based on the output format. */
>     if (spdif && !strcmp (device, "default"))
>     {
> ...
>         if (asprintf (&device,
>                       "iec958:AES0=0x%x,AES1=0x%x,AES2=0x%x,AES3=0x%x",
>                       IEC958_AES0_CON_EMPHASIS_NONE | IEC958_AES0_NONAUDIO,
>                       IEC958_AES1_CON_ORIGINAL | IEC958_AES1_CON_PCM_CODER,
>                       0, aes3) == -1)
> 
> Yes, technically an application bug, since VLC should allow the device
> to be selectable and/or detect hdmi devices.  I wonder if that's
> something which has changed between 2.0.8 and the latest vlc.

Yes, in the current version it appends the parameters to the configured
device if it contains "iec958" or "hdmi" (git log says iec958 was
un-hardcoded on the above line in Jan 2014).

So if you have selected the "HDMI Output" device from the audio dropdown
in settings, it would work.

However, the default device "default" is still mangled to "iec958", (the
code has a "TODO: hdmi" comment).


> I did consider having the hdmi output device, but also alias that to
> the iec958 device name, which I think can be done via:
> 
> <confdir:pcm/hdmi.conf>
> 
> dw-hdmi-ahb-aud.pcm.hdmi.0 {
> ...
> }
> 
> <confdir:pcm/iec958.conf>
> 
> dw-hdmi-ahb-aud.pcm.iec958.0 cards.dw-hdmi-ahb-aud.pcm.hdmi.0

If a userspace workaround is wanted, I think I'd prefer this one myself.

Personally, I don't believe a workaround is of much use here, because
using "iec958" is already broken for majority of HDMI users anyway (i.e.
those that use HDMI outputs on x86 PCs)... but I don't see the
workaround as strictly unacceptable, either.


Also, if this used "hdmi" and no workaround, i.MX6 devices with both
S/PDIF and HDMI connectors would then have just a single iec958* device
and a single hdmi* device. But I guess that doesn't matter *that* much...

> However, for HDMI sinks, I haven't seen any way in Linux to allow
> userspace to know the audio capabilities of the attached HDMI sink,
> and thus whether the video player can output compressed MPEG audio.
> Anyone know?

HDA driver exports the ELD as a mixer control, which the application can
then use to determine the capabilities.

Not the best idea IMHO (I think ELD should've preferably remained
internal since I think it should be considered an implementation detail,
plus it is not very application friendly), but it is what it is and I
guess it was the easiest way to do it from ALSA side.

I know pulseaudio uses the ELD control to:
- get receiver name
- trigger a device change event when it changes

Kodi uses it to:
- get receiver name
- get default passthrough (compressed audio) capabilities
- trigger re-enumeration and device reopening when it changes
(e.g. if the user starts audio playback before turning on their
amplifier/receiver, we might've started the playback with
less-than-optimal parameters due to ALSA driver ELD rate/channel
restrictions, since the amplifier/receiver might've passed us the
probably-more-limited monitor/TV SADs instead of its own SADs when the
receiver was in standby mode).

But I wouldn't be too surprised if those were the only users.

-- 
Anssi Hannula



_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

  reply index

Thread overview: 74+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-04-02  9:20 [RFC v2 0/13] dw_hdmi cleanups, audio preparation, helpers and ahb audio support Russell King - ARM Linux
2015-04-02  9:21 ` [PATCH RFC v2 01/13] drm: imx/dw_hdmi: move phy comments Russell King
2015-04-02  9:21 ` [PATCH RFC v2 02/13] drm: bridge/dw_hdmi: clean up phy configuration Russell King
2015-04-02  9:21 ` [PATCH RFC v2 03/13] drm: bridge/dw_hdmi: clean up hdmi_set_clk_regenerator() Russell King
2015-04-02  9:21 ` [PATCH RFC v2 04/13] drm: bridge/dw_hdmi: use drm_hdmi_avi_infoframe_from_display_mode() Russell King
2015-04-02  9:21 ` [PATCH RFC v2 05/13] drm: bridge/dw_hdmi: simplify hdmi_config_AVI() a little Russell King
2015-04-02  9:21 ` [PATCH RFC v2 06/13] drm: bridge/dw_hdmi: remove mhsyncpolarity/mvsyncpolarity/minterlaced Russell King
2015-04-02  9:21 ` [PATCH RFC v2 07/13] drm: bridge/dw_hdmi: introduce interface to setting sample rate Russell King
2015-04-02  9:21 ` [PATCH RFC v2 08/13] drm: bridge/dw_hdmi: introduce interfaces to enable and disable audio Russell King
2015-04-02  9:22 ` [PATCH RFC v2 09/13] drm/edid: add function to help find SADs Russell King
2015-04-02  9:22 ` [PATCH RFC v2 10/13] sound/core: add DRM ELD helper Russell King
2015-04-05 15:57   ` Takashi Iwai
2015-04-05 16:20     ` Russell King - ARM Linux
2015-04-05 16:46       ` Takashi Iwai
2015-04-05 17:26         ` Russell King - ARM Linux
2015-05-06 17:02           ` Anssi Hannula
2015-05-07 10:41             ` Russell King - ARM Linux
2015-05-07 11:11               ` Lars-Peter Clausen
2015-05-08 10:56           ` [alsa-devel] " Jyri Sarha
2015-05-08 11:42             ` Russell King - ARM Linux
2015-05-05 22:35     ` Mark Brown
2015-05-06  8:58       ` Liam Girdwood
2015-05-08 13:16   ` [alsa-devel] " Jyri Sarha
2015-05-08 13:27     ` Russell King - ARM Linux
2015-05-08 13:37       ` Jyri Sarha
2015-04-02  9:22 ` [PATCH RFC v2 11/13] sound/core: add IEC958 channel status helper Russell King
2015-04-02  9:22 ` [PATCH RFC v2 12/13] drm: bridge/dw_hdmi-ahb-audio: add audio driver Russell King
2015-04-02  9:22 ` [PATCH RFC v2 13/13] drm: bridge/dw_hdmi-ahb-audio: parse ELD from HDMI driver Russell King
2015-05-09 10:25 ` [PATCH v3 0/13] dw_hdmi cleanups, audio preparation, helpers and ahb audio support Russell King - ARM Linux
2015-05-09 10:25   ` [PATCH 01/13] drm: imx/dw_hdmi: move phy comments Russell King
2015-05-09 10:26   ` [PATCH 02/13] drm: bridge/dw_hdmi: clean up phy configuration Russell King
2015-05-22 15:19     ` Yakir
2015-05-09 10:26   ` [PATCH 03/13] drm: bridge/dw_hdmi: clean up hdmi_set_clk_regenerator() Russell King
2015-05-22 15:22     ` Yakir
2015-05-09 10:26   ` [PATCH 04/13] drm: bridge/dw_hdmi: use drm_hdmi_avi_infoframe_from_display_mode() Russell King
2015-05-09 10:26   ` [PATCH 05/13] drm: bridge/dw_hdmi: simplify hdmi_config_AVI() a little Russell King
2015-05-09 10:26   ` [PATCH 06/13] drm: bridge/dw_hdmi: remove mhsyncpolarity/mvsyncpolarity/minterlaced Russell King
2015-05-09 10:26   ` [PATCH 07/13] drm: bridge/dw_hdmi: introduce interface to setting sample rate Russell King
2015-05-22 15:26     ` Yakir
2015-05-09 10:26   ` [PATCH 08/13] drm: bridge/dw_hdmi: introduce interfaces to enable and disable audio Russell King
2015-05-22 15:28     ` Yakir
2015-05-09 10:26   ` [PATCH 09/13] drm/edid: add function to help find SADs Russell King
2015-05-09 10:26   ` [PATCH 10/13] sound/core: add DRM ELD helper Russell King
2015-05-22 12:20     ` [alsa-devel] " Mark Brown
2015-05-22 13:15       ` Russell King - ARM Linux
2015-05-22 13:30         ` Takashi Iwai
2015-05-22 13:53           ` Russell King - ARM Linux
2015-05-22 13:54             ` Takashi Iwai
2015-05-22 14:00               ` Russell King - ARM Linux
2015-05-22 14:02                 ` Takashi Iwai
2015-05-22 14:05                   ` Takashi Iwai
2015-05-22 16:12                     ` Russell King - ARM Linux
2015-05-09 10:26   ` [PATCH 11/13] sound/core: add IEC958 channel status helper Russell King
2015-05-22 12:40     ` Mark Brown
2015-05-09 10:26   ` [PATCH 12/13] drm: bridge/dw_hdmi-ahb-audio: add audio driver Russell King
2015-05-09 16:49     ` [alsa-devel] " Anssi Hannula
2015-05-09 16:55       ` Russell King - ARM Linux
2015-05-09 17:07         ` Anssi Hannula
2015-05-09 17:40           ` Russell King - ARM Linux
2015-05-09 17:53             ` Russell King - ARM Linux
2015-05-09 17:55             ` Anssi Hannula
2015-05-09 18:11               ` Russell King - ARM Linux
2015-05-10 18:59                 ` Anssi Hannula
2015-05-10 19:33                   ` Russell King - ARM Linux
2015-05-10 20:47                     ` Anssi Hannula [this message]
2015-05-11 15:58                     ` Mark Brown
2015-05-09 10:26   ` [PATCH 13/13] drm: bridge/dw_hdmi-ahb-audio: parse ELD from HDMI driver Russell King
2015-05-27 10:43     ` Daniel Vetter
2015-05-27 11:43       ` Mark Brown
2015-05-27 17:31       ` Russell King - ARM Linux
2015-05-27 21:29         ` Daniel Vetter
2015-05-27 21:44           ` Russell King - ARM Linux
2015-05-28  6:43             ` Daniel Vetter
2015-05-28  4:56         ` [alsa-devel] " Takashi Iwai

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=554FC3CB.3030807@iki.fi \
    --to=anssi.hannula@iki.fi \
    --cc=alsa-devel@alsa-project.org \
    --cc=broonie@kernel.org \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=fabio.estevam@freescale.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux@arm.linux.org.uk \
    --cc=ykk@rock-chips.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Alsa-Devel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/alsa-devel/0 alsa-devel/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 alsa-devel alsa-devel/ https://lore.kernel.org/alsa-devel \
		alsa-devel@alsa-project.org
	public-inbox-index alsa-devel

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.alsa-project.alsa-devel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git