Skip to content

Conversation

@RDharageswari
Copy link

Speaker amplifier feedback is not modeled as being dependent on any
active output. Even when there is no playback happening, parts of the graph,
specifically the IV sense->speaker protection->output remains active and
this prevents the DSP from entering low-power states.

This patch suggest a machine driver level approach where the speaker
pins are enabled/disabled dynamically depending on stream start/stop
events. DPAM graph representations show the feedback loop is indeed
disabled and low-power states can be reached.

Copy link
Author

@RDharageswari RDharageswari left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixing the errors

@RDharageswari RDharageswari force-pushed the sof_dev_15_05 branch 2 times, most recently from fbccb2b to 43bdcd2 Compare May 16, 2020 00:22
Copy link
Member

@plbossart plbossart left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good start but this needs more work.

case SNDRV_PCM_TRIGGER_STOP:
case SNDRV_PCM_TRIGGER_SUSPEND:
case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
val = snd_soc_dai_active(codec_dai);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you dont seem to use val at all?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@RDharageswari RDharageswari force-pushed the sof_dev_15_05 branch 2 times, most recently from 73f1579 to c2759d5 Compare May 28, 2020 00:34
ranj063
ranj063 previously approved these changes May 28, 2020
Copy link
Collaborator

@ranj063 ranj063 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks @RDharageswari

plbossart
plbossart previously approved these changes May 28, 2020
Copy link
Member

@plbossart plbossart left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we ever want to work with a single amplifier the code will need to be updated, see below. It's not hypothetical, see ASoC: Intel: sof_da7219_max98373: Add support for max98360a speaker amp

Speaker amplifier feedback is not modeled as being dependent on any
active output. Even when there is no playback happening, parts of the
graph, specifically the IV sense->speaker protection->output remains
active and this prevents the DSP from entering low-power states.

This patch suggests a machine driver level approach where the speaker
pins are enabled/disabled dynamically depending on stream start/stop
events. DPAM graph representations show the feedback loop is indeed
disabled and low-power states can be reached.

Signed-off-by: Dharageswari R <dharageswari.r@intel.com>
@RDharageswari RDharageswari dismissed stale reviews from plbossart and ranj063 via 8947163 May 29, 2020 19:52
ranj063
ranj063 previously approved these changes May 29, 2020
@RDharageswari RDharageswari requested a review from plbossart May 29, 2020 20:49
…onents

MAX_98373_DEV0_NAME is the Right speaker and MAX_98373_DEV1_NAME is the
Left speaker, hence updating the comments for max98373 dailink components
accordingly.

Signed-off-by: Dharageswari R <dharageswari.r@intel.com>
@plbossart plbossart merged commit 16d98cd into topic/sof-dev Jun 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants