Feedback-ID: ie8e14432:Fastmail
Date:   Thu, 9 Mar 2023 17:29:18 +0900
From:   Takashi Sakamoto <o-takashi@sakamocchi.jp>
To:     Wesley Cheng <quic_wcheng@quicinc.com>
Cc:     srinivas.kandagatla@linaro.org, mathias.nyman@intel.com,
        perex@perex.cz, broonie@kernel.org, lgirdwood@gmail.com,
        krzysztof.kozlowski+dt@linaro.org, agross@kernel.org,
        Thinh.Nguyen@synopsys.com, bgoswami@quicinc.com,
        andersson@kernel.org, robh+dt@kernel.org,
        gregkh@linuxfoundation.org, tiwai@suse.com,
        linux-kernel@vger.kernel.org, linux-arm-msm@vger.kernel.org,
        alsa-devel@alsa-project.org, devicetree@vger.kernel.org,
        linux-usb@vger.kernel.org, quic_jackp@quicinc.com,
        quic_plai@quicinc.com
Subject: Re: [PATCH v3 15/28] sound: usb: Introduce QC USB SND offloading
 support
Message-ID: <20230309082918.GA220616@workstation>
Mail-Followup-To: Wesley Cheng <quic_wcheng@quicinc.com>,
        srinivas.kandagatla@linaro.org, mathias.nyman@intel.com,
        perex@perex.cz, broonie@kernel.org, lgirdwood@gmail.com,
        krzysztof.kozlowski+dt@linaro.org, agross@kernel.org,
        Thinh.Nguyen@synopsys.com, bgoswami@quicinc.com,
        andersson@kernel.org, robh+dt@kernel.org,
        gregkh@linuxfoundation.org, tiwai@suse.com,
        linux-kernel@vger.kernel.org, linux-arm-msm@vger.kernel.org,
        alsa-devel@alsa-project.org, devicetree@vger.kernel.org,
        linux-usb@vger.kernel.org, quic_jackp@quicinc.com,
        quic_plai@quicinc.com
References: <20230308235751.495-1-quic_wcheng@quicinc.com>
 <20230308235751.495-16-quic_wcheng@quicinc.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230308235751.495-16-quic_wcheng@quicinc.com>
Precedence: bulk

Hi,

On Wed, Mar 08, 2023 at 03:57:38PM -0800, Wesley Cheng wrote:
> diff --git a/sound/usb/qcom/qc_audio_offload.c b/sound/usb/qcom/qc_audio_offload.c
> new file mode 100644
> index 000000000000..2663906644f2
> --- /dev/null
> +++ b/sound/usb/qcom/qc_audio_offload.c
> ...
> +static int enable_audio_stream(struct snd_usb_substream *subs,
> +				snd_pcm_format_t pcm_format,
> +				unsigned int channels, unsigned int cur_rate,
> +				int datainterval)
> +{
> +	struct snd_usb_audio *chip = subs->stream->chip;
> +	struct snd_pcm_hw_params params;
> +	const struct audioformat *fmt;
> +	int ret;
> +	bool fixed_rate;
> +
> +	_snd_pcm_hw_params_any(&params);
> +	_snd_pcm_hw_param_set(&params, SNDRV_PCM_HW_PARAM_FORMAT,
> +			(__force int) pcm_format, 0);
> +	_snd_pcm_hw_param_set(&params, SNDRV_PCM_HW_PARAM_CHANNELS,
> +			channels, 0);
> +	_snd_pcm_hw_param_set(&params, SNDRV_PCM_HW_PARAM_RATE,
> +			cur_rate, 0);

I think the above code is equivalent to below code.

```
// 1. Initialize the hardware parameter so that it expresses
// the maximum flags of mask parameters and the maximum range of integer
// parameters.
_snd_pcm_hw_params_any(&params);

// 2. Then shrink the mask parameters and integer parameters.
struct snd_mask *mask;
struct snd_interval *interval;

mask = hw_param_mask(params, SNDRV_PCM_HW_PARAM_FORMAT);
snd_mask_leave(mask, pcm_format);

interval = hw_param_interval(params, SNDRV_PCM_HW_PARAM_CHANNELS);
snd_interval_setinteger(&interval);
interval.min = interval.max = channels;

interval = hw_param_interval(params, SNDRV_PCM_HW_PARAM_RATE);
snd_interval_setinteger(&interval);
interval.min = interval.max = cur_rate;
```

In '[PATCH v3 10/28] sound: usb: Export USB SND APIs for modules', some
codes moved from ALSA Open Sound System compatibility layer to ALSA core
to export some kernel APIs. The '_snd_pcm_hw_param_set()' is one of
them. If they were needed just for the above operations, it would be
exaggerating just for the driver.

Of course, we can assume that the similar kernel API would be required
for the other drivers (OSS PCM, USB gadget, and so on.). However, at
present, it is preferable to focus just on your driver. 

(I note that typical sound PCM driver has code to shrink hardware
parameters in PCM rule. It consists of a set of test and refine API.)


Regards

Takashi Sakamoto