[PATCH v3 1/1] ALSA: hda: Refactor calculating SDnFMT according to specification

[Takashi Iwai]

On Tue, 22 Sep 2020 17:33:32 +0200,
Pawel Harlozinski wrote:
> 
> Fix setting SDnFMT based on :wq
> High Definition Audio Specification Rev. 1.0a page 48.
> 
> Bits per Sample (BITS):
> 000 = 8 bits. The data will be packed in memory in 8-bit containers on 16-bit boundaries.
> 001 = 16 bits. The data will be packed in memory in 16-bit containers on 16-bit boundaries.
> 010 = 20 bits. The data will be packed in memory in 32-bit containers on 32-bit boundaries.
> 011 = 24 bits. The data will be packed in memory in 32-bit containers on 32-bit boundaries.
> 100 = 32 bits. The data will be packed in memory in 32-bit containers on 32-bit boundaries.
> 101-111 = Reserved
> 
> Set SDnFMT depending on which format was given.
> Henceforth split cases for formats 20, 24, 32 bits,
> but leave constraints to maxbps.
> 
> Signed-off-by: Pawel Harlozinski <pawel.harlozinski at linux.intel.com>

As I repeatedly wrote, the bits 20 and 24 cases never hit practically,
hence this "refactoring" doesn't give any change in reality.  Your
commit misses that point.

That said, this can be taken as a complementary for the theoretical
case, but the situation has to be clarified in the commit log, and the
subject should be adjusted as well.


thanks,

Takashi

> 
> ---
> 
>  v3: drop gerrit Change-Id
>  v2: leave constraints to maxbps
>  
>  sound/hda/hdac_device.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/sound/hda/hdac_device.c b/sound/hda/hdac_device.c
> index 3e9e9ac804f6..ccc47a10ba63 100644
> --- a/sound/hda/hdac_device.c
> +++ b/sound/hda/hdac_device.c
> @@ -764,7 +764,14 @@ unsigned int snd_hdac_calc_stream_format(unsigned int rate,
>  		val |= AC_FMT_BITS_16;
>  		break;
>  	case 20:
> +		val |= AC_FMT_BITS_20;
> +		break;
>  	case 24:
> +		if (maxbps >= 24)
> +			val |= AC_FMT_BITS_24;
> +		else
> +			val |= AC_FMT_BITS_20;
> +		break;
>  	case 32:
>  		if (maxbps >= 32 || format == SNDRV_PCM_FORMAT_FLOAT_LE)
>  			val |= AC_FMT_BITS_32;
> -- 
> 2.17.1
>