[alsa-devel] [PATCH 0/2] 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(-)
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(-)
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);
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
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
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
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.
Signed-off-by: Jyri Sarha jsarha@ti.com --- sound/soc/omap/omap-hdmi-audio.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/sound/soc/omap/omap-hdmi-audio.c b/sound/soc/omap/omap-hdmi-audio.c index aeef25c..584b237 100644 --- a/sound/soc/omap/omap-hdmi-audio.c +++ b/sound/soc/omap/omap-hdmi-audio.c @@ -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; }
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 7d40acc38be55abb095f517e4e3a634818bc5253 Mon Sep 17 00:00:00 2001
From: Jyri Sarha jsarha@ti.com 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.
Signed-off-by: Jyri Sarha jsarha@ti.com Signed-off-by: Mark Brown broonie@kernel.org --- sound/soc/omap/omap-hdmi-audio.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/sound/soc/omap/omap-hdmi-audio.c b/sound/soc/omap/omap-hdmi-audio.c index aeef25c0cb3d..584b2372339e 100644 --- a/sound/soc/omap/omap-hdmi-audio.c +++ b/sound/soc/omap/omap-hdmi-audio.c @@ -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; }
On Wed, Aug 26, 2015 at 04:11:38PM +0300, Jyri Sarha wrote:
The two fixes are totally independent and the video and audio side patches applied separately trough their own trees.
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.
participants (3)
-
Jyri Sarha
-
Mark Brown
-
Tomi Valkeinen