linux-media.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Naushir Patuck <naush@raspberrypi.com>
To: Hans Verkuil <hverkuil@xs4all.nl>
Cc: "Laurent Pinchart" <laurent.pinchart@ideasonboard.com>,
	linux-media@vger.kernel.org,
	"Kieran Bingham" <kieran.bingham@ideasonboard.com>,
	"Jacopo Mondi" <jacopo@jmondi.org>,
	"Niklas Söderlund" <niklas.soderlund@ragnatech.se>,
	"Dave Stevenson" <dave.stevenson@raspberrypi.com>
Subject: Re: [PATCH v2 33/34] staging: bcm2835-isp: Add support for BC2835 ISP
Date: Tue, 19 May 2020 13:47:16 +0100	[thread overview]
Message-ID: <CAEmqJPo-ST8msiQVedLmH48vsMBz2WeaK6WXyN5fiP5z1b+wAA@mail.gmail.com> (raw)
In-Reply-To: <3de6ce28-1089-19c2-cdac-64796a46638f@xs4all.nl>

Hi Hans,

Thank you for the review and comments.  Just a follow up and
overlapping Dave's comments:

On Mon, 18 May 2020 at 13:02, Hans Verkuil <hverkuil@xs4all.nl> wrote:
>
> On 04/05/2020 11:26, Laurent Pinchart wrote:
> > From: Naushir Patuck <naush@raspberrypi.com>
> >
> > Port the V4L2 compatible driver for the ISP unit found on Broadcom BCM2835
> > chips.
> >
> > The driver interfaces though the VideoCore unit using the VCHIQ MMAL
> > interface.
> >
> > ISP driver upported from from RaspberryPi BSP at revision:
> > 6c3505be6c3e ("staging: vc04_services: isp: Make all references to bcm2835_isp_fmt const")
> >
> > Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
> > Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
> > [Adapt to staging by moving all modifications that in the BSP are scattered
> > in core components inside this directory]
> > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> > ---
> >  .../v4l/pixfmt-meta-bcm2835-isp-stats.rst     |   41 +
> >  drivers/staging/vc04_services/Kconfig         |    2 +
> >  drivers/staging/vc04_services/Makefile        |    1 +
> >  .../staging/vc04_services/bcm2835-isp/Kconfig |   14 +
> >  .../vc04_services/bcm2835-isp/Makefile        |   10 +
> >  .../bcm2835-isp/bcm2835-v4l2-isp.c            | 1632 +++++++++++++++++
> >  .../bcm2835-isp/bcm2835_isp_ctrls.h           |   67 +
> >  .../bcm2835-isp/bcm2835_isp_fmts.h            |  301 +++
> >  .../include/uapi/linux/bcm2835-isp.h          |  333 ++++
> >  .../staging/vc04_services/vchiq-mmal/Kconfig  |    3 +-
> >  .../vc04_services/vchiq-mmal/mmal-encodings.h |    4 +
> >  .../vchiq-mmal/mmal-parameters.h              |  153 +-
> >  12 files changed, 2559 insertions(+), 2 deletions(-)
> >  create mode 100644 drivers/staging/vc04_services/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-v4l2-isp.c
> >  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/include/uapi/linux/bcm2835-isp.h
> >
>
> <snip>
>
> > diff --git a/drivers/staging/vc04_services/bcm2835-isp/bcm2835-v4l2-isp.c b/drivers/staging/vc04_services/bcm2835-isp/bcm2835-v4l2-isp.c
> > new file mode 100644
> > index 000000000000..a32faab4b8dc
> > --- /dev/null
> > +++ b/drivers/staging/vc04_services/bcm2835-isp/bcm2835-v4l2-isp.c
> > @@ -0,0 +1,1632 @@
>
> <snip>
>
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Broadcom BCM2835 ISP driver
> > + *
> > + * Copyright © 2019-2020 Raspberry Pi (Trading) Ltd.
> > + *
> > + * Author: Naushir Patuck (naush@raspberrypi.com)
> > + *
> > + */
> > +
> > +#include <linux/module.h>
> > +#include <linux/platform_device.h>
> > +
> > +#include <media/v4l2-ctrls.h>
> > +#include <media/v4l2-device.h>
> > +#include <media/v4l2-event.h>
> > +#include <media/v4l2-ioctl.h>
> > +#include <media/videobuf2-dma-contig.h>
> > +
> > +#include "vchiq-mmal/mmal-msg.h"
> > +#include "vchiq-mmal/mmal-parameters.h"
> > +#include "vchiq-mmal/mmal-vchiq.h"
> > +
> > +#include "bcm2835_isp_ctrls.h"
> > +#include "bcm2835_isp_fmts.h"
> > +
> > +static unsigned int debug;
> > +module_param(debug, uint, 0644);
> > +MODULE_PARM_DESC(debug, "activates debug info");
> > +
> > +static unsigned int video_nr = 13;
> > +module_param(video_nr, uint, 0644);
> > +MODULE_PARM_DESC(video_nr, "base video device number");
> > +
> > +#define BCM2835_ISP_NAME "bcm2835-isp"
> > +#define BCM2835_ISP_ENTITY_NAME_LEN 32
> > +
> > +#define BCM2835_ISP_NUM_OUTPUTS 1
> > +#define BCM2835_ISP_NUM_CAPTURES 2
> > +#define BCM2835_ISP_NUM_METADATA 1
> > +
> > +#define BCM2835_ISP_NUM_NODES                                                \
> > +             (BCM2835_ISP_NUM_OUTPUTS + BCM2835_ISP_NUM_CAPTURES +   \
> > +              BCM2835_ISP_NUM_METADATA)
> > +
> > +/* Default frame dimension of 1280 pixels. */
> > +#define DEFAULT_DIM 1280U
> > +/*
> > + * Maximum frame dimension of 16384 pixels.  Even though the ISP runs in tiles,
> > + * have a sensible limit so that we do not create an excessive number of tiles
> > + * to process.
> > + */
> > +#define MAX_DIM 16384U
> > +/*
> > + * Minimum frame dimension of 64 pixels.  Anything lower, and the tiling
> > + * algorihtm may not be able to cope when applying filter context.
> > + */
> > +#define MIN_DIM 64U
> > +
> > +/* Per-queue, driver-specific private data */
> > +struct bcm2835_isp_q_data {
> > +     /*
> > +      * These parameters should be treated as gospel, with everything else
> > +      * being determined from them.
> > +      */
> > +     unsigned int bytesperline;
> > +     unsigned int width;
> > +     unsigned int height;
> > +     unsigned int sizeimage;
> > +     const struct bcm2835_isp_fmt *fmt;
> > +};
> > +
> > +/*
> > + * Structure to describe a single node /dev/video<N> which represents a single
> > + * input or output queue to the ISP device.
> > + */
> > +struct bcm2835_isp_node {
> > +     int vfl_dir;
> > +     unsigned int id;
> > +     const char *name;
> > +     struct video_device vfd;
> > +     struct media_pad pad;
> > +     struct media_intf_devnode *intf_devnode;
> > +     struct media_link *intf_link;
> > +     struct mutex lock; /* top level device node lock */
> > +     struct mutex queue_lock;
> > +
> > +     struct vb2_queue queue;
> > +     unsigned int sequence;
> > +
> > +     /* The list of formats supported on the node. */
> > +     struct bcm2835_isp_fmt_list supported_fmts;
> > +
> > +     struct bcm2835_isp_q_data q_data;
> > +
> > +     /* Parent device structure */
> > +     struct bcm2835_isp_dev *dev;
> > +
> > +     bool registered;
> > +     bool media_node_registered;
> > +     bool queue_init;
> > +};
> > +
> > +/*
> > + * Structure representing the entire ISP device, comprising several input and
> > + * output nodes /dev/video<N>.
> > + */
> > +struct bcm2835_isp_dev {
> > +     struct v4l2_device v4l2_dev;
> > +     struct device *dev;
> > +     struct v4l2_ctrl_handler ctrl_handler;
> > +     struct media_device mdev;
> > +     struct media_entity entity;
> > +     bool media_device_registered;
> > +     bool media_entity_registered;
> > +     struct vchiq_mmal_instance *mmal_instance;
> > +     struct vchiq_mmal_component *component;
> > +     struct completion frame_cmplt;
> > +
> > +     struct bcm2835_isp_node node[BCM2835_ISP_NUM_NODES];
> > +     struct media_pad pad[BCM2835_ISP_NUM_NODES];
> > +     atomic_t num_streaming;
> > +
> > +     /* Image pipeline controls. */
> > +     int r_gain;
> > +     int b_gain;
> > +};
> > +
> > +struct bcm2835_isp_buffer {
> > +     struct vb2_v4l2_buffer vb;
> > +     struct mmal_buffer mmal;
> > +};
> > +
> > +static
> > +inline struct bcm2835_isp_dev *node_get_dev(struct bcm2835_isp_node *node)
> > +{
> > +     return node->dev;
> > +}
> > +
> > +static inline bool node_is_output(struct bcm2835_isp_node *node)
> > +{
> > +     return node->queue.type == V4L2_BUF_TYPE_VIDEO_OUTPUT;
> > +}
> > +
> > +static inline bool node_is_capture(struct bcm2835_isp_node *node)
> > +{
> > +     return node->queue.type == V4L2_BUF_TYPE_VIDEO_CAPTURE;
> > +}
> > +
> > +static inline bool node_is_stats(struct bcm2835_isp_node *node)
> > +{
> > +     return node->queue.type == V4L2_BUF_TYPE_META_CAPTURE;
> > +}
> > +
> > +static inline enum v4l2_buf_type index_to_queue_type(int index)
> > +{
> > +     if (index < BCM2835_ISP_NUM_OUTPUTS)
> > +             return V4L2_BUF_TYPE_VIDEO_OUTPUT;
> > +     else if (index < BCM2835_ISP_NUM_OUTPUTS + BCM2835_ISP_NUM_CAPTURES)
> > +             return V4L2_BUF_TYPE_VIDEO_CAPTURE;
> > +     else
> > +             return V4L2_BUF_TYPE_META_CAPTURE;
> > +}
> > +
> > +static struct vchiq_mmal_port *get_port_data(struct bcm2835_isp_node *node)
> > +{
> > +     struct bcm2835_isp_dev *dev = node_get_dev(node);
> > +
> > +     if (!dev->component)
> > +             return NULL;
> > +
> > +     switch (node->queue.type) {
> > +     case V4L2_BUF_TYPE_VIDEO_OUTPUT:
> > +             return &dev->component->input[node->id];
> > +     case V4L2_BUF_TYPE_VIDEO_CAPTURE:
> > +     case V4L2_BUF_TYPE_META_CAPTURE:
> > +             return &dev->component->output[node->id];
> > +     default:
> > +             v4l2_err(&dev->v4l2_dev, "%s: Invalid queue type %u\n",
> > +                      __func__, node->queue.type);
> > +             break;
> > +     }
> > +     return NULL;
> > +}
> > +
> > +static int set_isp_param(struct bcm2835_isp_node *node, u32 parameter,
> > +                      void *value, u32 value_size)
> > +{
> > +     struct vchiq_mmal_port *port = get_port_data(node);
> > +     struct bcm2835_isp_dev *dev = node_get_dev(node);
> > +
> > +     return vchiq_mmal_port_parameter_set(dev->mmal_instance, port,
> > +                                          parameter, value, value_size);
> > +}
> > +
> > +static int set_wb_gains(struct bcm2835_isp_node *node)
> > +{
> > +     struct bcm2835_isp_dev *dev = node_get_dev(node);
> > +     struct mmal_parameter_awbgains gains = {
> > +             .r_gain = { dev->r_gain, 1000 },
> > +             .b_gain = { dev->b_gain, 1000 }
> > +     };
> > +
> > +     return set_isp_param(node, MMAL_PARAMETER_CUSTOM_AWB_GAINS,
> > +                          &gains, sizeof(gains));
> > +}
> > +
> > +static int set_digital_gain(struct bcm2835_isp_node *node, uint32_t gain)
> > +{
> > +     struct mmal_parameter_rational digital_gain = {
> > +             .num = gain,
> > +             .den = 1000
> > +     };
> > +
> > +     return set_isp_param(node, MMAL_PARAMETER_DIGITAL_GAIN,
> > +                          &digital_gain, sizeof(digital_gain));
> > +}
> > +
> > +static const struct bcm2835_isp_fmt *get_fmt(u32 mmal_fmt)
> > +{
> > +     unsigned int i;
> > +
> > +     for (i = 0; i < ARRAY_SIZE(supported_formats); i++) {
> > +             if (supported_formats[i].mmal_fmt == mmal_fmt)
> > +                     return &supported_formats[i];
> > +     }
> > +     return NULL;
> > +}
> > +
> > +static const
> > +struct bcm2835_isp_fmt *find_format_by_fourcc(unsigned int fourcc,
> > +                                           struct bcm2835_isp_node *node)
> > +{
> > +     struct bcm2835_isp_fmt_list *fmts = &node->supported_fmts;
> > +     const struct bcm2835_isp_fmt *fmt;
> > +     unsigned int i;
> > +
> > +     for (i = 0; i < fmts->num_entries; i++) {
> > +             fmt = fmts->list[i];
> > +             if (fmt->fourcc == fourcc)
> > +                     return fmt;
> > +     }
> > +
> > +     return NULL;
> > +}
> > +
> > +static const
> > +struct bcm2835_isp_fmt *find_format(struct v4l2_format *f,
> > +                                 struct bcm2835_isp_node *node)
> > +{
> > +     return find_format_by_fourcc(node_is_stats(node) ?
> > +                                  f->fmt.meta.dataformat :
> > +                                  f->fmt.pix.pixelformat,
> > +                                  node);
> > +}
> > +
> > +/* vb2_to_mmal_buffer() - converts vb2 buffer header to MMAL
> > + *
> > + * Copies all the required fields from a VB2 buffer to the MMAL buffer header,
> > + * ready for sending to the VPU.
> > + */
> > +static void vb2_to_mmal_buffer(struct mmal_buffer *buf,
> > +                            struct vb2_v4l2_buffer *vb2)
> > +{
> > +     u64 pts;
> > +
> > +     buf->mmal_flags = 0;
> > +     if (vb2->flags & V4L2_BUF_FLAG_KEYFRAME)
> > +             buf->mmal_flags |= MMAL_BUFFER_HEADER_FLAG_KEYFRAME;
> > +
> > +     /* Data must be framed correctly as one frame per buffer. */
> > +     buf->mmal_flags |= MMAL_BUFFER_HEADER_FLAG_FRAME_END;
> > +
> > +     buf->length = vb2->vb2_buf.planes[0].bytesused;
> > +     /*
> > +      * Minor ambiguity in the V4L2 spec as to whether passing in a 0 length
> > +      * buffer, or one with V4L2_BUF_FLAG_LAST set denotes end of stream.
> > +      * Handle either.
> > +      */
> > +     if (!buf->length || vb2->flags & V4L2_BUF_FLAG_LAST)
> > +             buf->mmal_flags |= MMAL_BUFFER_HEADER_FLAG_EOS;
> > +
> > +     /* vb2 timestamps in nsecs, mmal in usecs */
> > +     pts = vb2->vb2_buf.timestamp;
> > +     do_div(pts, 1000);
> > +     buf->pts = pts;
> > +     buf->dts = MMAL_TIME_UNKNOWN;
> > +}
> > +
> > +static void mmal_buffer_cb(struct vchiq_mmal_instance *instance,
> > +                        struct vchiq_mmal_port *port, int status,
> > +                        struct mmal_buffer *mmal_buf)
> > +{
> > +     struct bcm2835_isp_buffer *q_buf;
> > +     struct bcm2835_isp_node *node = port->cb_ctx;
> > +     struct bcm2835_isp_dev *dev = node_get_dev(node);
> > +     struct vb2_v4l2_buffer *vb2;
> > +
> > +     q_buf = container_of(mmal_buf, struct bcm2835_isp_buffer, mmal);
> > +     vb2 = &q_buf->vb;
> > +     v4l2_dbg(2, debug, &dev->v4l2_dev,
> > +              "%s: port:%s[%d], status:%d, buf:%p, dmabuf:%p, length:%lu, flags %u, pts %lld\n",
> > +              __func__, node_is_output(node) ? "input" : "output", node->id,
> > +              status, mmal_buf, mmal_buf->dma_buf, mmal_buf->length,
> > +              mmal_buf->mmal_flags, mmal_buf->pts);
> > +
> > +     if (mmal_buf->cmd)
> > +             v4l2_err(&dev->v4l2_dev,
> > +                      "%s: Unexpected event on output callback - %08x\n",
> > +                      __func__, mmal_buf->cmd);
> > +
> > +     if (status) {
> > +             /* error in transfer */
> > +             if (vb2) {
> > +                     /* there was a buffer with the error so return it */
> > +                     vb2_buffer_done(&vb2->vb2_buf, VB2_BUF_STATE_ERROR);
> > +             }
> > +             return;
> > +     }
> > +
> > +     /* vb2 timestamps in nsecs, mmal in usecs */
> > +     vb2->vb2_buf.timestamp = mmal_buf->pts * 1000;
> > +     vb2->sequence = node->sequence++;
> > +     vb2_set_plane_payload(&vb2->vb2_buf, 0, mmal_buf->length);
> > +     vb2_buffer_done(&vb2->vb2_buf, VB2_BUF_STATE_DONE);
> > +
> > +     if (!port->enabled)
> > +             complete(&dev->frame_cmplt);
> > +}
> > +
> > +static void setup_mmal_port_format(struct bcm2835_isp_node *node,
> > +                                struct vchiq_mmal_port *port)
> > +{
> > +     struct bcm2835_isp_q_data *q_data = &node->q_data;
> > +
> > +     port->format.encoding = q_data->fmt->mmal_fmt;
> > +     /* Raw image format - set width/height */
> > +     port->es.video.width = (q_data->bytesperline << 3) / q_data->fmt->depth;
> > +     port->es.video.height = q_data->height;
> > +     port->es.video.crop.width = q_data->width;
> > +     port->es.video.crop.height = q_data->height;
> > +     port->es.video.crop.x = 0;
> > +     port->es.video.crop.y = 0;
> > +};
> > +
> > +static int setup_mmal_port(struct bcm2835_isp_node *node)
> > +{
> > +     struct vchiq_mmal_port *port = get_port_data(node);
> > +     struct bcm2835_isp_dev *dev = node_get_dev(node);
> > +     unsigned int enable = 1;
> > +     int ret;
> > +
> > +     v4l2_dbg(2, debug, &dev->v4l2_dev, "%s: setup %s[%d]\n", __func__,
> > +              node->name, node->id);
> > +
> > +     vchiq_mmal_port_parameter_set(dev->mmal_instance, port,
> > +                                   MMAL_PARAMETER_ZERO_COPY, &enable,
> > +                                   sizeof(enable));
> > +     setup_mmal_port_format(node, port);
> > +     ret = vchiq_mmal_port_set_format(dev->mmal_instance, port);
> > +     if (ret < 0) {
> > +             v4l2_dbg(1, debug, &dev->v4l2_dev,
> > +                      "%s: vchiq_mmal_port_set_format failed\n",
> > +                      __func__);
> > +             return ret;
> > +     }
> > +
> > +     if (node->q_data.sizeimage < port->minimum_buffer.size) {
> > +             v4l2_err(&dev->v4l2_dev,
> > +                      "buffer size mismatch sizeimage %u < min size %u\n",
> > +                      node->q_data.sizeimage, port->minimum_buffer.size);
> > +             return -EINVAL;
> > +     }
> > +
> > +     return 0;
> > +}
> > +
> > +static int bcm2835_isp_mmal_buf_cleanup(struct mmal_buffer *mmal_buf)
> > +{
> > +     mmal_vchi_buffer_cleanup(mmal_buf);
> > +
> > +     if (mmal_buf->dma_buf) {
> > +             dma_buf_put(mmal_buf->dma_buf);
> > +             mmal_buf->dma_buf = NULL;
> > +     }
> > +
> > +     return 0;
> > +}
> > +
> > +static int bcm2835_isp_node_queue_setup(struct vb2_queue *q,
> > +                                     unsigned int *nbuffers,
> > +                                     unsigned int *nplanes,
> > +                                     unsigned int sizes[],
> > +                                     struct device *alloc_devs[])
> > +{
> > +     struct bcm2835_isp_node *node = vb2_get_drv_priv(q);
> > +     struct vchiq_mmal_port *port;
> > +     unsigned int size;
> > +
> > +     if (setup_mmal_port(node))
> > +             return -EINVAL;
> > +
> > +     size = node->q_data.sizeimage;
> > +     if (size == 0) {
> > +             v4l2_info(&node_get_dev(node)->v4l2_dev,
> > +                       "%s: Image size unset in queue_setup for node %s[%d]\n",
> > +                       __func__, node->name, node->id);
> > +             return -EINVAL;
> > +     }
> > +
> > +     if (*nplanes)
> > +             return sizes[0] < size ? -EINVAL : 0;
> > +
> > +     *nplanes = 1;
> > +     sizes[0] = size;
> > +
> > +     port = get_port_data(node);
> > +     port->current_buffer.size = size;
> > +
> > +     if (*nbuffers < port->minimum_buffer.num)
> > +             *nbuffers = port->minimum_buffer.num;
> > +
> > +     port->current_buffer.num = *nbuffers;
> > +
> > +     v4l2_dbg(2, debug, &node_get_dev(node)->v4l2_dev,
> > +              "%s: Image size %u, nbuffers %u for node %s[%d]\n",
> > +              __func__, sizes[0], *nbuffers, node->name, node->id);
> > +     return 0;
> > +}
> > +
> > +static int bcm2835_isp_buf_init(struct vb2_buffer *vb)
> > +{
> > +     struct bcm2835_isp_node *node = vb2_get_drv_priv(vb->vb2_queue);
> > +     struct bcm2835_isp_dev *dev = node_get_dev(node);
> > +     struct vb2_v4l2_buffer *vb2 = to_vb2_v4l2_buffer(vb);
> > +     struct bcm2835_isp_buffer *buf =
> > +             container_of(vb2, struct bcm2835_isp_buffer, vb);
> > +
> > +     v4l2_dbg(3, debug, &dev->v4l2_dev, "%s: vb %p\n", __func__, vb);
> > +
> > +     buf->mmal.buffer = vb2_plane_vaddr(&buf->vb.vb2_buf, 0);
> > +     buf->mmal.buffer_size = vb2_plane_size(&buf->vb.vb2_buf, 0);
> > +     mmal_vchi_buffer_init(dev->mmal_instance, &buf->mmal);
> > +     return 0;
> > +}
> > +
> > +static int bcm2835_isp_buf_prepare(struct vb2_buffer *vb)
> > +{
> > +     struct bcm2835_isp_node *node = vb2_get_drv_priv(vb->vb2_queue);
> > +     struct bcm2835_isp_dev *dev = node_get_dev(node);
> > +     struct vb2_v4l2_buffer *vb2 = to_vb2_v4l2_buffer(vb);
> > +     struct bcm2835_isp_buffer *buf =
> > +             container_of(vb2, struct bcm2835_isp_buffer, vb);
> > +     struct dma_buf *dma_buf;
> > +     int ret;
> > +
> > +     v4l2_dbg(3, debug, &dev->v4l2_dev, "%s: type: %d ptr %p\n",
> > +              __func__, vb->vb2_queue->type, vb);
> > +
> > +     if (V4L2_TYPE_IS_OUTPUT(vb->vb2_queue->type)) {
> > +             if (vb2->field == V4L2_FIELD_ANY)
> > +                     vb2->field = V4L2_FIELD_NONE;
> > +             if (vb2->field != V4L2_FIELD_NONE) {
> > +                     v4l2_err(&dev->v4l2_dev,
> > +                              "%s field isn't supported\n", __func__);
> > +                     return -EINVAL;
> > +             }
> > +     }
> > +
> > +     if (vb2_plane_size(vb, 0) < node->q_data.sizeimage) {
> > +             v4l2_err(&dev->v4l2_dev,
> > +                      "%s data will not fit into plane (%lu < %lu)\n",
> > +                      __func__, vb2_plane_size(vb, 0),
> > +                      (long)node->q_data.sizeimage);
> > +             return -EINVAL;
> > +     }
> > +
> > +     if (!V4L2_TYPE_IS_OUTPUT(vb->vb2_queue->type))
> > +             vb2_set_plane_payload(vb, 0, node->q_data.sizeimage);
> > +
> > +     switch (vb->memory) {
> > +     case VB2_MEMORY_DMABUF:
> > +             dma_buf = dma_buf_get(vb->planes[0].m.fd);
> > +
> > +             if (dma_buf != buf->mmal.dma_buf) {
> > +                     /*
> > +                      * dmabuf either hasn't already been mapped, or it has
> > +                      * changed.
> > +                      */
> > +                     if (buf->mmal.dma_buf) {
> > +                             v4l2_err(&dev->v4l2_dev,
> > +                                      "%s Buffer changed - why did the core not call cleanup?\n",
> > +                                      __func__);
> > +                             bcm2835_isp_mmal_buf_cleanup(&buf->mmal);
> > +                     }
> > +
> > +                     buf->mmal.dma_buf = dma_buf;
> > +             } else {
> > +                     /*
> > +                      * Already have a reference to the buffer, so release it
> > +                      * here.
> > +                      */
> > +                     dma_buf_put(dma_buf);
> > +             }
> > +             ret = 0;
> > +             break;
> > +     case VB2_MEMORY_MMAP:
> > +             /*
> > +              * We want to do this at init, but vb2_core_expbuf checks that
> > +              * the index < q->num_buffers, and q->num_buffers only gets
> > +              * updated once all the buffers are allocated.
> > +              */
> > +             if (!buf->mmal.dma_buf) {
> > +                     ret = vb2_core_expbuf_dmabuf(vb->vb2_queue,
> > +                                                  vb->vb2_queue->type,
> > +                                                  vb->index, 0, O_CLOEXEC,
> > +                                                  &buf->mmal.dma_buf);
> > +                     v4l2_dbg(3, debug, &dev->v4l2_dev,
> > +                              "%s: exporting ptr %p to dmabuf %p\n",
> > +                              __func__, vb, buf->mmal.dma_buf);
> > +                     if (ret)
> > +                             v4l2_err(&dev->v4l2_dev,
> > +                                      "%s: Failed to expbuf idx %d, ret %d\n",
> > +                                      __func__, vb->index, ret);
> > +             } else {
> > +                     ret = 0;
> > +             }
> > +             break;
> > +     default:
> > +             ret = -EINVAL;
> > +             break;
> > +     }
> > +
> > +     return ret;
> > +}
> > +
> > +static void bcm2835_isp_node_buffer_queue(struct vb2_buffer *buf)
> > +{
> > +     struct bcm2835_isp_node *node = vb2_get_drv_priv(buf->vb2_queue);
> > +     struct vb2_v4l2_buffer *vbuf =
> > +             container_of(buf, struct vb2_v4l2_buffer, vb2_buf);
> > +     struct bcm2835_isp_buffer *buffer =
> > +             container_of(vbuf, struct bcm2835_isp_buffer, vb);
> > +     struct bcm2835_isp_dev *dev = node_get_dev(node);
> > +
> > +     v4l2_dbg(3, debug, &dev->v4l2_dev, "%s: node %s[%d], buffer %p\n",
> > +              __func__, node->name, node->id, buffer);
> > +
> > +     vb2_to_mmal_buffer(&buffer->mmal, &buffer->vb);
> > +     v4l2_dbg(3, debug, &dev->v4l2_dev,
> > +              "%s: node %s[%d] - submitting  mmal dmabuf %p\n", __func__,
> > +              node->name, node->id, buffer->mmal.dma_buf);
> > +     vchiq_mmal_submit_buffer(dev->mmal_instance, get_port_data(node),
> > +                              &buffer->mmal);
> > +}
> > +
> > +static void bcm2835_isp_buffer_cleanup(struct vb2_buffer *vb)
> > +{
> > +     struct vb2_v4l2_buffer *vb2 = to_vb2_v4l2_buffer(vb);
> > +     struct bcm2835_isp_buffer *buffer =
> > +             container_of(vb2, struct bcm2835_isp_buffer, vb);
> > +
> > +     bcm2835_isp_mmal_buf_cleanup(&buffer->mmal);
> > +}
> > +
> > +static int bcm2835_isp_node_start_streaming(struct vb2_queue *q,
> > +                                         unsigned int count)
> > +{
> > +     struct bcm2835_isp_node *node = vb2_get_drv_priv(q);
> > +     struct bcm2835_isp_dev *dev = node_get_dev(node);
> > +     struct vchiq_mmal_port *port = get_port_data(node);
> > +     int ret;
> > +
> > +     v4l2_dbg(1, debug, &dev->v4l2_dev, "%s: node %s[%d] (count %u)\n",
> > +              __func__, node->name, node->id, count);
> > +
> > +     ret = vchiq_mmal_component_enable(dev->mmal_instance, dev->component);
> > +     if (ret) {
> > +             v4l2_err(&dev->v4l2_dev, "%s: Failed enabling component, ret %d\n",
> > +                      __func__, ret);
> > +             return -EIO;
> > +     }
> > +
> > +     node->sequence = 0;
> > +     port->cb_ctx = node;
> > +     ret = vchiq_mmal_port_enable(dev->mmal_instance, port,
> > +                                  mmal_buffer_cb);
> > +     if (!ret)
> > +             atomic_inc(&dev->num_streaming);
> > +     else
> > +             v4l2_err(&dev->v4l2_dev,
> > +                      "%s: Failed enabling port, ret %d\n", __func__, ret);
> > +
>
> It's not obvious from this code what happens with outstanding buffers if there
> is an error. They should be returned to vb2 with state VB2_BUF_STATE_QUEUED.
> Does that happen?
>
> > +     return ret;
> > +}
> > +
> > +static void bcm2835_isp_node_stop_streaming(struct vb2_queue *q)
> > +{
> > +     struct bcm2835_isp_node *node = vb2_get_drv_priv(q);
> > +     struct bcm2835_isp_dev *dev = node_get_dev(node);
> > +     struct vchiq_mmal_port *port = get_port_data(node);
> > +     unsigned int i;
> > +     int ret;
> > +
> > +     v4l2_dbg(1, debug, &dev->v4l2_dev, "%s: node %s[%d], mmal port %p\n",
> > +              __func__, node->name, node->id, port);
> > +
> > +     init_completion(&dev->frame_cmplt);
> > +
> > +     /* Disable MMAL port - this will flush buffers back */
> > +     ret = vchiq_mmal_port_disable(dev->mmal_instance, port);
> > +     if (ret)
> > +             v4l2_err(&dev->v4l2_dev,
> > +                      "%s: Failed disabling %s port, ret %d\n", __func__,
> > +                      node_is_output(node) ? "i/p" : "o/p",
> > +                      ret);
> > +
> > +     while (atomic_read(&port->buffers_with_vpu)) {
> > +             v4l2_dbg(1, debug, &dev->v4l2_dev,
> > +                      "%s: Waiting for buffers to be returned - %d outstanding\n",
> > +                      __func__, atomic_read(&port->buffers_with_vpu));
> > +             ret = wait_for_completion_timeout(&dev->frame_cmplt, HZ);
> > +             if (ret <= 0) {
> > +                     v4l2_err(&dev->v4l2_dev,
> > +                              "%s: Timeout waiting for buffers to be returned - %d outstanding\n",
> > +                              __func__,
> > +                              atomic_read(&port->buffers_with_vpu));
> > +                     break;
> > +             }
> > +     }
> > +
> > +     /* Release the VCSM handle here to release the associated dmabuf */
> > +     for (i = 0; i < q->num_buffers; i++) {
> > +             struct vb2_v4l2_buffer *vb2 = to_vb2_v4l2_buffer(q->bufs[i]);
> > +             struct bcm2835_isp_buffer *buf =
> > +                     container_of(vb2, struct bcm2835_isp_buffer, vb);
> > +             bcm2835_isp_mmal_buf_cleanup(&buf->mmal);
> > +     }
> > +
> > +     atomic_dec(&dev->num_streaming);
> > +     /* If all ports disabled, then disable the component */
> > +     if (atomic_read(&dev->num_streaming) == 0) {
> > +             ret = vchiq_mmal_component_disable(dev->mmal_instance,
> > +                                                dev->component);
> > +             if (ret) {
> > +                     v4l2_err(&dev->v4l2_dev,
> > +                              "%s: Failed disabling component, ret %d\n",
> > +                              __func__, ret);
> > +             }
> > +     }
> > +
> > +     /*
> > +      * Simply wait for any vb2 buffers to finish. We could take steps to
> > +      * make them complete more quickly if we care, or even return them
> > +      * ourselves.
> > +      */
> > +     vb2_wait_for_all_buffers(&node->queue);
> > +}
> > +
> > +static const struct vb2_ops bcm2835_isp_node_queue_ops = {
> > +     .queue_setup            = bcm2835_isp_node_queue_setup,
> > +     .buf_init               = bcm2835_isp_buf_init,
> > +     .buf_prepare            = bcm2835_isp_buf_prepare,
> > +     .buf_queue              = bcm2835_isp_node_buffer_queue,
> > +     .buf_cleanup            = bcm2835_isp_buffer_cleanup,
> > +     .start_streaming        = bcm2835_isp_node_start_streaming,
> > +     .stop_streaming         = bcm2835_isp_node_stop_streaming,
> > +};
> > +
> > +static const
> > +struct bcm2835_isp_fmt *get_default_format(struct bcm2835_isp_node *node)
> > +{
> > +     return node->supported_fmts.list[0];
> > +}
> > +
> > +static inline unsigned int get_bytesperline(int width,
> > +                                         const struct bcm2835_isp_fmt *fmt)
> > +{
> > +     return ALIGN((width * fmt->depth) >> 3, fmt->bytesperline_align);
> > +}
> > +
> > +static inline unsigned int get_sizeimage(int bpl, int width, int height,
> > +                                      const struct bcm2835_isp_fmt *fmt)
> > +{
> > +     return (bpl * height * fmt->size_multiplier_x2) >> 1;
> > +}
> > +
> > +static int bcm2835_isp_s_ctrl(struct v4l2_ctrl *ctrl)
> > +{
> > +     struct bcm2835_isp_dev *dev =
> > +           container_of(ctrl->handler, struct bcm2835_isp_dev, ctrl_handler);
> > +     struct bcm2835_isp_node *node = &dev->node[0];
> > +     int ret = 0;
> > +
> > +     /*
> > +      * The ISP firmware driver will ensure these settings are applied on
> > +      * a frame boundary, so we are safe to write them as they come in.
> > +      *
> > +      * Note that the bcm2835_isp_* param structures are identical to the
> > +      * mmal-parameters.h definitions.  This avoids the need for unnecessary
> > +      * field-by-field copying between structures.
> > +      */
> > +     switch (ctrl->id) {
> > +     case V4L2_CID_RED_BALANCE:
> > +             dev->r_gain = ctrl->val;
> > +             ret = set_wb_gains(node);
> > +             break;
> > +     case V4L2_CID_BLUE_BALANCE:
> > +             dev->b_gain = ctrl->val;
> > +             ret = set_wb_gains(node);
> > +             break;
> > +     case V4L2_CID_DIGITAL_GAIN:
> > +             ret = set_digital_gain(node, ctrl->val);
> > +             break;
> > +     case V4L2_CID_USER_BCM2835_ISP_CC_MATRIX:
> > +             ret = set_isp_param(node, MMAL_PARAMETER_CUSTOM_CCM,
> > +                                 ctrl->p_new.p_u8,
> > +                                 sizeof(struct bcm2835_isp_custom_ccm));
> > +             break;
> > +     case V4L2_CID_USER_BCM2835_ISP_LENS_SHADING:
> > +             ret = set_isp_param(node, MMAL_PARAMETER_LENS_SHADING_OVERRIDE,
> > +                                 ctrl->p_new.p_u8,
> > +                                 sizeof(struct bcm2835_isp_lens_shading));
> > +             break;
> > +     case V4L2_CID_USER_BCM2835_ISP_BLACK_LEVEL:
> > +             ret = set_isp_param(node, MMAL_PARAMETER_BLACK_LEVEL,
> > +                                 ctrl->p_new.p_u8,
> > +                                 sizeof(struct bcm2835_isp_black_level));
> > +             break;
> > +     case V4L2_CID_USER_BCM2835_ISP_GEQ:
> > +             ret = set_isp_param(node, MMAL_PARAMETER_GEQ,
> > +                                 ctrl->p_new.p_u8,
> > +                                 sizeof(struct bcm2835_isp_geq));
> > +             break;
> > +     case V4L2_CID_USER_BCM2835_ISP_GAMMA:
> > +             ret = set_isp_param(node, MMAL_PARAMETER_GAMMA,
> > +                                 ctrl->p_new.p_u8,
> > +                                 sizeof(struct bcm2835_isp_gamma));
> > +             break;
> > +     case V4L2_CID_USER_BCM2835_ISP_DENOISE:
> > +             ret = set_isp_param(node, MMAL_PARAMETER_DENOISE,
> > +                                 ctrl->p_new.p_u8,
> > +                                 sizeof(struct bcm2835_isp_denoise));
> > +             break;
> > +     case V4L2_CID_USER_BCM2835_ISP_SHARPEN:
> > +             ret = set_isp_param(node, MMAL_PARAMETER_SHARPEN,
> > +                                 ctrl->p_new.p_u8,
> > +                                 sizeof(struct bcm2835_isp_sharpen));
> > +             break;
> > +     case V4L2_CID_USER_BCM2835_ISP_DPC:
> > +             ret = set_isp_param(node, MMAL_PARAMETER_DPC,
> > +                                 ctrl->p_new.p_u8,
> > +                                 sizeof(struct bcm2835_isp_dpc));
> > +             break;
> > +     default:
> > +             v4l2_info(&dev->v4l2_dev, "Unrecognised control\n");
> > +             ret = -EINVAL;
> > +     }
> > +
> > +     if (ret) {
> > +             v4l2_err(&dev->v4l2_dev, "%s: Failed setting ctrl \"%s\" (%08x), err %d\n",
> > +                      __func__, ctrl->name, ctrl->id, ret);
> > +             ret = -EIO;
> > +     }
> > +
> > +     return ret;
> > +}
> > +
> > +static const struct v4l2_ctrl_ops bcm2835_isp_ctrl_ops = {
> > +     .s_ctrl = bcm2835_isp_s_ctrl,
> > +};
> > +
> > +static const struct v4l2_file_operations bcm2835_isp_fops = {
> > +     .owner          = THIS_MODULE,
> > +     .open           = v4l2_fh_open,
> > +     .release        = vb2_fop_release,
> > +     .poll           = vb2_fop_poll,
> > +     .unlocked_ioctl = video_ioctl2,
> > +     .mmap           = vb2_fop_mmap
> > +};
> > +
> > +static int populate_qdata_fmt(struct v4l2_format *f,
> > +                           struct bcm2835_isp_node *node)
> > +{
> > +     struct bcm2835_isp_dev *dev = node_get_dev(node);
> > +     struct bcm2835_isp_q_data *q_data = &node->q_data;
> > +     struct vchiq_mmal_port *port;
> > +     int ret;
> > +
> > +     if (!node_is_stats(node)) {
> > +             v4l2_dbg(1, debug, &dev->v4l2_dev,
> > +                      "%s: Setting pix format for type %d, wxh: %ux%u, fmt: %08x, size %u\n",
> > +                      __func__, f->type, f->fmt.pix.width, f->fmt.pix.height,
> > +                      f->fmt.pix.pixelformat, f->fmt.pix.sizeimage);
> > +
> > +             q_data->fmt = find_format(f, node);
> > +             q_data->width = f->fmt.pix.width;
> > +             q_data->height = f->fmt.pix.height;
> > +             q_data->height = f->fmt.pix.height;
> > +
> > +             /* All parameters should have been set correctly by try_fmt */
> > +             q_data->bytesperline = f->fmt.pix.bytesperline;
> > +             q_data->sizeimage = f->fmt.pix.sizeimage;
> > +     } else {
> > +             v4l2_dbg(1, debug, &dev->v4l2_dev,
> > +                      "%s: Setting meta format for fmt: %08x, size %u\n",
> > +                      __func__, f->fmt.meta.dataformat,
> > +                      f->fmt.meta.buffersize);
> > +
> > +             q_data->fmt = find_format(f, node);
> > +             q_data->width = 0;
> > +             q_data->height = 0;
> > +             q_data->bytesperline = 0;
> > +             q_data->sizeimage = f->fmt.meta.buffersize;
> > +     }
> > +
> > +     v4l2_dbg(1, debug, &dev->v4l2_dev,
> > +              "%s: Calculated bpl as %u, size %u\n", __func__,
> > +              q_data->bytesperline, q_data->sizeimage);
> > +
> > +     /* If we have a component then setup the port as well */
> > +     port = get_port_data(node);
> > +     setup_mmal_port_format(node, port);
> > +     ret = vchiq_mmal_port_set_format(dev->mmal_instance, port);
> > +     if (ret) {
> > +             v4l2_err(&dev->v4l2_dev,
> > +                      "%s: Failed vchiq_mmal_port_set_format on port, ret %d\n",
> > +                      __func__, ret);
> > +             ret = -EINVAL;
> > +     }
> > +
> > +     if (q_data->sizeimage < port->minimum_buffer.size) {
> > +             v4l2_err(&dev->v4l2_dev,
> > +                      "%s: Current buffer size of %u < min buf size %u - driver mismatch to MMAL\n",
> > +                      __func__,
> > +                      q_data->sizeimage,
> > +                      port->minimum_buffer.size);
> > +     }
> > +
> > +     v4l2_dbg(1, debug, &dev->v4l2_dev,
> > +              "%s: Set format for type %d, wxh: %dx%d, fmt: %08x, size %u\n",
> > +              __func__, f->type, q_data->width, q_data->height,
> > +              q_data->fmt->fourcc, q_data->sizeimage);
> > +
> > +     return ret;
> > +}
> > +
> > +static int bcm2835_isp_node_querycap(struct file *file, void *priv,
> > +                                  struct v4l2_capability *cap)
> > +{
> > +     strscpy(cap->driver, BCM2835_ISP_NAME, sizeof(cap->driver));
> > +     strscpy(cap->card, BCM2835_ISP_NAME, sizeof(cap->card));
> > +     snprintf(cap->bus_info, sizeof(cap->bus_info), "platform:%s",
> > +              BCM2835_ISP_NAME);
> > +
> > +     return 0;
> > +}
> > +
> > +static int bcm2835_isp_node_g_fmt(struct file *file, void *priv,
> > +                               struct v4l2_format *f)
> > +{
> > +     struct bcm2835_isp_node *node = video_drvdata(file);
> > +
> > +     if (f->type != node->queue.type)
> > +             return -EINVAL;
> > +
> > +     if (node_is_stats(node)) {
> > +             f->fmt.meta.dataformat = V4L2_META_FMT_BCM2835_ISP_STATS;
> > +             f->fmt.meta.buffersize =
> > +                     get_port_data(node)->minimum_buffer.size;
> > +     } else {
> > +             struct bcm2835_isp_q_data *q_data = &node->q_data;
> > +
> > +             f->fmt.pix.width = q_data->width;
> > +             f->fmt.pix.height = q_data->height;
> > +             f->fmt.pix.field = V4L2_FIELD_NONE;
> > +             f->fmt.pix.pixelformat = q_data->fmt->fourcc;
> > +             f->fmt.pix.bytesperline = q_data->bytesperline;
> > +             f->fmt.pix.sizeimage = q_data->sizeimage;
> > +             f->fmt.pix.colorspace = q_data->fmt->colorspace;
> > +     }
> > +
> > +     return 0;
> > +}
> > +
> > +static int bcm2835_isp_node_enum_fmt(struct file *file, void  *priv,
> > +                                  struct v4l2_fmtdesc *f)
> > +{
> > +     struct bcm2835_isp_node *node = video_drvdata(file);
> > +     struct bcm2835_isp_fmt_list *fmts = &node->supported_fmts;
> > +
> > +     if (f->type != node->queue.type)
> > +             return -EINVAL;
> > +
> > +     if (f->index < fmts->num_entries) {
> > +             /* Format found */
> > +             f->pixelformat = fmts->list[f->index]->fourcc;
> > +             f->flags = fmts->list[f->index]->flags;
> > +             return 0;
> > +     }
> > +
> > +     return -EINVAL;
> > +}
> > +
> > +static int bcm2835_isp_enum_framesizes(struct file *file, void *priv,
> > +                                    struct v4l2_frmsizeenum *fsize)
> > +{
> > +     struct bcm2835_isp_node *node = video_drvdata(file);
> > +     struct bcm2835_isp_dev *dev = node_get_dev(node);
> > +     const struct bcm2835_isp_fmt *fmt;
> > +
> > +     if (node_is_stats(node) || fsize->index)
> > +             return -EINVAL;
> > +
> > +     fmt = find_format_by_fourcc(fsize->pixel_format, node);
> > +     if (!fmt) {
> > +             v4l2_err(&dev->v4l2_dev, "Invalid pixel code: %x\n",
> > +                      fsize->pixel_format);
> > +             return -EINVAL;
> > +     }
> > +
> > +     fsize->type = V4L2_FRMSIZE_TYPE_STEPWISE;
> > +     fsize->stepwise.min_width = MIN_DIM;
> > +     fsize->stepwise.max_width = MAX_DIM;
> > +     fsize->stepwise.step_width = fmt->step_size;
> > +
> > +     fsize->stepwise.min_height = MIN_DIM;
> > +     fsize->stepwise.max_height = MAX_DIM;
> > +     fsize->stepwise.step_height = fmt->step_size;
> > +
> > +     return 0;
> > +}
> > +
> > +static int bcm2835_isp_node_try_fmt(struct file *file, void *priv,
> > +                                 struct v4l2_format *f)
> > +{
> > +     struct bcm2835_isp_node *node = video_drvdata(file);
> > +     const struct bcm2835_isp_fmt *fmt;
> > +
> > +     if (f->type != node->queue.type)
> > +             return -EINVAL;
> > +
> > +     fmt = find_format(f, node);
> > +     if (!fmt)
> > +             fmt = get_default_format(node);
> > +
> > +     if (!node_is_stats(node)) {
> > +             f->fmt.pix.width = max(min(f->fmt.pix.width, MAX_DIM),
> > +                                    MIN_DIM);
> > +             f->fmt.pix.height = max(min(f->fmt.pix.height, MAX_DIM),
> > +                                     MIN_DIM);
> > +
> > +             f->fmt.pix.pixelformat = fmt->fourcc;
> > +             f->fmt.pix.colorspace = fmt->colorspace;
> > +             f->fmt.pix.bytesperline = get_bytesperline(f->fmt.pix.width,
> > +                                                        fmt);
> > +             f->fmt.pix.field = V4L2_FIELD_NONE;
> > +             f->fmt.pix.sizeimage =
> > +                     get_sizeimage(f->fmt.pix.bytesperline, f->fmt.pix.width,
> > +                                   f->fmt.pix.height, fmt);
> > +     } else {
> > +             f->fmt.meta.dataformat = fmt->fourcc;
> > +             f->fmt.meta.buffersize =
> > +                             get_port_data(node)->minimum_buffer.size;
> > +     }
> > +
> > +     return 0;
> > +}
> > +
> > +static int bcm2835_isp_node_s_fmt(struct file *file, void *priv,
> > +                               struct v4l2_format *f)
> > +{
> > +     struct bcm2835_isp_node *node = video_drvdata(file);
> > +     int ret;
> > +
> > +     if (f->type != node->queue.type)
> > +             return -EINVAL;
> > +
> > +     ret = bcm2835_isp_node_try_fmt(file, priv, f);
> > +     if (ret)
> > +             return ret;
> > +
> > +     v4l2_dbg(1, debug, &node_get_dev(node)->v4l2_dev,
> > +              "%s: Set format for node %s[%d]\n",
> > +              __func__, node->name, node->id);
> > +
> > +     return populate_qdata_fmt(f, node);
> > +}
> > +
> > +static int bcm2835_isp_node_s_selection(struct file *file, void *fh,
> > +                                     struct v4l2_selection *s)
> > +{
> > +     struct mmal_parameter_crop crop;
> > +     struct bcm2835_isp_node *node = video_drvdata(file);
> > +     struct bcm2835_isp_dev *dev = node_get_dev(node);
> > +     struct vchiq_mmal_port *port = get_port_data(node);
> > +
> > +     /* This return value is required fro V4L2 compliance. */
> > +     if (node_is_stats(node))
> > +             return -ENOTTY;
> > +
> > +     if (!s->r.width || !s->r.height)
> > +             return -EINVAL;
>
> I'm missing a check for s->target.

Ack

>
> > +
> > +     /* Adjust the crop window if goes outside the frame dimensions. */
> > +     s->r.left = min((unsigned int)max(s->r.left, 0),
> > +                     node->q_data.width - MIN_DIM);
> > +     s->r.top = min((unsigned int)max(s->r.top, 0),
> > +                    node->q_data.height - MIN_DIM);
> > +     s->r.width = max(min(s->r.width, node->q_data.width - s->r.left),
> > +                      MIN_DIM);
> > +     s->r.height = max(min(s->r.height, node->q_data.height - s->r.top),
> > +                       MIN_DIM);
> > +
> > +     crop.rect.x = s->r.left;
> > +     crop.rect.y = s->r.top;
> > +     crop.rect.width = s->r.width;
> > +     crop.rect.height = s->r.height;
> > +
> > +     return vchiq_mmal_port_parameter_set(dev->mmal_instance, port,
> > +                                          MMAL_PARAMETER_CROP,
> > +                                          &crop, sizeof(crop));
> > +}
> > +
> > +static int bcm2835_isp_node_g_selection(struct file *file, void *fh,
> > +                                     struct v4l2_selection *s)
> > +{
> > +     struct bcm2835_isp_node *node = video_drvdata(file);
> > +     struct bcm2835_isp_dev *dev = node_get_dev(node);
> > +     struct vchiq_mmal_port *port = get_port_data(node);
> > +     struct mmal_parameter_crop crop;
> > +     u32 crop_size = sizeof(crop);
> > +     int ret;
> > +
> > +     /* This return value is required for V4L2 compliance. */
> > +     if (node_is_stats(node))
> > +             return -ENOTTY;
> > +
> > +     /* We can only return out an input crop. */
> > +     if (s->target != V4L2_SEL_TGT_CROP)
> > +             return -EINVAL;
>
> No support for _TGT_CROP_DEFAULT/BOUNDS? Those are usually supported
> by drivers and are typically set to the width/height of the current
> format.
>
> I recommend adding support for these targets.

Ack

>
> > +
> > +     ret = vchiq_mmal_port_parameter_get(dev->mmal_instance, port,
> > +                                         MMAL_PARAMETER_CROP,
> > +                                         &crop, &crop_size);
> > +     if (!ret)
> > +             return -EINVAL;
> > +
> > +     s->r.left = crop.rect.x;
> > +     s->r.top = crop.rect.y;
> > +     s->r.width = crop.rect.width;
> > +     s->r.height = crop.rect.height;
> > +
> > +     return 0;
> > +}
> > +
> > +static int bcm3285_isp_subscribe_event(struct v4l2_fh *fh,
> > +                                    const struct v4l2_event_subscription *s)
> > +{
> > +     switch (s->type) {
> > +     /* Cannot change source parameters dynamically at runtime. */
> > +     case V4L2_EVENT_SOURCE_CHANGE:
> > +             return -EINVAL;
> > +     case V4L2_EVENT_CTRL:
> > +             return v4l2_ctrl_subscribe_event(fh, s);
> > +     default:
> > +             return v4l2_event_subscribe(fh, s, 4, NULL);
> > +     }
> > +}
> > +
> > +static const struct v4l2_ioctl_ops bcm2835_isp_node_ioctl_ops = {
> > +     .vidioc_querycap                = bcm2835_isp_node_querycap,
> > +     .vidioc_g_fmt_vid_cap           = bcm2835_isp_node_g_fmt,
> > +     .vidioc_g_fmt_vid_out           = bcm2835_isp_node_g_fmt,
> > +     .vidioc_g_fmt_meta_cap          = bcm2835_isp_node_g_fmt,
> > +     .vidioc_s_fmt_vid_cap           = bcm2835_isp_node_s_fmt,
> > +     .vidioc_s_fmt_vid_out           = bcm2835_isp_node_s_fmt,
> > +     .vidioc_s_fmt_meta_cap          = bcm2835_isp_node_s_fmt,
> > +     .vidioc_try_fmt_vid_cap         = bcm2835_isp_node_try_fmt,
> > +     .vidioc_try_fmt_vid_out         = bcm2835_isp_node_try_fmt,
> > +     .vidioc_try_fmt_meta_cap        = bcm2835_isp_node_try_fmt,
> > +     .vidioc_s_selection             = bcm2835_isp_node_s_selection,
> > +     .vidioc_g_selection             = bcm2835_isp_node_g_selection,
> > +
> > +     .vidioc_enum_fmt_vid_cap        = bcm2835_isp_node_enum_fmt,
> > +     .vidioc_enum_fmt_vid_out        = bcm2835_isp_node_enum_fmt,
> > +     .vidioc_enum_fmt_meta_cap       = bcm2835_isp_node_enum_fmt,
> > +     .vidioc_enum_framesizes         = bcm2835_isp_enum_framesizes,
> > +
> > +     .vidioc_reqbufs                 = vb2_ioctl_reqbufs,
> > +     .vidioc_querybuf                = vb2_ioctl_querybuf,
> > +     .vidioc_qbuf                    = vb2_ioctl_qbuf,
> > +     .vidioc_dqbuf                   = vb2_ioctl_dqbuf,
> > +     .vidioc_expbuf                  = vb2_ioctl_expbuf,
> > +     .vidioc_create_bufs             = vb2_ioctl_create_bufs,
> > +     .vidioc_prepare_buf             = vb2_ioctl_prepare_buf,
> > +
> > +     .vidioc_streamon                = vb2_ioctl_streamon,
> > +     .vidioc_streamoff               = vb2_ioctl_streamoff,
> > +
> > +     .vidioc_subscribe_event         = bcm3285_isp_subscribe_event,
> > +     .vidioc_unsubscribe_event       = v4l2_event_unsubscribe,
> > +};
> > +
> > +/*
> > + * Size of the array to provide to the VPU when asking for the list of supported
> > + * formats.
> > + *
> > + * The ISP component currently advertises 33 input formats, so add a small
> > + * overhead on that.
> > + */
> > +#define MAX_SUPPORTED_ENCODINGS 40
> > +
> > +/* Populate node->supported_fmts with the formats supported by those ports. */
> > +static int bcm2835_isp_get_supported_fmts(struct bcm2835_isp_node *node)
> > +{
> > +     struct bcm2835_isp_dev *dev = node_get_dev(node);
> > +     struct bcm2835_isp_fmt const **list;
> > +     unsigned int i, j, num_encodings;
> > +     u32 fourccs[MAX_SUPPORTED_ENCODINGS];
> > +     u32 param_size = sizeof(fourccs);
> > +     int ret;
> > +
> > +     ret = vchiq_mmal_port_parameter_get(dev->mmal_instance,
> > +                                         get_port_data(node),
> > +                                         MMAL_PARAMETER_SUPPORTED_ENCODINGS,
> > +                                         &fourccs, &param_size);
> > +
> > +     if (ret) {
> > +             if (ret == MMAL_MSG_STATUS_ENOSPC) {
> > +                     v4l2_err(&dev->v4l2_dev,
> > +                              "%s: port has more encoding than we provided space for. Some are dropped.\n",
> > +                              __func__);
> > +                     num_encodings = MAX_SUPPORTED_ENCODINGS;
> > +             } else {
> > +                     v4l2_err(&dev->v4l2_dev, "%s: get_param ret %u.\n",
> > +                              __func__, ret);
> > +                     return -EINVAL;
> > +             }
> > +     } else {
> > +             num_encodings = param_size / sizeof(u32);
> > +     }
> > +
> > +     /*
> > +      * Assume at this stage that all encodings will be supported in V4L2.
> > +      * Any that aren't supported will waste a very small amount of memory.
> > +      */
> > +     list = devm_kzalloc(dev->dev,
> > +                         sizeof(struct bcm2835_isp_fmt *) * num_encodings,
> > +                         GFP_KERNEL);
> > +     if (!list)
> > +             return -ENOMEM;
> > +     node->supported_fmts.list = list;
> > +
> > +     for (i = 0, j = 0; i < num_encodings; i++) {
> > +             const struct bcm2835_isp_fmt *fmt = get_fmt(fourccs[i]);
> > +
> > +             if (fmt) {
> > +                     list[j] = fmt;
> > +                     j++;
> > +             }
> > +     }
> > +     node->supported_fmts.num_entries = j;
> > +
> > +     return 0;
> > +}
> > +
> > +/*
> > + * Register a device node /dev/video<N> to go along with one of the ISP's input
> > + * or output nodes.
> > + */
> > +static int register_node(struct bcm2835_isp_dev *dev,
> > +                      struct bcm2835_isp_node *node,
> > +                      int index)
> > +{
> > +     struct video_device *vfd;
> > +     struct vb2_queue *queue;
> > +     int ret;
> > +
> > +     mutex_init(&node->lock);
> > +
> > +     node->dev = dev;
> > +     vfd = &node->vfd;
> > +     queue = &node->queue;
> > +     queue->type = index_to_queue_type(index);
> > +     /*
> > +      * Setup the node type-specific params.
> > +      *
> > +      * Only the OUTPUT node can set controls and crop windows. However,
> > +      * we must allow the s/g_selection ioctl on the stats node as v4l2
> > +      * compliance expects it to return a -ENOTTY, and the framework
> > +      * does not handle it if the ioctl is disabled.
> > +      */
> > +     switch (queue->type) {
> > +     case V4L2_BUF_TYPE_VIDEO_OUTPUT:
> > +             vfd->device_caps = V4L2_CAP_VIDEO_OUTPUT | V4L2_CAP_STREAMING;
> > +             node->id = index;
> > +             node->vfl_dir = VFL_DIR_TX;
> > +             node->name = "output";
> > +             break;
> > +     case V4L2_BUF_TYPE_VIDEO_CAPTURE:
> > +             vfd->device_caps = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_STREAMING;
> > +             /* First Capture node starts at id 0, etc. */
> > +             node->id = index - BCM2835_ISP_NUM_OUTPUTS;
> > +             node->vfl_dir = VFL_DIR_RX;
> > +             node->name = "capture";
> > +             v4l2_disable_ioctl(&node->vfd, VIDIOC_S_CTRL);
> > +             v4l2_disable_ioctl(&node->vfd, VIDIOC_S_SELECTION);
> > +             v4l2_disable_ioctl(&node->vfd, VIDIOC_G_SELECTION);
> > +             break;
> > +     case V4L2_BUF_TYPE_META_CAPTURE:
> > +             vfd->device_caps = V4L2_CAP_META_CAPTURE | V4L2_CAP_STREAMING;
> > +             node->id = index - BCM2835_ISP_NUM_OUTPUTS;
> > +             node->vfl_dir = VFL_DIR_RX;
> > +             node->name = "stats";
> > +             v4l2_disable_ioctl(&node->vfd, VIDIOC_S_CTRL);
>
> Why not disable S/G_SELECTION for meta capture here rather than test for it
> in the op functions?

The impelmenation with S/G_SELECTION for meta capture disabled caused
problem with v4l2-compliance.  However, with the other changes
proposed in this review, it does seem to now work correcly.  So I will
update this code and disable the ioctl for the meta capture node.

>
> > +             break;
> > +     }
> > +
> > +     /* We use the selection API instead of the old crop API. */
> > +     v4l2_disable_ioctl(vfd, VIDIOC_CROPCAP);
> > +     v4l2_disable_ioctl(vfd, VIDIOC_G_CROP);
> > +     v4l2_disable_ioctl(vfd, VIDIOC_S_CROP);
>
> No need for this: the core handles this and will disable these ioctls
> automatically.

Without these explicitally disabled, we fail v4l2-compliance on the
video capture nodes with the following error:
Format ioctls:
    test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK
    test VIDIOC_G/S_PARM: OK (Not Supported)
    test VIDIOC_G_FBUF: OK (Not Supported)
    test VIDIOC_G_FMT: OK
    test VIDIOC_TRY_FMT: OK
    test VIDIOC_S_FMT: OK
    test VIDIOC_G_SLICED_VBI_CAP: OK (Not Supported)
    fail: v4l2-test-formats.cpp(1510): doioctl(node, VIDIOC_G_SELECTION, &sel)
    fail: v4l2-test-formats.cpp(1550): testLegacyCrop(node)
    test Cropping: FAIL
    test Composing: OK (Not Supported)
    test Scaling: OK (Not Supported)

>
> > +
> > +     ret = bcm2835_isp_get_supported_fmts(node);
> > +     if (ret)
> > +             return ret;
> > +
> > +     /* Initialise the the video node. */
> > +     vfd->vfl_type   = VFL_TYPE_VIDEO;
> > +     vfd->fops       = &bcm2835_isp_fops,
> > +     vfd->ioctl_ops  = &bcm2835_isp_node_ioctl_ops,
> > +     vfd->minor      = -1,
> > +     vfd->release    = video_device_release_empty,
> > +     vfd->queue      = &node->queue;
> > +     vfd->lock       = &node->lock;
> > +     vfd->v4l2_dev   = &dev->v4l2_dev;
> > +     vfd->vfl_dir    = node->vfl_dir;
> > +
> > +     node->q_data.fmt = get_default_format(node);
> > +     node->q_data.width = DEFAULT_DIM;
> > +     node->q_data.height = DEFAULT_DIM;
> > +     node->q_data.bytesperline =
> > +             get_bytesperline(DEFAULT_DIM, node->q_data.fmt);
> > +     node->q_data.sizeimage = node_is_stats(node) ?
> > +                              get_port_data(node)->recommended_buffer.size :
> > +                              get_sizeimage(node->q_data.bytesperline,
> > +                                            node->q_data.width,
> > +                                            node->q_data.height,
> > +                                            node->q_data.fmt);
> > +
> > +     queue->io_modes = VB2_MMAP | VB2_DMABUF;
> > +     queue->drv_priv = node;
> > +     queue->ops = &bcm2835_isp_node_queue_ops;
> > +     queue->mem_ops = &vb2_dma_contig_memops;
> > +     queue->buf_struct_size = sizeof(struct bcm2835_isp_buffer);
> > +     queue->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_COPY;
> > +     queue->dev = dev->dev;
> > +     queue->lock = &node->queue_lock;
> > +
> > +     ret = vb2_queue_init(queue);
> > +     if (ret < 0) {
> > +             v4l2_info(&dev->v4l2_dev, "vb2_queue_init failed\n");
> > +             return ret;
> > +     }
> > +     node->queue_init = true;
> > +
> > +     /* Define the device names */
> > +     snprintf(vfd->name, sizeof(node->vfd.name), "%s-%s%d", BCM2835_ISP_NAME,
> > +              node->name, node->id);
> > +
> > +     ret = video_register_device(vfd, VFL_TYPE_VIDEO, video_nr + index);
> > +     if (ret) {
> > +             v4l2_err(&dev->v4l2_dev,
> > +                      "Failed to register video %s[%d] device node\n",
> > +                      node->name, node->id);
> > +             return ret;
> > +     }
>
> Move registering the video device to the end of this function.
> Otherwise the output video device would be created (and available for
> userspace) before the controls are added.
>

Ack

> > +
> > +     node->registered = true;
> > +     video_set_drvdata(vfd, node);
> > +
> > +     /* Set some controls and defaults, but only on the VIDEO_OUTPUT node. */
> > +     if (node_is_output(node)) {
> > +             unsigned int i;
> > +
> > +             /* Use this ctrl template to assign all out ISP custom ctrls. */
> > +             struct v4l2_ctrl_config ctrl_template = {
> > +                     .ops            = &bcm2835_isp_ctrl_ops,
> > +                     .type           = V4L2_CTRL_TYPE_U8,
> > +                     .def            = 0,
> > +                     .min            = 0x00,
> > +                     .max            = 0xff,
> > +                     .step           = 1,
> > +             };
> > +
> > +             v4l2_ctrl_handler_init(&dev->ctrl_handler, 4);
> > +
> > +             dev->r_gain = 1000;
> > +             dev->b_gain = 1000;
> > +
> > +             v4l2_ctrl_new_std(&dev->ctrl_handler,  &bcm2835_isp_ctrl_ops,
> > +                               V4L2_CID_RED_BALANCE, 1, 0xffff, 1,
> > +                               dev->r_gain);
> > +
> > +             v4l2_ctrl_new_std(&dev->ctrl_handler, &bcm2835_isp_ctrl_ops,
> > +                               V4L2_CID_BLUE_BALANCE, 1, 0xffff, 1,
> > +                               dev->b_gain);
> > +
> > +             v4l2_ctrl_new_std(&dev->ctrl_handler, &bcm2835_isp_ctrl_ops,
> > +                               V4L2_CID_DIGITAL_GAIN, 1, 0xffff, 1, 1000);
> > +
> > +             for (i = 0; i < ARRAY_SIZE(custom_ctrls); i++) {
> > +                     ctrl_template.name = custom_ctrls[i].name;
> > +                     ctrl_template.id = custom_ctrls[i].id;
> > +                     ctrl_template.dims[0] = custom_ctrls[i].size;
> > +                     ctrl_template.flags = custom_ctrls[i].flags;
> > +                     v4l2_ctrl_new_custom(&dev->ctrl_handler,
> > +                                          &ctrl_template, NULL);
> > +             }
> > +
> > +             node->vfd.ctrl_handler = &dev->ctrl_handler;
>
> Missing error check.

Ack

>
> > +     }
> > +
> > +     v4l2_info(&dev->v4l2_dev,
> > +               "Device node %s[%d] registered as /dev/video%d\n",
> > +               node->name, node->id, vfd->num);
> > +
> > +     return 0;
> > +}
> > +
> > +/* Unregister one of the /dev/video<N> nodes associated with the ISP. */
> > +static void unregister_node(struct bcm2835_isp_node *node)
> > +{
> > +     struct bcm2835_isp_dev *dev = node_get_dev(node);
> > +
> > +     v4l2_info(&dev->v4l2_dev,
> > +               "Unregistering node %s[%d] device node /dev/video%d\n",
> > +               node->name, node->id, node->vfd.num);
> > +
> > +     if (node->queue_init)
> > +             vb2_queue_release(&node->queue);
> > +
> > +     if (node->registered) {
> > +             video_unregister_device(&node->vfd);
> > +             if (node_is_output(node))
> > +                     v4l2_ctrl_handler_free(&dev->ctrl_handler);
> > +     }
> > +
> > +     /*
> > +      * node->supported_fmts.list is free'd automatically
> > +      * as a managed resource.
> > +      */
> > +     node->supported_fmts.list = NULL;
> > +     node->supported_fmts.num_entries = 0;
> > +     node->vfd.ctrl_handler = NULL;
> > +     node->registered = false;
> > +     node->queue_init = false;
> > +}
> > +
> > +static void media_controller_unregister(struct bcm2835_isp_dev *dev)
> > +{
> > +     unsigned int i;
> > +
> > +     v4l2_info(&dev->v4l2_dev, "Unregister from media controller\n");
> > +
> > +     if (dev->media_device_registered) {
> > +             media_device_unregister(&dev->mdev);
> > +             media_device_cleanup(&dev->mdev);
> > +             dev->media_device_registered = false;
> > +     }
> > +
> > +     kfree(dev->entity.name);
> > +     dev->entity.name = NULL;
> > +
> > +     if (dev->media_entity_registered) {
> > +             media_device_unregister_entity(&dev->entity);
> > +             dev->media_entity_registered = false;
> > +     }
> > +
> > +     for (i = 0; i < BCM2835_ISP_NUM_NODES; i++) {
> > +             struct bcm2835_isp_node *node = &dev->node[i];
> > +
> > +             if (node->media_node_registered) {
> > +                     media_remove_intf_links(node->intf_link->intf);
> > +                     media_entity_remove_links(&dev->node[i].vfd.entity);
> > +                     media_devnode_remove(node->intf_devnode);
> > +                     media_device_unregister_entity(&node->vfd.entity);
> > +                     kfree(node->vfd.entity.name);
> > +             }
> > +             node->media_node_registered = false;
> > +     }
> > +
> > +     dev->v4l2_dev.mdev = NULL;
> > +}
> > +
> > +static int media_controller_register_node(struct bcm2835_isp_dev *dev, int num)
> > +{
> > +     struct bcm2835_isp_node *node = &dev->node[num];
> > +     struct media_entity *entity = &node->vfd.entity;
> > +     int output = node_is_output(node);
> > +     char *name;
> > +     int ret;
> > +
> > +     v4l2_info(&dev->v4l2_dev,
> > +               "Register %s node %d with media controller\n",
> > +               output ? "output" : "capture", num);
> > +     entity->obj_type = MEDIA_ENTITY_TYPE_VIDEO_DEVICE;
> > +     entity->function = MEDIA_ENT_F_IO_V4L;
> > +     entity->info.dev.major = VIDEO_MAJOR;
> > +     entity->info.dev.minor = node->vfd.minor;
> > +     name = kmalloc(BCM2835_ISP_ENTITY_NAME_LEN, GFP_KERNEL);
> > +     if (!name) {
> > +             ret = -ENOMEM;
> > +             goto error_no_mem;
> > +     }
> > +     snprintf(name, BCM2835_ISP_ENTITY_NAME_LEN, "%s0-%s%d",
> > +              BCM2835_ISP_NAME, output ? "output" : "capture", num);
> > +     entity->name = name;
> > +     node->pad.flags = output ? MEDIA_PAD_FL_SOURCE : MEDIA_PAD_FL_SINK;
> > +     ret = media_entity_pads_init(entity, 1, &node->pad);
> > +     if (ret)
> > +             goto error_pads_init;
> > +     ret = media_device_register_entity(&dev->mdev, entity);
> > +     if (ret)
> > +             goto error_register_entity;
> > +
> > +     node->intf_devnode = media_devnode_create(&dev->mdev,
> > +                                               MEDIA_INTF_T_V4L_VIDEO, 0,
> > +                                               VIDEO_MAJOR, node->vfd.minor);
> > +     if (!node->intf_devnode) {
> > +             ret = -ENOMEM;
> > +             goto error_devnode_create;
> > +     }
> > +
> > +     node->intf_link = media_create_intf_link(entity,
> > +                                              &node->intf_devnode->intf,
> > +                                              MEDIA_LNK_FL_IMMUTABLE |
> > +                                              MEDIA_LNK_FL_ENABLED);
> > +     if (!node->intf_link) {
> > +             ret = -ENOMEM;
> > +             goto error_create_intf_link;
> > +     }
> > +
> > +     if (output)
> > +             ret = media_create_pad_link(entity, 0, &dev->entity, num,
> > +                                         MEDIA_LNK_FL_IMMUTABLE |
> > +                                                 MEDIA_LNK_FL_ENABLED);
> > +     else
> > +             ret = media_create_pad_link(&dev->entity, num, entity, 0,
> > +                                         MEDIA_LNK_FL_IMMUTABLE |
> > +                                         MEDIA_LNK_FL_ENABLED);
> > +     if (ret)
> > +             goto error_create_pad_link;
> > +
> > +     dev->node[num].media_node_registered = true;
> > +     return 0;
> > +
> > +error_create_pad_link:
> > +     media_remove_intf_links(&node->intf_devnode->intf);
> > +error_create_intf_link:
> > +     media_devnode_remove(node->intf_devnode);
> > +error_devnode_create:
> > +     media_device_unregister_entity(&node->vfd.entity);
> > +error_register_entity:
> > +error_pads_init:
> > +     kfree(entity->name);
> > +     entity->name = NULL;
> > +error_no_mem:
> > +     if (ret)
> > +             v4l2_info(&dev->v4l2_dev, "Error registering node\n");
> > +
> > +     return ret;
> > +}
> > +
> > +static int media_controller_register(struct bcm2835_isp_dev *dev)
> > +{
> > +     char *name;
> > +     unsigned int i;
> > +     int ret;
> > +
> > +     v4l2_dbg(2, debug, &dev->v4l2_dev, "Registering with media controller\n");
> > +     dev->mdev.dev = dev->dev;
> > +     strscpy(dev->mdev.model, "bcm2835-isp",
> > +             sizeof(dev->mdev.model));
> > +     strscpy(dev->mdev.bus_info, "platform:bcm2835-isp",
> > +             sizeof(dev->mdev.bus_info));
> > +     media_device_init(&dev->mdev);
> > +     dev->v4l2_dev.mdev = &dev->mdev;
> > +
> > +     v4l2_dbg(2, debug, &dev->v4l2_dev, "Register entity for nodes\n");
> > +
> > +     name = kmalloc(BCM2835_ISP_ENTITY_NAME_LEN, GFP_KERNEL);
> > +     if (!name) {
> > +             ret = -ENOMEM;
> > +             goto done;
> > +     }
> > +     snprintf(name, BCM2835_ISP_ENTITY_NAME_LEN, "bcm2835_isp0");
> > +     dev->entity.name = name;
> > +     dev->entity.obj_type = MEDIA_ENTITY_TYPE_BASE;
> > +     dev->entity.function = MEDIA_ENT_F_PROC_VIDEO_SCALER;
> > +
> > +     for (i = 0; i < BCM2835_ISP_NUM_NODES; i++) {
> > +             dev->pad[i].flags = node_is_output(&dev->node[i]) ?
> > +                                     MEDIA_PAD_FL_SINK : MEDIA_PAD_FL_SOURCE;
> > +     }
> > +
> > +     ret = media_entity_pads_init(&dev->entity, BCM2835_ISP_NUM_NODES,
> > +                                  dev->pad);
> > +     if (ret)
> > +             goto done;
> > +
> > +     ret = media_device_register_entity(&dev->mdev, &dev->entity);
> > +     if (ret)
> > +             goto done;
> > +
> > +     dev->media_entity_registered = true;
> > +     for (i = 0; i < BCM2835_ISP_NUM_NODES; i++) {
> > +             ret = media_controller_register_node(dev, i);
> > +             if (ret)
> > +                     goto done;
> > +     }
> > +
> > +     ret = media_device_register(&dev->mdev);
> > +     if (!ret)
> > +             dev->media_device_registered = true;
> > +done:
> > +     return ret;
> > +}
> > +
> > +static int bcm2835_isp_remove(struct platform_device *pdev)
> > +{
> > +     struct bcm2835_isp_dev *dev = platform_get_drvdata(pdev);
> > +     unsigned int i;
> > +
> > +     media_controller_unregister(dev);
> > +
> > +     for (i = 0; i < BCM2835_ISP_NUM_NODES; i++)
> > +             unregister_node(&dev->node[i]);
> > +
> > +     v4l2_device_unregister(&dev->v4l2_dev);
> > +
> > +     if (dev->component)
> > +             vchiq_mmal_component_finalise(dev->mmal_instance,
> > +                                           dev->component);
> > +
> > +     vchiq_mmal_finalise(dev->mmal_instance);
> > +
> > +     return 0;
> > +}
> > +
> > +static int bcm2835_isp_probe(struct platform_device *pdev)
> > +{
> > +     struct bcm2835_isp_dev *dev;
> > +     unsigned int i;
> > +     int ret;
> > +
> > +     dev = devm_kzalloc(&pdev->dev, sizeof(*dev), GFP_KERNEL);
> > +     if (!dev)
> > +             return -ENOMEM;
> > +
> > +     dev->dev = &pdev->dev;
> > +
> > +     ret = v4l2_device_register(&pdev->dev, &dev->v4l2_dev);
> > +     if (ret)
> > +             return ret;
> > +
> > +     ret = vchiq_mmal_init(&dev->mmal_instance);
> > +     if (ret) {
> > +             v4l2_device_unregister(&dev->v4l2_dev);
> > +             return ret;
> > +     }
> > +
> > +     ret = vchiq_mmal_component_init(dev->mmal_instance, "ril.isp",
> > +                                     &dev->component);
> > +     if (ret) {
> > +             v4l2_err(&dev->v4l2_dev,
> > +                      "%s: failed to create ril.isp component\n", __func__);
> > +             goto error;
> > +     }
> > +
> > +     if ((dev->component->inputs != BCM2835_ISP_NUM_OUTPUTS) ||
> > +         (dev->component->outputs != BCM2835_ISP_NUM_CAPTURES +
> > +                                     BCM2835_ISP_NUM_METADATA)) {
> > +             v4l2_err(&dev->v4l2_dev,
> > +                      "%s: ril.isp returned %d i/p (%d expected), %d o/p (%d expected) ports\n",
> > +                       __func__, dev->component->inputs,
> > +                       BCM2835_ISP_NUM_OUTPUTS,
> > +                       dev->component->outputs,
> > +                       BCM2835_ISP_NUM_CAPTURES + BCM2835_ISP_NUM_METADATA);
> > +             goto error;
> > +     }
> > +
> > +     atomic_set(&dev->num_streaming, 0);
> > +
> > +     for (i = 0; i < BCM2835_ISP_NUM_NODES; i++) {
> > +             struct bcm2835_isp_node *node = &dev->node[i];
> > +
> > +             ret = register_node(dev, node, i);
> > +             if (ret)
> > +                     goto error;
> > +     }
> > +
> > +     ret = media_controller_register(dev);
> > +     if (ret)
> > +             goto error;
> > +
> > +     platform_set_drvdata(pdev, dev);
> > +     v4l2_info(&dev->v4l2_dev, "Loaded V4L2 %s\n", BCM2835_ISP_NAME);
> > +     return 0;
> > +
> > +error:
> > +     bcm2835_isp_remove(pdev);
> > +
> > +     return ret;
> > +}
> > +
> > +static struct platform_driver bcm2835_isp_pdrv = {
> > +     .probe = bcm2835_isp_probe,
> > +     .remove = bcm2835_isp_remove,
> > +     .driver = {
> > +                     .name = BCM2835_ISP_NAME,
> > +               },
> > +};
> > +
> > +module_platform_driver(bcm2835_isp_pdrv);
> > +
> > +MODULE_DESCRIPTION("BCM2835 ISP driver");
> > +MODULE_AUTHOR("Naushir Patuck <naush@raspberrypi.com>");
> > +MODULE_LICENSE("GPL");
> > +MODULE_VERSION("1.0");
> > +MODULE_ALIAS("platform:bcm2835-isp");
> > diff --git a/drivers/staging/vc04_services/bcm2835-isp/bcm2835_isp_ctrls.h b/drivers/staging/vc04_services/bcm2835-isp/bcm2835_isp_ctrls.h
> > new file mode 100644
> > index 000000000000..cfbb1063aad1
> > --- /dev/null
> > +++ b/drivers/staging/vc04_services/bcm2835-isp/bcm2835_isp_ctrls.h
> > @@ -0,0 +1,67 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +/*
> > + * Broadcom BCM2835 ISP driver
> > + *
> > + * Copyright © 2019-2020 Raspberry Pi (Trading) Ltd.
> > + *
> > + * Author: Naushir Patuck (naush@raspberrypi.com)
> > + *
> > + */
> > +
> > +#ifndef BCM2835_ISP_CTRLS
> > +#define BCM2835_ISP_CTRLS
> > +
> > +#include <linux/bcm2835-isp.h>
> > +
> > +struct bcm2835_isp_custom_ctrl {
> > +     const char *name;
> > +     u32 id;
> > +     u32 size;
> > +     u32 flags;
> > +};
> > +
> > +static const struct bcm2835_isp_custom_ctrl custom_ctrls[] = {
> > +     {
> > +             .name   = "Colour Correction Matrix",
> > +             .id     = V4L2_CID_USER_BCM2835_ISP_CC_MATRIX,
> > +             .size   = sizeof(struct bcm2835_isp_custom_ccm),
> > +             .flags  = 0
> > +     }, {
> > +             .name   = "Lens Shading",
> > +             .id     = V4L2_CID_USER_BCM2835_ISP_LENS_SHADING,
> > +             .size   = sizeof(struct bcm2835_isp_lens_shading),
> > +             .flags  = V4L2_CTRL_FLAG_EXECUTE_ON_WRITE
> > +     }, {
> > +             .name   = "Black Level",
> > +             .id     = V4L2_CID_USER_BCM2835_ISP_BLACK_LEVEL,
> > +             .size   = sizeof(struct bcm2835_isp_black_level),
> > +             .flags  = 0
> > +     }, {
> > +             .name   = "Green Equalisation",
> > +             .id     = V4L2_CID_USER_BCM2835_ISP_GEQ,
> > +             .size   = sizeof(struct bcm2835_isp_geq),
> > +             .flags  = 0
> > +     }, {
> > +             .name   = "Gamma",
> > +             .id     = V4L2_CID_USER_BCM2835_ISP_GAMMA,
> > +             .size   = sizeof(struct bcm2835_isp_gamma),
> > +             .flags  = 0
> > +     }, {
> > +             .name   = "Sharpen",
> > +             .id     = V4L2_CID_USER_BCM2835_ISP_SHARPEN,
> > +             .size   = sizeof(struct bcm2835_isp_sharpen),
> > +             .flags  = 0
> > +     }, {
> > +             .name   = "Denoise",
> > +             .id     = V4L2_CID_USER_BCM2835_ISP_DENOISE,
> > +             .size   = sizeof(struct bcm2835_isp_denoise),
> > +             .flags  = 0
> > +     }, {
> > +             .name   = "Defective Pixel Correction",
> > +             .id     = V4L2_CID_USER_BCM2835_ISP_DPC,
> > +             .size   = sizeof(struct bcm2835_isp_dpc),
> > +             .flags  = 0
> > +     }
> > +};
> > +
> > +#endif
> > diff --git a/drivers/staging/vc04_services/bcm2835-isp/bcm2835_isp_fmts.h b/drivers/staging/vc04_services/bcm2835-isp/bcm2835_isp_fmts.h
> > new file mode 100644
> > index 000000000000..af3bde152bb2
> > --- /dev/null
> > +++ b/drivers/staging/vc04_services/bcm2835-isp/bcm2835_isp_fmts.h
> > @@ -0,0 +1,301 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +/*
> > + * Broadcom BCM2835 ISP driver
> > + *
> > + * Copyright © 2019-2020 Raspberry Pi (Trading) Ltd.
> > + *
> > + * Author: Naushir Patuck (naush@raspberrypi.com)
> > + *
> > + */
> > +
> > +#ifndef BCM2835_ISP_FMTS
> > +#define BCM2835_ISP_FMTS
> > +
> > +#include <linux/videodev2.h>
> > +#include "vchiq-mmal/mmal-encodings.h"
> > +
> > +struct bcm2835_isp_fmt {
> > +     u32 fourcc;
> > +     int depth;
> > +     int bytesperline_align;
> > +     u32 flags;
> > +     u32 mmal_fmt;
> > +     int size_multiplier_x2;
> > +     enum v4l2_colorspace colorspace;
> > +     unsigned int step_size;
> > +};
> > +
> > +struct bcm2835_isp_fmt_list {
> > +     struct bcm2835_isp_fmt const **list;
> > +     unsigned int num_entries;
> > +};
> > +
> > +static const struct bcm2835_isp_fmt supported_formats[] = {
> > +     {
> > +             /* YUV formats */
> > +             .fourcc             = V4L2_PIX_FMT_YUV420,
> > +             .depth              = 8,
> > +             .bytesperline_align = 32,
> > +             .flags              = 0,
> > +             .mmal_fmt           = MMAL_ENCODING_I420,
> > +             .size_multiplier_x2 = 3,
> > +             .colorspace         = V4L2_COLORSPACE_SMPTE170M,
> > +             .step_size          = 2,
> > +     }, {
> > +             .fourcc             = V4L2_PIX_FMT_YVU420,
> > +             .depth              = 8,
> > +             .bytesperline_align = 32,
> > +             .flags              = 0,
> > +             .mmal_fmt           = MMAL_ENCODING_YV12,
> > +             .size_multiplier_x2 = 3,
> > +             .colorspace         = V4L2_COLORSPACE_SMPTE170M,
> > +             .step_size          = 2,
> > +     }, {
> > +             .fourcc             = V4L2_PIX_FMT_NV12,
> > +             .depth              = 8,
> > +             .bytesperline_align = 32,
> > +             .flags              = 0,
> > +             .mmal_fmt           = MMAL_ENCODING_NV12,
> > +             .size_multiplier_x2 = 3,
> > +             .colorspace         = V4L2_COLORSPACE_SMPTE170M,
> > +             .step_size          = 2,
> > +     }, {
> > +             .fourcc             = V4L2_PIX_FMT_NV21,
> > +             .depth              = 8,
> > +             .bytesperline_align = 32,
> > +             .flags              = 0,
> > +             .mmal_fmt           = MMAL_ENCODING_NV21,
> > +             .size_multiplier_x2 = 3,
> > +             .colorspace         = V4L2_COLORSPACE_SMPTE170M,
> > +             .step_size          = 2,
> > +     }, {
> > +             .fourcc             = V4L2_PIX_FMT_YUYV,
> > +             .depth              = 16,
> > +             .bytesperline_align = 32,
> > +             .flags              = 0,
> > +             .mmal_fmt           = MMAL_ENCODING_YUYV,
> > +             .size_multiplier_x2 = 2,
> > +             .colorspace         = V4L2_COLORSPACE_SMPTE170M,
> > +             .step_size          = 2,
> > +     }, {
> > +             .fourcc             = V4L2_PIX_FMT_UYVY,
> > +             .depth              = 16,
> > +             .bytesperline_align = 32,
> > +             .flags              = 0,
> > +             .mmal_fmt           = MMAL_ENCODING_UYVY,
> > +             .size_multiplier_x2 = 2,
> > +             .colorspace         = V4L2_COLORSPACE_SMPTE170M,
> > +             .step_size          = 2,
> > +     }, {
> > +             .fourcc             = V4L2_PIX_FMT_YVYU,
> > +             .depth              = 16,
> > +             .bytesperline_align = 32,
> > +             .flags              = 0,
> > +             .mmal_fmt           = MMAL_ENCODING_YVYU,
> > +             .size_multiplier_x2 = 2,
> > +             .colorspace         = V4L2_COLORSPACE_SMPTE170M,
> > +             .step_size          = 2,
> > +     }, {
> > +             .fourcc             = V4L2_PIX_FMT_VYUY,
> > +             .depth              = 16,
> > +             .bytesperline_align = 32,
> > +             .flags              = 0,
> > +             .mmal_fmt           = MMAL_ENCODING_VYUY,
> > +             .size_multiplier_x2 = 2,
> > +             .colorspace         = V4L2_COLORSPACE_SMPTE170M,
> > +             .step_size          = 2,
> > +     }, {
> > +             /* RGB formats */
> > +             .fourcc             = V4L2_PIX_FMT_RGB24,
> > +             .depth              = 24,
> > +             .bytesperline_align = 32,
> > +             .flags              = 0,
> > +             .mmal_fmt           = MMAL_ENCODING_RGB24,
> > +             .size_multiplier_x2 = 2,
> > +             .colorspace         = V4L2_COLORSPACE_SRGB,
> > +             .step_size          = 1,
> > +     }, {
> > +             .fourcc             = V4L2_PIX_FMT_RGB565,
> > +             .depth              = 16,
> > +             .bytesperline_align = 32,
> > +             .flags              = 0,
> > +             .mmal_fmt           = MMAL_ENCODING_RGB16,
> > +             .size_multiplier_x2 = 2,
> > +             .colorspace         = V4L2_COLORSPACE_SRGB,
> > +             .step_size          = 1,
> > +     }, {
> > +             .fourcc             = V4L2_PIX_FMT_BGR24,
> > +             .depth              = 24,
> > +             .bytesperline_align = 32,
> > +             .flags              = 0,
> > +             .mmal_fmt           = MMAL_ENCODING_BGR24,
> > +             .size_multiplier_x2 = 2,
> > +             .colorspace         = V4L2_COLORSPACE_SRGB,
> > +             .step_size          = 1,
> > +     }, {
> > +             .fourcc             = V4L2_PIX_FMT_ABGR32,
> > +             .depth              = 32,
> > +             .bytesperline_align = 32,
> > +             .flags              = 0,
> > +             .mmal_fmt           = MMAL_ENCODING_BGRA,
> > +             .size_multiplier_x2 = 2,
> > +             .colorspace         = V4L2_COLORSPACE_SRGB,
> > +             .step_size          = 1,
> > +     }, {
> > +             /* Bayer formats */
> > +             /* 8 bit */
> > +             .fourcc             = V4L2_PIX_FMT_SRGGB8,
> > +             .depth              = 8,
> > +             .bytesperline_align = 32,
> > +             .flags              = 0,
> > +             .mmal_fmt           = MMAL_ENCODING_BAYER_SRGGB8,
> > +             .size_multiplier_x2 = 2,
> > +             .colorspace         = V4L2_COLORSPACE_RAW,
> > +             .step_size          = 2,
> > +     }, {
> > +             .fourcc             = V4L2_PIX_FMT_SBGGR8,
> > +             .depth              = 8,
> > +             .bytesperline_align = 32,
> > +             .flags              = 0,
> > +             .mmal_fmt           = MMAL_ENCODING_BAYER_SBGGR8,
> > +             .size_multiplier_x2 = 2,
> > +             .colorspace         = V4L2_COLORSPACE_RAW,
> > +             .step_size          = 2,
> > +     }, {
> > +             .fourcc             = V4L2_PIX_FMT_SGRBG8,
> > +             .depth              = 8,
> > +             .bytesperline_align = 32,
> > +             .flags              = 0,
> > +             .mmal_fmt           = MMAL_ENCODING_BAYER_SGRBG8,
> > +             .size_multiplier_x2 = 2,
> > +             .colorspace         = V4L2_COLORSPACE_RAW,
> > +             .step_size          = 2,
> > +     }, {
> > +             .fourcc             = V4L2_PIX_FMT_SGBRG8,
> > +             .depth              = 8,
> > +             .bytesperline_align = 32,
> > +             .flags              = 0,
> > +             .mmal_fmt           = MMAL_ENCODING_BAYER_SGBRG8,
> > +             .size_multiplier_x2 = 2,
> > +             .colorspace         = V4L2_COLORSPACE_RAW,
> > +             .step_size          = 2,
> > +     }, {
> > +             /* 10 bit */
> > +             .fourcc             = V4L2_PIX_FMT_SRGGB10P,
> > +             .depth              = 10,
> > +             .bytesperline_align = 32,
> > +             .flags              = 0,
> > +             .mmal_fmt           = MMAL_ENCODING_BAYER_SRGGB10P,
> > +             .size_multiplier_x2 = 2,
> > +             .colorspace         = V4L2_COLORSPACE_RAW,
> > +             .step_size          = 2,
> > +     }, {
> > +             .fourcc             = V4L2_PIX_FMT_SBGGR10P,
> > +             .depth              = 10,
> > +             .bytesperline_align = 32,
> > +             .flags              = 0,
> > +             .mmal_fmt           = MMAL_ENCODING_BAYER_SBGGR10P,
> > +             .size_multiplier_x2 = 2,
> > +             .colorspace         = V4L2_COLORSPACE_RAW,
> > +             .step_size          = 2,
> > +     }, {
> > +             .fourcc             = V4L2_PIX_FMT_SGRBG10P,
> > +             .depth              = 10,
> > +             .bytesperline_align = 32,
> > +             .flags              = 0,
> > +             .mmal_fmt           = MMAL_ENCODING_BAYER_SGRBG10P,
> > +             .size_multiplier_x2 = 2,
> > +             .colorspace         = V4L2_COLORSPACE_RAW,
> > +             .step_size          = 2,
> > +     }, {
> > +             .fourcc             = V4L2_PIX_FMT_SGBRG10P,
> > +             .depth              = 10,
> > +             .bytesperline_align = 32,
> > +             .flags              = 0,
> > +             .mmal_fmt           = MMAL_ENCODING_BAYER_SGBRG10P,
> > +             .size_multiplier_x2 = 2,
> > +             .colorspace         = V4L2_COLORSPACE_RAW,
> > +             .step_size          = 2,
> > +     }, {
> > +             /* 12 bit */
> > +             .fourcc             = V4L2_PIX_FMT_SRGGB12P,
> > +             .depth              = 12,
> > +             .bytesperline_align = 32,
> > +             .flags              = 0,
> > +             .mmal_fmt           = MMAL_ENCODING_BAYER_SRGGB12P,
> > +             .size_multiplier_x2 = 2,
> > +             .colorspace         = V4L2_COLORSPACE_RAW,
> > +             .step_size          = 2,
> > +     }, {
> > +             .fourcc             = V4L2_PIX_FMT_SBGGR12P,
> > +             .depth              = 12,
> > +             .bytesperline_align = 32,
> > +             .flags              = 0,
> > +             .mmal_fmt           = MMAL_ENCODING_BAYER_SBGGR12P,
> > +             .size_multiplier_x2 = 2,
> > +             .colorspace         = V4L2_COLORSPACE_RAW,
> > +             .step_size          = 2,
> > +     }, {
> > +             .fourcc             = V4L2_PIX_FMT_SGRBG12P,
> > +             .depth              = 12,
> > +             .bytesperline_align = 32,
> > +             .flags              = 0,
> > +             .mmal_fmt           = MMAL_ENCODING_BAYER_SGRBG12P,
> > +             .size_multiplier_x2 = 2,
> > +             .colorspace         = V4L2_COLORSPACE_RAW,
> > +             .step_size          = 2,
> > +     }, {
> > +             .fourcc             = V4L2_PIX_FMT_SGBRG12P,
> > +             .depth              = 12,
> > +             .bytesperline_align = 32,
> > +             .flags              = 0,
> > +             .mmal_fmt           = MMAL_ENCODING_BAYER_SGBRG12P,
> > +             .size_multiplier_x2 = 2,
> > +             .colorspace         = V4L2_COLORSPACE_RAW,
> > +             .step_size          = 2,
> > +     }, {
> > +             /* 16 bit */
> > +             .fourcc             = V4L2_PIX_FMT_SRGGB16,
> > +             .depth              = 16,
> > +             .bytesperline_align = 32,
> > +             .flags              = 0,
> > +             .mmal_fmt           = MMAL_ENCODING_BAYER_SRGGB16,
> > +             .size_multiplier_x2 = 2,
> > +             .colorspace         = V4L2_COLORSPACE_RAW,
> > +             .step_size          = 2,
> > +     }, {
> > +             .fourcc             = V4L2_PIX_FMT_SBGGR16,
> > +             .depth              = 16,
> > +             .bytesperline_align = 32,
> > +             .flags              = 0,
> > +             .mmal_fmt           = MMAL_ENCODING_BAYER_SBGGR16,
> > +             .size_multiplier_x2 = 2,
> > +             .colorspace         = V4L2_COLORSPACE_RAW,
> > +             .step_size          = 2,
> > +     }, {
> > +             .fourcc             = V4L2_PIX_FMT_SGRBG16,
> > +             .depth              = 16,
> > +             .bytesperline_align = 32,
> > +             .flags              = 0,
> > +             .mmal_fmt           = MMAL_ENCODING_BAYER_SGRBG16,
> > +             .size_multiplier_x2 = 2,
> > +             .colorspace         = V4L2_COLORSPACE_RAW,
> > +             .step_size          = 2,
> > +     }, {
> > +             .fourcc             = V4L2_PIX_FMT_SGBRG16,
> > +             .depth              = 16,
> > +             .bytesperline_align = 32,
> > +             .flags              = 0,
> > +             .mmal_fmt           = MMAL_ENCODING_BAYER_SGBRG16,
> > +             .size_multiplier_x2 = 2,
> > +             .colorspace         = V4L2_COLORSPACE_RAW,
> > +             .step_size          = 2,
> > +     }, {
> > +             /* ISP statistics format */
> > +             .fourcc             = V4L2_META_FMT_BCM2835_ISP_STATS,
> > +             .mmal_fmt           = MMAL_ENCODING_BRCM_STATS,
> > +             /* The rest are not valid fields for stats. */
> > +     }
> > +};
> > +
> > +#endif
> > diff --git a/drivers/staging/vc04_services/include/uapi/linux/bcm2835-isp.h b/drivers/staging/vc04_services/include/uapi/linux/bcm2835-isp.h
> > new file mode 100644
> > index 000000000000..edc452fa8318
> > --- /dev/null
> > +++ b/drivers/staging/vc04_services/include/uapi/linux/bcm2835-isp.h
> > @@ -0,0 +1,333 @@
> > +/* SPDX-License-Identifier: ((GPL-2.0+ WITH Linux-syscall-note) OR BSD-3-Clause) */
> > +/*
> > + * bcm2835-isp.h
> > + *
> > + * BCM2835 ISP driver - user space header file.
> > + *
> > + * Copyright © 2019-2020 Raspberry Pi (Trading) Ltd.
> > + *
> > + * Author: Naushir Patuck (naush@raspberrypi.com)
> > + *
> > + */
> > +
> > +#ifndef __BCM2835_ISP_H_
> > +#define __BCM2835_ISP_H_
> > +
> > +#include <linux/v4l2-controls.h>
> > +
> > +/* TODO: move the control IDs definitions to v4l2-controls.h */
> > +#define V4L2_CID_USER_BCM2835_ISP_BASE         (V4L2_CID_USER_BASE + 0x10c0)
>
> As the TODO says: move this to v4l2-controls.h. Currently the 0x10c0 offset
> clashes with V4L2_CID_USER_ATMEL_ISC_BASE, so that certainly should be fixed.
>

Unfortunately, there seems to be a mixup here.  Laurent, we have
accidentally mailed a WIP revision of this patch.  The final version
does have V4L2_CID_USER_BCM2835_ISP_BASE with a unique id in
v4l2-controls.h.  I will talk with Laurent separately to get the
correct revison included in the next patch-set.

> > +
> > +/* TODO: move the formats definitions to videodev2.h */
> > +/* 12  Y/CbCr 4:2:0 128 pixel wide column */
> > +#define V4L2_PIX_FMT_NV12_COL128 v4l2_fourcc('N', 'C', '1', '2')
> > +/* Y/CbCr 4:2:0 10bpc, 3x10 packed as 4 bytes in a 128 bytes / 96 pixel wide column */
> > +#define V4L2_PIX_FMT_NV12_10_COL128 v4l2_fourcc('N', 'C', '3', '0')
> > +/* Sensor Ancillary metadata */
> > +#define V4L2_META_FMT_SENSOR_DATA v4l2_fourcc('S', 'E', 'N', 'S')
> > +/* BCM2835 ISP image statistics output */
> > +#define V4L2_META_FMT_BCM2835_ISP_STATS v4l2_fourcc('B', 'S', 'T', 'A')
> > +

Similarly, these have also been moved to the right header files.

> > +#define V4L2_CID_USER_BCM2835_ISP_CC_MATRIX  \
> > +                             (V4L2_CID_USER_BCM2835_ISP_BASE + 0x0001)
> > +#define V4L2_CID_USER_BCM2835_ISP_LENS_SHADING       \
> > +                             (V4L2_CID_USER_BCM2835_ISP_BASE + 0x0002)
> > +#define V4L2_CID_USER_BCM2835_ISP_BLACK_LEVEL        \
> > +                             (V4L2_CID_USER_BCM2835_ISP_BASE + 0x0003)
> > +#define V4L2_CID_USER_BCM2835_ISP_GEQ                \
> > +                             (V4L2_CID_USER_BCM2835_ISP_BASE + 0x0004)
> > +#define V4L2_CID_USER_BCM2835_ISP_GAMMA              \
> > +                             (V4L2_CID_USER_BCM2835_ISP_BASE + 0x0005)
> > +#define V4L2_CID_USER_BCM2835_ISP_DENOISE    \
> > +                             (V4L2_CID_USER_BCM2835_ISP_BASE + 0x0006)
> > +#define V4L2_CID_USER_BCM2835_ISP_SHARPEN    \
> > +                             (V4L2_CID_USER_BCM2835_ISP_BASE + 0x0007)
> > +#define V4L2_CID_USER_BCM2835_ISP_DPC                \
> > +                             (V4L2_CID_USER_BCM2835_ISP_BASE + 0x0008)
>
> There is no documentation for these controls. Specifically, it doesn't
> tell you which struct should be used.

As above, the documentaiton is available in the newer patch.

>
> > +
> > +/*
> > + * All structs below are directly mapped onto the equivalent structs in
> > + * drivers/staging/vc04_services/vchiq-mmal/mmal-parameters.h
> > + * for convenience.
> > + */
> > +
> > +/**
> > + * struct bcm2835_isp_rational - Rational value type.
> > + *
> > + * @num:     Numerator.
> > + * @den:     Denominator.
> > + */
> > +struct bcm2835_isp_rational {
> > +     __s32 num;
> > +     __s32 den;
>
> Wouldn't it make more sense if den is a __u32?

Ack

>
> > +};
> > +
> > +/**
> > + * struct bcm2835_isp_ccm - Colour correction matrix.
> > + *
> > + * @ccm:     3x3 correction matrix coefficients.
> > + * @offsets: 1x3 correction offsets.
> > + */
> > +struct bcm2835_isp_ccm {
> > +     struct bcm2835_isp_rational ccm[3][3];
> > +     __s32 offsets[3];
> > +};
> > +
> > +/**
> > + * struct bcm2835_isp_custom_ccm - Custom CCM applied with the
> > + *                              V4L2_CID_USER_BCM2835_ISP_CC_MATRIX ctrl.
> > + *
> > + * @enabled: Enable custom CCM.
> > + * @ccm:     Custom CCM coefficients and offsets.
> > + */
> > +struct bcm2835_isp_custom_ccm {
> > +     __u32 enabled;
> > +     struct bcm2835_isp_ccm ccm;
> > +};
> > +
> > +/**
> > + * enum bcm2835_isp_gain_format - format of the gains in the lens shading
> > + *                             tables used with the
> > + *                             V4L2_CID_USER_BCM2835_ISP_LENS_SHADING ctrl.
> > + *
> > + * @GAIN_FORMAT_U0P8_1:              Gains are u0.8 format, starting at 1.0
> > + * @GAIN_FORMAT_U1P7_0:              Gains are u1.7 format, starting at 0.0
> > + * @GAIN_FORMAT_U1P7_1:              Gains are u1.7 format, starting at 1.0
> > + * @GAIN_FORMAT_U2P6_0:              Gains are u2.6 format, starting at 0.0
> > + * @GAIN_FORMAT_U2P6_1:              Gains are u2.6 format, starting at 1.0
> > + * @GAIN_FORMAT_U3P5_0:              Gains are u3.5 format, starting at 0.0
> > + * @GAIN_FORMAT_U3P5_1:              Gains are u3.5 format, starting at 1.0
> > + * @GAIN_FORMAT_U4P10:               Gains are u4.10 format, starting at 0.0
> > + */
> > +enum bcm2835_isp_gain_format {
> > +     GAIN_FORMAT_U0P8_1 = 0,
> > +     GAIN_FORMAT_U1P7_0 = 1,
> > +     GAIN_FORMAT_U1P7_1 = 2,
> > +     GAIN_FORMAT_U2P6_0 = 3,
> > +     GAIN_FORMAT_U2P6_1 = 4,
> > +     GAIN_FORMAT_U3P5_0 = 5,
> > +     GAIN_FORMAT_U3P5_1 = 6,
> > +     GAIN_FORMAT_U4P10  = 7,
> > +};
> > +
> > +/**
> > + * struct bcm2835_isp_lens_shading - Lens shading tables supplied with the
> > + *                                V4L2_CID_USER_BCM2835_ISP_LENS_SHADING
> > + *                                ctrl.
> > + *
> > + * @enabled:         Enable lens shading.
> > + * @grid_cell_size:  Size of grid cells in samples (16, 32, 64, 128 or 256).
> > + * @grid_width:              Width of lens shading tables in grid cells.
> > + * @grid_stride:     Row to row distance (in grid cells) between grid cells
> > + *                   in the same horizontal location.
> > + * @grid_height:     Height of lens shading tables in grid cells.
> > + * @mem_handle_table:        Memory handle to the tables.
>
> What sort of handle is this? I.e. where does it come from?

I believe there was a separte discusion about this.  This is a vcsm
handle that is used to store the table coefficients in memory that is
accessible from Videocore.  There is work in progress to update this
to use dmabuf handles.

>
> > + * @ref_transform:   Reference transform - unsupported, please pass zero.
> > + * @corner_sampled:  Whether the gains are sampled at the corner points
> > + *                   of the grid cells or in the cell centres.
> > + * @gain_format:     Format of the gains (see enum &bcm2835_isp_gain_format).
> > + */
> > +struct bcm2835_isp_lens_shading {
> > +     __u32 enabled;
> > +     __u32 grid_cell_size;
> > +     __u32 grid_width;
> > +     __u32 grid_stride;
> > +     __u32 grid_height;
> > +     __u32 mem_handle_table;
> > +     __u32 ref_transform;
> > +     __u32 corner_sampled;
> > +     __u32 gain_format;
> > +};
> > +
> > +/**
> > + * struct bcm2835_isp_black_level - Sensor black level set with the
> > + *                               V4L2_CID_USER_BCM2835_ISP_BLACK_LEVEL ctrl.
> > + *
> > + * @enabled:         Enable black level.
> > + * @black_level_r:   Black level for red channel.
> > + * @black_level_g:   Black level for green channels.
> > + * @black_level_b:   Black level for blue channel.
> > + */
> > +struct bcm2835_isp_black_level {
> > +     __u32 enabled;
> > +     __u16 black_level_r;
> > +     __u16 black_level_g;
> > +     __u16 black_level_b;
> > +     __u8 pad_[2]; /* Unused */
>
> I prefer 'padding' over 'pad_'.

Ack

>
> > +};
> > +
> > +/**
> > + * struct bcm2835_isp_geq - Green equalisation parameters set with the
> > + *                       V4L2_CID_USER_BCM2835_ISP_GEQ ctrl.
> > + *
> > + * @enabled: Enable green equalisation.
> > + * @offset:  Fixed offset of the green equalisation threshold.
> > + * @slope:   Slope of the green equalisation threshold.
> > + */
> > +struct bcm2835_isp_geq {
> > +     __u32 enabled;
> > +     __u32 offset;
> > +     struct bcm2835_isp_rational slope;
> > +};
> > +
> > +#define BCM2835_NUM_GAMMA_PTS 33
> > +
> > +/**
> > + * struct bcm2835_isp_gamma - Gamma parameters set with the
> > + *                         V4L2_CID_USER_BCM2835_ISP_GAMMA ctrl.
> > + *
> > + * @enabled: Enable gamma adjustment.
> > + * @X:               X values of the points defining the gamma curve.
> > + *           Values should be scaled to 16 bits.
> > + * @Y:               Y values of the points defining the gamma curve.
> > + *           Values should be scaled to 16 bits.
>
> I assume 0 == black and 0xffff == white (or max luminance)?
>
> And so typically x[0] == y[0] == 0 and x[32] == y[32] == 0xffff?
>

Typically yes, but it is not strictily true.  We could have parameters
that clip the signal above min (0) and below max (0xffff).

> > + */
> > +struct bcm2835_isp_gamma {
> > +     __u32 enabled;
> > +     __u16 x[BCM2835_NUM_GAMMA_PTS];
> > +     __u16 y[BCM2835_NUM_GAMMA_PTS];
> > +};
> > +
> > +/**
> > + * struct bcm2835_isp_denoise - Denoise parameters set with the
> > + *                           V4L2_CID_USER_BCM2835_ISP_DENOISE ctrl.
> > + *
> > + * @enabled: Enable denoise.
> > + * @constant:        Fixed offset of the noise threshold.
> > + * @slope:   Slope of the noise threshold.
> > + * @strength:        Denoise strength between 0.0 (off) and 1.0 (maximum).
> > + */
> > +struct bcm2835_isp_denoise {
> > +     __u32 enabled;
> > +     __u32 constant;
> > +     struct bcm2835_isp_rational slope;
> > +     struct bcm2835_isp_rational strength;
> > +};
> > +
> > +/**
> > + * struct bcm2835_isp_sharpen - Sharpen parameters set with the
> > + *                           V4L2_CID_USER_BCM2835_ISP_SHARPEN ctrl.
> > + *
> > + * @enabled: Enable sharpening.
> > + * @threshold:       Threshold at which to start sharpening pixels.
> > + * @strength:        Strength with which pixel sharpening increases.
> > + * @limit:   Limit to the amount of sharpening applied.
> > + */
> > +struct bcm2835_isp_sharpen {
> > +     __u32 enabled;
> > +     struct bcm2835_isp_rational threshold;
> > +     struct bcm2835_isp_rational strength;
> > +     struct bcm2835_isp_rational limit;
> > +};
> > +
> > +/**
> > + * enum bcm2835_isp_dpc_mode - defective pixel correction (DPC) strength.
> > + *
> > + * @DPC_MODE_OFF:            No DPC.
> > + * @DPC_MODE_NORMAL:         Normal DPC.
> > + * @DPC_MODE_STRONG:         Strong DPC.
> > + */
> > +enum bcm2835_isp_dpc_mode {
> > +     DPC_MODE_OFF = 0,
> > +     DPC_MODE_NORMAL = 1,
> > +     DPC_MODE_STRONG = 2,
> > +};
> > +
> > +/**
> > + * struct bcm2835_isp_dpc - Defective pixel correction (DPC) parameters set
> > + *                       with the V4L2_CID_USER_BCM2835_ISP_DPC ctrl.
> > + *
> > + * @enabled: Enable DPC.
> > + * @strength:        DPC strength (see enum &bcm2835_isp_dpc_mode).
>
> Isn't DPC_MODE_OFF equal to just setting 'enabled' to false? If so,
> wouldn't the 'strength' field be sufficient?

This is a bit of a quirk of the hardware pipeline.  DPC_MODE_OFF still
keeps the block enabled, but running (mostly) as a passthrough.  The
enabled field physically switches off the block.

>
> > + */
> > +struct bcm2835_isp_dpc {
> > +     __u32 enabled;
> > +     __u32 strength;
> > +};
> > +
> > +/*
> > + * ISP statistics structures.
> > + *
> > + * The bcm2835_isp_stats structure is generated at the output of the
> > + * statistics node.  Note that this does not directly map onto the statistics
> > + * output of the ISP HW.  Instead, the MMAL firmware code maps the HW statistics
> > + * to the bcm2835_isp_stats structure.
> > + */
> > +#define DEFAULT_AWB_REGIONS_X 16
> > +#define DEFAULT_AWB_REGIONS_Y 12
> > +
> > +#define NUM_HISTOGRAMS 2
> > +#define NUM_HISTOGRAM_BINS 128
> > +#define AWB_REGIONS (DEFAULT_AWB_REGIONS_X * DEFAULT_AWB_REGIONS_Y)
> > +#define FLOATING_REGIONS 16
> > +#define AGC_REGIONS 16
> > +#define FOCUS_REGIONS 12
> > +
> > +/**
> > + * struct bcm2835_isp_stats_hist - Histogram statistics
> > + *
> > + * @r_hist:  Red channel histogram.
> > + * @g_hist:  Combined green channel histogram.
> > + * @b_hist:  Blue channel histogram.
> > + */
> > +struct bcm2835_isp_stats_hist {
> > +     __u32 r_hist[NUM_HISTOGRAM_BINS];
> > +     __u32 g_hist[NUM_HISTOGRAM_BINS];
> > +     __u32 b_hist[NUM_HISTOGRAM_BINS];
> > +};
> > +
> > +/**
> > + * struct bcm2835_isp_stats_region - Region sums.
> > + *
> > + * @counted: The number of 2x2 bayer tiles accumulated.
> > + * @notcounted:      The number of 2x2 bayer tiles not accumulated.
> > + * @r_sum:   Total sum of counted pixels in the red channel for a region.
> > + * @g_sum:   Total sum of counted pixels in the green channel for a region.
> > + * @b_sum:   Total sum of counted pixels in the blue channel for a region.
> > + */
> > +struct bcm2835_isp_stats_region {
> > +     __u32 counted;
> > +     __u32 notcounted;
> > +     __u64 r_sum;
> > +     __u64 g_sum;
> > +     __u64 b_sum;
> > +};
> > +
> > +/**
> > + * struct bcm2835_isp_stats_focus - Focus statistics.
> > + *
> > + * @contrast_val:    Focus measure - accumulated output of the focus filter.
> > + *                   In the first dimension, index [0] counts pixels below a
> > + *                   preset threshold, and index [1] counts pixels above the
> > + *                   threshold.  In the second dimension, index [0] uses the
> > + *                   first predefined filter, and index [1] uses the second
> > + *                   predefined filter.
> > + * @contrast_val_num:        The number of counted pixels in the above accumulation.
> > + */
> > +struct bcm2835_isp_stats_focus {
> > +     __u64 contrast_val[2][2];
> > +     __u32 contrast_val_num[2][2];
> > +};
> > +
> > +/**
> > + * struct bcm2835_isp_stats - ISP statistics.
> > + *
> > + * @version:         Version of the bcm2835_isp_stats structure.
> > + * @size:            Size of the bcm2835_isp_stats structure.
> > + * @hist:            Histogram statistics for the entire image.
> > + * @awb_stats:               Statistics for the regions defined for AWB calculations.
> > + * @floating_stats:  Statistics for arbitrarily placed (floating) regions.
> > + * @agc_stats:               Statistics for the regions defined for AGC calculations.
> > + * @focus_stats:     Focus filter statistics for the focus regions.
> > + */
> > +struct bcm2835_isp_stats {
> > +     __u32 version;
> > +     __u32 size;
> > +     struct bcm2835_isp_stats_hist hist[NUM_HISTOGRAMS];
> > +     struct bcm2835_isp_stats_region awb_stats[AWB_REGIONS];
> > +     struct bcm2835_isp_stats_region floating_stats[FLOATING_REGIONS];
> > +     struct bcm2835_isp_stats_region agc_stats[AGC_REGIONS];
> > +     struct bcm2835_isp_stats_focus focus_stats[FOCUS_REGIONS];
> > +};
> > +
> > +#endif /* __BCM2835_ISP_H_ */
> > diff --git a/drivers/staging/vc04_services/vchiq-mmal/Kconfig b/drivers/staging/vc04_services/vchiq-mmal/Kconfig
> > index 106f71e709df..072f3c755a68 100644
> > --- a/drivers/staging/vc04_services/vchiq-mmal/Kconfig
> > +++ b/drivers/staging/vc04_services/vchiq-mmal/Kconfig
> > @@ -5,4 +5,5 @@ config BCM2835_VCHIQ_MMAL
> >       help
> >         Enables the MMAL API over VCHIQ interface as used for the
> >         majority of the multimedia services on VideoCore.
> > -       Defaults to Y when the Broadcomd BCM2835 camera host is selected.
> > +       Defaults to Y when the Broadcomd BCM2835 camera host or ISP are
> > +       selected.
> > diff --git a/drivers/staging/vc04_services/vchiq-mmal/mmal-encodings.h b/drivers/staging/vc04_services/vchiq-mmal/mmal-encodings.h
> > index 44ba91aa6d47..8d904fcce388 100644
> > --- a/drivers/staging/vc04_services/vchiq-mmal/mmal-encodings.h
> > +++ b/drivers/staging/vc04_services/vchiq-mmal/mmal-encodings.h
> > @@ -100,6 +100,10 @@
> >   */
> >  #define MMAL_ENCODING_EGL_IMAGE        MMAL_FOURCC('E', 'G', 'L', 'I')
> >
> > +/** ISP image statistics format
> > + */
> > +#define MMAL_ENCODING_BRCM_STATS       MMAL_FOURCC('S', 'T', 'A', 'T')
> > +
> >  /* }@ */
> >
> >  /** \name Pre-defined audio encodings */
> > diff --git a/drivers/staging/vc04_services/vchiq-mmal/mmal-parameters.h b/drivers/staging/vc04_services/vchiq-mmal/mmal-parameters.h
> > index 1793103b18fd..b3552af5cf8f 100644
> > --- a/drivers/staging/vc04_services/vchiq-mmal/mmal-parameters.h
> > +++ b/drivers/staging/vc04_services/vchiq-mmal/mmal-parameters.h
> > @@ -221,6 +221,62 @@ enum mmal_parameter_camera_type {
> >       MMAL_PARAMETER_SHUTTER_SPEED,
> >               /**< Takes a @ref MMAL_PARAMETER_AWB_GAINS_T */
> >       MMAL_PARAMETER_CUSTOM_AWB_GAINS,
> > +             /**< Takes a @ref MMAL_PARAMETER_CAMERA_SETTINGS_T */
> > +     MMAL_PARAMETER_CAMERA_SETTINGS,
> > +             /**< Takes a @ref MMAL_PARAMETER_PRIVACY_INDICATOR_T */
> > +     MMAL_PARAMETER_PRIVACY_INDICATOR,
> > +             /**< Takes a @ref MMAL_PARAMETER_BOOLEAN_T */
> > +     MMAL_PARAMETER_VIDEO_DENOISE,
> > +             /**< Takes a @ref MMAL_PARAMETER_BOOLEAN_T */
> > +     MMAL_PARAMETER_STILLS_DENOISE,
> > +             /**< Takes a @ref MMAL_PARAMETER_CAMERA_ANNOTATE_T */
> > +     MMAL_PARAMETER_ANNOTATE,
> > +             /**< Takes a @ref MMAL_PARAMETER_STEREOSCOPIC_MODE_T */
> > +     MMAL_PARAMETER_STEREOSCOPIC_MODE,
> > +             /**< Takes a @ref MMAL_PARAMETER_CAMERA_INTERFACE_T */
> > +     MMAL_PARAMETER_CAMERA_INTERFACE,
> > +             /**< Takes a @ref MMAL_PARAMETER_CAMERA_CLOCKING_MODE_T */
> > +     MMAL_PARAMETER_CAMERA_CLOCKING_MODE,
> > +             /**< Takes a @ref MMAL_PARAMETER_CAMERA_RX_CONFIG_T */
> > +     MMAL_PARAMETER_CAMERA_RX_CONFIG,
> > +             /**< Takes a @ref MMAL_PARAMETER_CAMERA_RX_TIMING_T */
> > +     MMAL_PARAMETER_CAMERA_RX_TIMING,
> > +             /**< Takes a @ref MMAL_PARAMETER_UINT32_T */
> > +     MMAL_PARAMETER_DPF_CONFIG,
> > +
> > +     /* 0x50 */
> > +             /**< Takes a @ref MMAL_PARAMETER_UINT32_T */
> > +     MMAL_PARAMETER_JPEG_RESTART_INTERVAL,
> > +             /**< Takes a @ref MMAL_PARAMETER_UINT32_T */
> > +     MMAL_PARAMETER_CAMERA_ISP_BLOCK_OVERRIDE,
> > +             /**< Takes a @ref MMAL_PARAMETER_LENS_SHADING_T */
> > +     MMAL_PARAMETER_LENS_SHADING_OVERRIDE,
> > +             /**< Takes a @ref MMAL_PARAMETER_UINT32_T */
> > +     MMAL_PARAMETER_BLACK_LEVEL,
> > +             /**< Takes a @ref MMAL_PARAMETER_RESIZE_T */
> > +     MMAL_PARAMETER_RESIZE_PARAMS,
> > +             /**< Takes a @ref MMAL_PARAMETER_CROP_T */
> > +     MMAL_PARAMETER_CROP,
> > +             /**< Takes a @ref MMAL_PARAMETER_INT32_T */
> > +     MMAL_PARAMETER_OUTPUT_SHIFT,
> > +             /**< Takes a @ref MMAL_PARAMETER_INT32_T */
> > +     MMAL_PARAMETER_CCM_SHIFT,
> > +             /**< Takes a @ref MMAL_PARAMETER_CUSTOM_CCM_T */
> > +     MMAL_PARAMETER_CUSTOM_CCM,
> > +             /**< Takes a @ref MMAL_PARAMETER_RATIONAL_T */
> > +     MMAL_PARAMETER_ANALOG_GAIN,
> > +             /**< Takes a @ref MMAL_PARAMETER_RATIONAL_T */
> > +     MMAL_PARAMETER_DIGITAL_GAIN,
> > +             /**< Takes a @ref MMAL_PARAMETER_DENOISE_T */
> > +     MMAL_PARAMETER_DENOISE,
> > +             /**< Takes a @ref MMAL_PARAMETER_SHARPEN_T */
> > +     MMAL_PARAMETER_SHARPEN,
> > +             /**< Takes a @ref MMAL_PARAMETER_GEQ_T */
> > +     MMAL_PARAMETER_GEQ,
> > +             /**< Tales a @ref MMAP_PARAMETER_DPC_T */
> > +     MMAL_PARAMETER_DPC,
> > +             /**< Tales a @ref MMAP_PARAMETER_GAMMA_T */
> > +     MMAL_PARAMETER_GAMMA,
> >  };
> >
> >  struct mmal_parameter_rational {
> > @@ -779,7 +835,102 @@ struct mmal_parameter_camera_info {
> >       struct mmal_parameter_camera_info_camera
> >               cameras[MMAL_PARAMETER_CAMERA_INFO_MAX_CAMERAS];
> >       struct mmal_parameter_camera_info_flash
> > -                             flashes[MMAL_PARAMETER_CAMERA_INFO_MAX_FLASHES];
> > +             flashes[MMAL_PARAMETER_CAMERA_INFO_MAX_FLASHES];
> > +};
> > +
> > +struct mmal_parameter_ccm {
> > +     struct mmal_parameter_rational ccm[3][3];
> > +     s32 offsets[3];
> > +};
> > +
> > +struct mmal_parameter_custom_ccm {
> > +     u32 enabled; /**< Enable the custom CCM. */
> > +     struct mmal_parameter_ccm ccm; /**< CCM to be used. */
> > +};
> > +
> > +struct mmal_parameter_lens_shading {
> > +     u32 enabled;
> > +     u32 grid_cell_size;
> > +     u32 grid_width;
> > +     u32 grid_stride;
> > +     u32 grid_height;
> > +     u32 mem_handle_table;
> > +     u32 ref_transform;
> > +};
> > +
> > +enum mmal_parameter_ls_gain_format_type {
> > +     MMAL_PARAMETER_LS_GAIN_FORMAT_TYPE_U0P8_1 = 0,
> > +     MMAL_PARAMETER_LS_GAIN_FORMAT_TYPE_U1P7_0 = 1,
> > +     MMAL_PARAMETER_LS_GAIN_FORMAT_TYPE_U1P7_1 = 2,
> > +     MMAL_PARAMETER_LS_GAIN_FORMAT_TYPE_U2P6_0 = 3,
> > +     MMAL_PARAMETER_LS_GAIN_FORMAT_TYPE_U2P6_1 = 4,
> > +     MMAL_PARAMETER_LS_GAIN_FORMAT_TYPE_U3P5_0 = 5,
> > +     MMAL_PARAMETER_LS_GAIN_FORMAT_TYPE_U3P5_1 = 6,
> > +     MMAL_PARAMETER_LS_GAIN_FORMAT_TYPE_U4P10  = 7,
> > +     MMAL_PARAMETER_LS_GAIN_FORMAT_TYPE_DUMMY  = 0x7FFFFFFF
> > +};
> > +
> > +struct mmal_parameter_lens_shading_v2 {
> > +     u32 enabled;
> > +     u32 grid_cell_size;
> > +     u32 grid_width;
> > +     u32 grid_stride;
> > +     u32 grid_height;
> > +     u32 mem_handle_table;
> > +     u32 ref_transform;
> > +     u32 corner_sampled;
> > +     enum mmal_parameter_ls_gain_format_type gain_format;
> > +};
> > +
> > +struct mmal_parameter_black_level {
> > +     u32 enabled;
> > +     u16 black_level_r;
> > +     u16 black_level_g;
> > +     u16 black_level_b;
> > +     u8 pad_[2]; /* Unused */
> > +};
> > +
> > +struct mmal_parameter_geq {
> > +     u32 enabled;
> > +     u32 offset;
> > +     struct mmal_parameter_rational slope;
> > +};
> > +
> > +#define MMAL_NUM_GAMMA_PTS 33
> > +struct mmal_parameter_gamma {
> > +     u32 enabled;
> > +     u16 x[MMAL_NUM_GAMMA_PTS];
> > +     u16 y[MMAL_NUM_GAMMA_PTS];
> > +};
> > +
> > +struct mmal_parameter_denoise {
> > +     u32 enabled;
> > +     u32 constant;
> > +     struct mmal_parameter_rational slope;
> > +     struct mmal_parameter_rational strength;
> > +};
> > +
> > +struct mmal_parameter_sharpen {
> > +     u32 enabled;
> > +     struct mmal_parameter_rational threshold;
> > +     struct mmal_parameter_rational strength;
> > +     struct mmal_parameter_rational limit;
> > +};
> > +
> > +enum mmal_dpc_mode {
> > +     MMAL_DPC_MODE_OFF = 0,
> > +     MMAL_DPC_MODE_NORMAL = 1,
> > +     MMAL_DPC_MODE_STRONG = 2,
> > +     MMAL_DPC_MODE_MAX = 0x7FFFFFFF,
> > +};
> > +
> > +struct mmal_parameter_dpc {
> > +     u32 enabled;
> > +     u32 strength;
> > +};
> > +
> > +struct mmal_parameter_crop {
> > +     struct vchiq_mmal_rect rect;
> >  };
> >
> >  #endif
> >
>
> Regards,
>
>         Hans

Regards,
Naush

  parent reply	other threads:[~2020-05-19 12:47 UTC|newest]

Thread overview: 104+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-04  9:25 [PATCH v2 00/34] Drivers for the BCM283x CSI-2/CCP2 receiver and ISP Laurent Pinchart
2020-05-04  9:25 ` [PATCH v2 01/34] media: uapi: v4l2-core: Add sensor ancillary data V4L2 fourcc type Laurent Pinchart
2020-05-04 13:48   ` Hans Verkuil
2020-05-04 14:39     ` Dave Stevenson
2020-05-04 15:32       ` Hans Verkuil
2020-05-04 16:08         ` Laurent Pinchart
2020-05-05 11:20           ` Dave Stevenson
2020-05-04  9:25 ` [PATCH v2 02/34] media: uapi: Add MEDIA_BUS_FMT_SENSOR_DATA media bus format Laurent Pinchart
2020-05-04  9:25 ` [PATCH v2 03/34] dt-bindings: media: Document BCM283x CSI2/CCP2 receiver Laurent Pinchart
2020-05-04  9:25 ` [PATCH v2 04/34] media: bcm2835-unicam: Driver for CCP2/CSI2 camera interface Laurent Pinchart
2020-05-04 15:21   ` Hans Verkuil
2020-05-05  1:26   ` kbuild test robot
2020-05-06 18:01   ` Nicolas Saenz Julienne
2020-08-29 11:20   ` Jacopo Mondi
2020-08-29 18:32     ` Laurent Pinchart
2020-08-31  7:38       ` Jacopo Mondi
2020-08-31 14:17         ` Laurent Pinchart
2020-08-31 14:46           ` Jacopo Mondi
2020-08-31 14:56             ` Laurent Pinchart
2020-09-01  8:41               ` Dave Stevenson
2020-09-01 10:22                 ` Jacopo Mondi
2020-09-01 16:37                   ` Dave Stevenson
2020-09-01 17:11                     ` Laurent Pinchart
2020-09-15  7:03   ` Sakari Ailus
2020-09-15  9:32     ` Laurent Pinchart
2020-09-15 13:28       ` Dave Stevenson
2020-10-30 17:53         ` Jacopo Mondi
2020-09-15 17:30     ` Dave Stevenson
2020-05-04  9:25 ` [PATCH v2 05/34] ARM: dts: bcm2711: Add Unicam DT nodes Laurent Pinchart
2020-05-04  9:25 ` [PATCH v2 06/34] staging: vc04_services: Add new vc-sm-cma driver Laurent Pinchart
2020-05-05  2:38   ` kbuild test robot
2020-05-05 12:17   ` kbuild test robot
2020-05-06  3:05   ` kbuild test robot
2020-05-06 18:04   ` Nicolas Saenz Julienne
2020-05-06 19:24     ` Dave Stevenson
2020-05-08  0:11       ` Laurent Pinchart
2020-05-18 12:06         ` Hans Verkuil
2020-08-24 16:39       ` Jacopo Mondi
2020-08-25 17:52         ` Dave Stevenson
2020-08-27 10:38           ` Jacopo Mondi
2020-08-27 12:51             ` Dave Stevenson
2020-08-27 16:46               ` Jacopo Mondi
2020-08-27 17:19                 ` Dave Stevenson
2020-05-11 18:42   ` Nicolas Saenz Julienne
2020-05-18 15:48     ` Dave Stevenson
2020-05-20 14:41       ` Nicolas Saenz Julienne
2020-05-21 11:01         ` Dave Stevenson
2020-05-04  9:25 ` [PATCH v2 07/34] staging: bcm2835: Break MMAL support out from camera Laurent Pinchart
2020-05-04  9:25 ` [PATCH v2 08/34] staging: mmal-vchiq: Allocate and free components as required Laurent Pinchart
2020-05-04  9:25 ` [PATCH v2 09/34] staging: mmal-vchiq: Avoid use of bool in structures Laurent Pinchart
2020-05-04  9:25 ` [PATCH v2 10/34] staging: mmal-vchiq: Make timeout a defined parameter Laurent Pinchart
2020-05-04  9:25 ` [PATCH v2 11/34] staging: mmal-vchiq: Make a mmal_buf struct for passing parameters Laurent Pinchart
2020-05-04  9:25 ` [PATCH v2 12/34] staging: mmal-vchiq: Add support for event callbacks Laurent Pinchart
2020-05-04  9:25 ` [PATCH v2 13/34] staging: mmal-vchiq: Support sending data to MMAL ports Laurent Pinchart
2020-05-04  9:25 ` [PATCH v2 14/34] staging: mmal-vchiq: Fixup vchiq-mmal include ordering Laurent Pinchart
2020-05-04  9:25 ` [PATCH v2 15/34] staging: mmal-vchiq: Use vc-sm-cma to support zero copy Laurent Pinchart
2020-05-04 16:30   ` Nicolas Saenz Julienne
2020-05-05 16:07   ` kbuild test robot
2020-05-11 19:15   ` Nicolas Saenz Julienne
2020-05-04  9:25 ` [PATCH v2 16/34] staging: mmal-vchiq: Fix client_component for 64 bit kernel Laurent Pinchart
2020-05-18 10:19   ` Hans Verkuil
2020-05-04  9:25 ` [PATCH v2 17/34] staging: mmal_vchiq: Add in the Bayer encoding formats Laurent Pinchart
2020-05-04  9:25 ` [PATCH v2 18/34] staging: mmal-vchiq: Always return the param size from param_get Laurent Pinchart
2020-05-04  9:25 ` [PATCH v2 19/34] staging: mmal-vchiq: If the VPU returns an error, don't negate it Laurent Pinchart
2020-05-04  9:25 ` [PATCH v2 20/34] staging: mmal-vchiq: Fix handling of VB2_MEMORY_DMABUF buffers Laurent Pinchart
2020-05-04  9:25 ` [PATCH v2 21/34] staging: mmal-vchiq: Update mmal_parameters.h with recently defined params Laurent Pinchart
2020-05-04  9:25 ` [PATCH v2 22/34] staging: mmal-vchiq: Free the event context for control ports Laurent Pinchart
2020-05-04  9:26 ` [PATCH v2 23/34] staging: mmal-vchiq: Fix memory leak in error path Laurent Pinchart
2020-05-04  9:26 ` [PATCH v2 24/34] staging: mmal-vchiq: Fix formatting errors in mmal_parameters.h Laurent Pinchart
2020-05-04  9:26 ` [PATCH v2 25/34] staging: vchiq_arm: Register vcsm-cma as a platform driver Laurent Pinchart
2020-05-04  9:26 ` [PATCH v2 26/34] staging: vchiq_arm: Set up dma ranges on child devices Laurent Pinchart
2020-05-04 16:54   ` Nicolas Saenz Julienne
2020-08-25 16:57     ` Jacopo Mondi
2020-05-04  9:26 ` [PATCH v2 27/34] staging: vchiq: Use the old dma controller for OF config on platform devices Laurent Pinchart
2020-05-04 15:44   ` Nicolas Saenz Julienne
2020-05-04  9:26 ` [PATCH v2 28/34] staging: vchiq_2835_arm: Implement a DMA pool for small bulk transfers Laurent Pinchart
2020-05-04  9:26 ` [PATCH v2 29/34] staging: vchiq: Add 36-bit address support Laurent Pinchart
2020-05-04 17:40   ` Nicolas Saenz Julienne
2020-05-04 20:46     ` Phil Elwell
2020-05-05 10:13       ` Nicolas Saenz Julienne
2020-05-05 10:57         ` Phil Elwell
2020-05-05 13:22   ` kbuild test robot
2020-05-04  9:26 ` [PATCH v2 30/34] staging: vchiq_arm: Give vchiq children DT nodes Laurent Pinchart
2020-05-04 17:12   ` Nicolas Saenz Julienne
2020-05-04 19:42     ` Phil Elwell
2020-05-05 10:37       ` Nicolas Saenz Julienne
2020-05-05 10:50         ` Phil Elwell
2020-05-04  9:26 ` [PATCH v2 31/34] staging: vchiq_arm: Add a matching unregister call Laurent Pinchart
2020-05-04  9:26 ` [PATCH v2 32/34] media: videobuf2: Allow exporting of a struct dmabuf Laurent Pinchart
2020-05-04 13:36   ` Hans Verkuil
2020-05-04  9:26 ` [PATCH v2 33/34] staging: bcm2835-isp: Add support for BC2835 ISP Laurent Pinchart
2020-05-11 19:19   ` Nicolas Saenz Julienne
2020-05-18 13:38     ` Dave Stevenson
2020-05-20 13:46       ` Nicolas Saenz Julienne
2020-05-18 12:02   ` Hans Verkuil
2020-05-18 14:36     ` Dave Stevenson
2020-05-18 15:07       ` Hans Verkuil
2020-05-19 12:47     ` Naushir Patuck [this message]
2020-05-19 13:03       ` Jacopo Mondi
2020-06-24 11:28       ` Hans Verkuil
2020-05-04  9:26 ` [PATCH v2 34/34] staging: vchiq: Load bcm2835_isp driver from vchiq Laurent Pinchart
2020-05-04 15:15 ` [PATCH v2 00/34] Drivers for the BCM283x CSI-2/CCP2 receiver and ISP Nicolas Saenz Julienne
2020-05-04 15:38   ` Laurent Pinchart
2020-05-04 16:15     ` Nicolas Saenz Julienne

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAEmqJPo-ST8msiQVedLmH48vsMBz2WeaK6WXyN5fiP5z1b+wAA@mail.gmail.com \
    --to=naush@raspberrypi.com \
    --cc=dave.stevenson@raspberrypi.com \
    --cc=hverkuil@xs4all.nl \
    --cc=jacopo@jmondi.org \
    --cc=kieran.bingham@ideasonboard.com \
    --cc=laurent.pinchart@ideasonboard.com \
    --cc=linux-media@vger.kernel.org \
    --cc=niklas.soderlund@ragnatech.se \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).