Re: [PATCH 4/9] media: atomisp: Remove ISP controls which get passed through to the camera

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

 



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@xxxxxxxxxx>
> ---
>  .../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@xxxxxxxxxxxxxxxx>

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





[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