Re: [PATCH v4 1/4] media: uvcvideo: report AUTO_UPDATE controls as volatile

[Hans Verkuil <hverkuil+cisco@kernel.org>]

On 29/09/2026 14:34, Laurent Pinchart wrote:
> On Mon, Sep 28, 2026 at 05:50:18PM -0400, Michael Jordan wrote:
>> AUTO_UPDATE controls can change on their own, and the driver already
>> re-reads them from the device. Tell userspace by setting
>> V4L2_CTRL_FLAG_VOLATILE, plus EXECUTE_ON_WRITE for writable controls
>> so that writes are not ignored.
> 
> Writes are not ignored by the driver regardless of whether or not
> V4L2_CTRL_FLAG_EXECUTE_ON_WRITE is reported to userspace.
> 
>> Suggested-by: Ricardo Ribalda <ribalda@chromium.org>
>> Reviewed-by: Ricardo Ribalda <ribalda@chromium.org>
>> Reviewed-by: Hans de Goede <johannes.goede@oss.qualcomm.com>
>> Assisted-by: LLM
>> Signed-off-by: Michael Jordan <jordan.mymail@gmail.com>
>> ---
>>  drivers/media/usb/uvc/uvc_ctrl.c | 11 +++++++++++
>>  1 file changed, 11 insertions(+)
>>
>> diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
>> index 3ca108b83f1d..aceb263103e9 100644
>> --- a/drivers/media/usb/uvc/uvc_ctrl.c
>> +++ b/drivers/media/usb/uvc/uvc_ctrl.c
>> @@ -1840,6 +1840,17 @@ static int __uvc_query_v4l2_ctrl(struct uvc_video_chain *chain,
>>  	if ((ctrl->info.flags & UVC_CTRL_FLAG_GET_MAX) &&
>>  	    (ctrl->info.flags & UVC_CTRL_FLAG_GET_MIN))
>>  		v4l2_ctrl->flags |= V4L2_CTRL_FLAG_HAS_WHICH_MIN_MAX;
> 
> A blank line here would be nice.
> 
>> +	if (ctrl->info.flags & UVC_CTRL_FLAG_AUTO_UPDATE) {
>> +		v4l2_ctrl->flags |= V4L2_CTRL_FLAG_VOLATILE;
>> +		/*
>> +		 * Writes to a volatile control are documented to be ignored
>> +		 * unless EXECUTE_ON_WRITE is also reported. The driver sends
>> +		 * every write of a writable control to the device, so report
>> +		 * the flag accordingly.
>> +		 */
>> +		if (ctrl->info.flags & UVC_CTRL_FLAG_SET_CUR)
>> +			v4l2_ctrl->flags |= V4L2_CTRL_FLAG_EXECUTE_ON_WRITE;
>> +	}
> 
> The V4L2 documentation also states
> 
>   Setting a new value for a volatile control will never trigger a
>   V4L2_EVENT_CTRL_CH_VALUE event.
> 
> This patch seems to break that as we unconditionally send
> V4L2_EVENT_CTRL_CH_VALUE events on control write for controls that don't
> have UVC_CTRL_FLAG_ASYNCHRONOUS set.
> 
>>  
>>  	if (mapping->master_id)
>>  		__uvc_find_control(ctrl->entity, mapping->master_id,
> 

So this is not correct. The best place to see how this should be done in this
driver is the kernel doc comment for v4l2_ctrl_auto_cluster() in include/media/v4l2-ctrls.h.

A driver that uses the control framework can mark a set of controls as a cluster where
the first control switches between automatic and manual handling, and the other controls
are only active if manual handling is selected. If automatic handling is selected, then
the INACTIVE flag is automatically set. In addition, if the 'set_volatile' flag is true
when v4l2_ctrl_auto_cluster is called, then the VOLATILE flag is also set when automatic
handling is selected. That flag is cleared when you switch to manual mode.

So when in manual mode these are all normal, non-volatile controls. When in automatic mode,
and if set_volatile is true, then all but the first control are marked as inactive and
volatile, so reading one of those controls will call g_volatile_ctrl. Setting a volatile
control is just ignored as expected, since it is meaningless.

So don't set V4L2_CTRL_FLAG_EXECUTE_ON_WRITE, instead you have to modify the flags on
the fly whenever you switch between manual and automatic mode.

I hope this helps!

Regards,

	Hans