Re: [PATCH v9 4/7] ASoC: hdmi-codec: Add audio abort() callback for video side to use

[Jyri Sarha <jsarha@ti.com>]

On 04/13/16 11:31, Arnaud Pouliquen wrote:
> 
> 
> On 03/31/2016 03:36 PM, Jyri Sarha wrote:
>> Add audio abort() callback, that is provided at audio stream start,
>> for video side. This is for video side to use in case there is a
>> pressing need to tear down the audio playback for some reason.
>>
>> Signed-off-by: Jyri Sarha <jsarha@ti.com>
>> ---
>>  include/sound/hdmi-codec.h    |  8 ++++++--
>>  sound/soc/codecs/hdmi-codec.c | 20 +++++++++++++++++++-
>>  2 files changed, 25 insertions(+), 3 deletions(-)
>>
>> diff --git a/include/sound/hdmi-codec.h b/include/sound/hdmi-codec.h
>> index fc3a481..15fe70f 100644
>> --- a/include/sound/hdmi-codec.h
>> +++ b/include/sound/hdmi-codec.h
>> @@ -55,10 +55,14 @@ struct hdmi_codec_params {
>>  
>>  struct hdmi_codec_ops {
>>  	/*
>> -	 * Called when ASoC starts an audio stream setup.
>> +	 * Called when ASoC starts an audio stream setup. The call
>> +	 * provides an audio abort callback for stoping an ongoing
> stopping
>> +	 * stream from video side driver if the HDMI audio becomes
>> +	 * unavailable.
>>  	 * Optional
>>  	 */
>> -	int (*audio_startup)(struct device *dev);
>> +	int (*audio_startup)(struct device *dev,
>> +			     void (*abort_cb)(struct device *dev));
>>  
>>  	/*
>>  	 * Configures HDMI-encoder for audio stream.
>> diff --git a/sound/soc/codecs/hdmi-codec.c b/sound/soc/codecs/hdmi-codec.c
>> index b46b8ed..35151a4 100644
>> --- a/sound/soc/codecs/hdmi-codec.c
>> +++ b/sound/soc/codecs/hdmi-codec.c
>> @@ -47,6 +47,23 @@ enum {
>>  	DAI_ID_SPDIF,
>>  };
>>  
>> +static void hdmi_codec_abort(struct device *dev)
>> +{
>> +	struct hdmi_codec_priv *hcp = dev_get_drvdata(dev);
>> +
>> +	dev_dbg(dev, "%s()\n", __func__);
>> +
>> +	mutex_lock(&hcp->current_stream_lock);
>> +	if (hcp->current_stream && hcp->current_stream->runtime &&
>> +	    snd_pcm_running(hcp->current_stream)) {
>> +		dev_info(dev, "HDMI audio playback aborted\n");
>> +		snd_pcm_stream_lock_irq(hcp->current_stream);
>> +		snd_pcm_stop(hcp->current_stream, SNDRV_PCM_STATE_DISCONNECTED);
>> +		snd_pcm_stream_unlock_irq(hcp->current_stream);
>> +	}
>> +	mutex_unlock(&hcp->current_stream_lock);
>> +}
>> +
> still not understand the need... i can not find a use case that
> justifies it.
> As example, in case of HDMI plug/unplug i would not want that audio
> stream is stopped (live playback). From my point of view this should be
> a decision from user.
> But as it is optional, I'm ok if need is justified.
> 

Well, I think we can drop this as there is no use for it ATM.

I copied this feature from my omap-hdmi-audio driver, where the display
module on the SoC implements ASoC CPU-DAI itself. In that setup the DMA
simply stops if the display is turned off, which eventually causes the
audio stream to time out. However, it is unlikely to have a similar
situation here, since the purpose is to implement ASoC codec DAI for an
external HDMI encoder sitting behind i2s and/or spdif wires and the
CPU-DAI can keep on playing the audio even if the whole encoder powered
down. That is if the HDMI encoder does not provide the clocks for the
DAI link. Anyway, we can add this feature later if there ever is a need
for it.

>>  static int hdmi_codec_new_stream(struct snd_pcm_substream *substream,
>>  				 struct snd_soc_dai *dai)
>>  {
>> @@ -78,7 +95,8 @@ static int hdmi_codec_startup(struct snd_pcm_substream *substream,
>>  		return ret;
>>  
>>  	if (hcp->hcd.ops->audio_startup) {
>> -		ret = hcp->hcd.ops->audio_startup(dai->dev->parent);
>> +		ret = hcp->hcd.ops->audio_startup(dai->dev->parent,
>> +						  hdmi_codec_abort);
>>  		if (ret) {
>>  			mutex_lock(&hcp->current_stream_lock);
>>  			hcp->current_stream = NULL;
>>

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel