Message ID | E1Yr1y8-0006lG-Mw@rmk-PC.arm.linux.org.uk (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Hi, A couple of things I noticed below: 09.05.2015, 13:26, Russell King kirjoitti: > Add ALSA based HDMI AHB audio driver for dw_hdmi. The only buffer > format supported by the hardware is its own special IEC958 based format, > which is not compatible with any ALSA format. To avoid doing too much > data manipulation within the driver, we support only ALSAs IEC958 LE and > 24-bit PCM formats for 2 to 6 channels, which we convert to its hardware > format. > > A more desirable solution would be to have this conversion in userspace, > but ALSA does not appear to allow such transformations outside of > libasound itself. > > Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk> > --- > drivers/gpu/drm/bridge/Kconfig | 10 + > drivers/gpu/drm/bridge/Makefile | 1 + > drivers/gpu/drm/bridge/dw_hdmi-ahb-audio.c | 560 +++++++++++++++++++++++++++++ > drivers/gpu/drm/bridge/dw_hdmi-ahb-audio.h | 13 + > drivers/gpu/drm/bridge/dw_hdmi.c | 24 ++ > drivers/gpu/drm/bridge/dw_hdmi.h | 3 + > 6 files changed, 611 insertions(+) > create mode 100644 drivers/gpu/drm/bridge/dw_hdmi-ahb-audio.c > create mode 100644 drivers/gpu/drm/bridge/dw_hdmi-ahb-audio.h > [...] > +static void dw_hdmi_create_cs(struct snd_dw_hdmi *dw, > + struct snd_pcm_runtime *runtime) > +{ > + u8 cs[4]; > + unsigned ch, i, j; > + > + snd_pcm_create_iec958_consumer(runtime, cs, sizeof(cs)); I think generally drivers have left the iec958 bits for userspace to handle, i.e. via a "IEC958 Playback Default" (SNDRV_CTL_NAME_IEC958("",PLAYBACK,DEFAULT)) mixer element, with defaults coming from /usr/share/alsa/pcm/hdmi.conf... Looking at the sound driver tree, now, though, they are already somewhat inconsistent: 1. Most drivers: iec958 bits from userspace, except for hw-set bits. 2. Some drivers (e.g. ctxfi, fsl_spdif): Some bits such as rate set by driver, but everything is also exposed to userspace. At least in fsl_spdif case the driver sets the stuff in hw_params which would then get overwritten by userspace (which sets them after hw_params). 3. Some drivers (e.g. omap-hdmi-audio): Set by driver, not exposed to userspace, like in this patch. (Of course having userspace set them requires that the device has a proper entry in /usr/share/alsa/cards and the pcm device is accessed via the standard "hdmi" or "iec958" device names which perform the channel status word setup. I guess the ARM SoC stuff generally doesn't bother with that, explaining a bit why some kernel drivers set them by themselves). The main interest to iec958 bits from userspace is probably towards the non-audio bit, used for audio passthrough (the bit is not mandatory there, but it helps). Other drivers (well, I guess just HDA has this so far) also use the userspace-provided non-audio bit to also select (in conjunction with channels==8) the HBR mode, i.e. HD audio passthrough (which dw_hdmi also supports via DMA_CONF0 but not enabled on this patch). Since this isn't the first driver doing things this way, and this doesn't really prevent exposing them to userspace later on, I guess this patch is still OK here, unless the ALSA people think otherwise... [...] > +static struct snd_pcm_hardware dw_hdmi_hw = { > + .info = SNDRV_PCM_INFO_INTERLEAVED | > + SNDRV_PCM_INFO_BLOCK_TRANSFER | > + SNDRV_PCM_INFO_MMAP | > + SNDRV_PCM_INFO_MMAP_VALID, > + .formats = SNDRV_PCM_FMTBIT_IEC958_SUBFRAME_LE | > + SNDRV_PCM_FMTBIT_S24_LE, > + .rates = SNDRV_PCM_RATE_32000 | > + SNDRV_PCM_RATE_44100 | > + SNDRV_PCM_RATE_48000 | > + SNDRV_PCM_RATE_88200 | > + SNDRV_PCM_RATE_96000 | > + SNDRV_PCM_RATE_176400 | > + SNDRV_PCM_RATE_192000, > + .channels_min = 2, > + .channels_max = 8, You are providing multichannel support, but AFAICS you are not setting the channel allocation (CA) bits in the audio infoframe anywhere (HDMI_FC_AUDICONF2 register). HDMI_FC_AUDICONF2 register default value is 0x00, which means plain stereo (per CEA-861). If this is what goes on to the HDMI link as well, the audio sink should ignore the other channels. Did you check that multichannel PCM actually works? (maybe I'm just missing where CA is set) Note also that I think this HW uses the native HDMI channel order (from CEA-861), which is different from the ALSA default channel order, so you should inform userspace of the channel order (via snd_pcm_add_chmap_ctls()). The channel order is specified by the CA value I mentioned above. Assuming I'm not missing something which makes everything work already, one of these should be implemented: (a) Provide all the chmaps (i.e. one per CA value) as a list for userspace to select one, and provide the active chmap to userspace as well. (b) Just hardcode a single CA value per channel count (which covers 99% of use cases), and provide the corresponding active chmap to userspace. (c) channels_max = 2. Both (a) and (b) are generic stuff that could/should be in helpers for all drivers to use (if (b), preferably with an interface that allows easily extending it to (a) in the future). Some of the code from sound/pci/hda/patch_hdmi.c should be useful (at least the CA table it has - this stuff really shoud be outside the driver). Note that much of the complexity of patch_hdmi.c comes from the fact that the HW there supports channel remapping (SNDRV_CTL_TLVT_CHMAP_VAR or _PAIRED), which dw_hdmi doesn't (SNDRV_CTL_TLVT_CHMAP_FIXED).
On Sat, May 09, 2015 at 07:49:44PM +0300, Anssi Hannula wrote: > (Of course having userspace set them requires that the device has a > proper entry in /usr/share/alsa/cards and the pcm device is accessed via > the standard "hdmi" or "iec958" device names which perform the channel > status word setup. I guess the ARM SoC stuff generally doesn't bother > with that, explaining a bit why some kernel drivers set them by themselves). I'm not sure that's sufficient - I haven't yet found where in the ALSA userspace, the AES bits are appropriately set according to the sample rate. > HDMI_FC_AUDICONF2 register default value is 0x00, which means plain > stereo (per CEA-861). If this is what goes on to the HDMI link as well, > the audio sink should ignore the other channels. > Did you check that multichannel PCM actually works? (maybe I'm just > missing where CA is set) I have no way to test multichannel support - not everyone has a plethora of HDMI devices readily accessible.
09.05.2015, 19:55, Russell King - ARM Linux kirjoitti: > On Sat, May 09, 2015 at 07:49:44PM +0300, Anssi Hannula wrote: >> (Of course having userspace set them requires that the device has a >> proper entry in /usr/share/alsa/cards and the pcm device is accessed via >> the standard "hdmi" or "iec958" device names which perform the channel >> status word setup. I guess the ARM SoC stuff generally doesn't bother >> with that, explaining a bit why some kernel drivers set them by themselves). > > I'm not sure that's sufficient - I haven't yet found where in the ALSA > userspace, the AES bits are appropriately set according to the sample > rate. Right, that is left to the applications (e.g. VLC and Kodi do that). I'm under the impression that sinks do not normally care about this value, though, but that could just be because most desktop HW sets it by themselves. But indeed, that is one reason to have the kernel set it.
On Sat, May 09, 2015 at 08:07:45PM +0300, Anssi Hannula wrote: > 09.05.2015, 19:55, Russell King - ARM Linux kirjoitti: > > On Sat, May 09, 2015 at 07:49:44PM +0300, Anssi Hannula wrote: > >> (Of course having userspace set them requires that the device has a > >> proper entry in /usr/share/alsa/cards and the pcm device is accessed via > >> the standard "hdmi" or "iec958" device names which perform the channel > >> status word setup. I guess the ARM SoC stuff generally doesn't bother > >> with that, explaining a bit why some kernel drivers set them by themselves). > > > > I'm not sure that's sufficient - I haven't yet found where in the ALSA > > userspace, the AES bits are appropriately set according to the sample > > rate. > > Right, that is left to the applications (e.g. VLC and Kodi do that). I'm > under the impression that sinks do not normally care about this value, > though, but that could just be because most desktop HW sets it by > themselves. No, that seems totally wrong to me. What if you open the device using aplay? Or pulseaudio? Or madplay? Or another audio application which thinks it's addressing a standard PCM device. Why should every audio user have some code in it to generate the IEC bits? Even VLC _doesn't_ if it's outputting to a standard audio - in other words, if you don't tick the SPDIF direct output option which defaults to disabled (which, when enabled, opens the device passing the AES bits _and_ permits it to send a compressed audio stream.) I've looked at this in VLC many times... I think you're a little confused about what userspace does here.
On Sat, May 09, 2015 at 06:40:54PM +0100, Russell King - ARM Linux wrote: > Even VLC _doesn't_ if it's outputting to a standard audio - in other > words, if you don't tick the SPDIF direct output option which defaults > to disabled (which, when enabled, opens the device passing the AES > bits _and_ permits it to send a compressed audio stream.) I've looked > at this in VLC many times... FYI, here's the code: vlc_fourcc_t fourcc = aout->format.i_format; bool spdif = false; switch (fourcc) { ... other linear float/integer codec IDs ... case VLC_CODEC_U16B: pcm_format = SND_PCM_FORMAT_U16_BE; break; case VLC_CODEC_U16L: pcm_format = SND_PCM_FORMAT_U16_LE; break; ... default: if (AOUT_FMT_SPDIF(&aout->format)) spdif = var_InheritBool (aout, "spdif"); if (spdif) { fourcc = VLC_CODEC_SPDIFL; pcm_format = SND_PCM_FORMAT_S16; } else if (HAVE_FPU) { fourcc = VLC_CODEC_FL32; pcm_format = SND_PCM_FORMAT_FLOAT; } else { fourcc = VLC_CODEC_S16N; pcm_format = SND_PCM_FORMAT_S16; } } /* 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")) { unsigned aes3; 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; } free (device); 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) return VLC_ENOMEM; } ... /* VLC always has a resampler. No need for ALSA's. */ const int mode = SND_PCM_NO_AUTO_RESAMPLE; int val = snd_pcm_open (&pcm, device, SND_PCM_STREAM_PLAYBACK, mode); So, the result is: * VLC opens the ALSA device _without_ AES information if the fourcc being passed if the format is a linear PCM type. * If the format is not a linear PCM type _and_ it is a format which satisfies AOUT_FMT_SPDIF(), _and_ the SPDIF passthrough option is enabled, _then_ we open the IEC958 audio output device with the first four AES data bytes specified with SND_PCM_FORMAT_S16. Moreover, the first data byte always indicates that the stream is non-audio. So, in the case of linear PCM, the AES data bytes are *not* specified by VLC. So, if we don't have the kernel driver specifying the IEC958 information for HDMI/SPDIF outputs, and users of ALSA's userspace APIs don't generate them for linear PCM, how are the required AES data bytes generated?
09.05.2015, 20:40, Russell King - ARM Linux kirjoitti: > On Sat, May 09, 2015 at 08:07:45PM +0300, Anssi Hannula wrote: >> 09.05.2015, 19:55, Russell King - ARM Linux kirjoitti: >>> On Sat, May 09, 2015 at 07:49:44PM +0300, Anssi Hannula wrote: >>>> (Of course having userspace set them requires that the device has a >>>> proper entry in /usr/share/alsa/cards and the pcm device is accessed via >>>> the standard "hdmi" or "iec958" device names which perform the channel >>>> status word setup. I guess the ARM SoC stuff generally doesn't bother >>>> with that, explaining a bit why some kernel drivers set them by themselves). >>> >>> I'm not sure that's sufficient - I haven't yet found where in the ALSA >>> userspace, the AES bits are appropriately set according to the sample >>> rate. >> >> Right, that is left to the applications (e.g. VLC and Kodi do that). I'm >> under the impression that sinks do not normally care about this value, >> though, but that could just be because most desktop HW sets it by >> themselves. > > No, that seems totally wrong to me. > > What if you open the device using aplay? Or pulseaudio? Or madplay? > Or another audio application which thinks it's addressing a standard > PCM device. Then, unless the driver or HW sets it, it is not set. > Why should every audio user have some code in it to generate the IEC > bits? Yeah, it makes zero sense of course, I wasn't claiming otherwise (sorry if it seemed like it). > Even VLC _doesn't_ if it's outputting to a standard audio - in other > words, if you don't tick the SPDIF direct output option which defaults > to disabled (which, when enabled, opens the device passing the AES > bits _and_ permits it to send a compressed audio stream.) I've looked > at this in VLC many times... That is my understanding as well. Same for pulseaudio, it doesn't set any AES bits except for passthrough (and most other applications never set them). > I think you're a little confused about what userspace does here.
On Sat, May 09, 2015 at 08:55:10PM +0300, Anssi Hannula wrote: > 09.05.2015, 20:40, Russell King - ARM Linux kirjoitti: > > Even VLC _doesn't_ if it's outputting to a standard audio - in other > > words, if you don't tick the SPDIF direct output option which defaults > > to disabled (which, when enabled, opens the device passing the AES > > bits _and_ permits it to send a compressed audio stream.) I've looked > > at this in VLC many times... > > That is my understanding as well. Same for pulseaudio, it doesn't set > any AES bits except for passthrough (and most other applications never > set them). Right, so when you're dealing with HDMI, where it's required that the AES bits contain accurate information, the only real option is to set it appropriately in the driver if userspace doesn't specify the AES data bits. Now, with the dw-hdmi-ahb driver, I'm doing something sensible - and yes, I do have a card file in /usr/share/alsa (see below). What this does is ensure that linear PCM is converted to 24-bit PCM (which makes the kernel conversion to the required hardware format easier - I hate this, I'd much prefer it to be done in userspace.) 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.) dw-hdmi-ahb-aud.pcm.default { @args [ CARD ] @args.CARD { type string } type asym playback.pcm { type softvol slave.pcm { type plug slave.pcm { @func concat strings [ "dmix:" $CARD ",FORMAT=S24_LE" ] } } control { name "PCM Playback Volume" card $CARD } } } <confdir:pcm/iec958.conf> dw-hdmi-ahb-aud.pcm.iec958.0 { @args [ CARD AES0 AES1 AES2 AES3 ] @args.CARD { type string } @args.AES0 { type integer } @args.AES1 { type integer } @args.AES2 { type integer } @args.AES3 { type integer } type iec958 slave.pcm { type hw card $CARD device 0 } slave.format IEC958_SUBFRAME_LE status [ $AES0 $AES1 $AES2 $AES3 ] }
09.05.2015, 21:11, Russell King - ARM Linux kirjoitti: > On Sat, May 09, 2015 at 08:55:10PM +0300, Anssi Hannula wrote: >> 09.05.2015, 20:40, Russell King - ARM Linux kirjoitti: >>> Even VLC _doesn't_ if it's outputting to a standard audio - in other >>> words, if you don't tick the SPDIF direct output option which defaults >>> to disabled (which, when enabled, opens the device passing the AES >>> bits _and_ permits it to send a compressed audio stream.) I've looked >>> at this in VLC many times... >> >> That is my understanding as well. Same for pulseaudio, it doesn't set >> any AES bits except for passthrough (and most other applications never >> set them). > > Right, so when you're dealing with HDMI, where it's required that the > AES bits contain accurate information, the only real option is to set > it appropriately in the driver if userspace doesn't specify the AES > data bits. I wonder whether receivers actually care with HDMI (they generally don't with S/PDIF) - that's one tidbit for me to test later... But of course it doesn't change much with the matter at hand, in any case we should strive to get the bits correct if only because the HDMI spec requires them to be (I don't think they were optional in IEC 60958-3 either, though). > Now, with the dw-hdmi-ahb driver, I'm doing something sensible Right. 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... > - and > yes, I do have a card file in /usr/share/alsa (see below). > > What this does is ensure that linear PCM is converted to 24-bit PCM > (which makes the kernel conversion to the required hardware format > easier - I hate this, I'd much prefer it to be done in userspace.) 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)... > 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. [...] > > <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). > @args [ CARD AES0 AES1 AES2 AES3 ] > @args.CARD { type string } > @args.AES0 { type integer } > @args.AES1 { type integer } > @args.AES2 { type integer } > @args.AES3 { type integer } > type iec958 > slave.pcm { > type hw > card $CARD > device 0 > } > slave.format IEC958_SUBFRAME_LE > status [ $AES0 $AES1 $AES2 $AES3 ] > } > >
On Sun, May 10, 2015 at 09:59:42PM +0300, Anssi Hannula wrote: > I wonder whether receivers actually care with HDMI (they generally don't > with S/PDIF) - that's one tidbit for me to test later... But of course > it doesn't change much with the matter at hand, in any case we should > strive to get the bits correct if only because the HDMI spec requires > them to be (I don't think they were optional in IEC 60958-3 either, though). I suspect they don't care too much, but the HDMI spec does require them to be correct. If we're trying to aim for best compatibility, setting them correctly is something we should strive to do. > 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.) > 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. > > 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; > > <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. 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 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?
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.
On Sun, May 10, 2015 at 08:33:20PM +0100, Russell King - ARM Linux wrote: > 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.) For what it's worth this is exactly what I'd be expecting to happen - the kernel should do something by default and then let applications override it if they so desire.
diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig index acef3223772c..56ed35fe0734 100644 --- a/drivers/gpu/drm/bridge/Kconfig +++ b/drivers/gpu/drm/bridge/Kconfig @@ -3,6 +3,16 @@ config DRM_DW_HDMI depends on DRM select DRM_KMS_HELPER +config DRM_DW_HDMI_AHB_AUDIO + tristate "Synopsis Designware AHB Audio interface" + depends on DRM_DW_HDMI && SND + select SND_PCM + select SND_PCM_IEC958 + help + Support the AHB Audio interface which is part of the Synopsis + Designware HDMI block. This is used in conjunction with + the i.MX6 HDMI driver. + config DRM_PTN3460 tristate "PTN3460 DP/LVDS bridge" depends on DRM diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile index 8dfebd984370..eb80dbbb8365 100644 --- a/drivers/gpu/drm/bridge/Makefile +++ b/drivers/gpu/drm/bridge/Makefile @@ -3,3 +3,4 @@ ccflags-y := -Iinclude/drm obj-$(CONFIG_DRM_PS8622) += ps8622.o obj-$(CONFIG_DRM_PTN3460) += ptn3460.o obj-$(CONFIG_DRM_DW_HDMI) += dw_hdmi.o +obj-$(CONFIG_DRM_DW_HDMI_AHB_AUDIO) += dw_hdmi-ahb-audio.o diff --git a/drivers/gpu/drm/bridge/dw_hdmi-ahb-audio.c b/drivers/gpu/drm/bridge/dw_hdmi-ahb-audio.c new file mode 100644 index 000000000000..e98c291268f4 --- /dev/null +++ b/drivers/gpu/drm/bridge/dw_hdmi-ahb-audio.c @@ -0,0 +1,560 @@ +/* + * DesignWare HDMI audio driver + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + * Written and tested against the Designware HDMI Tx found in iMX6. + */ +#include <linux/io.h> +#include <linux/interrupt.h> +#include <linux/module.h> +#include <linux/platform_device.h> +#include <drm/bridge/dw_hdmi.h> + +#include <sound/asoundef.h> +#include <sound/core.h> +#include <sound/initval.h> +#include <sound/pcm.h> +#include <sound/pcm_iec958.h> + +#include "dw_hdmi-ahb-audio.h" + +#define DRIVER_NAME "dw-hdmi-ahb-audio" + +/* Provide some bits rather than bit offsets */ +enum { + HDMI_AHB_DMA_CONF0_SW_FIFO_RST = BIT(7), + HDMI_AHB_DMA_CONF0_EN_HLOCK = BIT(3), + HDMI_AHB_DMA_START_START = BIT(0), + HDMI_AHB_DMA_STOP_STOP = BIT(0), + HDMI_IH_MUTE_AHBDMAAUD_STAT0_ERROR = BIT(5), + HDMI_IH_MUTE_AHBDMAAUD_STAT0_LOST = BIT(4), + HDMI_IH_MUTE_AHBDMAAUD_STAT0_RETRY = BIT(3), + HDMI_IH_MUTE_AHBDMAAUD_STAT0_DONE = BIT(2), + HDMI_IH_MUTE_AHBDMAAUD_STAT0_BUFFFULL = BIT(1), + HDMI_IH_MUTE_AHBDMAAUD_STAT0_BUFFEMPTY = BIT(0), + HDMI_IH_MUTE_AHBDMAAUD_STAT0_ALL = + HDMI_IH_MUTE_AHBDMAAUD_STAT0_ERROR | + HDMI_IH_MUTE_AHBDMAAUD_STAT0_LOST | + HDMI_IH_MUTE_AHBDMAAUD_STAT0_RETRY | + HDMI_IH_MUTE_AHBDMAAUD_STAT0_DONE | + HDMI_IH_MUTE_AHBDMAAUD_STAT0_BUFFFULL | + HDMI_IH_MUTE_AHBDMAAUD_STAT0_BUFFEMPTY, + HDMI_IH_AHBDMAAUD_STAT0_ERROR = BIT(5), + HDMI_IH_AHBDMAAUD_STAT0_LOST = BIT(4), + HDMI_IH_AHBDMAAUD_STAT0_RETRY = BIT(3), + HDMI_IH_AHBDMAAUD_STAT0_DONE = BIT(2), + HDMI_IH_AHBDMAAUD_STAT0_BUFFFULL = BIT(1), + HDMI_IH_AHBDMAAUD_STAT0_BUFFEMPTY = BIT(0), + HDMI_IH_AHBDMAAUD_STAT0_ALL = + HDMI_IH_AHBDMAAUD_STAT0_ERROR | + HDMI_IH_AHBDMAAUD_STAT0_LOST | + HDMI_IH_AHBDMAAUD_STAT0_RETRY | + HDMI_IH_AHBDMAAUD_STAT0_DONE | + HDMI_IH_AHBDMAAUD_STAT0_BUFFFULL | + HDMI_IH_AHBDMAAUD_STAT0_BUFFEMPTY, + HDMI_AHB_DMA_CONF0_INCR16 = 2 << 1, + HDMI_AHB_DMA_CONF0_INCR8 = 1 << 1, + HDMI_AHB_DMA_CONF0_INCR4 = 0, + HDMI_AHB_DMA_CONF0_BURST_MODE = BIT(0), + HDMI_AHB_DMA_MASK_DONE = BIT(7), + HDMI_REVISION_ID = 0x0001, + HDMI_IH_AHBDMAAUD_STAT0 = 0x0109, + HDMI_IH_MUTE_AHBDMAAUD_STAT0 = 0x0189, + HDMI_AHB_DMA_CONF0 = 0x3600, + HDMI_AHB_DMA_START = 0x3601, + HDMI_AHB_DMA_STOP = 0x3602, + HDMI_AHB_DMA_THRSLD = 0x3603, + HDMI_AHB_DMA_STRADDR0 = 0x3604, + HDMI_AHB_DMA_STPADDR0 = 0x3608, + HDMI_AHB_DMA_MASK = 0x3614, + HDMI_AHB_DMA_POL = 0x3615, + HDMI_AHB_DMA_CONF1 = 0x3616, + HDMI_AHB_DMA_BUFFPOL = 0x361a, +}; + +struct snd_dw_hdmi { + struct snd_card *card; + struct snd_pcm *pcm; + struct dw_hdmi_audio_data data; + struct snd_pcm_substream *substream; + void (*reformat)(struct snd_dw_hdmi *, size_t, size_t); + void *buf_src; + void *buf_dst; + dma_addr_t buf_addr; + unsigned buf_offset; + unsigned buf_period; + unsigned buf_size; + unsigned channels; + u8 revision; + u8 iec_offset; + u8 cs[192][8]; +}; + +static void dw_hdmi_writel(unsigned long val, void __iomem *ptr) +{ + writeb_relaxed(val, ptr); + writeb_relaxed(val >> 8, ptr + 1); + writeb_relaxed(val >> 16, ptr + 2); + writeb_relaxed(val >> 24, ptr + 3); +} + +/* + * Convert to hardware format: The userspace buffer contains IEC958 samples, + * with the PCUV bits in bits 31..28 and audio samples in bits 27..4. We + * need these to be in bits 27..24, with the IEC B bit in bit 28, and audio + * samples in 23..0. + * + * Default preamble in bits 3..0: 8 = block start, 4 = even 2 = odd + * + * Ideally, we could do with having the data properly formatted in userspace. + */ +static void dw_hdmi_reformat_iec958(struct snd_dw_hdmi *dw, + size_t offset, size_t bytes) +{ + u32 *src = dw->buf_src + offset; + u32 *dst = dw->buf_dst + offset; + u32 *end = dw->buf_src + offset + bytes; + + do { + u32 b, sample = *src++; + + b = (sample & 8) << (28 - 3); + + sample >>= 4; + + *dst++ = sample | b; + } while (src < end); +} + +static u32 parity(u32 sample) +{ + sample ^= sample >> 16; + sample ^= sample >> 8; + sample ^= sample >> 4; + sample ^= sample >> 2; + sample ^= sample >> 1; + return (sample & 1) << 27; +} + +static void dw_hdmi_reformat_s24(struct snd_dw_hdmi *dw, + size_t offset, size_t bytes) +{ + u32 *src = dw->buf_src + offset; + u32 *dst = dw->buf_dst + offset; + u32 *end = dw->buf_src + offset + bytes; + + do { + unsigned i; + u8 *cs; + + cs = dw->cs[dw->iec_offset++]; + if (dw->iec_offset >= 192) + dw->iec_offset = 0; + + i = dw->channels; + do { + u32 sample = *src++; + + sample &= ~0xff000000; + sample |= *cs++ << 24; + sample |= parity(sample & ~0xf8000000); + + *dst++ = sample; + } while (--i); + } while (src < end); +} + +static void dw_hdmi_create_cs(struct snd_dw_hdmi *dw, + struct snd_pcm_runtime *runtime) +{ + u8 cs[4]; + unsigned ch, i, j; + + snd_pcm_create_iec958_consumer(runtime, cs, sizeof(cs)); + + memset(dw->cs, 0, sizeof(dw->cs)); + + for (ch = 0; ch < 8; ch++) { + cs[2] &= ~IEC958_AES2_CON_CHANNEL; + cs[2] |= (ch + 1) << 4; + + for (i = 0; i < ARRAY_SIZE(cs); i++) { + unsigned c = cs[i]; + + for (j = 0; j < 8; j++, c >>= 1) + dw->cs[i * 8 + j][ch] = (c & 1) << 2; + } + } + dw->cs[0][0] |= BIT(4); +} + +static void dw_hdmi_start_dma(struct snd_dw_hdmi *dw) +{ + void __iomem *base = dw->data.base; + unsigned offset = dw->buf_offset; + unsigned period = dw->buf_period; + u32 start, stop; + + dw->reformat(dw, offset, period); + + /* Clear all irqs before enabling irqs and starting DMA */ + writeb_relaxed(HDMI_IH_AHBDMAAUD_STAT0_ALL, + base + HDMI_IH_AHBDMAAUD_STAT0); + + start = dw->buf_addr + offset; + stop = start + period - 1; + + /* Setup the hardware start/stop addresses */ + dw_hdmi_writel(start, base + HDMI_AHB_DMA_STRADDR0); + dw_hdmi_writel(stop, base + HDMI_AHB_DMA_STPADDR0); + + writeb_relaxed((u8)~HDMI_AHB_DMA_MASK_DONE, base + HDMI_AHB_DMA_MASK); + writeb(HDMI_AHB_DMA_START_START, base + HDMI_AHB_DMA_START); + + offset += period; + if (offset >= dw->buf_size) + offset = 0; + dw->buf_offset = offset; +} + +static void dw_hdmi_stop_dma(struct snd_dw_hdmi *dw) +{ + dw->substream = NULL; + + /* Disable interrupts before disabling DMA */ + writeb_relaxed(~0, dw->data.base + HDMI_AHB_DMA_MASK); + writeb_relaxed(HDMI_AHB_DMA_STOP_STOP, dw->data.base + HDMI_AHB_DMA_STOP); +} + +static irqreturn_t snd_dw_hdmi_irq(int irq, void *data) +{ + struct snd_dw_hdmi *dw = data; + struct snd_pcm_substream *substream; + unsigned stat; + + stat = readb_relaxed(dw->data.base + HDMI_IH_AHBDMAAUD_STAT0); + if (!stat) + return IRQ_NONE; + + writeb_relaxed(stat, dw->data.base + HDMI_IH_AHBDMAAUD_STAT0); + + substream = dw->substream; + if (stat & HDMI_IH_AHBDMAAUD_STAT0_DONE && substream) { + snd_pcm_period_elapsed(substream); + if (dw->substream) + dw_hdmi_start_dma(dw); + } + + return IRQ_HANDLED; +} + +static struct snd_pcm_hardware dw_hdmi_hw = { + .info = SNDRV_PCM_INFO_INTERLEAVED | + SNDRV_PCM_INFO_BLOCK_TRANSFER | + SNDRV_PCM_INFO_MMAP | + SNDRV_PCM_INFO_MMAP_VALID, + .formats = SNDRV_PCM_FMTBIT_IEC958_SUBFRAME_LE | + SNDRV_PCM_FMTBIT_S24_LE, + .rates = SNDRV_PCM_RATE_32000 | + SNDRV_PCM_RATE_44100 | + SNDRV_PCM_RATE_48000 | + SNDRV_PCM_RATE_88200 | + SNDRV_PCM_RATE_96000 | + SNDRV_PCM_RATE_176400 | + SNDRV_PCM_RATE_192000, + .channels_min = 2, + .channels_max = 8, + .buffer_bytes_max = 64 * 1024, + .period_bytes_min = 256, + .period_bytes_max = 8192, /* ERR004323: must limit to 8k */ + .periods_min = 2, + .periods_max = 16, + .fifo_size = 0, +}; + +static int dw_hdmi_open(struct snd_pcm_substream *substream) +{ + struct snd_pcm_runtime *runtime = substream->runtime; + struct snd_dw_hdmi *dw = substream->private_data; + void __iomem *base = dw->data.base; + int ret; + + runtime->hw = dw_hdmi_hw; + + ret = snd_pcm_limit_hw_rates(runtime); + if (ret < 0) + return ret; + + ret = snd_pcm_hw_constraint_integer(runtime, SNDRV_PCM_HW_PARAM_PERIODS); + if (ret < 0) + return ret; + + /* Clear FIFO */ + writeb_relaxed(HDMI_AHB_DMA_CONF0_SW_FIFO_RST, + base + HDMI_AHB_DMA_CONF0); + + /* Configure interrupt polarities */ + writeb_relaxed(~0, base + HDMI_AHB_DMA_POL); + writeb_relaxed(~0, base + HDMI_AHB_DMA_BUFFPOL); + + /* Keep interrupts masked, and clear any pending */ + writeb_relaxed(~0, base + HDMI_AHB_DMA_MASK); + writeb_relaxed(~0, base + HDMI_IH_AHBDMAAUD_STAT0); + + ret = request_irq(dw->data.irq, snd_dw_hdmi_irq, IRQF_SHARED, + "dw-hdmi-audio", dw); + if (ret) + return ret; + + /* Un-mute done interrupt */ + writeb_relaxed(HDMI_IH_MUTE_AHBDMAAUD_STAT0_ALL & + ~HDMI_IH_MUTE_AHBDMAAUD_STAT0_DONE, + base + HDMI_IH_MUTE_AHBDMAAUD_STAT0); + + return 0; +} + +static int dw_hdmi_close(struct snd_pcm_substream *substream) +{ + struct snd_dw_hdmi *dw = substream->private_data; + + /* Mute all interrupts */ + writeb_relaxed(HDMI_IH_MUTE_AHBDMAAUD_STAT0_ALL, + dw->data.base + HDMI_IH_MUTE_AHBDMAAUD_STAT0); + + free_irq(dw->data.irq, dw); + + return 0; +} + +static int dw_hdmi_hw_free(struct snd_pcm_substream *substream) +{ + return snd_pcm_lib_free_vmalloc_buffer(substream); +} + +static int dw_hdmi_hw_params(struct snd_pcm_substream *substream, + struct snd_pcm_hw_params *params) +{ + return snd_pcm_lib_alloc_vmalloc_buffer(substream, + params_buffer_bytes(params)); +} + +static int dw_hdmi_prepare(struct snd_pcm_substream *substream) +{ + struct snd_pcm_runtime *runtime = substream->runtime; + struct snd_dw_hdmi *dw = substream->private_data; + u8 threshold, conf0, conf1; + + /* Setup as per 3.0.5 FSL 4.1.0 BSP */ + switch (dw->revision) { + case 0x0a: + conf0 = HDMI_AHB_DMA_CONF0_BURST_MODE | + HDMI_AHB_DMA_CONF0_INCR4; + if (runtime->channels == 2) + threshold = 126; + else + threshold = 124; + break; + case 0x1a: + conf0 = HDMI_AHB_DMA_CONF0_BURST_MODE | + HDMI_AHB_DMA_CONF0_INCR8; + threshold = 128; + break; + default: + /* NOTREACHED */ + return -EINVAL; + } + + dw_hdmi_set_sample_rate(dw->data.hdmi, runtime->rate); + + /* Minimum number of bytes in the fifo. */ + runtime->hw.fifo_size = threshold * 32; + + conf0 |= HDMI_AHB_DMA_CONF0_EN_HLOCK; + conf1 = (1 << runtime->channels) - 1; + + writeb_relaxed(threshold, dw->data.base + HDMI_AHB_DMA_THRSLD); + writeb_relaxed(conf0, dw->data.base + HDMI_AHB_DMA_CONF0); + writeb_relaxed(conf1, dw->data.base + HDMI_AHB_DMA_CONF1); + + switch (runtime->format) { + case SNDRV_PCM_FORMAT_IEC958_SUBFRAME_LE: + dw->reformat = dw_hdmi_reformat_iec958; + break; + case SNDRV_PCM_FORMAT_S24_LE: + dw_hdmi_create_cs(dw, runtime); + dw->reformat = dw_hdmi_reformat_s24; + break; + } + dw->iec_offset = 0; + dw->channels = runtime->channels; + dw->buf_src = runtime->dma_area; + dw->buf_dst = substream->dma_buffer.area; + dw->buf_addr = substream->dma_buffer.addr; + dw->buf_period = snd_pcm_lib_period_bytes(substream); + dw->buf_size = snd_pcm_lib_buffer_bytes(substream); + + return 0; +} + +static int dw_hdmi_trigger(struct snd_pcm_substream *substream, int cmd) +{ + struct snd_dw_hdmi *dw = substream->private_data; + int ret = 0; + + switch (cmd) { + case SNDRV_PCM_TRIGGER_START: + dw->buf_offset = 0; + dw->substream = substream; + dw_hdmi_start_dma(dw); + dw_hdmi_audio_enable(dw->data.hdmi); + substream->runtime->delay = substream->runtime->period_size; + break; + + case SNDRV_PCM_TRIGGER_STOP: + dw_hdmi_stop_dma(dw); + dw_hdmi_audio_disable(dw->data.hdmi); + break; + + default: + ret = -EINVAL; + break; + } + + return ret; +} + +static snd_pcm_uframes_t dw_hdmi_pointer(struct snd_pcm_substream *substream) +{ + struct snd_pcm_runtime *runtime = substream->runtime; + struct snd_dw_hdmi *dw = substream->private_data; + + return bytes_to_frames(runtime, dw->buf_offset); +} + +static struct snd_pcm_ops snd_dw_hdmi_ops = { + .open = dw_hdmi_open, + .close = dw_hdmi_close, + .ioctl = snd_pcm_lib_ioctl, + .hw_params = dw_hdmi_hw_params, + .hw_free = dw_hdmi_hw_free, + .prepare = dw_hdmi_prepare, + .trigger = dw_hdmi_trigger, + .pointer = dw_hdmi_pointer, + .page = snd_pcm_lib_get_vmalloc_page, +}; + +static int snd_dw_hdmi_probe(struct platform_device *pdev) +{ + const struct dw_hdmi_audio_data *data = pdev->dev.platform_data; + struct device *dev = pdev->dev.parent; + struct snd_dw_hdmi *dw; + struct snd_card *card; + struct snd_pcm *pcm; + unsigned revision; + int ret; + + writeb_relaxed(HDMI_IH_MUTE_AHBDMAAUD_STAT0_ALL, + data->base + HDMI_IH_MUTE_AHBDMAAUD_STAT0); + revision = readb_relaxed(data->base + HDMI_REVISION_ID); + if (revision != 0x0a && revision != 0x1a) { + dev_err(dev, "dw-hdmi-audio: unknown revision 0x%02x\n", + revision); + return -ENXIO; + } + + ret = snd_card_new(dev, SNDRV_DEFAULT_IDX1, SNDRV_DEFAULT_STR1, + THIS_MODULE, sizeof(struct snd_dw_hdmi), &card); + if (ret < 0) + return ret; + + strlcpy(card->driver, DRIVER_NAME, sizeof(card->driver)); + strlcpy(card->shortname, "DW-HDMI", sizeof(card->shortname)); + snprintf(card->longname, sizeof(card->longname), + "%s rev 0x%02x, irq %d", card->shortname, revision, + data->irq); + + dw = card->private_data; + dw->card = card; + dw->data = *data; + dw->revision = revision; + + ret = snd_pcm_new(card, "DW HDMI", 0, 1, 0, &pcm); + if (ret < 0) + goto err; + + dw->pcm = pcm; + pcm->private_data = dw; + strlcpy(pcm->name, DRIVER_NAME, sizeof(pcm->name)); + snd_pcm_set_ops(pcm, SNDRV_PCM_STREAM_PLAYBACK, &snd_dw_hdmi_ops); + + snd_pcm_lib_preallocate_pages_for_all(pcm, SNDRV_DMA_TYPE_DEV, + dev, 64 * 1024, 64 * 1024); + + ret = snd_card_register(card); + if (ret < 0) + goto err; + + platform_set_drvdata(pdev, dw); + + return 0; + +err: + snd_card_free(card); + return ret; +} + +static int snd_dw_hdmi_remove(struct platform_device *pdev) +{ + struct snd_dw_hdmi *dw = platform_get_drvdata(pdev); + + snd_card_free(dw->card); + + return 0; +} + +#ifdef CONFIG_PM_SLEEP +static int snd_dw_hdmi_suspend(struct device *dev) +{ + struct snd_dw_hdmi *dw = dev_get_drvdata(dev); + + snd_power_change_state(dw->card, SNDRV_CTL_POWER_D3cold); + snd_pcm_suspend_all(dw->pcm); + + return 0; +} + +static int snd_dw_hdmi_resume(struct device *dev) +{ + struct snd_dw_hdmi *dw = dev_get_drvdata(dev); + + snd_power_change_state(dw->card, SNDRV_CTL_POWER_D0); + + return 0; +} + +static SIMPLE_DEV_PM_OPS(snd_dw_hdmi_pm, snd_dw_hdmi_suspend, + snd_dw_hdmi_resume); +#define PM_OPS &snd_dw_hdmi_pm +#else +#define PM_OPS NULL +#endif + +static struct platform_driver snd_dw_hdmi_driver = { + .probe = snd_dw_hdmi_probe, + .remove = snd_dw_hdmi_remove, + .driver = { + .name = DRIVER_NAME, + .owner = THIS_MODULE, + .pm = PM_OPS, + }, +}; + +module_platform_driver(snd_dw_hdmi_driver); + +MODULE_AUTHOR("Russell King <rmk+kernel@arm.linux.org.uk>"); +MODULE_LICENSE("GPL"); +MODULE_ALIAS(PLATFORM_MODULE_PREFIX DRIVER_NAME); diff --git a/drivers/gpu/drm/bridge/dw_hdmi-ahb-audio.h b/drivers/gpu/drm/bridge/dw_hdmi-ahb-audio.h new file mode 100644 index 000000000000..1e840118d90a --- /dev/null +++ b/drivers/gpu/drm/bridge/dw_hdmi-ahb-audio.h @@ -0,0 +1,13 @@ +#ifndef DW_HDMI_AUDIO_H +#define DW_HDMI_AUDIO_H + +struct dw_hdmi; + +struct dw_hdmi_audio_data { + phys_addr_t phys; + void __iomem *base; + int irq; + struct dw_hdmi *hdmi; +}; + +#endif diff --git a/drivers/gpu/drm/bridge/dw_hdmi.c b/drivers/gpu/drm/bridge/dw_hdmi.c index adda3a988f36..1cb427935ed2 100644 --- a/drivers/gpu/drm/bridge/dw_hdmi.c +++ b/drivers/gpu/drm/bridge/dw_hdmi.c @@ -28,6 +28,7 @@ #include <drm/bridge/dw_hdmi.h> #include "dw_hdmi.h" +#include "dw_hdmi-ahb-audio.h" #define HDMI_EDID_LEN 512 @@ -105,6 +106,7 @@ struct dw_hdmi { struct drm_encoder *encoder; struct drm_bridge *bridge; + struct platform_device *audio; enum dw_hdmi_devtype dev_type; struct device *dev; struct clk *isfr_clk; @@ -1592,7 +1594,9 @@ int dw_hdmi_bind(struct device *dev, struct device *master, { struct drm_device *drm = data; struct device_node *np = dev->of_node; + struct platform_device_info pdevinfo; struct device_node *ddc_node; + struct dw_hdmi_audio_data audio; struct dw_hdmi *hdmi; int ret; u32 val = 1; @@ -1712,6 +1716,23 @@ int dw_hdmi_bind(struct device *dev, struct device *master, /* Unmute interrupts */ hdmi_writeb(hdmi, ~HDMI_IH_PHY_STAT0_HPD, HDMI_IH_MUTE_PHY_STAT0); + memset(&pdevinfo, 0, sizeof(pdevinfo)); + pdevinfo.parent = dev; + pdevinfo.id = PLATFORM_DEVID_AUTO; + + if (hdmi_readb(hdmi, HDMI_CONFIG1_ID) & HDMI_CONFIG1_AHB) { + audio.phys = iores->start; + audio.base = hdmi->regs; + audio.irq = irq; + audio.hdmi = hdmi; + + pdevinfo.name = "dw-hdmi-ahb-audio"; + pdevinfo.data = &audio; + pdevinfo.size_data = sizeof(audio); + pdevinfo.dma_mask = DMA_BIT_MASK(32); + hdmi->audio = platform_device_register_full(&pdevinfo); + } + dev_set_drvdata(dev, hdmi); return 0; @@ -1729,6 +1750,9 @@ void dw_hdmi_unbind(struct device *dev, struct device *master, void *data) { struct dw_hdmi *hdmi = dev_get_drvdata(dev); + if (hdmi->audio && !IS_ERR(hdmi->audio)) + platform_device_unregister(hdmi->audio); + /* Disable all interrupts */ hdmi_writeb(hdmi, ~0, HDMI_IH_MUTE_PHY_STAT0); diff --git a/drivers/gpu/drm/bridge/dw_hdmi.h b/drivers/gpu/drm/bridge/dw_hdmi.h index 175dbc89a824..78e54e813212 100644 --- a/drivers/gpu/drm/bridge/dw_hdmi.h +++ b/drivers/gpu/drm/bridge/dw_hdmi.h @@ -545,6 +545,9 @@ #define HDMI_I2CM_FS_SCL_LCNT_0_ADDR 0x7E12 enum { +/* CONFIG1_ID field values */ + HDMI_CONFIG1_AHB = 0x01, + /* IH_FC_INT2 field values */ HDMI_IH_FC_INT2_OVERFLOW_MASK = 0x03, HDMI_IH_FC_INT2_LOW_PRIORITY_OVERFLOW = 0x02,
Add ALSA based HDMI AHB audio driver for dw_hdmi. The only buffer format supported by the hardware is its own special IEC958 based format, which is not compatible with any ALSA format. To avoid doing too much data manipulation within the driver, we support only ALSAs IEC958 LE and 24-bit PCM formats for 2 to 6 channels, which we convert to its hardware format. A more desirable solution would be to have this conversion in userspace, but ALSA does not appear to allow such transformations outside of libasound itself. Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk> --- drivers/gpu/drm/bridge/Kconfig | 10 + drivers/gpu/drm/bridge/Makefile | 1 + drivers/gpu/drm/bridge/dw_hdmi-ahb-audio.c | 560 +++++++++++++++++++++++++++++ drivers/gpu/drm/bridge/dw_hdmi-ahb-audio.h | 13 + drivers/gpu/drm/bridge/dw_hdmi.c | 24 ++ drivers/gpu/drm/bridge/dw_hdmi.h | 3 + 6 files changed, 611 insertions(+) create mode 100644 drivers/gpu/drm/bridge/dw_hdmi-ahb-audio.c create mode 100644 drivers/gpu/drm/bridge/dw_hdmi-ahb-audio.h