[00/14] staging: vc04_services: bcm2835-isp support

  This series aims to upport bcm2835-isp from the RPi kernel [1] and is a
independent subset of earlier series [2] posted to upport CSI-2/CCP2
receiver IP core("Unicam) + the ISP driver found in BCM283x and compatible
SoCs (namely BCM2711). Unicam is still under active development to work
with multistream support to get into mainline. Hence only the ISP driver
will remain the primary area of this series.

Patch (01-02)/14  adds a new driver named vc-sm-cma to handle memory sharing
with the VC4 VPU. 

Patch 03/14 adds a small extension to videobuf2 to allow exporting as a
dma_buf instead of a file-descriptor.

Patch (04-05)/14 adds a couple of improvements/support for
bcm2835-isp(event callback and zero-copy) to vchiq-mmal.

Patch (06-09)/14 adds the core bcm2835-isp driver along with headers
and format defintions.

Patch (10-11)/14 deals with the colorspace support.
Note: This is still WIP since the implementation of colorspace is still
getting ironed out (especially around setting of colorspace flags handling).

Patch 12/14 allows multiple instances of the ISP.

Patch 13/14 adds a admin-guide document on bcm2835-isp.

Patch 14/14 wires all this up with the vchiq-mmal driver.

Testing:
Tested with RPi Model 4B running linux mainline v6.1-rc6. To test
end-to-end, I choose to cherry-pick the Unicam patches and OV5647 DT
bindings from [1]). Once done, functional testing was conducted with
libcamera[3] and its utility tools.

Also note: Reviews given on [2] for the relevant ISP driver patches have
been incorporated in this version.

Known issues:
- Colorspace handling 
- vc-sm-cma spamming kernel log with
	- pr_err("%s: Expecting an uncached alias for dma_addr %pad\n"

[1]: https://github.com/raspberrypi/linux
[2]: https://lore.kernel.org/linux-media/20200504092611.9798-1-laurent.pinchart@ideasonboard.com/
[3]: https://libcamera.org/getting-started.html

Dave Stevenson (7):
  staging: vc04_services: Add new vc-sm-cma driver
  staging: vchiq_arm: Register vcsm-cma as a platform driver
  media: videobuf2: Allow exporting of a struct dmabuf
  staging: mmal-vchiq: Add support for event callbacks
  staging: mmal-vchiq: Use vc-sm-cma to support zero copy
  staging: mmal_vchiq: Add image formats to be used by bcm2835-isp
  uapi: bcm2835-isp: Add bcm2835-isp uapi header file

David Plowman (2):
  vc04_services: bcm2835-isp: Allow formats with different colour spaces
  vc04_services: bcm2835-isp: Permit all sRGB colour spaces on ISP
    outputs

Naushir Patuck (5):
  media: uapi: v4l2-core: Add ISP statistics output V4L2 fourcc type
  staging: vc04_services: bcm2835-isp: Add a more complex ISP processing
    component
  staging: vc04_services: bcm2835_isp: Allow multiple users
  docs: admin-guide: media: bcm2835-isp: Add documentation for
    bcm2835-isp
  staging: vc04_services: vchiq: Load bcm2835_isp driver from vchiq

 .../admin-guide/media/bcm2835-isp.rst         |  127 ++
 .../userspace-api/media/drivers/index.rst     |    1 +
 .../userspace-api/media/v4l/meta-formats.rst  |    1 +
 .../v4l/pixfmt-meta-bcm2835-isp-stats.rst     |   41 +
 MAINTAINERS                                   |    9 +
 .../media/common/videobuf2/videobuf2-core.c   |   36 +-
 drivers/media/v4l2-core/v4l2-ioctl.c          |    1 +
 drivers/staging/vc04_services/Kconfig         |    4 +
 drivers/staging/vc04_services/Makefile        |    2 +
 .../staging/vc04_services/bcm2835-isp/Kconfig |   14 +
 .../vc04_services/bcm2835-isp/Makefile        |    8 +
 .../bcm2835-isp/bcm2835-isp-ctrls.h           |   72 +
 .../bcm2835-isp/bcm2835-isp-fmts.h            |  558 +++++
 .../bcm2835-isp/bcm2835-v4l2-isp.c            | 1817 +++++++++++++++++
 .../interface/vchiq_arm/vchiq_arm.c           |    6 +
 .../staging/vc04_services/vc-sm-cma/Kconfig   |   10 +
 .../staging/vc04_services/vc-sm-cma/Makefile  |   12 +
 .../staging/vc04_services/vc-sm-cma/vc_sm.c   |  801 ++++++++
 .../staging/vc04_services/vc-sm-cma/vc_sm.h   |   54 +
 .../vc04_services/vc-sm-cma/vc_sm_cma_vchi.c  |  507 +++++
 .../vc04_services/vc-sm-cma/vc_sm_cma_vchi.h  |   63 +
 .../vc04_services/vc-sm-cma/vc_sm_defs.h      |  187 ++
 .../vc04_services/vc-sm-cma/vc_sm_knl.h       |   28 +
 .../staging/vc04_services/vchiq-mmal/Kconfig  |    1 +
 .../vc04_services/vchiq-mmal/mmal-common.h    |    5 +
 .../vc04_services/vchiq-mmal/mmal-encodings.h |   66 +
 .../vc04_services/vchiq-mmal/mmal-msg.h       |   35 +
 .../vchiq-mmal/mmal-parameters.h              |  165 +-
 .../vc04_services/vchiq-mmal/mmal-vchiq.c     |  253 ++-
 .../vc04_services/vchiq-mmal/mmal-vchiq.h     |    5 +
 include/media/videobuf2-core.h                |   15 +
 include/uapi/linux/bcm2835-isp.h              |  347 ++++
 include/uapi/linux/v4l2-controls.h            |    6 +
 include/uapi/linux/videodev2.h                |    1 +
 34 files changed, 5225 insertions(+), 33 deletions(-)
 create mode 100644 Documentation/admin-guide/media/bcm2835-isp.rst
 create mode 100644 Documentation/userspace-api/media/v4l/pixfmt-meta-bcm2835-isp-stats.rst
 create mode 100644 drivers/staging/vc04_services/bcm2835-isp/Kconfig
 create mode 100644 drivers/staging/vc04_services/bcm2835-isp/Makefile
 create mode 100644 drivers/staging/vc04_services/bcm2835-isp/bcm2835-isp-ctrls.h
 create mode 100644 drivers/staging/vc04_services/bcm2835-isp/bcm2835-isp-fmts.h
 create mode 100644 drivers/staging/vc04_services/bcm2835-isp/bcm2835-v4l2-isp.c
 create mode 100644 drivers/staging/vc04_services/vc-sm-cma/Kconfig
 create mode 100644 drivers/staging/vc04_services/vc-sm-cma/Makefile
 create mode 100644 drivers/staging/vc04_services/vc-sm-cma/vc_sm.c
 create mode 100644 drivers/staging/vc04_services/vc-sm-cma/vc_sm.h
 create mode 100644 drivers/staging/vc04_services/vc-sm-cma/vc_sm_cma_vchi.c
 create mode 100644 drivers/staging/vc04_services/vc-sm-cma/vc_sm_cma_vchi.h
 create mode 100644 drivers/staging/vc04_services/vc-sm-cma/vc_sm_defs.h
 create mode 100644 drivers/staging/vc04_services/vc-sm-cma/vc_sm_knl.h
 create mode 100644 include/uapi/linux/bcm2835-isp.h
  

Hi Umang,

Nice to see this series on the list !

On Tue, Nov 22, 2022 at 03:17:08AM +0530, Umang Jain wrote:
> This series aims to upport bcm2835-isp from the RPi kernel [1] and is a
> independent subset of earlier series [2] posted to upport CSI-2/CCP2
> receiver IP core("Unicam) + the ISP driver found in BCM283x and compatible
> SoCs (namely BCM2711). Unicam is still under active development to work
> with multistream support to get into mainline. Hence only the ISP driver
> will remain the primary area of this series.
> 
> Patch (01-02)/14  adds a new driver named vc-sm-cma to handle memory sharing
> with the VC4 VPU. 
> 
> Patch 03/14 adds a small extension to videobuf2 to allow exporting as a
> dma_buf instead of a file-descriptor.
> 
> Patch (04-05)/14 adds a couple of improvements/support for
> bcm2835-isp(event callback and zero-copy) to vchiq-mmal.
> 
> Patch (06-09)/14 adds the core bcm2835-isp driver along with headers
> and format defintions.
> 
> Patch (10-11)/14 deals with the colorspace support.
> Note: This is still WIP since the implementation of colorspace is still
> getting ironed out (especially around setting of colorspace flags handling).
> 
> Patch 12/14 allows multiple instances of the ISP.
> 
> Patch 13/14 adds a admin-guide document on bcm2835-isp.
> 
> Patch 14/14 wires all this up with the vchiq-mmal driver.
> 
> Testing:
> Tested with RPi Model 4B running linux mainline v6.1-rc6. To test
> end-to-end, I choose to cherry-pick the Unicam patches and OV5647 DT
> bindings from [1]). Once done, functional testing was conducted with
> libcamera[3] and its utility tools.
> 
> Also note: Reviews given on [2] for the relevant ISP driver patches have
> been incorporated in this version.
> 
> Known issues:
> - Colorspace handling 

This will require further discussions, I'll try to comment on this topic
in the review of the ISP driver patch.

> - vc-sm-cma spamming kernel log with
> 	- pr_err("%s: Expecting an uncached alias for dma_addr %pad\n"

Do you have any plan to address this ? Is the root cause known ?

> [1]: https://github.com/raspberrypi/linux
> [2]: https://lore.kernel.org/linux-media/20200504092611.9798-1-laurent.pinchart@ideasonboard.com/
> [3]: https://libcamera.org/getting-started.html
> 
> Dave Stevenson (7):
>   staging: vc04_services: Add new vc-sm-cma driver
>   staging: vchiq_arm: Register vcsm-cma as a platform driver
>   media: videobuf2: Allow exporting of a struct dmabuf
>   staging: mmal-vchiq: Add support for event callbacks
>   staging: mmal-vchiq: Use vc-sm-cma to support zero copy
>   staging: mmal_vchiq: Add image formats to be used by bcm2835-isp
>   uapi: bcm2835-isp: Add bcm2835-isp uapi header file
> 
> David Plowman (2):
>   vc04_services: bcm2835-isp: Allow formats with different colour spaces
>   vc04_services: bcm2835-isp: Permit all sRGB colour spaces on ISP
>     outputs
> 
> Naushir Patuck (5):
>   media: uapi: v4l2-core: Add ISP statistics output V4L2 fourcc type
>   staging: vc04_services: bcm2835-isp: Add a more complex ISP processing
>     component
>   staging: vc04_services: bcm2835_isp: Allow multiple users
>   docs: admin-guide: media: bcm2835-isp: Add documentation for
>     bcm2835-isp
>   staging: vc04_services: vchiq: Load bcm2835_isp driver from vchiq
> 
>  .../admin-guide/media/bcm2835-isp.rst         |  127 ++
>  .../userspace-api/media/drivers/index.rst     |    1 +
>  .../userspace-api/media/v4l/meta-formats.rst  |    1 +
>  .../v4l/pixfmt-meta-bcm2835-isp-stats.rst     |   41 +
>  MAINTAINERS                                   |    9 +
>  .../media/common/videobuf2/videobuf2-core.c   |   36 +-
>  drivers/media/v4l2-core/v4l2-ioctl.c          |    1 +
>  drivers/staging/vc04_services/Kconfig         |    4 +
>  drivers/staging/vc04_services/Makefile        |    2 +
>  .../staging/vc04_services/bcm2835-isp/Kconfig |   14 +
>  .../vc04_services/bcm2835-isp/Makefile        |    8 +
>  .../bcm2835-isp/bcm2835-isp-ctrls.h           |   72 +
>  .../bcm2835-isp/bcm2835-isp-fmts.h            |  558 +++++
>  .../bcm2835-isp/bcm2835-v4l2-isp.c            | 1817 +++++++++++++++++
>  .../interface/vchiq_arm/vchiq_arm.c           |    6 +
>  .../staging/vc04_services/vc-sm-cma/Kconfig   |   10 +
>  .../staging/vc04_services/vc-sm-cma/Makefile  |   12 +
>  .../staging/vc04_services/vc-sm-cma/vc_sm.c   |  801 ++++++++
>  .../staging/vc04_services/vc-sm-cma/vc_sm.h   |   54 +
>  .../vc04_services/vc-sm-cma/vc_sm_cma_vchi.c  |  507 +++++
>  .../vc04_services/vc-sm-cma/vc_sm_cma_vchi.h  |   63 +
>  .../vc04_services/vc-sm-cma/vc_sm_defs.h      |  187 ++
>  .../vc04_services/vc-sm-cma/vc_sm_knl.h       |   28 +
>  .../staging/vc04_services/vchiq-mmal/Kconfig  |    1 +
>  .../vc04_services/vchiq-mmal/mmal-common.h    |    5 +
>  .../vc04_services/vchiq-mmal/mmal-encodings.h |   66 +
>  .../vc04_services/vchiq-mmal/mmal-msg.h       |   35 +
>  .../vchiq-mmal/mmal-parameters.h              |  165 +-
>  .../vc04_services/vchiq-mmal/mmal-vchiq.c     |  253 ++-
>  .../vc04_services/vchiq-mmal/mmal-vchiq.h     |    5 +
>  include/media/videobuf2-core.h                |   15 +
>  include/uapi/linux/bcm2835-isp.h              |  347 ++++
>  include/uapi/linux/v4l2-controls.h            |    6 +
>  include/uapi/linux/videodev2.h                |    1 +
>  34 files changed, 5225 insertions(+), 33 deletions(-)
>  create mode 100644 Documentation/admin-guide/media/bcm2835-isp.rst
>  create mode 100644 Documentation/userspace-api/media/v4l/pixfmt-meta-bcm2835-isp-stats.rst
>  create mode 100644 drivers/staging/vc04_services/bcm2835-isp/Kconfig
>  create mode 100644 drivers/staging/vc04_services/bcm2835-isp/Makefile
>  create mode 100644 drivers/staging/vc04_services/bcm2835-isp/bcm2835-isp-ctrls.h
>  create mode 100644 drivers/staging/vc04_services/bcm2835-isp/bcm2835-isp-fmts.h
>  create mode 100644 drivers/staging/vc04_services/bcm2835-isp/bcm2835-v4l2-isp.c
>  create mode 100644 drivers/staging/vc04_services/vc-sm-cma/Kconfig
>  create mode 100644 drivers/staging/vc04_services/vc-sm-cma/Makefile
>  create mode 100644 drivers/staging/vc04_services/vc-sm-cma/vc_sm.c
>  create mode 100644 drivers/staging/vc04_services/vc-sm-cma/vc_sm.h
>  create mode 100644 drivers/staging/vc04_services/vc-sm-cma/vc_sm_cma_vchi.c
>  create mode 100644 drivers/staging/vc04_services/vc-sm-cma/vc_sm_cma_vchi.h
>  create mode 100644 drivers/staging/vc04_services/vc-sm-cma/vc_sm_defs.h
>  create mode 100644 drivers/staging/vc04_services/vc-sm-cma/vc_sm_knl.h
>  create mode 100644 include/uapi/linux/bcm2835-isp.h
  

Hi Umang and Laurent

On Mon, 21 Nov 2022 at 22:16, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> Hi Umang,
>
> Nice to see this series on the list !
>
> On Tue, Nov 22, 2022 at 03:17:08AM +0530, Umang Jain wrote:
> > This series aims to upport bcm2835-isp from the RPi kernel [1] and is a
> > independent subset of earlier series [2] posted to upport CSI-2/CCP2
> > receiver IP core("Unicam) + the ISP driver found in BCM283x and compatible
> > SoCs (namely BCM2711). Unicam is still under active development to work
> > with multistream support to get into mainline. Hence only the ISP driver
> > will remain the primary area of this series.
> >
> > Patch (01-02)/14  adds a new driver named vc-sm-cma to handle memory sharing
> > with the VC4 VPU.
> >
> > Patch 03/14 adds a small extension to videobuf2 to allow exporting as a
> > dma_buf instead of a file-descriptor.
> >
> > Patch (04-05)/14 adds a couple of improvements/support for
> > bcm2835-isp(event callback and zero-copy) to vchiq-mmal.
> >
> > Patch (06-09)/14 adds the core bcm2835-isp driver along with headers
> > and format defintions.
> >
> > Patch (10-11)/14 deals with the colorspace support.
> > Note: This is still WIP since the implementation of colorspace is still
> > getting ironed out (especially around setting of colorspace flags handling).
> >
> > Patch 12/14 allows multiple instances of the ISP.
> >
> > Patch 13/14 adds a admin-guide document on bcm2835-isp.
> >
> > Patch 14/14 wires all this up with the vchiq-mmal driver.
> >
> > Testing:
> > Tested with RPi Model 4B running linux mainline v6.1-rc6. To test
> > end-to-end, I choose to cherry-pick the Unicam patches and OV5647 DT
> > bindings from [1]). Once done, functional testing was conducted with
> > libcamera[3] and its utility tools.
> >
> > Also note: Reviews given on [2] for the relevant ISP driver patches have
> > been incorporated in this version.
> >
> > Known issues:
> > - Colorspace handling
>
> This will require further discussions, I'll try to comment on this topic
> in the review of the ISP driver patch.
>
> > - vc-sm-cma spamming kernel log with
> >       - pr_err("%s: Expecting an uncached alias for dma_addr %pad\n"
>
> Do you have any plan to address this ? Is the root cause known ?

You've picked up an old version of the downstream driver.
Pi0&1 share the VPU L2 cache with the ARM in the architecture, so they
use the 0x8 cache alias.
See https://github.com/raspberrypi/linux/commit/e22927f8ec9dc87772ac61d6aba00dc8046b4f49

  Dave

> > [1]: https://github.com/raspberrypi/linux
> > [2]: https://lore.kernel.org/linux-media/20200504092611.9798-1-laurent.pinchart@ideasonboard.com/
> > [3]: https://libcamera.org/getting-started.html
> >
> > Dave Stevenson (7):
> >   staging: vc04_services: Add new vc-sm-cma driver
> >   staging: vchiq_arm: Register vcsm-cma as a platform driver
> >   media: videobuf2: Allow exporting of a struct dmabuf
> >   staging: mmal-vchiq: Add support for event callbacks
> >   staging: mmal-vchiq: Use vc-sm-cma to support zero copy
> >   staging: mmal_vchiq: Add image formats to be used by bcm2835-isp
> >   uapi: bcm2835-isp: Add bcm2835-isp uapi header file
> >
> > David Plowman (2):
> >   vc04_services: bcm2835-isp: Allow formats with different colour spaces
> >   vc04_services: bcm2835-isp: Permit all sRGB colour spaces on ISP
> >     outputs
> >
> > Naushir Patuck (5):
> >   media: uapi: v4l2-core: Add ISP statistics output V4L2 fourcc type
> >   staging: vc04_services: bcm2835-isp: Add a more complex ISP processing
> >     component
> >   staging: vc04_services: bcm2835_isp: Allow multiple users
> >   docs: admin-guide: media: bcm2835-isp: Add documentation for
> >     bcm2835-isp
> >   staging: vc04_services: vchiq: Load bcm2835_isp driver from vchiq
> >
> >  .../admin-guide/media/bcm2835-isp.rst         |  127 ++
> >  .../userspace-api/media/drivers/index.rst     |    1 +
> >  .../userspace-api/media/v4l/meta-formats.rst  |    1 +
> >  .../v4l/pixfmt-meta-bcm2835-isp-stats.rst     |   41 +
> >  MAINTAINERS                                   |    9 +
> >  .../media/common/videobuf2/videobuf2-core.c   |   36 +-
> >  drivers/media/v4l2-core/v4l2-ioctl.c          |    1 +
> >  drivers/staging/vc04_services/Kconfig         |    4 +
> >  drivers/staging/vc04_services/Makefile        |    2 +
> >  .../staging/vc04_services/bcm2835-isp/Kconfig |   14 +
> >  .../vc04_services/bcm2835-isp/Makefile        |    8 +
> >  .../bcm2835-isp/bcm2835-isp-ctrls.h           |   72 +
> >  .../bcm2835-isp/bcm2835-isp-fmts.h            |  558 +++++
> >  .../bcm2835-isp/bcm2835-v4l2-isp.c            | 1817 +++++++++++++++++
> >  .../interface/vchiq_arm/vchiq_arm.c           |    6 +
> >  .../staging/vc04_services/vc-sm-cma/Kconfig   |   10 +
> >  .../staging/vc04_services/vc-sm-cma/Makefile  |   12 +
> >  .../staging/vc04_services/vc-sm-cma/vc_sm.c   |  801 ++++++++
> >  .../staging/vc04_services/vc-sm-cma/vc_sm.h   |   54 +
> >  .../vc04_services/vc-sm-cma/vc_sm_cma_vchi.c  |  507 +++++
> >  .../vc04_services/vc-sm-cma/vc_sm_cma_vchi.h  |   63 +
> >  .../vc04_services/vc-sm-cma/vc_sm_defs.h      |  187 ++
> >  .../vc04_services/vc-sm-cma/vc_sm_knl.h       |   28 +
> >  .../staging/vc04_services/vchiq-mmal/Kconfig  |    1 +
> >  .../vc04_services/vchiq-mmal/mmal-common.h    |    5 +
> >  .../vc04_services/vchiq-mmal/mmal-encodings.h |   66 +
> >  .../vc04_services/vchiq-mmal/mmal-msg.h       |   35 +
> >  .../vchiq-mmal/mmal-parameters.h              |  165 +-
> >  .../vc04_services/vchiq-mmal/mmal-vchiq.c     |  253 ++-
> >  .../vc04_services/vchiq-mmal/mmal-vchiq.h     |    5 +
> >  include/media/videobuf2-core.h                |   15 +
> >  include/uapi/linux/bcm2835-isp.h              |  347 ++++
> >  include/uapi/linux/v4l2-controls.h            |    6 +
> >  include/uapi/linux/videodev2.h                |    1 +
> >  34 files changed, 5225 insertions(+), 33 deletions(-)
> >  create mode 100644 Documentation/admin-guide/media/bcm2835-isp.rst
> >  create mode 100644 Documentation/userspace-api/media/v4l/pixfmt-meta-bcm2835-isp-stats.rst
> >  create mode 100644 drivers/staging/vc04_services/bcm2835-isp/Kconfig
> >  create mode 100644 drivers/staging/vc04_services/bcm2835-isp/Makefile
> >  create mode 100644 drivers/staging/vc04_services/bcm2835-isp/bcm2835-isp-ctrls.h
> >  create mode 100644 drivers/staging/vc04_services/bcm2835-isp/bcm2835-isp-fmts.h
> >  create mode 100644 drivers/staging/vc04_services/bcm2835-isp/bcm2835-v4l2-isp.c
> >  create mode 100644 drivers/staging/vc04_services/vc-sm-cma/Kconfig
> >  create mode 100644 drivers/staging/vc04_services/vc-sm-cma/Makefile
> >  create mode 100644 drivers/staging/vc04_services/vc-sm-cma/vc_sm.c
> >  create mode 100644 drivers/staging/vc04_services/vc-sm-cma/vc_sm.h
> >  create mode 100644 drivers/staging/vc04_services/vc-sm-cma/vc_sm_cma_vchi.c
> >  create mode 100644 drivers/staging/vc04_services/vc-sm-cma/vc_sm_cma_vchi.h
> >  create mode 100644 drivers/staging/vc04_services/vc-sm-cma/vc_sm_defs.h
> >  create mode 100644 drivers/staging/vc04_services/vc-sm-cma/vc_sm_knl.h
> >  create mode 100644 include/uapi/linux/bcm2835-isp.h
>
> --
> Regards,
>
> Laurent Pinchart
  

Hi Dave,

On 11/22/22 5:12 PM, Dave Stevenson wrote:
> Hi Umang and Laurent
>
> On Mon, 21 Nov 2022 at 22:16, Laurent Pinchart
> <laurent.pinchart@ideasonboard.com> wrote:
>> Hi Umang,
>>
>> Nice to see this series on the list !
>>
>> On Tue, Nov 22, 2022 at 03:17:08AM +0530, Umang Jain wrote:
>>> This series aims to upport bcm2835-isp from the RPi kernel [1] and is a
>>> independent subset of earlier series [2] posted to upport CSI-2/CCP2
>>> receiver IP core("Unicam) + the ISP driver found in BCM283x and compatible
>>> SoCs (namely BCM2711). Unicam is still under active development to work
>>> with multistream support to get into mainline. Hence only the ISP driver
>>> will remain the primary area of this series.
>>>
>>> Patch (01-02)/14  adds a new driver named vc-sm-cma to handle memory sharing
>>> with the VC4 VPU.
>>>
>>> Patch 03/14 adds a small extension to videobuf2 to allow exporting as a
>>> dma_buf instead of a file-descriptor.
>>>
>>> Patch (04-05)/14 adds a couple of improvements/support for
>>> bcm2835-isp(event callback and zero-copy) to vchiq-mmal.
>>>
>>> Patch (06-09)/14 adds the core bcm2835-isp driver along with headers
>>> and format defintions.
>>>
>>> Patch (10-11)/14 deals with the colorspace support.
>>> Note: This is still WIP since the implementation of colorspace is still
>>> getting ironed out (especially around setting of colorspace flags handling).
>>>
>>> Patch 12/14 allows multiple instances of the ISP.
>>>
>>> Patch 13/14 adds a admin-guide document on bcm2835-isp.
>>>
>>> Patch 14/14 wires all this up with the vchiq-mmal driver.
>>>
>>> Testing:
>>> Tested with RPi Model 4B running linux mainline v6.1-rc6. To test
>>> end-to-end, I choose to cherry-pick the Unicam patches and OV5647 DT
>>> bindings from [1]). Once done, functional testing was conducted with
>>> libcamera[3] and its utility tools.
>>>
>>> Also note: Reviews given on [2] for the relevant ISP driver patches have
>>> been incorporated in this version.
>>>
>>> Known issues:
>>> - Colorspace handling
>> This will require further discussions, I'll try to comment on this topic
>> in the review of the ISP driver patch.
>>
>>> - vc-sm-cma spamming kernel log with
>>>        - pr_err("%s: Expecting an uncached alias for dma_addr %pad\n"
>> Do you have any plan to address this ? Is the root cause known ?
> You've picked up an old version of the downstream driver.

I didn't pick version of driver rather, I picked commits that were 
leading to the rpi-5.15.y branch of the downstream kernel

> Pi0&1 share the VPU L2 cache with the ARM in the architecture, so they
> use the 0x8 cache alias.
> See https://github.com/raspberrypi/linux/commit/e22927f8ec9dc87772ac61d6aba00dc8046b4f49

And certainly I missed to notice / pick this commit out on the vc-sm-cma 
driver.
Thanks for providing the link! I'll squash it for v2.
>
>    Dave
>
>>> [1]: https://github.com/raspberrypi/linux
>>> [2]: https://lore.kernel.org/linux-media/20200504092611.9798-1-laurent.pinchart@ideasonboard.com/
>>> [3]: https://libcamera.org/getting-started.html
>>>
>>> Dave Stevenson (7):
>>>    staging: vc04_services: Add new vc-sm-cma driver
>>>    staging: vchiq_arm: Register vcsm-cma as a platform driver
>>>    media: videobuf2: Allow exporting of a struct dmabuf
>>>    staging: mmal-vchiq: Add support for event callbacks
>>>    staging: mmal-vchiq: Use vc-sm-cma to support zero copy
>>>    staging: mmal_vchiq: Add image formats to be used by bcm2835-isp
>>>    uapi: bcm2835-isp: Add bcm2835-isp uapi header file
>>>
>>> David Plowman (2):
>>>    vc04_services: bcm2835-isp: Allow formats with different colour spaces
>>>    vc04_services: bcm2835-isp: Permit all sRGB colour spaces on ISP
>>>      outputs
>>>
>>> Naushir Patuck (5):
>>>    media: uapi: v4l2-core: Add ISP statistics output V4L2 fourcc type
>>>    staging: vc04_services: bcm2835-isp: Add a more complex ISP processing
>>>      component
>>>    staging: vc04_services: bcm2835_isp: Allow multiple users
>>>    docs: admin-guide: media: bcm2835-isp: Add documentation for
>>>      bcm2835-isp
>>>    staging: vc04_services: vchiq: Load bcm2835_isp driver from vchiq
>>>
>>>   .../admin-guide/media/bcm2835-isp.rst         |  127 ++
>>>   .../userspace-api/media/drivers/index.rst     |    1 +
>>>   .../userspace-api/media/v4l/meta-formats.rst  |    1 +
>>>   .../v4l/pixfmt-meta-bcm2835-isp-stats.rst     |   41 +
>>>   MAINTAINERS                                   |    9 +
>>>   .../media/common/videobuf2/videobuf2-core.c   |   36 +-
>>>   drivers/media/v4l2-core/v4l2-ioctl.c          |    1 +
>>>   drivers/staging/vc04_services/Kconfig         |    4 +
>>>   drivers/staging/vc04_services/Makefile        |    2 +
>>>   .../staging/vc04_services/bcm2835-isp/Kconfig |   14 +
>>>   .../vc04_services/bcm2835-isp/Makefile        |    8 +
>>>   .../bcm2835-isp/bcm2835-isp-ctrls.h           |   72 +
>>>   .../bcm2835-isp/bcm2835-isp-fmts.h            |  558 +++++
>>>   .../bcm2835-isp/bcm2835-v4l2-isp.c            | 1817 +++++++++++++++++
>>>   .../interface/vchiq_arm/vchiq_arm.c           |    6 +
>>>   .../staging/vc04_services/vc-sm-cma/Kconfig   |   10 +
>>>   .../staging/vc04_services/vc-sm-cma/Makefile  |   12 +
>>>   .../staging/vc04_services/vc-sm-cma/vc_sm.c   |  801 ++++++++
>>>   .../staging/vc04_services/vc-sm-cma/vc_sm.h   |   54 +
>>>   .../vc04_services/vc-sm-cma/vc_sm_cma_vchi.c  |  507 +++++
>>>   .../vc04_services/vc-sm-cma/vc_sm_cma_vchi.h  |   63 +
>>>   .../vc04_services/vc-sm-cma/vc_sm_defs.h      |  187 ++
>>>   .../vc04_services/vc-sm-cma/vc_sm_knl.h       |   28 +
>>>   .../staging/vc04_services/vchiq-mmal/Kconfig  |    1 +
>>>   .../vc04_services/vchiq-mmal/mmal-common.h    |    5 +
>>>   .../vc04_services/vchiq-mmal/mmal-encodings.h |   66 +
>>>   .../vc04_services/vchiq-mmal/mmal-msg.h       |   35 +
>>>   .../vchiq-mmal/mmal-parameters.h              |  165 +-
>>>   .../vc04_services/vchiq-mmal/mmal-vchiq.c     |  253 ++-
>>>   .../vc04_services/vchiq-mmal/mmal-vchiq.h     |    5 +
>>>   include/media/videobuf2-core.h                |   15 +
>>>   include/uapi/linux/bcm2835-isp.h              |  347 ++++
>>>   include/uapi/linux/v4l2-controls.h            |    6 +
>>>   include/uapi/linux/videodev2.h                |    1 +
>>>   34 files changed, 5225 insertions(+), 33 deletions(-)
>>>   create mode 100644 Documentation/admin-guide/media/bcm2835-isp.rst
>>>   create mode 100644 Documentation/userspace-api/media/v4l/pixfmt-meta-bcm2835-isp-stats.rst
>>>   create mode 100644 drivers/staging/vc04_services/bcm2835-isp/Kconfig
>>>   create mode 100644 drivers/staging/vc04_services/bcm2835-isp/Makefile
>>>   create mode 100644 drivers/staging/vc04_services/bcm2835-isp/bcm2835-isp-ctrls.h
>>>   create mode 100644 drivers/staging/vc04_services/bcm2835-isp/bcm2835-isp-fmts.h
>>>   create mode 100644 drivers/staging/vc04_services/bcm2835-isp/bcm2835-v4l2-isp.c
>>>   create mode 100644 drivers/staging/vc04_services/vc-sm-cma/Kconfig
>>>   create mode 100644 drivers/staging/vc04_services/vc-sm-cma/Makefile
>>>   create mode 100644 drivers/staging/vc04_services/vc-sm-cma/vc_sm.c
>>>   create mode 100644 drivers/staging/vc04_services/vc-sm-cma/vc_sm.h
>>>   create mode 100644 drivers/staging/vc04_services/vc-sm-cma/vc_sm_cma_vchi.c
>>>   create mode 100644 drivers/staging/vc04_services/vc-sm-cma/vc_sm_cma_vchi.h
>>>   create mode 100644 drivers/staging/vc04_services/vc-sm-cma/vc_sm_defs.h
>>>   create mode 100644 drivers/staging/vc04_services/vc-sm-cma/vc_sm_knl.h
>>>   create mode 100644 include/uapi/linux/bcm2835-isp.h
>> --
>> Regards,
>>
>> Laurent Pinchart
  

Hi Umang,

Am 21.11.22 um 22:47 schrieb Umang Jain:
> This series aims to upport bcm2835-isp from the RPi kernel [1] and is a
> independent subset of earlier series [2] posted to upport CSI-2/CCP2
> receiver IP core("Unicam) + the ISP driver found in BCM283x and compatible
> SoCs (namely BCM2711). Unicam is still under active development to work
> with multistream support to get into mainline. Hence only the ISP driver
> will remain the primary area of this series.

thanks for working on this. But honestly i would prefer that vchiq comes 
out of staging before adding more features. As Greg said some time ago 
staging is not a place to "dump code and run away". These new files are 
in the same bad shape as the rest of vc04 before the clean-up here in 
staging started.

I agree that VCSM is on the TODO list for vchiq, but this driver is not 
necessary for making bcm2835-audio & bcm2835-camera leave staging. It 
just binds more resources on a new feature.

Unfortuntately i hadn't much time to work on vchiq by myself.

Just my two cents
Stefan
  

Hi Stefan

On 11/26/22 8:12 PM, Stefan Wahren wrote:
> Hi Umang,
>
> Am 21.11.22 um 22:47 schrieb Umang Jain:
>> This series aims to upport bcm2835-isp from the RPi kernel [1] and is a
>> independent subset of earlier series [2] posted to upport CSI-2/CCP2
>> receiver IP core("Unicam) + the ISP driver found in BCM283x and 
>> compatible
>> SoCs (namely BCM2711). Unicam is still under active development to work
>> with multistream support to get into mainline. Hence only the ISP driver
>> will remain the primary area of this series.
>
> thanks for working on this. But honestly i would prefer that vchiq 
> comes out of staging before adding more features. As Greg said some 
> time ago staging is not a place to "dump code and run away". These new 
> files are in the same bad shape as the rest of vc04 before the 
> clean-up here in staging started.

Certainly, I am not here to do that - but I am still learning the ropes.

If the staging issue is becoming a blocker for bcm2835-isp going 
upstream, I would be happy to help here! Though I must mention that I 
still have limited visibility so my aim would be to chart out a plan of 
things needed to be done to get vc04_services out of staging!

>
> I agree that VCSM is on the TODO list for vchiq, but this driver is 
> not necessary for making bcm2835-audio & bcm2835-camera leave staging. 
> It just binds more resources on a new feature.

I see two TODO files in vc04_services:
     ./bcm2835-camera/TODO
     ./interface/TODO

One of the bcm2835-camera TODO points to the vc-sm-cma driver itself. So 
that's address in the series. The other remaining one - I will need to 
take a deeper look before commenting on it.

The main chunk of TODO are in vc04_services/interfaces/TODO.  Doing a 
cursory reading of them suggests that these apply to *all* vc04_services 
components? Am I right?

Are these are the specific bits of cleanup you are referring to in your 
comment?


>
> Unfortuntately i hadn't much time to work on vchiq by myself.
>
> Just my two cents
> Stefan
>
  

Hi Umang,

Am 26.11.22 um 17:26 schrieb Umang Jain:
> Hi Stefan
>
> On 11/26/22 8:12 PM, Stefan Wahren wrote:
>> Hi Umang,
>>
>> Am 21.11.22 um 22:47 schrieb Umang Jain:
>>> This series aims to upport bcm2835-isp from the RPi kernel [1] and is a
>>> independent subset of earlier series [2] posted to upport CSI-2/CCP2
>>> receiver IP core("Unicam) + the ISP driver found in BCM283x and 
>>> compatible
>>> SoCs (namely BCM2711). Unicam is still under active development to work
>>> with multistream support to get into mainline. Hence only the ISP 
>>> driver
>>> will remain the primary area of this series.
>>
>> thanks for working on this. But honestly i would prefer that vchiq 
>> comes out of staging before adding more features. As Greg said some 
>> time ago staging is not a place to "dump code and run away". These 
>> new files are in the same bad shape as the rest of vc04 before the 
>> clean-up here in staging started.
>
> Certainly, I am not here to do that - but I am still learning the ropes.
no problem.
>
> If the staging issue is becoming a blocker for bcm2835-isp going 
> upstream, I would be happy to help here! Though I must mention that I 
> still have limited visibility so my aim would be to chart out a plan 
> of things needed to be done to get vc04_services out of staging!

The vchiq driver is in staging since 2016, so every step forwards is 
good. Unfortunately all of the low hanging fruits has been gathered.

For me the most important, but not to tricky steps to get vchiq out of 
staging would be:

* Cleanup logging mechanism

* Get rid of custom function return values

There was already an attempt for this [1]

* Get rid of all non essential global structures and create a proper per
device structure

>
>>
>> I agree that VCSM is on the TODO list for vchiq, but this driver is 
>> not necessary for making bcm2835-audio & bcm2835-camera leave 
>> staging. It just binds more resources on a new feature.
>
> I see two TODO files in vc04_services:
>     ./bcm2835-camera/TODO
>     ./interface/TODO
>
> One of the bcm2835-camera TODO points to the vc-sm-cma driver itself. 
> So that's address in the series. The other remaining one - I will need 
> to take a deeper look before commenting on it.
>
> The main chunk of TODO are in vc04_services/interfaces/TODO. Doing a 
> cursory reading of them suggests that these apply to *all* 
> vc04_services components? Am I right?
Actually these applies just for the interfaces directory. Some of them 
could apply to the services, but this is no priority.
>
> Are these are the specific bits of cleanup you are referring to in 
> your comment?

You mean about bcm2835-isp? There were too many changes to vchiq that i 
don't remember them all. The first that come to my mind was those fancy 
comment sections which is not kernel coding style. It has been removed.

[1] - 
https://lore.kernel.org/linux-staging/20220712181928.17547-1-jslebodn@redhat.com/

>
>
>>
>> Unfortuntately i hadn't much time to work on vchiq by myself.
>>
>> Just my two cents
>> Stefan
>>
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
  

Hi Stefan,

On 11/27/22 6:56 AM, Stefan Wahren wrote:
> Hi Umang,
>
> Am 26.11.22 um 17:26 schrieb Umang Jain:
>> Hi Stefan
>>
>> On 11/26/22 8:12 PM, Stefan Wahren wrote:
>>> Hi Umang,
>>>
>>> Am 21.11.22 um 22:47 schrieb Umang Jain:
>>>> This series aims to upport bcm2835-isp from the RPi kernel [1] and 
>>>> is a
>>>> independent subset of earlier series [2] posted to upport CSI-2/CCP2
>>>> receiver IP core("Unicam) + the ISP driver found in BCM283x and 
>>>> compatible
>>>> SoCs (namely BCM2711). Unicam is still under active development to 
>>>> work
>>>> with multistream support to get into mainline. Hence only the ISP 
>>>> driver
>>>> will remain the primary area of this series.
>>>
>>> thanks for working on this. But honestly i would prefer that vchiq 
>>> comes out of staging before adding more features. As Greg said some 
>>> time ago staging is not a place to "dump code and run away". These 
>>> new files are in the same bad shape as the rest of vc04 before the 
>>> clean-up here in staging started.
>>
>> Certainly, I am not here to do that - but I am still learning the ropes.
> no problem.
>>
>> If the staging issue is becoming a blocker for bcm2835-isp going 
>> upstream, I would be happy to help here! Though I must mention that I 
>> still have limited visibility so my aim would be to chart out a plan 
>> of things needed to be done to get vc04_services out of staging!
>
> The vchiq driver is in staging since 2016, so every step forwards is 
> good. Unfortunately all of the low hanging fruits has been gathered.
>
> For me the most important, but not to tricky steps to get vchiq out of 
> staging would be:
>
> * Cleanup logging mechanism
>
> * Get rid of custom function return values
>
> There was already an attempt for this [1]
>
> * Get rid of all non essential global structures and create a proper per
> device structure
>
>>
>>>
>>> I agree that VCSM is on the TODO list for vchiq, but this driver is 
>>> not necessary for making bcm2835-audio & bcm2835-camera leave 
>>> staging. It just binds more resources on a new feature.

bcm2835-camera is the legacy camera stack which probably need to be 
dropped from hereon...
>>
>> I see two TODO files in vc04_services:
>>     ./bcm2835-camera/TODO
>>     ./interface/TODO
>>
>> One of the bcm2835-camera TODO points to the vc-sm-cma driver itself. 
>> So that's address in the series. The other remaining one - I will 
>> need to take a deeper look before commenting on it.
>>
>> The main chunk of TODO are in vc04_services/interfaces/TODO. Doing a 
>> cursory reading of them suggests that these apply to *all* 
>> vc04_services components? Am I right?
> Actually these applies just for the interfaces directory. Some of them 
> could apply to the services, but this is no priority.

By no priority, you mean this doesn't affect the criteria required to 
ful-fill to get these out of staging?
>>
>> Are these are the specific bits of cleanup you are referring to in 
>> your comment?
>
> You mean about bcm2835-isp? There were too many changes to vchiq that 
> i don't remember them all. The first that come to my mind was those 
> fancy comment sections which is not kernel coding style. It has been 
> removed.

No, I don't mean the bcm2835-isp changes (those are upcoming / 
out-of-tree still so...). I mean what are the specific bits / points 
that needs to be addressed to get vc04_services out of the staging.

You have mentioned it above now, so I'll follow up on those. The many 
vchiq changes you referred to above comment (that you don't remember) 
are from [1] as well or some other series ?

>
> [1] - 
> https://lore.kernel.org/linux-staging/20220712181928.17547-1-jslebodn@redhat.com/
>
>>
>>
>>>
>>> Unfortuntately i hadn't much time to work on vchiq by myself.
>>>
>>> Just my two cents
>>> Stefan
>>>
>>
>>
>> _______________________________________________
>> linux-arm-kernel mailing list
>> linux-arm-kernel@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
  

Hi Umang,

Am 30.11.22 um 11:58 schrieb Umang Jain:
> Hi Stefan,
>
> On 11/27/22 6:56 AM, Stefan Wahren wrote:
>> Hi Umang,
>>
>> Am 26.11.22 um 17:26 schrieb Umang Jain:
>>> Hi Stefan
>>>
>>> On 11/26/22 8:12 PM, Stefan Wahren wrote:
>>>> Hi Umang,
>>>>
>>>> Am 21.11.22 um 22:47 schrieb Umang Jain:
>>>>> This series aims to upport bcm2835-isp from the RPi kernel [1] and 
>>>>> is a
>>>>> independent subset of earlier series [2] posted to upport CSI-2/CCP2
>>>>> receiver IP core("Unicam) + the ISP driver found in BCM283x and 
>>>>> compatible
>>>>> SoCs (namely BCM2711). Unicam is still under active development to 
>>>>> work
>>>>> with multistream support to get into mainline. Hence only the ISP 
>>>>> driver
>>>>> will remain the primary area of this series.
>>>>
>>>> thanks for working on this. But honestly i would prefer that vchiq 
>>>> comes out of staging before adding more features. As Greg said some 
>>>> time ago staging is not a place to "dump code and run away". These 
>>>> new files are in the same bad shape as the rest of vc04 before the 
>>>> clean-up here in staging started.
>>>
>>> Certainly, I am not here to do that - but I am still learning the 
>>> ropes.
>> no problem.
>>>
>>> If the staging issue is becoming a blocker for bcm2835-isp going 
>>> upstream, I would be happy to help here! Though I must mention that 
>>> I still have limited visibility so my aim would be to chart out a 
>>> plan of things needed to be done to get vc04_services out of staging!
>>
>> The vchiq driver is in staging since 2016, so every step forwards is 
>> good. Unfortunately all of the low hanging fruits has been gathered.
>>
>> For me the most important, but not to tricky steps to get vchiq out 
>> of staging would be:
>>
>> * Cleanup logging mechanism
>>
>> * Get rid of custom function return values
>>
>> There was already an attempt for this [1]
>>
>> * Get rid of all non essential global structures and create a proper per
>> device structure
>>
>>>
>>>>
>>>> I agree that VCSM is on the TODO list for vchiq, but this driver is 
>>>> not necessary for making bcm2835-audio & bcm2835-camera leave 
>>>> staging. It just binds more resources on a new feature.
>
> bcm2835-camera is the legacy camera stack which probably need to be 
> dropped from hereon...
I don't not know if there any users left, so i would be careful here. 
Can bcm2835-isp completely replace bcm2835-camera? Sorry, for this dumb 
question but i'm not expert here.
>>>
>>> I see two TODO files in vc04_services:
>>>     ./bcm2835-camera/TODO
>>>     ./interface/TODO
>>>
>>> One of the bcm2835-camera TODO points to the vc-sm-cma driver 
>>> itself. So that's address in the series. The other remaining one - I 
>>> will need to take a deeper look before commenting on it.
>>>
>>> The main chunk of TODO are in vc04_services/interfaces/TODO. Doing a 
>>> cursory reading of them suggests that these apply to *all* 
>>> vc04_services components? Am I right?
>> Actually these applies just for the interfaces directory. Some of 
>> them could apply to the services, but this is no priority.
>
> By no priority, you mean this doesn't affect the criteria required to 
> ful-fill to get these out of staging?
Correct
>>>
>>> Are these are the specific bits of cleanup you are referring to in 
>>> your comment?
>>
>> You mean about bcm2835-isp? There were too many changes to vchiq that 
>> i don't remember them all. The first that come to my mind was those 
>> fancy comment sections which is not kernel coding style. It has been 
>> removed.
>
> No, I don't mean the bcm2835-isp changes (those are upcoming / 
> out-of-tree still so...). I mean what are the specific bits / points 
> that needs to be addressed to get vc04_services out of the staging.
These were the points which i mentioned in my last email. They came from 
interface/TODO.
>
> You have mentioned it above now, so I'll follow up on those.
That would be great :)
> The many vchiq changes you referred to above comment (that you don't 
> remember) are from [1] as well or some other series ?
Sorry, for the confusing. The many changes i refer were the dozens of 
clean up patches for vc04_interfaces in mainline staging since the last 
years. [1] was just a single patch which has been accepted yet.
>
>>
>> [1] - 
>> https://lore.kernel.org/linux-staging/20220712181928.17547-1-jslebodn@redhat.com/
>>
>>>
>>>
>>>>
>>>> Unfortuntately i hadn't much time to work on vchiq by myself.
>>>>
>>>> Just my two cents
>>>> Stefan
>>>>
>>>
>>>
>>> _______________________________________________
>>> linux-arm-kernel mailing list
>>> linux-arm-kernel@lists.infradead.org
>>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
  

Hi Stefan,

On 12/2/22 6:45 AM, Stefan Wahren wrote:
> Hi Umang,
>
> Am 30.11.22 um 11:58 schrieb Umang Jain:
>> Hi Stefan,
>>
>> On 11/27/22 6:56 AM, Stefan Wahren wrote:
>>> Hi Umang,
>>>
>>> Am 26.11.22 um 17:26 schrieb Umang Jain:
>>>> Hi Stefan
>>>>
>>>> On 11/26/22 8:12 PM, Stefan Wahren wrote:
>>>>> Hi Umang,
>>>>>
>>>>> Am 21.11.22 um 22:47 schrieb Umang Jain:
>>>>>> This series aims to upport bcm2835-isp from the RPi kernel [1] 
>>>>>> and is a
>>>>>> independent subset of earlier series [2] posted to upport CSI-2/CCP2
>>>>>> receiver IP core("Unicam) + the ISP driver found in BCM283x and 
>>>>>> compatible
>>>>>> SoCs (namely BCM2711). Unicam is still under active development 
>>>>>> to work
>>>>>> with multistream support to get into mainline. Hence only the ISP 
>>>>>> driver
>>>>>> will remain the primary area of this series.
>>>>>
>>>>> thanks for working on this. But honestly i would prefer that vchiq 
>>>>> comes out of staging before adding more features. As Greg said 
>>>>> some time ago staging is not a place to "dump code and run away". 
>>>>> These new files are in the same bad shape as the rest of vc04 
>>>>> before the clean-up here in staging started.
>>>>
>>>> Certainly, I am not here to do that - but I am still learning the 
>>>> ropes.
>>> no problem.
>>>>
>>>> If the staging issue is becoming a blocker for bcm2835-isp going 
>>>> upstream, I would be happy to help here! Though I must mention that 
>>>> I still have limited visibility so my aim would be to chart out a 
>>>> plan of things needed to be done to get vc04_services out of staging!
>>>
>>> The vchiq driver is in staging since 2016, so every step forwards is 
>>> good. Unfortunately all of the low hanging fruits has been gathered.
>>>
>>> For me the most important, but not to tricky steps to get vchiq out 
>>> of staging would be:
>>>
>>> * Cleanup logging mechanism
>>>
>>> * Get rid of custom function return values
>>>
>>> There was already an attempt for this [1]
>>>
>>> * Get rid of all non essential global structures and create a proper 
>>> per
>>> device structure
>>>
>>>>
>>>>>
>>>>> I agree that VCSM is on the TODO list for vchiq, but this driver 
>>>>> is not necessary for making bcm2835-audio & bcm2835-camera leave 
>>>>> staging. It just binds more resources on a new feature.
>>
>> bcm2835-camera is the legacy camera stack which probably need to be 
>> dropped from hereon...
> I don't not know if there any users left, so i would be careful here. 
> Can bcm2835-isp completely replace bcm2835-camera? Sorry, for this 
> dumb question but i'm not expert here.

I am careful too here and probably need Input from RaspberryPi in order 
to proceed to drop it. But from my perspective - bcm2835-camera is _not_ 
going out of staging - it'll either sit here (or probably dropped) as 
statied from [1]

```
+ * There are two camera drivers in the kernel for BCM283x - this one
+ * and bcm2835-camera (currently in staging).
```

The bcm2835-camera is meant to be replaced by unicam [1] , but the ISP 
(bcm2835-isp) is meant to be worked with unicam [1]. In fact, I have 
mentioned in my cover the testing of bcm2835-isp happened on top of 
unicam patches.

[1]: 
https://lore.kernel.org/linux-media/20220208155027.891055-5-jeanmichel.hautbois@ideasonboard.com/
>>>>
>>>> I see two TODO files in vc04_services:
>>>>     ./bcm2835-camera/TODO
>>>>     ./interface/TODO
>>>>
>>>> One of the bcm2835-camera TODO points to the vc-sm-cma driver 
>>>> itself. So that's address in the series. The other remaining one - 
>>>> I will need to take a deeper look before commenting on it.
>>>>
>>>> The main chunk of TODO are in vc04_services/interfaces/TODO. Doing 
>>>> a cursory reading of them suggests that these apply to *all* 
>>>> vc04_services components? Am I right?
>>> Actually these applies just for the interfaces directory. Some of 
>>> them could apply to the services, but this is no priority.
>>
>> By no priority, you mean this doesn't affect the criteria required to 
>> ful-fill to get these out of staging?
> Correct
>>>>
>>>> Are these are the specific bits of cleanup you are referring to in 
>>>> your comment?
>>>
>>> You mean about bcm2835-isp? There were too many changes to vchiq 
>>> that i don't remember them all. The first that come to my mind was 
>>> those fancy comment sections which is not kernel coding style. It 
>>> has been removed.
>>
>> No, I don't mean the bcm2835-isp changes (those are upcoming / 
>> out-of-tree still so...). I mean what are the specific bits / points 
>> that needs to be addressed to get vc04_services out of the staging.
> These were the points which i mentioned in my last email. They came 
> from interface/TODO.
>>
>> You have mentioned it above now, so I'll follow up on those.
> That would be great :)
>> The many vchiq changes you referred to above comment (that you don't 
>> remember) are from [1] as well or some other series ?
> Sorry, for the confusing. The many changes i refer were the dozens of 
> clean up patches for vc04_interfaces in mainline staging since the 
> last years. [1] was just a single patch which has been accepted yet.

Ah I see. There are many others that I've to dig out then. Thanks for 
clarifying!
>>
>>>
>>> [1] - 
>>> https://lore.kernel.org/linux-staging/20220712181928.17547-1-jslebodn@redhat.com/
>>>
>>>>
>>>>
>>>>>
>>>>> Unfortuntately i hadn't much time to work on vchiq by myself.
>>>>>
>>>>> Just my two cents
>>>>> Stefan
>>>>>
>>>>
>>>>
>>>> _______________________________________________
>>>> linux-arm-kernel mailing list
>>>> linux-arm-kernel@lists.infradead.org
>>>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>>
  

Hi Umang,

On Fri, Dec 02, 2022 at 11:57:18AM +0800, Umang Jain wrote:
> On 12/2/22 6:45 AM, Stefan Wahren wrote:
> > Am 30.11.22 um 11:58 schrieb Umang Jain:
> >> On 11/27/22 6:56 AM, Stefan Wahren wrote:
> >>> Am 26.11.22 um 17:26 schrieb Umang Jain:
> >>>> On 11/26/22 8:12 PM, Stefan Wahren wrote:
> >>>>> Am 21.11.22 um 22:47 schrieb Umang Jain:
> >>>>>> This series aims to upport bcm2835-isp from the RPi kernel [1] and is a
> >>>>>> independent subset of earlier series [2] posted to upport CSI-2/CCP2
> >>>>>> receiver IP core("Unicam) + the ISP driver found in BCM283x and compatible
> >>>>>> SoCs (namely BCM2711). Unicam is still under active development to work
> >>>>>> with multistream support to get into mainline. Hence only the ISP driver
> >>>>>> will remain the primary area of this series.
> >>>>>
> >>>>> thanks for working on this. But honestly i would prefer that vchiq 
> >>>>> comes out of staging before adding more features. As Greg said 
> >>>>> some time ago staging is not a place to "dump code and run away". 
> >>>>> These new files are in the same bad shape as the rest of vc04 
> >>>>> before the clean-up here in staging started.
> >>>>
> >>>> Certainly, I am not here to do that - but I am still learning the ropes.
> >>>
> >>> no problem.
> >>>
> >>>> If the staging issue is becoming a blocker for bcm2835-isp going 
> >>>> upstream, I would be happy to help here! Though I must mention that 
> >>>> I still have limited visibility so my aim would be to chart out a 
> >>>> plan of things needed to be done to get vc04_services out of staging!
> >>>
> >>> The vchiq driver is in staging since 2016, so every step forwards is 
> >>> good. Unfortunately all of the low hanging fruits has been gathered.
> >>>
> >>> For me the most important, but not to tricky steps to get vchiq out 
> >>> of staging would be:
> >>>
> >>> * Cleanup logging mechanism
> >>>
> >>> * Get rid of custom function return values
> >>>
> >>> There was already an attempt for this [1]
> >>>
> >>> * Get rid of all non essential global structures and create a proper per
> >>> device structure
> >>>
> >>>>> I agree that VCSM is on the TODO list for vchiq, but this driver 
> >>>>> is not necessary for making bcm2835-audio & bcm2835-camera leave 
> >>>>> staging. It just binds more resources on a new feature.
> >>
> >> bcm2835-camera is the legacy camera stack which probably need to be 
> >> dropped from hereon...
> >
> > I don't not know if there any users left, so i would be careful here. 
> > Can bcm2835-isp completely replace bcm2835-camera? Sorry, for this 
> > dumb question but i'm not expert here.
> 
> I am careful too here and probably need Input from RaspberryPi in order 
> to proceed to drop it. But from my perspective - bcm2835-camera is _not_ 
> going out of staging - it'll either sit here (or probably dropped) as 
> statied from [1]
> 
> ```
> + * There are two camera drivers in the kernel for BCM283x - this one
> + * and bcm2835-camera (currently in staging).
> ```
> 
> The bcm2835-camera is meant to be replaced by unicam [1] , but the ISP 
> (bcm2835-isp) is meant to be worked with unicam [1]. In fact, I have 
> mentioned in my cover the testing of bcm2835-isp happened on top of 
> unicam patches.

To be accurate, the bcm2835-camera driver supports the VC4
firmware-based camera stack. In that setup, the camera sensors (OV5647
or IMX219), CSI-2 receiver (Unicam) and ISP are all controlled by the
firmware, which provides a high-level interface towards the kernel. This
architecture has been replaced by Linux-side control of the camera
sensors (through existing drivers in drivers/media/i2c/), Unicam
(through the driver from [1]) and ISP (through this driver). Moving
control to the Linux side requires complex processing in userspace,
handled by libcamera.

bcm2835-camera is thus replaced by multiple drivers combined with
libcamera, and that is the camera stack that is shipped by Raspberry Pi
these days. While this may affect some userspace use cases), we will not
work on destaging bcm2835-camera, and as far as I'm aware, nobody else
is planning to do so either. I don't mind much if the driver stays in
staging for some more time, but I'd rather drop it if possible.

> [1]: https://lore.kernel.org/linux-media/20220208155027.891055-5-jeanmichel.hautbois@ideasonboard.com/
>
> >>>>
> >>>> I see two TODO files in vc04_services:
> >>>>     ./bcm2835-camera/TODO
> >>>>     ./interface/TODO
> >>>>
> >>>> One of the bcm2835-camera TODO points to the vc-sm-cma driver 
> >>>> itself. So that's address in the series. The other remaining one - 
> >>>> I will need to take a deeper look before commenting on it.
> >>>>
> >>>> The main chunk of TODO are in vc04_services/interfaces/TODO. Doing 
> >>>> a cursory reading of them suggests that these apply to *all* 
> >>>> vc04_services components? Am I right?
> >>>
> >>> Actually these applies just for the interfaces directory. Some of 
> >>> them could apply to the services, but this is no priority.
> >>
> >> By no priority, you mean this doesn't affect the criteria required to 
> >> ful-fill to get these out of staging?
> >
> > Correct
> >
> >>>> Are these are the specific bits of cleanup you are referring to in 
> >>>> your comment?
> >>>
> >>> You mean about bcm2835-isp? There were too many changes to vchiq 
> >>> that i don't remember them all. The first that come to my mind was 
> >>> those fancy comment sections which is not kernel coding style. It 
> >>> has been removed.
> >>
> >> No, I don't mean the bcm2835-isp changes (those are upcoming / 
> >> out-of-tree still so...). I mean what are the specific bits / points 
> >> that needs to be addressed to get vc04_services out of the staging.
> >
> > These were the points which i mentioned in my last email. They came 
> > from interface/TODO.
> >
> >> You have mentioned it above now, so I'll follow up on those.
> >
> > That would be great :)
> >
> >> The many vchiq changes you referred to above comment (that you don't 
> >> remember) are from [1] as well or some other series ?
> >
> > Sorry, for the confusing. The many changes i refer were the dozens of 
> > clean up patches for vc04_interfaces in mainline staging since the 
> > last years. [1] was just a single patch which has been accepted yet.
> 
> Ah I see. There are many others that I've to dig out then. Thanks for 
> clarifying!
>
> >>> [1] - 
> >>> https://lore.kernel.org/linux-staging/20220712181928.17547-1-jslebodn@redhat.com/
> >>>
> >>>>> Unfortuntately i hadn't much time to work on vchiq by myself.
> >>>>>
> >>>>> Just my two cents
> >>>>> Stefan
  

Hi Laurent, Umang, and Stefan.

On Fri, 2 Dec 2022 at 09:17, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> Hi Umang,
>
> On Fri, Dec 02, 2022 at 11:57:18AM +0800, Umang Jain wrote:
> > On 12/2/22 6:45 AM, Stefan Wahren wrote:
> > > Am 30.11.22 um 11:58 schrieb Umang Jain:
> > >> On 11/27/22 6:56 AM, Stefan Wahren wrote:
> > >>> Am 26.11.22 um 17:26 schrieb Umang Jain:
> > >>>> On 11/26/22 8:12 PM, Stefan Wahren wrote:
> > >>>>> Am 21.11.22 um 22:47 schrieb Umang Jain:
> > >>>>>> This series aims to upport bcm2835-isp from the RPi kernel [1] and is a
> > >>>>>> independent subset of earlier series [2] posted to upport CSI-2/CCP2
> > >>>>>> receiver IP core("Unicam) + the ISP driver found in BCM283x and compatible
> > >>>>>> SoCs (namely BCM2711). Unicam is still under active development to work
> > >>>>>> with multistream support to get into mainline. Hence only the ISP driver
> > >>>>>> will remain the primary area of this series.
> > >>>>>
> > >>>>> thanks for working on this. But honestly i would prefer that vchiq
> > >>>>> comes out of staging before adding more features. As Greg said
> > >>>>> some time ago staging is not a place to "dump code and run away".
> > >>>>> These new files are in the same bad shape as the rest of vc04
> > >>>>> before the clean-up here in staging started.
> > >>>>
> > >>>> Certainly, I am not here to do that - but I am still learning the ropes.
> > >>>
> > >>> no problem.
> > >>>
> > >>>> If the staging issue is becoming a blocker for bcm2835-isp going
> > >>>> upstream, I would be happy to help here! Though I must mention that
> > >>>> I still have limited visibility so my aim would be to chart out a
> > >>>> plan of things needed to be done to get vc04_services out of staging!
> > >>>
> > >>> The vchiq driver is in staging since 2016, so every step forwards is
> > >>> good. Unfortunately all of the low hanging fruits has been gathered.
> > >>>
> > >>> For me the most important, but not to tricky steps to get vchiq out
> > >>> of staging would be:
> > >>>
> > >>> * Cleanup logging mechanism
> > >>>
> > >>> * Get rid of custom function return values
> > >>>
> > >>> There was already an attempt for this [1]
> > >>>
> > >>> * Get rid of all non essential global structures and create a proper per
> > >>> device structure
> > >>>
> > >>>>> I agree that VCSM is on the TODO list for vchiq, but this driver
> > >>>>> is not necessary for making bcm2835-audio & bcm2835-camera leave
> > >>>>> staging. It just binds more resources on a new feature.
> > >>
> > >> bcm2835-camera is the legacy camera stack which probably need to be
> > >> dropped from hereon...
> > >
> > > I don't not know if there any users left, so i would be careful here.
> > > Can bcm2835-isp completely replace bcm2835-camera? Sorry, for this
> > > dumb question but i'm not expert here.
> >
> > I am careful too here and probably need Input from RaspberryPi in order
> > to proceed to drop it. But from my perspective - bcm2835-camera is _not_
> > going out of staging - it'll either sit here (or probably dropped) as
> > statied from [1]
> >
> > ```
> > + * There are two camera drivers in the kernel for BCM283x - this one
> > + * and bcm2835-camera (currently in staging).
> > ```
> >
> > The bcm2835-camera is meant to be replaced by unicam [1] , but the ISP
> > (bcm2835-isp) is meant to be worked with unicam [1]. In fact, I have
> > mentioned in my cover the testing of bcm2835-isp happened on top of
> > unicam patches.
>
> To be accurate, the bcm2835-camera driver supports the VC4
> firmware-based camera stack. In that setup, the camera sensors (OV5647
> or IMX219), CSI-2 receiver (Unicam) and ISP are all controlled by the
> firmware, which provides a high-level interface towards the kernel. This
> architecture has been replaced by Linux-side control of the camera
> sensors (through existing drivers in drivers/media/i2c/), Unicam
> (through the driver from [1]) and ISP (through this driver). Moving
> control to the Linux side requires complex processing in userspace,
> handled by libcamera.
>
> bcm2835-camera is thus replaced by multiple drivers combined with
> libcamera, and that is the camera stack that is shipped by Raspberry Pi
> these days. While this may affect some userspace use cases), we will not
> work on destaging bcm2835-camera, and as far as I'm aware, nobody else
> is planning to do so either. I don't mind much if the driver stays in
> staging for some more time, but I'd rather drop it if possible.

It would be reasonable to drop it at the point that Libcamera can work
to a similar level with at least the following list of applications:
- FFmpeg
- Gstreamer
- Chromium
- Firefox
- Motion
And that still leaves a huge number of existing V4L2 apps out in the cold.

Do you wish to make any predictions as to when that would be
achievable? Or even when a v1.0 release of libcamera is going to
happen?
Dropping anything prior to those points would be rather premature in my book.


The TODOs on bcm2835-camera are:
1) Zero copy. That comes almost for free as bcm2835-isp already does
this, but it does rely on vcsm-cma.
The main reason I haven't pushed it is that it then requires
reasonable amounts of CMA heap for all the buffers, which until
recently haven't been present in the default configurations. With the
vc4 DRM driver now being default (at least for the vendor kernel) and
also requiring CMA, making the change makes more sense.
AFAIK there is no easy way to have one driver choosing between using
vb2_vmalloc_memops and vb2_dma_contig_memops at runtime, but I may be
wrong.
Actually bcm2835_defconfig appears to only allocate a 32MB CMA heap,
so perhaps we don't get very far.

2) This isn't workable within the current V4L2 frameworks. The
multi-planar V4L2 pixel formats are currently allocated as independent
buffers for each plane, whereas the firmware needs a single buffer
with (currently) specific offsets for the chroma planes. The
V4L2/videobuf2 core changes required to implement that are going to be
significant, and have minimal gain.
The specific stride handling is already dealt with (set bytesperline
appropriately), it's the padding of the height to a multiple of 16
before the chroma planes on YUV420 and NV12 formats that require the
firmware to do a small amount of repacking. The performance hit is
actually minimal anyway.

If bcm2835-camera is the only thing holding back vc04_services, then I
can have a look at it.

  Dave

> > [1]: https://lore.kernel.org/linux-media/20220208155027.891055-5-jeanmichel.hautbois@ideasonboard.com/
> >
> > >>>>
> > >>>> I see two TODO files in vc04_services:
> > >>>>     ./bcm2835-camera/TODO
> > >>>>     ./interface/TODO
> > >>>>
> > >>>> One of the bcm2835-camera TODO points to the vc-sm-cma driver
> > >>>> itself. So that's address in the series. The other remaining one -
> > >>>> I will need to take a deeper look before commenting on it.
> > >>>>
> > >>>> The main chunk of TODO are in vc04_services/interfaces/TODO. Doing
> > >>>> a cursory reading of them suggests that these apply to *all*
> > >>>> vc04_services components? Am I right?
> > >>>
> > >>> Actually these applies just for the interfaces directory. Some of
> > >>> them could apply to the services, but this is no priority.
> > >>
> > >> By no priority, you mean this doesn't affect the criteria required to
> > >> ful-fill to get these out of staging?
> > >
> > > Correct
> > >
> > >>>> Are these are the specific bits of cleanup you are referring to in
> > >>>> your comment?
> > >>>
> > >>> You mean about bcm2835-isp? There were too many changes to vchiq
> > >>> that i don't remember them all. The first that come to my mind was
> > >>> those fancy comment sections which is not kernel coding style. It
> > >>> has been removed.
> > >>
> > >> No, I don't mean the bcm2835-isp changes (those are upcoming /
> > >> out-of-tree still so...). I mean what are the specific bits / points
> > >> that needs to be addressed to get vc04_services out of the staging.
> > >
> > > These were the points which i mentioned in my last email. They came
> > > from interface/TODO.
> > >
> > >> You have mentioned it above now, so I'll follow up on those.
> > >
> > > That would be great :)
> > >
> > >> The many vchiq changes you referred to above comment (that you don't
> > >> remember) are from [1] as well or some other series ?
> > >
> > > Sorry, for the confusing. The many changes i refer were the dozens of
> > > clean up patches for vc04_interfaces in mainline staging since the
> > > last years. [1] was just a single patch which has been accepted yet.
> >
> > Ah I see. There are many others that I've to dig out then. Thanks for
> > clarifying!
> >
> > >>> [1] -
> > >>> https://lore.kernel.org/linux-staging/20220712181928.17547-1-jslebodn@redhat.com/
> > >>>
> > >>>>> Unfortuntately i hadn't much time to work on vchiq by myself.
> > >>>>>
> > >>>>> Just my two cents
> > >>>>> Stefan
>
> --
> Regards,
>
> Laurent Pinchart
  

Hi Dave,

On Fri, Dec 02, 2022 at 11:23:29AM +0000, Dave Stevenson wrote:
> On Fri, 2 Dec 2022 at 09:17, Laurent Pinchart wrote:
> > On Fri, Dec 02, 2022 at 11:57:18AM +0800, Umang Jain wrote:
> > > On 12/2/22 6:45 AM, Stefan Wahren wrote:
> > > > Am 30.11.22 um 11:58 schrieb Umang Jain:
> > > >> On 11/27/22 6:56 AM, Stefan Wahren wrote:
> > > >>> Am 26.11.22 um 17:26 schrieb Umang Jain:
> > > >>>> On 11/26/22 8:12 PM, Stefan Wahren wrote:
> > > >>>>> Am 21.11.22 um 22:47 schrieb Umang Jain:
> > > >>>>>> This series aims to upport bcm2835-isp from the RPi kernel [1] and is a
> > > >>>>>> independent subset of earlier series [2] posted to upport CSI-2/CCP2
> > > >>>>>> receiver IP core("Unicam) + the ISP driver found in BCM283x and compatible
> > > >>>>>> SoCs (namely BCM2711). Unicam is still under active development to work
> > > >>>>>> with multistream support to get into mainline. Hence only the ISP driver
> > > >>>>>> will remain the primary area of this series.
> > > >>>>>
> > > >>>>> thanks for working on this. But honestly i would prefer that vchiq
> > > >>>>> comes out of staging before adding more features. As Greg said
> > > >>>>> some time ago staging is not a place to "dump code and run away".
> > > >>>>> These new files are in the same bad shape as the rest of vc04
> > > >>>>> before the clean-up here in staging started.
> > > >>>>
> > > >>>> Certainly, I am not here to do that - but I am still learning the ropes.
> > > >>>
> > > >>> no problem.
> > > >>>
> > > >>>> If the staging issue is becoming a blocker for bcm2835-isp going
> > > >>>> upstream, I would be happy to help here! Though I must mention that
> > > >>>> I still have limited visibility so my aim would be to chart out a
> > > >>>> plan of things needed to be done to get vc04_services out of staging!
> > > >>>
> > > >>> The vchiq driver is in staging since 2016, so every step forwards is
> > > >>> good. Unfortunately all of the low hanging fruits has been gathered.
> > > >>>
> > > >>> For me the most important, but not to tricky steps to get vchiq out
> > > >>> of staging would be:
> > > >>>
> > > >>> * Cleanup logging mechanism
> > > >>>
> > > >>> * Get rid of custom function return values
> > > >>>
> > > >>> There was already an attempt for this [1]
> > > >>>
> > > >>> * Get rid of all non essential global structures and create a proper per
> > > >>> device structure
> > > >>>
> > > >>>>> I agree that VCSM is on the TODO list for vchiq, but this driver
> > > >>>>> is not necessary for making bcm2835-audio & bcm2835-camera leave
> > > >>>>> staging. It just binds more resources on a new feature.
> > > >>
> > > >> bcm2835-camera is the legacy camera stack which probably need to be
> > > >> dropped from hereon...
> > > >
> > > > I don't not know if there any users left, so i would be careful here.
> > > > Can bcm2835-isp completely replace bcm2835-camera? Sorry, for this
> > > > dumb question but i'm not expert here.
> > >
> > > I am careful too here and probably need Input from RaspberryPi in order
> > > to proceed to drop it. But from my perspective - bcm2835-camera is _not_
> > > going out of staging - it'll either sit here (or probably dropped) as
> > > statied from [1]
> > >
> > > ```
> > > + * There are two camera drivers in the kernel for BCM283x - this one
> > > + * and bcm2835-camera (currently in staging).
> > > ```
> > >
> > > The bcm2835-camera is meant to be replaced by unicam [1] , but the ISP
> > > (bcm2835-isp) is meant to be worked with unicam [1]. In fact, I have
> > > mentioned in my cover the testing of bcm2835-isp happened on top of
> > > unicam patches.
> >
> > To be accurate, the bcm2835-camera driver supports the VC4
> > firmware-based camera stack. In that setup, the camera sensors (OV5647
> > or IMX219), CSI-2 receiver (Unicam) and ISP are all controlled by the
> > firmware, which provides a high-level interface towards the kernel. This
> > architecture has been replaced by Linux-side control of the camera
> > sensors (through existing drivers in drivers/media/i2c/), Unicam
> > (through the driver from [1]) and ISP (through this driver). Moving
> > control to the Linux side requires complex processing in userspace,
> > handled by libcamera.
> >
> > bcm2835-camera is thus replaced by multiple drivers combined with
> > libcamera, and that is the camera stack that is shipped by Raspberry Pi
> > these days. While this may affect some userspace use cases), we will not
> > work on destaging bcm2835-camera, and as far as I'm aware, nobody else
> > is planning to do so either. I don't mind much if the driver stays in
> > staging for some more time, but I'd rather drop it if possible.
> 
> It would be reasonable to drop it at the point that Libcamera can work
> to a similar level with at least the following list of applications:
> - FFmpeg
> - Gstreamer
> - Chromium
> - Firefox
> - Motion
> And that still leaves a huge number of existing V4L2 apps out in the cold.

That's exactly the kind of input we were looking for, thanks.

GStreamer is already addressed.

Chromium and Firefox will go through PipeWire. There is a working
implementation in libwebrtc, Kieran may be able to comment on the
upstreaming state. It will take some time for distributions to switch,
and I can't predict the time line, but that seems to clearly be the
direction the Linux desktop is taking.

I haven't looked at FFmpeg yet (maybe someone has ?). It probably makes
sense to add native libcamera support to FFmpeg, even if PipeWire
support could also make sense. It could also make sense to expose in the
libcamera V4L2 compat layer the libv4l2 API functions, as that would
allow linking FFmpeg (when compiled with CONFIG_LIBV4L2) to libcamera
instead of libv4l2, but Debian doesn't set CONFIG_LIBV4L2, so this isn't
an immediate solution to the problem.

Same thing for motion, except it has no libv4l2 support. The V4L2 compat
layer could still be used with LD_PRELOAD, but that's not a great
solution. Native libcamera support would make more sense (or possibly
even GStreamer support, I don't know if upstream would accept that).

In a side note, how do the above applications work today on Raspberry Pi
platforms that use a sensor not supported by the legacy camera stack ?

> Do you wish to make any predictions as to when that would be
> achievable? Or even when a v1.0 release of libcamera is going to
> happen?

Now that we have started tagging releases, we've also decided to publish
a roadmap with the development still needed to stabilize the API. We'll
likely start working on it this month.

> Dropping anything prior to those points would be rather premature in my book.

Something I forgot to mention is that there should be no issue at all
keeping bcm2835-camera fully supported in the Raspberry Pi downstream
kernel for a longer period of time. It's in upstream that I don't think
it should be destaged, as it's already considered legacy and should be
phased out. Do you know if there are users of that driver with a
mainline kernel ?

> The TODOs on bcm2835-camera are:
> 1) Zero copy. That comes almost for free as bcm2835-isp already does
> this, but it does rely on vcsm-cma.
> The main reason I haven't pushed it is that it then requires
> reasonable amounts of CMA heap for all the buffers, which until
> recently haven't been present in the default configurations. With the
> vc4 DRM driver now being default (at least for the vendor kernel) and
> also requiring CMA, making the change makes more sense.
> AFAIK there is no easy way to have one driver choosing between using
> vb2_vmalloc_memops and vb2_dma_contig_memops at runtime, but I may be
> wrong.

Some drivers use a module parameter for that, but that's not great.

> Actually bcm2835_defconfig appears to only allocate a 32MB CMA heap,
> so perhaps we don't get very far.
> 
> 2) This isn't workable within the current V4L2 frameworks. The
> multi-planar V4L2 pixel formats are currently allocated as independent
> buffers for each plane, whereas the firmware needs a single buffer
> with (currently) specific offsets for the chroma planes. The
> V4L2/videobuf2 core changes required to implement that are going to be
> significant, and have minimal gain.
> The specific stride handling is already dealt with (set bytesperline
> appropriately), it's the padding of the height to a multiple of 16
> before the chroma planes on YUV420 and NV12 formats that require the
> firmware to do a small amount of repacking. The performance hit is
> actually minimal anyway.
> 
> If bcm2835-camera is the only thing holding back vc04_services, then I
> can have a look at it.

I'll let Umang comment on whether it's holding vc04_services back, but
my understanding it that we could destage vc04_services while keeping
bcm2835-camera in staging for the time being. If anyone disagrees with
that, please let me know.

> > > [1]: https://lore.kernel.org/linux-media/20220208155027.891055-5-jeanmichel.hautbois@ideasonboard.com/
> > >
> > > >>>>
> > > >>>> I see two TODO files in vc04_services:
> > > >>>>     ./bcm2835-camera/TODO
> > > >>>>     ./interface/TODO
> > > >>>>
> > > >>>> One of the bcm2835-camera TODO points to the vc-sm-cma driver
> > > >>>> itself. So that's address in the series. The other remaining one -
> > > >>>> I will need to take a deeper look before commenting on it.
> > > >>>>
> > > >>>> The main chunk of TODO are in vc04_services/interfaces/TODO. Doing
> > > >>>> a cursory reading of them suggests that these apply to *all*
> > > >>>> vc04_services components? Am I right?
> > > >>>
> > > >>> Actually these applies just for the interfaces directory. Some of
> > > >>> them could apply to the services, but this is no priority.
> > > >>
> > > >> By no priority, you mean this doesn't affect the criteria required to
> > > >> ful-fill to get these out of staging?
> > > >
> > > > Correct
> > > >
> > > >>>> Are these are the specific bits of cleanup you are referring to in
> > > >>>> your comment?
> > > >>>
> > > >>> You mean about bcm2835-isp? There were too many changes to vchiq
> > > >>> that i don't remember them all. The first that come to my mind was
> > > >>> those fancy comment sections which is not kernel coding style. It
> > > >>> has been removed.
> > > >>
> > > >> No, I don't mean the bcm2835-isp changes (those are upcoming /
> > > >> out-of-tree still so...). I mean what are the specific bits / points
> > > >> that needs to be addressed to get vc04_services out of the staging.
> > > >
> > > > These were the points which i mentioned in my last email. They came
> > > > from interface/TODO.
> > > >
> > > >> You have mentioned it above now, so I'll follow up on those.
> > > >
> > > > That would be great :)
> > > >
> > > >> The many vchiq changes you referred to above comment (that you don't
> > > >> remember) are from [1] as well or some other series ?
> > > >
> > > > Sorry, for the confusing. The many changes i refer were the dozens of
> > > > clean up patches for vc04_interfaces in mainline staging since the
> > > > last years. [1] was just a single patch which has been accepted yet.
> > >
> > > Ah I see. There are many others that I've to dig out then. Thanks for
> > > clarifying!
> > >
> > > >>> [1] -
> > > >>> https://lore.kernel.org/linux-staging/20220712181928.17547-1-jslebodn@redhat.com/
> > > >>>
> > > >>>>> Unfortuntately i hadn't much time to work on vchiq by myself.
> > > >>>>>
> > > >>>>> Just my two cents
> > > >>>>> Stefan
  

Hi Laurent,

Am 02.12.22 um 13:10 schrieb Laurent Pinchart:
> Hi Dave,
>
> On Fri, Dec 02, 2022 at 11:23:29AM +0000, Dave Stevenson wrote:
>> Dropping anything prior to those points would be rather premature in my book.
> Something I forgot to mention is that there should be no issue at all
> keeping bcm2835-camera fully supported in the Raspberry Pi downstream
> kernel for a longer period of time. It's in upstream that I don't think
> it should be destaged, as it's already considered legacy and should be
> phased out. Do you know if there are users of that driver with a
> mainline kernel ?
>
In Fedora there seems to be no official support [1], but openSuSE seems 
to mention it [2].

[1] - 
https://docs.fedoraproject.org/en-US/quick-docs/raspberry-pi/#_does_the_add_on_camera_work

[2] - https://en.opensuse.org/HCL:Raspberry_Pi_Camera_Modules
  

Hi Laurent

On Fri, 2 Dec 2022 at 12:10, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> Hi Dave,
>
> On Fri, Dec 02, 2022 at 11:23:29AM +0000, Dave Stevenson wrote:
> > On Fri, 2 Dec 2022 at 09:17, Laurent Pinchart wrote:
> > > On Fri, Dec 02, 2022 at 11:57:18AM +0800, Umang Jain wrote:
> > > > On 12/2/22 6:45 AM, Stefan Wahren wrote:
> > > > > Am 30.11.22 um 11:58 schrieb Umang Jain:
> > > > >> On 11/27/22 6:56 AM, Stefan Wahren wrote:
> > > > >>> Am 26.11.22 um 17:26 schrieb Umang Jain:
> > > > >>>> On 11/26/22 8:12 PM, Stefan Wahren wrote:
> > > > >>>>> Am 21.11.22 um 22:47 schrieb Umang Jain:
> > > > >>>>>> This series aims to upport bcm2835-isp from the RPi kernel [1] and is a
> > > > >>>>>> independent subset of earlier series [2] posted to upport CSI-2/CCP2
> > > > >>>>>> receiver IP core("Unicam) + the ISP driver found in BCM283x and compatible
> > > > >>>>>> SoCs (namely BCM2711). Unicam is still under active development to work
> > > > >>>>>> with multistream support to get into mainline. Hence only the ISP driver
> > > > >>>>>> will remain the primary area of this series.
> > > > >>>>>
> > > > >>>>> thanks for working on this. But honestly i would prefer that vchiq
> > > > >>>>> comes out of staging before adding more features. As Greg said
> > > > >>>>> some time ago staging is not a place to "dump code and run away".
> > > > >>>>> These new files are in the same bad shape as the rest of vc04
> > > > >>>>> before the clean-up here in staging started.
> > > > >>>>
> > > > >>>> Certainly, I am not here to do that - but I am still learning the ropes.
> > > > >>>
> > > > >>> no problem.
> > > > >>>
> > > > >>>> If the staging issue is becoming a blocker for bcm2835-isp going
> > > > >>>> upstream, I would be happy to help here! Though I must mention that
> > > > >>>> I still have limited visibility so my aim would be to chart out a
> > > > >>>> plan of things needed to be done to get vc04_services out of staging!
> > > > >>>
> > > > >>> The vchiq driver is in staging since 2016, so every step forwards is
> > > > >>> good. Unfortunately all of the low hanging fruits has been gathered.
> > > > >>>
> > > > >>> For me the most important, but not to tricky steps to get vchiq out
> > > > >>> of staging would be:
> > > > >>>
> > > > >>> * Cleanup logging mechanism
> > > > >>>
> > > > >>> * Get rid of custom function return values
> > > > >>>
> > > > >>> There was already an attempt for this [1]
> > > > >>>
> > > > >>> * Get rid of all non essential global structures and create a proper per
> > > > >>> device structure
> > > > >>>
> > > > >>>>> I agree that VCSM is on the TODO list for vchiq, but this driver
> > > > >>>>> is not necessary for making bcm2835-audio & bcm2835-camera leave
> > > > >>>>> staging. It just binds more resources on a new feature.
> > > > >>
> > > > >> bcm2835-camera is the legacy camera stack which probably need to be
> > > > >> dropped from hereon...
> > > > >
> > > > > I don't not know if there any users left, so i would be careful here.
> > > > > Can bcm2835-isp completely replace bcm2835-camera? Sorry, for this
> > > > > dumb question but i'm not expert here.
> > > >
> > > > I am careful too here and probably need Input from RaspberryPi in order
> > > > to proceed to drop it. But from my perspective - bcm2835-camera is _not_
> > > > going out of staging - it'll either sit here (or probably dropped) as
> > > > statied from [1]
> > > >
> > > > ```
> > > > + * There are two camera drivers in the kernel for BCM283x - this one
> > > > + * and bcm2835-camera (currently in staging).
> > > > ```
> > > >
> > > > The bcm2835-camera is meant to be replaced by unicam [1] , but the ISP
> > > > (bcm2835-isp) is meant to be worked with unicam [1]. In fact, I have
> > > > mentioned in my cover the testing of bcm2835-isp happened on top of
> > > > unicam patches.
> > >
> > > To be accurate, the bcm2835-camera driver supports the VC4
> > > firmware-based camera stack. In that setup, the camera sensors (OV5647
> > > or IMX219), CSI-2 receiver (Unicam) and ISP are all controlled by the
> > > firmware, which provides a high-level interface towards the kernel. This
> > > architecture has been replaced by Linux-side control of the camera
> > > sensors (through existing drivers in drivers/media/i2c/), Unicam
> > > (through the driver from [1]) and ISP (through this driver). Moving
> > > control to the Linux side requires complex processing in userspace,
> > > handled by libcamera.
> > >
> > > bcm2835-camera is thus replaced by multiple drivers combined with
> > > libcamera, and that is the camera stack that is shipped by Raspberry Pi
> > > these days. While this may affect some userspace use cases), we will not
> > > work on destaging bcm2835-camera, and as far as I'm aware, nobody else
> > > is planning to do so either. I don't mind much if the driver stays in
> > > staging for some more time, but I'd rather drop it if possible.
> >
> > It would be reasonable to drop it at the point that Libcamera can work
> > to a similar level with at least the following list of applications:
> > - FFmpeg
> > - Gstreamer
> > - Chromium
> > - Firefox
> > - Motion
> > And that still leaves a huge number of existing V4L2 apps out in the cold.
>
> That's exactly the kind of input we were looking for, thanks.

Please note that those are only the 5 major ones that came immediately
to mind as having significant numbers of users. There are obviously
many more V4L2 apps around which currently "just work" if they use the
API correctly.

> GStreamer is already addressed.
>
> Chromium and Firefox will go through PipeWire. There is a working
> implementation in libwebrtc, Kieran may be able to comment on the
> upstreaming state. It will take some time for distributions to switch,
> and I can't predict the time line, but that seems to clearly be the
> direction the Linux desktop is taking.
>
> I haven't looked at FFmpeg yet (maybe someone has ?). It probably makes
> sense to add native libcamera support to FFmpeg, even if PipeWire
> support could also make sense. It could also make sense to expose in the
> libcamera V4L2 compat layer the libv4l2 API functions, as that would
> allow linking FFmpeg (when compiled with CONFIG_LIBV4L2) to libcamera
> instead of libv4l2, but Debian doesn't set CONFIG_LIBV4L2, so this isn't
> an immediate solution to the problem.

Native would be nice in my book, but isn't always as straightforward.

> Same thing for motion, except it has no libv4l2 support. The V4L2 compat
> layer could still be used with LD_PRELOAD, but that's not a great
> solution. Native libcamera support would make more sense (or possibly
> even GStreamer support, I don't know if upstream would accept that).

The main bit missing with the compat layer is controls. AIUI fixing
that isn't a priority.
Things like Motion being used for security purposes need to be able to
set properties like exposure modes in order to achieve usable images
under changing lighting conditions.

> In a side note, how do the above applications work today on Raspberry Pi
> platforms that use a sensor not supported by the legacy camera stack ?

They don't for raw sensors, but do work with USB webcams.
Raspberry Pi have sold camera modules since 2013 (OV5647), with v2
(IMX219) coming along in 2016, and HQ (IMX477) in 2020. Linux has only
had frameworks to sensibly support raw image sensors since libcamera
came on the scene around 2019.

> > Do you wish to make any predictions as to when that would be
> > achievable? Or even when a v1.0 release of libcamera is going to
> > happen?
>
> Now that we have started tagging releases, we've also decided to publish
> a roadmap with the development still needed to stabilize the API. We'll
> likely start working on it this month.
>
> > Dropping anything prior to those points would be rather premature in my book.
>
> Something I forgot to mention is that there should be no issue at all
> keeping bcm2835-camera fully supported in the Raspberry Pi downstream
> kernel for a longer period of time. It's in upstream that I don't think
> it should be destaged, as it's already considered legacy and should be
> phased out. Do you know if there are users of that driver with a
> mainline kernel ?

There are a number of distros that use mainline kernels rather than
our vendor kernel. I don't follow distros in detail, but I believe
both Gentoo and Opensuse fall into that category.
 Any users of the camera module under those will be using the mainline
bcm2835-camera module. I have no stats for who uses those on a Pi.

> > The TODOs on bcm2835-camera are:
> > 1) Zero copy. That comes almost for free as bcm2835-isp already does
> > this, but it does rely on vcsm-cma.
> > The main reason I haven't pushed it is that it then requires
> > reasonable amounts of CMA heap for all the buffers, which until
> > recently haven't been present in the default configurations. With the
> > vc4 DRM driver now being default (at least for the vendor kernel) and
> > also requiring CMA, making the change makes more sense.
> > AFAIK there is no easy way to have one driver choosing between using
> > vb2_vmalloc_memops and vb2_dma_contig_memops at runtime, but I may be
> > wrong.
>
> Some drivers use a module parameter for that, but that's not great.

I had thought of a module parameter, but it becomes yet another
configuration thing to get incorrectly set.
Perhaps Kconfig based on the setting for CMA_SIZE_MBYTES, but that can
be overridden.

  Dave

> > Actually bcm2835_defconfig appears to only allocate a 32MB CMA heap,
> > so perhaps we don't get very far.
> >
> > 2) This isn't workable within the current V4L2 frameworks. The
> > multi-planar V4L2 pixel formats are currently allocated as independent
> > buffers for each plane, whereas the firmware needs a single buffer
> > with (currently) specific offsets for the chroma planes. The
> > V4L2/videobuf2 core changes required to implement that are going to be
> > significant, and have minimal gain.
> > The specific stride handling is already dealt with (set bytesperline
> > appropriately), it's the padding of the height to a multiple of 16
> > before the chroma planes on YUV420 and NV12 formats that require the
> > firmware to do a small amount of repacking. The performance hit is
> > actually minimal anyway.
> >
> > If bcm2835-camera is the only thing holding back vc04_services, then I
> > can have a look at it.
>
> I'll let Umang comment on whether it's holding vc04_services back, but
> my understanding it that we could destage vc04_services while keeping
> bcm2835-camera in staging for the time being. If anyone disagrees with
> that, please let me know.
>
> > > > [1]: https://lore.kernel.org/linux-media/20220208155027.891055-5-jeanmichel.hautbois@ideasonboard.com/
> > > >
> > > > >>>>
> > > > >>>> I see two TODO files in vc04_services:
> > > > >>>>     ./bcm2835-camera/TODO
> > > > >>>>     ./interface/TODO
> > > > >>>>
> > > > >>>> One of the bcm2835-camera TODO points to the vc-sm-cma driver
> > > > >>>> itself. So that's address in the series. The other remaining one -
> > > > >>>> I will need to take a deeper look before commenting on it.
> > > > >>>>
> > > > >>>> The main chunk of TODO are in vc04_services/interfaces/TODO. Doing
> > > > >>>> a cursory reading of them suggests that these apply to *all*
> > > > >>>> vc04_services components? Am I right?
> > > > >>>
> > > > >>> Actually these applies just for the interfaces directory. Some of
> > > > >>> them could apply to the services, but this is no priority.
> > > > >>
> > > > >> By no priority, you mean this doesn't affect the criteria required to
> > > > >> ful-fill to get these out of staging?
> > > > >
> > > > > Correct
> > > > >
> > > > >>>> Are these are the specific bits of cleanup you are referring to in
> > > > >>>> your comment?
> > > > >>>
> > > > >>> You mean about bcm2835-isp? There were too many changes to vchiq
> > > > >>> that i don't remember them all. The first that come to my mind was
> > > > >>> those fancy comment sections which is not kernel coding style. It
> > > > >>> has been removed.
> > > > >>
> > > > >> No, I don't mean the bcm2835-isp changes (those are upcoming /
> > > > >> out-of-tree still so...). I mean what are the specific bits / points
> > > > >> that needs to be addressed to get vc04_services out of the staging.
> > > > >
> > > > > These were the points which i mentioned in my last email. They came
> > > > > from interface/TODO.
> > > > >
> > > > >> You have mentioned it above now, so I'll follow up on those.
> > > > >
> > > > > That would be great :)
> > > > >
> > > > >> The many vchiq changes you referred to above comment (that you don't
> > > > >> remember) are from [1] as well or some other series ?
> > > > >
> > > > > Sorry, for the confusing. The many changes i refer were the dozens of
> > > > > clean up patches for vc04_interfaces in mainline staging since the
> > > > > last years. [1] was just a single patch which has been accepted yet.
> > > >
> > > > Ah I see. There are many others that I've to dig out then. Thanks for
> > > > clarifying!
> > > >
> > > > >>> [1] -
> > > > >>> https://lore.kernel.org/linux-staging/20220712181928.17547-1-jslebodn@redhat.com/
> > > > >>>
> > > > >>>>> Unfortuntately i hadn't much time to work on vchiq by myself.
> > > > >>>>>
> > > > >>>>> Just my two cents
> > > > >>>>> Stefan
>
> --
> Regards,
>
> Laurent Pinchart
  

Hi Dave,

Am 02.12.22 um 12:23 schrieb Dave Stevenson:
> Hi Laurent, Umang, and Stefan.
>
> On Fri, 2 Dec 2022 at 09:17, Laurent Pinchart
> <laurent.pinchart@ideasonboard.com> wrote:
>> Hi Umang,
>>
>> On Fri, Dec 02, 2022 at 11:57:18AM +0800, Umang Jain wrote:
>>> On 12/2/22 6:45 AM, Stefan Wahren wrote:
>>>> Am 30.11.22 um 11:58 schrieb Umang Jain:
>>>>> On 11/27/22 6:56 AM, Stefan Wahren wrote:
>>>>>> Am 26.11.22 um 17:26 schrieb Umang Jain:
>>>>>>> On 11/26/22 8:12 PM, Stefan Wahren wrote:
>>>>>>>> Am 21.11.22 um 22:47 schrieb Umang Jain:
>>>>>>>>> This series aims to upport bcm2835-isp from the RPi kernel [1] and is a
>>>>>>>>> independent subset of earlier series [2] posted to upport CSI-2/CCP2
>>>>>>>>> receiver IP core("Unicam) + the ISP driver found in BCM283x and compatible
>>>>>>>>> SoCs (namely BCM2711). Unicam is still under active development to work
>>>>>>>>> with multistream support to get into mainline. Hence only the ISP driver
>>>>>>>>> will remain the primary area of this series.
>>>>>>>> thanks for working on this. But honestly i would prefer that vchiq
>>>>>>>> comes out of staging before adding more features. As Greg said
>>>>>>>> some time ago staging is not a place to "dump code and run away".
>>>>>>>> These new files are in the same bad shape as the rest of vc04
>>>>>>>> before the clean-up here in staging started.
>>>>>>> Certainly, I am not here to do that - but I am still learning the ropes.
>>>>>> no problem.
>>>>>>
>>>>>>> If the staging issue is becoming a blocker for bcm2835-isp going
>>>>>>> upstream, I would be happy to help here! Though I must mention that
>>>>>>> I still have limited visibility so my aim would be to chart out a
>>>>>>> plan of things needed to be done to get vc04_services out of staging!
>>>>>> The vchiq driver is in staging since 2016, so every step forwards is
>>>>>> good. Unfortunately all of the low hanging fruits has been gathered.
>>>>>>
>>>>>> For me the most important, but not to tricky steps to get vchiq out
>>>>>> of staging would be:
>>>>>>
>>>>>> * Cleanup logging mechanism
>>>>>>
>>>>>> * Get rid of custom function return values
>>>>>>
>>>>>> There was already an attempt for this [1]
>>>>>>
>>>>>> * Get rid of all non essential global structures and create a proper per
>>>>>> device structure
>>>>>>
>>>>>>>> I agree that VCSM is on the TODO list for vchiq, but this driver
>>>>>>>> is not necessary for making bcm2835-audio & bcm2835-camera leave
>>>>>>>> staging. It just binds more resources on a new feature.
>>>>> bcm2835-camera is the legacy camera stack which probably need to be
>>>>> dropped from hereon...
>>>> I don't not know if there any users left, so i would be careful here.
>>>> Can bcm2835-isp completely replace bcm2835-camera? Sorry, for this
>>>> dumb question but i'm not expert here.
>>> I am careful too here and probably need Input from RaspberryPi in order
>>> to proceed to drop it. But from my perspective - bcm2835-camera is _not_
>>> going out of staging - it'll either sit here (or probably dropped) as
>>> statied from [1]
>>>
>>> ```
>>> + * There are two camera drivers in the kernel for BCM283x - this one
>>> + * and bcm2835-camera (currently in staging).
>>> ```
>>>
>>> The bcm2835-camera is meant to be replaced by unicam [1] , but the ISP
>>> (bcm2835-isp) is meant to be worked with unicam [1]. In fact, I have
>>> mentioned in my cover the testing of bcm2835-isp happened on top of
>>> unicam patches.
>> To be accurate, the bcm2835-camera driver supports the VC4
>> firmware-based camera stack. In that setup, the camera sensors (OV5647
>> or IMX219), CSI-2 receiver (Unicam) and ISP are all controlled by the
>> firmware, which provides a high-level interface towards the kernel. This
>> architecture has been replaced by Linux-side control of the camera
>> sensors (through existing drivers in drivers/media/i2c/), Unicam
>> (through the driver from [1]) and ISP (through this driver). Moving
>> control to the Linux side requires complex processing in userspace,
>> handled by libcamera.
>>
>> bcm2835-camera is thus replaced by multiple drivers combined with
>> libcamera, and that is the camera stack that is shipped by Raspberry Pi
>> these days. While this may affect some userspace use cases), we will not
>> work on destaging bcm2835-camera, and as far as I'm aware, nobody else
>> is planning to do so either. I don't mind much if the driver stays in
>> staging for some more time, but I'd rather drop it if possible.
Thanks for clarification. Okay, so Unicam + bcm2835-isp are able to 
handle the old camera (OV5647)?
> It would be reasonable to drop it at the point that Libcamera can work
> to a similar level with at least the following list of applications:
> - FFmpeg
> - Gstreamer
> - Chromium
> - Firefox
> - Motion
> And that still leaves a huge number of existing V4L2 apps out in the cold.
>
> Do you wish to make any predictions as to when that would be
> achievable? Or even when a v1.0 release of libcamera is going to
> happen?
> Dropping anything prior to those points would be rather premature in my book.
>
>
> The TODOs on bcm2835-camera are:
> 1) Zero copy. That comes almost for free as bcm2835-isp already does
> this, but it does rely on vcsm-cma.
> The main reason I haven't pushed it is that it then requires
> reasonable amounts of CMA heap for all the buffers, which until
> recently haven't been present in the default configurations. With the
> vc4 DRM driver now being default (at least for the vendor kernel) and
> also requiring CMA, making the change makes more sense.
> AFAIK there is no easy way to have one driver choosing between using
> vb2_vmalloc_memops and vb2_dma_contig_memops at runtime, but I may be
> wrong.
> Actually bcm2835_defconfig appears to only allocate a 32MB CMA heap,
> so perhaps we don't get very far.
CMA configuration should actually happen in device tree or kernel 
cmdline. The bcm2835_defconfig is limited to make it work even with the 
original Pi.
>
> 2) This isn't workable within the current V4L2 frameworks. The
> multi-planar V4L2 pixel formats are currently allocated as independent
> buffers for each plane, whereas the firmware needs a single buffer
> with (currently) specific offsets for the chroma planes. The
> V4L2/videobuf2 core changes required to implement that are going to be
> significant, and have minimal gain.
> The specific stride handling is already dealt with (set bytesperline
> appropriately), it's the padding of the height to a multiple of 16
> before the chroma planes on YUV420 and NV12 formats that require the
> firmware to do a small amount of repacking. The performance hit is
> actually minimal anyway.
>
> If bcm2835-camera is the only thing holding back vc04_services, then I
> can have a look at it.

No, it's the vchiq interface which needs the work.

Thanks Stefan

>
>    Dave
>
>>> [1]: https://lore.kernel.org/linux-media/20220208155027.891055-5-jeanmichel.hautbois@ideasonboard.com/
>>>
>>>>>>> I see two TODO files in vc04_services:
>>>>>>>      ./bcm2835-camera/TODO
>>>>>>>      ./interface/TODO
>>>>>>>
>>>>>>> One of the bcm2835-camera TODO points to the vc-sm-cma driver
>>>>>>> itself. So that's address in the series. The other remaining one -
>>>>>>> I will need to take a deeper look before commenting on it.
>>>>>>>
>>>>>>> The main chunk of TODO are in vc04_services/interfaces/TODO. Doing
>>>>>>> a cursory reading of them suggests that these apply to *all*
>>>>>>> vc04_services components? Am I right?
>>>>>> Actually these applies just for the interfaces directory. Some of
>>>>>> them could apply to the services, but this is no priority.
>>>>> By no priority, you mean this doesn't affect the criteria required to
>>>>> ful-fill to get these out of staging?
>>>> Correct
>>>>
>>>>>>> Are these are the specific bits of cleanup you are referring to in
>>>>>>> your comment?
>>>>>> You mean about bcm2835-isp? There were too many changes to vchiq
>>>>>> that i don't remember them all. The first that come to my mind was
>>>>>> those fancy comment sections which is not kernel coding style. It
>>>>>> has been removed.
>>>>> No, I don't mean the bcm2835-isp changes (those are upcoming /
>>>>> out-of-tree still so...). I mean what are the specific bits / points
>>>>> that needs to be addressed to get vc04_services out of the staging.
>>>> These were the points which i mentioned in my last email. They came
>>>> from interface/TODO.
>>>>
>>>>> You have mentioned it above now, so I'll follow up on those.
>>>> That would be great :)
>>>>
>>>>> The many vchiq changes you referred to above comment (that you don't
>>>>> remember) are from [1] as well or some other series ?
>>>> Sorry, for the confusing. The many changes i refer were the dozens of
>>>> clean up patches for vc04_interfaces in mainline staging since the
>>>> last years. [1] was just a single patch which has been accepted yet.
>>> Ah I see. There are many others that I've to dig out then. Thanks for
>>> clarifying!
>>>
>>>>>> [1] -
>>>>>> https://lore.kernel.org/linux-staging/20220712181928.17547-1-jslebodn@redhat.com/
>>>>>>
>>>>>>>> Unfortuntately i hadn't much time to work on vchiq by myself.
>>>>>>>>
>>>>>>>> Just my two cents
>>>>>>>> Stefan
>> --
>> Regards,
>>
>> Laurent Pinchart
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
  

Hi Folks,

> > On Fri, Dec 02, 2022 at 11:23:29AM +0000, Dave Stevenson wrote:
> >> Dropping anything prior to those points would be rather premature in my book.
> > Something I forgot to mention is that there should be no issue at all
> > keeping bcm2835-camera fully supported in the Raspberry Pi downstream
> > kernel for a longer period of time. It's in upstream that I don't think
> > it should be destaged, as it's already considered legacy and should be
> > phased out. Do you know if there are users of that driver with a
> > mainline kernel ?
> >
> In Fedora there seems to be no official support [1], but openSuSE seems
> to mention it [2].

It's enabled in Fedora but I've never had a huge amount of luck with
it, I would be OK with the firmware driver to disappear when ever.

From our point of view I'd sooner we can just move to the unicam in
kernel one sooner rather than later even swapping one for the other in
staging to get the libcamera one in and improve it in kernel via the
staging process, I feel that would be better use of the very few
cycles I have for RPi bits.

Peter

> [1] -
> https://docs.fedoraproject.org/en-US/quick-docs/raspberry-pi/#_does_the_add_on_camera_work
>
> [2] - https://en.opensuse.org/HCL:Raspberry_Pi_Camera_Modules
>
  

Hi Dave,

On Fri, Dec 02, 2022 at 12:38:09PM +0000, Dave Stevenson wrote:
> On Fri, 2 Dec 2022 at 12:10, Laurent Pinchart wrote:
> > On Fri, Dec 02, 2022 at 11:23:29AM +0000, Dave Stevenson wrote:
> > > On Fri, 2 Dec 2022 at 09:17, Laurent Pinchart wrote:
> > > > On Fri, Dec 02, 2022 at 11:57:18AM +0800, Umang Jain wrote:
> > > > > On 12/2/22 6:45 AM, Stefan Wahren wrote:
> > > > > > Am 30.11.22 um 11:58 schrieb Umang Jain:
> > > > > >> On 11/27/22 6:56 AM, Stefan Wahren wrote:
> > > > > >>> Am 26.11.22 um 17:26 schrieb Umang Jain:
> > > > > >>>> On 11/26/22 8:12 PM, Stefan Wahren wrote:
> > > > > >>>>> Am 21.11.22 um 22:47 schrieb Umang Jain:
> > > > > >>>>>> This series aims to upport bcm2835-isp from the RPi kernel [1] and is a
> > > > > >>>>>> independent subset of earlier series [2] posted to upport CSI-2/CCP2
> > > > > >>>>>> receiver IP core("Unicam) + the ISP driver found in BCM283x and compatible
> > > > > >>>>>> SoCs (namely BCM2711). Unicam is still under active development to work
> > > > > >>>>>> with multistream support to get into mainline. Hence only the ISP driver
> > > > > >>>>>> will remain the primary area of this series.
> > > > > >>>>>
> > > > > >>>>> thanks for working on this. But honestly i would prefer that vchiq
> > > > > >>>>> comes out of staging before adding more features. As Greg said
> > > > > >>>>> some time ago staging is not a place to "dump code and run away".
> > > > > >>>>> These new files are in the same bad shape as the rest of vc04
> > > > > >>>>> before the clean-up here in staging started.
> > > > > >>>>
> > > > > >>>> Certainly, I am not here to do that - but I am still learning the ropes.
> > > > > >>>
> > > > > >>> no problem.
> > > > > >>>
> > > > > >>>> If the staging issue is becoming a blocker for bcm2835-isp going
> > > > > >>>> upstream, I would be happy to help here! Though I must mention that
> > > > > >>>> I still have limited visibility so my aim would be to chart out a
> > > > > >>>> plan of things needed to be done to get vc04_services out of staging!
> > > > > >>>
> > > > > >>> The vchiq driver is in staging since 2016, so every step forwards is
> > > > > >>> good. Unfortunately all of the low hanging fruits has been gathered.
> > > > > >>>
> > > > > >>> For me the most important, but not to tricky steps to get vchiq out
> > > > > >>> of staging would be:
> > > > > >>>
> > > > > >>> * Cleanup logging mechanism
> > > > > >>>
> > > > > >>> * Get rid of custom function return values
> > > > > >>>
> > > > > >>> There was already an attempt for this [1]
> > > > > >>>
> > > > > >>> * Get rid of all non essential global structures and create a proper per
> > > > > >>> device structure
> > > > > >>>
> > > > > >>>>> I agree that VCSM is on the TODO list for vchiq, but this driver
> > > > > >>>>> is not necessary for making bcm2835-audio & bcm2835-camera leave
> > > > > >>>>> staging. It just binds more resources on a new feature.
> > > > > >>
> > > > > >> bcm2835-camera is the legacy camera stack which probably need to be
> > > > > >> dropped from hereon...
> > > > > >
> > > > > > I don't not know if there any users left, so i would be careful here.
> > > > > > Can bcm2835-isp completely replace bcm2835-camera? Sorry, for this
> > > > > > dumb question but i'm not expert here.
> > > > >
> > > > > I am careful too here and probably need Input from RaspberryPi in order
> > > > > to proceed to drop it. But from my perspective - bcm2835-camera is _not_
> > > > > going out of staging - it'll either sit here (or probably dropped) as
> > > > > statied from [1]
> > > > >
> > > > > ```
> > > > > + * There are two camera drivers in the kernel for BCM283x - this one
> > > > > + * and bcm2835-camera (currently in staging).
> > > > > ```
> > > > >
> > > > > The bcm2835-camera is meant to be replaced by unicam [1] , but the ISP
> > > > > (bcm2835-isp) is meant to be worked with unicam [1]. In fact, I have
> > > > > mentioned in my cover the testing of bcm2835-isp happened on top of
> > > > > unicam patches.
> > > >
> > > > To be accurate, the bcm2835-camera driver supports the VC4
> > > > firmware-based camera stack. In that setup, the camera sensors (OV5647
> > > > or IMX219), CSI-2 receiver (Unicam) and ISP are all controlled by the
> > > > firmware, which provides a high-level interface towards the kernel. This
> > > > architecture has been replaced by Linux-side control of the camera
> > > > sensors (through existing drivers in drivers/media/i2c/), Unicam
> > > > (through the driver from [1]) and ISP (through this driver). Moving
> > > > control to the Linux side requires complex processing in userspace,
> > > > handled by libcamera.
> > > >
> > > > bcm2835-camera is thus replaced by multiple drivers combined with
> > > > libcamera, and that is the camera stack that is shipped by Raspberry Pi
> > > > these days. While this may affect some userspace use cases), we will not
> > > > work on destaging bcm2835-camera, and as far as I'm aware, nobody else
> > > > is planning to do so either. I don't mind much if the driver stays in
> > > > staging for some more time, but I'd rather drop it if possible.
> > >
> > > It would be reasonable to drop it at the point that Libcamera can work
> > > to a similar level with at least the following list of applications:
> > > - FFmpeg
> > > - Gstreamer
> > > - Chromium
> > > - Firefox
> > > - Motion
> > > And that still leaves a huge number of existing V4L2 apps out in the cold.
> >
> > That's exactly the kind of input we were looking for, thanks.
> 
> Please note that those are only the 5 major ones that came immediately
> to mind as having significant numbers of users. There are obviously
> many more V4L2 apps around which currently "just work" if they use the
> API correctly.

Sure, I'll keep that in mind.

> > GStreamer is already addressed.
> >
> > Chromium and Firefox will go through PipeWire. There is a working
> > implementation in libwebrtc, Kieran may be able to comment on the
> > upstreaming state. It will take some time for distributions to switch,
> > and I can't predict the time line, but that seems to clearly be the
> > direction the Linux desktop is taking.
> >
> > I haven't looked at FFmpeg yet (maybe someone has ?). It probably makes
> > sense to add native libcamera support to FFmpeg, even if PipeWire
> > support could also make sense. It could also make sense to expose in the
> > libcamera V4L2 compat layer the libv4l2 API functions, as that would
> > allow linking FFmpeg (when compiled with CONFIG_LIBV4L2) to libcamera
> > instead of libv4l2, but Debian doesn't set CONFIG_LIBV4L2, so this isn't
> > an immediate solution to the problem.
> 
> Native would be nice in my book, but isn't always as straightforward.

I agree, native would be good.

> > Same thing for motion, except it has no libv4l2 support. The V4L2 compat
> > layer could still be used with LD_PRELOAD, but that's not a great
> > solution. Native libcamera support would make more sense (or possibly
> > even GStreamer support, I don't know if upstream would accept that).
> 
> The main bit missing with the compat layer is controls. AIUI fixing
> that isn't a priority.

Good point. That's something we should fix. Not a high priority indeed,
but still good to have (and it shouldn't take too long).

> Things like Motion being used for security purposes need to be able to
> set properties like exposure modes in order to achieve usable images
> under changing lighting conditions.
> 
> > In a side note, how do the above applications work today on Raspberry Pi
> > platforms that use a sensor not supported by the legacy camera stack ?
> 
> They don't for raw sensors, but do work with USB webcams.
> Raspberry Pi have sold camera modules since 2013 (OV5647), with v2
> (IMX219) coming along in 2016, and HQ (IMX477) in 2020. Linux has only
> had frameworks to sensibly support raw image sensors since libcamera
> came on the scene around 2019.

Do I understand correctly that the need to keep bcm2835-camera working
is for support of the v1 and v2 camera modules only ? Or are there other
use cases ?

> > > Do you wish to make any predictions as to when that would be
> > > achievable? Or even when a v1.0 release of libcamera is going to
> > > happen?
> >
> > Now that we have started tagging releases, we've also decided to publish
> > a roadmap with the development still needed to stabilize the API. We'll
> > likely start working on it this month.
> >
> > > Dropping anything prior to those points would be rather premature in my book.
> >
> > Something I forgot to mention is that there should be no issue at all
> > keeping bcm2835-camera fully supported in the Raspberry Pi downstream
> > kernel for a longer period of time. It's in upstream that I don't think
> > it should be destaged, as it's already considered legacy and should be
> > phased out. Do you know if there are users of that driver with a
> > mainline kernel ?
> 
> There are a number of distros that use mainline kernels rather than
> our vendor kernel. I don't follow distros in detail, but I believe
> both Gentoo and Opensuse fall into that category.
>  Any users of the camera module under those will be using the mainline
> bcm2835-camera module. I have no stats for who uses those on a Pi.

OK.

> > > The TODOs on bcm2835-camera are:
> > > 1) Zero copy. That comes almost for free as bcm2835-isp already does
> > > this, but it does rely on vcsm-cma.
> > > The main reason I haven't pushed it is that it then requires
> > > reasonable amounts of CMA heap for all the buffers, which until
> > > recently haven't been present in the default configurations. With the
> > > vc4 DRM driver now being default (at least for the vendor kernel) and
> > > also requiring CMA, making the change makes more sense.
> > > AFAIK there is no easy way to have one driver choosing between using
> > > vb2_vmalloc_memops and vb2_dma_contig_memops at runtime, but I may be
> > > wrong.
> >
> > Some drivers use a module parameter for that, but that's not great.
> 
> I had thought of a module parameter, but it becomes yet another
> configuration thing to get incorrectly set.
> Perhaps Kconfig based on the setting for CMA_SIZE_MBYTES, but that can
> be overridden.

The trouble with vb2-vmalloc is that you'll end up copying data, which I
expect to cause various performance issues that may generate bug reports
difficult to address. Increasing the CMA size would be better.

> > > Actually bcm2835_defconfig appears to only allocate a 32MB CMA heap,
> > > so perhaps we don't get very far.
> > >
> > > 2) This isn't workable within the current V4L2 frameworks. The
> > > multi-planar V4L2 pixel formats are currently allocated as independent
> > > buffers for each plane, whereas the firmware needs a single buffer
> > > with (currently) specific offsets for the chroma planes. The
> > > V4L2/videobuf2 core changes required to implement that are going to be
> > > significant, and have minimal gain.
> > > The specific stride handling is already dealt with (set bytesperline
> > > appropriately), it's the padding of the height to a multiple of 16
> > > before the chroma planes on YUV420 and NV12 formats that require the
> > > firmware to do a small amount of repacking. The performance hit is
> > > actually minimal anyway.
> > >
> > > If bcm2835-camera is the only thing holding back vc04_services, then I
> > > can have a look at it.
> >
> > I'll let Umang comment on whether it's holding vc04_services back, but
> > my understanding it that we could destage vc04_services while keeping
> > bcm2835-camera in staging for the time being. If anyone disagrees with
> > that, please let me know.
> >
> > > > > [1]: https://lore.kernel.org/linux-media/20220208155027.891055-5-jeanmichel.hautbois@ideasonboard.com/
> > > > >
> > > > > >>>>
> > > > > >>>> I see two TODO files in vc04_services:
> > > > > >>>>     ./bcm2835-camera/TODO
> > > > > >>>>     ./interface/TODO
> > > > > >>>>
> > > > > >>>> One of the bcm2835-camera TODO points to the vc-sm-cma driver
> > > > > >>>> itself. So that's address in the series. The other remaining one -
> > > > > >>>> I will need to take a deeper look before commenting on it.
> > > > > >>>>
> > > > > >>>> The main chunk of TODO are in vc04_services/interfaces/TODO. Doing
> > > > > >>>> a cursory reading of them suggests that these apply to *all*
> > > > > >>>> vc04_services components? Am I right?
> > > > > >>>
> > > > > >>> Actually these applies just for the interfaces directory. Some of
> > > > > >>> them could apply to the services, but this is no priority.
> > > > > >>
> > > > > >> By no priority, you mean this doesn't affect the criteria required to
> > > > > >> ful-fill to get these out of staging?
> > > > > >
> > > > > > Correct
> > > > > >
> > > > > >>>> Are these are the specific bits of cleanup you are referring to in
> > > > > >>>> your comment?
> > > > > >>>
> > > > > >>> You mean about bcm2835-isp? There were too many changes to vchiq
> > > > > >>> that i don't remember them all. The first that come to my mind was
> > > > > >>> those fancy comment sections which is not kernel coding style. It
> > > > > >>> has been removed.
> > > > > >>
> > > > > >> No, I don't mean the bcm2835-isp changes (those are upcoming /
> > > > > >> out-of-tree still so...). I mean what are the specific bits / points
> > > > > >> that needs to be addressed to get vc04_services out of the staging.
> > > > > >
> > > > > > These were the points which i mentioned in my last email. They came
> > > > > > from interface/TODO.
> > > > > >
> > > > > >> You have mentioned it above now, so I'll follow up on those.
> > > > > >
> > > > > > That would be great :)
> > > > > >
> > > > > >> The many vchiq changes you referred to above comment (that you don't
> > > > > >> remember) are from [1] as well or some other series ?
> > > > > >
> > > > > > Sorry, for the confusing. The many changes i refer were the dozens of
> > > > > > clean up patches for vc04_interfaces in mainline staging since the
> > > > > > last years. [1] was just a single patch which has been accepted yet.
> > > > >
> > > > > Ah I see. There are many others that I've to dig out then. Thanks for
> > > > > clarifying!
> > > > >
> > > > > >>> [1] -
> > > > > >>> https://lore.kernel.org/linux-staging/20220712181928.17547-1-jslebodn@redhat.com/
> > > > > >>>
> > > > > >>>>> Unfortuntately i hadn't much time to work on vchiq by myself.
> > > > > >>>>>
> > > > > >>>>> Just my two cents
> > > > > >>>>> Stefan
  

Hi Stefan,

On Fri, Dec 02, 2022 at 01:41:53PM +0100, Stefan Wahren wrote:
> Am 02.12.22 um 12:23 schrieb Dave Stevenson:
> > On Fri, 2 Dec 2022 at 09:17, Laurent Pinchart wrote:
> >> On Fri, Dec 02, 2022 at 11:57:18AM +0800, Umang Jain wrote:
> >>> On 12/2/22 6:45 AM, Stefan Wahren wrote:
> >>>> Am 30.11.22 um 11:58 schrieb Umang Jain:
> >>>>> On 11/27/22 6:56 AM, Stefan Wahren wrote:
> >>>>>> Am 26.11.22 um 17:26 schrieb Umang Jain:
> >>>>>>> On 11/26/22 8:12 PM, Stefan Wahren wrote:
> >>>>>>>> Am 21.11.22 um 22:47 schrieb Umang Jain:
> >>>>>>>>> This series aims to upport bcm2835-isp from the RPi kernel [1] and is a
> >>>>>>>>> independent subset of earlier series [2] posted to upport CSI-2/CCP2
> >>>>>>>>> receiver IP core("Unicam) + the ISP driver found in BCM283x and compatible
> >>>>>>>>> SoCs (namely BCM2711). Unicam is still under active development to work
> >>>>>>>>> with multistream support to get into mainline. Hence only the ISP driver
> >>>>>>>>> will remain the primary area of this series.
> >>>>>>>> thanks for working on this. But honestly i would prefer that vchiq
> >>>>>>>> comes out of staging before adding more features. As Greg said
> >>>>>>>> some time ago staging is not a place to "dump code and run away".
> >>>>>>>> These new files are in the same bad shape as the rest of vc04
> >>>>>>>> before the clean-up here in staging started.
> >>>>>>>
> >>>>>>> Certainly, I am not here to do that - but I am still learning the ropes.
> >>>>>>
> >>>>>> no problem.
> >>>>>>
> >>>>>>> If the staging issue is becoming a blocker for bcm2835-isp going
> >>>>>>> upstream, I would be happy to help here! Though I must mention that
> >>>>>>> I still have limited visibility so my aim would be to chart out a
> >>>>>>> plan of things needed to be done to get vc04_services out of staging!
> >>>>>>
> >>>>>> The vchiq driver is in staging since 2016, so every step forwards is
> >>>>>> good. Unfortunately all of the low hanging fruits has been gathered.
> >>>>>>
> >>>>>> For me the most important, but not to tricky steps to get vchiq out
> >>>>>> of staging would be:
> >>>>>>
> >>>>>> * Cleanup logging mechanism
> >>>>>>
> >>>>>> * Get rid of custom function return values
> >>>>>>
> >>>>>> There was already an attempt for this [1]
> >>>>>>
> >>>>>> * Get rid of all non essential global structures and create a proper per
> >>>>>> device structure
> >>>>>>
> >>>>>>>> I agree that VCSM is on the TODO list for vchiq, but this driver
> >>>>>>>> is not necessary for making bcm2835-audio & bcm2835-camera leave
> >>>>>>>> staging. It just binds more resources on a new feature.
> >>>>>
> >>>>> bcm2835-camera is the legacy camera stack which probably need to be
> >>>>> dropped from hereon...
> >>>>
> >>>> I don't not know if there any users left, so i would be careful here.
> >>>> Can bcm2835-isp completely replace bcm2835-camera? Sorry, for this
> >>>> dumb question but i'm not expert here.
> >>>
> >>> I am careful too here and probably need Input from RaspberryPi in order
> >>> to proceed to drop it. But from my perspective - bcm2835-camera is _not_
> >>> going out of staging - it'll either sit here (or probably dropped) as
> >>> statied from [1]
> >>>
> >>> ```
> >>> + * There are two camera drivers in the kernel for BCM283x - this one
> >>> + * and bcm2835-camera (currently in staging).
> >>> ```
> >>>
> >>> The bcm2835-camera is meant to be replaced by unicam [1] , but the ISP
> >>> (bcm2835-isp) is meant to be worked with unicam [1]. In fact, I have
> >>> mentioned in my cover the testing of bcm2835-isp happened on top of
> >>> unicam patches.
> >>
> >> To be accurate, the bcm2835-camera driver supports the VC4
> >> firmware-based camera stack. In that setup, the camera sensors (OV5647
> >> or IMX219), CSI-2 receiver (Unicam) and ISP are all controlled by the
> >> firmware, which provides a high-level interface towards the kernel. This
> >> architecture has been replaced by Linux-side control of the camera
> >> sensors (through existing drivers in drivers/media/i2c/), Unicam
> >> (through the driver from [1]) and ISP (through this driver). Moving
> >> control to the Linux side requires complex processing in userspace,
> >> handled by libcamera.
> >>
> >> bcm2835-camera is thus replaced by multiple drivers combined with
> >> libcamera, and that is the camera stack that is shipped by Raspberry Pi
> >> these days. While this may affect some userspace use cases), we will not
> >> work on destaging bcm2835-camera, and as far as I'm aware, nobody else
> >> is planning to do so either. I don't mind much if the driver stays in
> >> staging for some more time, but I'd rather drop it if possible.
>
> Thanks for clarification. Okay, so Unicam + bcm2835-isp are able to 
> handle the old camera (OV5647)?

Yes, but only for applications using libcamera, directly or indirectly
(through GStreamer for instance, or the V4L2 LD_PRELOAD-ed compatibility
later), not natively through V4L2 only.

> > It would be reasonable to drop it at the point that Libcamera can work
> > to a similar level with at least the following list of applications:
> > - FFmpeg
> > - Gstreamer
> > - Chromium
> > - Firefox
> > - Motion
> > And that still leaves a huge number of existing V4L2 apps out in the cold.
> >
> > Do you wish to make any predictions as to when that would be
> > achievable? Or even when a v1.0 release of libcamera is going to
> > happen?
> > Dropping anything prior to those points would be rather premature in my book.
> >
> >
> > The TODOs on bcm2835-camera are:
> > 1) Zero copy. That comes almost for free as bcm2835-isp already does
> > this, but it does rely on vcsm-cma.
> > The main reason I haven't pushed it is that it then requires
> > reasonable amounts of CMA heap for all the buffers, which until
> > recently haven't been present in the default configurations. With the
> > vc4 DRM driver now being default (at least for the vendor kernel) and
> > also requiring CMA, making the change makes more sense.
> > AFAIK there is no easy way to have one driver choosing between using
> > vb2_vmalloc_memops and vb2_dma_contig_memops at runtime, but I may be
> > wrong.
> > Actually bcm2835_defconfig appears to only allocate a 32MB CMA heap,
> > so perhaps we don't get very far.
>
> CMA configuration should actually happen in device tree or kernel 
> cmdline. The bcm2835_defconfig is limited to make it work even with the 
> original Pi.
>
> > 2) This isn't workable within the current V4L2 frameworks. The
> > multi-planar V4L2 pixel formats are currently allocated as independent
> > buffers for each plane, whereas the firmware needs a single buffer
> > with (currently) specific offsets for the chroma planes. The
> > V4L2/videobuf2 core changes required to implement that are going to be
> > significant, and have minimal gain.
> > The specific stride handling is already dealt with (set bytesperline
> > appropriately), it's the padding of the height to a multiple of 16
> > before the chroma planes on YUV420 and NV12 formats that require the
> > firmware to do a small amount of repacking. The performance hit is
> > actually minimal anyway.
> >
> > If bcm2835-camera is the only thing holding back vc04_services, then I
> > can have a look at it.
> 
> No, it's the vchiq interface which needs the work.
> 
> >>> [1]: https://lore.kernel.org/linux-media/20220208155027.891055-5-jeanmichel.hautbois@ideasonboard.com/
> >>>
> >>>>>>> I see two TODO files in vc04_services:
> >>>>>>>      ./bcm2835-camera/TODO
> >>>>>>>      ./interface/TODO
> >>>>>>>
> >>>>>>> One of the bcm2835-camera TODO points to the vc-sm-cma driver
> >>>>>>> itself. So that's address in the series. The other remaining one -
> >>>>>>> I will need to take a deeper look before commenting on it.
> >>>>>>>
> >>>>>>> The main chunk of TODO are in vc04_services/interfaces/TODO. Doing
> >>>>>>> a cursory reading of them suggests that these apply to *all*
> >>>>>>> vc04_services components? Am I right?
> >>>>>>
> >>>>>> Actually these applies just for the interfaces directory. Some of
> >>>>>> them could apply to the services, but this is no priority.
> >>>>>
> >>>>> By no priority, you mean this doesn't affect the criteria required to
> >>>>> ful-fill to get these out of staging?
> >>>>
> >>>> Correct
> >>>>
> >>>>>>> Are these are the specific bits of cleanup you are referring to in
> >>>>>>> your comment?
> >>>>>>
> >>>>>> You mean about bcm2835-isp? There were too many changes to vchiq
> >>>>>> that i don't remember them all. The first that come to my mind was
> >>>>>> those fancy comment sections which is not kernel coding style. It
> >>>>>> has been removed.
> >>>>>
> >>>>> No, I don't mean the bcm2835-isp changes (those are upcoming /
> >>>>> out-of-tree still so...). I mean what are the specific bits / points
> >>>>> that needs to be addressed to get vc04_services out of the staging.
> >>>>
> >>>> These were the points which i mentioned in my last email. They came
> >>>> from interface/TODO.
> >>>>
> >>>>> You have mentioned it above now, so I'll follow up on those.
> >>>>
> >>>> That would be great :)
> >>>>
> >>>>> The many vchiq changes you referred to above comment (that you don't
> >>>>> remember) are from [1] as well or some other series ?
> >>>> Sorry, for the confusing. The many changes i refer were the dozens of
> >>>> clean up patches for vc04_interfaces in mainline staging since the
> >>>> last years. [1] was just a single patch which has been accepted yet.
> >>>
> >>> Ah I see. There are many others that I've to dig out then. Thanks for
> >>> clarifying!
> >>>
> >>>>>> [1] -
> >>>>>> https://lore.kernel.org/linux-staging/20220712181928.17547-1-jslebodn@redhat.com/
> >>>>>>
> >>>>>>>> Unfortuntately i hadn't much time to work on vchiq by myself.
> >>>>>>>>
> >>>>>>>> Just my two cents
> >>>>>>>> Stefan
  

Hi Stefan

On Fri, 2 Dec 2022 at 12:41, Stefan Wahren <stefan.wahren@i2se.com> wrote:
>
> Hi Dave,
>
> Am 02.12.22 um 12:23 schrieb Dave Stevenson:
> > Hi Laurent, Umang, and Stefan.
> >
> > On Fri, 2 Dec 2022 at 09:17, Laurent Pinchart
> > <laurent.pinchart@ideasonboard.com> wrote:
> >> Hi Umang,
> >>
> >> On Fri, Dec 02, 2022 at 11:57:18AM +0800, Umang Jain wrote:
> >>> On 12/2/22 6:45 AM, Stefan Wahren wrote:
> >>>> Am 30.11.22 um 11:58 schrieb Umang Jain:
> >>>>> On 11/27/22 6:56 AM, Stefan Wahren wrote:
> >>>>>> Am 26.11.22 um 17:26 schrieb Umang Jain:
> >>>>>>> On 11/26/22 8:12 PM, Stefan Wahren wrote:
> >>>>>>>> Am 21.11.22 um 22:47 schrieb Umang Jain:
> >>>>>>>>> This series aims to upport bcm2835-isp from the RPi kernel [1] and is a
> >>>>>>>>> independent subset of earlier series [2] posted to upport CSI-2/CCP2
> >>>>>>>>> receiver IP core("Unicam) + the ISP driver found in BCM283x and compatible
> >>>>>>>>> SoCs (namely BCM2711). Unicam is still under active development to work
> >>>>>>>>> with multistream support to get into mainline. Hence only the ISP driver
> >>>>>>>>> will remain the primary area of this series.
> >>>>>>>> thanks for working on this. But honestly i would prefer that vchiq
> >>>>>>>> comes out of staging before adding more features. As Greg said
> >>>>>>>> some time ago staging is not a place to "dump code and run away".
> >>>>>>>> These new files are in the same bad shape as the rest of vc04
> >>>>>>>> before the clean-up here in staging started.
> >>>>>>> Certainly, I am not here to do that - but I am still learning the ropes.
> >>>>>> no problem.
> >>>>>>
> >>>>>>> If the staging issue is becoming a blocker for bcm2835-isp going
> >>>>>>> upstream, I would be happy to help here! Though I must mention that
> >>>>>>> I still have limited visibility so my aim would be to chart out a
> >>>>>>> plan of things needed to be done to get vc04_services out of staging!
> >>>>>> The vchiq driver is in staging since 2016, so every step forwards is
> >>>>>> good. Unfortunately all of the low hanging fruits has been gathered.
> >>>>>>
> >>>>>> For me the most important, but not to tricky steps to get vchiq out
> >>>>>> of staging would be:
> >>>>>>
> >>>>>> * Cleanup logging mechanism
> >>>>>>
> >>>>>> * Get rid of custom function return values
> >>>>>>
> >>>>>> There was already an attempt for this [1]
> >>>>>>
> >>>>>> * Get rid of all non essential global structures and create a proper per
> >>>>>> device structure
> >>>>>>
> >>>>>>>> I agree that VCSM is on the TODO list for vchiq, but this driver
> >>>>>>>> is not necessary for making bcm2835-audio & bcm2835-camera leave
> >>>>>>>> staging. It just binds more resources on a new feature.
> >>>>> bcm2835-camera is the legacy camera stack which probably need to be
> >>>>> dropped from hereon...
> >>>> I don't not know if there any users left, so i would be careful here.
> >>>> Can bcm2835-isp completely replace bcm2835-camera? Sorry, for this
> >>>> dumb question but i'm not expert here.
> >>> I am careful too here and probably need Input from RaspberryPi in order
> >>> to proceed to drop it. But from my perspective - bcm2835-camera is _not_
> >>> going out of staging - it'll either sit here (or probably dropped) as
> >>> statied from [1]
> >>>
> >>> ```
> >>> + * There are two camera drivers in the kernel for BCM283x - this one
> >>> + * and bcm2835-camera (currently in staging).
> >>> ```
> >>>
> >>> The bcm2835-camera is meant to be replaced by unicam [1] , but the ISP
> >>> (bcm2835-isp) is meant to be worked with unicam [1]. In fact, I have
> >>> mentioned in my cover the testing of bcm2835-isp happened on top of
> >>> unicam patches.
> >> To be accurate, the bcm2835-camera driver supports the VC4
> >> firmware-based camera stack. In that setup, the camera sensors (OV5647
> >> or IMX219), CSI-2 receiver (Unicam) and ISP are all controlled by the
> >> firmware, which provides a high-level interface towards the kernel. This
> >> architecture has been replaced by Linux-side control of the camera
> >> sensors (through existing drivers in drivers/media/i2c/), Unicam
> >> (through the driver from [1]) and ISP (through this driver). Moving
> >> control to the Linux side requires complex processing in userspace,
> >> handled by libcamera.
> >>
> >> bcm2835-camera is thus replaced by multiple drivers combined with
> >> libcamera, and that is the camera stack that is shipped by Raspberry Pi
> >> these days. While this may affect some userspace use cases), we will not
> >> work on destaging bcm2835-camera, and as far as I'm aware, nobody else
> >> is planning to do so either. I don't mind much if the driver stays in
> >> staging for some more time, but I'd rather drop it if possible.
> Thanks for clarification. Okay, so Unicam + bcm2835-isp are able to
> handle the old camera (OV5647)?

There are kernel drivers in mainline for ov5647 (v1) and imx219 (v2)
cameras, with imx477 (HQ camera) due to be done. Connected to
bcm2835-unicam that gives you raw images from these sensors.

However you have to use libcamera to get useful images with these
drivers - raw Bayer isn't easy to consume.
Libcamera has support for all three (and more) sensors in the RPi IPA
(image processing algorithms), and will make use of bcm2835-isp to
process the images appropriately.

> > It would be reasonable to drop it at the point that Libcamera can work
> > to a similar level with at least the following list of applications:
> > - FFmpeg
> > - Gstreamer
> > - Chromium
> > - Firefox
> > - Motion
> > And that still leaves a huge number of existing V4L2 apps out in the cold.
> >
> > Do you wish to make any predictions as to when that would be
> > achievable? Or even when a v1.0 release of libcamera is going to
> > happen?
> > Dropping anything prior to those points would be rather premature in my book.
> >
> >
> > The TODOs on bcm2835-camera are:
> > 1) Zero copy. That comes almost for free as bcm2835-isp already does
> > this, but it does rely on vcsm-cma.
> > The main reason I haven't pushed it is that it then requires
> > reasonable amounts of CMA heap for all the buffers, which until
> > recently haven't been present in the default configurations. With the
> > vc4 DRM driver now being default (at least for the vendor kernel) and
> > also requiring CMA, making the change makes more sense.
> > AFAIK there is no easy way to have one driver choosing between using
> > vb2_vmalloc_memops and vb2_dma_contig_memops at runtime, but I may be
> > wrong.
> > Actually bcm2835_defconfig appears to only allocate a 32MB CMA heap,
> > so perhaps we don't get very far.
> CMA configuration should actually happen in device tree or kernel
> cmdline. The bcm2835_defconfig is limited to make it work even with the
> original Pi.

It's the original 256MB Pis that are the issue as memory is so tight.
Losing a bigger chunk to CMA is a problem there.

I see that bcm283x.dtsi sets it to 64MB, which is probably sufficient
for most bcm2835-camera use cases, but leaves me worried that you'll
steal it from other things.

> >
> > 2) This isn't workable within the current V4L2 frameworks. The
> > multi-planar V4L2 pixel formats are currently allocated as independent
> > buffers for each plane, whereas the firmware needs a single buffer
> > with (currently) specific offsets for the chroma planes. The
> > V4L2/videobuf2 core changes required to implement that are going to be
> > significant, and have minimal gain.
> > The specific stride handling is already dealt with (set bytesperline
> > appropriately), it's the padding of the height to a multiple of 16
> > before the chroma planes on YUV420 and NV12 formats that require the
> > firmware to do a small amount of repacking. The performance hit is
> > actually minimal anyway.
> >
> > If bcm2835-camera is the only thing holding back vc04_services, then I
> > can have a look at it.
>
> No, it's the vchiq interface which needs the work.

OK, I'll liaise with Umang and may look to deal with a couple of them.
Things like documenting the memory barriers probably falls to Phil.

  Dave

> Thanks Stefan
>
> >
> >    Dave
> >
> >>> [1]: https://lore.kernel.org/linux-media/20220208155027.891055-5-jeanmichel.hautbois@ideasonboard.com/
> >>>
> >>>>>>> I see two TODO files in vc04_services:
> >>>>>>>      ./bcm2835-camera/TODO
> >>>>>>>      ./interface/TODO
> >>>>>>>
> >>>>>>> One of the bcm2835-camera TODO points to the vc-sm-cma driver
> >>>>>>> itself. So that's address in the series. The other remaining one -
> >>>>>>> I will need to take a deeper look before commenting on it.
> >>>>>>>
> >>>>>>> The main chunk of TODO are in vc04_services/interfaces/TODO. Doing
> >>>>>>> a cursory reading of them suggests that these apply to *all*
> >>>>>>> vc04_services components? Am I right?
> >>>>>> Actually these applies just for the interfaces directory. Some of
> >>>>>> them could apply to the services, but this is no priority.
> >>>>> By no priority, you mean this doesn't affect the criteria required to
> >>>>> ful-fill to get these out of staging?
> >>>> Correct
> >>>>
> >>>>>>> Are these are the specific bits of cleanup you are referring to in
> >>>>>>> your comment?
> >>>>>> You mean about bcm2835-isp? There were too many changes to vchiq
> >>>>>> that i don't remember them all. The first that come to my mind was
> >>>>>> those fancy comment sections which is not kernel coding style. It
> >>>>>> has been removed.
> >>>>> No, I don't mean the bcm2835-isp changes (those are upcoming /
> >>>>> out-of-tree still so...). I mean what are the specific bits / points
> >>>>> that needs to be addressed to get vc04_services out of the staging.
> >>>> These were the points which i mentioned in my last email. They came
> >>>> from interface/TODO.
> >>>>
> >>>>> You have mentioned it above now, so I'll follow up on those.
> >>>> That would be great :)
> >>>>
> >>>>> The many vchiq changes you referred to above comment (that you don't
> >>>>> remember) are from [1] as well or some other series ?
> >>>> Sorry, for the confusing. The many changes i refer were the dozens of
> >>>> clean up patches for vc04_interfaces in mainline staging since the
> >>>> last years. [1] was just a single patch which has been accepted yet.
> >>> Ah I see. There are many others that I've to dig out then. Thanks for
> >>> clarifying!
> >>>
> >>>>>> [1] -
> >>>>>> https://lore.kernel.org/linux-staging/20220712181928.17547-1-jslebodn@redhat.com/
> >>>>>>
> >>>>>>>> Unfortuntately i hadn't much time to work on vchiq by myself.
> >>>>>>>>
> >>>>>>>> Just my two cents
> >>>>>>>> Stefan
> >> --
> >> Regards,
> >>
> >> Laurent Pinchart
> > _______________________________________________
> > linux-arm-kernel mailing list
> > linux-arm-kernel@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
  

Hi Dave,

Am 02.12.22 um 14:42 schrieb Dave Stevenson:
> Hi Stefan
>
> On Fri, 2 Dec 2022 at 12:41, Stefan Wahren <stefan.wahren@i2se.com> wrote:
>> Hi Dave,
>>
>> Am 02.12.22 um 12:23 schrieb Dave Stevenson:
>>> Hi Laurent, Umang, and Stefan.
>>>
>>> On Fri, 2 Dec 2022 at 09:17, Laurent Pinchart
>>> <laurent.pinchart@ideasonboard.com> wrote:
>>>> Hi Umang,
>>>>
>>>> On Fri, Dec 02, 2022 at 11:57:18AM +0800, Umang Jain wrote:
>>>>> On 12/2/22 6:45 AM, Stefan Wahren wrote:
>>>>>> Am 30.11.22 um 11:58 schrieb Umang Jain:
>>>>>>> On 11/27/22 6:56 AM, Stefan Wahren wrote:
>>>>>>>> Am 26.11.22 um 17:26 schrieb Umang Jain:
>>>>>>>>> On 11/26/22 8:12 PM, Stefan Wahren wrote:
>>>>>>>>>> Am 21.11.22 um 22:47 schrieb Umang Jain:
>>>>>>>>>>> This series aims to upport bcm2835-isp from the RPi kernel [1] and is a
>>>>>>>>>>> independent subset of earlier series [2] posted to upport CSI-2/CCP2
>>>>>>>>>>> receiver IP core("Unicam) + the ISP driver found in BCM283x and compatible
>>>>>>>>>>> SoCs (namely BCM2711). Unicam is still under active development to work
>>>>>>>>>>> with multistream support to get into mainline. Hence only the ISP driver
>>>>>>>>>>> will remain the primary area of this series.
>>>>>>>>>> thanks for working on this. But honestly i would prefer that vchiq
>>>>>>>>>> comes out of staging before adding more features. As Greg said
>>>>>>>>>> some time ago staging is not a place to "dump code and run away".
>>>>>>>>>> These new files are in the same bad shape as the rest of vc04
>>>>>>>>>> before the clean-up here in staging started.
>>>>>>>>> Certainly, I am not here to do that - but I am still learning the ropes.
>>>>>>>> no problem.
>>>>>>>>
>>>>>>>>> If the staging issue is becoming a blocker for bcm2835-isp going
>>>>>>>>> upstream, I would be happy to help here! Though I must mention that
>>>>>>>>> I still have limited visibility so my aim would be to chart out a
>>>>>>>>> plan of things needed to be done to get vc04_services out of staging!
>>>>>>>> The vchiq driver is in staging since 2016, so every step forwards is
>>>>>>>> good. Unfortunately all of the low hanging fruits has been gathered.
>>>>>>>>
>>>>>>>> For me the most important, but not to tricky steps to get vchiq out
>>>>>>>> of staging would be:
>>>>>>>>
>>>>>>>> * Cleanup logging mechanism
>>>>>>>>
>>>>>>>> * Get rid of custom function return values
>>>>>>>>
>>>>>>>> There was already an attempt for this [1]
>>>>>>>>
>>>>>>>> * Get rid of all non essential global structures and create a proper per
>>>>>>>> device structure
>>>>>>>>
>>>>>>>>>> I agree that VCSM is on the TODO list for vchiq, but this driver
>>>>>>>>>> is not necessary for making bcm2835-audio & bcm2835-camera leave
>>>>>>>>>> staging. It just binds more resources on a new feature.
>>>>>>> bcm2835-camera is the legacy camera stack which probably need to be
>>>>>>> dropped from hereon...
>>>>>> I don't not know if there any users left, so i would be careful here.
>>>>>> Can bcm2835-isp completely replace bcm2835-camera? Sorry, for this
>>>>>> dumb question but i'm not expert here.
>>>>> I am careful too here and probably need Input from RaspberryPi in order
>>>>> to proceed to drop it. But from my perspective - bcm2835-camera is _not_
>>>>> going out of staging - it'll either sit here (or probably dropped) as
>>>>> statied from [1]
>>>>>
>>>>> ```
>>>>> + * There are two camera drivers in the kernel for BCM283x - this one
>>>>> + * and bcm2835-camera (currently in staging).
>>>>> ```
>>>>>
>>>>> The bcm2835-camera is meant to be replaced by unicam [1] , but the ISP
>>>>> (bcm2835-isp) is meant to be worked with unicam [1]. In fact, I have
>>>>> mentioned in my cover the testing of bcm2835-isp happened on top of
>>>>> unicam patches.
>>>> To be accurate, the bcm2835-camera driver supports the VC4
>>>> firmware-based camera stack. In that setup, the camera sensors (OV5647
>>>> or IMX219), CSI-2 receiver (Unicam) and ISP are all controlled by the
>>>> firmware, which provides a high-level interface towards the kernel. This
>>>> architecture has been replaced by Linux-side control of the camera
>>>> sensors (through existing drivers in drivers/media/i2c/), Unicam
>>>> (through the driver from [1]) and ISP (through this driver). Moving
>>>> control to the Linux side requires complex processing in userspace,
>>>> handled by libcamera.
>>>>
>>>> bcm2835-camera is thus replaced by multiple drivers combined with
>>>> libcamera, and that is the camera stack that is shipped by Raspberry Pi
>>>> these days. While this may affect some userspace use cases), we will not
>>>> work on destaging bcm2835-camera, and as far as I'm aware, nobody else
>>>> is planning to do so either. I don't mind much if the driver stays in
>>>> staging for some more time, but I'd rather drop it if possible.
>> Thanks for clarification. Okay, so Unicam + bcm2835-isp are able to
>> handle the old camera (OV5647)?
> There are kernel drivers in mainline for ov5647 (v1) and imx219 (v2)
> cameras, with imx477 (HQ camera) due to be done. Connected to
> bcm2835-unicam that gives you raw images from these sensors.
>
> However you have to use libcamera to get useful images with these
> drivers - raw Bayer isn't easy to consume.
> Libcamera has support for all three (and more) sensors in the RPi IPA
> (image processing algorithms), and will make use of bcm2835-isp to
> process the images appropriately.
>
>>> It would be reasonable to drop it at the point that Libcamera can work
>>> to a similar level with at least the following list of applications:
>>> - FFmpeg
>>> - Gstreamer
>>> - Chromium
>>> - Firefox
>>> - Motion
>>> And that still leaves a huge number of existing V4L2 apps out in the cold.
>>>
>>> Do you wish to make any predictions as to when that would be
>>> achievable? Or even when a v1.0 release of libcamera is going to
>>> happen?
>>> Dropping anything prior to those points would be rather premature in my book.
>>>
>>>
>>> The TODOs on bcm2835-camera are:
>>> 1) Zero copy. That comes almost for free as bcm2835-isp already does
>>> this, but it does rely on vcsm-cma.
>>> The main reason I haven't pushed it is that it then requires
>>> reasonable amounts of CMA heap for all the buffers, which until
>>> recently haven't been present in the default configurations. With the
>>> vc4 DRM driver now being default (at least for the vendor kernel) and
>>> also requiring CMA, making the change makes more sense.
>>> AFAIK there is no easy way to have one driver choosing between using
>>> vb2_vmalloc_memops and vb2_dma_contig_memops at runtime, but I may be
>>> wrong.
>>> Actually bcm2835_defconfig appears to only allocate a 32MB CMA heap,
>>> so perhaps we don't get very far.
>> CMA configuration should actually happen in device tree or kernel
>> cmdline. The bcm2835_defconfig is limited to make it work even with the
>> original Pi.
> It's the original 256MB Pis that are the issue as memory is so tight.
> Losing a bigger chunk to CMA is a problem there.
>
> I see that bcm283x.dtsi sets it to 64MB, which is probably sufficient
> for most bcm2835-camera use cases, but leaves me worried that you'll
> steal it from other things.
>
>>> 2) This isn't workable within the current V4L2 frameworks. The
>>> multi-planar V4L2 pixel formats are currently allocated as independent
>>> buffers for each plane, whereas the firmware needs a single buffer
>>> with (currently) specific offsets for the chroma planes. The
>>> V4L2/videobuf2 core changes required to implement that are going to be
>>> significant, and have minimal gain.
>>> The specific stride handling is already dealt with (set bytesperline
>>> appropriately), it's the padding of the height to a multiple of 16
>>> before the chroma planes on YUV420 and NV12 formats that require the
>>> firmware to do a small amount of repacking. The performance hit is
>>> actually minimal anyway.
>>>
>>> If bcm2835-camera is the only thing holding back vc04_services, then I
>>> can have a look at it.
>> No, it's the vchiq interface which needs the work.
> OK, I'll liaise with Umang and may look to deal with a couple of them.
> Things like documenting the memory barriers probably falls to Phil.

actually this item is already done. I missed to update the TODO file and 
send a patch soon.

Best regards

>
>    Dave
>
>> Thanks Stefan
>>
>>>     Dave
>>>
>>>>> [1]: https://lore.kernel.org/linux-media/20220208155027.891055-5-jeanmichel.hautbois@ideasonboard.com/
>>>>>
>>>>>>>>> I see two TODO files in vc04_services:
>>>>>>>>>       ./bcm2835-camera/TODO
>>>>>>>>>       ./interface/TODO
>>>>>>>>>
>>>>>>>>> One of the bcm2835-camera TODO points to the vc-sm-cma driver
>>>>>>>>> itself. So that's address in the series. The other remaining one -
>>>>>>>>> I will need to take a deeper look before commenting on it.
>>>>>>>>>
>>>>>>>>> The main chunk of TODO are in vc04_services/interfaces/TODO. Doing
>>>>>>>>> a cursory reading of them suggests that these apply to *all*
>>>>>>>>> vc04_services components? Am I right?
>>>>>>>> Actually these applies just for the interfaces directory. Some of
>>>>>>>> them could apply to the services, but this is no priority.
>>>>>>> By no priority, you mean this doesn't affect the criteria required to
>>>>>>> ful-fill to get these out of staging?
>>>>>> Correct
>>>>>>
>>>>>>>>> Are these are the specific bits of cleanup you are referring to in
>>>>>>>>> your comment?
>>>>>>>> You mean about bcm2835-isp? There were too many changes to vchiq
>>>>>>>> that i don't remember them all. The first that come to my mind was
>>>>>>>> those fancy comment sections which is not kernel coding style. It
>>>>>>>> has been removed.
>>>>>>> No, I don't mean the bcm2835-isp changes (those are upcoming /
>>>>>>> out-of-tree still so...). I mean what are the specific bits / points
>>>>>>> that needs to be addressed to get vc04_services out of the staging.
>>>>>> These were the points which i mentioned in my last email. They came
>>>>>> from interface/TODO.
>>>>>>
>>>>>>> You have mentioned it above now, so I'll follow up on those.
>>>>>> That would be great :)
>>>>>>
>>>>>>> The many vchiq changes you referred to above comment (that you don't
>>>>>>> remember) are from [1] as well or some other series ?
>>>>>> Sorry, for the confusing. The many changes i refer were the dozens of
>>>>>> clean up patches for vc04_interfaces in mainline staging since the
>>>>>> last years. [1] was just a single patch which has been accepted yet.
>>>>> Ah I see. There are many others that I've to dig out then. Thanks for
>>>>> clarifying!
>>>>>
>>>>>>>> [1] -
>>>>>>>> https://lore.kernel.org/linux-staging/20220712181928.17547-1-jslebodn@redhat.com/
>>>>>>>>
>>>>>>>>>> Unfortuntately i hadn't much time to work on vchiq by myself.
>>>>>>>>>>
>>>>>>>>>> Just my two cents
>>>>>>>>>> Stefan
>>>> --
>>>> Regards,
>>>>
>>>> Laurent Pinchart
>>> _______________________________________________
>>> linux-arm-kernel mailing list
>>> linux-arm-kernel@lists.infradead.org
>>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel