Skip to content

Conversation

@ranj063
Copy link
Collaborator

@ranj063 ranj063 commented Sep 17, 2021

Fixes thesofproject/sof#4558

But it should also fix thesofproject/sof#4779 and possibly even #3034

And if it does fix #3034, it is replacement for #3099

DMA trace has been stopped and released at this point, so
a wake_up() is not needed anymore.

Fixes: 2291b8c ("ASoC: SOF: force end-of-file for debugfs trace at suspend")

Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Trace must be stopped when the DSP suspends to a D0
substate. Move the call to snd_sof_release_trace()
so that it can be released for all D0 substates and
add the call to re-init trace when resuming from a
low-power D0 substate.

Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
When the stream is cleared during the suspend trigger,
the dma_data must be set to NULL and
snd_hdac_ext_stream_release() must be called to release
the link dev. Without this, some platforms run into
issues with triggering the host DMA during system resume.

Add the missing sequences to both hda_link_pcm_trigger() to
handle all streams that get suspended and to
hda_dsp_set_hw_params_upon_resume() to handle paused streams that
are reset during system suspend.

Also, because the dma_data is set to NULL during suspend,
add the checks to ensure link_dev is not NULL during
hw_params and hw_free to prevent NULL pointer dereferences.

BugLink: thesofproject/sof#4779
Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Copy link
Collaborator

@kv2019i kv2019i left a comment

Choose a reason for hiding this comment

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

Partial review, commenting on the first patch.

sdev->dtrace_is_enabled = false;
sdev->dtrace_draining = true;
wake_up(&sdev->trace_sleep);
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hmm, how can you be sure the user-space thread is not in sof_wait_trace_avail() ? The release trace can be called when sof-logger is still running and waiting for traces.

snd_soc_dai_get_dma_data(dai, substream);
struct snd_sof_dev *sdev =
snd_soc_component_get_drvdata(dai->component);
struct snd_soc_pcm_runtime *rtd = asoc_substream_to_rtd(substream);
Copy link
Collaborator

Choose a reason for hiding this comment

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

typo in commit: dma_sata in subject

@plbossart
Copy link
Member

@ranj063 I think there should be two separate PRs here
a) your last commit that should apply to all platforms
b) S0ix-related changes - on which I agree with @kv2019i that the fix is controversial.

I don't think we should bundle the two parts, it's better for validation if we split, no?

plbossart added a commit to plbossart/sof that referenced this pull request Sep 20, 2021
This reverts commit 9fadef7.

After multiple trials on a CometLake SoundWire device, this revert to
bring the trace back to what it was seems to be the only solution, the
suggested PR thesofproject/linux#3166 does not
help on this SoundWire device.

We had similar issues with SD offset timeouts and a similar revert
with thesofproject#4578 at the end of
July, there's something that we are missing on what the trace does and
how it impacts the DMA handling.

BugLink: thesofproject#4779
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
@ranj063
Copy link
Collaborator Author

ranj063 commented Sep 21, 2021

to be replaced with a better fix

@ranj063 ranj063 closed this Sep 21, 2021
lgirdwood pushed a commit to thesofproject/sof that referenced this pull request Sep 21, 2021
This reverts commit 9fadef7.

After multiple trials on a CometLake SoundWire device, this revert to
bring the trace back to what it was seems to be the only solution, the
suggested PR thesofproject/linux#3166 does not
help on this SoundWire device.

We had similar issues with SD offset timeouts and a similar revert
with #4578 at the end of
July, there's something that we are missing on what the trace does and
how it impacts the DMA handling.

BugLink: #4779
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
@ranj063 ranj063 deleted the fix/trace_release branch October 19, 2021 17:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

3 participants