[PATCH 3/6] ASoC: SOF: Create client driver for IPC test

[Ertman, David M]

> -----Original Message-----
> From: Pierre-Louis Bossart <pierre-louis.bossart at linux.intel.com>
> Sent: Thursday, October 1, 2020 6:55 AM
> To: Greg KH <gregkh at linuxfoundation.org>; Ertman, David M
> <david.m.ertman at intel.com>
> Cc: alsa-devel at alsa-project.org; tiwai at suse.de; broonie at kernel.org;
> Sridharan, Ranjani <ranjani.sridharan at intel.com>; jgg at nvidia.com;
> parav at nvidia.com; Ranjani Sridharan <ranjani.sridharan at linux.intel.com>;
> Fred Oh <fred.oh at linux.intel.com>
> Subject: Re: [PATCH 3/6] ASoC: SOF: Create client driver for IPC test
> 
> Thanks for the review Greg.
> 
> On 10/1/20 8:09 AM, Greg KH wrote:
> > On Wed, Sep 30, 2020 at 03:50:48PM -0700, Dave Ertman wrote:
> >> From: Ranjani Sridharan <ranjani.sridharan at linux.intel.com>
> >>
> >> Create an SOF client driver for IPC flood test. This
> >> driver is used to set up the debugfs entries and the
> >> read/write ops for initiating the IPC flood test that
> >> would be used to measure the min/max/avg response times
> >> for sending IPCs to the DSP.
> >>
> >> Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart at linux.intel.com>
> >> Signed-off-by: Ranjani Sridharan <ranjani.sridharan at linux.intel.com>
> >> Co-developed-by: Fred Oh <fred.oh at linux.intel.com>
> >> Signed-off-by: Fred Oh <fred.oh at linux.intel.com>
> >> Signed-off-by: Dave Ertman <david.m.ertman at intel.com>
> >
> > Am I reading this series correct in that this is the only "user" of the
> > new ancilicary bus/driver code?
> 
> This is the first user, and it was meant to demonstrate how the client
> is instantiated and communicates with hardware controlled by the parent.
> The next users will be 'audio probes', which provides the ability to
> extract/inject data into the DSP. We also want to split the existing
> audio cards into several pieces, e.g. the HDMI parts should really be
> presented as a separate card.
> 
> The other users will be networking/RDMA, which were actually the first
> to suggest this bus.
> 
> > If so, why is it even needed?  These are just debugfs files for testing,
> > why does that need to be in a separate device?  What is being "shared"
> > here that needs multiple struct devices to handle?
> >
> > confused,
> 
> The parent PCI device provides access to the DSP firmware/hardware and
> is in complete control of the IPC with the DSP firmware. The parent
> plays the role of a 'server' providing shared hardware access to
> multiple clients.
> 
> Why is this needed?
> 
> With the current audio solutions, we have a monolithic solution that has
> proven difficult to maintain. We'd really like to expose unrelated DSP
> functionality with different devices.
> 
> The best example is really HDMI. HDMI/DP audio interfaces are controlled
> by the same hardware, but are logically independent. What we end-up
> doing is re-adding the same solution over and over for each machine driver:
> 
> sound/soc/intel/boards$ git grep hda_dsp_hdmi_build_controls
> bxt_da7219_max98357a.c:         return hda_dsp_hdmi_build_controls(card,
> component);
> bxt_rt298.c:            return hda_dsp_hdmi_build_controls(card, component);
> cml_rt1011_rt5682.c:            return hda_dsp_hdmi_build_controls(card,
> component);
> ehl_rt5660.c:   return hda_dsp_hdmi_build_controls(card,
> pcm->codec_dai->component);
> glk_rt5682_max98357a.c:         return hda_dsp_hdmi_build_controls(card,
> component);
> hda_dsp_common.c:int hda_dsp_hdmi_build_controls(struct snd_soc_card
> *card,
> hda_dsp_common.h:int hda_dsp_hdmi_build_controls(struct snd_soc_card
> *card,
> hda_dsp_common.h:static inline int hda_dsp_hdmi_build_controls(struct
> snd_soc_card *card,
> skl_hda_dsp_common.h:   return hda_dsp_hdmi_build_controls(card,
> component);
> sof_da7219_max98373.c:          return hda_dsp_hdmi_build_controls(card,
> sof_pcm512x.c:  return hda_dsp_hdmi_build_controls(card,
> pcm->codec_dai->component);
> sof_rt5682.c:           return hda_dsp_hdmi_build_controls(card, component);
> sof_sdw_hdmi.c:         return hda_dsp_hdmi_build_controls(card,
> component);
> 
> and we also keep adding HDMI-related ASoC topology definitions for all
> the cards.
> 
> It would make a lot more sense if we could have ONE HDMI/DP card which
> is created, instead of managing HDMI/DP from the card that is supposed
> to deal with local accessories based on HDaudio/DMIC/SoundWire/I2S.
> 
> The audio probes are similar, we want to have a single probe client
> instead of adding audio probes to every single card we have to maintain.
> 
> On platforms where the DSP can deal with sensors, this would also allow
> the parent to expose IIO and HID clients.
> 
> Going back to this IPC test, maybe the commit message is unclear: we
> already have this functionality in the mainline, it's been very helpful
> for stress tests. What this patch shows is that moving the functionality
> to a client makes it possible to scale to 2 or more clients with a
> simple set of register/unregister. The device model makes it really easy
> to scale.
> 
> So yes, you are correct that for now there is a single user with very
> limited functionality. This is intentional to make the reviews simpler,
> but if/when this bus is part of the mainline we'll have additional
> users, and not just from Intel if you look at the reviewed-by tags.
> 
> We might even remove the platform devices used for the SoundWire master
> and use this instead :-)
> 
> Does this help?
> 

The reason we switched from using RDMA/LAN as an example was the impasse
in trying to get patches for all three moving parts  up through two different trees
at the same time (netdev and rdma).  It was becoming a huge mess.  The Intel
sound driver folks provided a consumer in a single tree to make submission smoother.

The patches for the LAN driver and RDMA are waiting to see what the final form of
ancillary bus will be so that they can match to the final API.  We still have the same
use case in the LAN/RDMA model.

-DaveE