[PATCH] ice1712: fix: lock samplerate when samplerate locking is enabled.

[PATCH] ice1712: fix: lock samplerate when samplerate locking is enabled.

[Sebastien Alaiwan]

Hello,

I found that the sampling rate locking setting of the ice1712 sound driver was
only half-respected : when the driver was locked to, let's say, 44100Hz, and a
usermode app was requesting 48000Hz playback, the request was succesful although
the soundcard would continue to run at 44100Hz.

Here's a patch that will make those requests to fail.


diff --git a/sound/pci/ice1712/ice1712.c b/sound/pci/ice1712/ice1712.c
index c7cff6f..4775626 100644
--- a/sound/pci/ice1712/ice1712.c
+++ b/sound/pci/ice1712/ice1712.c
@@ -1181,6 +1181,8 @@ static int snd_ice1712_playback_pro_open(struct snd_pcm_substream *substream)
 	snd_pcm_hw_constraint_msbits(runtime, 0, 32, 24);
 	snd_pcm_hw_constraint_list(runtime, 0, SNDRV_PCM_HW_PARAM_RATE, &hw_constraints_rates);
 
+	if (is_pro_rate_locked(ice))
+		snd_pcm_hw_constraint_minmax(runtime, SNDRV_PCM_HW_PARAM_RATE, PRO_RATE_DEFAULT, PRO_RATE_DEFAULT);
 	if (ice->spdif.ops.open)
 		ice->spdif.ops.open(ice, substream);
 
@@ -1197,6 +1199,9 @@ static int snd_ice1712_capture_pro_open(struct snd_pcm_substream *substream)
 	snd_pcm_set_sync(substream);
 	snd_pcm_hw_constraint_msbits(runtime, 0, 32, 24);
 	snd_pcm_hw_constraint_list(runtime, 0, SNDRV_PCM_HW_PARAM_RATE, &hw_constraints_rates);
+	if (is_pro_rate_locked(ice))
+		snd_pcm_hw_constraint_minmax(runtime, SNDRV_PCM_HW_PARAM_RATE, PRO_RATE_DEFAULT, PRO_RATE_DEFAULT);
+
 	return 0;
 }
 

Re: [PATCH] ice1712: fix: lock samplerate when samplerate locking is enabled.

[Takashi Iwai]

At Wed, 03 Feb 2010 21:45:34 +0100,
Sebastien Alaiwan wrote:
> 
> Hello,
> 
> I found that the sampling rate locking setting of the ice1712 sound driver was
> only half-respected : when the driver was locked to, let's say, 44100Hz, and a
> usermode app was requesting 48000Hz playback, the request was succesful although
> the soundcard would continue to run at 44100Hz.
> 
> Here's a patch that will make those requests to fail.

Thanks, the patch looks OK.  But changing runtime->hw.rate_min and
rate_max would be cheaper.  It won't involve with kmalloc().
In theory, snd_pcm_hw_constraint*() can fail.

Could you fix in that way and give your sign-off to merge to the
upstream?  (And don't forget to add me to Cc.)


thanks,

Takashi


> diff --git a/sound/pci/ice1712/ice1712.c b/sound/pci/ice1712/ice1712.c
> index c7cff6f..4775626 100644
> --- a/sound/pci/ice1712/ice1712.c
> +++ b/sound/pci/ice1712/ice1712.c
> @@ -1181,6 +1181,8 @@ static int snd_ice1712_playback_pro_open(struct snd_pcm_substream *substream)
>  	snd_pcm_hw_constraint_msbits(runtime, 0, 32, 24);
>  	snd_pcm_hw_constraint_list(runtime, 0, SNDRV_PCM_HW_PARAM_RATE, &hw_constraints_rates);
>  
> +	if (is_pro_rate_locked(ice))
> +		snd_pcm_hw_constraint_minmax(runtime, SNDRV_PCM_HW_PARAM_RATE, PRO_RATE_DEFAULT, PRO_RATE_DEFAULT);
>  	if (ice->spdif.ops.open)
>  		ice->spdif.ops.open(ice, substream);
>  
> @@ -1197,6 +1199,9 @@ static int snd_ice1712_capture_pro_open(struct snd_pcm_substream *substream)
>  	snd_pcm_set_sync(substream);
>  	snd_pcm_hw_constraint_msbits(runtime, 0, 32, 24);
>  	snd_pcm_hw_constraint_list(runtime, 0, SNDRV_PCM_HW_PARAM_RATE, &hw_constraints_rates);
> +	if (is_pro_rate_locked(ice))
> +		snd_pcm_hw_constraint_minmax(runtime, SNDRV_PCM_HW_PARAM_RATE, PRO_RATE_DEFAULT, PRO_RATE_DEFAULT);
> +
>  	return 0;
>  }
>  
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 

Re: [PATCH] ice1712: fix: lock samplerate when samplerate locking is enabled.

[Sebastien Alaiwan]

Takashi Iwai wrote:
> At Wed, 03 Feb 2010 21:45:34 +0100,
> Sebastien Alaiwan wrote:
>> Hello,
>>
>> I found that the sampling rate locking setting of the ice1712 sound driver was
>> only half-respected : when the driver was locked to, let's say, 44100Hz, and a
>> usermode app was requesting 48000Hz playback, the request was succesful although
>> the soundcard would continue to run at 44100Hz.
>>
>> Here's a patch that will make those requests to fail.
> 
> Thanks, the patch looks OK.  But changing runtime->hw.rate_min and
> rate_max would be cheaper.  It won't involve with kmalloc().
> In theory, snd_pcm_hw_constraint*() can fail.
> 
> Could you fix in that way and give your sign-off to merge to the
> upstream?  (And don't forget to add me to Cc.)

Thank you for reviewing my patch. Here's an updated version according to your
advice. You can merge to the upstream if it's okay.

Sebastien Alaiwan

------
diff --git a/sound/pci/ice1712/ice1712.c b/sound/pci/ice1712/ice1712.c
index c7cff6f..319df17 100644
--- a/sound/pci/ice1712/ice1712.c
+++ b/sound/pci/ice1712/ice1712.c
@@ -1180,6 +1180,11 @@ static int snd_ice1712_playback_pro_open(struct snd_pcm_substream *substream)
 	snd_pcm_set_sync(substream);
 	snd_pcm_hw_constraint_msbits(runtime, 0, 32, 24);
 	snd_pcm_hw_constraint_list(runtime, 0, SNDRV_PCM_HW_PARAM_RATE, &hw_constraints_rates);
+	if (is_pro_rate_locked(ice))
+	{
+		runtime->hw.rate_min = PRO_RATE_DEFAULT;
+		runtime->hw.rate_max = PRO_RATE_DEFAULT;
+	}
 
 	if (ice->spdif.ops.open)
 		ice->spdif.ops.open(ice, substream);
@@ -1197,6 +1202,12 @@ static int snd_ice1712_capture_pro_open(struct snd_pcm_substream *substream)
 	snd_pcm_set_sync(substream);
 	snd_pcm_hw_constraint_msbits(runtime, 0, 32, 24);
 	snd_pcm_hw_constraint_list(runtime, 0, SNDRV_PCM_HW_PARAM_RATE, &hw_constraints_rates);
+	if (is_pro_rate_locked(ice))
+	{
+		runtime->hw.rate_min = PRO_RATE_DEFAULT;
+		runtime->hw.rate_max = PRO_RATE_DEFAULT;
+	}
+
 	return 0;
 }
 

Re: [PATCH] ice1712: fix: lock samplerate when samplerate locking is enabled.

[Sebastien Alaiwan]

Sebastien Alaiwan wrote:
 Takashi Iwai wrote:
> At Wed, 03 Feb 2010 21:45:34 +0100,
> Sebastien Alaiwan wrote:
>> Hello,
>>
>> I found that the sampling rate locking setting of the ice1712 sound driver was
>> only half-respected : when the driver was locked to, let's say, 44100Hz, and a
>> usermode app was requesting 48000Hz playback, the request was succesful although
>> the soundcard would continue to run at 44100Hz.
>>
>> Here's a patch that will make those requests to fail.
> Thanks, the patch looks OK.  But changing runtime->hw.rate_min and
> rate_max would be cheaper.  It won't involve with kmalloc().
> In theory, snd_pcm_hw_constraint*() can fail.
>
> Could you fix in that way and give your sign-off to merge to the
> upstream?  (And don't forget to add me to Cc.)

Thank you for reviewing my patch. Here's an updated version according to your
advice. You can merge to the upstream if it's okay. Sorry for the double
mail, here's a signed-off version.

Signed-off-by: Sebastien Alaiwan <sebastien.alaiwan@gmail.com>
------
diff --git a/sound/pci/ice1712/ice1712.c b/sound/pci/ice1712/ice1712.c
index c7cff6f..319df17 100644
--- a/sound/pci/ice1712/ice1712.c
+++ b/sound/pci/ice1712/ice1712.c
@@ -1180,6 +1180,11 @@ static int snd_ice1712_playback_pro_open(struct snd_pcm_substream *substream)
 	snd_pcm_set_sync(substream);
 	snd_pcm_hw_constraint_msbits(runtime, 0, 32, 24);
 	snd_pcm_hw_constraint_list(runtime, 0, SNDRV_PCM_HW_PARAM_RATE, &hw_constraints_rates);
+	if (is_pro_rate_locked(ice))
+	{
+		runtime->hw.rate_min = PRO_RATE_DEFAULT;
+		runtime->hw.rate_max = PRO_RATE_DEFAULT;
+	}
 
 	if (ice->spdif.ops.open)
 		ice->spdif.ops.open(ice, substream);
@@ -1197,6 +1202,12 @@ static int snd_ice1712_capture_pro_open(struct snd_pcm_substream *substream)
 	snd_pcm_set_sync(substream);
 	snd_pcm_hw_constraint_msbits(runtime, 0, 32, 24);
 	snd_pcm_hw_constraint_list(runtime, 0, SNDRV_PCM_HW_PARAM_RATE, &hw_constraints_rates);
+	if (is_pro_rate_locked(ice))
+	{
+		runtime->hw.rate_min = PRO_RATE_DEFAULT;
+		runtime->hw.rate_max = PRO_RATE_DEFAULT;
+	}
+
 	return 0;
 }
 

Re: [PATCH] ice1712: fix: lock samplerate when samplerate locking is enabled.

[Takashi Iwai]

At Thu, 04 Feb 2010 22:20:11 +0100,
Sebastien Alaiwan wrote:
> 
> Sebastien Alaiwan wrote:
>  Takashi Iwai wrote:
> > At Wed, 03 Feb 2010 21:45:34 +0100,
> > Sebastien Alaiwan wrote:
> >> Hello,
> >>
> >> I found that the sampling rate locking setting of the ice1712 sound driver was
> >> only half-respected : when the driver was locked to, let's say, 44100Hz, and a
> >> usermode app was requesting 48000Hz playback, the request was succesful although
> >> the soundcard would continue to run at 44100Hz.
> >>
> >> Here's a patch that will make those requests to fail.
> > Thanks, the patch looks OK.  But changing runtime->hw.rate_min and
> > rate_max would be cheaper.  It won't involve with kmalloc().
> > In theory, snd_pcm_hw_constraint*() can fail.
> >
> > Could you fix in that way and give your sign-off to merge to the
> > upstream?  (And don't forget to add me to Cc.)
> 
> Thank you for reviewing my patch. Here's an updated version according to your
> advice. You can merge to the upstream if it's okay. Sorry for the double
> mail, here's a signed-off version.
> 
> Signed-off-by: Sebastien Alaiwan <sebastien.alaiwan@gmail.com>

Thanks!  Applied now (with minor coding-style fixes).


Takashi

> ------
> diff --git a/sound/pci/ice1712/ice1712.c b/sound/pci/ice1712/ice1712.c
> index c7cff6f..319df17 100644
> --- a/sound/pci/ice1712/ice1712.c
> +++ b/sound/pci/ice1712/ice1712.c
> @@ -1180,6 +1180,11 @@ static int snd_ice1712_playback_pro_open(struct snd_pcm_substream *substream)
>  	snd_pcm_set_sync(substream);
>  	snd_pcm_hw_constraint_msbits(runtime, 0, 32, 24);
>  	snd_pcm_hw_constraint_list(runtime, 0, SNDRV_PCM_HW_PARAM_RATE, &hw_constraints_rates);
> +	if (is_pro_rate_locked(ice))
> +	{
> +		runtime->hw.rate_min = PRO_RATE_DEFAULT;
> +		runtime->hw.rate_max = PRO_RATE_DEFAULT;
> +	}
>  
>  	if (ice->spdif.ops.open)
>  		ice->spdif.ops.open(ice, substream);
> @@ -1197,6 +1202,12 @@ static int snd_ice1712_capture_pro_open(struct snd_pcm_substream *substream)
>  	snd_pcm_set_sync(substream);
>  	snd_pcm_hw_constraint_msbits(runtime, 0, 32, 24);
>  	snd_pcm_hw_constraint_list(runtime, 0, SNDRV_PCM_HW_PARAM_RATE, &hw_constraints_rates);
> +	if (is_pro_rate_locked(ice))
> +	{
> +		runtime->hw.rate_min = PRO_RATE_DEFAULT;
> +		runtime->hw.rate_max = PRO_RATE_DEFAULT;
> +	}
> +
>  	return 0;
>  }
>  
>