Re: [PATCH v4 11/12] intel-ipu3: Add imgu v4l2 driver

2017-11-02 Thread Sakari Ailus
Hi Yong,

On Mon, Oct 23, 2017 at 10:41:57PM +, Zhi, Yong wrote:
> > > + default:
> > > + return -EINVAL;
> > > + }
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static int ipu3_try_fmt(struct file *file, void *fh, struct
> > > +v4l2_format *f) {
> > > + struct v4l2_pix_format_mplane *pixm = >fmt.pix_mp;
> > > + const struct ipu3_fmt *fmt;
> > > +
> > > + if (f->type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE)
> > > + fmt = find_format(f, M2M_CAPTURE);
> > > + else if (f->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE)
> > > + fmt = find_format(f, M2M_OUTPUT);
> > > + else
> > > + return -EINVAL;
> > > +
> > > + pixm->pixelformat = fmt->fourcc;
> > > +
> > > + memset(pixm->plane_fmt[0].reserved, 0,
> > > +sizeof(pixm->plane_fmt[0].reserved));
> > 
> > No need for the memset here, the framework handles this.
> > 
> > Are there limits on the image size?
> > 
> 
> The memset is added to fix v4l2-compliance failure here.

Oops. Indeed, this is about the plane format. Please ignore the comment.

-- 
Sakari Ailus
e-mail: sakari.ai...@iki.fi


RE: [PATCH v4 11/12] intel-ipu3: Add imgu v4l2 driver

2017-10-23 Thread Zhi, Yong
Hi, Sakari,

> -Original Message-
> From: linux-media-ow...@vger.kernel.org [mailto:linux-media-
> ow...@vger.kernel.org] On Behalf Of Sakari Ailus
> Sent: Friday, October 20, 2017 4:30 AM
> To: Zhi, Yong <yong@intel.com>
> Cc: linux-media@vger.kernel.org; sakari.ai...@linux.intel.com; Zheng, Jian
> Xu <jian.xu.zh...@intel.com>; Mani, Rajmohan
> <rajmohan.m...@intel.com>; Toivonen, Tuukka
> <tuukka.toivo...@intel.com>; Hu, Jerry W <jerry.w...@intel.com>;
> Vijaykumar, Ramya <ramya.vijayku...@intel.com>
> Subject: Re: [PATCH v4 11/12] intel-ipu3: Add imgu v4l2 driver
> 
> Hi Yong,
> 
> On Tue, Oct 17, 2017 at 10:54:56PM -0500, Yong Zhi wrote:
> > ipu3 imgu video device based on v4l2, vb2 and media controller
> > framework.
> >
> > Signed-off-by: Yong Zhi <yong@intel.com>
> > Signed-off-by: Ramya Vijaykumar <ramya.vijayku...@intel.com>
> > ---
> >  drivers/media/pci/intel/ipu3/ipu3-v4l2.c | 1150
> > ++
> >  1 file changed, 1150 insertions(+)
> >  create mode 100644 drivers/media/pci/intel/ipu3/ipu3-v4l2.c
> >
> > diff --git a/drivers/media/pci/intel/ipu3/ipu3-v4l2.c
> > b/drivers/media/pci/intel/ipu3/ipu3-v4l2.c
> > new file mode 100644
> > index ..4618880b8675
> > --- /dev/null
> > +++ b/drivers/media/pci/intel/ipu3/ipu3-v4l2.c
> > @@ -0,0 +1,1150 @@
(snip)
> > +static int mem2mem2_g_selection(struct ipu3_mem2mem2_device
> *m2m2_dev,
> > +   int node, struct v4l2_selection *s) {
> > +   struct imgu_device *const imgu =
> > +   container_of(m2m2_dev, struct imgu_device, mem2mem2);
> > +
> > +   if (node != IPU3_CSS_QUEUE_IN)
> > +   return -ENOIOCTLCMD;
> > +
> > +   switch (s->target) {
> > +   case V4L2_SEL_TGT_CROP:
> > +   s->r = imgu->rect.eff;
> > +   break;
> > +   case V4L2_SEL_TGT_CROP_BOUNDS:
> > +   break;
> > +   case V4L2_SEL_TGT_CROP_DEFAULT:
> > +   break;
> > +   case V4L2_SEL_TGT_COMPOSE_PADDED:
> > +   break;
> > +   case V4L2_SEL_TGT_COMPOSE_BOUNDS:
> > +   break;
> > +   case V4L2_SEL_TGT_COMPOSE:
> > +   s->r = imgu->rect.bds;
> > +   break;
> > +   case V4L2_SEL_TGT_COMPOSE_DEFAULT:
> > +   break;
> > +
> 
> As the driver uses the V4L2 sub-device interface, the selection API belongsis
> implemented in the sub-device node, not the video nodes.
> 

Ok, will study how to make necessary changes to use sub-dev interface for above.

> > +   default:
> > +   return -EINVAL;
> > +   }
> > +
> > +   return 0;
> > +}
> > +
> > +static int ipu3_try_fmt(struct file *file, void *fh, struct
> > +v4l2_format *f) {
> > +   struct v4l2_pix_format_mplane *pixm = >fmt.pix_mp;
> > +   const struct ipu3_fmt *fmt;
> > +
> > +   if (f->type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE)
> > +   fmt = find_format(f, M2M_CAPTURE);
> > +   else if (f->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE)
> > +   fmt = find_format(f, M2M_OUTPUT);
> > +   else
> > +   return -EINVAL;
> > +
> > +   pixm->pixelformat = fmt->fourcc;
> > +
> > +   memset(pixm->plane_fmt[0].reserved, 0,
> > +  sizeof(pixm->plane_fmt[0].reserved));
> 
> No need for the memset here, the framework handles this.
> 
> Are there limits on the image size?
> 

The memset is added to fix v4l2-compliance failure here.

The image size limit is checked in ipu3-css.c/ipu3_css_queue_init().

(snip)
> > +int ipu3_v4l2_register(struct imgu_device *dev) {
> > +   struct ipu3_mem2mem2_device *m2m2 = >mem2mem2;
> > +   struct v4l2_mbus_framefmt def_bus_fmt;
> > +   struct v4l2_pix_format_mplane def_pix_fmt;
> > +
> > +   int i, r;
> > +
> > +   /* Initialize miscellaneous variables */
> > +   m2m2->streaming = false;
> > +   m2m2->v4l2_file_ops = ipu3_v4l2_fops;
> > +
> > +   /* Set up media device */
> > +   m2m2->media_dev.dev = m2m2->dev;
> > +   strlcpy(m2m2->media_dev.model, m2m2->model,
> > +   sizeof(m2m2->media_dev.model));
> > +   snprintf(m2m2->media_dev.bus_info, sizeof(m2m2-
> >media_dev.bus_info),
> > +"%s", dev_name(m2m2->dev));
> > +   m2m2->media_dev.hw_revision = 0;
> > +   media_device_init(>media_dev);
> > +   r = media_device_register(>media_dev);
> > +   if (r) {
> > +   dev_err(m2m2-&g

Re: [PATCH v4 11/12] intel-ipu3: Add imgu v4l2 driver

2017-10-20 Thread Sakari Ailus
Hi Yong,

On Tue, Oct 17, 2017 at 10:54:56PM -0500, Yong Zhi wrote:
> ipu3 imgu video device based on v4l2, vb2 and
> media controller framework.
> 
> Signed-off-by: Yong Zhi 
> Signed-off-by: Ramya Vijaykumar 
> ---
>  drivers/media/pci/intel/ipu3/ipu3-v4l2.c | 1150 
> ++
>  1 file changed, 1150 insertions(+)
>  create mode 100644 drivers/media/pci/intel/ipu3/ipu3-v4l2.c
> 
> diff --git a/drivers/media/pci/intel/ipu3/ipu3-v4l2.c 
> b/drivers/media/pci/intel/ipu3/ipu3-v4l2.c
> new file mode 100644
> index ..4618880b8675
> --- /dev/null
> +++ b/drivers/media/pci/intel/ipu3/ipu3-v4l2.c
> @@ -0,0 +1,1150 @@
> +/*
> + * Copyright (c) 2017 Intel Corporation.
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License version
> + * 2 as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include "ipu3.h"
> +#include "ipu3-dmamap.h"
> +
> +/ v4l2_subdev_ops /
> +
> +static int ipu3_subdev_s_stream(struct v4l2_subdev *sd, int enable)
> +{
> + struct ipu3_mem2mem2_device *m2m2 =
> + container_of(sd, struct ipu3_mem2mem2_device, subdev);
> + int r = 0;
> +
> + if (m2m2->ops && m2m2->ops->s_stream)
> + r = m2m2->ops->s_stream(m2m2, enable);
> +
> + if (!r)
> + m2m2->streaming = enable;
> +
> + return r;
> +}
> +
> +static int ipu3_subdev_get_fmt(struct v4l2_subdev *sd,
> +struct v4l2_subdev_pad_config *cfg,
> +struct v4l2_subdev_format *fmt)
> +{
> + struct ipu3_mem2mem2_device *m2m2 =
> + container_of(sd, struct ipu3_mem2mem2_device, subdev);
> + struct v4l2_mbus_framefmt *mf;
> + u32 pad = fmt->pad;
> +
> + if (fmt->which == V4L2_SUBDEV_FORMAT_ACTIVE) {
> + fmt->format = m2m2->nodes[pad].pad_fmt;
> + } else {
> + mf = v4l2_subdev_get_try_format(sd, cfg, pad);
> + fmt->format = *mf;
> + }
> +
> + return 0;
> +}
> +
> +static int ipu3_subdev_set_fmt(struct v4l2_subdev *sd,
> +struct v4l2_subdev_pad_config *cfg,
> +struct v4l2_subdev_format *fmt)
> +{
> + struct ipu3_mem2mem2_device *m2m2 =
> + container_of(sd, struct ipu3_mem2mem2_device, subdev);
> + struct v4l2_mbus_framefmt *mf;
> + u32 pad = fmt->pad;
> +
> + if (fmt->which == V4L2_SUBDEV_FORMAT_TRY)
> + mf = v4l2_subdev_get_try_format(sd, cfg, pad);
> + else
> + mf = >nodes[pad].pad_fmt;
> +
> + /* Clamp the w and h based on the hardware capabilities */
> + if (m2m2->subdev_pads[pad].flags & MEDIA_PAD_FL_SOURCE) {
> + fmt->format.width = clamp(fmt->format.width,
> +   IPU3_OUTPUT_MIN_WIDTH,
> +   IPU3_OUTPUT_MAX_WIDTH);
> + fmt->format.height = clamp(fmt->format.height,
> +IPU3_OUTPUT_MIN_HEIGHT,
> +IPU3_OUTPUT_MAX_HEIGHT);
> + } else {
> + fmt->format.width = clamp(fmt->format.width,
> +   IPU3_INPUT_MIN_WIDTH,
> +   IPU3_INPUT_MAX_WIDTH);
> + fmt->format.height = clamp(fmt->format.height,
> +IPU3_INPUT_MIN_HEIGHT,
> +IPU3_INPUT_MAX_HEIGHT);
> + }
> +
> + *mf = fmt->format;
> +
> + return 0;
> +}
> +
> +/ media_entity_operations /
> +
> +static int ipu3_link_setup(struct media_entity *entity,
> +const struct media_pad *local,
> +const struct media_pad *remote, u32 flags)
> +{
> + struct ipu3_mem2mem2_device *m2m2 =
> + container_of(entity, struct ipu3_mem2mem2_device, subdev.entity);
> + u32 pad = local->index;
> +
> + WARN_ON(pad >= m2m2->num_nodes);
> +
> + m2m2->nodes[pad].enabled = !!(flags & MEDIA_LNK_FL_ENABLED);
> +
> + return 0;
> +}
> +
> +/ vb2_ops /
> +
> +static int ipu3_vb2_buf_init(struct vb2_buffer *vb)
> +{
> + struct sg_table *sg = vb2_dma_sg_plane_desc(vb, 0);
> + struct ipu3_mem2mem2_device *m2m2 = vb2_get_drv_priv(vb->vb2_queue);
> + struct imgu_device *imgu =
> + container_of(m2m2, struct imgu_device, mem2mem2);
> + struct imgu_buffer *buf = container_of(vb,
> +  

[PATCH v4 11/12] intel-ipu3: Add imgu v4l2 driver

2017-10-17 Thread Yong Zhi
ipu3 imgu video device based on v4l2, vb2 and
media controller framework.

Signed-off-by: Yong Zhi 
Signed-off-by: Ramya Vijaykumar 
---
 drivers/media/pci/intel/ipu3/ipu3-v4l2.c | 1150 ++
 1 file changed, 1150 insertions(+)
 create mode 100644 drivers/media/pci/intel/ipu3/ipu3-v4l2.c

diff --git a/drivers/media/pci/intel/ipu3/ipu3-v4l2.c 
b/drivers/media/pci/intel/ipu3/ipu3-v4l2.c
new file mode 100644
index ..4618880b8675
--- /dev/null
+++ b/drivers/media/pci/intel/ipu3/ipu3-v4l2.c
@@ -0,0 +1,1150 @@
+/*
+ * Copyright (c) 2017 Intel Corporation.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License version
+ * 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#include 
+#include 
+#include 
+#include 
+
+#include "ipu3.h"
+#include "ipu3-dmamap.h"
+
+/ v4l2_subdev_ops /
+
+static int ipu3_subdev_s_stream(struct v4l2_subdev *sd, int enable)
+{
+   struct ipu3_mem2mem2_device *m2m2 =
+   container_of(sd, struct ipu3_mem2mem2_device, subdev);
+   int r = 0;
+
+   if (m2m2->ops && m2m2->ops->s_stream)
+   r = m2m2->ops->s_stream(m2m2, enable);
+
+   if (!r)
+   m2m2->streaming = enable;
+
+   return r;
+}
+
+static int ipu3_subdev_get_fmt(struct v4l2_subdev *sd,
+  struct v4l2_subdev_pad_config *cfg,
+  struct v4l2_subdev_format *fmt)
+{
+   struct ipu3_mem2mem2_device *m2m2 =
+   container_of(sd, struct ipu3_mem2mem2_device, subdev);
+   struct v4l2_mbus_framefmt *mf;
+   u32 pad = fmt->pad;
+
+   if (fmt->which == V4L2_SUBDEV_FORMAT_ACTIVE) {
+   fmt->format = m2m2->nodes[pad].pad_fmt;
+   } else {
+   mf = v4l2_subdev_get_try_format(sd, cfg, pad);
+   fmt->format = *mf;
+   }
+
+   return 0;
+}
+
+static int ipu3_subdev_set_fmt(struct v4l2_subdev *sd,
+  struct v4l2_subdev_pad_config *cfg,
+  struct v4l2_subdev_format *fmt)
+{
+   struct ipu3_mem2mem2_device *m2m2 =
+   container_of(sd, struct ipu3_mem2mem2_device, subdev);
+   struct v4l2_mbus_framefmt *mf;
+   u32 pad = fmt->pad;
+
+   if (fmt->which == V4L2_SUBDEV_FORMAT_TRY)
+   mf = v4l2_subdev_get_try_format(sd, cfg, pad);
+   else
+   mf = >nodes[pad].pad_fmt;
+
+   /* Clamp the w and h based on the hardware capabilities */
+   if (m2m2->subdev_pads[pad].flags & MEDIA_PAD_FL_SOURCE) {
+   fmt->format.width = clamp(fmt->format.width,
+ IPU3_OUTPUT_MIN_WIDTH,
+ IPU3_OUTPUT_MAX_WIDTH);
+   fmt->format.height = clamp(fmt->format.height,
+  IPU3_OUTPUT_MIN_HEIGHT,
+  IPU3_OUTPUT_MAX_HEIGHT);
+   } else {
+   fmt->format.width = clamp(fmt->format.width,
+ IPU3_INPUT_MIN_WIDTH,
+ IPU3_INPUT_MAX_WIDTH);
+   fmt->format.height = clamp(fmt->format.height,
+  IPU3_INPUT_MIN_HEIGHT,
+  IPU3_INPUT_MAX_HEIGHT);
+   }
+
+   *mf = fmt->format;
+
+   return 0;
+}
+
+/ media_entity_operations /
+
+static int ipu3_link_setup(struct media_entity *entity,
+  const struct media_pad *local,
+  const struct media_pad *remote, u32 flags)
+{
+   struct ipu3_mem2mem2_device *m2m2 =
+   container_of(entity, struct ipu3_mem2mem2_device, subdev.entity);
+   u32 pad = local->index;
+
+   WARN_ON(pad >= m2m2->num_nodes);
+
+   m2m2->nodes[pad].enabled = !!(flags & MEDIA_LNK_FL_ENABLED);
+
+   return 0;
+}
+
+/ vb2_ops /
+
+static int ipu3_vb2_buf_init(struct vb2_buffer *vb)
+{
+   struct sg_table *sg = vb2_dma_sg_plane_desc(vb, 0);
+   struct ipu3_mem2mem2_device *m2m2 = vb2_get_drv_priv(vb->vb2_queue);
+   struct imgu_device *imgu =
+   container_of(m2m2, struct imgu_device, mem2mem2);
+   struct imgu_buffer *buf = container_of(vb,
+   struct imgu_buffer, m2m2_buf.vbb.vb2_buf);
+   struct imgu_video_device *node =
+   container_of(vb->vb2_queue, struct imgu_video_device, vbq);
+   int queue = imgu_node_to_queue(node -