Re: [PATCH v3] [media] vimc: Virtual Media Controller core, capture and sensor

2016-05-24 Thread Helen Koike 

Hi Jeremy,

On 24-05-2016 21:00, Jeremy Gebben wrote:

Helen,

I am more of a v4l2 newb than a reviewer, but I got your driver working
on a qemu arm64 system. Using it to play with mediactl -p was
a good way to get started.

I did have 2 minor include path problems. Maybe they come in implicitly
on other architectures? See below:

On 4/27/16 10:33 AM, Helen Fornazier wrote:




diff --git a/drivers/media/platform/vimc/vimc-core.h
b/drivers/media/platform/vimc/vimc-core.h
new file mode 100644
index 000..be05469
--- /dev/null
+++ b/drivers/media/platform/vimc/vimc-core.h
@@ -0,0 +1,55 @@
+/*
+ * vimc-core.h Virtual Media Controller Driver
+ *
+ * Copyright (C) 2015 Helen Fornazier 
+ *
+ * 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.
+ *
+ * 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.
+ *
+ */
+
+#ifndef _VIMC_CORE_H_
+#define _VIMC_CORE_H_
+
+#include 
+
+/* Struct which matches the MEDIA_BUS_FMT_ codes with the corresponding
+ * V4L2_PIX_FMT_ fourcc pixelformat and its bytes per pixel (bpp) */
+struct vimc_pix_map {
+   unsigned int code;
+   unsigned int bpp;
+   u32 pixelformat;
+};
+extern const struct vimc_pix_map vimc_pix_map_list[];
+
+struct vimc_ent_device {
+   struct media_entity *ent;
+   struct media_pad *pads;
+   void (*destroy)(struct vimc_ent_device *);
+   void (*process_frame)(struct vimc_ent_device *ved,
+ struct media_pad *sink, const void
*frame);
+};
+
+int vimc_propagate_frame(struct device *dev,
+struct media_pad *src, const void *frame);
+
+/* Helper functions to allocate/initialize pads and free them */
+struct media_pad *vimc_pads_init(u16 num_pads,
+const unsigned long *pads_flag);
+static inline void vimc_pads_cleanup(struct media_pad *pads)
+{
+   kfree(pads);


I needed  to be included in this file, so that kfree() was
defined.




+}
+
+const struct vimc_pix_map *vimc_pix_map_by_code(u32 code);
+
+const struct vimc_pix_map *vimc_pix_map_by_pixelformat(u32
pixelformat);
+
+#endif
diff --git a/drivers/media/platform/vimc/vimc-sensor.c
b/drivers/media/platform/vimc/vimc-sensor.c
new file mode 100644
index 000..ae70b9e
--- /dev/null
+++ b/drivers/media/platform/vimc/vimc-sensor.c
@@ -0,0 +1,277 @@
+/*
+ * vimc-sensor.c Virtual Media Controller Driver
+ *
+ * Copyright (C) 2015 Helen Fornazier 
+ *
+ * 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.
+ *
+ * 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 "vimc-sensor.h"
+
+struct vimc_sen_device {
+   struct vimc_ent_device ved;
+   struct v4l2_subdev sd;
+   struct v4l2_device *v4l2_dev;
+   struct device *dev;
+   struct task_struct *kthread_sen;
+   u8 *frame;
+   /* The active format */
+   struct v4l2_mbus_framefmt mbus_format;
+   int frame_size;
+};
+
+static int vimc_sen_enum_mbus_code(struct v4l2_subdev *sd,
+  struct v4l2_subdev_pad_config *cfg,
+  struct v4l2_subdev_mbus_code_enum
*code)
+{
+   struct vimc_sen_device *vsen = v4l2_get_subdevdata(sd);
+
+   /* Check if it is a valid pad */
+   if (code->pad >= vsen->sd.entity.num_pads)
+   return -EINVAL;
+
+   code->code = vsen->mbus_format.code;
+
+   return 0;
+}
+
+static int vimc_sen_enum_frame_size(struct v4l2_subdev *sd,
+   struct v4l2_subdev_pad_config *cfg,
+   struct
v4l2_subdev_frame_size_enum *fse)
+{
+   struct vimc_sen_device *vsen = v4l2_get_subdevdata(sd);
+
+   /* Check if it is a valid pad */
+   if (fse->pad >= vsen->sd.entity.num_pads ||
+   !(vsen->sd.entity.pads[fse->pad].flags &
MEDIA_PAD_FL_SOURCE))
+   return -EINVAL;
+
+   /* TODO: Add support to other formats */
+   if (fse->index)
+   return -EINVAL;
+
+   /* TODO: Add support for other codes */
+   if (fse->code != vsen->mbus_format.code)
+   return -EINVAL;
+
+   fse->min_width =

Re: [PATCH v3] [media] vimc: Virtual Media Controller core, capture and sensor

2016-05-24 Thread Jeremy Gebben 

Helen,

I am more of a v4l2 newb than a reviewer, but I got your driver working 
on a qemu arm64 system. Using it to play with mediactl -p was

a good way to get started.

I did have 2 minor include path problems. Maybe they come in implicitly 
on other architectures? See below:


On 4/27/16 10:33 AM, Helen Fornazier wrote:




diff --git a/drivers/media/platform/vimc/vimc-core.h 
b/drivers/media/platform/vimc/vimc-core.h
new file mode 100644
index 000..be05469
--- /dev/null
+++ b/drivers/media/platform/vimc/vimc-core.h
@@ -0,0 +1,55 @@
+/*
+ * vimc-core.h Virtual Media Controller Driver
+ *
+ * Copyright (C) 2015 Helen Fornazier 
+ *
+ * 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.
+ *
+ * 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.
+ *
+ */
+
+#ifndef _VIMC_CORE_H_
+#define _VIMC_CORE_H_
+
+#include 
+
+/* Struct which matches the MEDIA_BUS_FMT_ codes with the corresponding
+ * V4L2_PIX_FMT_ fourcc pixelformat and its bytes per pixel (bpp) */
+struct vimc_pix_map {
+   unsigned int code;
+   unsigned int bpp;
+   u32 pixelformat;
+};
+extern const struct vimc_pix_map vimc_pix_map_list[];
+
+struct vimc_ent_device {
+   struct media_entity *ent;
+   struct media_pad *pads;
+   void (*destroy)(struct vimc_ent_device *);
+   void (*process_frame)(struct vimc_ent_device *ved,
+ struct media_pad *sink, const void *frame);
+};
+
+int vimc_propagate_frame(struct device *dev,
+struct media_pad *src, const void *frame);
+
+/* Helper functions to allocate/initialize pads and free them */
+struct media_pad *vimc_pads_init(u16 num_pads,
+const unsigned long *pads_flag);
+static inline void vimc_pads_cleanup(struct media_pad *pads)
+{
+   kfree(pads);


I needed  to be included in this file, so that kfree() was 
defined.





+}
+
+const struct vimc_pix_map *vimc_pix_map_by_code(u32 code);
+
+const struct vimc_pix_map *vimc_pix_map_by_pixelformat(u32 pixelformat);
+
+#endif
diff --git a/drivers/media/platform/vimc/vimc-sensor.c 
b/drivers/media/platform/vimc/vimc-sensor.c
new file mode 100644
index 000..ae70b9e
--- /dev/null
+++ b/drivers/media/platform/vimc/vimc-sensor.c
@@ -0,0 +1,277 @@
+/*
+ * vimc-sensor.c Virtual Media Controller Driver
+ *
+ * Copyright (C) 2015 Helen Fornazier 
+ *
+ * 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.
+ *
+ * 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 "vimc-sensor.h"
+
+struct vimc_sen_device {
+   struct vimc_ent_device ved;
+   struct v4l2_subdev sd;
+   struct v4l2_device *v4l2_dev;
+   struct device *dev;
+   struct task_struct *kthread_sen;
+   u8 *frame;
+   /* The active format */
+   struct v4l2_mbus_framefmt mbus_format;
+   int frame_size;
+};
+
+static int vimc_sen_enum_mbus_code(struct v4l2_subdev *sd,
+  struct v4l2_subdev_pad_config *cfg,
+  struct v4l2_subdev_mbus_code_enum *code)
+{
+   struct vimc_sen_device *vsen = v4l2_get_subdevdata(sd);
+
+   /* Check if it is a valid pad */
+   if (code->pad >= vsen->sd.entity.num_pads)
+   return -EINVAL;
+
+   code->code = vsen->mbus_format.code;
+
+   return 0;
+}
+
+static int vimc_sen_enum_frame_size(struct v4l2_subdev *sd,
+   struct v4l2_subdev_pad_config *cfg,
+   struct v4l2_subdev_frame_size_enum *fse)
+{
+   struct vimc_sen_device *vsen = v4l2_get_subdevdata(sd);
+
+   /* Check if it is a valid pad */
+   if (fse->pad >= vsen->sd.entity.num_pads ||
+   !(vsen->sd.entity.pads[fse->pad].flags & MEDIA_PAD_FL_SOURCE))
+   return -EINVAL;
+
+   /* TODO: Add support to other formats */
+   if (fse->index)
+   return -EINVAL;
+
+   /* TODO: Add support for other codes */
+   if (fse->code != vsen->mbus_format.code)
+   return -EINVAL;
+
+   fse->min_width = vsen->mbus_format.width;
+   fse->max_width =

Re: [PATCH v3] [media] vimc: Virtual Media Controller core, capture and sensor

2016-04-27 Thread Helen Fornazier 
Hi,

On Wed, Apr 6, 2016 at 5:20 PM, Helen Mae Koike Fornazier
 wrote:
> From: Helen Fornazier 
>
> First version of the Virtual Media Controller.
> Add a simple version of the core of the driver, the capture and
> sensor nodes in the topology, generating a grey image in a hardcoded
> format.
>
> Signed-off-by: Helen Fornazier 
> ---
>
> Changes since v2: update with current media master tree
> - Add struct media_pipeline in vimc_cap_device
> - Use vb2_v4l2_buffer instead of vb2_buffer
> - Typos
> - Remove usage of entity->type and use entity->function instead
> - Remove fmt argument from queue setup
> - Use ktime_get_ns instead of v4l2_get_timestamp
> - Iterate over link's list using list_for_each_entry
> - Use media_device_{init, cleanup}
> - Use entity->use_count to keep track of entities instead of the old
> entity->id
> - Replace media_entity_init by media_entity_pads_init
>
>  drivers/media/platform/Kconfig |   2 +
>  drivers/media/platform/Makefile|   1 +
>  drivers/media/platform/vimc/Kconfig|   6 +
>  drivers/media/platform/vimc/Makefile   |   3 +
>  drivers/media/platform/vimc/vimc-capture.c | 534 ++
>  drivers/media/platform/vimc/vimc-capture.h |  28 ++
>  drivers/media/platform/vimc/vimc-core.c| 595 
> +
>  drivers/media/platform/vimc/vimc-core.h|  55 +++
>  drivers/media/platform/vimc/vimc-sensor.c  | 277 ++
>  drivers/media/platform/vimc/vimc-sensor.h  |  28 ++
>  10 files changed, 1529 insertions(+)
>  create mode 100644 drivers/media/platform/vimc/Kconfig
>  create mode 100644 drivers/media/platform/vimc/Makefile
>  create mode 100644 drivers/media/platform/vimc/vimc-capture.c
>  create mode 100644 drivers/media/platform/vimc/vimc-capture.h
>  create mode 100644 drivers/media/platform/vimc/vimc-core.c
>  create mode 100644 drivers/media/platform/vimc/vimc-core.h
>  create mode 100644 drivers/media/platform/vimc/vimc-sensor.c
>  create mode 100644 drivers/media/platform/vimc/vimc-sensor.h
>
> diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
> index 201f5c2..14ed03f 100644
> --- a/drivers/media/platform/Kconfig
> +++ b/drivers/media/platform/Kconfig
> @@ -284,6 +284,8 @@ menuconfig V4L_TEST_DRIVERS
>
>  if V4L_TEST_DRIVERS
>
> +source "drivers/media/platform/vimc/Kconfig"
> +
>  source "drivers/media/platform/vivid/Kconfig"
>
>  config VIDEO_VIM2M
> diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile
> index bbb7bd1..e4508fe 100644
> --- a/drivers/media/platform/Makefile
> +++ b/drivers/media/platform/Makefile
> @@ -12,6 +12,7 @@ obj-$(CONFIG_VIDEO_OMAP3) += omap3isp/
>
>  obj-$(CONFIG_VIDEO_VIU) += fsl-viu.o
>
> +obj-$(CONFIG_VIDEO_VIMC)   += vimc/
>  obj-$(CONFIG_VIDEO_VIVID)  += vivid/
>  obj-$(CONFIG_VIDEO_VIM2M)  += vim2m.o
>
> diff --git a/drivers/media/platform/vimc/Kconfig 
> b/drivers/media/platform/vimc/Kconfig
> new file mode 100644
> index 000..81279f4
> --- /dev/null
> +++ b/drivers/media/platform/vimc/Kconfig
> @@ -0,0 +1,6 @@
> +config VIDEO_VIMC
> +   tristate "Virtual Media Controller Driver (VIMC)"
> +   select VIDEO_V4L2_SUBDEV_API
> +   default n
> +   ---help---
> + Skeleton driver for Virtual Media Controller
> diff --git a/drivers/media/platform/vimc/Makefile 
> b/drivers/media/platform/vimc/Makefile
> new file mode 100644
> index 000..c45195e
> --- /dev/null
> +++ b/drivers/media/platform/vimc/Makefile
> @@ -0,0 +1,3 @@
> +vimc-objs := vimc-core.o vimc-capture.o vimc-sensor.o
> +
> +obj-$(CONFIG_VIDEO_VIMC) += vimc.o
> diff --git a/drivers/media/platform/vimc/vimc-capture.c 
> b/drivers/media/platform/vimc/vimc-capture.c
> new file mode 100644
> index 000..3fb8bfe
> --- /dev/null
> +++ b/drivers/media/platform/vimc/vimc-capture.c
> @@ -0,0 +1,534 @@
> +/*
> + * vimc-capture.c Virtual Media Controller Driver
> + *
> + * Copyright (C) 2015 Helen Fornazier 
> + *
> + * 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.
> + *
> + * 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 "vimc-capture.h"
> +
> +struct vimc_cap_device {
> +   struct vimc_ent_device ved;
> +   struct video_device vdev;
> +   struct v4l2_device *v4l2_dev;
> +   struct device *dev;
> +   struct v4l2_pix_format format;
> +   struct vb2_queue queue;
> +

[PATCH v3] [media] vimc: Virtual Media Controller core, capture and sensor

2016-04-06 Thread Helen Mae Koike Fornazier 
From: Helen Fornazier 

First version of the Virtual Media Controller.
Add a simple version of the core of the driver, the capture and
sensor nodes in the topology, generating a grey image in a hardcoded
format.

Signed-off-by: Helen Fornazier 
---

Changes since v2: update with current media master tree
- Add struct media_pipeline in vimc_cap_device
- Use vb2_v4l2_buffer instead of vb2_buffer
- Typos
- Remove usage of entity->type and use entity->function instead
- Remove fmt argument from queue setup
- Use ktime_get_ns instead of v4l2_get_timestamp
- Iterate over link's list using list_for_each_entry
- Use media_device_{init, cleanup}
- Use entity->use_count to keep track of entities instead of the old
entity->id
- Replace media_entity_init by media_entity_pads_init

 drivers/media/platform/Kconfig |   2 +
 drivers/media/platform/Makefile|   1 +
 drivers/media/platform/vimc/Kconfig|   6 +
 drivers/media/platform/vimc/Makefile   |   3 +
 drivers/media/platform/vimc/vimc-capture.c | 534 ++
 drivers/media/platform/vimc/vimc-capture.h |  28 ++
 drivers/media/platform/vimc/vimc-core.c| 595 +
 drivers/media/platform/vimc/vimc-core.h|  55 +++
 drivers/media/platform/vimc/vimc-sensor.c  | 277 ++
 drivers/media/platform/vimc/vimc-sensor.h  |  28 ++
 10 files changed, 1529 insertions(+)
 create mode 100644 drivers/media/platform/vimc/Kconfig
 create mode 100644 drivers/media/platform/vimc/Makefile
 create mode 100644 drivers/media/platform/vimc/vimc-capture.c
 create mode 100644 drivers/media/platform/vimc/vimc-capture.h
 create mode 100644 drivers/media/platform/vimc/vimc-core.c
 create mode 100644 drivers/media/platform/vimc/vimc-core.h
 create mode 100644 drivers/media/platform/vimc/vimc-sensor.c
 create mode 100644 drivers/media/platform/vimc/vimc-sensor.h

diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
index 201f5c2..14ed03f 100644
--- a/drivers/media/platform/Kconfig
+++ b/drivers/media/platform/Kconfig
@@ -284,6 +284,8 @@ menuconfig V4L_TEST_DRIVERS
 
 if V4L_TEST_DRIVERS
 
+source "drivers/media/platform/vimc/Kconfig"
+
 source "drivers/media/platform/vivid/Kconfig"
 
 config VIDEO_VIM2M
diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile
index bbb7bd1..e4508fe 100644
--- a/drivers/media/platform/Makefile
+++ b/drivers/media/platform/Makefile
@@ -12,6 +12,7 @@ obj-$(CONFIG_VIDEO_OMAP3) += omap3isp/
 
 obj-$(CONFIG_VIDEO_VIU) += fsl-viu.o
 
+obj-$(CONFIG_VIDEO_VIMC)   += vimc/
 obj-$(CONFIG_VIDEO_VIVID)  += vivid/
 obj-$(CONFIG_VIDEO_VIM2M)  += vim2m.o
 
diff --git a/drivers/media/platform/vimc/Kconfig 
b/drivers/media/platform/vimc/Kconfig
new file mode 100644
index 000..81279f4
--- /dev/null
+++ b/drivers/media/platform/vimc/Kconfig
@@ -0,0 +1,6 @@
+config VIDEO_VIMC
+   tristate "Virtual Media Controller Driver (VIMC)"
+   select VIDEO_V4L2_SUBDEV_API
+   default n
+   ---help---
+ Skeleton driver for Virtual Media Controller
diff --git a/drivers/media/platform/vimc/Makefile 
b/drivers/media/platform/vimc/Makefile
new file mode 100644
index 000..c45195e
--- /dev/null
+++ b/drivers/media/platform/vimc/Makefile
@@ -0,0 +1,3 @@
+vimc-objs := vimc-core.o vimc-capture.o vimc-sensor.o
+
+obj-$(CONFIG_VIDEO_VIMC) += vimc.o
diff --git a/drivers/media/platform/vimc/vimc-capture.c 
b/drivers/media/platform/vimc/vimc-capture.c
new file mode 100644
index 000..3fb8bfe
--- /dev/null
+++ b/drivers/media/platform/vimc/vimc-capture.c
@@ -0,0 +1,534 @@
+/*
+ * vimc-capture.c Virtual Media Controller Driver
+ *
+ * Copyright (C) 2015 Helen Fornazier 
+ *
+ * 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.
+ *
+ * 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 "vimc-capture.h"
+
+struct vimc_cap_device {
+   struct vimc_ent_device ved;
+   struct video_device vdev;
+   struct v4l2_device *v4l2_dev;
+   struct device *dev;
+   struct v4l2_pix_format format;
+   struct vb2_queue queue;
+   struct list_head buf_list;
+   /* NOTE: in a real driver, a spin lock must be used to access the
+* queue because the frames are generated from a hardware interruption
+* and the isr is not allowed to sleep.
+* Even if it is not necessary a spinlock in the vimc driver, we
+* use it here as a code reference */
+