Re: [PATCH 2/2] ASoC: ti: davinci-mcasp: Set min period size using FIFO config

["Péter Ujfalusi" <peter.ujfalusi@gmail.com>]

Hi,

On 6/4/24 1:01 PM, Jai Luthra wrote:
> The minimum period size was enforced to 64 as older devices integrating
> McASP with EDMA used an internal FIFO of 64 samples.
> 
> With UDMA based platforms this internal McASP FIFO is optional, as the
> DMA engine internally does some buffering which is already accounted for
> when registering the platform. So we should read the actual FIFO
> configuration (txnumevt/rxnumevt) instead of hardcoding frames.min to
> 64.
> 
> Signed-off-by: Jai Luthra <j-luthra@ti.com>
> ---
>  sound/soc/ti/davinci-mcasp.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/sound/soc/ti/davinci-mcasp.c b/sound/soc/ti/davinci-mcasp.c
> index 1e760c315521..2a53fb7e72eb 100644
> --- a/sound/soc/ti/davinci-mcasp.c
> +++ b/sound/soc/ti/davinci-mcasp.c
> @@ -70,6 +70,7 @@ struct davinci_mcasp_context {
>  struct davinci_mcasp_ruledata {
>  	struct davinci_mcasp *mcasp;
>  	int serializers;
> +	u8 numevt;
>  };
>  
>  struct davinci_mcasp {
> @@ -1470,12 +1471,13 @@ static int davinci_mcasp_hw_rule_format(struct snd_pcm_hw_params *params,
>  static int davinci_mcasp_hw_rule_min_periodsize(
>  		struct snd_pcm_hw_params *params, struct snd_pcm_hw_rule *rule)
>  {
> +	struct davinci_mcasp_ruledata *rd = rule->private;
>  	struct snd_interval *period_size = hw_param_interval(params,
>  						SNDRV_PCM_HW_PARAM_PERIOD_SIZE);
>  	struct snd_interval frames;
>  
>  	snd_interval_any(&frames);
> -	frames.min = 64;
> +	frames.min = rd->numevt;

64 was a nice number ;)

>  	frames.integer = 1;
>  
>  	return snd_interval_refine(period_size, &frames);
> @@ -1516,6 +1518,9 @@ static int davinci_mcasp_startup(struct snd_pcm_substream *substream,
>  		if (mcasp->serial_dir[i] == dir)
>  			max_channels++;
>  	}
> +	ruledata->numevt = (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) ?
> +				   mcasp->txnumevt :
> +				   mcasp->rxnumevt;

Do this at the same location where the rest of the ruledata members are
initialized, or

>  	ruledata->serializers = max_channels;
>  	ruledata->mcasp = mcasp;
>  	max_channels *= tdm_slots;
> @@ -1591,7 +1596,7 @@ static int davinci_mcasp_startup(struct snd_pcm_substream *substream,
>  
>  	snd_pcm_hw_rule_add(substream->runtime, 0,
>  			    SNDRV_PCM_HW_PARAM_PERIOD_SIZE,
> -			    davinci_mcasp_hw_rule_min_periodsize, NULL,
> +			    davinci_mcasp_hw_rule_min_periodsize, ruledata,

You could just pass a pointer to txnumevt/rxnumevt directly...

>  			    SNDRV_PCM_HW_PARAM_PERIOD_SIZE, -1);
>  
>  	return 0;
> 

-- 
Péter