2022-09-12 08:03:35

by Takashi Iwai

[permalink] [raw]
Subject: Re: [PATCH v1] ALSA: dummy: Add customizable volume min/max.

On Mon, 12 Sep 2022 09:29:45 +0200,
YJ Lee wrote:
>
> Add module parameters to support customized min/max volume leveling,
> which will be useful to test devices with different volume granularity.
>
> Signed-off-by: YJ Lee <[email protected]>
> ---
> sound/drivers/dummy.c | 34 ++++++++++++++++++++++++----------
> 1 file changed, 24 insertions(+), 10 deletions(-)
>
> diff --git a/sound/drivers/dummy.c b/sound/drivers/dummy.c
> index 2a7fc49c1a7c5..64fb2778f1e9a 100644
> --- a/sound/drivers/dummy.c
> +++ b/sound/drivers/dummy.c
> @@ -42,6 +42,8 @@ MODULE_LICENSE("GPL");
> #define USE_CHANNELS_MAX 2
> #define USE_PERIODS_MIN 1
> #define USE_PERIODS_MAX 1024
> +#define USE_MIXER_VOLUME_LEVEL_MIN -50
> +#define USE_MIXER_VOLUME_LEVEL_MAX 100
>
> static int index[SNDRV_CARDS] = SNDRV_DEFAULT_IDX; /* Index 0-MAX */
> static char *id[SNDRV_CARDS] = SNDRV_DEFAULT_STR; /* ID for this card */
> @@ -50,6 +52,8 @@ static char *model[SNDRV_CARDS] = {[0 ... (SNDRV_CARDS - 1)] = NULL};
> static int pcm_devs[SNDRV_CARDS] = {[0 ... (SNDRV_CARDS - 1)] = 1};
> static int pcm_substreams[SNDRV_CARDS] = {[0 ... (SNDRV_CARDS - 1)] = 8};
> //static int midi_devs[SNDRV_CARDS] = {[0 ... (SNDRV_CARDS - 1)] = 2};
> +static int mixer_volume_level_min = USE_MIXER_VOLUME_LEVEL_MIN;
> +static int mixer_volume_level_max = USE_MIXER_VOLUME_LEVEL_MAX;
> #ifdef CONFIG_HIGH_RES_TIMERS
> static bool hrtimer = 1;
> #endif
> @@ -69,6 +73,10 @@ module_param_array(pcm_substreams, int, NULL, 0444);
> MODULE_PARM_DESC(pcm_substreams, "PCM substreams # (1-128) for dummy driver.");
> //module_param_array(midi_devs, int, NULL, 0444);
> //MODULE_PARM_DESC(midi_devs, "MIDI devices # (0-2) for dummy driver.");
> +module_param(mixer_volume_level_min, int, 0444);

I can imagine that the permission could be 0644, so that the
parameters can be changed dynamically via sysfs, too. But it may skip
the sanity check at probe, hence more code would be needed, OTOH.

So I applied the patch as is now.


thanks,

Takashi


2022-09-12 08:52:03

by YJ Lee

[permalink] [raw]
Subject: Re: [PATCH v1] ALSA: dummy: Add customizable volume min/max.

On Mon, Sep 12, 2022 at 3:52 PM Takashi Iwai <[email protected]> wrote:
>
> On Mon, 12 Sep 2022 09:29:45 +0200,
> YJ Lee wrote:
> >
> > Add module parameters to support customized min/max volume leveling,
> > which will be useful to test devices with different volume granularity.
> >
> > Signed-off-by: YJ Lee <[email protected]>
> > ---
> > sound/drivers/dummy.c | 34 ++++++++++++++++++++++++----------
> > 1 file changed, 24 insertions(+), 10 deletions(-)
> >
> > diff --git a/sound/drivers/dummy.c b/sound/drivers/dummy.c
> > index 2a7fc49c1a7c5..64fb2778f1e9a 100644
> > --- a/sound/drivers/dummy.c
> > +++ b/sound/drivers/dummy.c
> > @@ -42,6 +42,8 @@ MODULE_LICENSE("GPL");
> > #define USE_CHANNELS_MAX 2
> > #define USE_PERIODS_MIN 1
> > #define USE_PERIODS_MAX 1024
> > +#define USE_MIXER_VOLUME_LEVEL_MIN -50
> > +#define USE_MIXER_VOLUME_LEVEL_MAX 100
> >
> > static int index[SNDRV_CARDS] = SNDRV_DEFAULT_IDX; /* Index 0-MAX */
> > static char *id[SNDRV_CARDS] = SNDRV_DEFAULT_STR; /* ID for this card */
> > @@ -50,6 +52,8 @@ static char *model[SNDRV_CARDS] = {[0 ... (SNDRV_CARDS - 1)] = NULL};
> > static int pcm_devs[SNDRV_CARDS] = {[0 ... (SNDRV_CARDS - 1)] = 1};
> > static int pcm_substreams[SNDRV_CARDS] = {[0 ... (SNDRV_CARDS - 1)] = 8};
> > //static int midi_devs[SNDRV_CARDS] = {[0 ... (SNDRV_CARDS - 1)] = 2};
> > +static int mixer_volume_level_min = USE_MIXER_VOLUME_LEVEL_MIN;
> > +static int mixer_volume_level_max = USE_MIXER_VOLUME_LEVEL_MAX;
> > #ifdef CONFIG_HIGH_RES_TIMERS
> > static bool hrtimer = 1;
> > #endif
> > @@ -69,6 +73,10 @@ module_param_array(pcm_substreams, int, NULL, 0444);
> > MODULE_PARM_DESC(pcm_substreams, "PCM substreams # (1-128) for dummy driver.");
> > //module_param_array(midi_devs, int, NULL, 0444);
> > //MODULE_PARM_DESC(midi_devs, "MIDI devices # (0-2) for dummy driver.");
> > +module_param(mixer_volume_level_min, int, 0444);
>
> I can imagine that the permission could be 0644, so that the
> parameters can be changed dynamically via sysfs, too. But it may skip
> the sanity check at probe, hence more code would be needed, OTOH.
>
> So I applied the patch as is now.
>
>
> thanks,
>
> Takashi

Thanks for the insight! Learned a lot from your kind explanation and
constant help.

Appreciated,
YJ