* [PATCH 1/9] media: atomisp: Remove isp_subdev_propagate()
2024-02-17 11:24 [PATCH 0/9] media: atomisp: Changes for libcamera support Hans de Goede
@ 2024-02-17 11:24 ` Hans de Goede
2024-02-17 14:57 ` Kieran Bingham
2024-02-17 11:24 ` [PATCH 2/9] media: atomisp: Rename atomisp_set_crop_and_fmt() Hans de Goede
` (8 subsequent siblings)
9 siblings, 1 reply; 23+ messages in thread
From: Hans de Goede @ 2024-02-17 11:24 UTC (permalink / raw)
To: Mauro Carvalho Chehab, Sakari Ailus, Andy Shevchenko
Cc: Hans de Goede, Kate Hsuan, Tsuchiya Yuto, Yury Luneff, Nable,
andrey.i.trufanov, Fabio Aiuto, linux-media, linux-staging
isp_subdev_propagate() is a wrapper around atomisp_subdev_set_selection()
which gets only used in a single place.
Scall atomisp_subdev_set_selection() directly in that single place instead.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
.../media/atomisp/pci/atomisp_subdev.c | 38 ++++---------------
1 file changed, 7 insertions(+), 31 deletions(-)
diff --git a/drivers/staging/media/atomisp/pci/atomisp_subdev.c b/drivers/staging/media/atomisp/pci/atomisp_subdev.c
index a87fc74159e2..8293bda0c681 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_subdev.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_subdev.c
@@ -287,35 +287,6 @@ static void isp_get_fmt_rect(struct v4l2_subdev *sd,
}
}
-static void isp_subdev_propagate(struct v4l2_subdev *sd,
- struct v4l2_subdev_state *sd_state,
- u32 which, uint32_t pad, uint32_t target,
- uint32_t flags)
-{
- struct v4l2_mbus_framefmt *ffmt[ATOMISP_SUBDEV_PADS_NUM];
- struct v4l2_rect *crop[ATOMISP_SUBDEV_PADS_NUM],
- *comp[ATOMISP_SUBDEV_PADS_NUM];
-
- if (flags & V4L2_SEL_FLAG_KEEP_CONFIG)
- return;
-
- isp_get_fmt_rect(sd, sd_state, which, ffmt, crop, comp);
-
- switch (pad) {
- case ATOMISP_SUBDEV_PAD_SINK: {
- struct v4l2_rect r = {0};
-
- /* Only crop target supported on sink pad. */
- r.width = ffmt[pad]->width;
- r.height = ffmt[pad]->height;
-
- atomisp_subdev_set_selection(sd, sd_state, which, pad,
- target, flags, &r);
- break;
- }
- }
-}
-
static int isp_subdev_get_selection(struct v4l2_subdev *sd,
struct v4l2_subdev_state *sd_state,
struct v4l2_subdev_selection *sel)
@@ -541,6 +512,7 @@ void atomisp_subdev_set_ffmt(struct v4l2_subdev *sd,
case ATOMISP_SUBDEV_PAD_SINK: {
const struct atomisp_in_fmt_conv *fc =
atomisp_find_in_fmt_conv(ffmt->code);
+ struct v4l2_rect r = {};
if (!fc) {
fc = atomisp_in_fmt_conv;
@@ -551,8 +523,12 @@ void atomisp_subdev_set_ffmt(struct v4l2_subdev *sd,
*__ffmt = *ffmt;
- isp_subdev_propagate(sd, sd_state, which, pad,
- V4L2_SEL_TGT_CROP, 0);
+ /* Propagate new ffmt to selection */
+ r.width = ffmt->width;
+ r.height = ffmt->height;
+ /* Only crop target supported on sink pad. */
+ atomisp_subdev_set_selection(sd, sd_state, which, pad,
+ V4L2_SEL_TGT_CROP, 0, &r);
if (which == V4L2_SUBDEV_FORMAT_ACTIVE) {
atomisp_css_input_set_resolution(isp_sd,
--
2.43.0
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH 1/9] media: atomisp: Remove isp_subdev_propagate()
2024-02-17 11:24 ` [PATCH 1/9] media: atomisp: Remove isp_subdev_propagate() Hans de Goede
@ 2024-02-17 14:57 ` Kieran Bingham
0 siblings, 0 replies; 23+ messages in thread
From: Kieran Bingham @ 2024-02-17 14:57 UTC (permalink / raw)
To: Andy Shevchenko, Hans de Goede, Mauro Carvalho Chehab,
Sakari Ailus
Cc: Hans de Goede, Kate Hsuan, Tsuchiya Yuto, Yury Luneff, Nable,
andrey.i.trufanov, Fabio Aiuto, linux-media, linux-staging
Quoting Hans de Goede (2024-02-17 11:24:30)
> isp_subdev_propagate() is a wrapper around atomisp_subdev_set_selection()
> which gets only used in a single place.
>
> Scall atomisp_subdev_set_selection() directly in that single place instead.
s/Scall/Call/
Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
> .../media/atomisp/pci/atomisp_subdev.c | 38 ++++---------------
> 1 file changed, 7 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/staging/media/atomisp/pci/atomisp_subdev.c b/drivers/staging/media/atomisp/pci/atomisp_subdev.c
> index a87fc74159e2..8293bda0c681 100644
> --- a/drivers/staging/media/atomisp/pci/atomisp_subdev.c
> +++ b/drivers/staging/media/atomisp/pci/atomisp_subdev.c
> @@ -287,35 +287,6 @@ static void isp_get_fmt_rect(struct v4l2_subdev *sd,
> }
> }
>
> -static void isp_subdev_propagate(struct v4l2_subdev *sd,
> - struct v4l2_subdev_state *sd_state,
> - u32 which, uint32_t pad, uint32_t target,
> - uint32_t flags)
> -{
> - struct v4l2_mbus_framefmt *ffmt[ATOMISP_SUBDEV_PADS_NUM];
> - struct v4l2_rect *crop[ATOMISP_SUBDEV_PADS_NUM],
> - *comp[ATOMISP_SUBDEV_PADS_NUM];
> -
> - if (flags & V4L2_SEL_FLAG_KEEP_CONFIG)
> - return;
> -
> - isp_get_fmt_rect(sd, sd_state, which, ffmt, crop, comp);
> -
> - switch (pad) {
> - case ATOMISP_SUBDEV_PAD_SINK: {
> - struct v4l2_rect r = {0};
> -
> - /* Only crop target supported on sink pad. */
> - r.width = ffmt[pad]->width;
> - r.height = ffmt[pad]->height;
> -
> - atomisp_subdev_set_selection(sd, sd_state, which, pad,
> - target, flags, &r);
> - break;
> - }
> - }
> -}
> -
> static int isp_subdev_get_selection(struct v4l2_subdev *sd,
> struct v4l2_subdev_state *sd_state,
> struct v4l2_subdev_selection *sel)
> @@ -541,6 +512,7 @@ void atomisp_subdev_set_ffmt(struct v4l2_subdev *sd,
> case ATOMISP_SUBDEV_PAD_SINK: {
> const struct atomisp_in_fmt_conv *fc =
> atomisp_find_in_fmt_conv(ffmt->code);
> + struct v4l2_rect r = {};
>
> if (!fc) {
> fc = atomisp_in_fmt_conv;
> @@ -551,8 +523,12 @@ void atomisp_subdev_set_ffmt(struct v4l2_subdev *sd,
>
> *__ffmt = *ffmt;
>
> - isp_subdev_propagate(sd, sd_state, which, pad,
> - V4L2_SEL_TGT_CROP, 0);
> + /* Propagate new ffmt to selection */
> + r.width = ffmt->width;
> + r.height = ffmt->height;
> + /* Only crop target supported on sink pad. */
> + atomisp_subdev_set_selection(sd, sd_state, which, pad,
> + V4L2_SEL_TGT_CROP, 0, &r);
>
> if (which == V4L2_SUBDEV_FORMAT_ACTIVE) {
> atomisp_css_input_set_resolution(isp_sd,
> --
> 2.43.0
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 2/9] media: atomisp: Rename atomisp_set_crop_and_fmt()
2024-02-17 11:24 [PATCH 0/9] media: atomisp: Changes for libcamera support Hans de Goede
2024-02-17 11:24 ` [PATCH 1/9] media: atomisp: Remove isp_subdev_propagate() Hans de Goede
@ 2024-02-17 11:24 ` Hans de Goede
2024-02-17 15:07 ` Kieran Bingham
2024-02-17 11:24 ` [PATCH 3/9] media: atomisp: Remove custom VCM handling Hans de Goede
` (7 subsequent siblings)
9 siblings, 1 reply; 23+ messages in thread
From: Hans de Goede @ 2024-02-17 11:24 UTC (permalink / raw)
To: Mauro Carvalho Chehab, Sakari Ailus, Andy Shevchenko
Cc: Hans de Goede, Kate Hsuan, Tsuchiya Yuto, Yury Luneff, Nable,
andrey.i.trufanov, Fabio Aiuto, linux-media, linux-staging
Rename atomisp_set_crop_and_fmt() to atomisp_set_sensor_crop_and_fmt()
to make clear that it operates on the sensor subdev.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
drivers/staging/media/atomisp/pci/atomisp_cmd.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/staging/media/atomisp/pci/atomisp_cmd.c b/drivers/staging/media/atomisp/pci/atomisp_cmd.c
index 8593ba90605f..eb37bb6e41f9 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_cmd.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_cmd.c
@@ -3721,9 +3721,9 @@ void atomisp_get_padding(struct atomisp_device *isp, u32 width, u32 height,
*padding_h = max_t(u32, *padding_h, min_pad_h);
}
-static int atomisp_set_crop_and_fmt(struct atomisp_device *isp,
- struct v4l2_mbus_framefmt *ffmt,
- int which)
+static int atomisp_set_sensor_crop_and_fmt(struct atomisp_device *isp,
+ struct v4l2_mbus_framefmt *ffmt,
+ int which)
{
struct atomisp_input_subdev *input = &isp->inputs[isp->asd.input_curr];
struct v4l2_subdev_selection sel = {
@@ -3817,7 +3817,7 @@ int atomisp_try_fmt(struct atomisp_device *isp, struct v4l2_pix_format *f,
dev_dbg(isp->dev, "try_mbus_fmt: try %ux%u\n", ffmt.width, ffmt.height);
- ret = atomisp_set_crop_and_fmt(isp, &ffmt, V4L2_SUBDEV_FORMAT_TRY);
+ ret = atomisp_set_sensor_crop_and_fmt(isp, &ffmt, V4L2_SUBDEV_FORMAT_TRY);
if (ret)
return ret;
@@ -4263,7 +4263,7 @@ static int atomisp_set_fmt_to_snr(struct video_device *vdev, const struct v4l2_p
/* Disable dvs if resolution can't be supported by sensor */
if (asd->params.video_dis_en && asd->run_mode->val == ATOMISP_RUN_MODE_VIDEO) {
- ret = atomisp_set_crop_and_fmt(isp, &ffmt, V4L2_SUBDEV_FORMAT_TRY);
+ ret = atomisp_set_sensor_crop_and_fmt(isp, &ffmt, V4L2_SUBDEV_FORMAT_TRY);
if (ret)
return ret;
@@ -4281,7 +4281,7 @@ static int atomisp_set_fmt_to_snr(struct video_device *vdev, const struct v4l2_p
}
}
- ret = atomisp_set_crop_and_fmt(isp, &ffmt, V4L2_SUBDEV_FORMAT_ACTIVE);
+ ret = atomisp_set_sensor_crop_and_fmt(isp, &ffmt, V4L2_SUBDEV_FORMAT_ACTIVE);
if (ret)
return ret;
--
2.43.0
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH 2/9] media: atomisp: Rename atomisp_set_crop_and_fmt()
2024-02-17 11:24 ` [PATCH 2/9] media: atomisp: Rename atomisp_set_crop_and_fmt() Hans de Goede
@ 2024-02-17 15:07 ` Kieran Bingham
0 siblings, 0 replies; 23+ messages in thread
From: Kieran Bingham @ 2024-02-17 15:07 UTC (permalink / raw)
To: Andy Shevchenko, Hans de Goede, Mauro Carvalho Chehab,
Sakari Ailus
Cc: Hans de Goede, Kate Hsuan, Tsuchiya Yuto, Yury Luneff, Nable,
andrey.i.trufanov, Fabio Aiuto, linux-media, linux-staging
Quoting Hans de Goede (2024-02-17 11:24:31)
> Rename atomisp_set_crop_and_fmt() to atomisp_set_sensor_crop_and_fmt()
> to make clear that it operates on the sensor subdev.
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
> drivers/staging/media/atomisp/pci/atomisp_cmd.c | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/staging/media/atomisp/pci/atomisp_cmd.c b/drivers/staging/media/atomisp/pci/atomisp_cmd.c
> index 8593ba90605f..eb37bb6e41f9 100644
> --- a/drivers/staging/media/atomisp/pci/atomisp_cmd.c
> +++ b/drivers/staging/media/atomisp/pci/atomisp_cmd.c
> @@ -3721,9 +3721,9 @@ void atomisp_get_padding(struct atomisp_device *isp, u32 width, u32 height,
> *padding_h = max_t(u32, *padding_h, min_pad_h);
> }
>
> -static int atomisp_set_crop_and_fmt(struct atomisp_device *isp,
> - struct v4l2_mbus_framefmt *ffmt,
> - int which)
> +static int atomisp_set_sensor_crop_and_fmt(struct atomisp_device *isp,
> + struct v4l2_mbus_framefmt *ffmt,
> + int which)
I went to look this up and I only see atomisp_set_crop() so I guess I've
missed some patches.
Does the atomisp use the usual v4l2-subdev drivers for sensors? or are
they separate for now ?
I'm curious about how it's handling the binning in that function, but
that's not specific to this patch which looks reasonable so far so:
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> {
> struct atomisp_input_subdev *input = &isp->inputs[isp->asd.input_curr];
> struct v4l2_subdev_selection sel = {
> @@ -3817,7 +3817,7 @@ int atomisp_try_fmt(struct atomisp_device *isp, struct v4l2_pix_format *f,
>
> dev_dbg(isp->dev, "try_mbus_fmt: try %ux%u\n", ffmt.width, ffmt.height);
>
> - ret = atomisp_set_crop_and_fmt(isp, &ffmt, V4L2_SUBDEV_FORMAT_TRY);
> + ret = atomisp_set_sensor_crop_and_fmt(isp, &ffmt, V4L2_SUBDEV_FORMAT_TRY);
> if (ret)
> return ret;
>
> @@ -4263,7 +4263,7 @@ static int atomisp_set_fmt_to_snr(struct video_device *vdev, const struct v4l2_p
>
> /* Disable dvs if resolution can't be supported by sensor */
> if (asd->params.video_dis_en && asd->run_mode->val == ATOMISP_RUN_MODE_VIDEO) {
> - ret = atomisp_set_crop_and_fmt(isp, &ffmt, V4L2_SUBDEV_FORMAT_TRY);
> + ret = atomisp_set_sensor_crop_and_fmt(isp, &ffmt, V4L2_SUBDEV_FORMAT_TRY);
> if (ret)
> return ret;
>
> @@ -4281,7 +4281,7 @@ static int atomisp_set_fmt_to_snr(struct video_device *vdev, const struct v4l2_p
> }
> }
>
> - ret = atomisp_set_crop_and_fmt(isp, &ffmt, V4L2_SUBDEV_FORMAT_ACTIVE);
> + ret = atomisp_set_sensor_crop_and_fmt(isp, &ffmt, V4L2_SUBDEV_FORMAT_ACTIVE);
> if (ret)
> return ret;
>
> --
> 2.43.0
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 3/9] media: atomisp: Remove custom VCM handling
2024-02-17 11:24 [PATCH 0/9] media: atomisp: Changes for libcamera support Hans de Goede
2024-02-17 11:24 ` [PATCH 1/9] media: atomisp: Remove isp_subdev_propagate() Hans de Goede
2024-02-17 11:24 ` [PATCH 2/9] media: atomisp: Rename atomisp_set_crop_and_fmt() Hans de Goede
@ 2024-02-17 11:24 ` Hans de Goede
2024-02-17 15:09 ` Kieran Bingham
2024-02-17 11:24 ` [PATCH 4/9] media: atomisp: Remove ISP controls which get passed through to the camera Hans de Goede
` (6 subsequent siblings)
9 siblings, 1 reply; 23+ messages in thread
From: Hans de Goede @ 2024-02-17 11:24 UTC (permalink / raw)
To: Mauro Carvalho Chehab, Sakari Ailus, Andy Shevchenko
Cc: Hans de Goede, Kate Hsuan, Tsuchiya Yuto, Yury Luneff, Nable,
andrey.i.trufanov, Fabio Aiuto, linux-media, linux-staging
Remove the custom VCM handling, instead the VCM should be controlled
through its own v4l2-subdev and the new ipu-bridge.c code already
supports instantiating an i2c_client for this and setting up
the necessary endpoints in the fwnode graph.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
.../media/atomisp/include/linux/atomisp.h | 3 -
.../atomisp/include/linux/atomisp_platform.h | 5 +-
.../media/atomisp/pci/atomisp_internal.h | 4 -
.../staging/media/atomisp/pci/atomisp_ioctl.c | 86 +------------------
.../staging/media/atomisp/pci/atomisp_v4l2.c | 15 ----
5 files changed, 3 insertions(+), 110 deletions(-)
diff --git a/drivers/staging/media/atomisp/include/linux/atomisp.h b/drivers/staging/media/atomisp/include/linux/atomisp.h
index bbbd904b696a..d9a7a599038d 100644
--- a/drivers/staging/media/atomisp/include/linux/atomisp.h
+++ b/drivers/staging/media/atomisp/include/linux/atomisp.h
@@ -914,9 +914,6 @@ enum atomisp_burst_capture_options {
/* VCM step time */
#define V4L2_CID_VCM_TIMING (V4L2_CID_CAMERA_LASTP1 + 12)
-/* Query Focus Status */
-#define V4L2_CID_FOCUS_STATUS (V4L2_CID_CAMERA_LASTP1 + 14)
-
/* number of frames to skip at stream start */
#define V4L2_CID_G_SKIP_FRAMES (V4L2_CID_CAMERA_LASTP1 + 17)
diff --git a/drivers/staging/media/atomisp/include/linux/atomisp_platform.h b/drivers/staging/media/atomisp/include/linux/atomisp_platform.h
index 487ef5846c24..2535402afd73 100644
--- a/drivers/staging/media/atomisp/include/linux/atomisp_platform.h
+++ b/drivers/staging/media/atomisp/include/linux/atomisp_platform.h
@@ -111,9 +111,8 @@ enum atomisp_input_format {
enum intel_v4l2_subdev_type {
RAW_CAMERA = 1,
- CAMERA_MOTOR = 2,
- LED_FLASH = 3,
- TEST_PATTERN = 4,
+ LED_FLASH = 2,
+ TEST_PATTERN = 3,
};
struct intel_v4l2_subdev_id {
diff --git a/drivers/staging/media/atomisp/pci/atomisp_internal.h b/drivers/staging/media/atomisp/pci/atomisp_internal.h
index bba9bc64d447..ca8ed3a6b9b8 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_internal.h
+++ b/drivers/staging/media/atomisp/pci/atomisp_internal.h
@@ -134,9 +134,6 @@ struct atomisp_input_subdev {
struct v4l2_rect active_rect;
/* Sensor state for which == V4L2_SUBDEV_FORMAT_TRY calls */
struct v4l2_subdev_state *try_sd_state;
-
- struct v4l2_subdev *motor;
-
/*
* To show this resource is used by
* which stream, in ISP multiple stream mode
@@ -210,7 +207,6 @@ struct atomisp_device {
unsigned int input_cnt;
struct atomisp_input_subdev inputs[ATOM_ISP_MAX_INPUTS];
struct v4l2_subdev *flash;
- struct v4l2_subdev *motor;
struct atomisp_regs saved_regs;
struct atomisp_css_env css_env;
diff --git a/drivers/staging/media/atomisp/pci/atomisp_ioctl.c b/drivers/staging/media/atomisp/pci/atomisp_ioctl.c
index bb8e5e883b50..ef555054fdbf 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_ioctl.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_ioctl.c
@@ -557,7 +557,6 @@ static int atomisp_enum_input(struct file *file, void *fh,
struct video_device *vdev = video_devdata(file);
struct atomisp_device *isp = video_get_drvdata(vdev);
int index = input->index;
- struct v4l2_subdev *motor;
if (index >= isp->input_cnt)
return -EINVAL;
@@ -569,28 +568,6 @@ static int atomisp_enum_input(struct file *file, void *fh,
strscpy(input->name, isp->inputs[index].camera->name,
sizeof(input->name));
- /*
- * HACK: append actuator's name to sensor's
- * As currently userspace can't talk directly to subdev nodes, this
- * ioctl is the only way to enum inputs + possible external actuators
- * for 3A tuning purpose.
- */
- if (!IS_ISP2401)
- motor = isp->inputs[index].motor;
- else
- motor = isp->motor;
-
- if (motor && strlen(motor->name) > 0) {
- const int cur_len = strlen(input->name);
- const int max_size = sizeof(input->name) - cur_len - 1;
-
- if (max_size > 1) {
- input->name[cur_len] = '+';
- strscpy(&input->name[cur_len + 1],
- motor->name, max_size);
- }
- }
-
input->type = V4L2_INPUT_TYPE_CAMERA;
input->index = index;
input->reserved[0] = isp->inputs[index].type;
@@ -629,7 +606,6 @@ static int atomisp_s_input(struct file *file, void *fh, unsigned int input)
struct atomisp_video_pipe *pipe = atomisp_to_video_pipe(vdev);
struct atomisp_sub_device *asd = pipe->asd;
struct v4l2_subdev *camera = NULL;
- struct v4l2_subdev *motor;
int ret;
ret = atomisp_pipe_check(pipe, true);
@@ -666,17 +642,6 @@ static int atomisp_s_input(struct file *file, void *fh, unsigned int input)
return ret;
}
- if (!IS_ISP2401) {
- motor = isp->inputs[input].motor;
- } else {
- motor = isp->motor;
- if (motor)
- ret = v4l2_subdev_call(motor, core, s_power, 1);
- }
-
- if (motor)
- ret = v4l2_subdev_call(motor, core, init, 1);
-
asd->input_curr = input;
/* mark this camera is used by the current stream */
isp->inputs[input].asd = asd;
@@ -1433,26 +1398,8 @@ static int atomisp_s_ctrl(struct file *file, void *fh,
static int atomisp_queryctl(struct file *file, void *fh,
struct v4l2_queryctrl *qc)
{
- int i, ret = -EINVAL;
struct video_device *vdev = video_devdata(file);
- struct atomisp_sub_device *asd = atomisp_to_video_pipe(vdev)->asd;
- struct atomisp_device *isp = video_get_drvdata(vdev);
-
- switch (qc->id) {
- case V4L2_CID_FOCUS_ABSOLUTE:
- case V4L2_CID_FOCUS_RELATIVE:
- case V4L2_CID_FOCUS_STATUS:
- if (!IS_ISP2401) {
- return v4l2_queryctrl(isp->inputs[asd->input_curr].camera->
- ctrl_handler, qc);
- }
- /* ISP2401 */
- if (isp->motor)
- return v4l2_queryctrl(isp->motor->ctrl_handler, qc);
- else
- return v4l2_queryctrl(isp->inputs[asd->input_curr].
- camera->ctrl_handler, qc);
- }
+ int i, ret = -EINVAL;
if (qc->id & V4L2_CTRL_FLAG_NEXT_CTRL)
return ret;
@@ -1478,16 +1425,10 @@ static int atomisp_camera_g_ext_ctrls(struct file *file, void *fh,
struct video_device *vdev = video_devdata(file);
struct atomisp_sub_device *asd = atomisp_to_video_pipe(vdev)->asd;
struct atomisp_device *isp = video_get_drvdata(vdev);
- struct v4l2_subdev *motor;
struct v4l2_control ctrl;
int i;
int ret = 0;
- if (!IS_ISP2401)
- motor = isp->inputs[asd->input_curr].motor;
- else
- motor = isp->motor;
-
for (i = 0; i < c->count; i++) {
ctrl.id = c->controls[i].id;
ctrl.value = c->controls[i].value;
@@ -1509,13 +1450,6 @@ static int atomisp_camera_g_ext_ctrls(struct file *file, void *fh,
v4l2_g_ctrl(isp->inputs[asd->input_curr].camera->
ctrl_handler, &ctrl);
break;
- case V4L2_CID_FOCUS_ABSOLUTE:
- case V4L2_CID_FOCUS_RELATIVE:
- case V4L2_CID_FOCUS_STATUS:
- case V4L2_CID_FOCUS_AUTO:
- if (motor)
- ret = v4l2_g_ctrl(motor->ctrl_handler, &ctrl);
- break;
case V4L2_CID_FLASH_STATUS:
case V4L2_CID_FLASH_INTENSITY:
case V4L2_CID_FLASH_TORCH_INTENSITY:
@@ -1584,16 +1518,10 @@ static int atomisp_camera_s_ext_ctrls(struct file *file, void *fh,
struct video_device *vdev = video_devdata(file);
struct atomisp_sub_device *asd = atomisp_to_video_pipe(vdev)->asd;
struct atomisp_device *isp = video_get_drvdata(vdev);
- struct v4l2_subdev *motor;
struct v4l2_control ctrl;
int i;
int ret = 0;
- if (!IS_ISP2401)
- motor = isp->inputs[asd->input_curr].motor;
- else
- motor = isp->motor;
-
for (i = 0; i < c->count; i++) {
struct v4l2_ctrl *ctr;
@@ -1616,18 +1544,6 @@ static int atomisp_camera_s_ext_ctrls(struct file *file, void *fh,
isp->inputs[asd->input_curr].camera->
ctrl_handler, &ctrl);
break;
- case V4L2_CID_FOCUS_ABSOLUTE:
- case V4L2_CID_FOCUS_RELATIVE:
- case V4L2_CID_FOCUS_STATUS:
- case V4L2_CID_FOCUS_AUTO:
- if (motor)
- ret = v4l2_s_ctrl(NULL, motor->ctrl_handler,
- &ctrl);
- else
- ret = v4l2_s_ctrl(NULL,
- isp->inputs[asd->input_curr].
- camera->ctrl_handler, &ctrl);
- break;
case V4L2_CID_FLASH_STATUS:
case V4L2_CID_FLASH_INTENSITY:
case V4L2_CID_FLASH_TORCH_INTENSITY:
diff --git a/drivers/staging/media/atomisp/pci/atomisp_v4l2.c b/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
index f736e54c7df3..1a936dbe8eb4 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
@@ -823,13 +823,6 @@ static int atomisp_subdev_probe(struct atomisp_device *isp)
isp->sensor_lanes[mipi_port] = subdevs->lanes;
isp->sensor_subdevs[subdevs->port] = subdevs->subdev;
break;
- case CAMERA_MOTOR:
- if (isp->motor) {
- dev_warn(isp->dev, "too many atomisp motors\n");
- continue;
- }
- isp->motor = subdevs->subdev;
- break;
case LED_FLASH:
if (isp->flash) {
dev_warn(isp->dev, "too many atomisp flash devices\n");
@@ -1066,14 +1059,6 @@ int atomisp_register_device_nodes(struct atomisp_device *isp)
atomisp_init_sensor(input);
- /*
- * HACK: Currently VCM belongs to primary sensor only, but correct
- * approach must be to acquire from platform code which sensor
- * owns it.
- */
- if (i == ATOMISP_CAMERA_PORT_PRIMARY)
- input->motor = isp->motor;
-
err = media_create_pad_link(&input->camera->entity, 0,
&isp->csi2_port[i].subdev.entity,
CSI2_PAD_SINK,
--
2.43.0
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH 3/9] media: atomisp: Remove custom VCM handling
2024-02-17 11:24 ` [PATCH 3/9] media: atomisp: Remove custom VCM handling Hans de Goede
@ 2024-02-17 15:09 ` Kieran Bingham
0 siblings, 0 replies; 23+ messages in thread
From: Kieran Bingham @ 2024-02-17 15:09 UTC (permalink / raw)
To: Andy Shevchenko, Hans de Goede, Mauro Carvalho Chehab,
Sakari Ailus
Cc: Hans de Goede, Kate Hsuan, Tsuchiya Yuto, Yury Luneff, Nable,
andrey.i.trufanov, Fabio Aiuto, linux-media, linux-staging
Quoting Hans de Goede (2024-02-17 11:24:32)
> Remove the custom VCM handling, instead the VCM should be controlled
> through its own v4l2-subdev and the new ipu-bridge.c code already
> supports instantiating an i2c_client for this and setting up
> the necessary endpoints in the fwnode graph.
Agreed, this sounds like a good move forwards to align with other ISPs
and existing VCM driver frameworks. I must get back to my rework of the
VCM drivers I started though!
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
> .../media/atomisp/include/linux/atomisp.h | 3 -
> .../atomisp/include/linux/atomisp_platform.h | 5 +-
> .../media/atomisp/pci/atomisp_internal.h | 4 -
> .../staging/media/atomisp/pci/atomisp_ioctl.c | 86 +------------------
> .../staging/media/atomisp/pci/atomisp_v4l2.c | 15 ----
> 5 files changed, 3 insertions(+), 110 deletions(-)
>
> diff --git a/drivers/staging/media/atomisp/include/linux/atomisp.h b/drivers/staging/media/atomisp/include/linux/atomisp.h
> index bbbd904b696a..d9a7a599038d 100644
> --- a/drivers/staging/media/atomisp/include/linux/atomisp.h
> +++ b/drivers/staging/media/atomisp/include/linux/atomisp.h
> @@ -914,9 +914,6 @@ enum atomisp_burst_capture_options {
> /* VCM step time */
> #define V4L2_CID_VCM_TIMING (V4L2_CID_CAMERA_LASTP1 + 12)
>
> -/* Query Focus Status */
> -#define V4L2_CID_FOCUS_STATUS (V4L2_CID_CAMERA_LASTP1 + 14)
> -
> /* number of frames to skip at stream start */
> #define V4L2_CID_G_SKIP_FRAMES (V4L2_CID_CAMERA_LASTP1 + 17)
>
> diff --git a/drivers/staging/media/atomisp/include/linux/atomisp_platform.h b/drivers/staging/media/atomisp/include/linux/atomisp_platform.h
> index 487ef5846c24..2535402afd73 100644
> --- a/drivers/staging/media/atomisp/include/linux/atomisp_platform.h
> +++ b/drivers/staging/media/atomisp/include/linux/atomisp_platform.h
> @@ -111,9 +111,8 @@ enum atomisp_input_format {
>
> enum intel_v4l2_subdev_type {
> RAW_CAMERA = 1,
> - CAMERA_MOTOR = 2,
> - LED_FLASH = 3,
> - TEST_PATTERN = 4,
> + LED_FLASH = 2,
> + TEST_PATTERN = 3,
> };
>
> struct intel_v4l2_subdev_id {
> diff --git a/drivers/staging/media/atomisp/pci/atomisp_internal.h b/drivers/staging/media/atomisp/pci/atomisp_internal.h
> index bba9bc64d447..ca8ed3a6b9b8 100644
> --- a/drivers/staging/media/atomisp/pci/atomisp_internal.h
> +++ b/drivers/staging/media/atomisp/pci/atomisp_internal.h
> @@ -134,9 +134,6 @@ struct atomisp_input_subdev {
> struct v4l2_rect active_rect;
> /* Sensor state for which == V4L2_SUBDEV_FORMAT_TRY calls */
> struct v4l2_subdev_state *try_sd_state;
> -
> - struct v4l2_subdev *motor;
> -
> /*
> * To show this resource is used by
> * which stream, in ISP multiple stream mode
> @@ -210,7 +207,6 @@ struct atomisp_device {
> unsigned int input_cnt;
> struct atomisp_input_subdev inputs[ATOM_ISP_MAX_INPUTS];
> struct v4l2_subdev *flash;
> - struct v4l2_subdev *motor;
>
> struct atomisp_regs saved_regs;
> struct atomisp_css_env css_env;
> diff --git a/drivers/staging/media/atomisp/pci/atomisp_ioctl.c b/drivers/staging/media/atomisp/pci/atomisp_ioctl.c
> index bb8e5e883b50..ef555054fdbf 100644
> --- a/drivers/staging/media/atomisp/pci/atomisp_ioctl.c
> +++ b/drivers/staging/media/atomisp/pci/atomisp_ioctl.c
> @@ -557,7 +557,6 @@ static int atomisp_enum_input(struct file *file, void *fh,
> struct video_device *vdev = video_devdata(file);
> struct atomisp_device *isp = video_get_drvdata(vdev);
> int index = input->index;
> - struct v4l2_subdev *motor;
>
> if (index >= isp->input_cnt)
> return -EINVAL;
> @@ -569,28 +568,6 @@ static int atomisp_enum_input(struct file *file, void *fh,
> strscpy(input->name, isp->inputs[index].camera->name,
> sizeof(input->name));
>
> - /*
> - * HACK: append actuator's name to sensor's
> - * As currently userspace can't talk directly to subdev nodes, this
> - * ioctl is the only way to enum inputs + possible external actuators
> - * for 3A tuning purpose.
> - */
> - if (!IS_ISP2401)
> - motor = isp->inputs[index].motor;
> - else
> - motor = isp->motor;
> -
> - if (motor && strlen(motor->name) > 0) {
> - const int cur_len = strlen(input->name);
> - const int max_size = sizeof(input->name) - cur_len - 1;
> -
> - if (max_size > 1) {
> - input->name[cur_len] = '+';
> - strscpy(&input->name[cur_len + 1],
> - motor->name, max_size);
> - }
> - }
> -
> input->type = V4L2_INPUT_TYPE_CAMERA;
> input->index = index;
> input->reserved[0] = isp->inputs[index].type;
> @@ -629,7 +606,6 @@ static int atomisp_s_input(struct file *file, void *fh, unsigned int input)
> struct atomisp_video_pipe *pipe = atomisp_to_video_pipe(vdev);
> struct atomisp_sub_device *asd = pipe->asd;
> struct v4l2_subdev *camera = NULL;
> - struct v4l2_subdev *motor;
> int ret;
>
> ret = atomisp_pipe_check(pipe, true);
> @@ -666,17 +642,6 @@ static int atomisp_s_input(struct file *file, void *fh, unsigned int input)
> return ret;
> }
>
> - if (!IS_ISP2401) {
> - motor = isp->inputs[input].motor;
> - } else {
> - motor = isp->motor;
> - if (motor)
> - ret = v4l2_subdev_call(motor, core, s_power, 1);
> - }
> -
> - if (motor)
> - ret = v4l2_subdev_call(motor, core, init, 1);
> -
> asd->input_curr = input;
> /* mark this camera is used by the current stream */
> isp->inputs[input].asd = asd;
> @@ -1433,26 +1398,8 @@ static int atomisp_s_ctrl(struct file *file, void *fh,
> static int atomisp_queryctl(struct file *file, void *fh,
> struct v4l2_queryctrl *qc)
> {
> - int i, ret = -EINVAL;
> struct video_device *vdev = video_devdata(file);
> - struct atomisp_sub_device *asd = atomisp_to_video_pipe(vdev)->asd;
> - struct atomisp_device *isp = video_get_drvdata(vdev);
> -
> - switch (qc->id) {
> - case V4L2_CID_FOCUS_ABSOLUTE:
> - case V4L2_CID_FOCUS_RELATIVE:
> - case V4L2_CID_FOCUS_STATUS:
> - if (!IS_ISP2401) {
> - return v4l2_queryctrl(isp->inputs[asd->input_curr].camera->
> - ctrl_handler, qc);
> - }
> - /* ISP2401 */
> - if (isp->motor)
> - return v4l2_queryctrl(isp->motor->ctrl_handler, qc);
> - else
> - return v4l2_queryctrl(isp->inputs[asd->input_curr].
> - camera->ctrl_handler, qc);
> - }
> + int i, ret = -EINVAL;
>
> if (qc->id & V4L2_CTRL_FLAG_NEXT_CTRL)
> return ret;
> @@ -1478,16 +1425,10 @@ static int atomisp_camera_g_ext_ctrls(struct file *file, void *fh,
> struct video_device *vdev = video_devdata(file);
> struct atomisp_sub_device *asd = atomisp_to_video_pipe(vdev)->asd;
> struct atomisp_device *isp = video_get_drvdata(vdev);
> - struct v4l2_subdev *motor;
> struct v4l2_control ctrl;
> int i;
> int ret = 0;
>
> - if (!IS_ISP2401)
> - motor = isp->inputs[asd->input_curr].motor;
> - else
> - motor = isp->motor;
> -
> for (i = 0; i < c->count; i++) {
> ctrl.id = c->controls[i].id;
> ctrl.value = c->controls[i].value;
> @@ -1509,13 +1450,6 @@ static int atomisp_camera_g_ext_ctrls(struct file *file, void *fh,
> v4l2_g_ctrl(isp->inputs[asd->input_curr].camera->
> ctrl_handler, &ctrl);
> break;
> - case V4L2_CID_FOCUS_ABSOLUTE:
> - case V4L2_CID_FOCUS_RELATIVE:
> - case V4L2_CID_FOCUS_STATUS:
> - case V4L2_CID_FOCUS_AUTO:
> - if (motor)
> - ret = v4l2_g_ctrl(motor->ctrl_handler, &ctrl);
> - break;
> case V4L2_CID_FLASH_STATUS:
> case V4L2_CID_FLASH_INTENSITY:
> case V4L2_CID_FLASH_TORCH_INTENSITY:
> @@ -1584,16 +1518,10 @@ static int atomisp_camera_s_ext_ctrls(struct file *file, void *fh,
> struct video_device *vdev = video_devdata(file);
> struct atomisp_sub_device *asd = atomisp_to_video_pipe(vdev)->asd;
> struct atomisp_device *isp = video_get_drvdata(vdev);
> - struct v4l2_subdev *motor;
> struct v4l2_control ctrl;
> int i;
> int ret = 0;
>
> - if (!IS_ISP2401)
> - motor = isp->inputs[asd->input_curr].motor;
> - else
> - motor = isp->motor;
> -
> for (i = 0; i < c->count; i++) {
> struct v4l2_ctrl *ctr;
>
> @@ -1616,18 +1544,6 @@ static int atomisp_camera_s_ext_ctrls(struct file *file, void *fh,
> isp->inputs[asd->input_curr].camera->
> ctrl_handler, &ctrl);
> break;
> - case V4L2_CID_FOCUS_ABSOLUTE:
> - case V4L2_CID_FOCUS_RELATIVE:
> - case V4L2_CID_FOCUS_STATUS:
> - case V4L2_CID_FOCUS_AUTO:
> - if (motor)
> - ret = v4l2_s_ctrl(NULL, motor->ctrl_handler,
> - &ctrl);
> - else
> - ret = v4l2_s_ctrl(NULL,
> - isp->inputs[asd->input_curr].
> - camera->ctrl_handler, &ctrl);
> - break;
> case V4L2_CID_FLASH_STATUS:
> case V4L2_CID_FLASH_INTENSITY:
> case V4L2_CID_FLASH_TORCH_INTENSITY:
> diff --git a/drivers/staging/media/atomisp/pci/atomisp_v4l2.c b/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
> index f736e54c7df3..1a936dbe8eb4 100644
> --- a/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
> +++ b/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
> @@ -823,13 +823,6 @@ static int atomisp_subdev_probe(struct atomisp_device *isp)
> isp->sensor_lanes[mipi_port] = subdevs->lanes;
> isp->sensor_subdevs[subdevs->port] = subdevs->subdev;
> break;
> - case CAMERA_MOTOR:
> - if (isp->motor) {
> - dev_warn(isp->dev, "too many atomisp motors\n");
> - continue;
> - }
> - isp->motor = subdevs->subdev;
> - break;
> case LED_FLASH:
> if (isp->flash) {
> dev_warn(isp->dev, "too many atomisp flash devices\n");
> @@ -1066,14 +1059,6 @@ int atomisp_register_device_nodes(struct atomisp_device *isp)
>
> atomisp_init_sensor(input);
>
> - /*
> - * HACK: Currently VCM belongs to primary sensor only, but correct
> - * approach must be to acquire from platform code which sensor
> - * owns it.
> - */
> - if (i == ATOMISP_CAMERA_PORT_PRIMARY)
> - input->motor = isp->motor;
> -
> err = media_create_pad_link(&input->camera->entity, 0,
> &isp->csi2_port[i].subdev.entity,
> CSI2_PAD_SINK,
> --
> 2.43.0
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 4/9] media: atomisp: Remove ISP controls which get passed through to the camera
2024-02-17 11:24 [PATCH 0/9] media: atomisp: Changes for libcamera support Hans de Goede
` (2 preceding siblings ...)
2024-02-17 11:24 ` [PATCH 3/9] media: atomisp: Remove custom VCM handling Hans de Goede
@ 2024-02-17 11:24 ` Hans de Goede
2024-02-17 15:13 ` Kieran Bingham
2024-02-17 11:24 ` [PATCH 5/9] media: atomisp: Stop setting sd->devnode for the ATOMISP_SUBDEV v4l2-subdev Hans de Goede
` (5 subsequent siblings)
9 siblings, 1 reply; 23+ messages in thread
From: Hans de Goede @ 2024-02-17 11:24 UTC (permalink / raw)
To: Mauro Carvalho Chehab, Sakari Ailus, Andy Shevchenko
Cc: Hans de Goede, Kate Hsuan, Tsuchiya Yuto, Yury Luneff, Nable,
andrey.i.trufanov, Fabio Aiuto, linux-media, linux-staging
Drop all ISP controls and ioctls which just get passed through
to the camera subdev. Instead these calls should be done directly
at the sensor subdev.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
.../media/atomisp/include/linux/atomisp.h | 21 --
.../staging/media/atomisp/pci/atomisp_ioctl.c | 248 ------------------
2 files changed, 269 deletions(-)
diff --git a/drivers/staging/media/atomisp/include/linux/atomisp.h b/drivers/staging/media/atomisp/include/linux/atomisp.h
index d9a7a599038d..b2735a008052 100644
--- a/drivers/staging/media/atomisp/include/linux/atomisp.h
+++ b/drivers/staging/media/atomisp/include/linux/atomisp.h
@@ -837,9 +837,6 @@ enum atomisp_burst_capture_options {
#define ATOMISP_IOC_S_PARAMETERS \
_IOW('v', BASE_VIDIOC_PRIVATE + 32, struct atomisp_parameters)
-#define ATOMISP_IOC_EXT_ISP_CTRL \
- _IOWR('v', BASE_VIDIOC_PRIVATE + 35, struct atomisp_ext_isp_ctrl)
-
#define ATOMISP_IOC_EXP_ID_UNLOCK \
_IOW('v', BASE_VIDIOC_PRIVATE + 36, int)
@@ -909,19 +906,6 @@ enum atomisp_burst_capture_options {
/* Set the flash mode (see enum atomisp_flash_mode) */
#define V4L2_CID_FLASH_MODE (V4L2_CID_CAMERA_LASTP1 + 10)
-/* VCM slew control */
-#define V4L2_CID_VCM_SLEW (V4L2_CID_CAMERA_LASTP1 + 11)
-/* VCM step time */
-#define V4L2_CID_VCM_TIMING (V4L2_CID_CAMERA_LASTP1 + 12)
-
-/* number of frames to skip at stream start */
-#define V4L2_CID_G_SKIP_FRAMES (V4L2_CID_CAMERA_LASTP1 + 17)
-
-/* Query sensor's 2A status */
-#define V4L2_CID_2A_STATUS (V4L2_CID_CAMERA_LASTP1 + 18)
-#define V4L2_2A_STATUS_AE_READY BIT(0)
-#define V4L2_2A_STATUS_AWB_READY BIT(1)
-
#define V4L2_CID_RUN_MODE (V4L2_CID_CAMERA_LASTP1 + 20)
#define ATOMISP_RUN_MODE_VIDEO 1
#define ATOMISP_RUN_MODE_STILL_CAPTURE 2
@@ -952,11 +936,6 @@ enum atomisp_burst_capture_options {
/* Disable digital zoom */
#define V4L2_CID_DISABLE_DZ (V4L2_CID_CAMERA_LASTP1 + 32)
-#define V4L2_CID_TEST_PATTERN_COLOR_R (V4L2_CID_CAMERA_LASTP1 + 33)
-#define V4L2_CID_TEST_PATTERN_COLOR_GR (V4L2_CID_CAMERA_LASTP1 + 34)
-#define V4L2_CID_TEST_PATTERN_COLOR_GB (V4L2_CID_CAMERA_LASTP1 + 35)
-#define V4L2_CID_TEST_PATTERN_COLOR_B (V4L2_CID_CAMERA_LASTP1 + 36)
-
#define V4L2_CID_ATOMISP_SELECT_ISP_VERSION (V4L2_CID_CAMERA_LASTP1 + 38)
#define V4L2_BUF_FLAG_BUFFER_INVALID 0x0400
diff --git a/drivers/staging/media/atomisp/pci/atomisp_ioctl.c b/drivers/staging/media/atomisp/pci/atomisp_ioctl.c
index ef555054fdbf..74cf055cb09b 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_ioctl.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_ioctl.c
@@ -82,15 +82,6 @@ static struct v4l2_queryctrl ci_v4l2_controls[] = {
.step = 1,
.default_value = 0x00,
},
- {
- .id = V4L2_CID_POWER_LINE_FREQUENCY,
- .type = V4L2_CTRL_TYPE_MENU,
- .name = "Light frequency filter",
- .minimum = 1,
- .maximum = 2,
- .step = 1,
- .default_value = 1,
- },
{
.id = V4L2_CID_COLORFX,
.type = V4L2_CTRL_TYPE_INTEGER,
@@ -100,15 +91,6 @@ static struct v4l2_queryctrl ci_v4l2_controls[] = {
.step = 1,
.default_value = 0,
},
- {
- .id = V4L2_CID_COLORFX_CBCR,
- .type = V4L2_CTRL_TYPE_INTEGER,
- .name = "Image Color Effect CbCr",
- .minimum = 0,
- .maximum = 0xffff,
- .step = 1,
- .default_value = 0,
- },
{
.id = V4L2_CID_ATOMISP_BAD_PIXEL_DETECTION,
.type = V4L2_CTRL_TYPE_INTEGER,
@@ -172,142 +154,6 @@ static struct v4l2_queryctrl ci_v4l2_controls[] = {
.step = 1,
.default_value = 1,
},
- {
- .id = V4L2_CID_2A_STATUS,
- .type = V4L2_CTRL_TYPE_BITMASK,
- .name = "AE and AWB status",
- .minimum = 0,
- .maximum = V4L2_2A_STATUS_AE_READY | V4L2_2A_STATUS_AWB_READY,
- .step = 1,
- .default_value = 0,
- },
- {
- .id = V4L2_CID_EXPOSURE,
- .type = V4L2_CTRL_TYPE_INTEGER,
- .name = "exposure",
- .minimum = -4,
- .maximum = 4,
- .step = 1,
- .default_value = 0,
- },
- {
- .id = V4L2_CID_EXPOSURE_ZONE_NUM,
- .type = V4L2_CTRL_TYPE_INTEGER,
- .name = "one-time exposure zone number",
- .minimum = 0x0,
- .maximum = 0xffff,
- .step = 1,
- .default_value = 0,
- },
- {
- .id = V4L2_CID_EXPOSURE_AUTO_PRIORITY,
- .type = V4L2_CTRL_TYPE_INTEGER,
- .name = "Exposure auto priority",
- .minimum = V4L2_EXPOSURE_AUTO,
- .maximum = V4L2_EXPOSURE_APERTURE_PRIORITY,
- .step = 1,
- .default_value = V4L2_EXPOSURE_AUTO,
- },
- {
- .id = V4L2_CID_SCENE_MODE,
- .type = V4L2_CTRL_TYPE_INTEGER,
- .name = "scene mode",
- .minimum = 0,
- .maximum = 13,
- .step = 1,
- .default_value = 0,
- },
- {
- .id = V4L2_CID_ISO_SENSITIVITY,
- .type = V4L2_CTRL_TYPE_INTEGER,
- .name = "iso",
- .minimum = -4,
- .maximum = 4,
- .step = 1,
- .default_value = 0,
- },
- {
- .id = V4L2_CID_ISO_SENSITIVITY_AUTO,
- .type = V4L2_CTRL_TYPE_INTEGER,
- .name = "iso mode",
- .minimum = V4L2_ISO_SENSITIVITY_MANUAL,
- .maximum = V4L2_ISO_SENSITIVITY_AUTO,
- .step = 1,
- .default_value = V4L2_ISO_SENSITIVITY_AUTO,
- },
- {
- .id = V4L2_CID_AUTO_N_PRESET_WHITE_BALANCE,
- .type = V4L2_CTRL_TYPE_INTEGER,
- .name = "white balance",
- .minimum = 0,
- .maximum = 9,
- .step = 1,
- .default_value = 0,
- },
- {
- .id = V4L2_CID_EXPOSURE_METERING,
- .type = V4L2_CTRL_TYPE_MENU,
- .name = "metering",
- .minimum = 0,
- .maximum = 3,
- .step = 1,
- .default_value = 1,
- },
- {
- .id = V4L2_CID_3A_LOCK,
- .type = V4L2_CTRL_TYPE_BITMASK,
- .name = "3a lock",
- .minimum = 0,
- .maximum = V4L2_LOCK_EXPOSURE | V4L2_LOCK_WHITE_BALANCE
- | V4L2_LOCK_FOCUS,
- .step = 1,
- .default_value = 0,
- },
- {
- .id = V4L2_CID_TEST_PATTERN,
- .type = V4L2_CTRL_TYPE_INTEGER,
- .name = "Test Pattern",
- .minimum = 0,
- .maximum = 0xffff,
- .step = 1,
- .default_value = 0,
- },
- {
- .id = V4L2_CID_TEST_PATTERN_COLOR_R,
- .type = V4L2_CTRL_TYPE_INTEGER,
- .name = "Test Pattern Solid Color R",
- .minimum = INT_MIN,
- .maximum = INT_MAX,
- .step = 1,
- .default_value = 0,
- },
- {
- .id = V4L2_CID_TEST_PATTERN_COLOR_GR,
- .type = V4L2_CTRL_TYPE_INTEGER,
- .name = "Test Pattern Solid Color GR",
- .minimum = INT_MIN,
- .maximum = INT_MAX,
- .step = 1,
- .default_value = 0,
- },
- {
- .id = V4L2_CID_TEST_PATTERN_COLOR_GB,
- .type = V4L2_CTRL_TYPE_INTEGER,
- .name = "Test Pattern Solid Color GB",
- .minimum = INT_MIN,
- .maximum = INT_MAX,
- .step = 1,
- .default_value = 0,
- },
- {
- .id = V4L2_CID_TEST_PATTERN_COLOR_B,
- .type = V4L2_CTRL_TYPE_INTEGER,
- .name = "Test Pattern Solid Color B",
- .minimum = INT_MIN,
- .maximum = INT_MAX,
- .step = 1,
- .default_value = 0,
- },
};
static const u32 ctrls_num = ARRAY_SIZE(ci_v4l2_controls);
@@ -1248,7 +1094,6 @@ static int atomisp_g_ctrl(struct file *file, void *fh,
{
struct video_device *vdev = video_devdata(file);
struct atomisp_sub_device *asd = atomisp_to_video_pipe(vdev)->asd;
- struct atomisp_device *isp = video_get_drvdata(vdev);
int i, ret = -EINVAL;
for (i = 0; i < ctrls_num; i++) {
@@ -1262,27 +1107,6 @@ static int atomisp_g_ctrl(struct file *file, void *fh,
return ret;
switch (control->id) {
- case V4L2_CID_IRIS_ABSOLUTE:
- case V4L2_CID_EXPOSURE_ABSOLUTE:
- case V4L2_CID_2A_STATUS:
- case V4L2_CID_AUTO_N_PRESET_WHITE_BALANCE:
- case V4L2_CID_EXPOSURE:
- case V4L2_CID_EXPOSURE_AUTO:
- case V4L2_CID_SCENE_MODE:
- case V4L2_CID_ISO_SENSITIVITY:
- case V4L2_CID_ISO_SENSITIVITY_AUTO:
- case V4L2_CID_CONTRAST:
- case V4L2_CID_SATURATION:
- case V4L2_CID_SHARPNESS:
- case V4L2_CID_3A_LOCK:
- case V4L2_CID_EXPOSURE_ZONE_NUM:
- case V4L2_CID_TEST_PATTERN:
- case V4L2_CID_TEST_PATTERN_COLOR_R:
- case V4L2_CID_TEST_PATTERN_COLOR_GR:
- case V4L2_CID_TEST_PATTERN_COLOR_GB:
- case V4L2_CID_TEST_PATTERN_COLOR_B:
- return v4l2_g_ctrl(isp->inputs[asd->input_curr].camera->
- ctrl_handler, control);
case V4L2_CID_COLORFX:
ret = atomisp_color_effect(asd, 0, &control->value);
break;
@@ -1322,7 +1146,6 @@ static int atomisp_s_ctrl(struct file *file, void *fh,
{
struct video_device *vdev = video_devdata(file);
struct atomisp_sub_device *asd = atomisp_to_video_pipe(vdev)->asd;
- struct atomisp_device *isp = video_get_drvdata(vdev);
int i, ret = -EINVAL;
for (i = 0; i < ctrls_num; i++) {
@@ -1336,28 +1159,6 @@ static int atomisp_s_ctrl(struct file *file, void *fh,
return ret;
switch (control->id) {
- case V4L2_CID_AUTO_N_PRESET_WHITE_BALANCE:
- case V4L2_CID_EXPOSURE:
- case V4L2_CID_EXPOSURE_AUTO:
- case V4L2_CID_EXPOSURE_AUTO_PRIORITY:
- case V4L2_CID_SCENE_MODE:
- case V4L2_CID_ISO_SENSITIVITY:
- case V4L2_CID_ISO_SENSITIVITY_AUTO:
- case V4L2_CID_POWER_LINE_FREQUENCY:
- case V4L2_CID_EXPOSURE_METERING:
- case V4L2_CID_CONTRAST:
- case V4L2_CID_SATURATION:
- case V4L2_CID_SHARPNESS:
- case V4L2_CID_3A_LOCK:
- case V4L2_CID_COLORFX_CBCR:
- case V4L2_CID_TEST_PATTERN:
- case V4L2_CID_TEST_PATTERN_COLOR_R:
- case V4L2_CID_TEST_PATTERN_COLOR_GR:
- case V4L2_CID_TEST_PATTERN_COLOR_GB:
- case V4L2_CID_TEST_PATTERN_COLOR_B:
- return v4l2_s_ctrl(NULL,
- isp->inputs[asd->input_curr].camera->
- ctrl_handler, control);
case V4L2_CID_COLORFX:
ret = atomisp_color_effect(asd, 1, &control->value);
break;
@@ -1398,7 +1199,6 @@ static int atomisp_s_ctrl(struct file *file, void *fh,
static int atomisp_queryctl(struct file *file, void *fh,
struct v4l2_queryctrl *qc)
{
- struct video_device *vdev = video_devdata(file);
int i, ret = -EINVAL;
if (qc->id & V4L2_CTRL_FLAG_NEXT_CTRL)
@@ -1433,23 +1233,6 @@ static int atomisp_camera_g_ext_ctrls(struct file *file, void *fh,
ctrl.id = c->controls[i].id;
ctrl.value = c->controls[i].value;
switch (ctrl.id) {
- case V4L2_CID_EXPOSURE_ABSOLUTE:
- case V4L2_CID_EXPOSURE_AUTO:
- case V4L2_CID_IRIS_ABSOLUTE:
- case V4L2_CID_3A_LOCK:
- case V4L2_CID_TEST_PATTERN:
- case V4L2_CID_TEST_PATTERN_COLOR_R:
- case V4L2_CID_TEST_PATTERN_COLOR_GR:
- case V4L2_CID_TEST_PATTERN_COLOR_GB:
- case V4L2_CID_TEST_PATTERN_COLOR_B:
- /*
- * Exposure related control will be handled by sensor
- * driver
- */
- ret =
- v4l2_g_ctrl(isp->inputs[asd->input_curr].camera->
- ctrl_handler, &ctrl);
- break;
case V4L2_CID_FLASH_STATUS:
case V4L2_CID_FLASH_INTENSITY:
case V4L2_CID_FLASH_TORCH_INTENSITY:
@@ -1466,11 +1249,6 @@ static int atomisp_camera_g_ext_ctrls(struct file *file, void *fh,
case V4L2_CID_ZOOM_ABSOLUTE:
ret = atomisp_digital_zoom(asd, 0, &ctrl.value);
break;
- case V4L2_CID_G_SKIP_FRAMES:
- ret = v4l2_subdev_call(
- isp->inputs[asd->input_curr].camera,
- sensor, g_skip_frames, (u32 *)&ctrl.value);
- break;
default:
ret = -EINVAL;
}
@@ -1528,22 +1306,6 @@ static int atomisp_camera_s_ext_ctrls(struct file *file, void *fh,
ctrl.id = c->controls[i].id;
ctrl.value = c->controls[i].value;
switch (ctrl.id) {
- case V4L2_CID_EXPOSURE_ABSOLUTE:
- case V4L2_CID_EXPOSURE_AUTO:
- case V4L2_CID_EXPOSURE_METERING:
- case V4L2_CID_IRIS_ABSOLUTE:
- case V4L2_CID_VCM_TIMING:
- case V4L2_CID_VCM_SLEW:
- case V4L2_CID_3A_LOCK:
- case V4L2_CID_TEST_PATTERN:
- case V4L2_CID_TEST_PATTERN_COLOR_R:
- case V4L2_CID_TEST_PATTERN_COLOR_GR:
- case V4L2_CID_TEST_PATTERN_COLOR_GB:
- case V4L2_CID_TEST_PATTERN_COLOR_B:
- ret = v4l2_s_ctrl(NULL,
- isp->inputs[asd->input_curr].camera->
- ctrl_handler, &ctrl);
- break;
case V4L2_CID_FLASH_STATUS:
case V4L2_CID_FLASH_INTENSITY:
case V4L2_CID_FLASH_TORCH_INTENSITY:
@@ -1692,7 +1454,6 @@ static long atomisp_vidioc_default(struct file *file, void *fh,
bool valid_prio, unsigned int cmd, void *arg)
{
struct video_device *vdev = video_devdata(file);
- struct atomisp_device *isp = video_get_drvdata(vdev);
struct atomisp_sub_device *asd = atomisp_to_video_pipe(vdev)->asd;
int err;
@@ -1839,11 +1600,6 @@ static long atomisp_vidioc_default(struct file *file, void *fh,
err = atomisp_fixed_pattern_table(asd, arg);
break;
- case ATOMISP_IOC_S_EXPOSURE:
- err = v4l2_subdev_call(isp->inputs[asd->input_curr].camera,
- core, ioctl, cmd, arg);
- break;
-
case ATOMISP_IOC_S_ISP_SHD_TAB:
err = atomisp_set_shading_table(asd, arg);
break;
@@ -1860,10 +1616,6 @@ static long atomisp_vidioc_default(struct file *file, void *fh,
err = atomisp_set_parameters(vdev, arg);
break;
- case ATOMISP_IOC_EXT_ISP_CTRL:
- err = v4l2_subdev_call(isp->inputs[asd->input_curr].camera,
- core, ioctl, cmd, arg);
- break;
case ATOMISP_IOC_EXP_ID_UNLOCK:
err = atomisp_exp_id_unlock(asd, arg);
break;
--
2.43.0
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH 4/9] media: atomisp: Remove ISP controls which get passed through to the camera
2024-02-17 11:24 ` [PATCH 4/9] media: atomisp: Remove ISP controls which get passed through to the camera Hans de Goede
@ 2024-02-17 15:13 ` Kieran Bingham
0 siblings, 0 replies; 23+ messages in thread
From: Kieran Bingham @ 2024-02-17 15:13 UTC (permalink / raw)
To: Andy Shevchenko, Hans de Goede, Mauro Carvalho Chehab,
Sakari Ailus
Cc: Hans de Goede, Kate Hsuan, Tsuchiya Yuto, Yury Luneff, Nable,
andrey.i.trufanov, Fabio Aiuto, linux-media, linux-staging
Quoting Hans de Goede (2024-02-17 11:24:33)
> Drop all ISP controls and ioctls which just get passed through
> to the camera subdev. Instead these calls should be done directly
> at the sensor subdev.
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
> .../media/atomisp/include/linux/atomisp.h | 21 --
> .../staging/media/atomisp/pci/atomisp_ioctl.c | 248 ------------------
> 2 files changed, 269 deletions(-)
>
> diff --git a/drivers/staging/media/atomisp/include/linux/atomisp.h b/drivers/staging/media/atomisp/include/linux/atomisp.h
> index d9a7a599038d..b2735a008052 100644
> --- a/drivers/staging/media/atomisp/include/linux/atomisp.h
> +++ b/drivers/staging/media/atomisp/include/linux/atomisp.h
> @@ -837,9 +837,6 @@ enum atomisp_burst_capture_options {
> #define ATOMISP_IOC_S_PARAMETERS \
> _IOW('v', BASE_VIDIOC_PRIVATE + 32, struct atomisp_parameters)
>
> -#define ATOMISP_IOC_EXT_ISP_CTRL \
> - _IOWR('v', BASE_VIDIOC_PRIVATE + 35, struct atomisp_ext_isp_ctrl)
> -
> #define ATOMISP_IOC_EXP_ID_UNLOCK \
> _IOW('v', BASE_VIDIOC_PRIVATE + 36, int)
>
> @@ -909,19 +906,6 @@ enum atomisp_burst_capture_options {
> /* Set the flash mode (see enum atomisp_flash_mode) */
> #define V4L2_CID_FLASH_MODE (V4L2_CID_CAMERA_LASTP1 + 10)
>
> -/* VCM slew control */
> -#define V4L2_CID_VCM_SLEW (V4L2_CID_CAMERA_LASTP1 + 11)
> -/* VCM step time */
> -#define V4L2_CID_VCM_TIMING (V4L2_CID_CAMERA_LASTP1 + 12)
> -
> -/* number of frames to skip at stream start */
> -#define V4L2_CID_G_SKIP_FRAMES (V4L2_CID_CAMERA_LASTP1 + 17)
> -
> -/* Query sensor's 2A status */
> -#define V4L2_CID_2A_STATUS (V4L2_CID_CAMERA_LASTP1 + 18)
> -#define V4L2_2A_STATUS_AE_READY BIT(0)
> -#define V4L2_2A_STATUS_AWB_READY BIT(1)
> -
> #define V4L2_CID_RUN_MODE (V4L2_CID_CAMERA_LASTP1 + 20)
> #define ATOMISP_RUN_MODE_VIDEO 1
> #define ATOMISP_RUN_MODE_STILL_CAPTURE 2
> @@ -952,11 +936,6 @@ enum atomisp_burst_capture_options {
> /* Disable digital zoom */
> #define V4L2_CID_DISABLE_DZ (V4L2_CID_CAMERA_LASTP1 + 32)
>
> -#define V4L2_CID_TEST_PATTERN_COLOR_R (V4L2_CID_CAMERA_LASTP1 + 33)
> -#define V4L2_CID_TEST_PATTERN_COLOR_GR (V4L2_CID_CAMERA_LASTP1 + 34)
> -#define V4L2_CID_TEST_PATTERN_COLOR_GB (V4L2_CID_CAMERA_LASTP1 + 35)
> -#define V4L2_CID_TEST_PATTERN_COLOR_B (V4L2_CID_CAMERA_LASTP1 + 36)
> -
> #define V4L2_CID_ATOMISP_SELECT_ISP_VERSION (V4L2_CID_CAMERA_LASTP1 + 38)
>
> #define V4L2_BUF_FLAG_BUFFER_INVALID 0x0400
> diff --git a/drivers/staging/media/atomisp/pci/atomisp_ioctl.c b/drivers/staging/media/atomisp/pci/atomisp_ioctl.c
> index ef555054fdbf..74cf055cb09b 100644
> --- a/drivers/staging/media/atomisp/pci/atomisp_ioctl.c
> +++ b/drivers/staging/media/atomisp/pci/atomisp_ioctl.c
> @@ -82,15 +82,6 @@ static struct v4l2_queryctrl ci_v4l2_controls[] = {
> .step = 1,
> .default_value = 0x00,
> },
> - {
> - .id = V4L2_CID_POWER_LINE_FREQUENCY,
> - .type = V4L2_CTRL_TYPE_MENU,
> - .name = "Light frequency filter",
> - .minimum = 1,
> - .maximum = 2,
> - .step = 1,
> - .default_value = 1,
> - },
> {
> .id = V4L2_CID_COLORFX,
> .type = V4L2_CTRL_TYPE_INTEGER,
> @@ -100,15 +91,6 @@ static struct v4l2_queryctrl ci_v4l2_controls[] = {
> .step = 1,
> .default_value = 0,
> },
> - {
> - .id = V4L2_CID_COLORFX_CBCR,
> - .type = V4L2_CTRL_TYPE_INTEGER,
> - .name = "Image Color Effect CbCr",
> - .minimum = 0,
> - .maximum = 0xffff,
> - .step = 1,
> - .default_value = 0,
> - },
> {
> .id = V4L2_CID_ATOMISP_BAD_PIXEL_DETECTION,
> .type = V4L2_CTRL_TYPE_INTEGER,
> @@ -172,142 +154,6 @@ static struct v4l2_queryctrl ci_v4l2_controls[] = {
> .step = 1,
> .default_value = 1,
> },
> - {
> - .id = V4L2_CID_2A_STATUS,
> - .type = V4L2_CTRL_TYPE_BITMASK,
> - .name = "AE and AWB status",
> - .minimum = 0,
> - .maximum = V4L2_2A_STATUS_AE_READY | V4L2_2A_STATUS_AWB_READY,
> - .step = 1,
> - .default_value = 0,
> - },
> - {
> - .id = V4L2_CID_EXPOSURE,
> - .type = V4L2_CTRL_TYPE_INTEGER,
> - .name = "exposure",
> - .minimum = -4,
> - .maximum = 4,
> - .step = 1,
> - .default_value = 0,
> - },
> - {
> - .id = V4L2_CID_EXPOSURE_ZONE_NUM,
> - .type = V4L2_CTRL_TYPE_INTEGER,
> - .name = "one-time exposure zone number",
> - .minimum = 0x0,
> - .maximum = 0xffff,
> - .step = 1,
> - .default_value = 0,
> - },
> - {
> - .id = V4L2_CID_EXPOSURE_AUTO_PRIORITY,
> - .type = V4L2_CTRL_TYPE_INTEGER,
> - .name = "Exposure auto priority",
> - .minimum = V4L2_EXPOSURE_AUTO,
> - .maximum = V4L2_EXPOSURE_APERTURE_PRIORITY,
> - .step = 1,
> - .default_value = V4L2_EXPOSURE_AUTO,
> - },
> - {
> - .id = V4L2_CID_SCENE_MODE,
> - .type = V4L2_CTRL_TYPE_INTEGER,
> - .name = "scene mode",
> - .minimum = 0,
> - .maximum = 13,
> - .step = 1,
> - .default_value = 0,
> - },
> - {
> - .id = V4L2_CID_ISO_SENSITIVITY,
> - .type = V4L2_CTRL_TYPE_INTEGER,
> - .name = "iso",
> - .minimum = -4,
> - .maximum = 4,
> - .step = 1,
> - .default_value = 0,
> - },
> - {
> - .id = V4L2_CID_ISO_SENSITIVITY_AUTO,
> - .type = V4L2_CTRL_TYPE_INTEGER,
> - .name = "iso mode",
> - .minimum = V4L2_ISO_SENSITIVITY_MANUAL,
> - .maximum = V4L2_ISO_SENSITIVITY_AUTO,
> - .step = 1,
> - .default_value = V4L2_ISO_SENSITIVITY_AUTO,
> - },
> - {
> - .id = V4L2_CID_AUTO_N_PRESET_WHITE_BALANCE,
> - .type = V4L2_CTRL_TYPE_INTEGER,
> - .name = "white balance",
> - .minimum = 0,
> - .maximum = 9,
> - .step = 1,
> - .default_value = 0,
> - },
> - {
> - .id = V4L2_CID_EXPOSURE_METERING,
> - .type = V4L2_CTRL_TYPE_MENU,
> - .name = "metering",
> - .minimum = 0,
> - .maximum = 3,
> - .step = 1,
> - .default_value = 1,
> - },
> - {
> - .id = V4L2_CID_3A_LOCK,
> - .type = V4L2_CTRL_TYPE_BITMASK,
> - .name = "3a lock",
> - .minimum = 0,
> - .maximum = V4L2_LOCK_EXPOSURE | V4L2_LOCK_WHITE_BALANCE
> - | V4L2_LOCK_FOCUS,
> - .step = 1,
> - .default_value = 0,
> - },
> - {
> - .id = V4L2_CID_TEST_PATTERN,
> - .type = V4L2_CTRL_TYPE_INTEGER,
> - .name = "Test Pattern",
> - .minimum = 0,
> - .maximum = 0xffff,
> - .step = 1,
> - .default_value = 0,
> - },
> - {
> - .id = V4L2_CID_TEST_PATTERN_COLOR_R,
> - .type = V4L2_CTRL_TYPE_INTEGER,
> - .name = "Test Pattern Solid Color R",
> - .minimum = INT_MIN,
> - .maximum = INT_MAX,
> - .step = 1,
> - .default_value = 0,
> - },
> - {
> - .id = V4L2_CID_TEST_PATTERN_COLOR_GR,
> - .type = V4L2_CTRL_TYPE_INTEGER,
> - .name = "Test Pattern Solid Color GR",
> - .minimum = INT_MIN,
> - .maximum = INT_MAX,
> - .step = 1,
> - .default_value = 0,
> - },
> - {
> - .id = V4L2_CID_TEST_PATTERN_COLOR_GB,
> - .type = V4L2_CTRL_TYPE_INTEGER,
> - .name = "Test Pattern Solid Color GB",
> - .minimum = INT_MIN,
> - .maximum = INT_MAX,
> - .step = 1,
> - .default_value = 0,
> - },
> - {
> - .id = V4L2_CID_TEST_PATTERN_COLOR_B,
> - .type = V4L2_CTRL_TYPE_INTEGER,
> - .name = "Test Pattern Solid Color B",
> - .minimum = INT_MIN,
> - .maximum = INT_MAX,
> - .step = 1,
> - .default_value = 0,
> - },
> };
>
> static const u32 ctrls_num = ARRAY_SIZE(ci_v4l2_controls);
> @@ -1248,7 +1094,6 @@ static int atomisp_g_ctrl(struct file *file, void *fh,
> {
> struct video_device *vdev = video_devdata(file);
> struct atomisp_sub_device *asd = atomisp_to_video_pipe(vdev)->asd;
> - struct atomisp_device *isp = video_get_drvdata(vdev);
> int i, ret = -EINVAL;
>
> for (i = 0; i < ctrls_num; i++) {
> @@ -1262,27 +1107,6 @@ static int atomisp_g_ctrl(struct file *file, void *fh,
> return ret;
>
> switch (control->id) {
> - case V4L2_CID_IRIS_ABSOLUTE:
> - case V4L2_CID_EXPOSURE_ABSOLUTE:
> - case V4L2_CID_2A_STATUS:
> - case V4L2_CID_AUTO_N_PRESET_WHITE_BALANCE:
> - case V4L2_CID_EXPOSURE:
> - case V4L2_CID_EXPOSURE_AUTO:
> - case V4L2_CID_SCENE_MODE:
> - case V4L2_CID_ISO_SENSITIVITY:
> - case V4L2_CID_ISO_SENSITIVITY_AUTO:
> - case V4L2_CID_CONTRAST:
> - case V4L2_CID_SATURATION:
> - case V4L2_CID_SHARPNESS:
> - case V4L2_CID_3A_LOCK:
> - case V4L2_CID_EXPOSURE_ZONE_NUM:
> - case V4L2_CID_TEST_PATTERN:
> - case V4L2_CID_TEST_PATTERN_COLOR_R:
> - case V4L2_CID_TEST_PATTERN_COLOR_GR:
> - case V4L2_CID_TEST_PATTERN_COLOR_GB:
> - case V4L2_CID_TEST_PATTERN_COLOR_B:
> - return v4l2_g_ctrl(isp->inputs[asd->input_curr].camera->
> - ctrl_handler, control);
> case V4L2_CID_COLORFX:
> ret = atomisp_color_effect(asd, 0, &control->value);
> break;
> @@ -1322,7 +1146,6 @@ static int atomisp_s_ctrl(struct file *file, void *fh,
> {
> struct video_device *vdev = video_devdata(file);
> struct atomisp_sub_device *asd = atomisp_to_video_pipe(vdev)->asd;
> - struct atomisp_device *isp = video_get_drvdata(vdev);
> int i, ret = -EINVAL;
>
> for (i = 0; i < ctrls_num; i++) {
> @@ -1336,28 +1159,6 @@ static int atomisp_s_ctrl(struct file *file, void *fh,
> return ret;
>
> switch (control->id) {
> - case V4L2_CID_AUTO_N_PRESET_WHITE_BALANCE:
> - case V4L2_CID_EXPOSURE:
> - case V4L2_CID_EXPOSURE_AUTO:
> - case V4L2_CID_EXPOSURE_AUTO_PRIORITY:
> - case V4L2_CID_SCENE_MODE:
> - case V4L2_CID_ISO_SENSITIVITY:
> - case V4L2_CID_ISO_SENSITIVITY_AUTO:
> - case V4L2_CID_POWER_LINE_FREQUENCY:
> - case V4L2_CID_EXPOSURE_METERING:
> - case V4L2_CID_CONTRAST:
> - case V4L2_CID_SATURATION:
> - case V4L2_CID_SHARPNESS:
> - case V4L2_CID_3A_LOCK:
> - case V4L2_CID_COLORFX_CBCR:
> - case V4L2_CID_TEST_PATTERN:
> - case V4L2_CID_TEST_PATTERN_COLOR_R:
> - case V4L2_CID_TEST_PATTERN_COLOR_GR:
> - case V4L2_CID_TEST_PATTERN_COLOR_GB:
> - case V4L2_CID_TEST_PATTERN_COLOR_B:
> - return v4l2_s_ctrl(NULL,
> - isp->inputs[asd->input_curr].camera->
> - ctrl_handler, control);
Looks like a reasonable clean up from that.
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> case V4L2_CID_COLORFX:
> ret = atomisp_color_effect(asd, 1, &control->value);
> break;
> @@ -1398,7 +1199,6 @@ static int atomisp_s_ctrl(struct file *file, void *fh,
> static int atomisp_queryctl(struct file *file, void *fh,
> struct v4l2_queryctrl *qc)
> {
> - struct video_device *vdev = video_devdata(file);
> int i, ret = -EINVAL;
>
> if (qc->id & V4L2_CTRL_FLAG_NEXT_CTRL)
> @@ -1433,23 +1233,6 @@ static int atomisp_camera_g_ext_ctrls(struct file *file, void *fh,
> ctrl.id = c->controls[i].id;
> ctrl.value = c->controls[i].value;
> switch (ctrl.id) {
> - case V4L2_CID_EXPOSURE_ABSOLUTE:
> - case V4L2_CID_EXPOSURE_AUTO:
> - case V4L2_CID_IRIS_ABSOLUTE:
> - case V4L2_CID_3A_LOCK:
> - case V4L2_CID_TEST_PATTERN:
> - case V4L2_CID_TEST_PATTERN_COLOR_R:
> - case V4L2_CID_TEST_PATTERN_COLOR_GR:
> - case V4L2_CID_TEST_PATTERN_COLOR_GB:
> - case V4L2_CID_TEST_PATTERN_COLOR_B:
> - /*
> - * Exposure related control will be handled by sensor
> - * driver
> - */
> - ret =
> - v4l2_g_ctrl(isp->inputs[asd->input_curr].camera->
> - ctrl_handler, &ctrl);
> - break;
> case V4L2_CID_FLASH_STATUS:
> case V4L2_CID_FLASH_INTENSITY:
> case V4L2_CID_FLASH_TORCH_INTENSITY:
> @@ -1466,11 +1249,6 @@ static int atomisp_camera_g_ext_ctrls(struct file *file, void *fh,
> case V4L2_CID_ZOOM_ABSOLUTE:
> ret = atomisp_digital_zoom(asd, 0, &ctrl.value);
> break;
> - case V4L2_CID_G_SKIP_FRAMES:
> - ret = v4l2_subdev_call(
> - isp->inputs[asd->input_curr].camera,
> - sensor, g_skip_frames, (u32 *)&ctrl.value);
> - break;
> default:
> ret = -EINVAL;
> }
> @@ -1528,22 +1306,6 @@ static int atomisp_camera_s_ext_ctrls(struct file *file, void *fh,
> ctrl.id = c->controls[i].id;
> ctrl.value = c->controls[i].value;
> switch (ctrl.id) {
> - case V4L2_CID_EXPOSURE_ABSOLUTE:
> - case V4L2_CID_EXPOSURE_AUTO:
> - case V4L2_CID_EXPOSURE_METERING:
> - case V4L2_CID_IRIS_ABSOLUTE:
> - case V4L2_CID_VCM_TIMING:
> - case V4L2_CID_VCM_SLEW:
> - case V4L2_CID_3A_LOCK:
> - case V4L2_CID_TEST_PATTERN:
> - case V4L2_CID_TEST_PATTERN_COLOR_R:
> - case V4L2_CID_TEST_PATTERN_COLOR_GR:
> - case V4L2_CID_TEST_PATTERN_COLOR_GB:
> - case V4L2_CID_TEST_PATTERN_COLOR_B:
> - ret = v4l2_s_ctrl(NULL,
> - isp->inputs[asd->input_curr].camera->
> - ctrl_handler, &ctrl);
> - break;
> case V4L2_CID_FLASH_STATUS:
> case V4L2_CID_FLASH_INTENSITY:
> case V4L2_CID_FLASH_TORCH_INTENSITY:
> @@ -1692,7 +1454,6 @@ static long atomisp_vidioc_default(struct file *file, void *fh,
> bool valid_prio, unsigned int cmd, void *arg)
> {
> struct video_device *vdev = video_devdata(file);
> - struct atomisp_device *isp = video_get_drvdata(vdev);
> struct atomisp_sub_device *asd = atomisp_to_video_pipe(vdev)->asd;
> int err;
>
> @@ -1839,11 +1600,6 @@ static long atomisp_vidioc_default(struct file *file, void *fh,
> err = atomisp_fixed_pattern_table(asd, arg);
> break;
>
> - case ATOMISP_IOC_S_EXPOSURE:
> - err = v4l2_subdev_call(isp->inputs[asd->input_curr].camera,
> - core, ioctl, cmd, arg);
> - break;
> -
> case ATOMISP_IOC_S_ISP_SHD_TAB:
> err = atomisp_set_shading_table(asd, arg);
> break;
> @@ -1860,10 +1616,6 @@ static long atomisp_vidioc_default(struct file *file, void *fh,
> err = atomisp_set_parameters(vdev, arg);
> break;
>
> - case ATOMISP_IOC_EXT_ISP_CTRL:
> - err = v4l2_subdev_call(isp->inputs[asd->input_curr].camera,
> - core, ioctl, cmd, arg);
> - break;
> case ATOMISP_IOC_EXP_ID_UNLOCK:
> err = atomisp_exp_id_unlock(asd, arg);
> break;
> --
> 2.43.0
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 5/9] media: atomisp: Stop setting sd->devnode for the ATOMISP_SUBDEV v4l2-subdev
2024-02-17 11:24 [PATCH 0/9] media: atomisp: Changes for libcamera support Hans de Goede
` (3 preceding siblings ...)
2024-02-17 11:24 ` [PATCH 4/9] media: atomisp: Remove ISP controls which get passed through to the camera Hans de Goede
@ 2024-02-17 11:24 ` Hans de Goede
2024-02-17 11:24 ` [PATCH 6/9] media: atomisp: Add DMABUF support Hans de Goede
` (4 subsequent siblings)
9 siblings, 0 replies; 23+ messages in thread
From: Hans de Goede @ 2024-02-17 11:24 UTC (permalink / raw)
To: Mauro Carvalho Chehab, Sakari Ailus, Andy Shevchenko
Cc: Hans de Goede, Kate Hsuan, Tsuchiya Yuto, Yury Luneff, Nable,
andrey.i.trufanov, Fabio Aiuto, linux-media, linux-staging
Stop setting sd->devnode for the ATOMISP_SUBDEV v4l2-subdev, so that
a proper /dev/v4l-subdev# gets created for it.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
drivers/staging/media/atomisp/pci/atomisp_subdev.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/staging/media/atomisp/pci/atomisp_subdev.c b/drivers/staging/media/atomisp/pci/atomisp_subdev.c
index 8293bda0c681..7f1ca05ce54a 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_subdev.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_subdev.c
@@ -802,7 +802,6 @@ static int isp_subdev_init_entities(struct atomisp_sub_device *asd)
sprintf(sd->name, "ATOMISP_SUBDEV");
v4l2_set_subdevdata(sd, asd);
sd->flags |= V4L2_SUBDEV_FL_HAS_EVENTS | V4L2_SUBDEV_FL_HAS_DEVNODE;
- sd->devnode = &asd->video_out.vdev;
pads[ATOMISP_SUBDEV_PAD_SINK].flags = MEDIA_PAD_FL_SINK;
pads[ATOMISP_SUBDEV_PAD_SOURCE].flags = MEDIA_PAD_FL_SOURCE;
--
2.43.0
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH 6/9] media: atomisp: Add DMABUF support
2024-02-17 11:24 [PATCH 0/9] media: atomisp: Changes for libcamera support Hans de Goede
` (4 preceding siblings ...)
2024-02-17 11:24 ` [PATCH 5/9] media: atomisp: Stop setting sd->devnode for the ATOMISP_SUBDEV v4l2-subdev Hans de Goede
@ 2024-02-17 11:24 ` Hans de Goede
2024-02-17 15:21 ` Kieran Bingham
2024-02-17 11:24 ` [PATCH 7/9] media: atomisp: Change ISP subdev name to "ATOM ISP" Hans de Goede
` (3 subsequent siblings)
9 siblings, 1 reply; 23+ messages in thread
From: Hans de Goede @ 2024-02-17 11:24 UTC (permalink / raw)
To: Mauro Carvalho Chehab, Sakari Ailus, Andy Shevchenko
Cc: Hans de Goede, Kate Hsuan, Tsuchiya Yuto, Yury Luneff, Nable,
andrey.i.trufanov, Fabio Aiuto, linux-media, linux-staging
Add DMABUF support and while at it drop userptr support.
Now that atomisp has been ported to videobuf2 this is trivial.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
drivers/staging/media/atomisp/pci/atomisp_ioctl.c | 1 +
drivers/staging/media/atomisp/pci/atomisp_subdev.c | 2 +-
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/media/atomisp/pci/atomisp_ioctl.c b/drivers/staging/media/atomisp/pci/atomisp_ioctl.c
index 74cf055cb09b..fa6c9f0ea6eb 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_ioctl.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_ioctl.c
@@ -1666,6 +1666,7 @@ const struct v4l2_ioctl_ops atomisp_ioctl_ops = {
.vidioc_querybuf = vb2_ioctl_querybuf,
.vidioc_qbuf = atomisp_qbuf_wrapper,
.vidioc_dqbuf = atomisp_dqbuf_wrapper,
+ .vidioc_expbuf = vb2_ioctl_expbuf,
.vidioc_streamon = vb2_ioctl_streamon,
.vidioc_streamoff = vb2_ioctl_streamoff,
.vidioc_default = atomisp_vidioc_default,
diff --git a/drivers/staging/media/atomisp/pci/atomisp_subdev.c b/drivers/staging/media/atomisp/pci/atomisp_subdev.c
index 7f1ca05ce54a..8253b6faf8cd 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_subdev.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_subdev.c
@@ -765,7 +765,7 @@ static int atomisp_init_subdev_pipe(struct atomisp_sub_device *asd,
/* Init videobuf2 queue structure */
pipe->vb_queue.type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
- pipe->vb_queue.io_modes = VB2_MMAP | VB2_USERPTR;
+ pipe->vb_queue.io_modes = VB2_MMAP | VB2_DMABUF;
pipe->vb_queue.buf_struct_size = sizeof(struct ia_css_frame);
pipe->vb_queue.ops = &atomisp_vb2_ops;
pipe->vb_queue.mem_ops = &vb2_vmalloc_memops;
--
2.43.0
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH 6/9] media: atomisp: Add DMABUF support
2024-02-17 11:24 ` [PATCH 6/9] media: atomisp: Add DMABUF support Hans de Goede
@ 2024-02-17 15:21 ` Kieran Bingham
0 siblings, 0 replies; 23+ messages in thread
From: Kieran Bingham @ 2024-02-17 15:21 UTC (permalink / raw)
To: Andy Shevchenko, Hans de Goede, Mauro Carvalho Chehab,
Sakari Ailus
Cc: Hans de Goede, Kate Hsuan, Tsuchiya Yuto, Yury Luneff, Nable,
andrey.i.trufanov, Fabio Aiuto, linux-media, linux-staging
Quoting Hans de Goede (2024-02-17 11:24:35)
> Add DMABUF support and while at it drop userptr support.
>
> Now that atomisp has been ported to videobuf2 this is trivial.
Seems 'too' easy, Nice.
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
> drivers/staging/media/atomisp/pci/atomisp_ioctl.c | 1 +
> drivers/staging/media/atomisp/pci/atomisp_subdev.c | 2 +-
> 2 files changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/staging/media/atomisp/pci/atomisp_ioctl.c b/drivers/staging/media/atomisp/pci/atomisp_ioctl.c
> index 74cf055cb09b..fa6c9f0ea6eb 100644
> --- a/drivers/staging/media/atomisp/pci/atomisp_ioctl.c
> +++ b/drivers/staging/media/atomisp/pci/atomisp_ioctl.c
> @@ -1666,6 +1666,7 @@ const struct v4l2_ioctl_ops atomisp_ioctl_ops = {
> .vidioc_querybuf = vb2_ioctl_querybuf,
> .vidioc_qbuf = atomisp_qbuf_wrapper,
> .vidioc_dqbuf = atomisp_dqbuf_wrapper,
> + .vidioc_expbuf = vb2_ioctl_expbuf,
> .vidioc_streamon = vb2_ioctl_streamon,
> .vidioc_streamoff = vb2_ioctl_streamoff,
> .vidioc_default = atomisp_vidioc_default,
> diff --git a/drivers/staging/media/atomisp/pci/atomisp_subdev.c b/drivers/staging/media/atomisp/pci/atomisp_subdev.c
> index 7f1ca05ce54a..8253b6faf8cd 100644
> --- a/drivers/staging/media/atomisp/pci/atomisp_subdev.c
> +++ b/drivers/staging/media/atomisp/pci/atomisp_subdev.c
> @@ -765,7 +765,7 @@ static int atomisp_init_subdev_pipe(struct atomisp_sub_device *asd,
>
> /* Init videobuf2 queue structure */
> pipe->vb_queue.type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
> - pipe->vb_queue.io_modes = VB2_MMAP | VB2_USERPTR;
> + pipe->vb_queue.io_modes = VB2_MMAP | VB2_DMABUF;
> pipe->vb_queue.buf_struct_size = sizeof(struct ia_css_frame);
> pipe->vb_queue.ops = &atomisp_vb2_ops;
> pipe->vb_queue.mem_ops = &vb2_vmalloc_memops;
> --
> 2.43.0
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 7/9] media: atomisp: Change ISP subdev name to "ATOM ISP"
2024-02-17 11:24 [PATCH 0/9] media: atomisp: Changes for libcamera support Hans de Goede
` (5 preceding siblings ...)
2024-02-17 11:24 ` [PATCH 6/9] media: atomisp: Add DMABUF support Hans de Goede
@ 2024-02-17 11:24 ` Hans de Goede
2024-02-17 16:00 ` Kieran Bingham
2024-02-17 11:24 ` [PATCH 8/9] media: atomisp: Make MC link from ISP to /dev/video# output node immutable Hans de Goede
` (2 subsequent siblings)
9 siblings, 1 reply; 23+ messages in thread
From: Hans de Goede @ 2024-02-17 11:24 UTC (permalink / raw)
To: Mauro Carvalho Chehab, Sakari Ailus, Andy Shevchenko
Cc: Hans de Goede, Kate Hsuan, Tsuchiya Yuto, Yury Luneff, Nable,
andrey.i.trufanov, Fabio Aiuto, linux-media, linux-staging
Change the generic "ATOMISP_SUBDEV" name to "ATOM ISP" to make clear
that this is the subdev for the ISP itself.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
drivers/staging/media/atomisp/pci/atomisp_subdev.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/media/atomisp/pci/atomisp_subdev.c b/drivers/staging/media/atomisp/pci/atomisp_subdev.c
index 8253b6faf8cd..822fe7d129e2 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_subdev.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_subdev.c
@@ -799,7 +799,7 @@ static int isp_subdev_init_entities(struct atomisp_sub_device *asd)
int ret;
v4l2_subdev_init(sd, &isp_subdev_v4l2_ops);
- sprintf(sd->name, "ATOMISP_SUBDEV");
+ sprintf(sd->name, "ATOM ISP");
v4l2_set_subdevdata(sd, asd);
sd->flags |= V4L2_SUBDEV_FL_HAS_EVENTS | V4L2_SUBDEV_FL_HAS_DEVNODE;
--
2.43.0
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH 7/9] media: atomisp: Change ISP subdev name to "ATOM ISP"
2024-02-17 11:24 ` [PATCH 7/9] media: atomisp: Change ISP subdev name to "ATOM ISP" Hans de Goede
@ 2024-02-17 16:00 ` Kieran Bingham
2024-02-17 18:48 ` Andy Shevchenko
2024-02-18 11:10 ` Hans de Goede
0 siblings, 2 replies; 23+ messages in thread
From: Kieran Bingham @ 2024-02-17 16:00 UTC (permalink / raw)
To: Andy Shevchenko, Hans de Goede, Mauro Carvalho Chehab,
Sakari Ailus
Cc: Hans de Goede, Kate Hsuan, Tsuchiya Yuto, Yury Luneff, Nable,
andrey.i.trufanov, Fabio Aiuto, linux-media, linux-staging
Quoting Hans de Goede (2024-02-17 11:24:36)
> Change the generic "ATOMISP_SUBDEV" name to "ATOM ISP" to make clear
> that this is the subdev for the ISP itself.
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
> drivers/staging/media/atomisp/pci/atomisp_subdev.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/staging/media/atomisp/pci/atomisp_subdev.c b/drivers/staging/media/atomisp/pci/atomisp_subdev.c
> index 8253b6faf8cd..822fe7d129e2 100644
> --- a/drivers/staging/media/atomisp/pci/atomisp_subdev.c
> +++ b/drivers/staging/media/atomisp/pci/atomisp_subdev.c
> @@ -799,7 +799,7 @@ static int isp_subdev_init_entities(struct atomisp_sub_device *asd)
> int ret;
>
> v4l2_subdev_init(sd, &isp_subdev_v4l2_ops);
> - sprintf(sd->name, "ATOMISP_SUBDEV");
> + sprintf(sd->name, "ATOM ISP");
Pure bikeshedding, but I'd probably lower the shouting to just
"Atom ISP"
Either way saying it's a subdev on a subdev isn't much of a value add so
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> v4l2_set_subdevdata(sd, asd);
> sd->flags |= V4L2_SUBDEV_FL_HAS_EVENTS | V4L2_SUBDEV_FL_HAS_DEVNODE;
>
> --
> 2.43.0
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 7/9] media: atomisp: Change ISP subdev name to "ATOM ISP"
2024-02-17 16:00 ` Kieran Bingham
@ 2024-02-17 18:48 ` Andy Shevchenko
2024-02-18 11:10 ` Hans de Goede
1 sibling, 0 replies; 23+ messages in thread
From: Andy Shevchenko @ 2024-02-17 18:48 UTC (permalink / raw)
To: Kieran Bingham
Cc: Andy Shevchenko, Hans de Goede, Mauro Carvalho Chehab,
Sakari Ailus, Kate Hsuan, Tsuchiya Yuto, Yury Luneff, Nable,
andrey.i.trufanov, Fabio Aiuto, linux-media, linux-staging
On Sat, Feb 17, 2024 at 6:00 PM Kieran Bingham
<kieran.bingham@ideasonboard.com> wrote:
> Quoting Hans de Goede (2024-02-17 11:24:36)
...
> Pure bikeshedding, but I'd probably lower the shouting to just
> "Atom ISP"
You beat me to it, +1.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 7/9] media: atomisp: Change ISP subdev name to "ATOM ISP"
2024-02-17 16:00 ` Kieran Bingham
2024-02-17 18:48 ` Andy Shevchenko
@ 2024-02-18 11:10 ` Hans de Goede
1 sibling, 0 replies; 23+ messages in thread
From: Hans de Goede @ 2024-02-18 11:10 UTC (permalink / raw)
To: Kieran Bingham, Andy Shevchenko, Mauro Carvalho Chehab,
Sakari Ailus
Cc: Kate Hsuan, Tsuchiya Yuto, Yury Luneff, Nable, andrey.i.trufanov,
Fabio Aiuto, linux-media, linux-staging
Hi,
On 2/17/24 17:00, Kieran Bingham wrote:
> Quoting Hans de Goede (2024-02-17 11:24:36)
>> Change the generic "ATOMISP_SUBDEV" name to "ATOM ISP" to make clear
>> that this is the subdev for the ISP itself.
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>> drivers/staging/media/atomisp/pci/atomisp_subdev.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/staging/media/atomisp/pci/atomisp_subdev.c b/drivers/staging/media/atomisp/pci/atomisp_subdev.c
>> index 8253b6faf8cd..822fe7d129e2 100644
>> --- a/drivers/staging/media/atomisp/pci/atomisp_subdev.c
>> +++ b/drivers/staging/media/atomisp/pci/atomisp_subdev.c
>> @@ -799,7 +799,7 @@ static int isp_subdev_init_entities(struct atomisp_sub_device *asd)
>> int ret;
>>
>> v4l2_subdev_init(sd, &isp_subdev_v4l2_ops);
>> - sprintf(sd->name, "ATOMISP_SUBDEV");
>> + sprintf(sd->name, "ATOM ISP");
>
> Pure bikeshedding, but I'd probably lower the shouting to just
> "Atom ISP"
Works for me, I've fixed this in my personal tree.
> Either way saying it's a subdev on a subdev isn't much of a value add so
>
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Thank you for this and all the other reviews.
Regards,
Hans
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 8/9] media: atomisp: Make MC link from ISP to /dev/video# output node immutable
2024-02-17 11:24 [PATCH 0/9] media: atomisp: Changes for libcamera support Hans de Goede
` (6 preceding siblings ...)
2024-02-17 11:24 ` [PATCH 7/9] media: atomisp: Change ISP subdev name to "ATOM ISP" Hans de Goede
@ 2024-02-17 11:24 ` Hans de Goede
2024-02-17 15:22 ` Kieran Bingham
2024-02-17 11:24 ` [PATCH 9/9] media: atomisp: Implement link_setup op for ISP subdev MC entity Hans de Goede
2024-02-17 18:53 ` [PATCH 0/9] media: atomisp: Changes for libcamera support Andy Shevchenko
9 siblings, 1 reply; 23+ messages in thread
From: Hans de Goede @ 2024-02-17 11:24 UTC (permalink / raw)
To: Mauro Carvalho Chehab, Sakari Ailus, Andy Shevchenko
Cc: Hans de Goede, Kate Hsuan, Tsuchiya Yuto, Yury Luneff, Nable,
andrey.i.trufanov, Fabio Aiuto, linux-media, linux-staging
The link from the ISP's source pad to the /dev/video# output v4l2-dev
is always enabled and immutable, mark it as such.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
drivers/staging/media/atomisp/pci/atomisp_v4l2.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/media/atomisp/pci/atomisp_v4l2.c b/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
index 1a936dbe8eb4..86028721e7cb 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
@@ -1090,7 +1090,8 @@ int atomisp_register_device_nodes(struct atomisp_device *isp)
return err;
err = media_create_pad_link(&isp->asd.subdev.entity, ATOMISP_SUBDEV_PAD_SOURCE,
- &isp->asd.video_out.vdev.entity, 0, 0);
+ &isp->asd.video_out.vdev.entity, 0,
+ MEDIA_LNK_FL_ENABLED | MEDIA_LNK_FL_IMMUTABLE);
if (err)
return err;
--
2.43.0
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH 8/9] media: atomisp: Make MC link from ISP to /dev/video# output node immutable
2024-02-17 11:24 ` [PATCH 8/9] media: atomisp: Make MC link from ISP to /dev/video# output node immutable Hans de Goede
@ 2024-02-17 15:22 ` Kieran Bingham
0 siblings, 0 replies; 23+ messages in thread
From: Kieran Bingham @ 2024-02-17 15:22 UTC (permalink / raw)
To: Andy Shevchenko, Hans de Goede, Mauro Carvalho Chehab,
Sakari Ailus
Cc: Hans de Goede, Kate Hsuan, Tsuchiya Yuto, Yury Luneff, Nable,
andrey.i.trufanov, Fabio Aiuto, linux-media, linux-staging
Quoting Hans de Goede (2024-02-17 11:24:37)
> The link from the ISP's source pad to the /dev/video# output v4l2-dev
> is always enabled and immutable, mark it as such.
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
> drivers/staging/media/atomisp/pci/atomisp_v4l2.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/staging/media/atomisp/pci/atomisp_v4l2.c b/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
> index 1a936dbe8eb4..86028721e7cb 100644
> --- a/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
> +++ b/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
> @@ -1090,7 +1090,8 @@ int atomisp_register_device_nodes(struct atomisp_device *isp)
> return err;
>
> err = media_create_pad_link(&isp->asd.subdev.entity, ATOMISP_SUBDEV_PAD_SOURCE,
> - &isp->asd.video_out.vdev.entity, 0, 0);
> + &isp->asd.video_out.vdev.entity, 0,
> + MEDIA_LNK_FL_ENABLED | MEDIA_LNK_FL_IMMUTABLE);
> if (err)
> return err;
>
> --
> 2.43.0
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 9/9] media: atomisp: Implement link_setup op for ISP subdev MC entity
2024-02-17 11:24 [PATCH 0/9] media: atomisp: Changes for libcamera support Hans de Goede
` (7 preceding siblings ...)
2024-02-17 11:24 ` [PATCH 8/9] media: atomisp: Make MC link from ISP to /dev/video# output node immutable Hans de Goede
@ 2024-02-17 11:24 ` Hans de Goede
2024-02-17 18:52 ` Andy Shevchenko
2024-02-17 18:53 ` [PATCH 0/9] media: atomisp: Changes for libcamera support Andy Shevchenko
9 siblings, 1 reply; 23+ messages in thread
From: Hans de Goede @ 2024-02-17 11:24 UTC (permalink / raw)
To: Mauro Carvalho Chehab, Sakari Ailus, Andy Shevchenko
Cc: Hans de Goede, Kate Hsuan, Tsuchiya Yuto, Yury Luneff, Nable,
andrey.i.trufanov, Fabio Aiuto, linux-media, linux-staging
The atomisp driver's Android heritage makes it weird in that
even though it uses MC + subdev-s it is designed to primarily
be controlled through its /dev/video# node.
It implements s_input on /dev/video# to select which sensor to use,
while ignoring setup_link calls to enable a link to another sensor.
Add support for selecting the active sensor the MC way by adding
setup_link support.
The implementation is a bit convoluted due to the atomisp driver's
heritage.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
.../media/atomisp/pci/atomisp_subdev.c | 52 +++++++++++++++++++
1 file changed, 52 insertions(+)
diff --git a/drivers/staging/media/atomisp/pci/atomisp_subdev.c b/drivers/staging/media/atomisp/pci/atomisp_subdev.c
index 822fe7d129e2..17819d7586dd 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_subdev.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_subdev.c
@@ -30,6 +30,7 @@
#include "atomisp_common.h"
#include "atomisp_compat.h"
#include "atomisp_fops.h"
+#include "atomisp_ioctl.h"
#include "atomisp_internal.h"
const struct atomisp_in_fmt_conv atomisp_in_fmt_conv[] = {
@@ -634,8 +635,59 @@ static void isp_subdev_init_params(struct atomisp_sub_device *asd)
}
/* media operations */
+static int atomisp_link_setup(struct media_entity *entity,
+ const struct media_pad *local,
+ const struct media_pad *remote, u32 flags)
+{
+ struct v4l2_subdev *sd = container_of(entity, struct v4l2_subdev,
+ entity);
+ struct atomisp_sub_device *asd = v4l2_get_subdevdata(sd);
+ struct atomisp_device *isp = asd->isp;
+ int i, csi_idx, ret;
+
+ /* ISP's source is immutable */
+ if (local != &asd->pads[ATOMISP_SUBDEV_PAD_SINK]) {
+ v4l2_err(sd, "Error pad %d does not support changing flags\n",
+ local->index);
+ return -EINVAL;
+ }
+
+ for (csi_idx = 0; csi_idx < ATOMISP_CAMERA_NR_PORTS; csi_idx++) {
+ if (&isp->csi2_port[csi_idx].pads[CSI2_PAD_SOURCE] == remote)
+ break;
+ }
+
+ if (csi_idx == ATOMISP_CAMERA_NR_PORTS) {
+ v4l2_err(sd, "Error cannot find CSI receiver for remote pad\n");
+ return -EINVAL;
+ }
+
+ /* Ignore disables, input_curr should only be updated on enables */
+ if (!(flags & MEDIA_LNK_FL_ENABLED))
+ return 0;
+
+ for (i = 0; i < isp->input_cnt; i++) {
+ if (isp->inputs[i].camera == isp->sensor_subdevs[csi_idx])
+ break;
+ }
+
+ if (i == isp->input_cnt) {
+ v4l2_err(sd, "Error no sensor for CSI receiver %d\n", csi_idx);
+ return -EINVAL;
+ }
+
+ mutex_lock(&isp->mutex);
+ ret = atomisp_pipe_check(&asd->video_out, true);
+ if (ret == 0)
+ asd->input_curr = i;
+ mutex_unlock(&isp->mutex);
+
+ return ret;
+}
+
static const struct media_entity_operations isp_subdev_media_ops = {
.link_validate = v4l2_subdev_link_validate,
+ .link_setup = atomisp_link_setup,
/* .set_power = v4l2_subdev_set_power, */
};
--
2.43.0
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH 9/9] media: atomisp: Implement link_setup op for ISP subdev MC entity
2024-02-17 11:24 ` [PATCH 9/9] media: atomisp: Implement link_setup op for ISP subdev MC entity Hans de Goede
@ 2024-02-17 18:52 ` Andy Shevchenko
2024-02-18 11:08 ` Hans de Goede
0 siblings, 1 reply; 23+ messages in thread
From: Andy Shevchenko @ 2024-02-17 18:52 UTC (permalink / raw)
To: Hans de Goede
Cc: Mauro Carvalho Chehab, Sakari Ailus, Andy Shevchenko, Kate Hsuan,
Tsuchiya Yuto, Yury Luneff, Nable, andrey.i.trufanov, Fabio Aiuto,
linux-media, linux-staging
On Sat, Feb 17, 2024 at 1:25 PM Hans de Goede <hdegoede@redhat.com> wrote:
>
> The atomisp driver's Android heritage makes it weird in that
> even though it uses MC + subdev-s it is designed to primarily
> be controlled through its /dev/video# node.
>
> It implements s_input on /dev/video# to select which sensor to use,
> while ignoring setup_link calls to enable a link to another sensor.
>
> Add support for selecting the active sensor the MC way by adding
> setup_link support.
->link_setup()
> The implementation is a bit convoluted due to the atomisp driver's
> heritage.
...
> #include "atomisp_common.h"
> #include "atomisp_compat.h"
> #include "atomisp_fops.h"
> +#include "atomisp_ioctl.h"
> #include "atomisp_internal.h"
Hmm... Perhaps keep it ordered?
...
> + mutex_lock(&isp->mutex);
Side note: Are you planning to use cleanup.h at some point?
> + ret = atomisp_pipe_check(&asd->video_out, true);
> + if (ret == 0)
> + asd->input_curr = i;
> + mutex_unlock(&isp->mutex);
> +
> + return ret;
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 9/9] media: atomisp: Implement link_setup op for ISP subdev MC entity
2024-02-17 18:52 ` Andy Shevchenko
@ 2024-02-18 11:08 ` Hans de Goede
0 siblings, 0 replies; 23+ messages in thread
From: Hans de Goede @ 2024-02-18 11:08 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Mauro Carvalho Chehab, Sakari Ailus, Andy Shevchenko, Kate Hsuan,
Tsuchiya Yuto, Yury Luneff, Nable, andrey.i.trufanov, Fabio Aiuto,
linux-media, linux-staging
Hi,
On 2/17/24 19:52, Andy Shevchenko wrote:
> On Sat, Feb 17, 2024 at 1:25 PM Hans de Goede <hdegoede@redhat.com> wrote:
>>
>> The atomisp driver's Android heritage makes it weird in that
>> even though it uses MC + subdev-s it is designed to primarily
>> be controlled through its /dev/video# node.
>>
>> It implements s_input on /dev/video# to select which sensor to use,
>> while ignoring setup_link calls to enable a link to another sensor.
>>
>> Add support for selecting the active sensor the MC way by adding
>> setup_link support.
>
> ->link_setup()
>
>> The implementation is a bit convoluted due to the atomisp driver's
>> heritage.
>
> ...
>
>> #include "atomisp_common.h"
>> #include "atomisp_compat.h"
>> #include "atomisp_fops.h"
>> +#include "atomisp_ioctl.h"
>> #include "atomisp_internal.h"
>
> Hmm... Perhaps keep it ordered?
Yeah my bad, I intended to keep it ordered but I somehow got it wrong.
this and the commit msg remark are both fixed in my personal tree now.
>
> ...
>
>> + mutex_lock(&isp->mutex);
>
> Side note: Are you planning to use cleanup.h at some point?
Maybe. I have no objections against. For functions needing
e.g. heap memory only locally it certainly makes sense.
Regards,
Hans
>
>> + ret = atomisp_pipe_check(&asd->video_out, true);
>> + if (ret == 0)
>> + asd->input_curr = i;
>> + mutex_unlock(&isp->mutex);
>> +
>> + return ret;
>
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 0/9] media: atomisp: Changes for libcamera support
2024-02-17 11:24 [PATCH 0/9] media: atomisp: Changes for libcamera support Hans de Goede
` (8 preceding siblings ...)
2024-02-17 11:24 ` [PATCH 9/9] media: atomisp: Implement link_setup op for ISP subdev MC entity Hans de Goede
@ 2024-02-17 18:53 ` Andy Shevchenko
2024-02-18 11:15 ` Hans de Goede
9 siblings, 1 reply; 23+ messages in thread
From: Andy Shevchenko @ 2024-02-17 18:53 UTC (permalink / raw)
To: Hans de Goede
Cc: Mauro Carvalho Chehab, Sakari Ailus, Andy Shevchenko, Kate Hsuan,
Tsuchiya Yuto, Yury Luneff, Nable, andrey.i.trufanov, Fabio Aiuto,
linux-media, linux-staging
On Sat, Feb 17, 2024 at 1:24 PM Hans de Goede <hdegoede@redhat.com> wrote:
>
> Hi All,
>
> Here is a series of cleanups (prep work) + some small(ish)
> changes to make the atomisp driver work with libcamera.
>
> This has been tested with libcamera's simple pipelinehandler
> together with an ov2680 sensor.
With some minor comments,
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> p.s.
>
> Andy, I known I still need to do some small cleanups from my last
> pull-requests. I have this on my TODO list :)
Hope you will find time to address. Thanks for maintaining this driver!
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH 0/9] media: atomisp: Changes for libcamera support
2024-02-17 18:53 ` [PATCH 0/9] media: atomisp: Changes for libcamera support Andy Shevchenko
@ 2024-02-18 11:15 ` Hans de Goede
0 siblings, 0 replies; 23+ messages in thread
From: Hans de Goede @ 2024-02-18 11:15 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Mauro Carvalho Chehab, Sakari Ailus, Andy Shevchenko, Kate Hsuan,
Tsuchiya Yuto, Yury Luneff, Nable, andrey.i.trufanov, Fabio Aiuto,
linux-media, linux-staging
Hi Andy,
On 2/17/24 19:53, Andy Shevchenko wrote:
> On Sat, Feb 17, 2024 at 1:24 PM Hans de Goede <hdegoede@redhat.com> wrote:
>>
>> Hi All,
>>
>> Here is a series of cleanups (prep work) + some small(ish)
>> changes to make the atomisp driver work with libcamera.
>>
>> This has been tested with libcamera's simple pipelinehandler
>> together with an ov2680 sensor.
>
> With some minor comments,
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Thank you for all the reviews.
Regards,
Hans
^ permalink raw reply [flat|nested] 23+ messages in thread