mbox series

[v10,00/11] Rockchip ISP Driver

Message ID 20191107005617.12302-1-helen.koike@collabora.com (mailing list archive)
Headers show
Series Rockchip ISP Driver | expand

Message

Helen Koike Nov. 7, 2019, 12:56 a.m. UTC
Hello,

This series adds the Rockchip Image Signal Processing Unit v1 driver to
staging.

The main reason to be in staging is that people are already using it from the
mailing list (including libcamera), and having it in mainlin makes the workflow
easier. Also, it is easier for other people to contribute back (with code
or testing the driver).

We plan to actively work on this driver to get it our of staging.

This patchset is also available at:
https://gitlab.collabora.com/koike/linux/tree/rockchip/isp/v10

Libcamera patched to work with this version:
https://gitlab.collabora.com/koike/libcamera
(also sent to the mailing list)

The major difference in v10 are:
* I splitted the patches again in different commits, since it was too big for
the media mailing list and also it's easier to get dt-bindings approval.
* We don't expose the metadata pixelformats to the uapi yet, so  patch
"media: videodev2.h, v4l2-ioctl: add rkisp1 meta buffer format" was
removed from the series.
* TODO list was updated to remind to test uapi structs in different
architectures.

This series only touches MAINTAINERS file and drivers/staging/

MAINTAINERS
drivers/staging/media/Kconfig
drivers/staging/media/Makefile
drivers/staging/media/phy-rockchip-dphy/Kconfig
drivers/staging/media/phy-rockchip-dphy/Makefile
drivers/staging/media/phy-rockchip-dphy/TODO
drivers/staging/media/phy-rockchip-dphy/phy-rockchip-dphy.c
drivers/staging/media/rkisp1/Documentation/devicetree/bindings/media/rockchip-isp1.txt
drivers/staging/media/rkisp1/Documentation/devicetree/bindings/media/rockchip-mipi-dphy.txt
drivers/staging/media/rkisp1/Documentation/media/uapi/v4l/pixfmt-meta-rkisp1-params.rst
drivers/staging/media/rkisp1/Documentation/media/uapi/v4l/pixfmt-meta-rkisp1-stat.rst
drivers/staging/media/rkisp1/Kconfig
drivers/staging/media/rkisp1/Makefile
drivers/staging/media/rkisp1/TODO
drivers/staging/media/rkisp1/capture.c
drivers/staging/media/rkisp1/capture.h
drivers/staging/media/rkisp1/common.h
drivers/staging/media/rkisp1/dev.c
drivers/staging/media/rkisp1/dev.h
drivers/staging/media/rkisp1/isp_params.c
drivers/staging/media/rkisp1/isp_params.h
drivers/staging/media/rkisp1/isp_stats.c
drivers/staging/media/rkisp1/isp_stats.h
drivers/staging/media/rkisp1/regs.c
drivers/staging/media/rkisp1/regs.h
drivers/staging/media/rkisp1/rkisp1.c
drivers/staging/media/rkisp1/rkisp1.h
drivers/staging/media/rkisp1/uapi/rkisp1-config.h

Two drivers were added, including a TODO list for removing it from
staging:

* phy-rockchip-dphy: mipi dphy driver used by csi
* rkisp1: the image signal processing unit driver

Any feedback is welcome.

Thanks,
Helen

Helen Koike (1):
  MAINTAINERS: add entry for Rockchip ISP1 driver

Jacob Chen (9):
  media: staging: phy-rockchip-dphy: add Rockchip MIPI Synopsys DPHY
    driver
  media: staging: rkisp1: add document for rkisp1 meta buffer format
  media: staging: rkisp1: add Rockchip ISP1 subdev driver
  media: staging: rkisp1: add ISP1 statistics driver
  media: staging: rkisp1: add ISP1 params driver
  media: staging: rkisp1: add capture device driver
  media: staging: rkisp1: add rockchip isp1 core driver
  media: staging: dt-bindings: Document the Rockchip ISP1 bindings
  media: staging: dt-bindings: Document the Rockchip MIPI RX D-PHY
    bindings

Jeffy Chen (1):
  media: staging: rkisp1: add user space ABI definitions

 MAINTAINERS                                   |    6 +
 drivers/staging/media/Kconfig                 |    4 +
 drivers/staging/media/Makefile                |    2 +
 .../staging/media/phy-rockchip-dphy/Kconfig   |   11 +
 .../staging/media/phy-rockchip-dphy/Makefile  |    2 +
 drivers/staging/media/phy-rockchip-dphy/TODO  |    6 +
 .../phy-rockchip-dphy/phy-rockchip-dphy.c     |  400 ++++
 .../bindings/media/rockchip-isp1.txt          |   71 +
 .../bindings/media/rockchip-mipi-dphy.txt     |   38 +
 .../uapi/v4l/pixfmt-meta-rkisp1-params.rst    |   23 +
 .../uapi/v4l/pixfmt-meta-rkisp1-stat.rst      |   22 +
 drivers/staging/media/rkisp1/Kconfig          |   13 +
 drivers/staging/media/rkisp1/Makefile         |    7 +
 drivers/staging/media/rkisp1/TODO             |   23 +
 drivers/staging/media/rkisp1/capture.c        | 1869 +++++++++++++++++
 drivers/staging/media/rkisp1/capture.h        |  164 ++
 drivers/staging/media/rkisp1/common.h         |   98 +
 drivers/staging/media/rkisp1/dev.c            |  439 ++++
 drivers/staging/media/rkisp1/dev.h            |   67 +
 drivers/staging/media/rkisp1/isp_params.c     | 1604 ++++++++++++++
 drivers/staging/media/rkisp1/isp_params.h     |   50 +
 drivers/staging/media/rkisp1/isp_stats.c      |  494 +++++
 drivers/staging/media/rkisp1/isp_stats.h      |   60 +
 drivers/staging/media/rkisp1/regs.c           |  224 ++
 drivers/staging/media/rkisp1/regs.h           | 1525 ++++++++++++++
 drivers/staging/media/rkisp1/rkisp1.c         | 1246 +++++++++++
 drivers/staging/media/rkisp1/rkisp1.h         |   97 +
 .../staging/media/rkisp1/uapi/rkisp1-config.h |  819 ++++++++
 28 files changed, 9384 insertions(+)
 create mode 100644 drivers/staging/media/phy-rockchip-dphy/Kconfig
 create mode 100644 drivers/staging/media/phy-rockchip-dphy/Makefile
 create mode 100644 drivers/staging/media/phy-rockchip-dphy/TODO
 create mode 100644 drivers/staging/media/phy-rockchip-dphy/phy-rockchip-dphy.c
 create mode 100644 drivers/staging/media/rkisp1/Documentation/devicetree/bindings/media/rockchip-isp1.txt
 create mode 100644 drivers/staging/media/rkisp1/Documentation/devicetree/bindings/media/rockchip-mipi-dphy.txt
 create mode 100644 drivers/staging/media/rkisp1/Documentation/media/uapi/v4l/pixfmt-meta-rkisp1-params.rst
 create mode 100644 drivers/staging/media/rkisp1/Documentation/media/uapi/v4l/pixfmt-meta-rkisp1-stat.rst
 create mode 100644 drivers/staging/media/rkisp1/Kconfig
 create mode 100644 drivers/staging/media/rkisp1/Makefile
 create mode 100644 drivers/staging/media/rkisp1/TODO
 create mode 100644 drivers/staging/media/rkisp1/capture.c
 create mode 100644 drivers/staging/media/rkisp1/capture.h
 create mode 100644 drivers/staging/media/rkisp1/common.h
 create mode 100644 drivers/staging/media/rkisp1/dev.c
 create mode 100644 drivers/staging/media/rkisp1/dev.h
 create mode 100644 drivers/staging/media/rkisp1/isp_params.c
 create mode 100644 drivers/staging/media/rkisp1/isp_params.h
 create mode 100644 drivers/staging/media/rkisp1/isp_stats.c
 create mode 100644 drivers/staging/media/rkisp1/isp_stats.h
 create mode 100644 drivers/staging/media/rkisp1/regs.c
 create mode 100644 drivers/staging/media/rkisp1/regs.h
 create mode 100644 drivers/staging/media/rkisp1/rkisp1.c
 create mode 100644 drivers/staging/media/rkisp1/rkisp1.h
 create mode 100644 drivers/staging/media/rkisp1/uapi/rkisp1-config.h

Comments

Hans Verkuil Nov. 9, 2019, 3:07 p.m. UTC | #1
Hi Helen,

On 11/7/19 1:56 AM, Helen Koike wrote:
> Hello,
> 
> This series adds the Rockchip Image Signal Processing Unit v1 driver to
> staging.
> 
> The main reason to be in staging is that people are already using it from the
> mailing list (including libcamera), and having it in mainlin makes the workflow
> easier. Also, it is easier for other people to contribute back (with code
> or testing the driver).
> 
> We plan to actively work on this driver to get it our of staging.
> 
> This patchset is also available at:
> https://gitlab.collabora.com/koike/linux/tree/rockchip/isp/v10
> 
> Libcamera patched to work with this version:
> https://gitlab.collabora.com/koike/libcamera
> (also sent to the mailing list)
> 
> The major difference in v10 are:
> * I splitted the patches again in different commits, since it was too big for
> the media mailing list and also it's easier to get dt-bindings approval.
> * We don't expose the metadata pixelformats to the uapi yet, so  patch
> "media: videodev2.h, v4l2-ioctl: add rkisp1 meta buffer format" was
> removed from the series.
> * TODO list was updated to remind to test uapi structs in different
> architectures.
> 
> This series only touches MAINTAINERS file and drivers/staging/
> 
> MAINTAINERS
> drivers/staging/media/Kconfig
> drivers/staging/media/Makefile
> drivers/staging/media/phy-rockchip-dphy/Kconfig
> drivers/staging/media/phy-rockchip-dphy/Makefile
> drivers/staging/media/phy-rockchip-dphy/TODO
> drivers/staging/media/phy-rockchip-dphy/phy-rockchip-dphy.c
> drivers/staging/media/rkisp1/Documentation/devicetree/bindings/media/rockchip-isp1.txt
> drivers/staging/media/rkisp1/Documentation/devicetree/bindings/media/rockchip-mipi-dphy.txt
> drivers/staging/media/rkisp1/Documentation/media/uapi/v4l/pixfmt-meta-rkisp1-params.rst
> drivers/staging/media/rkisp1/Documentation/media/uapi/v4l/pixfmt-meta-rkisp1-stat.rst
> drivers/staging/media/rkisp1/Kconfig
> drivers/staging/media/rkisp1/Makefile
> drivers/staging/media/rkisp1/TODO
> drivers/staging/media/rkisp1/capture.c
> drivers/staging/media/rkisp1/capture.h
> drivers/staging/media/rkisp1/common.h
> drivers/staging/media/rkisp1/dev.c
> drivers/staging/media/rkisp1/dev.h
> drivers/staging/media/rkisp1/isp_params.c
> drivers/staging/media/rkisp1/isp_params.h
> drivers/staging/media/rkisp1/isp_stats.c
> drivers/staging/media/rkisp1/isp_stats.h
> drivers/staging/media/rkisp1/regs.c
> drivers/staging/media/rkisp1/regs.h
> drivers/staging/media/rkisp1/rkisp1.c
> drivers/staging/media/rkisp1/rkisp1.h
> drivers/staging/media/rkisp1/uapi/rkisp1-config.h
> 
> Two drivers were added, including a TODO list for removing it from
> staging:
> 
> * phy-rockchip-dphy: mipi dphy driver used by csi
> * rkisp1: the image signal processing unit driver
> 
> Any feedback is welcome.

I get a lot of checkpatch.pl --strict warnings for this series that for the most
part seem very trivial. Please fix these, it's much easier to maintain the driver
if this is done before merging it.

I also get these compile warnings:

drivers/staging/media/rkisp1/isp_stats.c: In function ‘rkisp1_register_stats_vdev’:
drivers/staging/media/rkisp1/isp_stats.c:452:10: warning: comparison of unsigned expression < 0 is always false [-Wtype-limits]
  452 |  if (ret < 0)
      |          ^
drivers/staging/media/rkisp1/isp_stats.c:456:10: warning: comparison of unsigned expression < 0 is always false [-Wtype-limits]
  456 |  if (ret < 0) {
      |          ^
drivers/staging/media/rkisp1/rkisp1.c: In function ‘rkisp1_config_isp’:
drivers/staging/media/rkisp1/rkisp1.c:154:20: warning: variable ‘out_crop’ set but not used [-Wunused-but-set-variable]
  154 |  struct v4l2_rect *out_crop, *in_crop;
      |                    ^~~~~~~~

I also got these sparse warnings:

SPARSE:/home/hans/work/build/media-git/drivers/staging/media/rkisp1/isp_params.c
/home/hans/work/build/media-git/drivers/staging/media/rkisp1/isp_params.c:1511:29:  warning: symbol 'rkisp1_params_fops' was not declared.
Should it be static?
SPARSE:/home/hans/work/build/media-git/drivers/staging/media/rkisp1/rkisp1.c
/home/hans/work/build/media-git/drivers/staging/media/rkisp1/rkisp1.c:943:43:  warning: missing braces around initializer

Regards,

	Hans

> 
> Thanks,
> Helen
> 
> Helen Koike (1):
>   MAINTAINERS: add entry for Rockchip ISP1 driver
> 
> Jacob Chen (9):
>   media: staging: phy-rockchip-dphy: add Rockchip MIPI Synopsys DPHY
>     driver
>   media: staging: rkisp1: add document for rkisp1 meta buffer format
>   media: staging: rkisp1: add Rockchip ISP1 subdev driver
>   media: staging: rkisp1: add ISP1 statistics driver
>   media: staging: rkisp1: add ISP1 params driver
>   media: staging: rkisp1: add capture device driver
>   media: staging: rkisp1: add rockchip isp1 core driver
>   media: staging: dt-bindings: Document the Rockchip ISP1 bindings
>   media: staging: dt-bindings: Document the Rockchip MIPI RX D-PHY
>     bindings
> 
> Jeffy Chen (1):
>   media: staging: rkisp1: add user space ABI definitions
> 
>  MAINTAINERS                                   |    6 +
>  drivers/staging/media/Kconfig                 |    4 +
>  drivers/staging/media/Makefile                |    2 +
>  .../staging/media/phy-rockchip-dphy/Kconfig   |   11 +
>  .../staging/media/phy-rockchip-dphy/Makefile  |    2 +
>  drivers/staging/media/phy-rockchip-dphy/TODO  |    6 +
>  .../phy-rockchip-dphy/phy-rockchip-dphy.c     |  400 ++++
>  .../bindings/media/rockchip-isp1.txt          |   71 +
>  .../bindings/media/rockchip-mipi-dphy.txt     |   38 +
>  .../uapi/v4l/pixfmt-meta-rkisp1-params.rst    |   23 +
>  .../uapi/v4l/pixfmt-meta-rkisp1-stat.rst      |   22 +
>  drivers/staging/media/rkisp1/Kconfig          |   13 +
>  drivers/staging/media/rkisp1/Makefile         |    7 +
>  drivers/staging/media/rkisp1/TODO             |   23 +
>  drivers/staging/media/rkisp1/capture.c        | 1869 +++++++++++++++++
>  drivers/staging/media/rkisp1/capture.h        |  164 ++
>  drivers/staging/media/rkisp1/common.h         |   98 +
>  drivers/staging/media/rkisp1/dev.c            |  439 ++++
>  drivers/staging/media/rkisp1/dev.h            |   67 +
>  drivers/staging/media/rkisp1/isp_params.c     | 1604 ++++++++++++++
>  drivers/staging/media/rkisp1/isp_params.h     |   50 +
>  drivers/staging/media/rkisp1/isp_stats.c      |  494 +++++
>  drivers/staging/media/rkisp1/isp_stats.h      |   60 +
>  drivers/staging/media/rkisp1/regs.c           |  224 ++
>  drivers/staging/media/rkisp1/regs.h           | 1525 ++++++++++++++
>  drivers/staging/media/rkisp1/rkisp1.c         | 1246 +++++++++++
>  drivers/staging/media/rkisp1/rkisp1.h         |   97 +
>  .../staging/media/rkisp1/uapi/rkisp1-config.h |  819 ++++++++
>  28 files changed, 9384 insertions(+)
>  create mode 100644 drivers/staging/media/phy-rockchip-dphy/Kconfig
>  create mode 100644 drivers/staging/media/phy-rockchip-dphy/Makefile
>  create mode 100644 drivers/staging/media/phy-rockchip-dphy/TODO
>  create mode 100644 drivers/staging/media/phy-rockchip-dphy/phy-rockchip-dphy.c
>  create mode 100644 drivers/staging/media/rkisp1/Documentation/devicetree/bindings/media/rockchip-isp1.txt
>  create mode 100644 drivers/staging/media/rkisp1/Documentation/devicetree/bindings/media/rockchip-mipi-dphy.txt
>  create mode 100644 drivers/staging/media/rkisp1/Documentation/media/uapi/v4l/pixfmt-meta-rkisp1-params.rst
>  create mode 100644 drivers/staging/media/rkisp1/Documentation/media/uapi/v4l/pixfmt-meta-rkisp1-stat.rst
>  create mode 100644 drivers/staging/media/rkisp1/Kconfig
>  create mode 100644 drivers/staging/media/rkisp1/Makefile
>  create mode 100644 drivers/staging/media/rkisp1/TODO
>  create mode 100644 drivers/staging/media/rkisp1/capture.c
>  create mode 100644 drivers/staging/media/rkisp1/capture.h
>  create mode 100644 drivers/staging/media/rkisp1/common.h
>  create mode 100644 drivers/staging/media/rkisp1/dev.c
>  create mode 100644 drivers/staging/media/rkisp1/dev.h
>  create mode 100644 drivers/staging/media/rkisp1/isp_params.c
>  create mode 100644 drivers/staging/media/rkisp1/isp_params.h
>  create mode 100644 drivers/staging/media/rkisp1/isp_stats.c
>  create mode 100644 drivers/staging/media/rkisp1/isp_stats.h
>  create mode 100644 drivers/staging/media/rkisp1/regs.c
>  create mode 100644 drivers/staging/media/rkisp1/regs.h
>  create mode 100644 drivers/staging/media/rkisp1/rkisp1.c
>  create mode 100644 drivers/staging/media/rkisp1/rkisp1.h
>  create mode 100644 drivers/staging/media/rkisp1/uapi/rkisp1-config.h
>