Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ASoC: SOF: ipc4-loader: remove the CPC check warnings #4753

Merged

Conversation

kv2019i
Copy link
Collaborator

@kv2019i kv2019i commented Dec 20, 2023

Warnings related to missing data in firmware manifest have
proven to be too verbose. This relates to description of
DSP module cost expressed in cycles per chunk (CPC). If
a matching value is not found in the manifest, kernel will
pass a zero value and DSP firmware will use a conservative
value in its place.

Downgrade the warnings to dev_dbg().

Fixes: d8a2c98 ("ASoC: SOF: ipc4-loader/topology: Query the CPC value from manifest")

Copy link
Member

@lgirdwood lgirdwood left a comment

Choose a reason for hiding this comment

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

One improvement if you need to do an update, is commit message to state that the existing kernel error text is also erroneous as FW will still function correctly but use the default DSP clock. i.e. fixes a bug and squashes spam

@kv2019i
Copy link
Collaborator Author

kv2019i commented Dec 20, 2023

Bug that prompted this PR: thesofproject/sof#8659

Warnings related to missing data in firmware manifest have
proven to be too verbose. This relates to description of
DSP module cost expressed in cycles per chunk (CPC). If
a matching value is not found in the manifest, kernel will
pass a zero value and DSP firmware will use a conservative
value in its place.

Downgrade the warnings to dev_dbg().

Fixes: d8a2c98 ("ASoC: SOF: ipc4-loader/topology: Query the CPC value from manifest")
Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
@kv2019i kv2019i force-pushed the 202312-shut-up-about-cpc branch from 39d061b to 073a86f Compare December 20, 2023 19:09
@kv2019i
Copy link
Collaborator Author

kv2019i commented Dec 20, 2023

V2 pushed:

  • amended the commit text to explain better why the warning was too scary (although to note: the old/existing kernel code and comments do not claim this to be an error. there's just a warning emitted with text "CPC not found" .. "warning" level is used, so that's really the only cue about the severity)

dev_dbg(sdev->dev, "%s (UUID: %pUL): %s (ibs/obs: %u/%u)\n",
fw_module->man4_module_entry.name,
&fw_module->man4_module_entry.uuid, msg, basecfg->ibs,
basecfg->obs);
Copy link
Collaborator

Choose a reason for hiding this comment

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

The reason why it ended up as dev_warn() + dev_warn_once() is to make sure it shows up in the logs and 'annoy' users that they will file a bug so the CPC value is fixed. Initially I had dev_dbg() as well.
Do we need to print it every time a stream starts? Would a dev_dbg_once() can suffice? Well, it is dev_dbg(), so only developers going to see it, right?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think the original intent still has merit. It's just we need to scale back on the "annoy dose" given the fixes come via quarterly SOF releases (plus adoption to upstream). You can add or modify a tplg at any time, but you can't get rid of the warnings without a new (typically signed) image.

@btian1
Copy link

btian1 commented Dec 21, 2023

@kv2019i ,if no matter sof change or kernel change both need after Chrismas release, please also consider: thesofproject/sof#8660
this is because if without these warnings, we don't know whether CPC missed.
We may need added it back once new CPC mechanism are finalized。

leave it may always have warnings, however, it can remind us there is CPC missing.

I mean, we have to add back once clock adjust fully applied to SOF.

@plbossart plbossart merged commit 28839a5 into thesofproject:topic/sof-dev Dec 21, 2023
12 of 14 checks passed
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