ASoC: SOF: imx: imx driver refactorization and imx95 driver introduction #5307
Conversation
LaurentiuM1234
force-pushed
the
feat/imx_driver_refactor
branch
5 times, most recently
from
252b1bd to
386ce80
Compare
LaurentiuM1234
requested review from
lgirdwood,
plbossart,
ranj063,
kv2019i,
dbaluta,
bardliao and
ujfalusi
as code owners
LaurentiuM1234
force-pushed
the
feat/imx_driver_refactor
branch
from
386ce80 to
8554d07
Compare
The SOF drivers for imx chips have a lot of duplicate code and routines/code snippets that could certainly be reused among drivers. As such, introduce a new set of structures and functions that will help eliminate the redundancy and code size of the drivers. Signed-off-by: Laurentiu Mihalcea <[email protected]>
The common interface for imx chips (defined in imx-common.c) contains the definitions for a lot of functions required by the SOF core. As such, the platform driver can just use the common definitions instead of duplicating code by re-defining aforementioned functions. Make the transition to the new common interface. This consists of: 1) Removing unneeded functions, which are already defined in the common interface. 2) Defining some chip-specific operations/structures required by the interface to work. 3) Dropping structure definitions that are no longer needed. 4) Adapting some existing functions to the new interface. Signed-off-by: Laurentiu Mihalcea <[email protected]>
The definition of 'struct sof_dev_desc' has the following properties
for imx chips:
1) FW path is the same for all chips.
2) Topology path is the same for all chips.
3) FW name can be written as: "sof-${machine_name}.ri"
4) IPC3 is the only supported protocol
The structure takes quite a few lines of code. Since the intention
is to add support for more imx8 chips in the same driver, we need
to try and reduce the number of lines taken by information that's
not particularly useful. As such, we can use 'IMX_SOF_DEV_DESC()'
to reduce the declaration of the structure to just one line. The
only information that's particularly useful can be seen from the
parameters of the macro.
Of course, if any of the assumptions don't apply anymore, driver
writers can simply declare the 'struct sof_dev_desc' the "old
fashioned way". No reason to make the macro suit multiple needs.
The same logic applies to the array of 'struct snd_soc_dai_driver'.
Signed-off-by: Laurentiu Mihalcea <[email protected]>
Shuffle the definitions of some structures and functions such that they are better grouped. This is purely a cosmetic change. Signed-off-by: Laurentiu Mihalcea <[email protected]>
Drop some unneeded/unused macro definitions and header includes. Signed-off-by: Laurentiu Mihalcea <[email protected]>
The definition for the ops_init() operation is similar among the imx8 chips (namely: imx8, imx8x, imx8m, and imx8ulp). The only difference is the name of the 'struct snd_soc_dai_driver' array used to fill the SOF ops structure. As such, 'imx8_ops_init()' can be made into an utility function that takes the 'struct snd_soc_dai_driver' array and its size as parameters and fills the SOF ops structure fields accordingly. This will allow us to reuse this function when the other drivers (imx8m, imx8ulp) are merged into this one. Since the definition of the function is changed, it can no longer be used directly by the SOF core. Therefore, also introduce a wrapper: 'imx_ops_init()' that will call 'imx8_ops_init()' with the right parameters based on the chip. Signed-off-by: Laurentiu Mihalcea <[email protected]>
Now that the common interface for imx chip has been introduced, there's no longer a need to have a separate platform driver for imx8m. As such, merge the driver with the imx8 driver. Furthermore, delete the old driver as it's no longer useful. Signed-off-by: Laurentiu Mihalcea <[email protected]>
Now that the common interface for imx chip has been introduced, there's no longer a need to have a separate platform driver for imx8ulp. As such, merge the driver with the imx8 driver. Furthermore, delete the old driver as it's no longer useful. Signed-off-by: Laurentiu Mihalcea <[email protected]>
Add SOF support for the imx95 chip. Although the support is just for the imx95 chip, the driver is intended for all chips in the imx9 family. Note that the imx95 support could have just as easily been added to the imx8 platform driver but a new platform driver was created because the intention is to keep the families in separate drivers. Signed-off-by: Laurentiu Mihalcea <[email protected]>
LaurentiuM1234
force-pushed
the
feat/imx_driver_refactor
branch
from
8554d07 to
c078c30
Compare
|
V2 changes:
|
|
Looks good to me. I think we can remove the DNM flag so that people can start looking at it. |
|
CI testing done, dropping |
LaurentiuM1234
changed the title
|
@lgirdwood this is good to merge. |
submitted recently via https://lore.kernel.org/imx/[email protected]/T/ |
|
@bardliao Yes, patch is now in linux-next https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=b76b3ee5573fd6ff8761d82feb74d707eb2139ef Please let us know if there is a problem with rebasing as this patch was long forgotten in SOF tree and we pushed it now upstream and we needed to do some rebase. I think the best way to proceed when you'll do rebase is to remove the patch in the SOF tree as it already in the linux-next. |
|
Thanks @LaurentiuM1234 @dbaluta |
|
@bardliao planning on submitting the series to ASoC tree. Using linux-next |
|
@LaurentiuM1234 @dbaluta I rebased and fixed the conflicts. Please double check the commits in https://github.com/thesofproject/linux/commits/topic/sof-dev-rebase/. And please take the patches below to submit to ASoC tree if they look correct to you. |
compared with the version of patches sent before the rebase via https://lore.kernel.org/imx/[email protected]/ and everything seems fine (also added @iuliana-prodan R-b in the version sent upstream) now, we've received some comments on the patches sent to the ASoC tree and I'll most likely have to modify the patches. How would you recommend proceeding with this? The idea is to make the rebase process as smooth and easy as possible. |
|
@LaurentiuM1234 Usually, we create a PR for the change and merge it into the topic/sof-dev branch. Then squash the commits in the topic/sof-dev-rebase branch. Then submit the commits from the topic/sof-dev-rebase branch. |
|
@bardliao would it be possible to just revert the changes in topic/sof-dev and then have them back once they've made it into the ASoC tree? At this point, I think it'd be simpler. |
Yes, we can do it. We can leave it as it is for now and revert the current commits and pick the upstream version once your commits are applied to the ASoC tree. |
A rather aggressive but arguably much needed refactorization of the imx SOF drivers. The refactorization is meant to address the code duplication in the imx drivers and decrease the coding effort required for introducing a new imx platform.
After the refactorization, only two drivers remain: imx8 (meant for imx chips belonging to the imx8 family: imx8 (aka imx8qm), imx8x (aka imx8qxp), imx8m, imx8ulp) and imx9 (mean for imx chips belonging to the imx9 family: imx95).
The series also includes the introduction of the imx95 driver.
The series requires some CI testing, which is why it's tagged with [DNM]. Submitted for initial comments.
Notes: