diff mbox series

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

Message ID 20200922153332.4919-1-pawel.harlozinski@linux.intel.com (mailing list archive)
State New, archived
Headers show
Series [v3,1/1] ALSA: hda: Refactor calculating SDnFMT according to specification | expand

Commit Message

Pawel Harlozinski Sept. 22, 2020, 3:33 p.m. UTC
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@linux.intel.com>

---

 v3: drop gerrit Change-Id
 v2: leave constraints to maxbps
 
 sound/hda/hdac_device.c | 7 +++++++
 1 file changed, 7 insertions(+)

Comments

Takashi Iwai Sept. 22, 2020, 3:58 p.m. UTC | #1
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@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
>
diff mbox series

Patch

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;