在 2025/1/15 19:47, Jiebing Chen 写道:

在 2025/1/15 18:36, Jiebing Chen 写道:

quoted

在 2025/1/15 16:36, Jerome Brunet 写道:

quoted

[ EXTERNAL EMAIL ]

On Wed 15 Jan 2025 at 10:56, Jiebing Chen [off-list ref] 
wrote:

quoted

在 2025/1/14 22:05, Jerome Brunet 写道:

quoted

[ EXTERNAL EMAIL ]

On Tue 14 Jan 2025 at 19:20, Jiebing Chen 
[off-list ref] wrote:

quoted

quoted

quoted

quoted

+
+MODULE_DESCRIPTION("Amlogic to codec driver");
+MODULE_AUTHOR("jiebing.chen@amlogic.com");
+MODULE_LICENSE("GPL");

diff --git a/sound/soc/meson/t9015.c b/sound/soc/meson/t9015.c

index
571f65788c592050abdca264f5656d4d1a9d99f6..2db1cd18cf2cea507f3d7282054e03d953586648 

100644

--- a/sound/soc/meson/t9015.c
+++ b/sound/soc/meson/t9015.c

@@ -89,10 +89,7 @@ static struct snd_soc_dai_driver t9015_dai = {

                 .channels_min = 1,
                 .channels_max = 2,
                 .rates = SNDRV_PCM_RATE_8000_96000,
-             .formats = (SNDRV_PCM_FMTBIT_S8 |
-                         SNDRV_PCM_FMTBIT_S16_LE |
-                         SNDRV_PCM_FMTBIT_S20_LE |
- SNDRV_PCM_FMTBIT_S24_LE),
+             .formats = (SNDRV_PCM_FMTBIT_S16_LE |
SNDRV_PCM_FMTBIT_S32_LE),

Again, mixed up changes with zero justification.

This drops S8 and S16 format support for the existing SoCs 
(such as GXL)
which is known to work and add S32 support on an HW documented 
as 24bits
only. Can you explain ?

for g12a, sm1 etc, it is use new audio ip, GXL is old ip,

If there are chips difference we did not know about, then you should
introduce those difference, without breaking existing support -
including for GXL, which is what you did IIUC.

quoted

the new ip not support 24 bit,

Are sure about that ? that code has been there for a while.

If sm1 does not support SNDRV_PCM_FMTBIT_S24_LE, you should a fix 
up patch for
that, with the proper "Fixes:" tag, how to reproduce the problem and
explaining the fix.

At first I thought you said 24bit is s24_3le, becasue of the fddr can't 
get it,

so  spend a lot of time trying to explain it

I think it may be because of this difference that we are not focused on 
the same problem

yes, we support s24_le,  the internal codec also process it 24 bit of 32 
bit,

if we send valid 32bit data, it only process 24bit, discard 8 bit data, 
if this behavior should be prohibit

we will follow this rule

quoted

quoted

quoted

maybe there are some gap , we support SNDRV_PCM_FMTBIT_S24, not 
support the

SNDRV_PCM_FMTBIT_S24_3LE,  for SNDRV_PCM_FMTBIT_S24

it is  Signed, 24-bit (32-bit in memory), little endian , the audio 
dma
busrt is 64bit

It makes absolutely no sense to discuss memory layout for the codec.

quoted

it can get the full data. we send the 32 bit data  mclk = 32bit* 
48k *4,
use the clk to send

the  SNDRV_PCM_FMTBIT_S24,   the hadware always send the 32bit data

No it does not. It send 24 bits of data over a 32 bits physical word 
with
8 bits ignored.

The original intention, we play 32bit data from tdmout, it play 
error, so we add the s32_le

for tdmouta ,it can bind Multiple codec, one codec is the internal 
codec,

other is external codec, tdmout can send the data to internal codec 
and external codec from the output pad

for example, tdmout send 4 ch, 2 ch is send the internal codec, 2 ch 
send the data pad

it aplay error, due to the internal codec fomat parameter limiting 
condition

There is a contradiction here,  Considering our internal codec can 
process this it, drop 8bit, 24bit valid

therefore software can set s24_le/s32_le, still  work ok for 
hardware, so Multiple ch can support for internal codec and external 
codec

add the s32_le format for the t9015.c,  it can resovle Multiple codec 
s32_le format case,  although hardware only process 24bit of 32bit, 
but it can't affect the acodec hardware work, it still work fine

usually we add the s32_le format support, if not allow to do it, think 
of it as a limitation, we can  remove 32bit test for it

quoted

quoted

quoted

so, i think we only add the SNDRV_PCM_FMTBIT_S32 base on it

That's wrong if the codec does not actually use the full 32bits ... and
I have clear indication that's what the codec is doing, on GXL at 
least.

quoted

we think the 24 bit is the SNDRV_PCM_FMTBIT_S24_3LE, it is 24bit in 
memroy,

due to the dma busrt 64 bit limit, it can't align the sample bit, 
if it is
24 bit

Again, memory layout makes no sense here.

quoted

so the clock configure can't 24bit clock,

I disagree and this has been tested. If you have a test case showing
otherwise please share it.

quoted

by the way, We discuss internally for gxl,

it also support the SNDRV_PCM_FMTBIT_S32

Does it really ? If it is just to ignore the 8bits LSB, that not a 
support.

quoted

quoted

quoted

usually support 16/32 bit for new audio ip , for 
SNDRV_PCM_FMTBIT_S24_LE,
it width =24, phy =32

Yes physical of SNDRV_PCM_FMTBIT_S24_LE, so most chip supporting 
32 bits
width would support this S24_LE, unless there is something odd.

quoted

it was  treated as 32 bit to send for tdm, so we can only add the 
S32LE
base on it , right ?

You are asking me ? How am I suppose to know ?

quoted

but if the gxl not support the 32bit

I don't see a problem with a DAC taking input on 32bits physical
interface and ignoring some bit on processing.

If that's not the case, please send a proper fix change with some 
explanation

quoted

we need add new snd_soc_dai_driver t9015_dai_s4 ?

If I understood correctly format depends on the chip and needs to
adjusted including for sm1.

quoted

quoted

quoted

quoted

         },
         .ops = &t9015_dai_ops,
    };

-- Jerome

-- 
Jerome

-- 
Jerome