On Fri, 2017-01-06 at 18:11 -0800, Steve Longerbeam wrote:
> This is a media entity subdevice for the i.MX Camera
> Serial Interface module.

s/Serial/Sensor/

> Signed-off-by: Steve Longerbeam <steve_longerb...@mentor.com>
> ---
>  drivers/staging/media/imx/Kconfig   |  13 +
>  drivers/staging/media/imx/Makefile  |   2 +
>  drivers/staging/media/imx/imx-csi.c | 644 
> ++++++++++++++++++++++++++++++++++++
>  3 files changed, 659 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

s/CAMERA/CSI/ ?

> +     tristate "i.MX5/6 Camera driver"

i.MX5/6 Camera Sensor Interface 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 0000000..64ef862
> --- /dev/null
> +++ b/drivers/staging/media/imx/imx-csi.c
> @@ -0,0 +1,644 @@
> +/*
> + * 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 <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <media/v4l2-ctrls.h>
> +#include <media/v4l2-device.h>
> +#include <media/v4l2-of.h>
> +#include <media/v4l2-subdev.h>
> +#include <media/videobuf2-dma-contig.h>
> +#include <video/imx-ipu-v3.h>
> +#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;

src_sd is not used except that its presence marks an enabled input link.
-> could be changed to bool.

> +     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, &priv->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;

That should be SEQ_TB/BT depending on video standard.

> +     ipu_csi_init_interface(priv->csi, &priv->sensor_mbus_cfg, &infmt);
> +
> +     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(&priv->sd, "no sensor attached\n");
> +             return -EINVAL;
> +     }
> +
> +     ret = csi_setup(priv);
> +     if (ret)
> +             return ret;
> +
> +     /* start the frame interval monitor */
> +     if (priv->fim) {
> +             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(&priv->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 void csi_stop(struct csi_priv *priv)
> +{
> +     /* stop the frame interval monitor */
> +     if (priv->fim)
> +             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");

These could be silenced a bit.

[...]
> +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 (priv->fim && on != priv->power_on)
> +             ret = imx_media_fim_set_power(priv->fim, on);
> +
> +     if (!ret)
> +             priv->power_on = on;
> +     return ret;
> +}

Is this called multiple times? I'd expect a poweron during open and a
poweroff during close, so no need for priv->power_on.

> +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;
> +     }
> +
> +     if (flags & MEDIA_LNK_FL_ENABLED) {
> +             if (priv->sink_sd)
> +                     return -EBUSY;
> +             priv->sink_sd = remote_sd;
> +     } else {
> +             priv->sink_sd = NULL;
> +             return 0;
> +     }
> +
> +     /* set CSI destination */
> +     switch (remote_sd->grp_id) {
> +     case IMX_MEDIA_GRP_ID_SMFC0:
> +     case IMX_MEDIA_GRP_ID_SMFC1:
> +     case IMX_MEDIA_GRP_ID_SMFC2:
> +     case IMX_MEDIA_GRP_ID_SMFC3:

With removal of the SMFC entities, CSI0 could be fixed to SMFC0 and CSI1
to the SMFC2 channel.

[...]
> +static int csi_set_fmt(struct v4l2_subdev *sd,
> +                    struct v4l2_subdev_pad_config *cfg,
> +                    struct v4l2_subdev_format *sdformat)
> +{
> +     struct csi_priv *priv = v4l2_get_subdevdata(sd);
> +     struct v4l2_mbus_framefmt *infmt, *outfmt;
> +     struct v4l2_rect crop;
> +     int ret;
> +
> +     if (sdformat->pad >= CSI_NUM_PADS)
> +             return -EINVAL;
> +
> +     if (priv->stream_on)
> +             return -EBUSY;
> +
> +     infmt = &priv->format_mbus[priv->input_pad];
> +     outfmt = &priv->format_mbus[priv->output_pad];
> +
> +     if (sdformat->pad == priv->output_pad) {
> +             sdformat->format.code = infmt->code;
> +             sdformat->format.field = infmt->field;
> +             crop.left = priv->crop.left;
> +             crop.top = priv->crop.top;
> +             crop.width = sdformat->format.width;
> +             crop.height = sdformat->format.height;
> +             ret = csi_try_crop(priv, &crop);
> +             if (ret)
> +                     return ret;
> +             sdformat->format.width = crop.width;
> +             sdformat->format.height = crop.height;
> +     }
> +
> +     if (sdformat->which == V4L2_SUBDEV_FORMAT_TRY) {

Should there be some limitations on the format here?

regards
Philipp

_______________________________________________
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Reply via email to