[PATCH] ALSA: Add rate defines for 352k8 and 384k

[PATCH] ALSA: Add rate defines for 352k8 and 384k

[DigitalDreamtime]

Add SNDRV_PCM_RATE_352800 and SNDRV_PCM_RATE_384000 defines to pcm.h,
 for 352k8 and 384k sample rates.

Add SNDRV_PCM_RATE_8000_384000 define to pcm.h.

Update rates list in pcm_native.c.

Yes, we can use CONTINUOUS/KNOT and constraints, to support the x8
 sample rates, but having the defines requires less code in drivers.
Many more DAC chips are now supporting the x8 44k1/48k multiples.
>From a high-res viewpoint, the DXD standard is 352k8, 24 bit.
That alone should justify the addition of a define for it.

Signed-off-by: DigitalDreamtime <clive.messer@digitaldreamtime.co.uk>
---
 include/sound/pcm.h     | 5 +++++
 sound/core/pcm_native.c | 5 +++--
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/include/sound/pcm.h b/include/sound/pcm.h
index af1fb37..86e0bbe 100644
--- a/include/sound/pcm.h
+++ b/include/sound/pcm.h
@@ -129,6 +129,8 @@ struct snd_pcm_ops {
 #define SNDRV_PCM_RATE_96000		(1<<10)		/* 96000Hz */
 #define SNDRV_PCM_RATE_176400		(1<<11)		/* 176400Hz */
 #define SNDRV_PCM_RATE_192000		(1<<12)		/* 192000Hz */
+#define SNDRV_PCM_RATE_352800		(1<<13)		/* 352800Hz */
+#define SNDRV_PCM_RATE_384000		(1<<14)		/* 384000Hz */
 
 #define SNDRV_PCM_RATE_CONTINUOUS	(1<<30)		/* continuous range */
 #define SNDRV_PCM_RATE_KNOT		(1<<31)		/* supports more non-continuos rates */
@@ -141,6 +143,9 @@ struct snd_pcm_ops {
 					 SNDRV_PCM_RATE_88200|SNDRV_PCM_RATE_96000)
 #define SNDRV_PCM_RATE_8000_192000	(SNDRV_PCM_RATE_8000_96000|SNDRV_PCM_RATE_176400|\
 					 SNDRV_PCM_RATE_192000)
+#define SNDRV_PCM_RATE_8000_384000	(SNDRV_PCM_RATE_8000_192000 |\
+					 SNDRV_PCM_RATE_352800 |\
+					 SNDRV_PCM_RATE_384000)
 #define _SNDRV_PCM_FMTBIT(fmt)		(1ULL << (__force int)SNDRV_PCM_FORMAT_##fmt)
 #define SNDRV_PCM_FMTBIT_S8		_SNDRV_PCM_FMTBIT(S8)
 #define SNDRV_PCM_FMTBIT_U8		_SNDRV_PCM_FMTBIT(U8)
diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
index c61fd50..21cc13d 100644
--- a/sound/core/pcm_native.c
+++ b/sound/core/pcm_native.c
@@ -1981,12 +1981,13 @@ static int snd_pcm_hw_rule_sample_bits(struct snd_pcm_hw_params *params,
 	return snd_interval_refine(hw_param_interval(params, rule->var), &t);
 }
 
-#if SNDRV_PCM_RATE_5512 != 1 << 0 || SNDRV_PCM_RATE_192000 != 1 << 12
+#if SNDRV_PCM_RATE_5512 != 1 << 0 || SNDRV_PCM_RATE_384000 != 1 << 14
 #error "Change this table"
 #endif
 
 static unsigned int rates[] = { 5512, 8000, 11025, 16000, 22050, 32000, 44100,
-                                 48000, 64000, 88200, 96000, 176400, 192000 };
+				 48000, 64000, 88200, 96000, 176400, 192000,
+				 352800, 384000 };
 
 const struct snd_pcm_hw_constraint_list snd_pcm_known_rates = {
 	.count = ARRAY_SIZE(rates),
-- 
2.5.5

Re: [PATCH] ALSA: Add rate defines for 352k8 and 384k

[Takashi Iwai]

On Mon, 06 Jun 2016 14:19:00 +0200,
DigitalDreamtime wrote:
> 
> Add SNDRV_PCM_RATE_352800 and SNDRV_PCM_RATE_384000 defines to pcm.h,
>  for 352k8 and 384k sample rates.
> 
> Add SNDRV_PCM_RATE_8000_384000 define to pcm.h.
> 
> Update rates list in pcm_native.c.
> 
> Yes, we can use CONTINUOUS/KNOT and constraints, to support the x8
>  sample rates, but having the defines requires less code in drivers.

Well, it'd be more convincing if you actually show the reduction of
the code after this patch in the current tree.  The number speaks more
than words.

> Many more DAC chips are now supporting the x8 44k1/48k multiples.
> From a high-res viewpoint, the DXD standard is 352k8, 24 bit.
> That alone should justify the addition of a define for it.
> 
> Signed-off-by: DigitalDreamtime <clive.messer@digitaldreamtime.co.uk>

A sign-off has to be the real name.  Please see "sign your work"
section in SubmittingPatches.


thanks,

Takashi

Re: [PATCH] ALSA: Add rate defines for 352k8 and 384k

[Clive Messer]

On Mon, 2016-06-06 at 14:59 +0200, Takashi Iwai wrote:
> Well, it'd be more convincing if you actually show the reduction of
> the code after this patch in the current tree.  The number speaks
> more than words.

pcm5102a codec driver with 352k8/384k defines patch...

https://github.com/DigitalDreamtimeLtd/linux/commit/83669837232018909e97623588de355fe072546b

versus, with KNOT and startup constraint...

https://github.com/DigitalDreamtimeLtd/linux/commit/06a68d757ff641b94aeb8b6385312923c5a86e18

Regards

Clive
-- 
Clive Messer <clive.messer@digitaldreamtime.co.uk>
Digital Dreamtime Ltd

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

[PATCH] ALSA: Add rate defines for 352k8 and 384k

[Clive Messer]

Add SNDRV_PCM_RATE_352800 and SNDRV_PCM_RATE_384000 defines to pcm.h,
 for 352k8 and 384k sample rates.

Add SNDRV_PCM_RATE_8000_384000 define to pcm.h.

Update rates list in pcm_native.c.

Yes, we can use CONTINUOUS/KNOT and constraints, to support the x8
 sample rates, but having the defines requires less code in drivers.
Many more DAC chips are now supporting the x8 44k1/48k multiples.
>From a high-res viewpoint, the DXD standard is 352k8, 24 bit.
That alone should justify the addition of a define for it.

Signed-off-by: Clive Messer <clive.messer@digitaldreamtime.co.uk>
---
 include/sound/pcm.h     | 5 +++++
 sound/core/pcm_native.c | 5 +++--
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/include/sound/pcm.h b/include/sound/pcm.h
index af1fb37..86e0bbe 100644
--- a/include/sound/pcm.h
+++ b/include/sound/pcm.h
@@ -129,6 +129,8 @@ struct snd_pcm_ops {
 #define SNDRV_PCM_RATE_96000		(1<<10)		/* 96000Hz */
 #define SNDRV_PCM_RATE_176400		(1<<11)		/* 176400Hz */
 #define SNDRV_PCM_RATE_192000		(1<<12)		/* 192000Hz */
+#define SNDRV_PCM_RATE_352800		(1<<13)		/* 352800Hz */
+#define SNDRV_PCM_RATE_384000		(1<<14)		/* 384000Hz */
 
 #define SNDRV_PCM_RATE_CONTINUOUS	(1<<30)		/* continuous range */
 #define SNDRV_PCM_RATE_KNOT		(1<<31)		/* supports more non-continuos rates */
@@ -141,6 +143,9 @@ struct snd_pcm_ops {
 					 SNDRV_PCM_RATE_88200|SNDRV_PCM_RATE_96000)
 #define SNDRV_PCM_RATE_8000_192000	(SNDRV_PCM_RATE_8000_96000|SNDRV_PCM_RATE_176400|\
 					 SNDRV_PCM_RATE_192000)
+#define SNDRV_PCM_RATE_8000_384000	(SNDRV_PCM_RATE_8000_192000 |\
+					 SNDRV_PCM_RATE_352800 |\
+					 SNDRV_PCM_RATE_384000)
 #define _SNDRV_PCM_FMTBIT(fmt)		(1ULL << (__force int)SNDRV_PCM_FORMAT_##fmt)
 #define SNDRV_PCM_FMTBIT_S8		_SNDRV_PCM_FMTBIT(S8)
 #define SNDRV_PCM_FMTBIT_U8		_SNDRV_PCM_FMTBIT(U8)
diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
index c61fd50..21cc13d 100644
--- a/sound/core/pcm_native.c
+++ b/sound/core/pcm_native.c
@@ -1981,12 +1981,13 @@ static int snd_pcm_hw_rule_sample_bits(struct snd_pcm_hw_params *params,
 	return snd_interval_refine(hw_param_interval(params, rule->var), &t);
 }
 
-#if SNDRV_PCM_RATE_5512 != 1 << 0 || SNDRV_PCM_RATE_192000 != 1 << 12
+#if SNDRV_PCM_RATE_5512 != 1 << 0 || SNDRV_PCM_RATE_384000 != 1 << 14
 #error "Change this table"
 #endif
 
 static unsigned int rates[] = { 5512, 8000, 11025, 16000, 22050, 32000, 44100,
-                                 48000, 64000, 88200, 96000, 176400, 192000 };
+				 48000, 64000, 88200, 96000, 176400, 192000,
+				 352800, 384000 };
 
 const struct snd_pcm_hw_constraint_list snd_pcm_known_rates = {
 	.count = ARRAY_SIZE(rates),
-- 
2.5.5

Re: [PATCH] ALSA: Add rate defines for 352k8 and 384k

[Takashi Iwai]

On Mon, 06 Jun 2016 19:07:43 +0200,
Clive Messer wrote:
> 
> On Mon, 2016-06-06 at 14:59 +0200, Takashi Iwai wrote:
> > Well, it'd be more convincing if you actually show the reduction of
> > the code after this patch in the current tree.  The number speaks
> > more than words.
> 
> pcm5102a codec driver with 352k8/384k defines patch...
> 
> https://github.com/DigitalDreamtimeLtd/linux/commit/83669837232018909e97623588de355fe072546b
> 
> versus, with KNOT and startup constraint...
> 
> https://github.com/DigitalDreamtimeLtd/linux/commit/06a68d757ff641b94aeb8b6385312923c5a86e18

I'm asking about "the current tree".  In other words, after applying
your patch, how many codes in my current tree can be reduced?

In the case of such a cleanup patch, the interesting part isn't in the
patch itself -- which is often a trivial change -- but rather the end
result after the change.  You can see the analogy in dietary foods:
what's more convincing is how many pounds are cut, not which fruit to
eat.

So, please prove the cleanup results as patches, and send together
with your patch as a complete patchset.  Then it'll become more
convincing.


thanks,

Takashi
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH] ALSA: Add rate defines for 352k8 and 384k

[Clive Messer]

On Mon, 2016-06-06 at 22:29 +0200, Takashi Iwai wrote:

Takashi,

> I'm asking about "the current tree".  In other words, after applying
> your patch, how many codes in my current tree can be reduced?

Well, in the current tree, the pcm5102a codec isn't enabled for 384k.
You want me to submit a patch for doing that, using constraints and
then re-submit my patch to add the sample rate defines and then a 3rd
patch, removing the constraint code from the pcm5102a codec, now it
would result in a one line change with the sample rate defines?

> So, please prove the cleanup results as patches, and send together
> with your patch as a complete patchset.  Then it'll become more
> convincing.

There must be something I am missing. I don't understand what this has
to do with "cleanup"? I never proposed anything other than adding a
couple of defines, for what are now becoming more common high-res
sample rates.

There has been resistance to adding these defines in the past. I didn't
understand why then, and I don't understand why now. What is it you
need convincing of? That DXD (352k8/24) is a standard resolution? That
192k used to be the max fs of most DAC chips.... Time marches on....
Now many are max fs 384k? 

The last time, 20160127, someone from Cirrus tried submitting a patch
to add a 384k define, the thread ended with you inviting them to re-
submit it. AFAIK, it never was re-submitted. 

http://mailman.alsa-project.org/pipermail/alsa-devel/2016-January/103506.html

And again, I don't understand why you are talking about "cleanup". I am
not proposing to cleanup any code, only add a couple of defines to a
header, which would result in one line changes, at the point someone
wanted to add 352k8/384k support to an existing codec driver, rather
than adding multiple lines of methods/code to add 352k8/384k support
via KNOT/constraints. The point I was trying to show, with the pcm5102a
example.

Regards

Clive
-- 
Clive Messer <clive.messer@digitaldreamtime.co.uk>
Digital Dreamtime Ltd

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH] ALSA: Add rate defines for 352k8 and 384k

[Takashi Iwai]

On Mon, 06 Jun 2016 23:22:20 +0200,
Clive Messer wrote:
> 
> On Mon, 2016-06-06 at 22:29 +0200, Takashi Iwai wrote:
> 
> Takashi,
> 
> > I'm asking about "the current tree".  In other words, after applying
> > your patch, how many codes in my current tree can be reduced?
> 
> Well, in the current tree, the pcm5102a codec isn't enabled for 384k.
> You want me to submit a patch for doing that, using constraints and
> then re-submit my patch to add the sample rate defines and then a 3rd
> patch, removing the constraint code from the pcm5102a codec, now it
> would result in a one line change with the sample rate defines?

That would be better than nothing, really.  You submitted only a patch
changing the core part without showing the end result.  It's
unacceptable.  At least, you should have submitted with the change of
pcm5102a driver using the new definition.

> > So, please prove the cleanup results as patches, and send together
> > with your patch as a complete patchset.  Then it'll become more
> > convincing.
> 
> There must be something I am missing. I don't understand what this has
> to do with "cleanup"? I never proposed anything other than adding a
> couple of defines, for what are now becoming more common high-res
> sample rates.

Well, I assumed that your patch was intended to reduce the redundant
code we already have.  If it's only for your new code, then it's even
less convincing...

> There has been resistance to adding these defines in the past. I didn't
> understand why then, and I don't understand why now. What is it you
> need convincing of? That DXD (352k8/24) is a standard resolution? That
> 192k used to be the max fs of most DAC chips.... Time marches on....
> Now many are max fs 384k? 
> 
> The last time, 20160127, someone from Cirrus tried submitting a patch
> to add a 384k define, the thread ended with you inviting them to re-
> submit it. AFAIK, it never was re-submitted. 
> 
> http://mailman.alsa-project.org/pipermail/alsa-devel/2016-January/103506.html
> 
> And again, I don't understand why you are talking about "cleanup". I am
> not proposing to cleanup any code, only add a couple of defines to a
> header, which would result in one line changes, at the point someone
> wanted to add 352k8/384k support to an existing codec driver, rather
> than adding multiple lines of methods/code to add 352k8/384k support
> via KNOT/constraints. The point I was trying to show, with the pcm5102a
> example.

The rule is simple: if you change a core code, it must be the benefit
allover the tree.  That is, if your patch helps other multiple
drivers, it's fine, let's take it.  If your patch is only for your
own, it needs more evaluation and it's often no-go.
How is your case?  Show it by the result of patches.


thanks,

Takashi
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH] ALSA: Add rate defines for 352k8 and 384k

[Clive Messer]

On Mon, 2016-06-06 at 23:58 +0200, Takashi Iwai wrote:
> You submitted only a patch changing the core part without showing the
> end result.  It's unacceptable.

OK. Sorry for wasting your time.

Clive
-- 
Clive Messer <clive.messer@digitaldreamtime.co.uk>
Digital Dreamtime Ltd

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH] ALSA: Add rate defines for 352k8 and 384k

[Takashi Iwai]

On Tue, 07 Jun 2016 01:00:37 +0200,
Clive Messer wrote:
> 
> On Mon, 2016-06-06 at 23:58 +0200, Takashi Iwai wrote:
> > You submitted only a patch changing the core part without showing the
> > end result.  It's unacceptable.
> 
> OK. Sorry for wasting your time.

Don't get me wrong: I'm not against your change to the core code.

What's missing is the proof of the usefulness by your patch.  That is,
please give more patches to drivers to apply your new change, for
demonstrating how great your patch is.  This is how to convince people
accepting the changes.


thanks,

Takashi
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH] ALSA: Add rate defines for 352k8 and 384k

[Charles Keepax]

On Sun, Jun 05, 2016 at 03:35:01PM +0100, DigitalDreamtime wrote:
> Add SNDRV_PCM_RATE_352800 and SNDRV_PCM_RATE_384000 defines to pcm.h,
>  for 352k8 and 384k sample rates.
> 
> Add SNDRV_PCM_RATE_8000_384000 define to pcm.h.
> 
> Update rates list in pcm_native.c.
> 
> Yes, we can use CONTINUOUS/KNOT and constraints, to support the x8
>  sample rates, but having the defines requires less code in drivers.
> Many more DAC chips are now supporting the x8 44k1/48k multiples.
> >From a high-res viewpoint, the DXD standard is 352k8, 24 bit.
> That alone should justify the addition of a define for it.
> 
> Signed-off-by: DigitalDreamtime <clive.messer@digitaldreamtime.co.uk>
> ---

Certainly looks like these rates are going to become pretty
widely used from our side so for what is worth:

Acked-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>

I am sure Brian would weight in on the side of this one as
well but he is on holiday this week.

Thanks,
Charles

Re: [PATCH] ALSA: Add rate defines for 352k8 and 384k

[Clive Messer]

On Sun, 2016-06-05 at 15:35 +0100, DigitalDreamtime wrote:

> Yes, we can use CONTINUOUS/KNOT and constraints, to support the x8
>  sample rates, but having the defines requires less code in drivers.
> Many more DAC chips are now supporting the x8 44k1/48k multiples.
> From a high-res viewpoint, the DXD standard is 352k8, 24 bit.
> That alone should justify the addition of a define for it.

I recall that previous requests to add these defines have not met with
approval. Although I think Takashi invited the last person who tried
submitting a similar patch to resubmit it, but I'm not sure it ever
was.

http://mailman.alsa-project.org/pipermail/alsa-devel/2016-January/10350
6.html

Regards

Clive
-- 
Clive Messer <clive.m.messer@gmail.com>

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

[PATCH] ALSA: Add rate defines for 352k8 and 384k

[DigitalDreamtime]

Add SNDRV_PCM_RATE_352800 and SNDRV_PCM_RATE_384000 defines to pcm.h,
 for 352k8 and 384k sample rates.

Add SNDRV_PCM_RATE_8000_384000 define to pcm.h.

Update rates list in pcm_native.c.

Yes, we can use CONTINUOUS/KNOT and constraints, to support the x8
 sample rates, but having the defines requires less code in drivers.
Many more DAC chips are now supporting the x8 44k1/48k multiples.
>From a high-res viewpoint, the DXD standard is 352k8, 24 bit.
That alone should justify the addition of a define for it.

Signed-off-by: DigitalDreamtime <clive.messer@digitaldreamtime.co.uk>
---
 include/sound/pcm.h     | 5 +++++
 sound/core/pcm_native.c | 5 +++--
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/include/sound/pcm.h b/include/sound/pcm.h
index af1fb37..86e0bbe 100644
--- a/include/sound/pcm.h
+++ b/include/sound/pcm.h
@@ -129,6 +129,8 @@ struct snd_pcm_ops {
 #define SNDRV_PCM_RATE_96000		(1<<10)		/* 96000Hz */
 #define SNDRV_PCM_RATE_176400		(1<<11)		/* 176400Hz */
 #define SNDRV_PCM_RATE_192000		(1<<12)		/* 192000Hz */
+#define SNDRV_PCM_RATE_352800		(1<<13)		/* 352800Hz */
+#define SNDRV_PCM_RATE_384000		(1<<14)		/* 384000Hz */
 
 #define SNDRV_PCM_RATE_CONTINUOUS	(1<<30)		/* continuous range */
 #define SNDRV_PCM_RATE_KNOT		(1<<31)		/* supports more non-continuos rates */
@@ -141,6 +143,9 @@ struct snd_pcm_ops {
 					 SNDRV_PCM_RATE_88200|SNDRV_PCM_RATE_96000)
 #define SNDRV_PCM_RATE_8000_192000	(SNDRV_PCM_RATE_8000_96000|SNDRV_PCM_RATE_176400|\
 					 SNDRV_PCM_RATE_192000)
+#define SNDRV_PCM_RATE_8000_384000	(SNDRV_PCM_RATE_8000_192000 |\
+					 SNDRV_PCM_RATE_352800 |\
+					 SNDRV_PCM_RATE_384000)
 #define _SNDRV_PCM_FMTBIT(fmt)		(1ULL << (__force int)SNDRV_PCM_FORMAT_##fmt)
 #define SNDRV_PCM_FMTBIT_S8		_SNDRV_PCM_FMTBIT(S8)
 #define SNDRV_PCM_FMTBIT_U8		_SNDRV_PCM_FMTBIT(U8)
diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
index c61fd50..21cc13d 100644
--- a/sound/core/pcm_native.c
+++ b/sound/core/pcm_native.c
@@ -1981,12 +1981,13 @@ static int snd_pcm_hw_rule_sample_bits(struct snd_pcm_hw_params *params,
 	return snd_interval_refine(hw_param_interval(params, rule->var), &t);
 }
 
-#if SNDRV_PCM_RATE_5512 != 1 << 0 || SNDRV_PCM_RATE_192000 != 1 << 12
+#if SNDRV_PCM_RATE_5512 != 1 << 0 || SNDRV_PCM_RATE_384000 != 1 << 14
 #error "Change this table"
 #endif
 
 static unsigned int rates[] = { 5512, 8000, 11025, 16000, 22050, 32000, 44100,
-                                 48000, 64000, 88200, 96000, 176400, 192000 };
+				 48000, 64000, 88200, 96000, 176400, 192000,
+				 352800, 384000 };
 
 const struct snd_pcm_hw_constraint_list snd_pcm_known_rates = {
 	.count = ARRAY_SIZE(rates),
-- 
2.5.5