Re: [RFC 07/11] ALSA: hda - add generic functions to set hdac stream params

From: Takashi Iwai <tiwai@suse.de>
To: "Subhransu S. Prusty" <subhransu.s.prusty@intel.com>
Cc: alsa-devel@alsa-project.org, patches.audio@intel.com,
	lgirdwood@gmail.com, Vinod Koul <vinod.koul@intel.com>,
	broonie@kernel.org, Jeeja KP <jeeja.kp@intel.com>
Subject: Re: [RFC 07/11] ALSA: hda - add generic functions to set hdac stream params
Date: Mon, 13 Apr 2015 14:04:10 +0200	[thread overview]
Message-ID: <s5h6190p745.wl-tiwai@suse.de> (raw)
In-Reply-To: <1428842178-7105-8-git-send-email-subhransu.s.prusty@intel.com>

At Sun, 12 Apr 2015 18:06:14 +0530,
Subhransu S. Prusty wrote:
> 
> From: Jeeja KP <jeeja.kp@intel.com>
> 
> This will be used by hda controller driver to
> setup stream params in prepare. This function will
> setup the bdl and periods.
> 
> Signed-off-by: Jeeja KP <jeeja.kp@intel.com>
> Signed-off-by: Subhransu S. Prusty <subhransu.s.prusty@intel.com>
> Signed-off-by: Vinod Koul <vinod.koul@intel.com>
> ---
>  include/sound/hdaudio.h |  2 ++
>  sound/hda/hdac_stream.c | 29 +++++++++++++++++++++++++++++
>  2 files changed, 31 insertions(+)
> 
> diff --git a/include/sound/hdaudio.h b/include/sound/hdaudio.h
> index 5fc6d81..1f9f4ec 100644
> --- a/include/sound/hdaudio.h
> +++ b/include/sound/hdaudio.h
> @@ -364,6 +364,8 @@ void snd_hdac_stream_sync(struct hdac_stream *azx_dev, bool start,
>  			  unsigned int streams);
>  void snd_hdac_stream_timecounter_init(struct hdac_stream *azx_dev,
>  				      unsigned int streams);
> +int snd_set_hdac_stream_params(struct hdac_stream *azx_dev,
> +				 unsigned int format_val);
>  
>  /*DSP loader functions */
>  int snd_hdac_load_dsp_prepare(struct hdac_stream *azx_dev, unsigned int format,
> diff --git a/sound/hda/hdac_stream.c b/sound/hda/hdac_stream.c
> index 394fd15..ce25f8f 100644
> --- a/sound/hda/hdac_stream.c
> +++ b/sound/hda/hdac_stream.c
> @@ -472,6 +472,35 @@ void snd_hdac_stream_sync(struct hdac_stream *azx_dev, bool start,
>  }
>  EXPORT_SYMBOL_GPL(snd_hdac_stream_sync);
>  
> +int snd_set_hdac_stream_params(struct hdac_stream *azx_dev,
> +				 unsigned int format_val)

I'd name it a bit more consistently, e.g. snd_hdac_stream_set_format()
or snd_hdac_stream_set_params() or so.

> +{
> +
> +	unsigned int bufsize, period_bytes;
> +	struct snd_pcm_substream *substream = azx_dev->substream;
> +	struct snd_pcm_runtime *runtime = substream->runtime;

I would do a NULL check for substream to be sure.

thanks,

Takashi

> +	int err;
> +
> +	bufsize = snd_pcm_lib_buffer_bytes(substream);
> +	period_bytes = snd_pcm_lib_period_bytes(substream);
> +
> +	if (bufsize != azx_dev->bufsize ||
> +	    period_bytes != azx_dev->period_bytes ||
> +	    format_val != azx_dev->format_val ||
> +	    runtime->no_period_wakeup != azx_dev->no_period_wakeup) {
> +		azx_dev->bufsize = bufsize;
> +		azx_dev->period_bytes = period_bytes;
> +		azx_dev->format_val = format_val;
> +		azx_dev->no_period_wakeup = runtime->no_period_wakeup;
> +		err = snd_hdac_stream_setup_periods(azx_dev);
> +		if (err < 0)
> +			return err;
> +	}
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(snd_set_hdac_stream_params);
> +
> +
>  int snd_hdac_load_dsp_prepare(struct hdac_stream *azx_dev, unsigned int format,
>  				unsigned int byte_size,
>  				struct snd_dma_buffer *bufp)
> -- 
> 1.9.0
> 