[alsa-devel] [PATCH 12/13] drm: bridge/dw_hdmi-ahb-audio: add audio driver
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
.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.
>>>
>>>
>>> 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 (,
> "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
[alsa-devel] [PATCH 12/13] drm: bridge/dw_hdmi-ahb-audio: add audio driver
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.
[...]
>
>
>
> 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 ]
> }
>
>
--
Anssi Hannula
[alsa-devel] [PATCH 12/13] drm: bridge/dw_hdmi-ahb-audio: add audio driver
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. -- Anssi Hannula
[alsa-devel] [PATCH 12/13] drm: bridge/dw_hdmi-ahb-audio: add audio driver
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. -- Anssi Hannula
[alsa-devel] [PATCH 12/13] drm: bridge/dw_hdmi-ahb-audio: add audio driver
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). -- -- Anssi Hannula
[PATCH RFC v2 10/13] sound/core: add DRM ELD helper
05.04.2015, 20:26, Russell King - ARM Linux kirjoitti: > On Sun, Apr 05, 2015 at 06:46:13PM +0200, Takashi Iwai wrote: >> >> One another question: don't we need to deal with the sample bits in >> sad[2]? > > It should, but I'm very wary about doing that without seeing more > examples of real SADs. Right now, all my examples only support > one SAD with either 2 channel or 6 channel audio at the standard > (basic) 32, 44.1 and 48kHz rates. > > The HDMI / CEA specs are very loose in their wording about the > short audio descriptors. I've no idea whether a sink can provide > (for example) descriptors such as: > > LPCM, 6 channel 32, 44.1, 48kHz > LPCM, 2 channel, 32, 44.1, 48, 96, 192kHz > > or whether have to describe that as a single descriptor. I only > have two TVs to test with here. For the record, yes, multiple LPCM SADs are relatively common, and I think I've seen some that offered more rates on a stereo SAD than on a multichannel SAD (like in your example). > What I'm concerned about is that when the ALSA parameter refining > starts, we start with (eg) 2-8 channels, 32-192kHz. Given that, > if we invoke the channel restriction before the rate restriction, > we would end up limiting to 2 channel at 32-192kHz. If we apply > the restrictions in the opposite order, we'd restrict to 6 > channel, 32-48kHz. Neither are obviously correct in this > circumstance, and I don't really see a way to solve it given my > understanding of the way ALSA's parameter refinement works. > > I suspect this is why most HDMI drivers are implemented such that > they take the maximum capabilities over all SADs, which would end > up restricting audio in the above case to: up to 6 channels, at > 32, 44.1, 48, 96 and 192kHz, even though 6 channel @ 192kHz isn't > hasn't been indicated as supported. > > Most of this is speculation though, based off what is in the > documentation. As I say, I don't have enough real-world examples > to get a feel for what manufacturers _actually_ do to give a hint > as to how the documentation should be interpreted. > > So, maybe I should just copy what everyone else does and take the > maximum of all descriptors... > -- Anssi Hannula
[alsa-devel] [PATCH RFC v2 REPOST 3/8] ASoC: davinci-evm: HDMI audio support for TDA998x trough McASP I2S bus
15.01.2014 13:27, Jyri Sarha kirjoitti: > On 12/31/2013 03:25 PM, Mark Brown wrote: >> On Fri, Dec 20, 2013 at 12:39:38PM +0200, Jyri Sarha wrote: >>> support. The only supported sample format is SNDRV_PCM_FORMAT_S32_LE. >>> The 8 least significant bits are ignored. >> >> Where does this constraint come from? >> > > From driver/gpu/drm/i2c/tda998x_drv.c. The driver configures CTS_N > register statically to a value that works only with 4 byte samples. > According to my tests it is possible to support 3 and 2 byte samples too > by changing the CTS_N register value, but I am not sure if the > configuration can be changed on the fly. My data sheet of the nxp chip > is very vague about the register definitions, but I suppose the register > configures some clock divider on the chip. HDMI supports only upto 24bit > audio and the data sheet states that any extraneous least significant > bits are ignored. That sounds strange, CTS/N values only depend on audio sample rate and TMDS/video clock, not on the audio format or the size of samples (HDMI spec sec 7.2 - Audio Sample Clock Capture and Regeneration). Sure there isn't anything more going on (like the used HDMI sink being more tolerant to wrong CTS/N with 4-byte samples, or more likely something else I can't immediately think of)? BTW, radeon driver has some code that calculates static CTS/N values since the hw autogeneration is not reliable there: https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/radeon/r600_hdmi.c Not that it directly helps now, but just for the record... -- Anssi Hannula
[PATCH 1/2] drm/radeon/audio: write audio/video latency info for DCE4/5
11.11.2013 17:55, Alex Deucher kirjoitti: > On Fri, Nov 8, 2013 at 6:24 AM, Anssi Hannula wrote: >> 18.10.2013 23:41, Alex Deucher kirjoitti: >>> Needed by the hda driver to properly set up synchronization >>> on the audio side. >>> >>> Signed-off-by: Alex Deucher >>> --- >>> drivers/gpu/drm/radeon/evergreen_hdmi.c | 37 >>> >>> drivers/gpu/drm/radeon/evergreend.h | 38 >>> + >>> 2 files changed, 75 insertions(+) >>> >>> diff --git a/drivers/gpu/drm/radeon/evergreen_hdmi.c >>> b/drivers/gpu/drm/radeon/evergreen_hdmi.c >>> index 5fbe486..abdc893 100644 >>> --- a/drivers/gpu/drm/radeon/evergreen_hdmi.c >>> +++ b/drivers/gpu/drm/radeon/evergreen_hdmi.c >> [...] >>> + if (connector->latency_present[0]) >>> + tmp = VIDEO_LIPSYNC(connector->video_latency[0]) | >>> + AUDIO_LIPSYNC(connector->audio_latency[0]); >>> + else >>> + tmp = VIDEO_LIPSYNC(255) | AUDIO_LIPSYNC(255); >>> + } >>> + WREG32(AZ_F0_CODEC_PIN0_CONTROL_RESPONSE_LIPSYNC, tmp); >> [...] >>> +#define AZ_F0_CODEC_PIN0_CONTROL_RESPONSE_LIPSYNC 0x5fe8 >>> +# define VIDEO_LIPSYNC(x) (((x) & 0xff) << >>> 0) >>> +# define AUDIO_LIPSYNC(x) (((x) & 0xff) << >>> 8) >>> +/* VIDEO_LIPSYNC, AUDIO_LIPSYNC >>> + * 0 = invalid >>> + * x = legal delay value >>> + * 255 = sync not supported >>> + */ >> >> Hmm, AMD_HDA_verbs_v2.pdf says that: >> 0 = unknown latency >> >> HDMI spec 1.4 says that: >> 0 = not valid or unknown latency >> 1..251 = valid delay value >> 255= video not supported / audio not supported >> >> Are you sure you shouldn't use 0 instead for unknown (no latency_present)? > > I'm not sure. The comment in the code above is what the register spec > says which seems to match the HDMI spec. I can dig around a bit more > internally. OK, though don't waste too much time on that, ALSA has to handle 0 and 255 the same in any case (a patch has just been pushed to sound git to handle 255). :) -- Anssi Hannula
[PATCH 1/2] drm/radeon/audio: write audio/video latency info for DCE4/5
18.10.2013 23:41, Alex Deucher kirjoitti: > Needed by the hda driver to properly set up synchronization > on the audio side. > > Signed-off-by: Alex Deucher > --- > drivers/gpu/drm/radeon/evergreen_hdmi.c | 37 > drivers/gpu/drm/radeon/evergreend.h | 38 > + > 2 files changed, 75 insertions(+) > > diff --git a/drivers/gpu/drm/radeon/evergreen_hdmi.c > b/drivers/gpu/drm/radeon/evergreen_hdmi.c > index 5fbe486..abdc893 100644 > --- a/drivers/gpu/drm/radeon/evergreen_hdmi.c > +++ b/drivers/gpu/drm/radeon/evergreen_hdmi.c [...] > + if (connector->latency_present[0]) > + tmp = VIDEO_LIPSYNC(connector->video_latency[0]) | > + AUDIO_LIPSYNC(connector->audio_latency[0]); > + else > + tmp = VIDEO_LIPSYNC(255) | AUDIO_LIPSYNC(255); > + } > + WREG32(AZ_F0_CODEC_PIN0_CONTROL_RESPONSE_LIPSYNC, tmp); [...] > +#define AZ_F0_CODEC_PIN0_CONTROL_RESPONSE_LIPSYNC 0x5fe8 > +# define VIDEO_LIPSYNC(x) (((x) & 0xff) << 0) > +# define AUDIO_LIPSYNC(x) (((x) & 0xff) << 8) > +/* VIDEO_LIPSYNC, AUDIO_LIPSYNC > + * 0 = invalid > + * x = legal delay value > + * 255 = sync not supported > + */ Hmm, AMD_HDA_verbs_v2.pdf says that: 0 = unknown latency HDMI spec 1.4 says that: 0 = not valid or unknown latency 1..251 = valid delay value 255= video not supported / audio not supported Are you sure you shouldn't use 0 instead for unknown (no latency_present)? Not sure this matters much, though, since the only consumer here is ALSA which we can write however we wish (and it is missing handling for 255 since it was missing from AMD_HDA_verbs_v2.pdf, but I'll add it in any case since it is mentioned in HDMI specs). [...] -- Anssi Hannula
[PATCH] drm/radeon/audio: fix missing multichannel PCM SAD in some cases
02.11.2013 03:01, Rafa? Mi?ecki kirjoitti: > 2013/10/29 Anssi Hannula : >> Fix the code to pick the PCM SAD with the highest number of channels, >> while merging the rate masks of PCM SADs with lower amount of channels >> into the additional stereo rate mask byte. > > Don't you think that we should use SUPPORTED_FREQUENCIES_STEREO for > stereo frequencies only? > > >> if (sad->format == >> HDMI_AUDIO_CODING_TYPE_PCM) >> + stereo_freqs |= sad->freq; > > I mean making that (... && sad->channels == 1) > SAD with channels=6,freqs=32..96kHz,bits=16..24 implies that those freqs and bps are supported for all channel counts up to 6 (since it is "Max Number of channels"). Therefore the specified rates are supported in stereo mode as well and I believe should be included in the stereo bitmask. As per AMD HDA Verbs documentation the 4th byte is "Rates supported for stereo". And since those rates _are_ supported by stereo, IMO they should be there. -- Anssi Hannula
[PATCH] drm/radeon/audio: fix missing multichannel PCM SAD in some cases
01.11.2013 01:38, Rafa? Mi?ecki kirjoitti: > 2013/10/29 Anssi Hannula : >> Because of this, only the 2-channel SAD may be used if it appears before >> the 8-channel SAD. Unless other SADs require otherwise, this may cause >> the ALSA HDA driver to allow stereo playback only. > > I can confirm that the problem exists. My SADs (Onkyo TX-SR605): > Format: 1 (PCM)Channels:1Freq:0x7F (32-192)B2:0x07 (16-24b) > Format: 1 (PCM)Channels:7Freq:0x7F (32-192)B2:0x07 (16-24b) > Format: 2 (AC3)Channels:7Freq:0x07 (32-48)B2:0x50 (640?) > Format: 7 (DTS)Channels:7Freq:0x06 (44-48)B2:0xC0 (1536?) > Format: 10 (EAC3)Channels:7Freq:0x06 (44-48)B2:0x00 > Format: 11 (DTS_HD)Channels:7Freq:0x7E (44-192)B2:0x01 > Format: 12 (MLP)Channels:7Freq:0x1E (44-96)B2:0x00 > > Unfortunately I get only 1 emulated SAD entry for PCM > (/proc/asound/card1/eld#0.0): > sad0_coding_type[0x1] LPCM > sad0_channels 2 > sad0_rates [0x1ee0] 32000 44100 48000 88200 96000 176400 192000 > sad0_bits [0xe] 16 20 24 > > So I can not play 8 channel LPCM "legally" (without forcing player to do so). > > >> Fix the code to pick the PCM SAD with the highest number of channels, >> while merging the rate masks of PCM SADs with lower amount of channels >> into the additional stereo rate mask byte. > > Does it mean that now instead of 2 real SADs: > 1) 192kHz support for 2 channels > 2) 96kHz support for 8 channels > you will only get: > 1) 192kHz support for 8 channels > ? No, the SADs will be separated again by the ATI/AMD ELD generator in hda_eld.c (as the 4th byte in the register contains the PCM stereo rate mask). I.e., assuming a receiver with 96kHz PCM multichannel limitation, with the patch the video-side sets the three standard SAD bytes according to the SAD with the highest number of channels (8ch+96kHz+24b), and the extra byte contains the or'ed rate mask of all PCM SADs (i.e. 192kHz). The audio-side then notices that 96kHz (in SAD) != 192kHz (in extra byte) and generates two SADs, one with 8ch 96kHz and one with 2ch 192kHz. > Does it mean alsa may try to play 8channels audio at 192kHz and fail at this? > That doesn't sound good, but on the other hand I can't propose > anything better :| Well, ALSA does not restrict that currently in any case (the ALSA HDMI code currently only takes the maximum channels/rate/etc into account, it doesn't check if the specific combination is allowed by SADs), but that is mostly just because nobody has added such code yet. > Out of curiosity, what does fglrx set in the verbs? I don't know, though I'd expect it to put the same values that get put there with this patch. >> Technically there are even more cases to handle (multiple non-PCM SADs >> of the same type, more than two PCM SADs with varying channel counts, >> etc), but those have not actually been encountered in the field and >> handling them would be non-trivial. >> >> Example affected EDID from Onkyo TX-SR674 specifying 192kHz stereo >> support and 96kHz 8-channel support (and other 8-channel compressed >> formats): > -- Anssi Hannula
[PATCH] drm/radeon/audio: fix missing multichannel PCM SAD in some cases
The current code writing SADs to the audio registers seems to assume
that there is at most a single SAD per audio format.
However, that is not the case. Especially for PCM it is somewhat common
for sinks to have two SADs, one for 8-channel and one for 2-channel
audio, which may have different supported sample rates (i.e. the sink
supports stereo audio at higher sample rates than multichannel audio).
Because of this, only the 2-channel SAD may be used if it appears before
the 8-channel SAD. Unless other SADs require otherwise, this may cause
the ALSA HDA driver to allow stereo playback only.
Fix the code to pick the PCM SAD with the highest number of channels,
while merging the rate masks of PCM SADs with lower amount of channels
into the additional stereo rate mask byte.
Technically there are even more cases to handle (multiple non-PCM SADs
of the same type, more than two PCM SADs with varying channel counts,
etc), but those have not actually been encountered in the field and
handling them would be non-trivial.
Example affected EDID from Onkyo TX-SR674 specifying 192kHz stereo
support and 96kHz 8-channel support (and other 8-channel compressed
formats):
00003dcb0101
010380780a0dc9a057479827
12484c0001010101010101010101
010101010101011d8018711c1620582c
2500c48e219e011d007251d01e20
6e285500c48e211e00fc0054
582d5352363734202020202000fd
00313d0f2e08000a202020202020019b
02032f724f8504030f0e07069413121e
1d1615012f097f070f1f071707503707
503f07c0834f66030c00808c
0ad08a20e02d10103e9600c48e21
18011d80d0721c1620102c2580c48e21
9e011d00bc52d01e20b8285540c4
8e211e8c0ad090204031200c4055
00c48e2118a8
Signed-off-by: Anssi Hannula
Tested-by: Andre Heider
Cc: Rafa? Mi?ecki
---
drivers/gpu/drm/radeon/dce6_afmt.c | 20 +++-
drivers/gpu/drm/radeon/evergreen_hdmi.c | 20 +++-
drivers/gpu/drm/radeon/r600_hdmi.c | 20 +++-
3 files changed, 45 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/radeon/dce6_afmt.c
b/drivers/gpu/drm/radeon/dce6_afmt.c
index 85a69d2..0a0fcee 100644
--- a/drivers/gpu/drm/radeon/dce6_afmt.c
+++ b/drivers/gpu/drm/radeon/dce6_afmt.c
@@ -198,20 +198,30 @@ void dce6_afmt_write_sad_regs(struct drm_encoder *encoder)
for (i = 0; i < ARRAY_SIZE(eld_reg_to_type); i++) {
u32 value = 0;
+ u8 stereo_freqs = 0;
+ int max_channels = -1;
int j;
for (j = 0; j < sad_count; j++) {
struct cea_sad *sad = [j];
if (sad->format == eld_reg_to_type[i][1]) {
- value = MAX_CHANNELS(sad->channels) |
- DESCRIPTOR_BYTE_2(sad->byte2) |
- SUPPORTED_FREQUENCIES(sad->freq);
+ if (sad->channels > max_channels) {
+ value = MAX_CHANNELS(sad->channels) |
+ DESCRIPTOR_BYTE_2(sad->byte2) |
+
SUPPORTED_FREQUENCIES(sad->freq);
+ max_channels = sad->channels;
+ }
+
if (sad->format == HDMI_AUDIO_CODING_TYPE_PCM)
- value |=
SUPPORTED_FREQUENCIES_STEREO(sad->freq);
- break;
+ stereo_freqs |= sad->freq;
+ else
+ break;
}
}
+
+ value |= SUPPORTED_FREQUENCIES_STEREO(stereo_freqs);
+
WREG32_ENDPOINT(offset, eld_reg_to_type[i][0], value);
}
diff --git a/drivers/gpu/drm/radeon/evergreen_hdmi.c
b/drivers/gpu/drm/radeon/evergreen_hdmi.c
index f71ce39..2a4837d 100644
--- a/drivers/gpu/drm/radeon/evergreen_hdmi.c
+++ b/drivers/gpu/drm/radeon/evergreen_hdmi.c
@@ -139,20 +139,30 @@ static void evergreen_hdmi_write_sad_regs(struct
drm_encoder *encoder)
for (i = 0; i < ARRAY_SIZE(eld_reg_to_type); i++) {
u32 value = 0;
+ u8 stereo_freqs = 0;
+ int max_channels = -1;
int j;
for (j = 0; j < sad_count; j++) {
struct cea_sad *sad = [j];
if (sad->format == eld_reg_to_type[i][1]) {
- value = MAX_CHANNELS(sad->channels) |
- DESCRIPTOR_BYTE_2(sad->byte2) |
- SUPPORTED_FREQUENCIES(sad->freq);
+ if (sad->channels > max_channels) {
+ value = MAX_CHANNELS(sad->channels) |
+
[PATCH] drm/radeon/audio: use actual pll clock for setting up dto
WREG32(DCCG_AUDIO_DTO_SELECT, 0); /* select DTO0 */
> } else {
> WREG32(DCCG_AUDIO_DTO1_PHASE, base_rate * 100);
> - WREG32(DCCG_AUDIO_DTO1_MODULE, clock * 100);
> + WREG32(DCCG_AUDIO_DTO1_MODULE, dto_modulo * 100);
> WREG32(DCCG_AUDIO_DTO_SELECT, 1); /* select DTO1 */
> }
> } else {
> /* according to the reg specs, this should be DCE2.0 and
> DCE3.0/3.1 */
> WREG32(AUDIO_DTO, AUDIO_DTO_PHASE(base_rate / 10) |
> -AUDIO_DTO_MODULE(clock / 10));
> +AUDIO_DTO_MODULE(dto_modulo / 10));
> }
> }
>
> @@ -420,7 +422,7 @@ void r600_hdmi_setmode(struct drm_encoder *encoder,
> struct drm_display_mode *mod
> return;
> offset = dig->afmt->offset;
>
> - r600_audio_set_dto(encoder, mode->clock);
> + r600_audio_set_dto(encoder);
>
> WREG32(HDMI0_VBI_PACKET_CONTROL + offset,
> HDMI0_NULL_SEND); /* send null packets when required */
> diff --git a/drivers/gpu/drm/radeon/radeon_mode.h
> b/drivers/gpu/drm/radeon/radeon_mode.h
> index 8b4e712..5b5339b 100644
> --- a/drivers/gpu/drm/radeon/radeon_mode.h
> +++ b/drivers/gpu/drm/radeon/radeon_mode.h
> @@ -341,6 +341,7 @@ struct radeon_crtc {
> u32 wm_low;
> u32 wm_high;
> struct drm_display_mode hw_mode;
> + u32 pll_clock; /* actual clock generated by the pll */
> };
>
> struct radeon_encoder_primary_dac {
>
--
Anssi Hannula
[PATCH 1/2] drm/radeon/audio: write audio/video latency info for DCE4/5
efine DISPLAY1_TYPE(x) (((x) & 0x3) << 8) > +# define DISPLAY1_ID(x) (((x) & 0x3f) << > 10) > +# define DISPLAY2_TYPE(x) (((x) & 0x3) << 16) > +# define DISPLAY2_ID(x) (((x) & 0x3f) << > 18) > +# define DISPLAY3_TYPE(x) (((x) & 0x3) << 24) > +# define DISPLAY3_ID(x) (((x) & 0x3f) << > 26) > +#define AZ_F0_CODEC_PIN0_CONTROL_RESPONSE_AV_ASSOCIATION1 0x5ff8 > +# define DISPLAY4_TYPE(x) (((x) & 0x3) << 0) > +# define DISPLAY4_ID(x) (((x) & 0x3f) << 2) > +# define DISPLAY5_TYPE(x) (((x) & 0x3) << 8) > +# define DISPLAY5_ID(x) (((x) & 0x3f) << > 10) > +#define AZ_F0_CODEC_PIN0_CONTROL_RESPONSE_AV_NUMBER 0x5ffc > +# define NUMBER_OF_DISPLAY_ID(x)(((x) & 0x7) << 0) > + > #define AZ_HOT_PLUG_CONTROL 0x5e78 > # define AZ_FORCE_CODEC_WAKE(1 << 0) > # define PIN0_JACK_DETECTION_ENABLE (1 << 4) > -- Anssi Hannula
[PATCH 4/4] drm/i915: make DBLCLK modes work
13.04.2012 22:31, Paulo Zanoni kirjoitti:
> From: Paulo Zanoni
>
> They require an AVI InfoFrame with a proper Pixel Repetition field.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=45729
> Signed-off-by: Paulo Zanoni
> ---
>
> I'm still waiting for confirmation on bugzilla, but I have access to a
> TV that reproduces the problem and the problem goes away with this patch
> series.
Shouldn't all this infoframe stuff be shared between the drivers (like
e.g. EDID stuff is)? I see i915, radeon, nouveau all have separate
implementations of it, and i915 seems to have it even twice (in
intel_hdmi.c and intel_sdvo.c).
The patch below only fixes the issue on one of those four places where
the avi/video infoframe is generated.
> drivers/gpu/drm/i915/intel_drv.h |2 ++
> drivers/gpu/drm/i915/intel_hdmi.c |8 ++--
> 2 files changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h
> index 175cca7..3afa7ab 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -215,6 +215,8 @@ struct intel_plane {
> #define DIP_TYPE_AVI0x82
> #define DIP_VERSION_AVI 0x2
> #define DIP_LEN_AVI 13
> +#define DIP_AVI_PR_10
> +#define DIP_AVI_PR_21
>
> #define DIP_TYPE_SPD 0x83
> #define DIP_VERSION_SPD 0x1
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c
> b/drivers/gpu/drm/i915/intel_hdmi.c
> index 7de2d3b..8d25017 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -220,7 +220,8 @@ static void intel_set_infoframe(struct drm_encoder
> *encoder,
> intel_hdmi->write_infoframe(encoder, frame);
> }
>
> -static void intel_hdmi_set_avi_infoframe(struct drm_encoder *encoder)
> +static void intel_hdmi_set_avi_infoframe(struct drm_encoder *encoder,
> + struct drm_display_mode *adjusted_mode)
> {
> struct dip_infoframe avi_if = {
> .type = DIP_TYPE_AVI,
> @@ -228,6 +229,9 @@ static void intel_hdmi_set_avi_infoframe(struct
> drm_encoder *encoder)
> .len = DIP_LEN_AVI,
> };
>
> + if (adjusted_mode->flags & DRM_MODE_FLAG_DBLCLK)
> + avi_if.body.avi.YQ_CN_PR |= DIP_AVI_PR_2;
> +
> intel_set_infoframe(encoder, _if);
> }
>
> @@ -290,7 +294,7 @@ static void intel_hdmi_mode_set(struct drm_encoder
> *encoder,
> I915_WRITE(intel_hdmi->sdvox_reg, sdvox);
> POSTING_READ(intel_hdmi->sdvox_reg);
>
> - intel_hdmi_set_avi_infoframe(encoder);
> + intel_hdmi_set_avi_infoframe(encoder, adjusted_mode);
> intel_hdmi_set_spd_infoframe(encoder);
> }
>
--
Anssi Hannula
Re: [PATCH 4/4] drm/i915: make DBLCLK modes work
13.04.2012 22:31, Paulo Zanoni kirjoitti:
From: Paulo Zanoni paulo.r.zan...@intel.com
They require an AVI InfoFrame with a proper Pixel Repetition field.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=45729
Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com
---
I'm still waiting for confirmation on bugzilla, but I have access to a
TV that reproduces the problem and the problem goes away with this patch
series.
Shouldn't all this infoframe stuff be shared between the drivers (like
e.g. EDID stuff is)? I see i915, radeon, nouveau all have separate
implementations of it, and i915 seems to have it even twice (in
intel_hdmi.c and intel_sdvo.c).
The patch below only fixes the issue on one of those four places where
the avi/video infoframe is generated.
drivers/gpu/drm/i915/intel_drv.h |2 ++
drivers/gpu/drm/i915/intel_hdmi.c |8 ++--
2 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_drv.h
b/drivers/gpu/drm/i915/intel_drv.h
index 175cca7..3afa7ab 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -215,6 +215,8 @@ struct intel_plane {
#define DIP_TYPE_AVI0x82
#define DIP_VERSION_AVI 0x2
#define DIP_LEN_AVI 13
+#define DIP_AVI_PR_10
+#define DIP_AVI_PR_21
#define DIP_TYPE_SPD 0x83
#define DIP_VERSION_SPD 0x1
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c
b/drivers/gpu/drm/i915/intel_hdmi.c
index 7de2d3b..8d25017 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -220,7 +220,8 @@ static void intel_set_infoframe(struct drm_encoder
*encoder,
intel_hdmi-write_infoframe(encoder, frame);
}
-static void intel_hdmi_set_avi_infoframe(struct drm_encoder *encoder)
+static void intel_hdmi_set_avi_infoframe(struct drm_encoder *encoder,
+ struct drm_display_mode *adjusted_mode)
{
struct dip_infoframe avi_if = {
.type = DIP_TYPE_AVI,
@@ -228,6 +229,9 @@ static void intel_hdmi_set_avi_infoframe(struct
drm_encoder *encoder)
.len = DIP_LEN_AVI,
};
+ if (adjusted_mode-flags DRM_MODE_FLAG_DBLCLK)
+ avi_if.body.avi.YQ_CN_PR |= DIP_AVI_PR_2;
+
intel_set_infoframe(encoder, avi_if);
}
@@ -290,7 +294,7 @@ static void intel_hdmi_mode_set(struct drm_encoder
*encoder,
I915_WRITE(intel_hdmi-sdvox_reg, sdvox);
POSTING_READ(intel_hdmi-sdvox_reg);
- intel_hdmi_set_avi_infoframe(encoder);
+ intel_hdmi_set_avi_infoframe(encoder, adjusted_mode);
intel_hdmi_set_spd_infoframe(encoder);
}
--
Anssi Hannula
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
