[alsa-devel] [RESEND PATCH v2 0/7] ASoC: Intel: Skylake: Driver fundaments overhaul

Vinod Koul vkoul at kernel.org
Wed Jul 24 14:39:06 CEST 2019


On 23-07-19, 16:58, Cezary Rojewski wrote:
> Skylake driver is divided into two modules:
> - snd_soc_skl
> - snd_soc_skl_ipc
> 
> and nothing would be wrong if not for the fact that both cannot exist
> without one another. IPC module is not some kind of extension, as it is
> the case for snd_hda_ext_core which is separated from snd_hda_core -
> legacy hda interface. It's as much core Skylake module as snd_soc_skl
> is.

Well it is an extension as it represents the view of the firmware and we
always had moving IPCs! Even when skylake was developed we had two
different versions and even when i left we had two (sof being other one
at that time)

The reason for split was to ensure we can modularize lower level IPCs.
We have hardware layouts change, fw formats change so it helped if that
route was again taken! IIRC even the IPC was built on generic
IPC work by Liam, so yes there are layers but for a reason.

If you feel merging the two helps, I am okay with the change.

> Statement backup by existence of circular dependency between this two.
> To eliminate said problem, struct skl_sst has been created. From that
> momment, Skylake has been plagued by header errors (incomplete sturcts,
> unknown references etc.) whenever something new is to be added or code
> is cleaned up.

Any reason why new is being added here, aren't you guys moving to SOF?

> Fix this flawed design by merging snd_soc_skl and snd_soc_skl_ipc.
> Also, do not forget about struct skl_sst redundancy.
> Followup changes address harmful assumptions and false logic which
> driver currently implements e.g.: attempt to take role of master for
> DSP scheduling when in fact entire control takes place in DSP.

Where is the basis of that assumption, driver was a mere accomplice,
getting the data of topology and passing it down to firmware, whilst try
to do some book keeping and keep things for falling!

> 
> Changes since v1:
> - Rebased onto 5.4
> 
> Amadeusz Sławiński (2):
>   ASoC: Intel: Skylake: Combine snd_soc_skl_ipc and snd_soc_skl
>   ASoC: Intel: Skylake: Do not disable FW notifications
> 
> Cezary Rojewski (5):
>   ASoC: Intel: Skylake: Merge skl_sst and skl into skl_dev struct
>   ASoC: Intel: Skylake: Remove MCPS available check
>   ASoC: Intel: Skylake: Remove memory available check
>   ASoC: Intel: Skylake: Make MCPS and CPS params obsolete
>   ASoC: Intel: Skylake: Cleanup skl_module_cfg declaration
> 
>  sound/soc/intel/common/sst-ipc.h        |   1 +
>  sound/soc/intel/skylake/Makefile        |  12 +-
>  sound/soc/intel/skylake/bxt-sst.c       |  50 +--
>  sound/soc/intel/skylake/cnl-sst-dsp.h   |   7 +-
>  sound/soc/intel/skylake/cnl-sst.c       |  37 +-
>  sound/soc/intel/skylake/skl-debug.c     |  14 +-
>  sound/soc/intel/skylake/skl-messages.c  | 245 ++++++-------
>  sound/soc/intel/skylake/skl-nhlt.c      |  18 +-
>  sound/soc/intel/skylake/skl-pcm.c       |  74 ++--
>  sound/soc/intel/skylake/skl-ssp-clk.c   |   4 +-
>  sound/soc/intel/skylake/skl-sst-dsp.c   |  10 +-
>  sound/soc/intel/skylake/skl-sst-dsp.h   |  29 +-
>  sound/soc/intel/skylake/skl-sst-ipc.c   |   8 +-
>  sound/soc/intel/skylake/skl-sst-ipc.h   |  52 +--
>  sound/soc/intel/skylake/skl-sst-utils.c |  37 +-
>  sound/soc/intel/skylake/skl-sst.c       |  51 +--
>  sound/soc/intel/skylake/skl-topology.c  | 441 ++++++++----------------
>  sound/soc/intel/skylake/skl-topology.h  |  43 +--
>  sound/soc/intel/skylake/skl.c           |  54 +--
>  sound/soc/intel/skylake/skl.h           | 102 ++++--
>  20 files changed, 546 insertions(+), 743 deletions(-)
> 
> -- 
> 2.17.1
> 
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel at alsa-project.org
> https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

-- 
~Vinod


More information about the Alsa-devel mailing list