lkml.org 
[lkml]   [2017]   [Jan]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH v3 17/24] media: imx: Add CSI subdev driver
From
Date
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_longerbeam@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

\
 
 \ /
  Last update: 2017-01-16 16:04    [W:0.549 / U:2.160 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site