Re: [PATCH v2 11/19] media: imx: Add CSI subdev driver

2017-01-06 Thread Steve Longerbeam 



On 01/04/2017 05:44 AM, Vladimir Zapolskiy wrote:



diff --git a/drivers/staging/media/imx/imx-csi.c 
b/drivers/staging/media/imx/imx-csi.c
new file mode 100644
index 000..975eafb
--- /dev/null
+++ b/drivers/staging/media/imx/imx-csi.c
@@ -0,0 +1,638 @@
+/*
+ * V4L2 Capture CSI Subdev for Freescale i.MX5/6 SOC
+ *
+ * Copyright (c) 2014-2016 Mentor Graphics Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 

Please add the headers alphabetically ordered.


done.


+
+static int csi_start(struct csi_priv *priv)
+{
+   int ret;
+
+   if (!priv->sensor) {
+   v4l2_err(>sd, "no sensor attached\n");
+   return -EINVAL;
+   }
+
+   ret = csi_setup(priv);
+   if (ret)
+   return ret;
+
+   /* start the frame interval monitor */
+   ret = imx_media_fim_set_stream(priv->fim, priv->sensor, true);
+   if (ret)
+   return ret;
+
+   ret = ipu_csi_enable(priv->csi);
+   if (ret) {
+   v4l2_err(>sd, "CSI enable error: %d\n", ret);
+   return ret;
+   }
+
+   return 0;

if (ret)
v4l2_err(>sd, "CSI enable error: %d\n", ret);

return ret;


I failed to cleanup in this path, so it is now:

ret = ipu_csi_enable(priv->csi);
if (ret) {
v4l2_err(>sd, "CSI enable error: %d\n", ret);
goto fim_off;
}

return 0;
fim_off:
if (priv->fim)
imx_media_fim_set_stream(priv->fim, priv->sensor, false);
return ret;


+
+static int csi_link_setup(struct media_entity *entity,
+ const struct media_pad *local,
+ const struct media_pad *remote, u32 flags)
+{
+   struct v4l2_subdev *sd = media_entity_to_v4l2_subdev(entity);
+   struct csi_priv *priv = v4l2_get_subdevdata(sd);
+   struct v4l2_subdev *remote_sd;
+
+   dev_dbg(priv->dev, "link setup %s -> %s", remote->entity->name,
+   local->entity->name);
+
+   remote_sd = media_entity_to_v4l2_subdev(remote->entity);
+
+   if (local->flags & MEDIA_PAD_FL_SINK) {
+   if (flags & MEDIA_LNK_FL_ENABLED) {
+   if (priv->src_sd)
+   return -EBUSY;
+   priv->src_sd = remote_sd;
+   } else {
+   priv->src_sd = NULL;
+   return 0;

You can remove the return above.


right, fixed.




+
+   ret = v4l2_async_register_subdev(>sd);
+   if (ret)
+   goto free_ctrls;
+
+   return 0;
+free_ctrls:
+   v4l2_ctrl_handler_free(>ctrl_hdlr);
+   return ret;

This is a functionally equal and simplified version:

if (ret)
v4l2_ctrl_handler_free(>ctrl_hdlr);

return ret;


thanks, done.


+
+static struct platform_driver imx_csi_driver = {
+   .probe = imx_csi_probe,
+   .remove = imx_csi_remove,
+   .id_table = imx_csi_ids,
+   .driver = {
+   .name = "imx-ipuv3-csi",
+   .owner = THIS_MODULE,

Please drop .owner.


ok, I tested this and there are no regressions, done
for all modules.

Steve

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 11/19] media: imx: Add CSI subdev driver

2017-01-04 Thread Vladimir Zapolskiy 
On 01/03/2017 10:57 PM, Steve Longerbeam wrote:
> This is a media entity subdevice for the i.MX Camera
> Serial Interface module.
> 
> Signed-off-by: Steve Longerbeam 
> ---

[snip]

> diff --git a/drivers/staging/media/imx/imx-csi.c 
> b/drivers/staging/media/imx/imx-csi.c
> new file mode 100644
> index 000..975eafb
> --- /dev/null
> +++ b/drivers/staging/media/imx/imx-csi.c
> @@ -0,0 +1,638 @@
> +/*
> + * V4L2 Capture CSI Subdev for Freescale i.MX5/6 SOC
> + *
> + * Copyright (c) 2014-2016 Mentor Graphics Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + */
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 

Please add the headers alphabetically ordered.

> +#include 
> +#include "imx-media.h"
> +
> +#define CSI_NUM_PADS 2
> +
> +struct csi_priv {
> + struct device *dev;
> + struct ipu_soc *ipu;
> + struct imx_media_dev *md;
> + struct v4l2_subdev sd;
> + struct media_pad pad[CSI_NUM_PADS];
> + struct v4l2_mbus_framefmt format_mbus[CSI_NUM_PADS];
> + struct v4l2_mbus_config sensor_mbus_cfg;
> + struct v4l2_rect crop;
> + struct ipu_csi *csi;
> + int csi_id;
> + int input_pad;
> + int output_pad;
> + bool power_on;  /* power is on */
> + bool stream_on; /* streaming is on */
> +
> + /* the sink for the captured frames */
> + struct v4l2_subdev *sink_sd;
> + enum ipu_csi_dest dest;
> + struct v4l2_subdev *src_sd;
> +
> + struct v4l2_ctrl_handler ctrl_hdlr;
> + struct imx_media_fim *fim;
> +
> + /* the attached sensor at stream on */
> + struct imx_media_subdev *sensor;
> +};
> +
> +static inline struct csi_priv *sd_to_dev(struct v4l2_subdev *sdev)
> +{
> + return container_of(sdev, struct csi_priv, sd);
> +}
> +
> +/* Update the CSI whole sensor and active windows */
> +static int csi_setup(struct csi_priv *priv)
> +{
> + struct v4l2_mbus_framefmt infmt;
> +
> + ipu_csi_set_window(priv->csi, >crop);
> +
> + /*
> +  * the ipu-csi doesn't understand ALTERNATE, but it only
> +  * needs to know whether the stream is interlaced, so set
> +  * to INTERLACED if infmt field is ALTERNATE.
> +  */
> + infmt = priv->format_mbus[priv->input_pad];
> + if (infmt.field == V4L2_FIELD_ALTERNATE)
> + infmt.field = V4L2_FIELD_INTERLACED;
> +
> + ipu_csi_init_interface(priv->csi, >sensor_mbus_cfg, );
> +
> + ipu_csi_set_dest(priv->csi, priv->dest);
> +
> + ipu_csi_dump(priv->csi);
> +
> + return 0;
> +}
> +
> +static int csi_start(struct csi_priv *priv)
> +{
> + int ret;
> +
> + if (!priv->sensor) {
> + v4l2_err(>sd, "no sensor attached\n");
> + return -EINVAL;
> + }
> +
> + ret = csi_setup(priv);
> + if (ret)
> + return ret;
> +
> + /* start the frame interval monitor */
> + ret = imx_media_fim_set_stream(priv->fim, priv->sensor, true);
> + if (ret)
> + return ret;
> +
> + ret = ipu_csi_enable(priv->csi);
> + if (ret) {
> + v4l2_err(>sd, "CSI enable error: %d\n", ret);
> + return ret;
> + }
> +
> + return 0;

if (ret)
v4l2_err(>sd, "CSI enable error: %d\n", ret);

return ret;

> +}
> +
> +static void csi_stop(struct csi_priv *priv)
> +{
> + /* stop the frame interval monitor */
> + imx_media_fim_set_stream(priv->fim, priv->sensor, false);
> +
> + ipu_csi_disable(priv->csi);
> +}
> +
> +static int csi_s_stream(struct v4l2_subdev *sd, int enable)
> +{
> + struct csi_priv *priv = v4l2_get_subdevdata(sd);
> + int ret = 0;
> +
> + if (!priv->src_sd || !priv->sink_sd)
> + return -EPIPE;
> +
> + v4l2_info(sd, "stream %s\n", enable ? "ON" : "OFF");
> +
> + if (enable && !priv->stream_on)
> + ret = csi_start(priv);
> + else if (!enable && priv->stream_on)
> + csi_stop(priv);
> +
> + if (!ret)
> + priv->stream_on = enable;
> + return ret;
> +}
> +
> +static int csi_s_power(struct v4l2_subdev *sd, int on)
> +{
> + struct csi_priv *priv = v4l2_get_subdevdata(sd);
> + int ret = 0;
> +
> + v4l2_info(sd, "power %s\n", on ? "ON" : "OFF");
> +
> + if (on != priv->power_on)
> + ret = imx_media_fim_set_power(priv->fim, on);
> +
> + if (!ret)
> + priv->power_on = on;
> + return ret;
> +}
> +
> +static int csi_link_setup(struct media_entity *entity,
> +   const struct media_pad *local,
> +   const struct media_pad *remote, u32 flags)
> +{
> + struct v4l2_subdev *sd = media_entity_to_v4l2_subdev(entity);
> + struct csi_priv *priv = v4l2_get_subdevdata(sd);
> + struct v4l2_subdev

[PATCH v2 11/19] media: imx: Add CSI subdev driver

2017-01-03 Thread Steve Longerbeam 
This is a media entity subdevice for the i.MX Camera
Serial Interface module.

Signed-off-by: Steve Longerbeam 
---
 drivers/staging/media/imx/Kconfig   |  13 +
 drivers/staging/media/imx/Makefile  |   2 +
 drivers/staging/media/imx/imx-csi.c | 638 
 3 files changed, 653 insertions(+)
 create mode 100644 drivers/staging/media/imx/imx-csi.c

diff --git a/drivers/staging/media/imx/Kconfig 
b/drivers/staging/media/imx/Kconfig
index bfde58d..ce2d2c8 100644
--- a/drivers/staging/media/imx/Kconfig
+++ b/drivers/staging/media/imx/Kconfig
@@ -6,3 +6,16 @@ config VIDEO_IMX_MEDIA
  Say yes here to enable support for video4linux media controller
  driver for the i.MX5/6 SOC.
 
+if VIDEO_IMX_MEDIA
+menu "i.MX5/6 Media Sub devices"
+
+config VIDEO_IMX_CAMERA
+   tristate "i.MX5/6 Camera driver"
+   depends on VIDEO_IMX_MEDIA && VIDEO_DEV && I2C
+   select VIDEOBUF2_DMA_CONTIG
+   default y
+   ---help---
+ A video4linux camera capture driver for i.MX5/6.
+
+endmenu
+endif
diff --git a/drivers/staging/media/imx/Makefile 
b/drivers/staging/media/imx/Makefile
index ef9f11b..133672a 100644
--- a/drivers/staging/media/imx/Makefile
+++ b/drivers/staging/media/imx/Makefile
@@ -4,3 +4,5 @@ imx-media-objs := imx-media-dev.o imx-media-fim.o 
imx-media-internal-sd.o \
 obj-$(CONFIG_VIDEO_IMX_MEDIA) += imx-media.o
 obj-$(CONFIG_VIDEO_IMX_MEDIA) += imx-media-common.o
 
+obj-$(CONFIG_VIDEO_IMX_CAMERA) += imx-csi.o
+
diff --git a/drivers/staging/media/imx/imx-csi.c 
b/drivers/staging/media/imx/imx-csi.c
new file mode 100644
index 000..975eafb
--- /dev/null
+++ b/drivers/staging/media/imx/imx-csi.c
@@ -0,0 +1,638 @@
+/*
+ * V4L2 Capture CSI Subdev for Freescale i.MX5/6 SOC
+ *
+ * Copyright (c) 2014-2016 Mentor Graphics Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include "imx-media.h"
+
+#define CSI_NUM_PADS 2
+
+struct csi_priv {
+   struct device *dev;
+   struct ipu_soc *ipu;
+   struct imx_media_dev *md;
+   struct v4l2_subdev sd;
+   struct media_pad pad[CSI_NUM_PADS];
+   struct v4l2_mbus_framefmt format_mbus[CSI_NUM_PADS];
+   struct v4l2_mbus_config sensor_mbus_cfg;
+   struct v4l2_rect crop;
+   struct ipu_csi *csi;
+   int csi_id;
+   int input_pad;
+   int output_pad;
+   bool power_on;  /* power is on */
+   bool stream_on; /* streaming is on */
+
+   /* the sink for the captured frames */
+   struct v4l2_subdev *sink_sd;
+   enum ipu_csi_dest dest;
+   struct v4l2_subdev *src_sd;
+
+   struct v4l2_ctrl_handler ctrl_hdlr;
+   struct imx_media_fim *fim;
+
+   /* the attached sensor at stream on */
+   struct imx_media_subdev *sensor;
+};
+
+static inline struct csi_priv *sd_to_dev(struct v4l2_subdev *sdev)
+{
+   return container_of(sdev, struct csi_priv, sd);
+}
+
+/* Update the CSI whole sensor and active windows */
+static int csi_setup(struct csi_priv *priv)
+{
+   struct v4l2_mbus_framefmt infmt;
+
+   ipu_csi_set_window(priv->csi, >crop);
+
+   /*
+* the ipu-csi doesn't understand ALTERNATE, but it only
+* needs to know whether the stream is interlaced, so set
+* to INTERLACED if infmt field is ALTERNATE.
+*/
+   infmt = priv->format_mbus[priv->input_pad];
+   if (infmt.field == V4L2_FIELD_ALTERNATE)
+   infmt.field = V4L2_FIELD_INTERLACED;
+
+   ipu_csi_init_interface(priv->csi, >sensor_mbus_cfg, );
+
+   ipu_csi_set_dest(priv->csi, priv->dest);
+
+   ipu_csi_dump(priv->csi);
+
+   return 0;
+}
+
+static int csi_start(struct csi_priv *priv)
+{
+   int ret;
+
+   if (!priv->sensor) {
+   v4l2_err(>sd, "no sensor attached\n");
+   return -EINVAL;
+   }
+
+   ret = csi_setup(priv);
+   if (ret)
+   return ret;
+
+   /* start the frame interval monitor */
+   ret = imx_media_fim_set_stream(priv->fim, priv->sensor, true);
+   if (ret)
+   return ret;
+
+   ret = ipu_csi_enable(priv->csi);
+   if (ret) {
+   v4l2_err(>sd, "CSI enable error: %d\n", ret);
+   return ret;
+   }
+
+   return 0;
+}
+
+static void csi_stop(struct csi_priv *priv)
+{
+   /* stop the frame interval monitor */
+   imx_media_fim_set_stream(priv->fim, priv->sensor, false);
+
+   ipu_csi_disable(priv->csi);
+}
+
+static int csi_s_stream(struct v4l2_subdev *sd, int enable)
+{
+   struct csi_priv *priv = v4l2_get_subdevdata(sd);
+   int ret = 0;
+
+   if (!priv->src_sd || !priv->sink_sd)
+   return -EPIPE;
+
+