diff mbox

[1/2] OMAPDSS: hdmi: Reconfigure and restart audio when display is enabled

Message ID 44089dc819903c71e1d0ed0c14dd3ab060ed8c68.1440594174.git.jsarha@ti.com (mailing list archive)
State New, archived
Headers show

Commit Message

Jyri Sarha Aug. 26, 2015, 1:11 p.m. UTC
Reconfigure and restart audio when display is enabled, if audio
playback was active before. The audio configuration is stored when is
is successfully applied and a boolean is set when playback has started
and unset when stopped. This data is used to reconfigure the audio
when display is re-enabled. Abort audio playback if reconfiguration
fails.

A new spin lock is introduced to in order protect state variables
related to audio playback status. The wp_idlemode protection relies on
PM not touching it after the original initialization. A power
management API for controlling the idlemode would be needed for proper
synchronization.

Signed-off-by: Jyri Sarha <jsarha@ti.com>
---
 drivers/video/fbdev/omap2/dss/hdmi.h  | 10 +++-
 drivers/video/fbdev/omap2/dss/hdmi4.c | 65 ++++++++++++++++++++-----
 drivers/video/fbdev/omap2/dss/hdmi5.c | 89 +++++++++++++++++++++++++++++------
 3 files changed, 137 insertions(+), 27 deletions(-)

Comments

Tomi Valkeinen Aug. 27, 2015, 2:30 p.m. UTC | #1
Hi,

On 26/08/15 16:11, Jyri Sarha wrote:

I few comments, for the parts I had time to review:

> diff --git a/drivers/video/fbdev/omap2/dss/hdmi5.c b/drivers/video/fbdev/omap2/dss/hdmi5.c
> index 7f87578..f352c4b 100644
> --- a/drivers/video/fbdev/omap2/dss/hdmi5.c
> +++ b/drivers/video/fbdev/omap2/dss/hdmi5.c
> @@ -352,6 +352,7 @@ static int read_edid(u8 *buf, int len)
>  static int hdmi_display_enable(struct omap_dss_device *dssdev)
>  {
>  	struct omap_dss_device *out = &hdmi.output;
> +	unsigned long flags;
>  	int r = 0;
>  
>  	DSSDBG("ENTER hdmi_display_enable\n");
> @@ -370,7 +371,37 @@ static int hdmi_display_enable(struct omap_dss_device *dssdev)
>  		goto err0;
>  	}
>  

I think you could refactor parts of the code below into small helper
functions:

> +	if (hdmi.audio_configured) {
> +		spin_lock_irqsave(&hdmi.audio_playing_lock, flags);
> +		hdmi_wp_audio_core_req_enable(&hdmi.wp, false);
> +		hdmi_wp_audio_enable(&hdmi.wp, false);
> +		if (hdmi.wp_idlemode > 0)
> +			REG_FLD_MOD(hdmi.wp.base, HDMI_WP_SYSCONFIG,
> +				    hdmi.wp_idlemode, 3, 2);
> +		hdmi.wp_idlemode = -1;

The above looks like it's disabling audio output, the same that's done
in hdmi_audio_stop(). Adding a helper func for that makes the code more
readable.

For the wp_idlemode, I think hdmi.wp_idlemode could be initialized to a
valid value, so that it can be set at any time without the if check above.

> +		spin_unlock_irqrestore(&hdmi.audio_playing_lock, flags);
> +
> +		r = hdmi5_audio_config(&hdmi.core, &hdmi.wp, &hdmi.audio_config,
> +				       hdmi.cfg.timings.pixelclock);
> +		if (r) {
> +			DSSERR("Error restoring audio configuration: %d", r);
> +			hdmi.audio_abort_cb(&hdmi.pdev->dev);
> +			hdmi.audio_configured = false;
> +		}
> +	}
> +
> +	spin_lock_irqsave(&hdmi.audio_playing_lock, flags);
> +	if (hdmi.audio_configured && hdmi.audio_playing) {
> +		/* No-idle while playing audio, store the old value */
> +		hdmi.wp_idlemode =
> +			REG_GET(hdmi.wp.base, HDMI_WP_SYSCONFIG, 3, 2);
> +		REG_FLD_MOD(hdmi.wp.base, HDMI_WP_SYSCONFIG, 1, 3, 2);
> +
> +		hdmi_wp_audio_enable(&hdmi.wp, true);
> +		hdmi_wp_audio_core_req_enable(&hdmi.wp, true);

And here maybe a helper func to enable the audio output.

> +	}

>  	hdmi.display_enabled = true;
> +	spin_unlock_irqrestore(&hdmi.audio_playing_lock, flags);
>  
>  	mutex_unlock(&hdmi.lock);
>  	return 0;
> @@ -382,17 +413,18 @@ err0:
>  
>  static void hdmi_display_disable(struct omap_dss_device *dssdev)
>  {
> +	unsigned long flags;
> +
>  	DSSDBG("Enter hdmi_display_disable\n");
>  
>  	mutex_lock(&hdmi.lock);
>  
> -	if (hdmi.audio_pdev && hdmi.audio_abort_cb)
> -		hdmi.audio_abort_cb(&hdmi.audio_pdev->dev);
> +	spin_lock_irqsave(&hdmi.audio_playing_lock, flags);
> +	hdmi.display_enabled = false;
> +	spin_unlock_irqrestore(&hdmi.audio_playing_lock, flags);

Shouldn't something be done here in hdmi_display_disable about audio?
You probably want to keep the state flag enabled, so that we know the
user is playing audio, but you could still disable the HDMI audio HW.
I'm sure the audio output dies when the video is disabled, but being
more explicit here makes the code logic easier to follow.

 Tomi
Tomi Valkeinen Aug. 28, 2015, 10:37 a.m. UTC | #2
On 26/08/15 16:11, Jyri Sarha wrote:

> diff --git a/drivers/video/fbdev/omap2/dss/hdmi5.c b/drivers/video/fbdev/omap2/dss/hdmi5.c
> index 7f87578..f352c4b 100644
> --- a/drivers/video/fbdev/omap2/dss/hdmi5.c
> +++ b/drivers/video/fbdev/omap2/dss/hdmi5.c
> @@ -352,6 +352,7 @@ static int read_edid(u8 *buf, int len)
>  static int hdmi_display_enable(struct omap_dss_device *dssdev)
>  {
>  	struct omap_dss_device *out = &hdmi.output;
> +	unsigned long flags;
>  	int r = 0;
>  
>  	DSSDBG("ENTER hdmi_display_enable\n");
> @@ -370,7 +371,37 @@ static int hdmi_display_enable(struct omap_dss_device *dssdev)
>  		goto err0;
>  	}
>  
> +	if (hdmi.audio_configured) {
> +		spin_lock_irqsave(&hdmi.audio_playing_lock, flags);
> +		hdmi_wp_audio_core_req_enable(&hdmi.wp, false);
> +		hdmi_wp_audio_enable(&hdmi.wp, false);
> +		if (hdmi.wp_idlemode > 0)
> +			REG_FLD_MOD(hdmi.wp.base, HDMI_WP_SYSCONFIG,
> +				    hdmi.wp_idlemode, 3, 2);
> +		hdmi.wp_idlemode = -1;
> +		spin_unlock_irqrestore(&hdmi.audio_playing_lock, flags);

Here I think the audio HW is always disabled already. It has to be,
because the whole HDMI IP has been off. So the above should not be needed.

> +
> +		r = hdmi5_audio_config(&hdmi.core, &hdmi.wp, &hdmi.audio_config,
> +				       hdmi.cfg.timings.pixelclock);
> +		if (r) {
> +			DSSERR("Error restoring audio configuration: %d", r);
> +			hdmi.audio_abort_cb(&hdmi.pdev->dev);
> +			hdmi.audio_configured = false;
> +		}
> +	}
> +
> +	spin_lock_irqsave(&hdmi.audio_playing_lock, flags);
> +	if (hdmi.audio_configured && hdmi.audio_playing) {
> +		/* No-idle while playing audio, store the old value */
> +		hdmi.wp_idlemode =
> +			REG_GET(hdmi.wp.base, HDMI_WP_SYSCONFIG, 3, 2);
> +		REG_FLD_MOD(hdmi.wp.base, HDMI_WP_SYSCONFIG, 1, 3, 2);
> +
> +		hdmi_wp_audio_enable(&hdmi.wp, true);
> +		hdmi_wp_audio_core_req_enable(&hdmi.wp, true);
> +	}
>  	hdmi.display_enabled = true;
> +	spin_unlock_irqrestore(&hdmi.audio_playing_lock, flags);

Maybe you've looked at the locking carefully, but it's not obvious to
me. So is hdmi_audio_start and hdmi_audio_stop the only functions that
are called from atomic context? Every other function is protected with
the mutex?

 Tomi
Jyri Sarha Aug. 28, 2015, 11:35 a.m. UTC | #3
On 08/28/15 13:37, Tomi Valkeinen wrote:
>
> On 26/08/15 16:11, Jyri Sarha wrote:
>
>> diff --git a/drivers/video/fbdev/omap2/dss/hdmi5.c b/drivers/video/fbdev/omap2/dss/hdmi5.c
>> index 7f87578..f352c4b 100644
>> --- a/drivers/video/fbdev/omap2/dss/hdmi5.c
>> +++ b/drivers/video/fbdev/omap2/dss/hdmi5.c
>> @@ -352,6 +352,7 @@ static int read_edid(u8 *buf, int len)
>>   static int hdmi_display_enable(struct omap_dss_device *dssdev)
>>   {
>>   	struct omap_dss_device *out = &hdmi.output;
>> +	unsigned long flags;
>>   	int r = 0;
>>
>>   	DSSDBG("ENTER hdmi_display_enable\n");
>> @@ -370,7 +371,37 @@ static int hdmi_display_enable(struct omap_dss_device *dssdev)
>>   		goto err0;
>>   	}
>>
>> +	if (hdmi.audio_configured) {
>> +		spin_lock_irqsave(&hdmi.audio_playing_lock, flags);
>> +		hdmi_wp_audio_core_req_enable(&hdmi.wp, false);
>> +		hdmi_wp_audio_enable(&hdmi.wp, false);
>> +		if (hdmi.wp_idlemode > 0)
>> +			REG_FLD_MOD(hdmi.wp.base, HDMI_WP_SYSCONFIG,
>> +				    hdmi.wp_idlemode, 3, 2);
>> +		hdmi.wp_idlemode = -1;
>> +		spin_unlock_irqrestore(&hdmi.audio_playing_lock, flags);
>
> Here I think the audio HW is always disabled already. It has to be,
> because the whole HDMI IP has been off. So the above should not be needed.
>
>> +
>> +		r = hdmi5_audio_config(&hdmi.core, &hdmi.wp, &hdmi.audio_config,
>> +				       hdmi.cfg.timings.pixelclock);
>> +		if (r) {
>> +			DSSERR("Error restoring audio configuration: %d", r);
>> +			hdmi.audio_abort_cb(&hdmi.pdev->dev);
>> +			hdmi.audio_configured = false;
>> +		}
>> +	}
>> +
>> +	spin_lock_irqsave(&hdmi.audio_playing_lock, flags);
>> +	if (hdmi.audio_configured && hdmi.audio_playing) {
>> +		/* No-idle while playing audio, store the old value */
>> +		hdmi.wp_idlemode =
>> +			REG_GET(hdmi.wp.base, HDMI_WP_SYSCONFIG, 3, 2);
>> +		REG_FLD_MOD(hdmi.wp.base, HDMI_WP_SYSCONFIG, 1, 3, 2);
>> +
>> +		hdmi_wp_audio_enable(&hdmi.wp, true);
>> +		hdmi_wp_audio_core_req_enable(&hdmi.wp, true);
>> +	}
>>   	hdmi.display_enabled = true;
>> +	spin_unlock_irqrestore(&hdmi.audio_playing_lock, flags);
>
> Maybe you've looked at the locking carefully, but it's not obvious to
> me. So is hdmi_audio_start and hdmi_audio_stop the only functions that
> are called from atomic context? Every other function is protected with
> the mutex?
>

The idea is for the spinlock to make audio start, audio stop, and 
updates to hdmi.display_enabled and hdmi.audio_playing variable atomic.

This is needed for the transitions from display enabled state (when 
audio start/stop commands can be written to HW) to display disabled 
state (when audio start/stop commands update only the hdmi.audio_playing 
variable) to always serialize correctly.

IOW, the idea is to make sure the hdmi.audio_playing variable is always 
in sync with what is in the HW when hdmi.display_enabled == true.

For example: when display is turned back on we take the spinlock and we 
can be sure that the audio start/stop status won't change while we 
update the HW according to current state and set hdmi.display_enabled to 
true. After releasing the lock hdmi.display_enabled is true and all 
audio_start and audio_stop commands write their stuff directly to HW.

In theory, just making hdmi.display_enabled and hdmi.audio_playing 
atomic-variables and touching them always in correct oreder should be 
enough, but explaining the mechanism would then be even trickier.

Cheers,
Jyri
--
To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/video/fbdev/omap2/dss/hdmi.h b/drivers/video/fbdev/omap2/dss/hdmi.h
index e4a32fe..1e45b84 100644
--- a/drivers/video/fbdev/omap2/dss/hdmi.h
+++ b/drivers/video/fbdev/omap2/dss/hdmi.h
@@ -351,12 +351,20 @@  struct omap_hdmi {
 	struct regulator *vdda_reg;
 
 	bool core_enabled;
-	bool display_enabled;
 
 	struct omap_dss_device output;
 
 	struct platform_device *audio_pdev;
 	void (*audio_abort_cb)(struct device *dev);
+
+	bool audio_configured;
+	struct omap_dss_audio audio_config;
+
+	/* This lock should be taken when any one of the three
+	   state variables bellow are touched. */
+	spinlock_t audio_playing_lock;
+	bool audio_playing;
+	bool display_enabled;
 	int wp_idlemode;
 };
 
diff --git a/drivers/video/fbdev/omap2/dss/hdmi4.c b/drivers/video/fbdev/omap2/dss/hdmi4.c
index 6d3aa3f..ea1fa64 100644
--- a/drivers/video/fbdev/omap2/dss/hdmi4.c
+++ b/drivers/video/fbdev/omap2/dss/hdmi4.c
@@ -324,6 +324,7 @@  static int read_edid(u8 *buf, int len)
 static int hdmi_display_enable(struct omap_dss_device *dssdev)
 {
 	struct omap_dss_device *out = &hdmi.output;
+	unsigned long flags;
 	int r = 0;
 
 	DSSDBG("ENTER hdmi_display_enable\n");
@@ -342,7 +343,26 @@  static int hdmi_display_enable(struct omap_dss_device *dssdev)
 		goto err0;
 	}
 
+	if (hdmi.audio_configured) {
+		hdmi4_audio_stop(&hdmi.core, &hdmi.wp);
+		hdmi_wp_audio_enable(&hdmi.wp, false);
+
+		r = hdmi4_audio_config(&hdmi.core, &hdmi.wp, &hdmi.audio_config,
+				       hdmi.cfg.timings.pixelclock);
+		if (r) {
+			DSSERR("Error restoring audio configuration: %d", r);
+			hdmi.audio_abort_cb(&hdmi.pdev->dev);
+			hdmi.audio_configured = false;
+		}
+	}
+
+	spin_lock_irqsave(&hdmi.audio_playing_lock, flags);
+	if (hdmi.audio_configured && hdmi.audio_playing) {
+		hdmi_wp_audio_enable(&hdmi.wp, true);
+		hdmi4_audio_start(&hdmi.core, &hdmi.wp);
+	}
 	hdmi.display_enabled = true;
+	spin_unlock_irqrestore(&hdmi.audio_playing_lock, flags);
 
 	mutex_unlock(&hdmi.lock);
 	return 0;
@@ -354,17 +374,18 @@  err0:
 
 static void hdmi_display_disable(struct omap_dss_device *dssdev)
 {
+	unsigned long flags;
+
 	DSSDBG("Enter hdmi_display_disable\n");
 
 	mutex_lock(&hdmi.lock);
 
-	if (hdmi.audio_pdev && hdmi.audio_abort_cb)
-		hdmi.audio_abort_cb(&hdmi.audio_pdev->dev);
+	spin_lock_irqsave(&hdmi.audio_playing_lock, flags);
+	hdmi.display_enabled = false;
+	spin_unlock_irqrestore(&hdmi.audio_playing_lock, flags);
 
 	hdmi_power_off_full(dssdev);
 
-	hdmi.display_enabled = false;
-
 	mutex_unlock(&hdmi.lock);
 }
 
@@ -565,9 +586,14 @@  out:
 static int hdmi_audio_shutdown(struct device *dev)
 {
 	struct omap_hdmi *hd = dev_get_drvdata(dev);
+	unsigned long flags;
 
 	mutex_lock(&hd->lock);
 	hd->audio_abort_cb = NULL;
+	hd->audio_configured = false;
+	spin_lock_irqsave(&hd->audio_playing_lock, flags);
+	hd->audio_playing = false;
+	spin_unlock_irqrestore(&hd->audio_playing_lock, flags);
 	mutex_unlock(&hd->lock);
 
 	return 0;
@@ -576,25 +602,38 @@  static int hdmi_audio_shutdown(struct device *dev)
 static int hdmi_audio_start(struct device *dev)
 {
 	struct omap_hdmi *hd = dev_get_drvdata(dev);
+	unsigned long flags;
 
 	WARN_ON(!hdmi_mode_has_audio(&hd->cfg));
-	WARN_ON(!hd->display_enabled);
 
-	hdmi_wp_audio_enable(&hd->wp, true);
-	hdmi4_audio_start(&hd->core, &hd->wp);
+	spin_lock_irqsave(&hd->audio_playing_lock, flags);
+
+	if (hd->display_enabled) {
+		hdmi_wp_audio_enable(&hd->wp, true);
+		hdmi4_audio_start(&hd->core, &hd->wp);
+	}
+	hd->audio_playing = true;
 
+	spin_unlock_irqrestore(&hd->audio_playing_lock, flags);
 	return 0;
 }
 
 static void hdmi_audio_stop(struct device *dev)
 {
 	struct omap_hdmi *hd = dev_get_drvdata(dev);
+	unsigned long flags;
 
 	WARN_ON(!hdmi_mode_has_audio(&hd->cfg));
-	WARN_ON(!hd->display_enabled);
 
-	hdmi4_audio_stop(&hd->core, &hd->wp);
-	hdmi_wp_audio_enable(&hd->wp, false);
+	spin_lock_irqsave(&hd->audio_playing_lock, flags);
+
+	if (hd->display_enabled) {
+		hdmi4_audio_stop(&hd->core, &hd->wp);
+		hdmi_wp_audio_enable(&hd->wp, false);
+	}
+	hd->audio_playing = false;
+
+	spin_unlock_irqrestore(&hd->audio_playing_lock, flags);
 }
 
 static int hdmi_audio_config(struct device *dev,
@@ -612,7 +651,10 @@  static int hdmi_audio_config(struct device *dev,
 
 	ret = hdmi4_audio_config(&hd->core, &hd->wp, dss_audio,
 				 hd->cfg.timings.pixelclock);
-
+	if (!ret) {
+		hd->audio_configured = true;
+		hd->audio_config = *dss_audio;
+	}
 out:
 	mutex_unlock(&hd->lock);
 
@@ -657,6 +699,7 @@  static int hdmi4_bind(struct device *dev, struct device *master, void *data)
 	dev_set_drvdata(&pdev->dev, &hdmi);
 
 	mutex_init(&hdmi.lock);
+	spin_lock_init(&hdmi.audio_playing_lock);
 
 	if (pdev->dev.of_node) {
 		r = hdmi_probe_of(pdev);
diff --git a/drivers/video/fbdev/omap2/dss/hdmi5.c b/drivers/video/fbdev/omap2/dss/hdmi5.c
index 7f87578..f352c4b 100644
--- a/drivers/video/fbdev/omap2/dss/hdmi5.c
+++ b/drivers/video/fbdev/omap2/dss/hdmi5.c
@@ -352,6 +352,7 @@  static int read_edid(u8 *buf, int len)
 static int hdmi_display_enable(struct omap_dss_device *dssdev)
 {
 	struct omap_dss_device *out = &hdmi.output;
+	unsigned long flags;
 	int r = 0;
 
 	DSSDBG("ENTER hdmi_display_enable\n");
@@ -370,7 +371,37 @@  static int hdmi_display_enable(struct omap_dss_device *dssdev)
 		goto err0;
 	}
 
+	if (hdmi.audio_configured) {
+		spin_lock_irqsave(&hdmi.audio_playing_lock, flags);
+		hdmi_wp_audio_core_req_enable(&hdmi.wp, false);
+		hdmi_wp_audio_enable(&hdmi.wp, false);
+		if (hdmi.wp_idlemode > 0)
+			REG_FLD_MOD(hdmi.wp.base, HDMI_WP_SYSCONFIG,
+				    hdmi.wp_idlemode, 3, 2);
+		hdmi.wp_idlemode = -1;
+		spin_unlock_irqrestore(&hdmi.audio_playing_lock, flags);
+
+		r = hdmi5_audio_config(&hdmi.core, &hdmi.wp, &hdmi.audio_config,
+				       hdmi.cfg.timings.pixelclock);
+		if (r) {
+			DSSERR("Error restoring audio configuration: %d", r);
+			hdmi.audio_abort_cb(&hdmi.pdev->dev);
+			hdmi.audio_configured = false;
+		}
+	}
+
+	spin_lock_irqsave(&hdmi.audio_playing_lock, flags);
+	if (hdmi.audio_configured && hdmi.audio_playing) {
+		/* No-idle while playing audio, store the old value */
+		hdmi.wp_idlemode =
+			REG_GET(hdmi.wp.base, HDMI_WP_SYSCONFIG, 3, 2);
+		REG_FLD_MOD(hdmi.wp.base, HDMI_WP_SYSCONFIG, 1, 3, 2);
+
+		hdmi_wp_audio_enable(&hdmi.wp, true);
+		hdmi_wp_audio_core_req_enable(&hdmi.wp, true);
+	}
 	hdmi.display_enabled = true;
+	spin_unlock_irqrestore(&hdmi.audio_playing_lock, flags);
 
 	mutex_unlock(&hdmi.lock);
 	return 0;
@@ -382,17 +413,18 @@  err0:
 
 static void hdmi_display_disable(struct omap_dss_device *dssdev)
 {
+	unsigned long flags;
+
 	DSSDBG("Enter hdmi_display_disable\n");
 
 	mutex_lock(&hdmi.lock);
 
-	if (hdmi.audio_pdev && hdmi.audio_abort_cb)
-		hdmi.audio_abort_cb(&hdmi.audio_pdev->dev);
+	spin_lock_irqsave(&hdmi.audio_playing_lock, flags);
+	hdmi.display_enabled = false;
+	spin_unlock_irqrestore(&hdmi.audio_playing_lock, flags);
 
 	hdmi_power_off_full(dssdev);
 
-	hdmi.display_enabled = false;
-
 	mutex_unlock(&hdmi.lock);
 }
 
@@ -593,9 +625,14 @@  out:
 static int hdmi_audio_shutdown(struct device *dev)
 {
 	struct omap_hdmi *hd = dev_get_drvdata(dev);
+	unsigned long flags;
 
 	mutex_lock(&hd->lock);
 	hd->audio_abort_cb = NULL;
+	hd->audio_configured = false;
+	spin_lock_irqsave(&hd->audio_playing_lock, flags);
+	hd->audio_playing = false;
+	spin_unlock_irqrestore(&hd->audio_playing_lock, flags);
 	mutex_unlock(&hd->lock);
 
 	return 0;
@@ -604,32 +641,48 @@  static int hdmi_audio_shutdown(struct device *dev)
 static int hdmi_audio_start(struct device *dev)
 {
 	struct omap_hdmi *hd = dev_get_drvdata(dev);
+	unsigned long flags;
 
 	WARN_ON(!hdmi_mode_has_audio(&hd->cfg));
-	WARN_ON(!hd->display_enabled);
 
-	/* No-idle while playing audio, store the old value */
-	hd->wp_idlemode = REG_GET(hdmi.wp.base, HDMI_WP_SYSCONFIG, 3, 2);
-	REG_FLD_MOD(hdmi.wp.base, HDMI_WP_SYSCONFIG, 1, 3, 2);
+	spin_lock_irqsave(&hd->audio_playing_lock, flags);
 
-	hdmi_wp_audio_enable(&hd->wp, true);
-	hdmi_wp_audio_core_req_enable(&hd->wp, true);
+	if (hd->display_enabled) {
+		/* No-idle while playing audio, store the old value */
+		hd->wp_idlemode =
+			REG_GET(hdmi.wp.base, HDMI_WP_SYSCONFIG, 3, 2);
+		REG_FLD_MOD(hdmi.wp.base, HDMI_WP_SYSCONFIG, 1, 3, 2);
+
+		hdmi_wp_audio_enable(&hd->wp, true);
+		hdmi_wp_audio_core_req_enable(&hd->wp, true);
+	}
+	hd->audio_playing = true;
 
+	spin_unlock_irqrestore(&hd->audio_playing_lock, flags);
 	return 0;
 }
 
 static void hdmi_audio_stop(struct device *dev)
 {
 	struct omap_hdmi *hd = dev_get_drvdata(dev);
+	unsigned long flags;
 
 	WARN_ON(!hdmi_mode_has_audio(&hd->cfg));
-	WARN_ON(!hd->display_enabled);
 
-	hdmi_wp_audio_core_req_enable(&hd->wp, false);
-	hdmi_wp_audio_enable(&hd->wp, false);
+	spin_lock_irqsave(&hd->audio_playing_lock, flags);
 
-	/* Playback stopped, restore original idlemode */
-	REG_FLD_MOD(hdmi.wp.base, HDMI_WP_SYSCONFIG, hd->wp_idlemode, 3, 2);
+	if (hd->display_enabled) {
+		hdmi_wp_audio_core_req_enable(&hd->wp, false);
+		hdmi_wp_audio_enable(&hd->wp, false);
+
+		/* Playback stopped, restore original idlemode */
+		REG_FLD_MOD(hdmi.wp.base, HDMI_WP_SYSCONFIG, hd->wp_idlemode,
+			    3, 2);
+		hd->wp_idlemode = -1;
+	}
+	hd->audio_playing = false;
+
+	spin_unlock_irqrestore(&hd->audio_playing_lock, flags);
 }
 
 static int hdmi_audio_config(struct device *dev,
@@ -648,6 +701,10 @@  static int hdmi_audio_config(struct device *dev,
 	ret = hdmi5_audio_config(&hd->core, &hd->wp, dss_audio,
 				 hd->cfg.timings.pixelclock);
 
+	if (!ret) {
+		hd->audio_configured = true;
+		hd->audio_config = *dss_audio;
+	}
 out:
 	mutex_unlock(&hd->lock);
 
@@ -692,6 +749,8 @@  static int hdmi5_bind(struct device *dev, struct device *master, void *data)
 	dev_set_drvdata(&pdev->dev, &hdmi);
 
 	mutex_init(&hdmi.lock);
+	spin_lock_init(&hdmi.audio_playing_lock);
+	hdmi.wp_idlemode = -1;
 
 	if (pdev->dev.of_node) {
 		r = hdmi_probe_of(pdev);