On 27/06/2022 12:10, Nas Chung wrote:
> Hi, Hans.
>
> Thanks for the feedback.
>
> I have one question for g_volatile, please refer the below.
>
>> -----Original Message-----
>> From: Hans Verkuil <hverkuil@xxxxxxxxx>
>> Sent: Wednesday, June 22, 2022 7:16 PM
>> To: Nas Chung <nas.chung@xxxxxxxxxxxxxxx>; linux-media@xxxxxxxxxxxxxxx
>> Cc: Robert Beckett <bob.beckett@xxxxxxxxxxxxx>; Dafna Hirschfeld
>> <dafna.hirschfeld@xxxxxxxxxxxxx>
>> Subject: Re: [PATCH v8 3/6] staging: media: wave5: Add the v4l2 layer
>>
>> Hi Nas,
>>
>> Some review comments below:
>>
<snip>
>>> +static int wave5_vpu_dec_g_volatile_ctrl(struct v4l2_ctrl *ctrl)
>>> +{
>>> + struct vpu_instance *inst = wave5_ctrl_to_vpu_inst(ctrl);
>>> +
>>> + dev_dbg(inst->dev->dev, "name : %s\n", ctrl->name);
>>> +
>>> + switch (ctrl->id) {
>>> + case V4L2_CID_MIN_BUFFERS_FOR_CAPTURE:
>>> + if (inst->state != VPU_INST_STATE_NONE && inst->state !=
>> VPU_INST_STATE_OPEN)
>>> + ctrl->val = inst->min_dst_frame_buf_count;
>>
>> This isn't a volatile control, it's just a regular control. Whenever
>> inst->min_dst_frame_buf_count is changed by the driver it should also
>> update this control by calling v4l2_ctrl_s_ctrl().
>>
>> Volatile controls are for cases where the hardware is autonomously
>> changing a value (e.g. the gain when auto gain is enabled).
>>
>> But here the value is controlled by the driver, so it's not volatile.
>>
>
> v4l2_ctrl_s_ctrl() returns EINVAL error, when I change the value of V4L2_CID_MIN_BUFFERS_FOR_CAPTURE/V4L2_CID_MIN_BUFFERS_FOR_OUTPUT.
> Is it possible to change the value for READ_ONLY interface?
Yes. The vivid driver does the same thing for the read only integer control that it creates.
Could it be that you set it to an out-of-range value?
>
> Actually, min_dst_frame_buf_count value is updated by driver based on HW return value.
> So, That's why I use the g_volatile function.
It's not the same thing: you know when the value changes and you just update it in the control.
A true volatile control will take a snapshot from the hardware whenever userspace reads it.
E.g. hardware that automatically changes the gain will do so without informing userspace when
the gain value changes, so when userspace tries to read the current gain control value it always
has to ask the hardware what that value is. The VOLATILE flag means that the control framework
can't return the cached value of the control, it always has to ask the hardware.
Regards,
Hans
