* Re: [PATCH v0] ASoC: rsnd: core: Check convert rate in rsnd_hw_params
2020-12-02 11:08 [PATCH v0] ASoC: rsnd: core: Check convert rate in rsnd_hw_params mdurnev
@ 2020-12-02 13:48 ` kernel test robot
2020-12-02 17:16 ` kernel test robot
2020-12-02 22:57 ` Kuninori Morimoto
2 siblings, 0 replies; 5+ messages in thread
From: kernel test robot @ 2020-12-02 13:48 UTC (permalink / raw)
To: mdurnev, kuninori.morimoto.gx, alsa-devel, linux-kernel
Cc: kbuild-all, mikhail_durnev
[-- Attachment #1: Type: text/plain, Size: 6989 bytes --]
Hi,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on asoc/for-next]
[also build test WARNING on v5.10-rc6 next-20201201]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/mdurnev-gmail-com/ASoC-rsnd-core-Check-convert-rate-in-rsnd_hw_params/20201202-191131
base: https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
config: riscv-allyesconfig (attached as .config)
compiler: riscv64-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/2ffb6c8ec578fd78d5962f7bc7c38eeb5c6a4bd1
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review mdurnev-gmail-com/ASoC-rsnd-core-Check-convert-rate-in-rsnd_hw_params/20201202-191131
git checkout 2ffb6c8ec578fd78d5962f7bc7c38eeb5c6a4bd1
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=riscv
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
sound/soc/sh/rcar/core.c: In function 'rsnd_hw_params':
>> sound/soc/sh/rcar/core.c:1442:4: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
1442 | int k_up = 6;
| ^~~
sound/soc/sh/rcar/core.c:1447:56: error: 'fe_channel' undeclared (first use in this function); did you mean 'channel'?
1447 | channel = io->converted_chan ? io->converted_chan : fe_channel;
| ^~~~~~~~~~
| channel
sound/soc/sh/rcar/core.c:1447:56: note: each undeclared identifier is reported only once for each function it appears in
>> sound/soc/sh/rcar/core.c:1460:8: warning: this statement may fall through [-Wimplicit-fallthrough=]
1460 | if (channel > 4) {
| ^
sound/soc/sh/rcar/core.c:1464:4: note: here
1464 | case 0:
| ^~~~
vim +1442 sound/soc/sh/rcar/core.c
1391
1392 /*
1393 * pcm ops
1394 */
1395 static int rsnd_hw_params(struct snd_soc_component *component,
1396 struct snd_pcm_substream *substream,
1397 struct snd_pcm_hw_params *hw_params)
1398 {
1399 struct snd_soc_dai *dai = rsnd_substream_to_dai(substream);
1400 struct rsnd_dai *rdai = rsnd_dai_to_rdai(dai);
1401 struct rsnd_dai_stream *io = rsnd_rdai_to_io(rdai, substream);
1402 struct snd_soc_pcm_runtime *fe = asoc_substream_to_rtd(substream);
1403
1404 /*
1405 * rsnd assumes that it might be used under DPCM if user want to use
1406 * channel / rate convert. Then, rsnd should be FE.
1407 * And then, this function will be called *after* BE settings.
1408 * this means, each BE already has fixuped hw_params.
1409 * see
1410 * dpcm_fe_dai_hw_params()
1411 * dpcm_be_dai_hw_params()
1412 */
1413 io->converted_rate = 0;
1414 io->converted_chan = 0;
1415 if (fe->dai_link->dynamic) {
1416 struct rsnd_priv *priv = rsnd_io_to_priv(io);
1417 struct device *dev = rsnd_priv_to_dev(priv);
1418 struct snd_soc_dpcm *dpcm;
1419 struct snd_pcm_hw_params *be_params;
1420 int stream = substream->stream;
1421
1422 for_each_dpcm_be(fe, stream, dpcm) {
1423 be_params = &dpcm->hw_params;
1424 if (params_channels(hw_params) != params_channels(be_params))
1425 io->converted_chan = params_channels(be_params);
1426 if (params_rate(hw_params) != params_rate(be_params))
1427 io->converted_rate = params_rate(be_params);
1428 }
1429 if (io->converted_chan)
1430 dev_dbg(dev, "convert channels = %d\n", io->converted_chan);
1431 if (io->converted_rate) {
1432 dev_dbg(dev, "convert rate = %d\n", io->converted_rate);
1433
1434 /*
1435 * SRC supports convert rates from params_rate(hw_params)/k_down
1436 * to params_rate(hw_params)*k_up, where k_up is always 6, and
1437 * k_down depends on number of channels and SRC unit.
1438 * So all SRC units can upsample audio up to 6 times regardless
1439 * its number of channels. And all SRC units can downsample
1440 * 2 channel audio up to 6 times too.
1441 */
> 1442 int k_up = 6;
1443 int k_down = 6;
1444 int channel;
1445 struct rsnd_mod *src_mod = rsnd_io_to_mod_src(io);
1446
1447 channel = io->converted_chan ? io->converted_chan : fe_channel;
1448
1449 switch (rsnd_mod_id(src_mod)) {
1450 /*
1451 * SRC0 can downsample 4, 6 and 8 channel audio up to 4 times.
1452 * SRC1, SRC3 and SRC4 can downsample 4 channel audio
1453 * up to 4 times.
1454 * SRC1, SRC3 and SRC4 can downsample 6 and 8 channel audio
1455 * no more than twice.
1456 */
1457 case 1:
1458 case 3:
1459 case 4:
> 1460 if (channel > 4) {
1461 k_down = 2;
1462 break;
1463 }
1464 case 0:
1465 if (channel > 2)
1466 k_down = 4;
1467 break;
1468
1469 /* Other SRC units do not support more than 2 channels */
1470 default:
1471 if (channel > 2)
1472 return -EINVAL;
1473 }
1474
1475 if (params_rate(hw_params) > io->converted_rate * k_down) {
1476 hw_param_interval(hw_params, SNDRV_PCM_HW_PARAM_RATE)->min =
1477 io->converted_rate * k_down;
1478 hw_param_interval(hw_params, SNDRV_PCM_HW_PARAM_RATE)->max =
1479 io->converted_rate * k_down;
1480 hw_params->cmask |= SNDRV_PCM_HW_PARAM_RATE;
1481 } else if (params_rate(hw_params) * k_up < io->converted_rate) {
1482 hw_param_interval(hw_params, SNDRV_PCM_HW_PARAM_RATE)->min =
1483 (io->converted_rate + k_up - 1) / k_up;
1484 hw_param_interval(hw_params, SNDRV_PCM_HW_PARAM_RATE)->max =
1485 (io->converted_rate + k_up - 1) / k_up;
1486 hw_params->cmask |= SNDRV_PCM_HW_PARAM_RATE;
1487 }
1488
1489 /*
1490 * TBD: Max SRC input and output rates also depend on number
1491 * of channels and SRC unit:
1492 * SRC1, SRC3 and SRC4 do not support more than 128kHz
1493 * for 6 channel and 96kHz for 8 channel audio.
1494 * Perhaps this function should return EINVAL if the input or
1495 * the output rate exceeds the limitation.
1496 */
1497 }
1498 }
1499
1500 return rsnd_dai_call(hw_params, io, substream, hw_params);
1501 }
1502
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 67754 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v0] ASoC: rsnd: core: Check convert rate in rsnd_hw_params
2020-12-02 11:08 [PATCH v0] ASoC: rsnd: core: Check convert rate in rsnd_hw_params mdurnev
2020-12-02 13:48 ` kernel test robot
@ 2020-12-02 17:16 ` kernel test robot
2020-12-02 22:57 ` Kuninori Morimoto
2 siblings, 0 replies; 5+ messages in thread
From: kernel test robot @ 2020-12-02 17:16 UTC (permalink / raw)
To: mdurnev, kuninori.morimoto.gx, alsa-devel, linux-kernel
Cc: kbuild-all, clang-built-linux, mikhail_durnev
[-- Attachment #1: Type: text/plain, Size: 6984 bytes --]
Hi,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on asoc/for-next]
[also build test ERROR on v5.10-rc6 next-20201201]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/mdurnev-gmail-com/ASoC-rsnd-core-Check-convert-rate-in-rsnd_hw_params/20201202-191131
base: https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
config: arm64-randconfig-r012-20201202 (attached as .config)
compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project 2671fccf0381769276ca8246ec0499adcb9b0355)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install arm64 cross compiling tool for clang build
# apt-get install binutils-aarch64-linux-gnu
# https://github.com/0day-ci/linux/commit/2ffb6c8ec578fd78d5962f7bc7c38eeb5c6a4bd1
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review mdurnev-gmail-com/ASoC-rsnd-core-Check-convert-rate-in-rsnd_hw_params/20201202-191131
git checkout 2ffb6c8ec578fd78d5962f7bc7c38eeb5c6a4bd1
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=arm64
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All error/warnings (new ones prefixed by >>):
>> sound/soc/sh/rcar/core.c:1447:56: error: use of undeclared identifier 'fe_channel'; did you mean 'channel'?
channel = io->converted_chan ? io->converted_chan : fe_channel;
^~~~~~~~~~
channel
sound/soc/sh/rcar/core.c:1444:8: note: 'channel' declared here
int channel;
^
>> sound/soc/sh/rcar/core.c:1442:8: warning: ISO C90 forbids mixing declarations and code [-Wdeclaration-after-statement]
int k_up = 6;
^
1 warning and 1 error generated.
vim +1447 sound/soc/sh/rcar/core.c
1391
1392 /*
1393 * pcm ops
1394 */
1395 static int rsnd_hw_params(struct snd_soc_component *component,
1396 struct snd_pcm_substream *substream,
1397 struct snd_pcm_hw_params *hw_params)
1398 {
1399 struct snd_soc_dai *dai = rsnd_substream_to_dai(substream);
1400 struct rsnd_dai *rdai = rsnd_dai_to_rdai(dai);
1401 struct rsnd_dai_stream *io = rsnd_rdai_to_io(rdai, substream);
1402 struct snd_soc_pcm_runtime *fe = asoc_substream_to_rtd(substream);
1403
1404 /*
1405 * rsnd assumes that it might be used under DPCM if user want to use
1406 * channel / rate convert. Then, rsnd should be FE.
1407 * And then, this function will be called *after* BE settings.
1408 * this means, each BE already has fixuped hw_params.
1409 * see
1410 * dpcm_fe_dai_hw_params()
1411 * dpcm_be_dai_hw_params()
1412 */
1413 io->converted_rate = 0;
1414 io->converted_chan = 0;
1415 if (fe->dai_link->dynamic) {
1416 struct rsnd_priv *priv = rsnd_io_to_priv(io);
1417 struct device *dev = rsnd_priv_to_dev(priv);
1418 struct snd_soc_dpcm *dpcm;
1419 struct snd_pcm_hw_params *be_params;
1420 int stream = substream->stream;
1421
1422 for_each_dpcm_be(fe, stream, dpcm) {
1423 be_params = &dpcm->hw_params;
1424 if (params_channels(hw_params) != params_channels(be_params))
1425 io->converted_chan = params_channels(be_params);
1426 if (params_rate(hw_params) != params_rate(be_params))
1427 io->converted_rate = params_rate(be_params);
1428 }
1429 if (io->converted_chan)
1430 dev_dbg(dev, "convert channels = %d\n", io->converted_chan);
1431 if (io->converted_rate) {
1432 dev_dbg(dev, "convert rate = %d\n", io->converted_rate);
1433
1434 /*
1435 * SRC supports convert rates from params_rate(hw_params)/k_down
1436 * to params_rate(hw_params)*k_up, where k_up is always 6, and
1437 * k_down depends on number of channels and SRC unit.
1438 * So all SRC units can upsample audio up to 6 times regardless
1439 * its number of channels. And all SRC units can downsample
1440 * 2 channel audio up to 6 times too.
1441 */
> 1442 int k_up = 6;
1443 int k_down = 6;
1444 int channel;
1445 struct rsnd_mod *src_mod = rsnd_io_to_mod_src(io);
1446
> 1447 channel = io->converted_chan ? io->converted_chan : fe_channel;
1448
1449 switch (rsnd_mod_id(src_mod)) {
1450 /*
1451 * SRC0 can downsample 4, 6 and 8 channel audio up to 4 times.
1452 * SRC1, SRC3 and SRC4 can downsample 4 channel audio
1453 * up to 4 times.
1454 * SRC1, SRC3 and SRC4 can downsample 6 and 8 channel audio
1455 * no more than twice.
1456 */
1457 case 1:
1458 case 3:
1459 case 4:
1460 if (channel > 4) {
1461 k_down = 2;
1462 break;
1463 }
1464 case 0:
1465 if (channel > 2)
1466 k_down = 4;
1467 break;
1468
1469 /* Other SRC units do not support more than 2 channels */
1470 default:
1471 if (channel > 2)
1472 return -EINVAL;
1473 }
1474
1475 if (params_rate(hw_params) > io->converted_rate * k_down) {
1476 hw_param_interval(hw_params, SNDRV_PCM_HW_PARAM_RATE)->min =
1477 io->converted_rate * k_down;
1478 hw_param_interval(hw_params, SNDRV_PCM_HW_PARAM_RATE)->max =
1479 io->converted_rate * k_down;
1480 hw_params->cmask |= SNDRV_PCM_HW_PARAM_RATE;
1481 } else if (params_rate(hw_params) * k_up < io->converted_rate) {
1482 hw_param_interval(hw_params, SNDRV_PCM_HW_PARAM_RATE)->min =
1483 (io->converted_rate + k_up - 1) / k_up;
1484 hw_param_interval(hw_params, SNDRV_PCM_HW_PARAM_RATE)->max =
1485 (io->converted_rate + k_up - 1) / k_up;
1486 hw_params->cmask |= SNDRV_PCM_HW_PARAM_RATE;
1487 }
1488
1489 /*
1490 * TBD: Max SRC input and output rates also depend on number
1491 * of channels and SRC unit:
1492 * SRC1, SRC3 and SRC4 do not support more than 128kHz
1493 * for 6 channel and 96kHz for 8 channel audio.
1494 * Perhaps this function should return EINVAL if the input or
1495 * the output rate exceeds the limitation.
1496 */
1497 }
1498 }
1499
1500 return rsnd_dai_call(hw_params, io, substream, hw_params);
1501 }
1502
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 45550 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v0] ASoC: rsnd: core: Check convert rate in rsnd_hw_params
2020-12-02 11:08 [PATCH v0] ASoC: rsnd: core: Check convert rate in rsnd_hw_params mdurnev
2020-12-02 13:48 ` kernel test robot
2020-12-02 17:16 ` kernel test robot
@ 2020-12-02 22:57 ` Kuninori Morimoto
2020-12-03 6:46 ` Mikhail Durnev
2 siblings, 1 reply; 5+ messages in thread
From: Kuninori Morimoto @ 2020-12-02 22:57 UTC (permalink / raw)
To: mdurnev; +Cc: alsa-devel, linux-kernel, mikhail_durnev
Hi Mikhail
Thank you for your patch
> + switch (rsnd_mod_id(src_mod)) {
> + /*
> + * SRC0 can downsample 4, 6 and 8 channel audio up to 4 times.
> + * SRC1, SRC3 and SRC4 can downsample 4 channel audio
> + * up to 4 times.
> + * SRC1, SRC3 and SRC4 can downsample 6 and 8 channel audio
> + * no more than twice.
> + */
> + case 1:
> + case 3:
> + case 4:
> + if (channel > 4) {
> + k_down = 2;
> + break;
> + }
> + case 0:
> + if (channel > 2)
> + k_down = 4;
> + break;
> +
> + /* Other SRC units do not support more than 2 channels */
> + default:
> + if (channel > 2)
> + return -EINVAL;
> + }
I think you want to add "fallthrough" between case 1/3/4 and case 0 ?
Thank you for your help !!
Best regards
---
Kuninori Morimoto
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v0] ASoC: rsnd: core: Check convert rate in rsnd_hw_params
2020-12-02 22:57 ` Kuninori Morimoto
@ 2020-12-03 6:46 ` Mikhail Durnev
0 siblings, 0 replies; 5+ messages in thread
From: Mikhail Durnev @ 2020-12-03 6:46 UTC (permalink / raw)
To: Kuninori Morimoto; +Cc: alsa-devel, linux-kernel
On 03.12.2020 08:57, Kuninori Morimoto wrote:
> I think you want to add "fallthrough" between case 1/3/4 and case 0 ?
Hi Morimoto-san,
Yes, I will add it in the updated version of the patch. Thanks.
The checkpatch script did not tell me about that.
Best regards,
Misha
^ permalink raw reply [flat|nested] 5+ messages in thread