Skip to content

ASoC: SOF: intel: hda-stream: Print stream name on STREAM_SD_OFFSET t… - #3121

Merged
plbossart merged 1 commit into
thesofproject:topic/sof-devfrom
ujfalusi:peter/sof/pr/hda_STREAM_SD_OFFSET_error
Aug 27, 2021
Merged

ASoC: SOF: intel: hda-stream: Print stream name on STREAM_SD_OFFSET t…#3121
plbossart merged 1 commit into
thesofproject:topic/sof-devfrom
ujfalusi:peter/sof/pr/hda_STREAM_SD_OFFSET_error

Conversation

@ujfalusi

Copy link
Copy Markdown
Collaborator

…imeout

In order to provide more information in case of timeout observed while
reading STREAM_SD_OFFSET, print out the stream name or in case there is
no audio stream associated (like dma-trace), print "--"

Signed-off-by: Peter Ujfalusi peter.ujfalusi@linux.intel.com

@ujfalusi

Copy link
Copy Markdown
CollaboratorAuthor

@plbossart, @lgirdwood: FYI. Is this something can be of use as mentioned in thesofproject/sof#4548?

@lgirdwoodlgirdwood left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks, this is useful to help root causing the stream DMA stop offset timeout.

Comment threadsound/soc/sof/intel/hda-stream.c Outdated
if (ret < 0) {
dev_err(sdev->dev,
"%s: cmd %d on substream %s: timeout on STREAM_SD_OFFSET read\n",
__func__, cmd, hstream->substream ? hstream->substream->name : "--");

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Would it be correct to assume that only the trace and or probes would not have a substream name ? if so, could we asign then a name as part of their open() ?

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

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

neither the probes, dma-trace or the real compressed would have substream. We don't have place to assign custom name, let me try something...

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

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

We could print out the stream_tag which is always unique

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

ok, stream_tag is in the FW logs so could be aligned with the dmesg and would work for debug.

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

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

Let me try to squeeze it there. We are 90 char already which is the limit I rather not step, but under 100 is still OK for checkpatch

@ujfalusi
ujfalusiforce-pushed the peter/sof/pr/hda_STREAM_SD_OFFSET_error branch from ee77a51 to 27b016eCompareAugust 26, 2021 14:01
@ujfalusi

Copy link
Copy Markdown
CollaboratorAuthor

Changes since v1:

  • helper function to get the stream name (hda_hstream_get_stream_name)
  • return either the substream->name or the cstream->name or NULL

@lgirdwoodlgirdwood left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, stream_tag would be a bonus.

@ujfalusi
ujfalusiforce-pushed the peter/sof/pr/hda_STREAM_SD_OFFSET_error branch from 27b016e to fb4185eCompareAugust 26, 2021 14:20
@ujfalusi

ujfalusi commented Aug 26, 2021

Copy link
Copy Markdown
CollaboratorAuthor

Changes since v2:

  • print the stream_tag as well as it is unique for each stream, regardless of type (audio/compressed/dtrace)
  • replace substream %s with stream %s in the prints to save three characters, now the longest line is at 92 char

@ujfalusi
ujfalusiforce-pushed the peter/sof/pr/hda_STREAM_SD_OFFSET_error branch from fb4185e to c21ae64CompareAugust 26, 2021 14:23
plbossart
plbossart previously approved these changes Aug 26, 2021
ranj063
ranj063 previously approved these changes Aug 26, 2021

@ranj063ranj063 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM @ujfalusi. out of curiosity what does stream name print? Usually, I can find out which streamis it my seeing the PCM device number and the direction. Just wondering what this would show

lgirdwood
lgirdwood previously approved these changes Aug 26, 2021
@ujfalusi

Copy link
Copy Markdown
CollaboratorAuthor

@ranj063,
dma-trace: hda_dsp_stream_trigger: cmd 1 on stream -- (1): timeout on STREAM_SD_OFFSET read
aplay -Dplughw:0,0: hda_dsp_stream_trigger: cmd 1 on stream subdevice #0 (1): timeout on STREAM_SD_OFFSET read

on a tgl-h laptop.

@ranj063

ranj063 commented Aug 26, 2021

Copy link
Copy Markdown
Collaborator

@ranj063,
dma-trace: hda_dsp_stream_trigger: cmd 1 on stream -- (1): timeout on STREAM_SD_OFFSET read
aplay -Dplughw:0,0: hda_dsp_stream_trigger: cmd 1 on stream subdevice #0 (1): timeout on STREAM_SD_OFFSET read

on a tgl-h laptop.

@ujfalusi Thanks. I wonder if it then makes sense to also print the direction? subdevice #0 ?

@plbossart

Copy link
Copy Markdown
Member

@ranj063,
dma-trace: hda_dsp_stream_trigger: cmd 1 on stream -- (1): timeout on STREAM_SD_OFFSET read
aplay -Dplughw:0,0: hda_dsp_stream_trigger: cmd 1 on stream subdevice #0 (1): timeout on STREAM_SD_OFFSET read
on a tgl-h laptop.

@ujfalusi Thanks. I wonder if it then makes sense to also print the direction? subdevice #0 ?

I tried to use the stream name for SoundWire stuff and all I could get was a bunch of 'subdevice #0' and 'subdevice #1'. Not very helpful when you have multiple dailinks. I ended-up adding the dai name to at least know which stream this really was.

@ujfalusi

Copy link
Copy Markdown
CollaboratorAuthor

@ranj063,
dma-trace: hda_dsp_stream_trigger: cmd 1 on stream -- (1): timeout on STREAM_SD_OFFSET read
aplay -Dplughw:0,0: hda_dsp_stream_trigger: cmd 1 on stream subdevice #0 (1): timeout on STREAM_SD_OFFSET read
on a tgl-h laptop.

@ujfalusi Thanks. I wonder if it then makes sense to also print the direction? subdevice #0 ?

I tried to use the stream name for SoundWire stuff and all I could get was a bunch of 'subdevice #0' and 'subdevice #1'. Not very helpful when you have multiple dailinks. I ended-up adding the dai name to at least know which stream this really was.

@plbossart, @ranj063, or if we can reach the dai-link name for the given stream? Looking at the prints the stream_tag is not the most useful information I have seen, but when multiple streams are running they should give some pointer.
I'll check tomorrow if we can swap the substream->name with something more helpful.

@ujfalusi
ujfalusi dismissed stale reviews from lgirdwood, ranj063, and plbossart via b0846b5August 27, 2021 09:13
@ujfalusi
ujfalusiforce-pushed the peter/sof/pr/hda_STREAM_SD_OFFSET_error branch from c21ae64 to b0846b5CompareAugust 27, 2021 09:13
@ujfalusi

Copy link
Copy Markdown
CollaboratorAuthor

Changes since v3:

  • the printed information contains more and more accurate information

Tested on top of SOF clients branch to have working probes and the prints are:
dma-trace
hda_dsp_stream_trigger: cmd 1 on -- (Capture, stream_tag: 1): timeout on STREAM_SD_OFFSET read

aplay -Dplughw:1,0
hda_dsp_stream_trigger: cmd 1 on dai_link "HDA Analog" (Playback, stream_tag: 1): timeout on STREAM_SD_OFFSET read

probes crecord
hda_dsp_stream_trigger: cmd 1 on dai_link "Compress Probe Capture" (Capture, stream_tag: 2): timeout on STREAM_SD_OFFSET read

Comment threadsound/soc/sof/intel/hda-stream.c
@ujfalusi

Copy link
Copy Markdown
CollaboratorAuthor

Changes since v4:

  • Add comment for non audio user case in hda_hstream_dbg_get_stream_info_str()
  • remove the dai_link casting, simply use rtd->dai_link->name

@ujfalusi
ujfalusiforce-pushed the peter/sof/pr/hda_STREAM_SD_OFFSET_error branch from b0846b5 to 666c187CompareAugust 27, 2021 13:12
…imeout
In order to provide more information in case of timeout observed while
reading STREAM_SD_OFFSET, print out the stream name or in case there is
no audio stream associated (like dma-trace), print "--"
Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>

@ranj063ranj063 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks @ujfalusi. this is quite nice!

@plbossart
plbossart merged commit b324ced into thesofproject:topic/sof-devAug 27, 2021
Sign up for freeto 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.

4 participants

@ujfalusi@ranj063@plbossart@lgirdwood