Repository navigation
topology2: intel: hdmi-generic: Update rates and format support in pc… - #9004
Conversation
| formats 'S32_LE,S16_LE' | ||
| formats 'S32_LE,S16_LE,IEC958_SUBFRAME_LE' | ||
| channels_max 8 | ||
| rate_min 32000 |
There was a problem hiding this comment.
IIRC 64kHz is not supported by HDaudio and HDMI, is it?
There was a problem hiding this comment.
yeah, it is not, but can we specify rates or mask rate out with tplg?
There was a problem hiding this comment.
patch_hmdi.c uses a explicit
#define SUPPORTED_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)
btw do we support 44.1kHz with the CHAIN_DMA? I would be surprised if it worked.
There was a problem hiding this comment.
I have tested locally: 44.1, 48, 96 and 192 32bit stereo.
There was a problem hiding this comment.
I wonder what the buffer sizes are and how we avoid fractional number of samples in the 44.1kHz case?
There was a problem hiding this comment.
I think this should work:
rate_min 0
rate_max 0
rates '32000,44100,48000,88200,96000,176400,192000'
Confirmed: it does
There was a problem hiding this comment.
I wonder what the buffer sizes are and how we avoid fractional number of samples in the 44.1kHz case?
I have other wip branch to sort out the buffer/period sizes.
The max is the max size allowed, if it is fraction then it is fraction, application does not need to use the whole buffer
81efb77 to
4b49fef
Compare
|
Changes since v1:
|
| formats 'S32_LE,S16_LE,IEC958_SUBFRAME_LE' | ||
| channels_max 8 | ||
| rate_min 0 | ||
| rate_max 0 |
There was a problem hiding this comment.
do we really need those set to zero? it's weird. @ranj063 any comments?
There was a problem hiding this comment.
I think this is the first and only use of a integer constraint with enumerated valid values, so there's not a lot of examples (in SOF or elsewhere in ALSA). The code to parse was added to alsa-utils by @ranj063 , so I'll let you confirm. I wonder if a comment would be nice to explain why this is needed. As this is the first example, this bit is likely to be copy'n'pasted to many places down the line....
There was a problem hiding this comment.
@kv2019i what is the error without these set to 0?
There was a problem hiding this comment.
@ranj063, I have not tested w/o these set to 0. It just felt natural to 'disable' the rate_min/max when setting the rates individually, let me test it.
There was a problem hiding this comment.
@ranj063, @plbossart, @kv2019i, the rate_min/max needs to be set to zero for the rates to be picked up. I only dropped it for HDMI1 (hw:0,3) and [rate_min/max is not set to 0]:
alsa_conformance_test -P hw:0,3 --dev_info_only
------DEVICE INFORMATION------
PCM handle name: hw:0,3
PCM type: HW
card: sofhdadsp [sof-hda-dsp]
device: HDMI1 (*) []
stream: PLAYBACK
available channels: 2 4 6 8
available formats: S16_LE S32_LE IEC958_SUBFRAME_LE
rate range: [48000, 48000]
available rates: 48000
period size range: [12, 4096]
buffer size range: [192, 16384]
vs the unmodified HDMI2 (hw:0,4) [rate_min/max is set to 0]:
alsa_conformance_test -P hw:0,4 --dev_info_only
------DEVICE INFORMATION------
PCM handle name: hw:0,4
PCM type: HW
card: sofhdadsp [sof-hda-dsp]
device: HDMI2 (*) []
stream: PLAYBACK
available channels: 2 4 6 8
available formats: S16_LE S32_LE IEC958_SUBFRAME_LE
rate range: [32000, 192000]
available rates: 32000 44100 48000 88200 96000 176400 192000
period size range: [8, 4096]
buffer size range: [128, 16384]
------------------------------
There was a problem hiding this comment.
Model is that base object/class will set the default value which can be inherited and overridden if needed. If not set by base class/object then its always 0.
There was a problem hiding this comment.
the problem is that the inheritance is irrelevant since we are moving from a max_rate/min_rate representation to a 'rates' representation.
There was a problem hiding this comment.
@plbossart the alternative we can use is to have only rates and drop rate_min/max. That way the default rates will be 48K and when needed can be overridden with other supported rates
There was a problem hiding this comment.
yes, if the default rate_min == rate_max == 48000 then we might as well use rates = 48000 :-)
| formats 'S32_LE,S16_LE,IEC958_SUBFRAME_LE' | ||
| channels_max 8 | ||
| rate_min 0 | ||
| rate_max 0 |
There was a problem hiding this comment.
I think this is the first and only use of a integer constraint with enumerated valid values, so there's not a lot of examples (in SOF or elsewhere in ALSA). The code to parse was added to alsa-utils by @ranj063 , so I'll let you confirm. I wonder if a comment would be nice to explain why this is needed. As this is the first example, this bit is likely to be copy'n'pasted to many places down the line....
@kv2019i I think the simplest solution would be what @ujfalusi has in this PR. The right way to do it would be remove th edefaults for rate_min/max from the class definition though but that would mean we'd need to fix the pcm_caps objects in every other topology that has them |
lgirdwood
left a comment
There was a problem hiding this comment.
If we need to set 0 today (to WA a CC issue) then lets at least comment it in the topology as rate min/max being 0 looks like all rates will fail (unless we expect other objects to override).
| formats 'S32_LE,S16_LE,IEC958_SUBFRAME_LE' | ||
| channels_max 8 | ||
| rate_min 0 | ||
| rate_max 0 |
There was a problem hiding this comment.
Model is that base object/class will set the default value which can be inherited and overridden if needed. If not set by base class/object then its always 0.
…m.caps With ChainDMA the HDMI PCM can support IEC958_SUBFRAME_LE and the rate is not limited to 48K only. To be able to use bytestream passthrough (DD/DTS/etc). Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
4b49fef to
badf31c
Compare
|
Changes since v2:
|
…m.caps
With ChainDMA the HDMI PCM can support IEC958_SUBFRAME_LE and the rate is not limited to 48K only.
To be able to use bytestream passthrough (DD/DTS/etc).