Hi Laurent, Thanks for your feedback! > From: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx> > Sent: 02 March 2021 12:32 > Subject: Re: [PATCH 4/7] misc: Add driver for DAB IP found on Renesas R- > Car devices > > Hi Fabrizio, > > On Tue, Mar 02, 2021 at 12:20:17PM +0000, Fabrizio Castro wrote: > > On 02 March 2021 11:17, Ezequiel Garcia wrote: > > > On Mon, 1 Mar 2021 at 14:36, Fabrizio Castro wrote: > > > > On 26 February 2021 10:38, Arnd Bergmann wrote: > > > > > On Thu, Feb 25, 2021 at 11:51 PM Fabrizio Castro wrote: > > > > > > > > > > > > The DAB hardware accelerator found on R-Car E3 and R-Car M3-N > devices is > > > > > > a hardware accelerator for software DAB demodulators. > > > > > > It consists of one FFT (Fast Fourier Transform) module and one > decoder > > > > > > module, compatible with DAB specification (ETSI EN 300 401 and > > > > > > ETSI TS 102 563). > > > > > > The decoder module can perform FIC decoding and MSC decoding > processing > > > > > > from de-puncture to final decoded result. > > > > > > > > > > > > This patch adds a device driver to support the FFT module only. > > > > > > > > > > > > Signed-off-by: Fabrizio Castro <fabrizio.castro.jz@xxxxxxxxxxx> > > > > > > --- > > > > > > MAINTAINERS | 7 ++ > > > > > > drivers/misc/Kconfig | 1 + > > > > > > drivers/misc/Makefile | 1 + > > > > > > drivers/misc/rcar_dab/Kconfig | 11 ++ > > > > > > drivers/misc/rcar_dab/Makefile | 8 ++ > > > > > > drivers/misc/rcar_dab/rcar_dev.c | 176 > +++++++++++++++++++++++++++++++ > > > > > > drivers/misc/rcar_dab/rcar_dev.h | 116 ++++++++++++++++++++ > > > > > > drivers/misc/rcar_dab/rcar_fft.c | 160 > ++++++++++++++++++++++++++++ > > > > > > include/uapi/linux/rcar_dab.h | 35 ++++++ > > > > > > > > > > Can you explain why this is not in drivers/media/? > > > > > > > > I wasn't entirely sure were to put it to be honest as I couldn't > find > > > > anything similar, that's why "misc" for v1, to mainly get a feedback > > > > and advice. > > > > > > > > > I don't think we want a custom ioctl interface for a device that > > > > > implements > > > > > a generic specification. My first feeling would be that this > should not > > > > > have a user-level API but instead get called by the DAB radio > driver. > > > > > > > > I hear you, the trouble is that the modules of this IP should be > seen > > > > as part of a SW and HW processing pipeline. > > > > Some of the SW components of the pipeline can be proprietary, and > > > > unfortunately we don't have access (yet) to a framework that allows > us to > > > > test and demonstrate the whole pipeline, for the moment we can only > test > > > > things in isolation. It'll take us a while to come up with a full > solution > > > > (or to have access to one), and in the meantime our SoCs come with > an IP > > > > that can't be used because there is no driver for it (yet). > > > > > > > > After discussing things further with Geert and Laurent, I think they > > > > are right in saying that the best path for this is probably to add > this > > > > driver under "drivers/staging" as an interim solution, so that the > IP will > > > > be accessible by developers, and when we have everything we need (a > fully > > > > fledged processing framework), we will able to integrate it better > with > > > > the other components (if possible). > > > > > > > > > What is the intended usage model here? I assume the idea is to > > > > > use it in an application that receives audio or metadata from DAB. > > > > > What driver do you use for that? > > > > > > > > This IP is similar to a DMA to some extent, in the sense that it > takes > > > > input data from a buffer in memory and it writes output data onto a > buffer > > > > in memory, and of course it does some processing in between. > > > > > > That sounds like something that can fit V4L2 MEM2MEM driver. > > > You queue two buffers and an operation, and then run a job. > > > > V4L2 MEM2MEM seems good for this in general, however the DAB IP is going > > to be shared by multiple processing pipelines (as usually we have > several > > DRIF interfaces, and only 1 DAB IP), and for what concerns FFT > specifically > > there is only 1 FFT module inside the DAB IP. > > My understanding is that the capabilities of V4L2 MEM2MEM devices are > > configured for the specific pipeline, but in the this context user space > > would have to continuously switch the capabilities of the DAB IP (at the > > right moment) to allow processing for data streams requiring different > > capabilities. > > > > Am I wrong? > > V4L2 M2M devices can be opened multiple times, but different processes, > which can configure the device in different ways, and submit jobs that > are then scheduled by the V4L2 M2M framework. I'll give it a try in due time then. I think the clock related patches are worth merging. Thanks, Fab > > > > > It's not directly connected to any other Radio related IP. > > > > The user space application can read DAB IQ samples from the R-Car > DRIF > > > > interface (via driver drivers/media/platform/rcar_drif.c, or from > other > > > > sources since this IP is decoupled from DRIF), and then when it's > time > > > > to accelerate FFT, FIC, or MSC, it can request services to the DAB > IP, so > > > > that the app can go on and use the processor to do some work, while > the DAB > > > > IP processes things. > > > > A framework to connect and exchange processing blocks (either SW or > HW) and > > > > interfaces is therefore vital to properly place a kernel > implementation > > > > in the great scheme of things, in its absence a simple driver can > help > > > > > > I'm not entirely sure we are missing a framework. What's missing in > > > V4L2 MEM2MEM? > > > > I was referring to a user space framework (I should have been more > specific > > with my previous email). > > > > > Before considering drivers/staging, it would be interesting to figure > > > out if V4L2 can do it as-is, and if not, discuss what's missing. > > > > I think an interim solution would allow us and users to evaluate things > a > > little bit better, so that we can integrate this IP with V4L2 later on. > > -- > Regards, > > Laurent Pinchart
