Re: [PATCH 3/9] media: atomisp: Remove custom VCM handling

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxxxxxxxx>

> 
> Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx>
> ---
>  .../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
>





[Index of Archives]     [Linux Driver Development]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux