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

driver: ssp: update Intel SSP DAI driver to support dynamic SSP link management #70660

Merged
merged 1 commit into from
Apr 25, 2024

Conversation

dnikodem
Copy link
Collaborator

@dnikodem dnikodem commented Mar 25, 2024

This commit refactors the Intel SSP DAI driver to support dynamic management of SSP links. The changes include the introduction of a new structure intel_ssp_link to encapsulate SSP link-specific data.

Key changes:

  • Add new static functions to manage SSP link power.
  • Update the DAI SSP configuration functions to use the new link management approach.
  • Update device tree bindings and instances to reflect the new SSP link management mechanism.

The new approach allows for more flexible and maintainable management of SSP links.

Related, necessary changes:

@dnikodem dnikodem added the DNM This PR should not be merged (Do Not Merge) label Mar 25, 2024
@dnikodem dnikodem force-pushed the update_ssp_driver_flow branch 2 times, most recently from 9a28be8 to 4d1f412 Compare March 25, 2024 14:08
@dnikodem dnikodem force-pushed the update_ssp_driver_flow branch 3 times, most recently from a9113b3 to e90b5c7 Compare March 28, 2024 15:10
@dnikodem dnikodem marked this pull request as ready for review April 2, 2024 10:31
@zephyrbot zephyrbot added area: I2S area: Xtensa Xtensa Architecture area: DAI platform: Intel ADSP Intel Audio platforms area: Devicetree Binding PR modifies or adds a Device Tree binding labels Apr 2, 2024
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.

@dnikodem can you ack this works with SOF main?

LOG_INF("plat_data base: %u", dp->plat_data.base);
LOG_INF("plat_data irq: %u", dp->plat_data.irq);
LOG_INF("plat_data fifo playback offset: %u", dp->plat_data.fifo[DAI_DIR_PLAYBACK].offset);
LOG_INF("dai index: %u", dp->dai_index);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we display the link number in the logs somehow as it's a new piece of config data?

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.

Oops, missed one comment in dai_ssp_remove()

struct intel_ssp_link *ssp_link = dai_get_ssp_link(dp);

if (ssp_link_is_acquired(ssp_link)) {
k_free(dai_get_drvdata(dp));
Copy link
Collaborator

Choose a reason for hiding this comment

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

One more, can you explain the logic here? If the link is acquired, how come we proceed to free the "'dp' object in this case?

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.

Marking -1 for now for the remove handling.

@dnikodem dnikodem force-pushed the update_ssp_driver_flow branch 3 times, most recently from ff344f6 to 163eba8 Compare April 15, 2024 16:41
@wszypelt
Copy link

wszypelt commented Apr 17, 2024

@dnikodem @kv2019i Based on the SOF PR 8980, I checked the changes for MTL and LNL, in MTL everything works properly, in LNL I have to wait a while until all the tests are done, however, I can tell you that so far everything looks good. I will write again as soon as all tests are completed.

@wszypelt
Copy link

@dnikodem @kv2019i all LNL tests look good :)

kv2019i
kv2019i previously approved these changes Apr 18, 2024
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.

Tests in SOF (8980) seem good and my earlier comments are addressed, so I'm good. There' potential conflict with the dts removal extra ssp nodes, but this will be easy to adapt to.

dts/xtensa/intel/intel_adsp_ace15_mtpm.dtsi Outdated Show resolved Hide resolved
kv2019i
kv2019i previously approved these changes Apr 22, 2024
Copy link
Contributor

@iganakov iganakov 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 to me

@dnikodem dnikodem removed the DNM This PR should not be merged (Do Not Merge) label Apr 22, 2024
@kv2019i
Copy link
Collaborator

kv2019i commented Apr 23, 2024

@nashif This would be ready to go.

drivers/dai/intel/ssp/ssp.c Outdated Show resolved Hide resolved
drivers/dai/intel/ssp/ssp.c Outdated Show resolved Hide resolved
…ement

This commit refactors the Intel SSP DAI driver to support dynamic
management of SSP IP. This change additionally separates the management
of the DAI part from the management part of the SSP IP.

Key changes:
- Add new static functions to manage SSP IP power.
- Update the DAI SSP configuration functions to use the new management
  approach.
- Update device tree bindings and instances to reflect the new SSP IP
  management mechanism.

Signed-off-by: Damian Nikodem <damian.nikodem@intel.com>
@aescolar aescolar merged commit 2455436 into zephyrproject-rtos:main Apr 25, 2024
21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: DAI area: Devicetree Binding PR modifies or adds a Device Tree binding area: I2S area: Xtensa Xtensa Architecture platform: Intel ADSP Intel Audio platforms
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants