Skip to content

Conversation

@fredoh9
Copy link
Collaborator

@fredoh9 fredoh9 commented Oct 31, 2020

Updated Nov 2,
First 3 commits are from PR 2297. Last a commit is for ICL ICCMAX support change.

Updated Nov 3,
First 3 commits are from PR 2297. Last four commits are for ICL ICCMAX support change.

For last commit, for explicit header inclusion. This is the right header required. This is valid style compliance fix per Documentation/process/submit-checklist.rst.

#include <linux/kernel.h>	// ARRAY_SIZE
#include <linux/kconfig.h>	// IS_ENABLED, CONFIG_
#include <linux/export.h>	// EXPORT_SYMBOL_
#include <linux/bits.h>		// GENMASK, BIT

#include <linux/module.h>	// MODULE_

Updated Nov 4,
Dropped symatrical changes for other dsp interface.
Re-ordered and combining commits. Now last two are ICL ICCMAX change.

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.

Thanks @fredoh9 ! Could we run some stress tests whether this PR helps with thesofproject/sof#3395 ? It would be really useful info and help to prioritize this PR.

@fredoh9 fredoh9 changed the title [WIP] ICL ICCMAX support ICL ICCMAX support Nov 2, 2020
@fredoh9
Copy link
Collaborator Author

fredoh9 commented Nov 2, 2020

Thanks @fredoh9 ! Could we run some stress tests whether this PR helps with thesofproject/sof#3395 ? It would be really useful info and help to prioritize this PR.

I will run stress-test with this PR. I'm also curious about the result.

@fredoh9
Copy link
Collaborator Author

fredoh9 commented Nov 3, 2020

I'm able to reproduce same error with the DEVICE TEST failure on ICL RVP. Looking at it.

jf-icl-rvp-hda-4 kernel: [    5.405310] sof-audio-pci 0000:00:1f.3: error: ipc timed out for 0x90030000 size 80
jf-icl-rvp-hda-4 kernel: [    5.405314] sof-audio-pci 0000:00:1f.3: info: preventing DSP entering D3 state to preserve context
jf-icl-rvp-hda-4 kernel: [    5.405335] sof-audio-pci 0000:00:1f.3: status: fw entered - code 00000005
jf-icl-rvp-hda-4 kernel: [    5.405512] sof-audio-pci 0000:00:1f.3: error: unexpected fault 0x00000000 trace 0x00004000
jf-icl-rvp-hda-4 kernel: [    5.405521] sof-audio-pci 0000:00:1f.3: error: hda irq intsts 0x00000000 intlctl 0xc0000000 rirb 00
jf-icl-rvp-hda-4 kernel: [    5.405522] sof-audio-pci 0000:00:1f.3: error: dsp irq ppsts 0x00000000 adspis 0x00000000
jf-icl-rvp-hda-4 kernel: [    5.405529] sof-audio-pci 0000:00:1f.3: error: host status 0x00000000 dsp status 0x00000000 mask 0x00000003
jf-icl-rvp-hda-4 kernel: [    5.405530] sof-audio-pci 0000:00:1f.3: error: can't set params for DMA for trace -110
jf-icl-rvp-hda-4 kernel: [    5.405556] sof-audio-pci 0000:00:1f.3: warning: failed to initialize trace -110

@fredoh9 fredoh9 force-pushed the fix/icl_iccmax branch 2 times, most recently from d610910 to ab2ec32 Compare November 5, 2020 23:34
ranj063
ranj063 previously approved these changes Nov 6, 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.

LGTM. Thanks @fredoh9 for being patient.

abonislawski
abonislawski previously approved these changes Nov 6, 2020
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.

Thanks @fredoh9 ! This indeed starts to look. I'll leave a bit more time to double-check before giving my approval. I left a few notes inline already now.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Btw, @ranj063 and @fredoh9 -- while ICCMAX is somewhat known as a concept, in this case this is a very specific and non-obvious quirk of our DSP bootflow. I won't complain now as the same text is already merged for TGL in upstream, but the commit message is not very helpful for people outside SOF project. But yeah, I'm ok to go with this The bit about "recommended hw programming sequence" is key and it's there...

@fredoh9 fredoh9 dismissed stale reviews from abonislawski and ranj063 via 15edbec November 7, 2020 01:25
@fredoh9 fredoh9 requested a review from kv2019i November 7, 2020 01:26
@fredoh9
Copy link
Collaborator Author

fredoh9 commented Nov 10, 2020

@ranj063 I lost your approval after addressing Kai's comment.
@plbossart @kv2019i do you have any comment or concern?

ranj063
ranj063 previously approved these changes Nov 10, 2020
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.

Thanks @fredoh9 ! Code looks good now and seems to align as expected with merged FW.

I do not three warnings about lines over 100 columns in checkpatch. Please address these.

@fredoh9
Copy link
Collaborator Author

fredoh9 commented Nov 11, 2020

I do not three warnings about lines over 100 columns in checkpatch. Please address these.

Sure, I will address that now!

Add parse_platform_ext_manifest() op to parse platform-specific config
data in the extended manifest.

Signed-off-by: Fred Oh <fred.oh@linux.intel.com>
Define the parse_platform_ext_manifest() op for HDA platforms to parse
the SOF_EXT_MAN_CAVS_CONFIG_CAVS_LPRO config item to determine if the FW
is configured for LPRO. The default clock configuration is assumed to be
HPRO in the absence of this item in the extended manifest.
New member clk_config_lpro is added to struct sof_intel_hda_dev to store
the FW clock config information and that this will be used later to perform
platform-specific operations in the post_fw_run op.

Signed-off-by: Fred Oh <fred.oh@linux.intel.com>
OUTBOX_SIZE, INBOX_SIZE are defined but not being used yet. Handle
these elements to avoid warning about unknown token type.

Signed-off-by: Fred Oh <fred.oh@linux.intel.com>
Modify the signature of stall op to specify core_mask to stall cores.

Signed-off-by: Fred Oh <fred.oh@linux.intel.com>
Separate the dsp ops for ICL ops to specify the use of ICCMAX
FW boot sequence in the run op. All other ops are identical with TGL
except post_fw_run. The recommended HW programming sequence for ICL
is to power up core 3 and keep it in stall if HPRO is enabled.

Signed-off-by: Fred Oh <fred.oh@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.

Thanks @fredoh9 !

@kv2019i
Copy link
Collaborator

kv2019i commented Nov 13, 2020

Ping @plbossart , you still have changes requested. Otherwise, I'm ready to press merge on this.

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.

@kv2019i I don't know what this fixes or improves, and the code looks good enough so I'll let you merge if you think it's fine.

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.

8 participants