[v1,5/6] media: uvcvideo: Initialize roi to default value

Message ID 20220516092209.1801656-6-yunkec@google.com (mailing list archive)
State Superseded
Delegated to: Laurent Pinchart
Headers
Series media: Implement UVC v1.5 ROI |

Commit Message

Yunke Cao May 16, 2022, 9:22 a.m. UTC
  For the devices we tested with, firmwares have wrong initial
ROI configuration. Initialize roi by setting to default value.

Signed-off-by: Yunke Cao <yunkec@google.com>
---
 drivers/media/usb/uvc/uvc_ctrl.c | 39 ++++++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)
  

Comments

kernel test robot May 16, 2022, 1:25 p.m. UTC | #1
Hi Yunke,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on media-tree/master]
[also build test ERROR on linus/master v5.18-rc7 next-20220513]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/intel-lab-lkp/linux/commits/Yunke-Cao/media-Implement-UVC-v1-5-ROI/20220516-172509
base:   git://linuxtv.org/media_tree.git master
config: i386-buildonly-randconfig-r002-20220516 (https://download.01.org/0day-ci/archive/20220516/202205162134.uZvFM4c1-lkp@intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project 853fa8ee225edf2d0de94b0dcbd31bea916e825e)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/453e68b0e012e424659c3fd4c4c7824ad768f440
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Yunke-Cao/media-Implement-UVC-v1-5-ROI/20220516-172509
        git checkout 453e68b0e012e424659c3fd4c4c7824ad768f440
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 SHELL=/bin/bash drivers/media/usb/uvc/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> drivers/media/usb/uvc/uvc_ctrl.c:2587:44: error: use of undeclared identifier 'camera_entity'
                               uvc_entity_match_guid(ctrl->entity, camera_entity))
                                                                   ^
   1 error generated.


vim +/camera_entity +2587 drivers/media/usb/uvc/uvc_ctrl.c

  2551	
  2552	/*
  2553	 * Add control information and hardcoded stock control mappings to the given
  2554	 * device.
  2555	 */
  2556	static void uvc_ctrl_init_ctrl(struct uvc_video_chain *chain,
  2557				       struct uvc_control *ctrl)
  2558	{
  2559		const struct uvc_control_info *info = uvc_ctrls;
  2560		const struct uvc_control_info *iend = info + ARRAY_SIZE(uvc_ctrls);
  2561		const struct uvc_control_mapping *mapping = uvc_ctrl_mappings;
  2562		const struct uvc_control_mapping *mend =
  2563			mapping + ARRAY_SIZE(uvc_ctrl_mappings);
  2564	
  2565		/* XU controls initialization requires querying the device for control
  2566		 * information. As some buggy UVC devices will crash when queried
  2567		 * repeatedly in a tight loop, delay XU controls initialization until
  2568		 * first use.
  2569		 */
  2570		if (UVC_ENTITY_TYPE(ctrl->entity) == UVC_VC_EXTENSION_UNIT)
  2571			return;
  2572	
  2573		for (; info < iend; ++info) {
  2574			if (uvc_entity_match_guid(ctrl->entity, info->entity) &&
  2575			    ctrl->index == info->index) {
  2576				uvc_ctrl_add_info(chain->dev, ctrl, info);
  2577				/*
  2578				 * Retrieve control flags from the device. Ignore errors
  2579				 * and work with default flag values from the uvc_ctrl
  2580				 * array when the device doesn't properly implement
  2581				 * GET_INFO on standard controls.
  2582				 */
  2583				uvc_ctrl_get_flags(chain->dev, ctrl, &ctrl->info);
  2584	
  2585				if (ctrl->info.selector ==
  2586					UVC_CT_REGION_OF_INTEREST_CONTROL &&
> 2587				    uvc_entity_match_guid(ctrl->entity, camera_entity))
  2588					uvc_ctrl_init_roi(chain->dev, ctrl);
  2589				break;
  2590			 }
  2591		}
  2592	
  2593		if (!ctrl->initialized)
  2594			return;
  2595	
  2596		for (; mapping < mend; ++mapping) {
  2597			if (uvc_entity_match_guid(ctrl->entity, mapping->entity) &&
  2598			    ctrl->info.selector == mapping->selector)
  2599				__uvc_ctrl_add_mapping(chain, ctrl, mapping);
  2600		}
  2601	}
  2602
  

Patch

diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
index c3d816985f13..f90b425a0a20 100644
--- a/drivers/media/usb/uvc/uvc_ctrl.c
+++ b/drivers/media/usb/uvc/uvc_ctrl.c
@@ -2505,6 +2505,40 @@  static void uvc_ctrl_prune_entity(struct uvc_device *dev,
 	}
 }
 
+static int uvc_ctrl_init_roi(struct uvc_device *dev, struct uvc_control *ctrl)
+{
+	int ret;
+
+	ret = uvc_query_ctrl(dev, UVC_GET_DEF, ctrl->entity->id, dev->intfnum,
+			     UVC_CT_REGION_OF_INTEREST_CONTROL,
+			     uvc_ctrl_data(ctrl, UVC_CTRL_DATA_DEF),
+			     ctrl->info.size);
+	if (ret)
+		goto out;
+
+	/*
+	 * Some firmwares have wrong GET_CURRENT configuration. E.g. it's
+	 * below GET_MIN, or have rectangle coordinates mixed up. This
+	 * causes problems sometimes, because we are unable to set
+	 * auto-controls value without first setting ROI rectangle to
+	 * valid configuration.
+	 *
+	 * We expect that default configuration is always correct and
+	 * is within the GET_MIN / GET_MAX boundaries.
+	 *
+	 * Set current ROI configuration to GET_DEF, so that we will
+	 * always have properly configured ROI.
+	 */
+	ret = uvc_query_ctrl(dev, UVC_SET_CUR, 1, dev->intfnum,
+			     UVC_CT_REGION_OF_INTEREST_CONTROL,
+			     uvc_ctrl_data(ctrl, UVC_CTRL_DATA_DEF),
+			     ctrl->info.size);
+out:
+	if (ret)
+		dev_err(&dev->udev->dev, "Failed to fixup ROI (%d).\n", ret);
+	return ret;
+}
+
 /*
  * Add control information and hardcoded stock control mappings to the given
  * device.
@@ -2537,6 +2571,11 @@  static void uvc_ctrl_init_ctrl(struct uvc_video_chain *chain,
 			 * GET_INFO on standard controls.
 			 */
 			uvc_ctrl_get_flags(chain->dev, ctrl, &ctrl->info);
+
+			if (ctrl->info.selector ==
+				UVC_CT_REGION_OF_INTEREST_CONTROL &&
+			    uvc_entity_match_guid(ctrl->entity, camera_entity))
+				uvc_ctrl_init_roi(chain->dev, ctrl);
 			break;
 		 }
 	}