[PATCH v2 0/2] media: vimc: add ancillary lens

[PATCH v2 0/2] media: vimc: add ancillary lens

[Yunke Cao]

Add a basic version of vimc lens.
Link lens with sensors using ancillary links.
Update vimc documentation to reflect this change.

Changelog from v1:
-Better commit log.
-A bit more detailed documentation in 2/2.

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      |   4 +
 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, 178 insertions(+), 21 deletions(-)
 create mode 100644 drivers/media/test-drivers/vimc/vimc-lens.c

-- 
2.37.0.rc0.104.g0611611a94-goog

[PATCH v2 1/2] media: vimc: add ancillary lens

[Yunke Cao]

Add a basic version of vimc lens.
The lens supports V4L2_CID_FOCUS_ABSOLUTE control.
Link the lens with vimc sensors using media-controller
ancillary links.

This change can be used to test the recently added ancillary
links.

Reviewed-by: Shuah Khan <skhan@linuxfoundation.org>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
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.37.0.rc0.104.g0611611a94-goog

[PATCH v2 2/2] media: vimc: documentation for lens

[Yunke Cao]

Add documentation for vimc-lens.
Add a lens into the vimc topology graph.

Reviewed-by: Shuah Khan <skhan@linuxfoundation.org>
Signed-off-by: Yunke Cao <yunkec@google.com>
---
 Documentation/admin-guide/media/vimc.dot | 4 ++++
 Documentation/admin-guide/media/vimc.rst | 4 ++++
 2 files changed, 8 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..3ec2198ce68d 100644
--- a/Documentation/admin-guide/media/vimc.rst
+++ b/Documentation/admin-guide/media/vimc.rst
@@ -53,6 +53,10 @@ vimc-sensor:
 
 	* 1 Pad source
 
+vimc-lens:
+	Ancillary lens for a sensor. Supports auto focus control. Linked to
+	a vimc-sensor using an ancillary link.
+
 vimc-debayer:
 	Transforms images in bayer format into a non-bayer format.
 	Exposes:
-- 
2.37.0.rc0.104.g0611611a94-goog

Re: [PATCH v2 1/2] media: vimc: add ancillary lens

[Shuah Khan]

On 6/16/22 7:57 PM, Yunke Cao wrote:
> Add a basic version of vimc lens.
> The lens supports V4L2_CID_FOCUS_ABSOLUTE control.
> Link the lens with vimc sensors using media-controller
> ancillary links.
> 

Commit log lines are usually ~75 charracters long. Make it easier
to read.

> This change can be used to test the recently added ancillary
> links.
> 

Care to add instructions on how one would test ancillary with
this feature?

> Reviewed-by: Shuah Khan <skhan@linuxfoundation.org>
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> 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
> 

thanks,
-- Shuah

Re: [PATCH v2 1/2] media: vimc: add ancillary lens

[Yunke Cao]

Hi Shuah,

Thanks for the review.

On Sat, Jun 18, 2022 at 5:01 AM Shuah Khan <skhan@linuxfoundation.org> wrote:
>
> On 6/16/22 7:57 PM, Yunke Cao wrote:
> > Add a basic version of vimc lens.
> > The lens supports V4L2_CID_FOCUS_ABSOLUTE control.
> > Link the lens with vimc sensors using media-controller
> > ancillary links.
> >
>
> Commit log lines are usually ~75 charracters long. Make it easier
> to read.
That's good to know. Thanks!
Should I send v3 and trim the commit log?
I'm thinking something like this:

The lens supports FOCUS_ABSOLUTE control.
Link the lens with sensors using ancillary links.

>
> > This change can be used to test the recently added ancillary
> > links.
> >
>
> Care to add instructions on how one would test ancillary with
> this feature?

The lens shows up in the media topology. I documented it in 2/2.
Not sure what else is necessary here.

Best,
Yunke

>
> > Reviewed-by: Shuah Khan <skhan@linuxfoundation.org>
> > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> > 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
> >
>
> thanks,
> -- Shuah

Re: [PATCH v2 1/2] media: vimc: add ancillary lens

[Shuah Khan]

On 6/19/22 7:57 PM, Yunke Cao wrote:
> Hi Shuah,
> 
> Thanks for the review.
> 
> On Sat, Jun 18, 2022 at 5:01 AM Shuah Khan <skhan@linuxfoundation.org> wrote:
>>
>> On 6/16/22 7:57 PM, Yunke Cao wrote:
>>> Add a basic version of vimc lens.
>>> The lens supports V4L2_CID_FOCUS_ABSOLUTE control.
>>> Link the lens with vimc sensors using media-controller
>>> ancillary links.
>>>
>>
>> Commit log lines are usually ~75 charracters long. Make it easier
>> to read.
> That's good to know. Thanks!
> Should I send v3 and trim the commit log?
> I'm thinking something like this:
> 
> The lens supports FOCUS_ABSOLUTE control.
> Link the lens with sensors using ancillary links.
> 

Why is this necessary? How did you test this change? How could
one use this feature?

>>
>>> This change can be used to test the recently added ancillary
>>> links.
>>>
>>
>> Care to add instructions on how one would test ancillary with
>> this feature?
> 
> The lens shows up in the media topology. I documented it in 2/2.
> Not sure what else is necessary here.
> 

Why is this necessary? How did you test this change? How could
one use this feature?

Take a look at the some of the other commit messages e.g:
4a2e0a806cb58a4d3106add079488e0b56a221b6
5f3fb5c54d67670fa6743d2434a5bd43a97c01de

This one is a good example of what would a commit log adding a
new feature should include.

commit 9b4a9b31b9aeef262b4fa211f2083c30c4391df7
Author: Pedro Terra <pedro@terraco.de>
Date:   Tue Aug 31 19:48:22 2021 +0200

     media: vimc: Enable set resolution at the scaler src pad

thanks,
-- Shuah

Re: [PATCH v2 1/2] media: vimc: add ancillary lens

[Yunke Cao]

Hi Shuah,

Thanks for the pointers.

On Wed, Jun 22, 2022 at 12:57 AM Shuah Khan <skhan@linuxfoundation.org> wrote:
>
> On 6/19/22 7:57 PM, Yunke Cao wrote:
> > Hi Shuah,
> >
> > Thanks for the review.
> >
> > On Sat, Jun 18, 2022 at 5:01 AM Shuah Khan <skhan@linuxfoundation.org> wrote:
> >>
> >> On 6/16/22 7:57 PM, Yunke Cao wrote:
> >>> Add a basic version of vimc lens.
> >>> The lens supports V4L2_CID_FOCUS_ABSOLUTE control.
> >>> Link the lens with vimc sensors using media-controller
> >>> ancillary links.
> >>>
> >>
> >> Commit log lines are usually ~75 charracters long. Make it easier
> >> to read.
> > That's good to know. Thanks!
> > Should I send v3 and trim the commit log?
> > I'm thinking something like this:
> >
> > The lens supports FOCUS_ABSOLUTE control.
> > Link the lens with sensors using ancillary links.
> >
>
> Why is this necessary? How did you test this change? How could
> one use this feature?
>

Add lens to vimc driver and link them with sensors using ancillary links.
Provides an example of ancillary link usage.The lens supports
FOCUS_ABSOLUTE control.

Test example: With default vimc topology
> media-ctl -p
Media controller API version 5.18.0
…
- entity 28: Lens A (0 pad, 0 link)
             type V4L2 subdev subtype Lens flags 0
             device node name /dev/v4l-subdev6
- entity 29: Lens B (0 pad, 0 link)
             type V4L2 subdev subtype Lens flags 0
             device node name /dev/v4l-subdev7
>  v4l2-ctl -d /dev/v4l-subdev7 -C focus_absolute
focus_absolute: 0

Let me know what you think. Thanks!

Best,
Yunke

> >>
> >>> This change can be used to test the recently added ancillary
> >>> links.
> >>>
> >>
> >> Care to add instructions on how one would test ancillary with
> >> this feature?
> >
> > The lens shows up in the media topology. I documented it in 2/2.
> > Not sure what else is necessary here.
> >
>
> Why is this necessary? How did you test this change? How could
> one use this feature?
>
> Take a look at the some of the other commit messages e.g:
> 4a2e0a806cb58a4d3106add079488e0b56a221b6
> 5f3fb5c54d67670fa6743d2434a5bd43a97c01de
>
> This one is a good example of what would a commit log adding a
> new feature should include.
>
> commit 9b4a9b31b9aeef262b4fa211f2083c30c4391df7
> Author: Pedro Terra <pedro@terraco.de>
> Date:   Tue Aug 31 19:48:22 2021 +0200
>
>      media: vimc: Enable set resolution at the scaler src pad
>
> thanks,
> -- Shuah

Re: [PATCH v2 1/2] media: vimc: add ancillary lens

[Shuah Khan]

On 6/26/22 7:30 PM, Yunke Cao wrote:
> Hi Shuah,
> 
> Thanks for the pointers.
> 

>>
>> Why is this necessary? How did you test this change? How could
>> one use this feature?
>>
> 
> Add lens to vimc driver and link them with sensors using ancillary links.
> Provides an example of ancillary link usage.The lens supports
> FOCUS_ABSOLUTE control.
> 
> Test example: With default vimc topology
>> media-ctl -p
> Media controller API version 5.18.0
> …
> - entity 28: Lens A (0 pad, 0 link)
>               type V4L2 subdev subtype Lens flags 0
>               device node name /dev/v4l-subdev6
> - entity 29: Lens B (0 pad, 0 link)
>               type V4L2 subdev subtype Lens flags 0
>               device node name /dev/v4l-subdev7
>>   v4l2-ctl -d /dev/v4l-subdev7 -C focus_absolute
> focus_absolute: 0
> 

Send me v3 with all of this information in the commit log and add
this to document as well.

thanks,
-- Shuah

Re: [PATCH v2 1/2] media: vimc: add ancillary lens

[Yunke Cao]

Thanks Shuah. Just sent out v3.

On Tue, Jun 28, 2022 at 12:43 AM Shuah Khan <skhan@linuxfoundation.org> wrote:
>
> On 6/26/22 7:30 PM, Yunke Cao wrote:
> > Hi Shuah,
> >
> > Thanks for the pointers.
> >
>
> >>
> >> Why is this necessary? How did you test this change? How could
> >> one use this feature?
> >>
> >
> > Add lens to vimc driver and link them with sensors using ancillary links.
> > Provides an example of ancillary link usage.The lens supports
> > FOCUS_ABSOLUTE control.
> >
> > Test example: With default vimc topology
> >> media-ctl -p
> > Media controller API version 5.18.0
> > …
> > - entity 28: Lens A (0 pad, 0 link)
> >               type V4L2 subdev subtype Lens flags 0
> >               device node name /dev/v4l-subdev6
> > - entity 29: Lens B (0 pad, 0 link)
> >               type V4L2 subdev subtype Lens flags 0
> >               device node name /dev/v4l-subdev7
> >>   v4l2-ctl -d /dev/v4l-subdev7 -C focus_absolute
> > focus_absolute: 0
> >
>
> Send me v3 with all of this information in the commit log and add
> this to document as well.
>
> thanks,
> -- Shuah