OMAPDSS: Couple of HDMI audio fixes

The first fix is a hairy one, but I think the locking is fool proof now. It is needed because there is no telling in which order user space starts an audio and video stream playback. If the audio is started first and the video mode is changed when video playback starts the audio setup needs to survive display turning off and back on again. After this patch the audio streams should survive a suspend-resume cycle too.

The second one is a trivial work-around to a problem in ALSA constraint resolver code.

The two fixes are totally independent and the video and audio side patches applied separately trough their own trees.

Jyri Sarha (2):
OMAPDSS: hdmi: Reconfigure and restart audio when display is enabled
ASoC: omap-hdmi-audio: Set buffer bytes step constraint to 128

 drivers/video/fbdev/omap2/dss/hdmi.h  | 10 +++-
 drivers/video/fbdev/omap2/dss/hdmi4.c | 65 ++++++++++++++++++++-----
 drivers/video/fbdev/omap2/dss/hdmi5.c | 89 +++++++++++++++++++++++++++++------
 sound/soc/omap/omap-hdmi-audio.c      | 10 +++-
 4 files changed, 146 insertions(+), 28 deletions(-)

1.9.1

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.

---
 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(-)

index e4a32fe..1e45b84 100644
@@ -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;
 };
 

1.9.1

Set buffer bytes step constraint to 128. A matching constraint has already been set to period size. This helps PCM setup to tolerate ALSA clients that set the PCM hw params in unusual order.

---
 sound/soc/omap/omap-hdmi-audio.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

index aeef25c..584b237 100644
@@ -81,7 +81,15 @@ static int hdmi_dai_startup(struct snd_pcm_substream *substream,
 	ret = snd_pcm_hw_constraint_step(substream->runtime, 0,
 					 SNDRV_PCM_HW_PARAM_PERIOD_BYTES, 128);
 	if (ret < 0) {
-		dev_err(dai->dev, "could not apply constraint\n");
+		dev_err(dai->dev, "Could not apply period constraint: %d\n",
+			ret);
+		return ret;
+	}
+	ret = snd_pcm_hw_constraint_step(substream->runtime, 0,
+					 SNDRV_PCM_HW_PARAM_BUFFER_BYTES, 128);
+	if (ret < 0) {
+		dev_err(dai->dev, "Could not apply buffer constraint: %d\n",
+			ret);
 		return ret;
 	}
 

1.9.1

In general if patches aren't related to each other either through code overlap or through content it's better to just send them as individual changes, that avoids any potential for confusion.

The patch

ASoC: omap-hdmi-audio: Set buffer bytes step constraint to 128

has been applied to the asoc tree at

git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git

All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying to this mail.

Thanks,
Mark

From: Jyri Sarha <[email protected]>
Date: Wed, 26 Aug 2015 16:11:40 +0300
Subject: [PATCH] ASoC: omap-hdmi-audio: Set buffer bytes step constraint to
128

Set buffer bytes step constraint to 128. A matching constraint has already been set to period size. This helps PCM setup to tolerate ALSA clients that set the PCM hw params in unusual order.

---
 sound/soc/omap/omap-hdmi-audio.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

index aeef25c0cb3d..584b2372339e 100644
@@ -81,7 +81,15 @@ static int hdmi_dai_startup(struct snd_pcm_substream *substream,
 	ret = snd_pcm_hw_constraint_step(substream->runtime, 0,
 					 SNDRV_PCM_HW_PARAM_PERIOD_BYTES, 128);
 	if (ret < 0) {
-		dev_err(dai->dev, "could not apply constraint\n");
+		dev_err(dai->dev, "Could not apply period constraint: %d\n",
+			ret);
+		return ret;
+	}
+	ret = snd_pcm_hw_constraint_step(substream->runtime, 0,
+					 SNDRV_PCM_HW_PARAM_BUFFER_BYTES, 128);
+	if (ret < 0) {
+		dev_err(dai->dev, "Could not apply buffer constraint: %d\n",
+			ret);
 		return ret;
 	}
 

2.5.0

Hi,

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

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

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