| Message ID | 20240328-uvc-fix-relative-ptz-speed-v1-1-17373eb8b2be@securitylive.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | media: uvcvideo: UVC minimum relative pan/tilt/zoom speed fix | expand |
Hi John, kernel test robot noticed the following build warnings: [auto build test WARNING on 23956900041d968f9ad0f30db6dede4daccd7aa9] url: https://github.com/intel-lab-lkp/linux/commits/John-Bauer-via-B4-Relay/media-uvcvideo-UVC-minimum-relative-pan-tilt-zoom-speed-fix/20240329-071938 base: 23956900041d968f9ad0f30db6dede4daccd7aa9 patch link: https://lore.kernel.org/r/20240328-uvc-fix-relative-ptz-speed-v1-1-17373eb8b2be%40securitylive.com patch subject: [PATCH 1/2] media: uvcvideo: UVC minimum relative pan/tilt/zoom speed fix config: arm-defconfig (https://download.01.org/0day-ci/archive/20240330/202403300745.omqy1CdY-lkp@intel.com/config) compiler: clang version 14.0.6 (https://github.com/llvm/llvm-project.git f28c006a5895fc0e329fe15fead81e37457cb1d1) reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240330/202403300745.omqy1CdY-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202403300745.omqy1CdY-lkp@intel.com/ All warnings (new ones prefixed by >>): >> drivers/media/usb/uvc/uvc_ctrl.c:1944:3: warning: unannotated fall-through between switch labels [-Wimplicit-fallthrough] default: ^ drivers/media/usb/uvc/uvc_ctrl.c:1944:3: note: insert 'break;' to avoid fall-through default: ^ break; 1 warning generated. vim +1944 drivers/media/usb/uvc/uvc_ctrl.c 1898 1899 int uvc_ctrl_set(struct uvc_fh *handle, 1900 struct v4l2_ext_control *xctrl) 1901 { 1902 struct uvc_video_chain *chain = handle->chain; 1903 struct uvc_control *ctrl; 1904 struct uvc_control_mapping *mapping; 1905 s32 value; 1906 u32 step; 1907 s32 min; 1908 s32 max; 1909 int ret; 1910 1911 if (__uvc_query_v4l2_class(chain, xctrl->id, 0) >= 0) 1912 return -EACCES; 1913 1914 ctrl = uvc_find_control(chain, xctrl->id, &mapping); 1915 if (ctrl == NULL) 1916 return -EINVAL; 1917 if (!(ctrl->info.flags & UVC_CTRL_FLAG_SET_CUR)) 1918 return -EACCES; 1919 1920 /* Clamp out of range values. */ 1921 switch (mapping->v4l2_type) { 1922 case V4L2_CTRL_TYPE_INTEGER: 1923 if (!ctrl->cached) { 1924 ret = uvc_ctrl_populate_cache(chain, ctrl); 1925 if (ret < 0) 1926 return ret; 1927 } 1928 1929 min = mapping->get(mapping, UVC_GET_MIN, 1930 uvc_ctrl_data(ctrl, UVC_CTRL_DATA_MIN)); 1931 max = mapping->get(mapping, UVC_GET_MAX, 1932 uvc_ctrl_data(ctrl, UVC_CTRL_DATA_MAX)); 1933 1934 /* 1935 * For the relative speed implementation the minimum 1936 * value cannot be probed so it becomes the additive 1937 * inverse of maximum. 1938 */ 1939 switch (xctrl->id) { 1940 case V4L2_CID_ZOOM_CONTINUOUS: 1941 case V4L2_CID_PAN_SPEED: 1942 case V4L2_CID_TILT_SPEED: 1943 min = max * -1; > 1944 default: 1945 break; 1946 } 1947 1948 step = mapping->get(mapping, UVC_GET_RES, 1949 uvc_ctrl_data(ctrl, UVC_CTRL_DATA_RES)); 1950 if (step == 0) 1951 step = 1; 1952 1953 xctrl->value = min + DIV_ROUND_CLOSEST((u32)(xctrl->value - min), 1954 step) * step; 1955 if (mapping->data_type == UVC_CTRL_DATA_TYPE_SIGNED) 1956 xctrl->value = clamp(xctrl->value, min, max); 1957 else 1958 xctrl->value = clamp_t(u32, xctrl->value, min, max); 1959 value = xctrl->value; 1960 break; 1961 1962 case V4L2_CTRL_TYPE_BITMASK: 1963 if (!ctrl->cached) { 1964 ret = uvc_ctrl_populate_cache(chain, ctrl); 1965 if (ret < 0) 1966 return ret; 1967 } 1968 1969 xctrl->value &= uvc_get_ctrl_bitmap(ctrl, mapping); 1970 value = xctrl->value; 1971 break; 1972 1973 case V4L2_CTRL_TYPE_BOOLEAN: 1974 xctrl->value = clamp(xctrl->value, 0, 1); 1975 value = xctrl->value; 1976 break; 1977 1978 case V4L2_CTRL_TYPE_MENU: 1979 if (xctrl->value < (ffs(mapping->menu_mask) - 1) || 1980 xctrl->value > (fls(mapping->menu_mask) - 1)) 1981 return -ERANGE; 1982 1983 if (!test_bit(xctrl->value, &mapping->menu_mask)) 1984 return -EINVAL; 1985 1986 value = uvc_mapping_get_menu_value(mapping, xctrl->value); 1987 1988 /* 1989 * Valid menu indices are reported by the GET_RES request for 1990 * UVC controls that support it. 1991 */ 1992 if (mapping->data_type == UVC_CTRL_DATA_TYPE_BITMASK) { 1993 if (!ctrl->cached) { 1994 ret = uvc_ctrl_populate_cache(chain, ctrl); 1995 if (ret < 0) 1996 return ret; 1997 } 1998 1999 if (!(uvc_get_ctrl_bitmap(ctrl, mapping) & value)) 2000 return -EINVAL; 2001 } 2002 2003 break; 2004 2005 default: 2006 value = xctrl->value; 2007 break; 2008 } 2009 2010 /* 2011 * If the mapping doesn't span the whole UVC control, the current value 2012 * needs to be loaded from the device to perform the read-modify-write 2013 * operation. 2014 */ 2015 if ((ctrl->info.size * 8) != mapping->size) { 2016 ret = __uvc_ctrl_load_cur(chain, ctrl); 2017 if (ret < 0) 2018 return ret; 2019 } 2020 2021 /* Backup the current value in case we need to rollback later. */ 2022 if (!ctrl->dirty) { 2023 memcpy(uvc_ctrl_data(ctrl, UVC_CTRL_DATA_BACKUP), 2024 uvc_ctrl_data(ctrl, UVC_CTRL_DATA_CURRENT), 2025 ctrl->info.size); 2026 } 2027 2028 mapping->set(mapping, value, 2029 uvc_ctrl_data(ctrl, UVC_CTRL_DATA_CURRENT)); 2030 2031 if (ctrl->info.flags & UVC_CTRL_FLAG_ASYNCHRONOUS) 2032 ctrl->handle = handle; 2033 2034 ctrl->dirty = 1; 2035 ctrl->modified = 1; 2036 return 0; 2037 } 2038
Hi John
Maybe you could add a function like
static bool is_relative_ptz_ctr(__u32 id)
{
switch (xctrl->id) {
case V4L2_CID_ZOOM_CONTINUOUS:
case V4L2_CID_PAN_SPEED:
case V4L2_CID_TILT_SPEED:
return true;
}
return false;
}
to figure out if a control is relative or not, and avoid code duplication.
On Fri, 29 Mar 2024 at 00:18, John Bauer via B4 Relay
<devnull+johnebgood.securitylive.com@kernel.org> wrote:
>
> From: John Bauer <johnebgood@securitylive.com>
>
> The minimum UVC control value for the relative pan/tilt/zoom speeds
> cannot be probed as the implementation condenses the pan and tilt
> direction and speed into two 16 bit values. The minimum cannot be
> set at probe time because it is probed first and the maximum is not
> yet known. With this fix if a relative speed control is queried
> or set the minimum is set and checked based on the additive inverse of
> the maximum at that time.
>
> Signed-off-by: John Bauer <johnebgood@securitylive.com>
> ---
> drivers/media/usb/uvc/uvc_ctrl.c | 37 ++++++++++++++++++++++++++++++++++---
> 1 file changed, 34 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
> index e59a463c2761..b389ab3ee05d 100644
> --- a/drivers/media/usb/uvc/uvc_ctrl.c
> +++ b/drivers/media/usb/uvc/uvc_ctrl.c
> @@ -1322,9 +1322,25 @@ static int __uvc_query_v4l2_ctrl(struct uvc_video_chain *chain,
> break;
> }
>
> - if (ctrl->info.flags & UVC_CTRL_FLAG_GET_MIN)
> - v4l2_ctrl->minimum = mapping->get(mapping, UVC_GET_MIN,
> - uvc_ctrl_data(ctrl, UVC_CTRL_DATA_MIN));
> + if (ctrl->info.flags & UVC_CTRL_FLAG_GET_MIN) {
> + switch (v4l2_ctrl->id) {
> + case V4L2_CID_ZOOM_CONTINUOUS:
> + case V4L2_CID_PAN_SPEED:
> + case V4L2_CID_TILT_SPEED:
> + /*
> + * For the relative speed implementation the minimum
> + * value cannot be probed so it becomes the additive
> + * inverse of maximum.
> + */
> + v4l2_ctrl->minimum = -1 * mapping->get(mapping, UVC_GET_MAX,
> + uvc_ctrl_data(ctrl, UVC_CTRL_DATA_MAX));
> + break;
> + default:
> + v4l2_ctrl->minimum = mapping->get(mapping, UVC_GET_MIN,
> + uvc_ctrl_data(ctrl, UVC_CTRL_DATA_MIN));
> + break;
> + }
> + }
>
> if (ctrl->info.flags & UVC_CTRL_FLAG_GET_MAX)
> v4l2_ctrl->maximum = mapping->get(mapping, UVC_GET_MAX,
> @@ -1914,6 +1930,21 @@ int uvc_ctrl_set(struct uvc_fh *handle,
> uvc_ctrl_data(ctrl, UVC_CTRL_DATA_MIN));
> max = mapping->get(mapping, UVC_GET_MAX,
> uvc_ctrl_data(ctrl, UVC_CTRL_DATA_MAX));
> +
> + /*
> + * For the relative speed implementation the minimum
> + * value cannot be probed so it becomes the additive
> + * inverse of maximum.
> + */
> + switch (xctrl->id) {
> + case V4L2_CID_ZOOM_CONTINUOUS:
> + case V4L2_CID_PAN_SPEED:
> + case V4L2_CID_TILT_SPEED:
> + min = max * -1;
> + default:
> + break;
> + }
> +
> step = mapping->get(mapping, UVC_GET_RES,
> uvc_ctrl_data(ctrl, UVC_CTRL_DATA_RES));
> if (step == 0)
>
> --
> 2.34.1
>
>
diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c index e59a463c2761..b389ab3ee05d 100644 --- a/drivers/media/usb/uvc/uvc_ctrl.c +++ b/drivers/media/usb/uvc/uvc_ctrl.c @@ -1322,9 +1322,25 @@ static int __uvc_query_v4l2_ctrl(struct uvc_video_chain *chain, break; } - if (ctrl->info.flags & UVC_CTRL_FLAG_GET_MIN) - v4l2_ctrl->minimum = mapping->get(mapping, UVC_GET_MIN, - uvc_ctrl_data(ctrl, UVC_CTRL_DATA_MIN)); + if (ctrl->info.flags & UVC_CTRL_FLAG_GET_MIN) { + switch (v4l2_ctrl->id) { + case V4L2_CID_ZOOM_CONTINUOUS: + case V4L2_CID_PAN_SPEED: + case V4L2_CID_TILT_SPEED: + /* + * For the relative speed implementation the minimum + * value cannot be probed so it becomes the additive + * inverse of maximum. + */ + v4l2_ctrl->minimum = -1 * mapping->get(mapping, UVC_GET_MAX, + uvc_ctrl_data(ctrl, UVC_CTRL_DATA_MAX)); + break; + default: + v4l2_ctrl->minimum = mapping->get(mapping, UVC_GET_MIN, + uvc_ctrl_data(ctrl, UVC_CTRL_DATA_MIN)); + break; + } + } if (ctrl->info.flags & UVC_CTRL_FLAG_GET_MAX) v4l2_ctrl->maximum = mapping->get(mapping, UVC_GET_MAX, @@ -1914,6 +1930,21 @@ int uvc_ctrl_set(struct uvc_fh *handle, uvc_ctrl_data(ctrl, UVC_CTRL_DATA_MIN)); max = mapping->get(mapping, UVC_GET_MAX, uvc_ctrl_data(ctrl, UVC_CTRL_DATA_MAX)); + + /* + * For the relative speed implementation the minimum + * value cannot be probed so it becomes the additive + * inverse of maximum. + */ + switch (xctrl->id) { + case V4L2_CID_ZOOM_CONTINUOUS: + case V4L2_CID_PAN_SPEED: + case V4L2_CID_TILT_SPEED: + min = max * -1; + default: + break; + } + step = mapping->get(mapping, UVC_GET_RES, uvc_ctrl_data(ctrl, UVC_CTRL_DATA_RES)); if (step == 0)