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

Make Intel IPC stream ops generic #3092

Merged
merged 3 commits into from
Aug 20, 2021

Conversation

Potochi
Copy link

@Potochi Potochi commented Aug 12, 2021

Operations implemented in sof/intel/intel-ipc.c should be generic and are of a good use to IMX also. So, make them generic.

This is a rebase of #2914.

sound/soc/sof/stream-ipc.c Outdated Show resolved Hide resolved
sound/soc/sof/core.c Outdated Show resolved Hide resolved
sound/soc/sof/sof-priv.h Show resolved Hide resolved
@Potochi Potochi force-pushed the generic_ipc branch 3 times, most recently from 6a5a311 to eff0d2f Compare August 17, 2021 11:58
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.

ARM compilation failure?

CC [M] sound/soc/fsl/fsl-asoc-card.o
sound/soc/sof/imx/imx8.c:388:12: error: ‘imx8_ipc_pcm_params’ defined but not used [-Werror=unused-function]
388 | static int imx8_ipc_pcm_params(struct snd_sof_dev *sdev,
| ^~~~~~~~~~~~~~~~~~~
sound/soc/sof/imx/imx8.c:380:12: error: ‘imx8_ipc_msg_data’ defined but not used [-Werror=unused-function]
380 | static int imx8_ipc_msg_data(struct snd_sof_dev *sdev,
| ^~~~~~~~~~~~~~~~~

@dbaluta
Copy link
Collaborator

dbaluta commented Aug 17, 2021

@Potochi can you have a look. Why this didn't fail on our tests? Can you check the .config file?

@Potochi Potochi force-pushed the generic_ipc branch 2 times, most recently from c60b59f to 2888f0f Compare August 17, 2021 13:57
@@ -310,6 +310,7 @@ int snd_sof_device_probe(struct device *dev, struct snd_sof_pdata *plat_data)
/* check all mandatory ops */
if (!sof_ops(sdev) || !sof_ops(sdev)->probe || !sof_ops(sdev)->run ||
!sof_ops(sdev)->block_read || !sof_ops(sdev)->block_write ||
!sof_ops(sdev)->mailbox_read || !sof_ops(sdev)->mailbox_write ||

Choose a reason for hiding this comment

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

AMD platform is not using mmaped IO for IPC so make it an optional ops() callback and not a mandatory one.

Copy link
Member

Choose a reason for hiding this comment

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

Not following @ajitkupandey. This patch precisely enables you to not use mmap IO but the mechanism of your choice.

Choose a reason for hiding this comment

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

yeah i saw we can implement our custom mailbox_read()/write() but we aren't using mailbox read and write at the moment. If we make it mandatory we will be forced to implement such callbacks as directly sof_mailbox_read() not going to work for us so we prefer to make it an optional dsp ops () callback for now.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@ajitkupandey OK. I think for now we can make this ops optional.

Choose a reason for hiding this comment

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

@dbaluta @plbossart Thanks.. FYI we will implement custom mailbox_read()/write() for AMD platform to make our code more streamline with SOF core but for now keeping these ops optional is really helpful.

dbaluta and others added 3 commits August 18, 2021 16:27
…lbacks

We need to introduce snd_sof_mailbox_{read/write} in order to provide
a generic way for mailbox access. These routines are optional, each
platform can implement their own specific routines.

So far, all platforms use mmaped I/O thus they can use custom made
routines sof_mailbox_read / sof_mailbox_write that use MMIO.

Signed-off-by: Daniel Baluta <[email protected]>
Signed-off-by: Bud Liviu-Alexandru <[email protected]>
This operations should be generic as there is nothing Intel
specific. This works well for NXP i.MX8 stream IPC ops.

We start by moving sof/intel/intel-ipc.c into sof/stream-ipc.c and
rename the functions to be generic.

Notice that we use newly introduced snd_sof_dsp_mailbox_read
instead of sof_mailbox_read, to make sure that we are not
bound to existing MMIO memory access, and we allow platform
to implement their own memory access routines.

Signed-off-by: Daniel Baluta <[email protected]>
Signed-off-by: Bud Liviu-Alexandru <[email protected]>
This makes IMX use the newly introduced generic IPC ops
instead of imx specific ones, and removes the old IMX
ipc ops, as they are no longer needed.

Signed-off-by: Daniel Baluta <[email protected]>
Signed-off-by: Bud Liviu-Alexandru <[email protected]>
@plbossart plbossart requested a review from ujfalusi August 19, 2021 22:27
Copy link
Collaborator

@ujfalusi ujfalusi left a comment

Choose a reason for hiding this comment

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

@Potochi , @dbaluta, thank you, looks nice.

@dbaluta dbaluta merged commit 0c0b27a into thesofproject:topic/sof-dev Aug 20, 2021
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