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_xxxx to hwd_affine_xxxx
>
> MAINTAINERS: Add entries for Toshiba Visconti AFFINE image processing accelerator
> v1 -> v2:
> - No update
>
> Change in V2:
> - apply checkpatch.pl --strict
> - rename hwd_AFFINE_xxxx to hwd_affine_xxxx
>
> 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, 1111 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
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 <[email protected]>
> Sent: Thursday, April 28, 2022 7:04 AM
> To: ishikawa yuji(石川 悠司 ○ＲＤＣ□ＡＩＴＣ○ＥＡ開)
> <[email protected]>
> Cc: Rob Herring <[email protected]>; iwamatsu nobuhiro(岩松 信洋 □ＳＷ
> Ｃ◯ＡＣＴ) <[email protected]>; Sumit Semwal
> <[email protected]>; Christian König <[email protected]>;
> [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]
> 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_xxxx to hwd_affine_xxxx
> >
> > MAINTAINERS: Add entries for Toshiba Visconti AFFINE image processing
> accelerator
> > v1 -> v2:
> > - No update
> >
> > Change in V2:
> > - apply checkpatch.pl --strict
> > - rename hwd_AFFINE_xxxx to hwd_affine_xxxx
> >
> > 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, 1111 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