* [PATCH v1 0/2] media: vimc: add ancillary lens
@ 2022-04-15 2:38 Yunke Cao
2022-04-15 2:38 ` [PATCH v1 1/2] " Yunke Cao
2022-04-15 2:38 ` [PATCH v1 2/2] media: vimc: documentation for lens Yunke Cao
0 siblings, 2 replies; 7+ messages in thread
From: Yunke Cao @ 2022-04-15 2:38 UTC (permalink / raw)
To: Kieran Bingham
Cc: Sakari Ailus, Tomasz Figa, Mauro Carvalho Chehab, Daniel Scally,
linux-media, Yunke Cao
Add a basic version of vimc lens.
Link lens with sensors using ancillary links.
Update vimc documentation to reflect this change.
*** BLURB HERE ***
Yunke Cao (2):
media: vimc: add ancillary lens
media: vimc: documentation for lens
Documentation/admin-guide/media/vimc.dot | 4 +
Documentation/admin-guide/media/vimc.rst | 3 +
drivers/media/test-drivers/vimc/Makefile | 2 +-
drivers/media/test-drivers/vimc/vimc-common.h | 1 +
drivers/media/test-drivers/vimc/vimc-core.c | 86 +++++++++++----
drivers/media/test-drivers/vimc/vimc-lens.c | 102 ++++++++++++++++++
6 files changed, 177 insertions(+), 21 deletions(-)
create mode 100644 drivers/media/test-drivers/vimc/vimc-lens.c
--
2.36.0.rc0.470.gd361397f0d-goog
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v1 1/2] media: vimc: add ancillary lens
2022-04-15 2:38 [PATCH v1 0/2] media: vimc: add ancillary lens Yunke Cao
@ 2022-04-15 2:38 ` Yunke Cao
2022-04-19 12:16 ` Kieran Bingham
2022-04-15 2:38 ` [PATCH v1 2/2] media: vimc: documentation for lens Yunke Cao
1 sibling, 1 reply; 7+ messages in thread
From: Yunke Cao @ 2022-04-15 2:38 UTC (permalink / raw)
To: Kieran Bingham
Cc: Sakari Ailus, Tomasz Figa, Mauro Carvalho Chehab, Daniel Scally,
linux-media, Yunke Cao
Add a basic version of vimc lens.
Link lens with sensors using ancillary links.
Signed-off-by: Yunke Cao <yunkec@google.com>
---
drivers/media/test-drivers/vimc/Makefile | 2 +-
drivers/media/test-drivers/vimc/vimc-common.h | 1 +
drivers/media/test-drivers/vimc/vimc-core.c | 86 +++++++++++----
drivers/media/test-drivers/vimc/vimc-lens.c | 102 ++++++++++++++++++
4 files changed, 170 insertions(+), 21 deletions(-)
create mode 100644 drivers/media/test-drivers/vimc/vimc-lens.c
diff --git a/drivers/media/test-drivers/vimc/Makefile b/drivers/media/test-drivers/vimc/Makefile
index a53b2b532e9f..9b9631562473 100644
--- a/drivers/media/test-drivers/vimc/Makefile
+++ b/drivers/media/test-drivers/vimc/Makefile
@@ -1,6 +1,6 @@
# SPDX-License-Identifier: GPL-2.0
vimc-y := vimc-core.o vimc-common.o vimc-streamer.o vimc-capture.o \
- vimc-debayer.o vimc-scaler.o vimc-sensor.o
+ vimc-debayer.o vimc-scaler.o vimc-sensor.o vimc-lens.o
obj-$(CONFIG_VIDEO_VIMC) += vimc.o
diff --git a/drivers/media/test-drivers/vimc/vimc-common.h b/drivers/media/test-drivers/vimc/vimc-common.h
index ba1930772589..37f6b687ce10 100644
--- a/drivers/media/test-drivers/vimc/vimc-common.h
+++ b/drivers/media/test-drivers/vimc/vimc-common.h
@@ -171,6 +171,7 @@ extern struct vimc_ent_type vimc_sen_type;
extern struct vimc_ent_type vimc_deb_type;
extern struct vimc_ent_type vimc_sca_type;
extern struct vimc_ent_type vimc_cap_type;
+extern struct vimc_ent_type vimc_len_type;
/**
* vimc_pix_map_by_index - get vimc_pix_map struct by its index
diff --git a/drivers/media/test-drivers/vimc/vimc-core.c b/drivers/media/test-drivers/vimc/vimc-core.c
index 06edf9d4d92c..166323406c6b 100644
--- a/drivers/media/test-drivers/vimc/vimc-core.c
+++ b/drivers/media/test-drivers/vimc/vimc-core.c
@@ -24,7 +24,7 @@ MODULE_PARM_DESC(allocator, " memory allocator selection, default is 0.\n"
#define VIMC_MDEV_MODEL_NAME "VIMC MDEV"
-#define VIMC_ENT_LINK(src, srcpad, sink, sinkpad, link_flags) { \
+#define VIMC_DATA_LINK(src, srcpad, sink, sinkpad, link_flags) { \
.src_ent = src, \
.src_pad = srcpad, \
.sink_ent = sink, \
@@ -32,8 +32,13 @@ MODULE_PARM_DESC(allocator, " memory allocator selection, default is 0.\n"
.flags = link_flags, \
}
-/* Structure which describes links between entities */
-struct vimc_ent_link {
+#define VIMC_ANCILLARY_LINK(primary, ancillary) { \
+ .primary_ent = primary, \
+ .ancillary_ent = ancillary \
+}
+
+/* Structure which describes data links between entities */
+struct vimc_data_link {
unsigned int src_ent;
u16 src_pad;
unsigned int sink_ent;
@@ -41,12 +46,20 @@ struct vimc_ent_link {
u32 flags;
};
+/* Structure which describes ancillary links between entities */
+struct vimc_ancillary_link {
+ unsigned int primary_ent;
+ unsigned int ancillary_ent;
+};
+
/* Structure which describes the whole topology */
struct vimc_pipeline_config {
const struct vimc_ent_config *ents;
size_t num_ents;
- const struct vimc_ent_link *links;
- size_t num_links;
+ const struct vimc_data_link *data_links;
+ size_t num_data_links;
+ const struct vimc_ancillary_link *ancillary_links;
+ size_t num_ancillary_links;
};
/* --------------------------------------------------------------------------
@@ -91,32 +104,49 @@ static struct vimc_ent_config ent_config[] = {
.name = "RGB/YUV Capture",
.type = &vimc_cap_type
},
+ {
+ .name = "Lens A",
+ .type = &vimc_len_type
+ },
+ {
+ .name = "Lens B",
+ .type = &vimc_len_type
+ },
};
-static const struct vimc_ent_link ent_links[] = {
+static const struct vimc_data_link data_links[] = {
/* Link: Sensor A (Pad 0)->(Pad 0) Debayer A */
- VIMC_ENT_LINK(0, 0, 2, 0, MEDIA_LNK_FL_ENABLED | MEDIA_LNK_FL_IMMUTABLE),
+ VIMC_DATA_LINK(0, 0, 2, 0, MEDIA_LNK_FL_ENABLED | MEDIA_LNK_FL_IMMUTABLE),
/* Link: Sensor A (Pad 0)->(Pad 0) Raw Capture 0 */
- VIMC_ENT_LINK(0, 0, 4, 0, MEDIA_LNK_FL_ENABLED | MEDIA_LNK_FL_IMMUTABLE),
+ VIMC_DATA_LINK(0, 0, 4, 0, MEDIA_LNK_FL_ENABLED | MEDIA_LNK_FL_IMMUTABLE),
/* Link: Sensor B (Pad 0)->(Pad 0) Debayer B */
- VIMC_ENT_LINK(1, 0, 3, 0, MEDIA_LNK_FL_ENABLED | MEDIA_LNK_FL_IMMUTABLE),
+ VIMC_DATA_LINK(1, 0, 3, 0, MEDIA_LNK_FL_ENABLED | MEDIA_LNK_FL_IMMUTABLE),
/* Link: Sensor B (Pad 0)->(Pad 0) Raw Capture 1 */
- VIMC_ENT_LINK(1, 0, 5, 0, MEDIA_LNK_FL_ENABLED | MEDIA_LNK_FL_IMMUTABLE),
+ VIMC_DATA_LINK(1, 0, 5, 0, MEDIA_LNK_FL_ENABLED | MEDIA_LNK_FL_IMMUTABLE),
/* Link: Debayer A (Pad 1)->(Pad 0) Scaler */
- VIMC_ENT_LINK(2, 1, 7, 0, MEDIA_LNK_FL_ENABLED),
+ VIMC_DATA_LINK(2, 1, 7, 0, MEDIA_LNK_FL_ENABLED),
/* Link: Debayer B (Pad 1)->(Pad 0) Scaler */
- VIMC_ENT_LINK(3, 1, 7, 0, 0),
+ VIMC_DATA_LINK(3, 1, 7, 0, 0),
/* Link: RGB/YUV Input (Pad 0)->(Pad 0) Scaler */
- VIMC_ENT_LINK(6, 0, 7, 0, 0),
+ VIMC_DATA_LINK(6, 0, 7, 0, 0),
/* Link: Scaler (Pad 1)->(Pad 0) RGB/YUV Capture */
- VIMC_ENT_LINK(7, 1, 8, 0, MEDIA_LNK_FL_ENABLED | MEDIA_LNK_FL_IMMUTABLE),
+ VIMC_DATA_LINK(7, 1, 8, 0, MEDIA_LNK_FL_ENABLED | MEDIA_LNK_FL_IMMUTABLE),
+};
+
+static const struct vimc_ancillary_link ancillary_links[] = {
+ /* Link: Sensor A -> Lens A */
+ VIMC_ANCILLARY_LINK(0, 9),
+ /* Link: Sensor B -> Lens B */
+ VIMC_ANCILLARY_LINK(1, 10),
};
static struct vimc_pipeline_config pipe_cfg = {
- .ents = ent_config,
- .num_ents = ARRAY_SIZE(ent_config),
- .links = ent_links,
- .num_links = ARRAY_SIZE(ent_links)
+ .ents = ent_config,
+ .num_ents = ARRAY_SIZE(ent_config),
+ .data_links = data_links,
+ .num_data_links = ARRAY_SIZE(data_links),
+ .ancillary_links = ancillary_links,
+ .num_ancillary_links = ARRAY_SIZE(ancillary_links),
};
/* -------------------------------------------------------------------------- */
@@ -135,8 +165,8 @@ static int vimc_create_links(struct vimc_device *vimc)
int ret;
/* Initialize the links between entities */
- for (i = 0; i < vimc->pipe_cfg->num_links; i++) {
- const struct vimc_ent_link *link = &vimc->pipe_cfg->links[i];
+ for (i = 0; i < vimc->pipe_cfg->num_data_links; i++) {
+ const struct vimc_data_link *link = &vimc->pipe_cfg->data_links[i];
struct vimc_ent_device *ved_src =
vimc->ent_devs[link->src_ent];
@@ -150,6 +180,22 @@ static int vimc_create_links(struct vimc_device *vimc)
goto err_rm_links;
}
+ for (i = 0; i < vimc->pipe_cfg->num_ancillary_links; i++) {
+ const struct vimc_ancillary_link *link = &vimc->pipe_cfg->ancillary_links[i];
+
+ struct vimc_ent_device *ved_primary =
+ vimc->ent_devs[link->primary_ent];
+ struct vimc_ent_device *ved_ancillary =
+ vimc->ent_devs[link->ancillary_ent];
+ struct media_link *ret_link =
+ media_create_ancillary_link(ved_primary->ent, ved_ancillary->ent);
+
+ if (IS_ERR(ret_link)) {
+ ret = PTR_ERR(link);
+ goto err_rm_links;
+ }
+ }
+
return 0;
err_rm_links:
diff --git a/drivers/media/test-drivers/vimc/vimc-lens.c b/drivers/media/test-drivers/vimc/vimc-lens.c
new file mode 100644
index 000000000000..dfe824d3addb
--- /dev/null
+++ b/drivers/media/test-drivers/vimc/vimc-lens.c
@@ -0,0 +1,102 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * vimc-lens.c Virtual Media Controller Driver
+ * Copyright (C) 2022 Google, Inc
+ * Author: yunkec@google.com (Yunke Cao)
+ */
+
+#include <media/v4l2-ctrls.h>
+#include <media/v4l2-event.h>
+#include <media/v4l2-subdev.h>
+
+#include "vimc-common.h"
+
+#define VIMC_LEN_MAX_FOCUS_POS 1023
+#define VIMC_LEN_MAX_FOCUS_STEP 1
+
+struct vimc_len_device {
+ struct vimc_ent_device ved;
+ struct v4l2_subdev sd;
+ struct v4l2_ctrl_handler hdl;
+ u32 focus_absolute;
+};
+
+static const struct v4l2_subdev_core_ops vimc_len_core_ops = {
+ .log_status = v4l2_ctrl_subdev_log_status,
+ .subscribe_event = v4l2_ctrl_subdev_subscribe_event,
+ .unsubscribe_event = v4l2_event_subdev_unsubscribe,
+};
+
+static const struct v4l2_subdev_ops vimc_len_ops = {
+ .core = &vimc_len_core_ops
+};
+
+static int vimc_len_s_ctrl(struct v4l2_ctrl *ctrl)
+{
+ struct vimc_len_device *vlen =
+ container_of(ctrl->handler, struct vimc_len_device, hdl);
+ if (ctrl->id == V4L2_CID_FOCUS_ABSOLUTE) {
+ vlen->focus_absolute = ctrl->val;
+ return 0;
+ }
+ return -EINVAL;
+}
+
+static const struct v4l2_ctrl_ops vimc_len_ctrl_ops = {
+ .s_ctrl = vimc_len_s_ctrl,
+};
+
+static struct vimc_ent_device *vimc_len_add(struct vimc_device *vimc,
+ const char *vcfg_name)
+{
+ struct v4l2_device *v4l2_dev = &vimc->v4l2_dev;
+ struct vimc_len_device *vlen;
+ int ret;
+
+ /* Allocate the vlen struct */
+ vlen = kzalloc(sizeof(*vlen), GFP_KERNEL);
+ if (!vlen)
+ return ERR_PTR(-ENOMEM);
+
+ v4l2_ctrl_handler_init(&vlen->hdl, 1);
+
+ v4l2_ctrl_new_std(&vlen->hdl, &vimc_len_ctrl_ops,
+ V4L2_CID_FOCUS_ABSOLUTE, 0,
+ VIMC_LEN_MAX_FOCUS_POS, VIMC_LEN_MAX_FOCUS_STEP, 0);
+ vlen->sd.ctrl_handler = &vlen->hdl;
+ if (vlen->hdl.error) {
+ ret = vlen->hdl.error;
+ goto err_free_vlen;
+ }
+ vlen->ved.dev = vimc->mdev.dev;
+
+ ret = vimc_ent_sd_register(&vlen->ved, &vlen->sd, v4l2_dev,
+ vcfg_name, MEDIA_ENT_F_LENS, 0,
+ NULL, &vimc_len_ops);
+ if (ret)
+ goto err_free_hdl;
+
+ return &vlen->ved;
+
+err_free_hdl:
+ v4l2_ctrl_handler_free(&vlen->hdl);
+err_free_vlen:
+ kfree(vlen);
+
+ return ERR_PTR(ret);
+}
+
+static void vimc_len_release(struct vimc_ent_device *ved)
+{
+ struct vimc_len_device *vlen =
+ container_of(ved, struct vimc_len_device, ved);
+
+ v4l2_ctrl_handler_free(&vlen->hdl);
+ media_entity_cleanup(vlen->ved.ent);
+ kfree(vlen);
+}
+
+struct vimc_ent_type vimc_len_type = {
+ .add = vimc_len_add,
+ .release = vimc_len_release
+};
--
2.36.0.rc0.470.gd361397f0d-goog
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v1 2/2] media: vimc: documentation for lens
2022-04-15 2:38 [PATCH v1 0/2] media: vimc: add ancillary lens Yunke Cao
2022-04-15 2:38 ` [PATCH v1 1/2] " Yunke Cao
@ 2022-04-15 2:38 ` Yunke Cao
1 sibling, 0 replies; 7+ messages in thread
From: Yunke Cao @ 2022-04-15 2:38 UTC (permalink / raw)
To: Kieran Bingham
Cc: Sakari Ailus, Tomasz Figa, Mauro Carvalho Chehab, Daniel Scally,
linux-media, Yunke Cao
Signed-off-by: Yunke Cao <yunkec@google.com>
---
Documentation/admin-guide/media/vimc.dot | 4 ++++
Documentation/admin-guide/media/vimc.rst | 3 +++
2 files changed, 7 insertions(+)
diff --git a/Documentation/admin-guide/media/vimc.dot b/Documentation/admin-guide/media/vimc.dot
index 57863a13fa39..5a6e231c3d21 100644
--- a/Documentation/admin-guide/media/vimc.dot
+++ b/Documentation/admin-guide/media/vimc.dot
@@ -5,9 +5,13 @@ digraph board {
n00000001 [label="{{} | Sensor A\n/dev/v4l-subdev0 | {<port0> 0}}", shape=Mrecord, style=filled, fillcolor=green]
n00000001:port0 -> n00000005:port0 [style=bold]
n00000001:port0 -> n0000000b [style=bold]
+ n00000001 -> n00000002
+ n00000002 [label="{{} | Lens A\n/dev/v4l-subdev5 | {<port0>}}", shape=Mrecord, style=filled, fillcolor=green]
n00000003 [label="{{} | Sensor B\n/dev/v4l-subdev1 | {<port0> 0}}", shape=Mrecord, style=filled, fillcolor=green]
n00000003:port0 -> n00000008:port0 [style=bold]
n00000003:port0 -> n0000000f [style=bold]
+ n00000003 -> n00000004
+ n00000004 [label="{{} | Lens B\n/dev/v4l-subdev6 | {<port0>}}", shape=Mrecord, style=filled, fillcolor=green]
n00000005 [label="{{<port0> 0} | Debayer A\n/dev/v4l-subdev2 | {<port1> 1}}", shape=Mrecord, style=filled, fillcolor=green]
n00000005:port1 -> n00000017:port0
n00000008 [label="{{<port0> 0} | Debayer B\n/dev/v4l-subdev3 | {<port1> 1}}", shape=Mrecord, style=filled, fillcolor=green]
diff --git a/Documentation/admin-guide/media/vimc.rst b/Documentation/admin-guide/media/vimc.rst
index 0b07f05dde25..1723eb5ec56a 100644
--- a/Documentation/admin-guide/media/vimc.rst
+++ b/Documentation/admin-guide/media/vimc.rst
@@ -53,6 +53,9 @@ vimc-sensor:
* 1 Pad source
+vimc-lens:
+ Ancillary lens for a sensor.
+
vimc-debayer:
Transforms images in bayer format into a non-bayer format.
Exposes:
--
2.36.0.rc0.470.gd361397f0d-goog
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v1 1/2] media: vimc: add ancillary lens
2022-04-15 2:38 ` [PATCH v1 1/2] " Yunke Cao
@ 2022-04-19 12:16 ` Kieran Bingham
2022-06-09 0:57 ` Yunke Cao
0 siblings, 1 reply; 7+ messages in thread
From: Kieran Bingham @ 2022-04-19 12:16 UTC (permalink / raw)
To: Yunke Cao
Cc: Sakari Ailus, Tomasz Figa, Mauro Carvalho Chehab, Daniel Scally,
linux-media, Yunke Cao
Hi Yunke,
This is a very interesting development!
Quoting Yunke Cao (2022-04-15 03:38:54)
> Add a basic version of vimc lens.
> Link lens with sensors using ancillary links.
>
> Signed-off-by: Yunke Cao <yunkec@google.com>
> ---
> drivers/media/test-drivers/vimc/Makefile | 2 +-
> drivers/media/test-drivers/vimc/vimc-common.h | 1 +
> drivers/media/test-drivers/vimc/vimc-core.c | 86 +++++++++++----
> drivers/media/test-drivers/vimc/vimc-lens.c | 102 ++++++++++++++++++
> 4 files changed, 170 insertions(+), 21 deletions(-)
> create mode 100644 drivers/media/test-drivers/vimc/vimc-lens.c
>
> diff --git a/drivers/media/test-drivers/vimc/Makefile b/drivers/media/test-drivers/vimc/Makefile
> index a53b2b532e9f..9b9631562473 100644
> --- a/drivers/media/test-drivers/vimc/Makefile
> +++ b/drivers/media/test-drivers/vimc/Makefile
> @@ -1,6 +1,6 @@
> # SPDX-License-Identifier: GPL-2.0
> vimc-y := vimc-core.o vimc-common.o vimc-streamer.o vimc-capture.o \
> - vimc-debayer.o vimc-scaler.o vimc-sensor.o
> + vimc-debayer.o vimc-scaler.o vimc-sensor.o vimc-lens.o
>
> obj-$(CONFIG_VIDEO_VIMC) += vimc.o
>
> diff --git a/drivers/media/test-drivers/vimc/vimc-common.h b/drivers/media/test-drivers/vimc/vimc-common.h
> index ba1930772589..37f6b687ce10 100644
> --- a/drivers/media/test-drivers/vimc/vimc-common.h
> +++ b/drivers/media/test-drivers/vimc/vimc-common.h
> @@ -171,6 +171,7 @@ extern struct vimc_ent_type vimc_sen_type;
> extern struct vimc_ent_type vimc_deb_type;
> extern struct vimc_ent_type vimc_sca_type;
> extern struct vimc_ent_type vimc_cap_type;
> +extern struct vimc_ent_type vimc_len_type;
Aha, I see 'len' is short for 'lens'... I think that confused me below.
I wonder if these should be longer. 'len' makes me think of 'length' too
much, and 'lens' is only one char longer. But I guess this is
established in this driver already so it would need a patch to change
vimc_{deb,sca,cap}_type before calling it lens.
> /**
> * vimc_pix_map_by_index - get vimc_pix_map struct by its index
> diff --git a/drivers/media/test-drivers/vimc/vimc-core.c b/drivers/media/test-drivers/vimc/vimc-core.c
> index 06edf9d4d92c..166323406c6b 100644
> --- a/drivers/media/test-drivers/vimc/vimc-core.c
> +++ b/drivers/media/test-drivers/vimc/vimc-core.c
> @@ -24,7 +24,7 @@ MODULE_PARM_DESC(allocator, " memory allocator selection, default is 0.\n"
>
> #define VIMC_MDEV_MODEL_NAME "VIMC MDEV"
>
> -#define VIMC_ENT_LINK(src, srcpad, sink, sinkpad, link_flags) { \
> +#define VIMC_DATA_LINK(src, srcpad, sink, sinkpad, link_flags) { \
> .src_ent = src, \
> .src_pad = srcpad, \
> .sink_ent = sink, \
> @@ -32,8 +32,13 @@ MODULE_PARM_DESC(allocator, " memory allocator selection, default is 0.\n"
> .flags = link_flags, \
> }
>
> -/* Structure which describes links between entities */
> -struct vimc_ent_link {
> +#define VIMC_ANCILLARY_LINK(primary, ancillary) { \
> + .primary_ent = primary, \
> + .ancillary_ent = ancillary \
> +}
> +
> +/* Structure which describes data links between entities */
> +struct vimc_data_link {
> unsigned int src_ent;
> u16 src_pad;
> unsigned int sink_ent;
> @@ -41,12 +46,20 @@ struct vimc_ent_link {
> u32 flags;
> };
>
> +/* Structure which describes ancillary links between entities */
> +struct vimc_ancillary_link {
> + unsigned int primary_ent;
> + unsigned int ancillary_ent;
> +};
> +
> /* Structure which describes the whole topology */
> struct vimc_pipeline_config {
> const struct vimc_ent_config *ents;
> size_t num_ents;
> - const struct vimc_ent_link *links;
> - size_t num_links;
> + const struct vimc_data_link *data_links;
> + size_t num_data_links;
> + const struct vimc_ancillary_link *ancillary_links;
> + size_t num_ancillary_links;
> };
>
> /* --------------------------------------------------------------------------
> @@ -91,32 +104,49 @@ static struct vimc_ent_config ent_config[] = {
> .name = "RGB/YUV Capture",
> .type = &vimc_cap_type
> },
> + {
> + .name = "Lens A",
> + .type = &vimc_len_type
> + },
> + {
> + .name = "Lens B",
> + .type = &vimc_len_type
> + },
> };
>
> -static const struct vimc_ent_link ent_links[] = {
> +static const struct vimc_data_link data_links[] = {
> /* Link: Sensor A (Pad 0)->(Pad 0) Debayer A */
> - VIMC_ENT_LINK(0, 0, 2, 0, MEDIA_LNK_FL_ENABLED | MEDIA_LNK_FL_IMMUTABLE),
> + VIMC_DATA_LINK(0, 0, 2, 0, MEDIA_LNK_FL_ENABLED | MEDIA_LNK_FL_IMMUTABLE),
> /* Link: Sensor A (Pad 0)->(Pad 0) Raw Capture 0 */
> - VIMC_ENT_LINK(0, 0, 4, 0, MEDIA_LNK_FL_ENABLED | MEDIA_LNK_FL_IMMUTABLE),
> + VIMC_DATA_LINK(0, 0, 4, 0, MEDIA_LNK_FL_ENABLED | MEDIA_LNK_FL_IMMUTABLE),
> /* Link: Sensor B (Pad 0)->(Pad 0) Debayer B */
> - VIMC_ENT_LINK(1, 0, 3, 0, MEDIA_LNK_FL_ENABLED | MEDIA_LNK_FL_IMMUTABLE),
> + VIMC_DATA_LINK(1, 0, 3, 0, MEDIA_LNK_FL_ENABLED | MEDIA_LNK_FL_IMMUTABLE),
> /* Link: Sensor B (Pad 0)->(Pad 0) Raw Capture 1 */
> - VIMC_ENT_LINK(1, 0, 5, 0, MEDIA_LNK_FL_ENABLED | MEDIA_LNK_FL_IMMUTABLE),
> + VIMC_DATA_LINK(1, 0, 5, 0, MEDIA_LNK_FL_ENABLED | MEDIA_LNK_FL_IMMUTABLE),
> /* Link: Debayer A (Pad 1)->(Pad 0) Scaler */
> - VIMC_ENT_LINK(2, 1, 7, 0, MEDIA_LNK_FL_ENABLED),
> + VIMC_DATA_LINK(2, 1, 7, 0, MEDIA_LNK_FL_ENABLED),
> /* Link: Debayer B (Pad 1)->(Pad 0) Scaler */
> - VIMC_ENT_LINK(3, 1, 7, 0, 0),
> + VIMC_DATA_LINK(3, 1, 7, 0, 0),
> /* Link: RGB/YUV Input (Pad 0)->(Pad 0) Scaler */
> - VIMC_ENT_LINK(6, 0, 7, 0, 0),
> + VIMC_DATA_LINK(6, 0, 7, 0, 0),
> /* Link: Scaler (Pad 1)->(Pad 0) RGB/YUV Capture */
> - VIMC_ENT_LINK(7, 1, 8, 0, MEDIA_LNK_FL_ENABLED | MEDIA_LNK_FL_IMMUTABLE),
> + VIMC_DATA_LINK(7, 1, 8, 0, MEDIA_LNK_FL_ENABLED | MEDIA_LNK_FL_IMMUTABLE),
> +};
> +
> +static const struct vimc_ancillary_link ancillary_links[] = {
> + /* Link: Sensor A -> Lens A */
> + VIMC_ANCILLARY_LINK(0, 9),
> + /* Link: Sensor B -> Lens B */
> + VIMC_ANCILLARY_LINK(1, 10),
> };
There's a lot of magic indexes here (not a fault of your patch) - I
would probably restructure this to have the indexes named in an enum.
But - I don't think that is required for your patch - just something I
think should be done for VIMC to make this clearer and less likely to
get an incorrect index one day.
> static struct vimc_pipeline_config pipe_cfg = {
> - .ents = ent_config,
> - .num_ents = ARRAY_SIZE(ent_config),
> - .links = ent_links,
> - .num_links = ARRAY_SIZE(ent_links)
> + .ents = ent_config,
> + .num_ents = ARRAY_SIZE(ent_config),
> + .data_links = data_links,
> + .num_data_links = ARRAY_SIZE(data_links),
> + .ancillary_links = ancillary_links,
> + .num_ancillary_links = ARRAY_SIZE(ancillary_links),
> };
>
> /* -------------------------------------------------------------------------- */
> @@ -135,8 +165,8 @@ static int vimc_create_links(struct vimc_device *vimc)
> int ret;
>
> /* Initialize the links between entities */
> - for (i = 0; i < vimc->pipe_cfg->num_links; i++) {
> - const struct vimc_ent_link *link = &vimc->pipe_cfg->links[i];
> + for (i = 0; i < vimc->pipe_cfg->num_data_links; i++) {
> + const struct vimc_data_link *link = &vimc->pipe_cfg->data_links[i];
>
> struct vimc_ent_device *ved_src =
> vimc->ent_devs[link->src_ent];
> @@ -150,6 +180,22 @@ static int vimc_create_links(struct vimc_device *vimc)
> goto err_rm_links;
> }
>
> + for (i = 0; i < vimc->pipe_cfg->num_ancillary_links; i++) {
> + const struct vimc_ancillary_link *link = &vimc->pipe_cfg->ancillary_links[i];
> +
> + struct vimc_ent_device *ved_primary =
> + vimc->ent_devs[link->primary_ent];
> + struct vimc_ent_device *ved_ancillary =
> + vimc->ent_devs[link->ancillary_ent];
> + struct media_link *ret_link =
> + media_create_ancillary_link(ved_primary->ent, ved_ancillary->ent);
> +
> + if (IS_ERR(ret_link)) {
> + ret = PTR_ERR(link);
> + goto err_rm_links;
> + }
> + }
> +
> return 0;
>
> err_rm_links:
> diff --git a/drivers/media/test-drivers/vimc/vimc-lens.c b/drivers/media/test-drivers/vimc/vimc-lens.c
> new file mode 100644
> index 000000000000..dfe824d3addb
> --- /dev/null
> +++ b/drivers/media/test-drivers/vimc/vimc-lens.c
> @@ -0,0 +1,102 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * vimc-lens.c Virtual Media Controller Driver
> + * Copyright (C) 2022 Google, Inc
> + * Author: yunkec@google.com (Yunke Cao)
> + */
> +
> +#include <media/v4l2-ctrls.h>
> +#include <media/v4l2-event.h>
> +#include <media/v4l2-subdev.h>
> +
> +#include "vimc-common.h"
> +
> +#define VIMC_LEN_MAX_FOCUS_POS 1023
> +#define VIMC_LEN_MAX_FOCUS_STEP 1
> +
> +struct vimc_len_device {
> + struct vimc_ent_device ved;
> + struct v4l2_subdev sd;
> + struct v4l2_ctrl_handler hdl;
> + u32 focus_absolute;
I'm not 100% certain we need to actually store this, as I think having
the control itself is enough - but I think it's good to keep this here.
I wonder if we might have some filter on the value sometime for
retrieval so we can emulate the physical movement delays ... but I'm
getting ahead of myself there, and this is fine as is.
I can't actually see any particular issues with this as it stands, and
my comments so far are really only about separate developments or
patches on top of this - so I am already tempted to offer:
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> +};
> +
> +static const struct v4l2_subdev_core_ops vimc_len_core_ops = {
> + .log_status = v4l2_ctrl_subdev_log_status,
Oh nice so that already logs the current control settings.
> + .subscribe_event = v4l2_ctrl_subdev_subscribe_event,
> + .unsubscribe_event = v4l2_event_subdev_unsubscribe,
> +};
> +
> +static const struct v4l2_subdev_ops vimc_len_ops = {
> + .core = &vimc_len_core_ops
> +};
> +
> +static int vimc_len_s_ctrl(struct v4l2_ctrl *ctrl)
> +{
> + struct vimc_len_device *vlen =
> + container_of(ctrl->handler, struct vimc_len_device, hdl);
> + if (ctrl->id == V4L2_CID_FOCUS_ABSOLUTE) {
> + vlen->focus_absolute = ctrl->val;
> + return 0;
> + }
> + return -EINVAL;
> +}
> +
> +static const struct v4l2_ctrl_ops vimc_len_ctrl_ops = {
> + .s_ctrl = vimc_len_s_ctrl,
> +};
> +
> +static struct vimc_ent_device *vimc_len_add(struct vimc_device *vimc,
> + const char *vcfg_name)
> +{
> + struct v4l2_device *v4l2_dev = &vimc->v4l2_dev;
> + struct vimc_len_device *vlen;
> + int ret;
> +
> + /* Allocate the vlen struct */
> + vlen = kzalloc(sizeof(*vlen), GFP_KERNEL);
> + if (!vlen)
> + return ERR_PTR(-ENOMEM);
> +
> + v4l2_ctrl_handler_init(&vlen->hdl, 1);
> +
> + v4l2_ctrl_new_std(&vlen->hdl, &vimc_len_ctrl_ops,
> + V4L2_CID_FOCUS_ABSOLUTE, 0,
> + VIMC_LEN_MAX_FOCUS_POS, VIMC_LEN_MAX_FOCUS_STEP, 0);
> + vlen->sd.ctrl_handler = &vlen->hdl;
> + if (vlen->hdl.error) {
> + ret = vlen->hdl.error;
> + goto err_free_vlen;
> + }
> + vlen->ved.dev = vimc->mdev.dev;
> +
> + ret = vimc_ent_sd_register(&vlen->ved, &vlen->sd, v4l2_dev,
> + vcfg_name, MEDIA_ENT_F_LENS, 0,
> + NULL, &vimc_len_ops);
> + if (ret)
> + goto err_free_hdl;
> +
> + return &vlen->ved;
> +
> +err_free_hdl:
> + v4l2_ctrl_handler_free(&vlen->hdl);
> +err_free_vlen:
> + kfree(vlen);
> +
> + return ERR_PTR(ret);
> +}
> +
> +static void vimc_len_release(struct vimc_ent_device *ved)
> +{
> + struct vimc_len_device *vlen =
> + container_of(ved, struct vimc_len_device, ved);
> +
> + v4l2_ctrl_handler_free(&vlen->hdl);
> + media_entity_cleanup(vlen->ved.ent);
> + kfree(vlen);
> +}
> +
> +struct vimc_ent_type vimc_len_type = {
> + .add = vimc_len_add,
> + .release = vimc_len_release
> +};
> --
> 2.36.0.rc0.470.gd361397f0d-goog
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v1 1/2] media: vimc: add ancillary lens
2022-04-19 12:16 ` Kieran Bingham
@ 2022-06-09 0:57 ` Yunke Cao
2022-06-09 16:04 ` Shuah Khan
0 siblings, 1 reply; 7+ messages in thread
From: Yunke Cao @ 2022-06-09 0:57 UTC (permalink / raw)
To: Kieran Bingham, skhan
Cc: Sakari Ailus, Tomasz Figa, Mauro Carvalho Chehab, Daniel Scally,
linux-media
Hi Kieran,
Thanks for the review!
I wonder what's the status of this. Will this patch get merged?
Best,
Yunke
On Tue, Apr 19, 2022 at 9:16 PM Kieran Bingham
<kieran.bingham@ideasonboard.com> wrote:
>
> Hi Yunke,
>
> This is a very interesting development!
>
> Quoting Yunke Cao (2022-04-15 03:38:54)
> > Add a basic version of vimc lens.
> > Link lens with sensors using ancillary links.
> >
> > Signed-off-by: Yunke Cao <yunkec@google.com>
> > ---
> > drivers/media/test-drivers/vimc/Makefile | 2 +-
> > drivers/media/test-drivers/vimc/vimc-common.h | 1 +
> > drivers/media/test-drivers/vimc/vimc-core.c | 86 +++++++++++----
> > drivers/media/test-drivers/vimc/vimc-lens.c | 102 ++++++++++++++++++
> > 4 files changed, 170 insertions(+), 21 deletions(-)
> > create mode 100644 drivers/media/test-drivers/vimc/vimc-lens.c
> >
> > diff --git a/drivers/media/test-drivers/vimc/Makefile b/drivers/media/test-drivers/vimc/Makefile
> > index a53b2b532e9f..9b9631562473 100644
> > --- a/drivers/media/test-drivers/vimc/Makefile
> > +++ b/drivers/media/test-drivers/vimc/Makefile
> > @@ -1,6 +1,6 @@
> > # SPDX-License-Identifier: GPL-2.0
> > vimc-y := vimc-core.o vimc-common.o vimc-streamer.o vimc-capture.o \
> > - vimc-debayer.o vimc-scaler.o vimc-sensor.o
> > + vimc-debayer.o vimc-scaler.o vimc-sensor.o vimc-lens.o
> >
> > obj-$(CONFIG_VIDEO_VIMC) += vimc.o
> >
> > diff --git a/drivers/media/test-drivers/vimc/vimc-common.h b/drivers/media/test-drivers/vimc/vimc-common.h
> > index ba1930772589..37f6b687ce10 100644
> > --- a/drivers/media/test-drivers/vimc/vimc-common.h
> > +++ b/drivers/media/test-drivers/vimc/vimc-common.h
> > @@ -171,6 +171,7 @@ extern struct vimc_ent_type vimc_sen_type;
> > extern struct vimc_ent_type vimc_deb_type;
> > extern struct vimc_ent_type vimc_sca_type;
> > extern struct vimc_ent_type vimc_cap_type;
> > +extern struct vimc_ent_type vimc_len_type;
>
> Aha, I see 'len' is short for 'lens'... I think that confused me below.
>
> I wonder if these should be longer. 'len' makes me think of 'length' too
> much, and 'lens' is only one char longer. But I guess this is
> established in this driver already so it would need a patch to change
> vimc_{deb,sca,cap}_type before calling it lens.
>
>
> > /**
> > * vimc_pix_map_by_index - get vimc_pix_map struct by its index
> > diff --git a/drivers/media/test-drivers/vimc/vimc-core.c b/drivers/media/test-drivers/vimc/vimc-core.c
> > index 06edf9d4d92c..166323406c6b 100644
> > --- a/drivers/media/test-drivers/vimc/vimc-core.c
> > +++ b/drivers/media/test-drivers/vimc/vimc-core.c
> > @@ -24,7 +24,7 @@ MODULE_PARM_DESC(allocator, " memory allocator selection, default is 0.\n"
> >
> > #define VIMC_MDEV_MODEL_NAME "VIMC MDEV"
> >
> > -#define VIMC_ENT_LINK(src, srcpad, sink, sinkpad, link_flags) { \
> > +#define VIMC_DATA_LINK(src, srcpad, sink, sinkpad, link_flags) { \
> > .src_ent = src, \
> > .src_pad = srcpad, \
> > .sink_ent = sink, \
> > @@ -32,8 +32,13 @@ MODULE_PARM_DESC(allocator, " memory allocator selection, default is 0.\n"
> > .flags = link_flags, \
> > }
> >
> > -/* Structure which describes links between entities */
> > -struct vimc_ent_link {
> > +#define VIMC_ANCILLARY_LINK(primary, ancillary) { \
> > + .primary_ent = primary, \
> > + .ancillary_ent = ancillary \
> > +}
> > +
> > +/* Structure which describes data links between entities */
> > +struct vimc_data_link {
> > unsigned int src_ent;
> > u16 src_pad;
> > unsigned int sink_ent;
> > @@ -41,12 +46,20 @@ struct vimc_ent_link {
> > u32 flags;
> > };
> >
> > +/* Structure which describes ancillary links between entities */
> > +struct vimc_ancillary_link {
> > + unsigned int primary_ent;
> > + unsigned int ancillary_ent;
> > +};
> > +
> > /* Structure which describes the whole topology */
> > struct vimc_pipeline_config {
> > const struct vimc_ent_config *ents;
> > size_t num_ents;
> > - const struct vimc_ent_link *links;
> > - size_t num_links;
> > + const struct vimc_data_link *data_links;
> > + size_t num_data_links;
> > + const struct vimc_ancillary_link *ancillary_links;
> > + size_t num_ancillary_links;
> > };
> >
> > /* --------------------------------------------------------------------------
> > @@ -91,32 +104,49 @@ static struct vimc_ent_config ent_config[] = {
> > .name = "RGB/YUV Capture",
> > .type = &vimc_cap_type
> > },
> > + {
> > + .name = "Lens A",
> > + .type = &vimc_len_type
> > + },
> > + {
> > + .name = "Lens B",
> > + .type = &vimc_len_type
> > + },
> > };
> >
> > -static const struct vimc_ent_link ent_links[] = {
> > +static const struct vimc_data_link data_links[] = {
> > /* Link: Sensor A (Pad 0)->(Pad 0) Debayer A */
> > - VIMC_ENT_LINK(0, 0, 2, 0, MEDIA_LNK_FL_ENABLED | MEDIA_LNK_FL_IMMUTABLE),
> > + VIMC_DATA_LINK(0, 0, 2, 0, MEDIA_LNK_FL_ENABLED | MEDIA_LNK_FL_IMMUTABLE),
> > /* Link: Sensor A (Pad 0)->(Pad 0) Raw Capture 0 */
> > - VIMC_ENT_LINK(0, 0, 4, 0, MEDIA_LNK_FL_ENABLED | MEDIA_LNK_FL_IMMUTABLE),
> > + VIMC_DATA_LINK(0, 0, 4, 0, MEDIA_LNK_FL_ENABLED | MEDIA_LNK_FL_IMMUTABLE),
> > /* Link: Sensor B (Pad 0)->(Pad 0) Debayer B */
> > - VIMC_ENT_LINK(1, 0, 3, 0, MEDIA_LNK_FL_ENABLED | MEDIA_LNK_FL_IMMUTABLE),
> > + VIMC_DATA_LINK(1, 0, 3, 0, MEDIA_LNK_FL_ENABLED | MEDIA_LNK_FL_IMMUTABLE),
> > /* Link: Sensor B (Pad 0)->(Pad 0) Raw Capture 1 */
> > - VIMC_ENT_LINK(1, 0, 5, 0, MEDIA_LNK_FL_ENABLED | MEDIA_LNK_FL_IMMUTABLE),
> > + VIMC_DATA_LINK(1, 0, 5, 0, MEDIA_LNK_FL_ENABLED | MEDIA_LNK_FL_IMMUTABLE),
> > /* Link: Debayer A (Pad 1)->(Pad 0) Scaler */
> > - VIMC_ENT_LINK(2, 1, 7, 0, MEDIA_LNK_FL_ENABLED),
> > + VIMC_DATA_LINK(2, 1, 7, 0, MEDIA_LNK_FL_ENABLED),
> > /* Link: Debayer B (Pad 1)->(Pad 0) Scaler */
> > - VIMC_ENT_LINK(3, 1, 7, 0, 0),
> > + VIMC_DATA_LINK(3, 1, 7, 0, 0),
> > /* Link: RGB/YUV Input (Pad 0)->(Pad 0) Scaler */
> > - VIMC_ENT_LINK(6, 0, 7, 0, 0),
> > + VIMC_DATA_LINK(6, 0, 7, 0, 0),
> > /* Link: Scaler (Pad 1)->(Pad 0) RGB/YUV Capture */
> > - VIMC_ENT_LINK(7, 1, 8, 0, MEDIA_LNK_FL_ENABLED | MEDIA_LNK_FL_IMMUTABLE),
> > + VIMC_DATA_LINK(7, 1, 8, 0, MEDIA_LNK_FL_ENABLED | MEDIA_LNK_FL_IMMUTABLE),
> > +};
> > +
> > +static const struct vimc_ancillary_link ancillary_links[] = {
> > + /* Link: Sensor A -> Lens A */
> > + VIMC_ANCILLARY_LINK(0, 9),
> > + /* Link: Sensor B -> Lens B */
> > + VIMC_ANCILLARY_LINK(1, 10),
> > };
>
> There's a lot of magic indexes here (not a fault of your patch) - I
> would probably restructure this to have the indexes named in an enum.
>
> But - I don't think that is required for your patch - just something I
> think should be done for VIMC to make this clearer and less likely to
> get an incorrect index one day.
>
>
> > static struct vimc_pipeline_config pipe_cfg = {
> > - .ents = ent_config,
> > - .num_ents = ARRAY_SIZE(ent_config),
> > - .links = ent_links,
> > - .num_links = ARRAY_SIZE(ent_links)
> > + .ents = ent_config,
> > + .num_ents = ARRAY_SIZE(ent_config),
> > + .data_links = data_links,
> > + .num_data_links = ARRAY_SIZE(data_links),
> > + .ancillary_links = ancillary_links,
> > + .num_ancillary_links = ARRAY_SIZE(ancillary_links),
> > };
> >
> > /* -------------------------------------------------------------------------- */
> > @@ -135,8 +165,8 @@ static int vimc_create_links(struct vimc_device *vimc)
> > int ret;
> >
> > /* Initialize the links between entities */
> > - for (i = 0; i < vimc->pipe_cfg->num_links; i++) {
> > - const struct vimc_ent_link *link = &vimc->pipe_cfg->links[i];
> > + for (i = 0; i < vimc->pipe_cfg->num_data_links; i++) {
> > + const struct vimc_data_link *link = &vimc->pipe_cfg->data_links[i];
> >
> > struct vimc_ent_device *ved_src =
> > vimc->ent_devs[link->src_ent];
> > @@ -150,6 +180,22 @@ static int vimc_create_links(struct vimc_device *vimc)
> > goto err_rm_links;
> > }
> >
> > + for (i = 0; i < vimc->pipe_cfg->num_ancillary_links; i++) {
> > + const struct vimc_ancillary_link *link = &vimc->pipe_cfg->ancillary_links[i];
> > +
> > + struct vimc_ent_device *ved_primary =
> > + vimc->ent_devs[link->primary_ent];
> > + struct vimc_ent_device *ved_ancillary =
> > + vimc->ent_devs[link->ancillary_ent];
> > + struct media_link *ret_link =
> > + media_create_ancillary_link(ved_primary->ent, ved_ancillary->ent);
> > +
> > + if (IS_ERR(ret_link)) {
> > + ret = PTR_ERR(link);
> > + goto err_rm_links;
> > + }
> > + }
> > +
> > return 0;
> >
> > err_rm_links:
> > diff --git a/drivers/media/test-drivers/vimc/vimc-lens.c b/drivers/media/test-drivers/vimc/vimc-lens.c
> > new file mode 100644
> > index 000000000000..dfe824d3addb
> > --- /dev/null
> > +++ b/drivers/media/test-drivers/vimc/vimc-lens.c
> > @@ -0,0 +1,102 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later
> > +/*
> > + * vimc-lens.c Virtual Media Controller Driver
> > + * Copyright (C) 2022 Google, Inc
> > + * Author: yunkec@google.com (Yunke Cao)
> > + */
> > +
> > +#include <media/v4l2-ctrls.h>
> > +#include <media/v4l2-event.h>
> > +#include <media/v4l2-subdev.h>
> > +
> > +#include "vimc-common.h"
> > +
> > +#define VIMC_LEN_MAX_FOCUS_POS 1023
> > +#define VIMC_LEN_MAX_FOCUS_STEP 1
> > +
> > +struct vimc_len_device {
> > + struct vimc_ent_device ved;
> > + struct v4l2_subdev sd;
> > + struct v4l2_ctrl_handler hdl;
> > + u32 focus_absolute;
>
> I'm not 100% certain we need to actually store this, as I think having
> the control itself is enough - but I think it's good to keep this here.
>
> I wonder if we might have some filter on the value sometime for
> retrieval so we can emulate the physical movement delays ... but I'm
> getting ahead of myself there, and this is fine as is.
>
> I can't actually see any particular issues with this as it stands, and
> my comments so far are really only about separate developments or
> patches on top of this - so I am already tempted to offer:
>
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
>
>
> > +};
> > +
> > +static const struct v4l2_subdev_core_ops vimc_len_core_ops = {
> > + .log_status = v4l2_ctrl_subdev_log_status,
>
> Oh nice so that already logs the current control settings.
>
> > + .subscribe_event = v4l2_ctrl_subdev_subscribe_event,
> > + .unsubscribe_event = v4l2_event_subdev_unsubscribe,
> > +};
> > +
> > +static const struct v4l2_subdev_ops vimc_len_ops = {
> > + .core = &vimc_len_core_ops
> > +};
> > +
> > +static int vimc_len_s_ctrl(struct v4l2_ctrl *ctrl)
> > +{
> > + struct vimc_len_device *vlen =
> > + container_of(ctrl->handler, struct vimc_len_device, hdl);
> > + if (ctrl->id == V4L2_CID_FOCUS_ABSOLUTE) {
> > + vlen->focus_absolute = ctrl->val;
> > + return 0;
> > + }
> > + return -EINVAL;
> > +}
> > +
> > +static const struct v4l2_ctrl_ops vimc_len_ctrl_ops = {
> > + .s_ctrl = vimc_len_s_ctrl,
> > +};
> > +
> > +static struct vimc_ent_device *vimc_len_add(struct vimc_device *vimc,
> > + const char *vcfg_name)
> > +{
> > + struct v4l2_device *v4l2_dev = &vimc->v4l2_dev;
> > + struct vimc_len_device *vlen;
> > + int ret;
> > +
> > + /* Allocate the vlen struct */
> > + vlen = kzalloc(sizeof(*vlen), GFP_KERNEL);
> > + if (!vlen)
> > + return ERR_PTR(-ENOMEM);
> > +
> > + v4l2_ctrl_handler_init(&vlen->hdl, 1);
> > +
> > + v4l2_ctrl_new_std(&vlen->hdl, &vimc_len_ctrl_ops,
> > + V4L2_CID_FOCUS_ABSOLUTE, 0,
> > + VIMC_LEN_MAX_FOCUS_POS, VIMC_LEN_MAX_FOCUS_STEP, 0);
> > + vlen->sd.ctrl_handler = &vlen->hdl;
> > + if (vlen->hdl.error) {
> > + ret = vlen->hdl.error;
> > + goto err_free_vlen;
> > + }
> > + vlen->ved.dev = vimc->mdev.dev;
> > +
> > + ret = vimc_ent_sd_register(&vlen->ved, &vlen->sd, v4l2_dev,
> > + vcfg_name, MEDIA_ENT_F_LENS, 0,
> > + NULL, &vimc_len_ops);
> > + if (ret)
> > + goto err_free_hdl;
> > +
> > + return &vlen->ved;
> > +
> > +err_free_hdl:
> > + v4l2_ctrl_handler_free(&vlen->hdl);
> > +err_free_vlen:
> > + kfree(vlen);
> > +
> > + return ERR_PTR(ret);
> > +}
> > +
> > +static void vimc_len_release(struct vimc_ent_device *ved)
> > +{
> > + struct vimc_len_device *vlen =
> > + container_of(ved, struct vimc_len_device, ved);
> > +
> > + v4l2_ctrl_handler_free(&vlen->hdl);
> > + media_entity_cleanup(vlen->ved.ent);
> > + kfree(vlen);
> > +}
> > +
> > +struct vimc_ent_type vimc_len_type = {
> > + .add = vimc_len_add,
> > + .release = vimc_len_release
> > +};
> > --
> > 2.36.0.rc0.470.gd361397f0d-goog
> >
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v1 1/2] media: vimc: add ancillary lens
2022-06-09 0:57 ` Yunke Cao
@ 2022-06-09 16:04 ` Shuah Khan
2022-06-10 1:04 ` Yunke Cao
0 siblings, 1 reply; 7+ messages in thread
From: Shuah Khan @ 2022-06-09 16:04 UTC (permalink / raw)
To: Yunke Cao, Kieran Bingham
Cc: Sakari Ailus, Tomasz Figa, Mauro Carvalho Chehab, Daniel Scally,
linux-media, Shuah Khan
On 6/8/22 6:57 PM, Yunke Cao wrote:
> Hi Kieran,
>
> Thanks for the review!
> I wonder what's the status of this. Will this patch get merged?
>
> Best,
> Yunke
>
> On Tue, Apr 19, 2022 at 9:16 PM Kieran Bingham
> <kieran.bingham@ideasonboard.com> wrote:
>>
>> Hi Yunke,
>>
>> This is a very interesting development!
>>
>> Quoting Yunke Cao (2022-04-15 03:38:54)
>>> Add a basic version of vimc lens.
>>> Link lens with sensors using ancillary links.
>>>
>>> Signed-off-by: Yunke Cao <yunkec@google.com>
>>> ---
>>> drivers/media/test-drivers/vimc/Makefile | 2 +-
>>> drivers/media/test-drivers/vimc/vimc-common.h | 1 +
>>> drivers/media/test-drivers/vimc/vimc-core.c | 86 +++++++++++----
>>> drivers/media/test-drivers/vimc/vimc-lens.c | 102 ++++++++++++++++++
>>> 4 files changed, 170 insertions(+), 21 deletions(-)
>>> create mode 100644 drivers/media/test-drivers/vimc/vimc-lens.c
>>>
Hmm. Couldn't find this patch in my Inbox and spam folder. Explains the
delay on my side.
Please resend so I can do a proper review.
thanks,
-- Shuah
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v1 1/2] media: vimc: add ancillary lens
2022-06-09 16:04 ` Shuah Khan
@ 2022-06-10 1:04 ` Yunke Cao
0 siblings, 0 replies; 7+ messages in thread
From: Yunke Cao @ 2022-06-10 1:04 UTC (permalink / raw)
To: Shuah Khan
Cc: Kieran Bingham, Sakari Ailus, Tomasz Figa, Mauro Carvalho Chehab,
Daniel Scally, linux-media
Thanks Shuah. I just resent the patch.
Best,
Yunke
On Fri, Jun 10, 2022 at 1:04 AM Shuah Khan <skhan@linuxfoundation.org> wrote:
>
> On 6/8/22 6:57 PM, Yunke Cao wrote:
> > Hi Kieran,
> >
> > Thanks for the review!
> > I wonder what's the status of this. Will this patch get merged?
> >
> > Best,
> > Yunke
> >
> > On Tue, Apr 19, 2022 at 9:16 PM Kieran Bingham
> > <kieran.bingham@ideasonboard.com> wrote:
> >>
> >> Hi Yunke,
> >>
> >> This is a very interesting development!
> >>
> >> Quoting Yunke Cao (2022-04-15 03:38:54)
> >>> Add a basic version of vimc lens.
> >>> Link lens with sensors using ancillary links.
> >>>
> >>> Signed-off-by: Yunke Cao <yunkec@google.com>
> >>> ---
> >>> drivers/media/test-drivers/vimc/Makefile | 2 +-
> >>> drivers/media/test-drivers/vimc/vimc-common.h | 1 +
> >>> drivers/media/test-drivers/vimc/vimc-core.c | 86 +++++++++++----
> >>> drivers/media/test-drivers/vimc/vimc-lens.c | 102 ++++++++++++++++++
> >>> 4 files changed, 170 insertions(+), 21 deletions(-)
> >>> create mode 100644 drivers/media/test-drivers/vimc/vimc-lens.c
> >>>
>
> Hmm. Couldn't find this patch in my Inbox and spam folder. Explains the
> delay on my side.
>
> Please resend so I can do a proper review.
>
> thanks,
> -- Shuah
>
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2022-06-10 1:05 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-15 2:38 [PATCH v1 0/2] media: vimc: add ancillary lens Yunke Cao
2022-04-15 2:38 ` [PATCH v1 1/2] " Yunke Cao
2022-04-19 12:16 ` Kieran Bingham
2022-06-09 0:57 ` Yunke Cao
2022-06-09 16:04 ` Shuah Khan
2022-06-10 1:04 ` Yunke Cao
2022-04-15 2:38 ` [PATCH v1 2/2] media: vimc: documentation for lens Yunke Cao
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.