RE: New subsystem for acceleration devices

2022-08-10 Thread yuji2.ishikawa
> -Original Message-
> From: Oded Gabbay 
> Sent: Wednesday, August 10, 2022 6:42 AM
> To: Dave Airlie ; Greg Kroah-Hartman
> ; ishikawa yuji(石川 悠司 ○RDC□AITC○
> EA開) ; Jiho Chu 
> Cc: dri-devel ; Arnd Bergmann
> ; Linux-Kernel@Vger. Kernel. Org
> ; Jason Gunthorpe 
> Subject: Re: New subsystem for acceleration devices
> 
> Hi Jiho, Yuji.
> 
> I want to update that I'm currently in discussions with Dave to figure out 
> what's
> the best way to move forward. We are writing it down to do a proper comparison
> between the two paths (new accel subsystem or using drm). I guess it will take
> a week or so.
> 
> In the meantime, I'm putting the accel code on hold. I have only managed to do
> the very basic infra and add a demo driver that shows how to register and
> unregister from it.
> You can check the code at:
> https://git.kernel.org/pub/scm/linux/kernel/git/ogabbay/linux.git/log/?h=ac
> cel
> 
> It has two commits. The first adds the subsystem code and the second adds the
> demo driver.
> The subsystem code is basically drm code copied and renamed and slightly
> modified, but I really only worked on it for a couple of hours so take that 
> into
> consideration.
> 
> The important thing is that the demo driver shows the basic steps are really
> simple. You need to add two function calls in your probe and one function 
> call in
> your release. Of course you will need to supply some function callbacks, but I
> haven't got to fill that in the demo driver. Once you register, you get
> /dev/accel/ac0 and
> /dev/accel/ac_controlD64 (if you want a control device). If I were to continue
> this, the next step is to do the open and close part.
> 
> I will update once we know where things are heading. As I said, I imagine it 
> can
> take a few weeks.
> 
> Thanks,
> Oded

Hi Odded,
Thank you for uploading the framework as well as a sample. 
It's exciting to see new software is growing up.

Since Visconti DNN is a platform device, I'll write some test code to 
initialize driver and see if it works.

Regards,
Yuji


RE: [PATCH v2 2/5] soc: visconti: Add Toshiba Visconti image processing accelerator common source

2022-07-26 Thread yuji2.ishikawa
Hi Greg,

Thank you for your comments.

> -Original Message-
> From: Greg KH 
> Sent: Monday, July 25, 2022 9:46 PM
> To: ishikawa yuji(石川 悠司 ○RDC□AITC○EA開)
> 
> Cc: Rob Herring ; Hans Verkuil ;
> iwamatsu nobuhiro(岩松 信洋 □SWC◯ACT)
> ; Jonathan Corbet ;
> Sumit Semwal ; Christian König
> ; linux-arm-ker...@lists.infradead.org;
> linux-ker...@vger.kernel.org; linux-me...@vger.kernel.org;
> dri-devel@lists.freedesktop.org; linaro-mm-...@lists.linaro.org
> Subject: Re: [PATCH v2 2/5] soc: visconti: Add Toshiba Visconti image
> processing accelerator common source
> 
> On Fri, Jul 22, 2022 at 05:28:55PM +0900, Yuji Ishikawa wrote:
> > This commit adds common definitions shared among image processing
> > accelerator drivers for Toshiba Visconti SoCs.
> 
> Please wrap your changelog text lines properly at 72 columns.
>
> And you need to provide a lot more information here as to what this is, it's 
> not
> enough to be able to properly review this with just a single sentence.
>

I'll update changelog.

> >
> > Signed-off-by: Yuji Ishikawa 
> > Reviewed-by: Nobuhiro Iwamatsu 
> > ---
> > v1 -> v2:
> >   - applied checkpatch.pl --strict
> > ---
> >  drivers/soc/Kconfig   |  1 +
> >  drivers/soc/Makefile  |  1 +
> >  drivers/soc/visconti/Kconfig  |  1 +
> >  drivers/soc/visconti/Makefile |  6 +++
> >  drivers/soc/visconti/ipa_common.c | 55 +++
> > drivers/soc/visconti/ipa_common.h | 18 +++
> >  drivers/soc/visconti/uapi/ipa.h   | 90
> +++
> >  7 files changed, 172 insertions(+)
> >  create mode 100644 drivers/soc/visconti/Kconfig  create mode 100644
> > drivers/soc/visconti/Makefile  create mode 100644
> > drivers/soc/visconti/ipa_common.c  create mode 100644
> > drivers/soc/visconti/ipa_common.h  create mode 100644
> > drivers/soc/visconti/uapi/ipa.h
> >
> > diff --git a/drivers/soc/Kconfig b/drivers/soc/Kconfig index
> > e8a30c4c5..c99139aa8 100644
> > --- a/drivers/soc/Kconfig
> > +++ b/drivers/soc/Kconfig
> > @@ -22,6 +22,7 @@ source "drivers/soc/tegra/Kconfig"
> >  source "drivers/soc/ti/Kconfig"
> >  source "drivers/soc/ux500/Kconfig"
> >  source "drivers/soc/versatile/Kconfig"
> > +source "drivers/soc/visconti/Kconfig"
> >  source "drivers/soc/xilinx/Kconfig"
> >
> >  endmenu
> > diff --git a/drivers/soc/Makefile b/drivers/soc/Makefile index
> > a05e9fbcd..455b993c2 100644
> > --- a/drivers/soc/Makefile
> > +++ b/drivers/soc/Makefile
> > @@ -28,4 +28,5 @@ obj-$(CONFIG_ARCH_TEGRA)  += tegra/
> >  obj-y  += ti/
> >  obj-$(CONFIG_ARCH_U8500)   += ux500/
> >  obj-$(CONFIG_PLAT_VERSATILE)   += versatile/
> > +obj-$(CONFIG_ARCH_VISCONTI)+= visconti/
> >  obj-y  += xilinx/
> > diff --git a/drivers/soc/visconti/Kconfig
> > b/drivers/soc/visconti/Kconfig new file mode 100644 index
> > 0..8b1378917
> > --- /dev/null
> > +++ b/drivers/soc/visconti/Kconfig
> > @@ -0,0 +1 @@
> > +
> > diff --git a/drivers/soc/visconti/Makefile
> > b/drivers/soc/visconti/Makefile new file mode 100644 index
> > 0..8d710da08
> > --- /dev/null
> > +++ b/drivers/soc/visconti/Makefile
> > @@ -0,0 +1,6 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +#
> > +# Makefile for the Visconti specific device drivers.
> > +#
> > +
> > +obj-y += ipa_common.o
> > diff --git a/drivers/soc/visconti/ipa_common.c
> > b/drivers/soc/visconti/ipa_common.c
> > new file mode 100644
> > index 0..6345f33c5
> > --- /dev/null
> > +++ b/drivers/soc/visconti/ipa_common.c
> > @@ -0,0 +1,55 @@
> > +// SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause
> 
> Why is this dual-licensed?  I have to ask, and also, have to see some sort of
> justification as to why this is needed.  Doing dual-licensed kernel code is
> tough and a pain and we need to know that you, and your lawyers, understand
> the issues involved here.
>

I'll talk with development members.

> 
> > +/* Toshiba Visconti Image Processing Accelerator Support
> > + *
> > + * (C) Copyright 2022 TOSHIBA CORPORATION
> > + * (C) Copyright 2022 Toshiba Electronic Devices & Storage
> > +Corporation  */
> > +
> > +#include "ipa_common.h"
> > +
> > +int ipa_attach_dmabuf(struct device *dev, int fd, struct
> dma_buf_attachment **a,
> > + struct sg_table **s, dma_addr_t *addr, enum
> > +dma_data_direction dma_dir) {
> > +   struct dma_buf_attachment *attachment;
> > +   struct dma_buf *dmabuf;
> > +   struct sg_table *sgt;
> > +   int ret;
> > +
> > +   dmabuf = dma_buf_get(fd);
> > +   if (IS_ERR(dmabuf)) {
> > +   dev_err(dev, "Invalid dmabuf FD\n");
> > +   return PTR_ERR(dmabuf);
> > +   }
> > +   attachment = dma_buf_attach(dmabuf, dev);
> > +
> > +   if (IS_ERR(attachment)) {
> > +   dev_err(dev, "Failed to attach dmabuf\n");
> > +   ret = PTR_ERR(attachment);
> > +   goto err_put;
> > +   }
> > +   sgt = dma_buf_map_attachment(attachment, dma_dir);
> > +   if (IS_ERR(sgt)) {
> > +   

RE: [PATCH v2 5/5] Documentation: driver-api: visconti: add a description of DNN driver.

2022-07-26 Thread yuji2.ishikawa
Hi Jonathan

Thank you for your comments

> -Original Message-
> From: Jonathan Corbet 
> Sent: Friday, July 22, 2022 10:33 PM
> To: ishikawa yuji(石川 悠司 ○RDC□AITC○EA開)
> ; Rob Herring ; Hans
> Verkuil ; iwamatsu nobuhiro(岩松 信洋 □SWC◯ACT)
> ; Sumit Semwal
> ; Christian König 
> Cc: linux-arm-ker...@lists.infradead.org; linux-ker...@vger.kernel.org;
> linux-me...@vger.kernel.org; dri-devel@lists.freedesktop.org;
> linaro-mm-...@lists.linaro.org; ishikawa yuji(石川 悠司 ○RDC□AITC○
> EA開) 
> Subject: Re: [PATCH v2 5/5] Documentation: driver-api: visconti: add a
> description of DNN driver.
> 
> Yuji Ishikawa  writes:
> 
> No changelog?
> 

I'll add more detailed changelog.

> > Signed-off-by: Yuji Ishikawa 
> > ---
> > v1 -> v2:
> >   - newly added documents
> > ---
> >  Documentation/driver-api/visconti/common.rst | 115 ++
> >  Documentation/driver-api/visconti/dnn.rst| 394
> +++
> >  2 files changed, 509 insertions(+)
> >  create mode 100644 Documentation/driver-api/visconti/common.rst
> >  create mode 100644 Documentation/driver-api/visconti/dnn.rst
> 
> Two overall comments:
> 
>  - You've added new RST files without adding them to index.rst; that
>will keep them from being part of the kernel docs build and will add
>new warnings.

I'll add index.rst to put these documents into the toc tree.

>  - Please avoid the use of flat-table and just use regular RST
>ascii-art tables.  Otherwise the result is nearly unreadable in the
>plain-test format.

All right, flat-table will be replaced with regular table.

> Thanks,
> 
> jon

Regards,
  Yuji


RE: [PATCH v2 4/5] MAINTAINERS: Add entries for Toshiba Visconti DNN image processing accelerator

2022-07-26 Thread yuji2.ishikawa
Sorry for this patch not being well formatted.

I'll add change log and the signed-off-by tag.
I should have checked patches with checkpatch.pl as well as testing sources 
with --file option.

Regards,
  Yuji 

> -Original Message-
> From: Greg KH 
> Sent: Monday, July 25, 2022 9:47 PM
> To: ishikawa yuji(石川 悠司 ○RDC□AITC○EA開)
> 
> Cc: Rob Herring ; Hans Verkuil ;
> iwamatsu nobuhiro(岩松 信洋 □SWC◯ACT)
> ; Jonathan Corbet ;
> Sumit Semwal ; Christian König
> ; linux-arm-ker...@lists.infradead.org;
> linux-ker...@vger.kernel.org; linux-me...@vger.kernel.org;
> dri-devel@lists.freedesktop.org; linaro-mm-...@lists.linaro.org
> Subject: Re: [PATCH v2 4/5] MAINTAINERS: Add entries for Toshiba Visconti
> DNN image processing accelerator
> 
> On Fri, Jul 22, 2022 at 05:28:57PM +0900, Yuji Ishikawa wrote:
> > ---
> >  MAINTAINERS | 2 ++
> >  1 file changed, 2 insertions(+)
> 
> No changelog text?
> 
> No signed-off-by?
> 
> Are you sure that checkpatch passed this patch?
> 
> greg k-h


RE: [PATCH v2 3/5] soc: visconti: Add Toshiba Visconti DNN image processing accelerator

2022-07-26 Thread yuji2.ishikawa
Hi Greg

Thank you for your comments.

> -Original Message-
> From: Greg KH 
> Sent: Monday, July 25, 2022 9:51 PM
> To: ishikawa yuji(石川 悠司 ○RDC□AITC○EA開)
> 
> Cc: Rob Herring ; Hans Verkuil ;
> iwamatsu nobuhiro(岩松 信洋 □SWC◯ACT)
> ; Jonathan Corbet ;
> Sumit Semwal ; Christian König
> ; linux-arm-ker...@lists.infradead.org;
> linux-ker...@vger.kernel.org; linux-me...@vger.kernel.org;
> dri-devel@lists.freedesktop.org; linaro-mm-...@lists.linaro.org
> Subject: Re: [PATCH v2 3/5] soc: visconti: Add Toshiba Visconti DNN image
> processing accelerator
> 
> On Fri, Jul 22, 2022 at 05:28:56PM +0900, Yuji Ishikawa wrote:
> > --- /dev/null
> > +++ b/drivers/soc/visconti/uapi/dnn.h
> > @@ -0,0 +1,77 @@
> > +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
> > +/* Toshiba Visconti DNN Accelerator Support
> > + *
> > + * (C) Copyright 2022 TOSHIBA CORPORATION
> > + * (C) Copyright 2022 Toshiba Electronic Devices & Storage
> > +Corporation  */
> > +
> > +#ifndef _UAPI_LINUX_DNN_H
> > +#define _UAPI_LINUX_DNN_H
> > +
> > +#include 
> > +#include 
> > +#include "ipa.h"
> > +
> > +#define DRV_DNN_BIT_CONFIG_DESC_FINAL (0x8000U)
> > +#define DRV_DNN_BUFFER_INDEX_MAX  (15)
> > +
> > +#define DRV_DNN_BASE_ADDR_NUM (8U) /* DNN number of base
> address */
> > +
> > +#define DRV_DNN_BASE_ADDR_PURPOSE_INPUT(1U)
> > +#define DRV_DNN_BASE_ADDR_PURPOSE_OUTPUT(2U)
> > +#define DRV_DNN_BASE_ADDR_PURPOSE_AWB  (3U)
> > +#define DRV_DNN_BASE_ADDR_PURPOSE_TEMPORARY (4U)
> > +
> > +/**
> > + * struct drv_dnn_status - DNN IPA status for IOC_IPA_GET_STATUS
> > + *
> > + * @state: State of driver
> > + * @eer_cmd:   Execution error command
> > + * @eer:   Execution error
> > + * @reserved:  Padding
> > + * @eer_flags: Execution error flags
> > + */
> > +struct drv_dnn_status {
> > +   enum drv_ipa_state state;
> > +   __u32 eer_cmd;
> > +   __u32 eer : 1;
> > +   __u32 reserved : 31;
> 
> bitfields will not work like this for uapi files, sorry.

I'll change the type of the member eer from bitfield to bool.

> 
> > +   __u32 eer_flags[32];
> 
> What endian is all of these?  Big?  Little?  Unknown?

The processors and accelerators are little endian in Visconti SoC.
Do I have to use more specific type such as __le32 ?

> 
> > +};
> > +
> > +struct drv_dnn_base_addr {
> > +   __u32 purpose;
> > +   union {
> > +   struct drv_ipa_addr ipa_addr;
> > +   uintptr_t list_addr;
> 
> You really do not ever want a uintptr_t in a uapi file, that's not going to be
> portable at all.  It's also not a valid kernel type :(

I understand. The member list_addr should be typed "struct drv_ipa_addr*".

> No documentation on what "purpose" is for?

I'll add description for struct drv_dnn_base_addr with kernel-doc style.

>
> > +   } addr;
> > +};
> > +
> > +/**
> > + * struct drv_dnn_descriptor - DNN IPA Descriptor for IOC_IPA_START
> > + *
> > + * @configuration:Address of configuration data
> > + * @configuration_offset: Configuration offset
> > + * @configuration_size:   Configuration data size
> > + * @list_num: Number of input/output list
> > + * @base_addr:Base addresses
> > + * @base_addr_flag:   Bit-fields of base_addr list config;
> > + *0 for fixed address,
> > + *1 for address list.
> 
> Where are the bitfield definitions?
> What about unused bits, what happens with them?  You are checking them,
> right?
>

I'll update comments for "base_addr" and "base_addr_flag".
The member base_addr[n] are handled differently
according to the n'th bit of the member base_addr_flag;
where 0 <= n < HWD_DNN_BASE_ADDR_NUM-1.

> > + * @config_done:  Flags of called configuration
> > + * @buffer_info:  Table of buffer information
> > + * @buffer_info_num:  Number of buffer_info
> > + */
> > +struct drv_dnn_descriptor {
> > +   struct drv_ipa_addr configuration;
> > +   __u32 configuration_offset;
> 
> What endian are any of these?

They are little endian as processors and accelerators are LE.
Do I have to use specific type such as __le32?

Do we need special care for endianness  when userland and kernel are sharing 
data (a drv_dnn_descriptor instance) ?
I thought there're no endianness problem when the driver is reading/writing 
HW's 32bit registers.
I understand, generally, special cares are needed for some cases like:
* network byte order --- endianness is specified in a specification.
* data blocks are stored to byte-addressable memory and read by another 
processing elements (HW or CPU in another system).
* HW designed for little endian is placed in big endian system.

> thanks,
> 
> greg k-h

Regards,
  Yuji


RE: [PATCH v2 3/5] soc: visconti: Add Toshiba Visconti DNN image processing accelerator

2022-07-26 Thread yuji2.ishikawa
Hi Greg

Thanks for your comments

> -Original Message-
> From: Greg KH 
> Sent: Monday, July 25, 2022 9:48 PM
> To: ishikawa yuji(石川 悠司 ○RDC□AITC○EA開)
> Hi
> Cc: Rob Herring ; Hans Verkuil ;
> iwamatsu nobuhiro(岩松 信洋 □SWC◯ACT)
> ; Jonathan Corbet ;
> Sumit Semwal ; Christian König
> ; linux-arm-ker...@lists.infradead.org;
> linux-ker...@vger.kernel.org; linux-me...@vger.kernel.org;
> dri-devel@lists.freedesktop.org; linaro-mm-...@lists.linaro.org
> Subject: Re: [PATCH v2 3/5] soc: visconti: Add Toshiba Visconti DNN image
> processing accelerator
> 
> On Fri, Jul 22, 2022 at 05:28:56PM +0900, Yuji Ishikawa wrote:
> > Add support to DNN image processing accelerator on Toshiba Visconti ARM
> SoCs.
> > The accelerator is applicable to DNN inference tasks.
> >
> > Signed-off-by: Yuji Ishikawa 
> > Reviewed-by: Nobuhiro Iwamatsu 
> > ---
> > v1 -> v2:
> >   - applied checkpatch.pl --strict
> >   - removed unused code
> > ---
> >  drivers/soc/visconti/Kconfig   |   6 +
> >  drivers/soc/visconti/Makefile  |   2 +
> >  drivers/soc/visconti/dnn/Makefile  |   6 +
> >  drivers/soc/visconti/dnn/dnn.c | 523
> +
> >  drivers/soc/visconti/dnn/hwd_dnn.c | 183 +
> >  drivers/soc/visconti/dnn/hwd_dnn.h |  68 
> >  drivers/soc/visconti/dnn/hwd_dnn_reg.h | 228 +++
> >  drivers/soc/visconti/uapi/dnn.h|  77 
> >  8 files changed, 1093 insertions(+)
> >  create mode 100644 drivers/soc/visconti/dnn/Makefile  create mode
> > 100644 drivers/soc/visconti/dnn/dnn.c  create mode 100644
> > drivers/soc/visconti/dnn/hwd_dnn.c
> >  create mode 100644 drivers/soc/visconti/dnn/hwd_dnn.h
> >  create mode 100644 drivers/soc/visconti/dnn/hwd_dnn_reg.h
> >  create mode 100644 drivers/soc/visconti/uapi/dnn.h
> >
> > diff --git a/drivers/soc/visconti/Kconfig
> > b/drivers/soc/visconti/Kconfig index 8b1378917..a25287d0c 100644
> > --- a/drivers/soc/visconti/Kconfig
> > +++ b/drivers/soc/visconti/Kconfig
> > @@ -1 +1,7 @@
> > +if ARCH_VISCONTI
> > +
> > +config VISCONTI_DNN
> > +bool "Visconti DNN driver"
> > +
> 
> We can't take Kconfig options with no help text at all, please provide the
> needed information here.

I'll add detailed description to Kconfig help text.

> 
> And why can't this be a module?

There're no special reasons to disable module build.
I'll test module build and add the option.

Regards,
  Yuji


RE: [PATCH v2 0/5] Add Toshiba Visconti DNN image processing accelerator driver

2022-07-26 Thread yuji2.ishikawa
Hi Greg

Thank you for your comments.

> -Original Message-
> From: Greg KH 
> Sent: Monday, July 25, 2022 9:47 PM
> To: ishikawa yuji(石川 悠司 ○RDC□AITC○EA開)
> 
> Cc: Rob Herring ; Hans Verkuil ;
> iwamatsu nobuhiro(岩松 信洋 □SWC◯ACT)
> ; Jonathan Corbet ;
> Sumit Semwal ; Christian König
> ; linux-arm-ker...@lists.infradead.org;
> linux-ker...@vger.kernel.org; linux-me...@vger.kernel.org;
> dri-devel@lists.freedesktop.org; linaro-mm-...@lists.linaro.org
> Subject: Re: [PATCH v2 0/5] Add Toshiba Visconti DNN image processing
> accelerator driver
> 
> On Fri, Jul 22, 2022 at 05:28:53PM +0900, Yuji Ishikawa wrote:
> > This series is the DNN image processing accelerator driver for Toshiba's ARM
> SoC, Visconti[0].
> > This provides DT binding documentation, device driver, MAINTAINER files
> and documents.
> >
> > Best regards,
> > Yuji
> >
> > [0]:
> >
> https://toshiba.semicon-storage.com/ap-en/semiconductor/product/image-
> > recognition-processors-visconti.html
> >
> > dt-bindings: soc: visconti: Add Toshiba Visconti DNN image processing
> accelerator bindings
> >   v1 -> v2:
> > - No update
> >
> > soc: visconti: Add Toshiba Visconti image processing accelerator common
> source
> >   v1 -> v2:
> > - checked with checkpatch.pl --strict
> >
> > soc: visconti: Add Toshiba Visconti DNN image processing accelerator
> >   v1 -> v2:
> > - checked with checkpatch.pl --strict
> > - removed unused code
> >
> > MAINTAINERS: Add entries for Toshiba Visconti DNN image processing
> >   v1 -> v2:
> > - No update
> >
> > Documentation: driver-api: visconti: add a description of DNN driver.
> >   v1 -> v2:
> > - newly added documents
> >
> > Yuji Ishikawa (5):
> >   dt-bindings: soc: visconti: Add Toshiba Visconti DNN image processing
> > accelerator bindings
> >   soc: visconti: Add Toshiba Visconti image processing accelerator
> > common source
> >   soc: visconti: Add Toshiba Visconti DNN image processing accelerator
> >   MAINTAINERS: Add entries for Toshiba Visconti DNN image processing
> > accelerator
> >   Documentation: driver-api: visconti: add a description of DNN driver.
> >
> >  .../soc/visconti/toshiba,visconti-dnn.yaml|  54 ++
> >  Documentation/driver-api/visconti/common.rst  | 115 
> >  Documentation/driver-api/visconti/dnn.rst | 394 +
> >  MAINTAINERS   |   2 +
> >  drivers/soc/Kconfig   |   1 +
> >  drivers/soc/Makefile  |   1 +
> >  drivers/soc/visconti/Kconfig  |   7 +
> >  drivers/soc/visconti/Makefile |   8 +
> >  drivers/soc/visconti/dnn/Makefile |   6 +
> >  drivers/soc/visconti/dnn/dnn.c| 523
> ++
> >  drivers/soc/visconti/dnn/hwd_dnn.c| 183 ++
> >  drivers/soc/visconti/dnn/hwd_dnn.h|  68 +++
> >  drivers/soc/visconti/dnn/hwd_dnn_reg.h| 228 
> >  drivers/soc/visconti/ipa_common.c |  55 ++
> >  drivers/soc/visconti/ipa_common.h |  18 +
> >  drivers/soc/visconti/uapi/dnn.h   |  77 +++
> >  drivers/soc/visconti/uapi/ipa.h   |  90 +++
> 
> Why is this in drivers/soc/?

Actually, I'm not sure where his module should live in.
The directory drivers/soc were chosen just because the driver is specific to 
Visconti SoC.
Is it better to move the driver to another directory such as drivers/misc ?

> And uapi files belong in the correct include path, not burried in a driver
> subdirectory where they will never be picked up correctly by the build system.
> How did you test these?

I understand it's not a good idea to place uapi files under driver subdirectory.
A build command "make headers_install" did not pick out headers.
I used additional shell script to install headers for Visconti.
I'll move uapi headers to include/uapi/linux.

> thanks,
> 
> greg k-h

Regards,
  Yuji


RE: [PATCH 0/4] Add Toshiba Visconti DNN image processing accelerator driver

2022-07-11 Thread yuji2.ishikawa
Hi Hans,

I'm going to submit DNN driver (with misc driver style) again.
Updated version will include corrections suggested in reviews.

For example:
* apply checkpatch.pl --strict
* uppercase letters in function name -> make them lowercase
* use of integer types: uint32_t -> u32 ( _u32 in user API header )

Please let me know further actions which should be done before submission.

Regards,
Yuji Ishikawa

> -Original Message-
> From: ishikawa yuji(石川 悠司 ○RDC□AITC○EA開)
> Sent: Tuesday, July 5, 2022 10:17 AM
> To: Hans Verkuil ; robh...@kernel.org; iwamatsu
> nobuhiro(岩松 信洋 □SWC◯ACT) ;
> sumit.sem...@linaro.org; christian.koe...@amd.com
> Cc: linux-arm-ker...@lists.infradead.org; linux-ker...@vger.kernel.org;
> linux-me...@vger.kernel.org; dri-devel@lists.freedesktop.org;
> linaro-mm-...@lists.linaro.org
> Subject: RE: [PATCH 0/4] Add Toshiba Visconti DNN image processing
> accelerator driver
> 
> Hi Hans,
> 
> Thank you for your advice.
> I understand that V4L2 M2M driver is recommended for a group including
> Affine and Pyramid.
> 
> As for DSPIF, our application SW engineer said that there is another version 
> of
> firmware to handle neural network.
> Therefore, DSP can handle not only images, but also some array/list typed
> data.
> It would be better to handle DSPIF as a misc device as well as DNN.
> 
> It is still difficult for me to imagine a whole structure of M2M type driver 
> and
> corresponding userland application.
> What do they look like? Is there any existing driver suitable for reference?
> 
> Let me think about a Pyramid HW which receives a planar (R/G/B as different
> planes, not in interleaved RGBRGB format) image, and yields 3 planar images.
> Is it covered by only 1 fd, or multiple fds are required?
> * A fd for Pyramid driver:
>   * OUTPUT queue for receiving a picture?
> * OUTPUT or OUTPUT_MPLANE?
> * Q: Typically, RGB image is handled with single plane buffer in V4L2. RGB
> images do not have fourcc for multi-planar format (while YUV images do).
> * How should I explicitly request R/G/B planar buffer in V4L2
> semantics?
>   * CAPTURE queue for yielding a picture?
> * CAPTURE or CAPTURE_MPLANE?
> * Again: RGB interleaved vs R/G/B planar
> * Q: If I want to capture multiple images (with different resolution) at 
> a time,
> how should I do?
> * Do I have to add other fds for 2nd, 3rd CAPTURE queue?
> * Or, multiple images can be packed in the same CAPTURE buffer?
> * Or, multiple images can be placed in CAPTURE_MPLANE buffer ?
>   * Compound control to receive a configuration.
> 
> Regards,
>   Yuji
> 
> > -Original Message-
> > From: Hans Verkuil 
> > Sent: Wednesday, June 29, 2022 10:44 PM
> > To: ishikawa yuji(石川 悠司 ○RDC□AITC○EA開)
> > ; robh...@kernel.org; iwamatsu
> > nobuhiro(岩松
> > 信洋 □SWC◯ACT) ;
> > sumit.sem...@linaro.org; christian.koe...@amd.com
> > Cc: linux-arm-ker...@lists.infradead.org;
> > linux-ker...@vger.kernel.org; linux-me...@vger.kernel.org;
> > dri-devel@lists.freedesktop.org; linaro-mm-...@lists.linaro.org
> > Subject: Re: [PATCH 0/4] Add Toshiba Visconti DNN image processing
> > accelerator driver
> >
> > My apologies for the late reply...
> >
> > On 01/06/2022 03:40, yuji2.ishik...@toshiba.co.jp wrote:
> > > Hi Hans,
> > >
> > > Thank you for your advice.
> > > I prepared some description of DNN accelerator and its usage.
> > >
> > >  Handling memory blocks for Visconti5 accelerators
> > >
> > > Visconti5 Image-Processing-Accelerators do not have fine grained
> > > IOMMU,
> > as CPU have.
> > > Therefore, memory region to be passed to the accelerators should be
> > physically contiguous.
> > > We use DMA-BUF backed by CMA (Contiguous Memory Allocator) to
> > > allocate
> > memory regions for sharing between CPU/IPAs.
> > > Originally, in v4.19 based implementation, the ION allocator was
> > > used to
> > allocate DMA-BUF instances.
> > > For the latest implementation, DMA-BUF HEAPS is used.
> > >
> > > Two structure types are used to represent memory region passed to drivers.
> > > * struct drv_ipa_buffer_info
> > >   * to describe whole DMA-BUF instance
> > > * struct drv_ipa_addr
> > >   * to describe a memory region in a DMA-BUF instance
> > >
> > > for details, see usage sample of each IPA driver
> > >
> > >
> > >  Image Processing Accelerators overview
> > >
> > > Visconti5 SoC has following image processing accererators
> > >
> > > * AFFINE: 1 input image, 1 output image;
> > Affine transform, Homography transform, Polynomial lens distortion,
> > LUT transform
> > > * DNN:N input feature vector, N output feature vector;
> > Deep neural network operation
> > > * PYRAMID 3 input image, 3 * N output image;
> > Resize grayscale/color image with N different parameters
> > > * DSPIF:  M input image, N output image;
> > Various opeations on images
> > > * HOX:1 input image (multi ROI), 1 input dictionary1 
> > > likelihood/feature
> > vector;  Extended 

RE: [PATCH 0/4] Add Toshiba Visconti DNN image processing accelerator driver

2022-07-05 Thread yuji2.ishikawa
Hi Hans,

Thank you for your advice.
I understand that V4L2 M2M driver is recommended for a group including Affine 
and Pyramid.

As for DSPIF, our application SW engineer said that there is another version of 
firmware to handle neural network.
Therefore, DSP can handle not only images, but also some array/list typed data.
It would be better to handle DSPIF as a misc device as well as DNN.

It is still difficult for me to imagine a whole structure of M2M type driver 
and corresponding userland application.
What do they look like? Is there any existing driver suitable for reference?

Let me think about a Pyramid HW which receives a planar (R/G/B as different 
planes, not in interleaved RGBRGB format) image, and yields 3 planar images.
Is it covered by only 1 fd, or multiple fds are required?
* A fd for Pyramid driver:
  * OUTPUT queue for receiving a picture?
* OUTPUT or OUTPUT_MPLANE?
* Q: Typically, RGB image is handled with single plane buffer in V4L2. RGB 
images do not have fourcc for multi-planar format (while YUV images do).
* How should I explicitly request R/G/B planar buffer in V4L2 semantics?
  * CAPTURE queue for yielding a picture?
* CAPTURE or CAPTURE_MPLANE?
* Again: RGB interleaved vs R/G/B planar 
* Q: If I want to capture multiple images (with different resolution) at a 
time, how should I do?
* Do I have to add other fds for 2nd, 3rd CAPTURE queue?
* Or, multiple images can be packed in the same CAPTURE buffer?
* Or, multiple images can be placed in CAPTURE_MPLANE buffer ?
  * Compound control to receive a configuration.

Regards,
Yuji

> -Original Message-
> From: Hans Verkuil 
> Sent: Wednesday, June 29, 2022 10:44 PM
> To: ishikawa yuji(石川 悠司 ○RDC□AITC○EA開)
> ; robh...@kernel.org; iwamatsu nobuhiro(岩松
> 信洋 □SWC◯ACT) ;
> sumit.sem...@linaro.org; christian.koe...@amd.com
> Cc: linux-arm-ker...@lists.infradead.org; linux-ker...@vger.kernel.org;
> linux-me...@vger.kernel.org; dri-devel@lists.freedesktop.org;
> linaro-mm-...@lists.linaro.org
> Subject: Re: [PATCH 0/4] Add Toshiba Visconti DNN image processing
> accelerator driver
> 
> My apologies for the late reply...
> 
> On 01/06/2022 03:40, yuji2.ishik...@toshiba.co.jp wrote:
> > Hi Hans,
> >
> > Thank you for your advice.
> > I prepared some description of DNN accelerator and its usage.
> >
> >  Handling memory blocks for Visconti5 accelerators
> >
> > Visconti5 Image-Processing-Accelerators do not have fine grained IOMMU,
> as CPU have.
> > Therefore, memory region to be passed to the accelerators should be
> physically contiguous.
> > We use DMA-BUF backed by CMA (Contiguous Memory Allocator) to allocate
> memory regions for sharing between CPU/IPAs.
> > Originally, in v4.19 based implementation, the ION allocator was used to
> allocate DMA-BUF instances.
> > For the latest implementation, DMA-BUF HEAPS is used.
> >
> > Two structure types are used to represent memory region passed to drivers.
> > * struct drv_ipa_buffer_info
> >   * to describe whole DMA-BUF instance
> > * struct drv_ipa_addr
> >   * to describe a memory region in a DMA-BUF instance
> >
> > for details, see usage sample of each IPA driver
> >
> >
> >  Image Processing Accelerators overview
> >
> > Visconti5 SoC has following image processing accererators
> >
> > * AFFINE: 1 input image, 1 output image;
> Affine transform, Homography transform, Polynomial lens distortion, LUT
> transform
> > * DNN:N input feature vector, N output feature vector;
> Deep neural network operation
> > * PYRAMID 3 input image, 3 * N output image;
> Resize grayscale/color image with N different parameters
> > * DSPIF:  M input image, N output image;
> Various opeations on images
> > * HOX:1 input image (multi ROI), 1 input dictionary1 likelihood/feature
> vector;  Extended Histogram of Oriented Gradient based pattern matching
> > * HAMAT:  2 input feature vectors: 1 output corrdinate vector;
> Hamming distance matching for stereo vision
> > * FLMAT:  3 input image, N input feature point, N output matched point;
> Optical flow matching
> > * SMLDB:  1 input image, N input feature point, N output feature vector;
> Accelerated-KAZE feature descriptor accelerator
> > * STMAT:  2 input image, 1 output disparity image;
> Stereo disparity
> 
> It's not really easy to decide what is best. I would say that if both input 
> and
> output are images (RGB, YUV, Grayscale), then a V4L2 memory-to-memory
> driver is what I would expect to see.
> 
> Where that is not the case, then a more custom approach makes sense.
> 
> In the list above I would put AFFINE, PYRAMID, DSPIF and possible STMAT in
> the V4L2 driver group, and the others more as custom drivers.
> 
> I think it also depends on how it is used: if a captured sensor image is 
> typically
> passed in for further processing, i.e. it is closely related to the video 
> ISP, then
> V4L2 is a reasonable choice.
> 
> A DNN driver, on the other hand, isn't using 

RE: [PATCH v2 3/4] soc: visconti: Add Toshiba Visconti AFFINE image processing accelerator

2022-05-31 Thread yuji2.ishikawa
Hi, Hans,

With your advice for DNN accelerator, I prepared some description of AFFINE IPA 
and its usage.

 Handling memory blocks for Visconti5 accelerators (same description for 
DNN accelerator)

Visconti5 Image-Processing-Accelerators do not have fine grained IOMMU, as CPU 
have.
Therefore, memory region to be passed to the accelerators should be physically 
contiguous.
We use DMA-BUF backed by CMA (Contiguous Memory Allocator) to allocate memory 
regions for sharing between CPU/IPAs.
Originally, in v4.19 based implementation, the ION allocator was used to 
allocate DMA-BUF instances.
For the latest implementation, DMA-BUF HEAPS is used.

Two structure types are used to represent memory region passed to drivers.
* struct drv_ipa_buffer_info
  * to describe whole DMA-BUF instance
* struct drv_ipa_addr
  * to describe a memory region in a DMA-BUF instance

for details, see usage sample of each IPA driver


 Image Processing Accelerators overview (same description for DNN 
accelerator)

Visconti5 SoC has following image processing accererators

* AFFINE: 1 input image, 1 output image;
 Affine transform, Homography transform, Polynomial lens distortion, LUT 
transform
* DNN:N input feature vector, N output feature vector;  
 Deep neural network operation
* PYRAMID 3 input image, 3 * N output image;
 Resize grayscale/color image with N different parameters
* DSPIF:  M input image, N output image;
 Various opeations on images
* HOX:1 input image (multi ROI), 1 input dictionary1 likelihood/feature 
vector;  Extended Histogram of Oriented Gradient based pattern matching
* HAMAT:  2 input feature vectors: 1 output corrdinate vector;  
 Hamming distance matching for stereo vision
* FLMAT:  3 input image, N input feature point, N output matched point; 
 Optical flow matching
* SMLDB:  1 input image, N input feature point, N output feature vector;
 Accelerated-KAZE feature descriptor accelerator
* STMAT:  2 input image, 1 output disparity image;  
 Stereo disparity

see [0] Fig 7.2.1 for block diagram (of prototype chip)


 Affine Overview

AFFINE IPA is a proprietary image processing hardware developed by Toshiba.
Visconti5 SoC has 2 instances of AFFINE IPA hardware.
It has 4 operation modes:
  * Affine (Linear) transformation
  * Homography transformation
  * Polynomial lens distortion
  * LUT based transformation
It accepts 1 input image and yields 1 output image at an operation.


 Input / Output

Input: 8bit grayscale or 16bit grayscale image
Output: 8bit grayscale or 16bit grayscale image

AFFINE IPA driver accepts an instance of "struct drv_affine_descriptor" which 
includes input/output images and operation parameters.


 Descriptor Builder at userland

Following APIs are provided to build a descriptor instance at userland.

/* defined in drv_affine_util.h */
int32_t drv_AFFINE_config_descript_init(struct drv_affine_descriptor *desc, 
struct drv_ipa_buffer_info *buffer, int32_t buffer_num);
int32_t drv_AFFINE_config_input_image(struct drv_affine_descriptor *desc, 
struct drv_ipa_addr src_addr, int32_t src_width, int32_t src_height, int32_t 
src_pitch, int32_t src_depth);  
int32_t drv_AFFINE_config_output_image(struct drv_affine_descriptor *desc, 
struct drv_ipa_addr dst_addr, int32_t dst_width, int32_t dst_height, int32_t 
dst_offset_x, int32_t dst_offset_y, uint16_t fill_value, int32_t dst_pitch, 
int32_t dst_depth)
int32_t drv_AFFINE_config_interpolation_mode(...)
int32_t drv_AFFINE_config_linear(...)
int32_t drv_AFFINE_config_homography(...)
int32_t drv_AFFINE_config_undist(...)
int32_t drv_AFFINE_config_table(...)
int32_t drv_AFFINE_config_descript_finalize(struct drv_affine_descriptor *desc)

struct drv_affine_descriptor is defined in drivers/soc/visconti/uapi/affine.h.
I think this header should be placed anywhere else to be collected on "make 
headers_install" action of kernel building.


 Usage sample (without error handlers)

#include 
#include "drv_ipa.h" 
#include "drv_affine.h"
#include "drv_affine_util.h" 

int allocate_buffer(int fd_heap, int size) 
{
struct dma_heap_allocation_data heap_data_in={0};
int ret;

heap_data_in.len = ROUNDUP_POW2(size);
heap_data_in.fd_flags = O_RDWR | O_CLOEXEC;

ret = ioctl(fd_heap, DMA_HEAP_IOCTL_ALLOC, _data_in);
if (ret <0)
return -1;
else
return heap_data_in.fd;
}

void affine_sample(int fd_affine, int fd_src, int fd_dst) 
{
struct drv_ipa_buffer_info bufinfo[2] = {
{.fd=fd_src, .coherent=true, .direction=DRV_IPA_DIR_TO_DEVICE},
{.fd=fd_dst, .coherent=true, .direction=DRV_IPA_DIR_FROM_DEVICE},
};
struct 

RE: [PATCH 0/4] Add Toshiba Visconti DNN image processing accelerator driver

2022-05-31 Thread yuji2.ishikawa
Hi Hans,

Thank you for your advice.
I prepared some description of DNN accelerator and its usage.

 Handling memory blocks for Visconti5 accelerators

Visconti5 Image-Processing-Accelerators do not have fine grained IOMMU, as CPU 
have.
Therefore, memory region to be passed to the accelerators should be physically 
contiguous.
We use DMA-BUF backed by CMA (Contiguous Memory Allocator) to allocate memory 
regions for sharing between CPU/IPAs.
Originally, in v4.19 based implementation, the ION allocator was used to 
allocate DMA-BUF instances.
For the latest implementation, DMA-BUF HEAPS is used.

Two structure types are used to represent memory region passed to drivers.
* struct drv_ipa_buffer_info
  * to describe whole DMA-BUF instance
* struct drv_ipa_addr
  * to describe a memory region in a DMA-BUF instance

for details, see usage sample of each IPA driver


 Image Processing Accelerators overview

Visconti5 SoC has following image processing accererators

* AFFINE: 1 input image, 1 output image;
 Affine transform, Homography transform, Polynomial lens distortion, LUT 
transform
* DNN:N input feature vector, N output feature vector;  
 Deep neural network operation
* PYRAMID 3 input image, 3 * N output image;
 Resize grayscale/color image with N different parameters
* DSPIF:  M input image, N output image;
 Various opeations on images
* HOX:1 input image (multi ROI), 1 input dictionary1 likelihood/feature 
vector;  Extended Histogram of Oriented Gradient based pattern matching
* HAMAT:  2 input feature vectors: 1 output corrdinate vector;  
 Hamming distance matching for stereo vision
* FLMAT:  3 input image, N input feature point, N output matched point; 
 Optical flow matching
* SMLDB:  1 input image, N input feature point, N output feature vector;
 Accelerated-KAZE feature descriptor accelerator
* STMAT:  2 input image, 1 output disparity image;  
 Stereo disparity

see [0] Fig 7.2.1 for block diagram (of prototype chip)


 DNN accelerator overview

DNN accelerator is a proprietary CNN/DCNN processing accelerator developed by 
Toshiba.
Visconti5 SoC has 2 instances of DNN acclerator hardware.
Users convert existing Caffe/ONNX models to Visconti compatible models with an 
offline tool.
A converted model "Configuration Binary" includes:
  * instruction sequence for given network
  * weight/bias information
  * DMA configuration from/to global memory (for input/output feature)

DNN acccelerator can handle either 1 plane or multiple ROIs at a single call.

see [0] Fig 7.2.2 for block diagram of DNN accelerator

CNN: Convolutional Neural Network
DCNN: Deep Convolutional Neural Network


 Input / Output

Input image or feature: base type is either of FP16, FP32, INT8, UINT8, INT16
Output feature vector:  base type is either of FP16, FP32, INT8, UINT8, INT16

Input, Output, Weight, Bias can be placed on global memory and loaded/stored 
with DMA within DNN accelerator.
These data on global memory can be specified as either of:
  * single address to point single data block
  * list of address to point multiple data blocks (i.e. ROIs)

DNN acclerator driver accepts an instance of "struct drv_dnn_descriptor" which 
includes addresses of input/output features and a configuration binary.


 Descriptor Builder at userland

Following APIs are provided to build a descriptor instance at userland.

/* defined in drv_dnn_util.h */
int32_t drv_DNN_config_descript_init(struct drv_dnn_descriptor *desc, struct 
drv_ipa_buffer_info *buffer, int32_t buffer_num);
int32_t drv_DNN_config_exec_configuration(struct drv_dnn_descriptor *desc, 
const void *configuration_binary,
  struct drv_ipa_addr 
configuration_binary_addr, struct drv_ipa_addr *src_list,
  struct drv_ipa_addr *dst_list, 
int32_t list_num, struct drv_ipa_addr temporary_addr,
  int32_t temporary_size);
int32_t drv_DNN_config_descript_finalize(struct drv_dnn_descriptor *desc);

struct drv_dnn_descriptor is defined in drivers/soc/visconti/uapi/dnn.h.
I think this header should be placed anywhere else to be collected on "make 
headers_install" action of kernel building.


 Usage sample (without error handlers)

#include 
#include "drv_ipa.h"
#include "drv_dnn.h"
#include "drv_dnn_util.h" 

int allocate_buffer(int fd_heap, int size) 
{
struct dma_heap_allocation_data heap_data_in={0};
int ret;

heap_data_in.len = ROUNDUP_POW2(size);
heap_data_in.fd_flags = O_RDWR | O_CLOEXEC;

ret = ioctl(fd_heap, DMA_HEAP_IOCTL_ALLOC, _data_in);
if (ret <0)
return -1;
else
return heap_data_in.fd;
}


RE: [PATCH 0/4] Add Toshiba Visconti DNN image processing accelerator driver

2022-05-20 Thread yuji2.ishikawa
Hi Hans,

Thank you for your comment.
I agree that this submission lacks documents sharing basic idea of the 
accelerators; what do they accept and what do they yield.
Where can I put a new document? Can I put it as a comment in a source? Can I 
add a file under Documentation/misc-devices directory?

Thanks,
Yuji Ishikawa

> -Original Message-
> From: Hans Verkuil 
> Sent: Thursday, May 12, 2022 8:15 PM
> To: ishikawa yuji(石川 悠司 ○RDC□AITC○EA開)
> ; Rob Herring ;
> iwamatsu nobuhiro(岩松 信洋 □SWC◯ACT)
> ; Sumit Semwal
> ; Christian König 
> Cc: linux-arm-ker...@lists.infradead.org; linux-ker...@vger.kernel.org;
> linux-me...@vger.kernel.org; dri-devel@lists.freedesktop.org;
> linaro-mm-...@lists.linaro.org
> Subject: Re: [PATCH 0/4] Add Toshiba Visconti DNN image processing
> accelerator driver
> 
> Hi Yuji,
> 
> On 4/28/22 15:11, Yuji Ishikawa wrote:
> > This series is the DNN image processing accelerator driver for Toshiba's ARM
> SoC, Visconti[0].
> > This provides DT binding documentation, device driver, MAINTAINER files.
> >
> > The second patch "soc: visconti: Add Toshiba Visconti image processing
> accelerator common source"
> > and the fourth patch "MAINTAINERS: ..." are the same as the ones in the
> preceding post for affine driver.
> 
> There appears to be no documentation whatsoever, unless I am missing
> something.
> 
> How is the uAPI supposed to be used? What does it do? What formats does it
> accept or produce?
> 
> If this processes images, then (as Laurent mentioned) this is more suitable 
> as a
> V4L2 mem2mem driver.
> 
> See
> https://linuxtv.org/downloads/v4l-dvb-apis-new/userspace-api/v4l/dev-me
> m2mem.html
> and the many drivers in drivers/media that use it (git grep v4l2-mem2mem.h).
> 
> But without any explanation whatsoever I have no idea what does or does not
> make sense.
> 
> Regards,
> 
>   Hans
> 
> >
> > Best regards,
> > Yuji
> >
> > [0]:
> >
> https://toshiba.semicon-storage.com/ap-en/semiconductor/product/image-
> > recognition-processors-visconti.html
> >
> > Yuji Ishikawa (4):
> >   dt-bindings: soc: visconti: Add Toshiba Visconti DNN image processing
> > accelerator bindings
> >   soc: visconti: Add Toshiba Visconti image processing accelerator
> > common source
> >   soc: visconti: Add Toshiba Visconti DNN image processing accelerator
> >   MAINTAINERS: Add entries for Toshiba Visconti DNN image processing
> > accelerator
> >
> >  .../soc/visconti/toshiba,visconti-dnn.yaml|  54 ++
> >  MAINTAINERS   |   2 +
> >  drivers/soc/Kconfig   |   1 +
> >  drivers/soc/Makefile  |   1 +
> >  drivers/soc/visconti/Kconfig  |   7 +
> >  drivers/soc/visconti/Makefile |   8 +
> >  drivers/soc/visconti/dnn/Makefile |   6 +
> >  drivers/soc/visconti/dnn/dnn.c| 533
> ++
> >  drivers/soc/visconti/dnn/hwd_dnn.c| 183 ++
> >  drivers/soc/visconti/dnn/hwd_dnn.h|  68 +++
> >  drivers/soc/visconti/dnn/hwd_dnn_reg.h| 228 
> >  drivers/soc/visconti/ipa_common.c |  55 ++
> >  drivers/soc/visconti/ipa_common.h |  18 +
> >  drivers/soc/visconti/uapi/dnn.h   |  77 +++
> >  drivers/soc/visconti/uapi/ipa.h   |  88 +++
> >  15 files changed, 1329 insertions(+)
> >  create mode 100644
> > Documentation/devicetree/bindings/soc/visconti/toshiba,visconti-dnn.ya
> > ml  create mode 100644 drivers/soc/visconti/Kconfig  create mode
> > 100644 drivers/soc/visconti/Makefile  create mode 100644
> > drivers/soc/visconti/dnn/Makefile  create mode 100644
> > drivers/soc/visconti/dnn/dnn.c  create mode 100644
> > drivers/soc/visconti/dnn/hwd_dnn.c
> >  create mode 100644 drivers/soc/visconti/dnn/hwd_dnn.h
> >  create mode 100644 drivers/soc/visconti/dnn/hwd_dnn_reg.h
> >  create mode 100644 drivers/soc/visconti/ipa_common.c  create mode
> > 100644 drivers/soc/visconti/ipa_common.h  create mode 100644
> > drivers/soc/visconti/uapi/dnn.h  create mode 100644
> > drivers/soc/visconti/uapi/ipa.h
> >


RE: [PATCH v2 0/4] Add Toshiba Visconti AFFINE image processing accelerator driver

2022-04-27 Thread yuji2.ishikawa
Hi Laurent,

Thank you for your comment.

We had never imagined that affine driver can be a V4L2 driver.
Affine is one of the accelerators in Visconti, and some accelerators 
receive/yield non-picture data.
Also, as the original accelerator drivers were implemented for kernel 4.19.x, 
we were not aware of the latest V4L2 architecture.
Currently, we assume accelerator drivers are kicked individually, not in an 
image processing pipeline, therefore simple misc driver is enough solution.

Is there any memory-to-memory driver sample/skelton to bring me better 
understanding?

Best regards,
Yuji

> -Original Message-
> From: Laurent Pinchart 
> Sent: Thursday, April 28, 2022 7:04 AM
> To: ishikawa yuji(石川 悠司 ○RDC□AITC○EA開)
> 
> Cc: Rob Herring ; iwamatsu nobuhiro(岩松 信洋 □SW
> C◯ACT) ; Sumit Semwal
> ; Christian König ;
> linux-arm-ker...@lists.infradead.org; linux-ker...@vger.kernel.org;
> linux-me...@vger.kernel.org; dri-devel@lists.freedesktop.org;
> linaro-mm-...@lists.linaro.org
> Subject: Re: [PATCH v2 0/4] Add Toshiba Visconti AFFINE image processing
> accelerator driver
> 
> Hi Yuji,
> 
> Thank you for the patch. It's nice to see contributions from Toshiba in the 
> image
> acceleration domain :-)
> 
> I'll start with a high-level question before diving into detailed review. Why 
> is
> this implemented in drivers/soc/ with a custom userspace API, and not as a
> V4L2 memory-to-memory driver ?
> 
> On Wed, Apr 27, 2022 at 10:23:41PM +0900, Yuji Ishikawa wrote:
> > This series is the AFFINE image processing accelerator driver for Toshiba's
> ARM SoC, Visconti[0].
> > This provides DT binding documentation, device driver, MAINTAINER files.
> >
> > The second patch "soc: visconti: Add Toshiba Visconti image processing
> accelerator common source"
> > is commonly used among acclerator drivers (affine, dnn, dspif, pyramid).
> >
> > Best regards,
> > Yuji
> >
> > [0]:
> >
> https://toshiba.semicon-storage.com/ap-en/semiconductor/product/image-
> > recognition-processors-visconti.html
> >
> >   dt-bindings: soc: visconti: Add Toshiba Visconti AFFINE image
> > v1 -> v2:
> >   - No update
> >
> >   soc: visconti: Add Toshiba Visconti image processing accelerator common
> source
> > v1 -> v2:
> >   - apply checkpatch.pl --strict
> >
> >   soc: visconti: Add Toshiba Visconti AFFINE image processing accelerator
> > v1 -> v2:
> >   - apply checkpatch.pl --strict
> >   - rename hwd_AFFINE_ to hwd_affine_
> >
> >   MAINTAINERS: Add entries for Toshiba Visconti AFFINE image processing
> accelerator
> > v1 -> v2:
> >   - No update
> >
> > Change in V2:
> >   - apply checkpatch.pl --strict
> >   - rename hwd_AFFINE_ to hwd_affine_
> >
> > Yuji Ishikawa (4):
> >   dt-bindings: soc: visconti: Add Toshiba Visconti AFFINE image
> > processing accelerator bindings
> >   soc: visconti: Add Toshiba Visconti image processing accelerator
> > common source
> >   soc: visconti: Add Toshiba Visconti AFFINE image processing
> > accelerator
> >   MAINTAINERS: Add entries for Toshiba Visconti AFFINE image processing
> > accelerator
> >
> >  .../soc/visconti/toshiba,visconti-affine.yaml |  53 ++
> >  MAINTAINERS   |   2 +
> >  drivers/soc/Kconfig   |   1 +
> >  drivers/soc/Makefile  |   1 +
> >  drivers/soc/visconti/Kconfig  |   7 +
> >  drivers/soc/visconti/Makefile |   8 +
> >  drivers/soc/visconti/affine/Makefile  |   6 +
> >  drivers/soc/visconti/affine/affine.c  | 451
> ++
> >  drivers/soc/visconti/affine/hwd_affine.c  | 206 
> >  drivers/soc/visconti/affine/hwd_affine.h  |  83 
> >  drivers/soc/visconti/affine/hwd_affine_reg.h  |  45 ++
> >  drivers/soc/visconti/ipa_common.c |  55 +++
> >  drivers/soc/visconti/ipa_common.h |  18 +
> >  drivers/soc/visconti/uapi/affine.h|  87 
> >  drivers/soc/visconti/uapi/ipa.h   |  88 
> >  15 files changed,  insertions(+)
> >  create mode 100644
> > Documentation/devicetree/bindings/soc/visconti/toshiba,visconti-affine
> > .yaml  create mode 100644 drivers/soc/visconti/Kconfig  create mode
> > 100644 drivers/soc/visconti/Makefile  create mode 100644
> > drivers/soc/visconti/affine/Makefile
> >  create mode 100644 drivers/soc/visconti/affine/affine.c
> >  create mode 100644 drivers/soc/visconti/affine/hwd_affine.c
> >  create mode 100644 drivers/soc/visconti/affine/hwd_affine.h
> >  create mode 100644 drivers/soc/visconti/affine/hwd_affine_reg.h
> >  create mode 100644 drivers/soc/visconti/ipa_common.c  create mode
> > 100644 drivers/soc/visconti/ipa_common.h  create mode 100644
> > drivers/soc/visconti/uapi/affine.h
> >  create mode 100644 drivers/soc/visconti/uapi/ipa.h
> 
> --
> Regards,
> 
> Laurent Pinchart


RE: [PATCH 0/4] Add Toshiba Visconti AFFINE image processing accelerator driver

2022-04-27 Thread yuji2.ishikawa
Hi,

I withdraw this series of patches.
I'm going to submit updated version after applying checkpatch.pl with strict 
option.

Best regards,
Yuji

> -Original Message-
> From: Yuji Ishikawa 
> Sent: Tuesday, April 19, 2022 4:20 PM
> To: iwamatsu nobuhiro(岩松 信洋 □SWC◯ACT)
> ; Sumit Semwal
> ; Christian König 
> Cc: linux-arm-ker...@lists.infradead.org; linux-ker...@vger.kernel.org;
> linux-me...@vger.kernel.org; dri-devel@lists.freedesktop.org;
> linaro-mm-...@lists.linaro.org; ishikawa yuji(石川 悠司 ○RDC□AITC○
> EA開) 
> Subject: [PATCH 0/4] Add Toshiba Visconti AFFINE image processing
> accelerator driver
> 
> This series is the AFFINE image processing accelerator driver for Toshiba's
> ARM SoC, Visconti[0].
> This provides DT binding documentation, device driver, MAINTAINER files.
> 
> Best regards,
> Yuji
> 
> [0]:
> https://toshiba.semicon-storage.com/ap-en/semiconductor/product/image-r
> ecognition-processors-visconti.html
> 
> Yuji Ishikawa (4):
>   dt-bindings: soc: visconti: Add Toshiba Visconti AFFINE image
> processing accelerator bindings
>   soc: visconti: Add Toshiba Visconti image processing accelerator
> common source
>   soc: visconti: Add Toshiba Visconti AFFINE image processing
> accelerator
>   MAINTAINERS: Add entries for Toshiba Visconti AFFINE image processing
> accelerator
> 
>  .../soc/visconti/toshiba,visconti-affine.yaml |  53 ++
>  MAINTAINERS   |   2 +
>  drivers/soc/Kconfig   |   1 +
>  drivers/soc/Makefile  |   1 +
>  drivers/soc/visconti/Kconfig  |   7 +
>  drivers/soc/visconti/Makefile |   8 +
>  drivers/soc/visconti/affine/Makefile  |   6 +
>  drivers/soc/visconti/affine/affine.c  | 451
> ++
>  drivers/soc/visconti/affine/hwd_affine.c  | 207 
>  drivers/soc/visconti/affine/hwd_affine.h  |  83 
>  drivers/soc/visconti/affine/hwd_affine_reg.h  |  45 ++
>  drivers/soc/visconti/ipa_common.c |  55 +++
>  drivers/soc/visconti/ipa_common.h |  19 +
>  drivers/soc/visconti/uapi/affine.h|  87 
>  drivers/soc/visconti/uapi/ipa.h   |  87 
>  15 files changed, 1112 insertions(+)
>  create mode 100644
> Documentation/devicetree/bindings/soc/visconti/toshiba,visconti-affine.yaml
>  create mode 100644 drivers/soc/visconti/Kconfig  create mode 100644
> drivers/soc/visconti/Makefile  create mode 100644
> drivers/soc/visconti/affine/Makefile
>  create mode 100644 drivers/soc/visconti/affine/affine.c
>  create mode 100644 drivers/soc/visconti/affine/hwd_affine.c
>  create mode 100644 drivers/soc/visconti/affine/hwd_affine.h
>  create mode 100644 drivers/soc/visconti/affine/hwd_affine_reg.h
>  create mode 100644 drivers/soc/visconti/ipa_common.c  create mode
> 100644 drivers/soc/visconti/ipa_common.h  create mode 100644
> drivers/soc/visconti/uapi/affine.h
>  create mode 100644 drivers/soc/visconti/uapi/ipa.h
> 
> --
> 2.17.1