Re: [PATCH] media: vimc: fla: Add virtual flash subdevice
On 9/10/19 1:00 AM, Lucas Magalhães wrote:
> Hi Hans,
> Thanks for the review. I fixed most of the issues you found. Just have
> the question below.
>
> On Mon, Sep 2, 2019 at 9:04 AM Hans Verkuil wrote:
>>
>>> +
>>> +int vimc_fla_add(struct vimc_device *vimc, struct vimc_ent_config *vcfg)
>>> +{
>>> + struct v4l2_device *v4l2_dev = &vimc->v4l2_dev;
>>> + struct vimc_fla_device *vfla;
>>> + int ret;
>>> +
>>> + /* Allocate the vfla struct */
>>> + vfla = kzalloc(sizeof(*vfla), GFP_KERNEL);
>>> + if (!vfla)
>>> + return -ENOMEM;
>>> +
>>> + v4l2_ctrl_handler_init(&vfla->hdl, 4);
>>> +
>>> + v4l2_ctrl_new_std_menu(&vfla->hdl, &vimc_fla_ctrl_ops,
>>> +V4L2_CID_FLASH_LED_MODE,
>>> +V4L2_FLASH_LED_MODE_TORCH, ~0x7,
>>> +V4L2_FLASH_LED_MODE_NONE);
>>> + v4l2_ctrl_new_std_menu(&vfla->hdl, &vimc_fla_ctrl_ops,
>>> +V4L2_CID_FLASH_STROBE_SOURCE, 0x1, ~0x3,
>>> +V4L2_FLASH_STROBE_SOURCE_SOFTWARE);
>>> + v4l2_ctrl_new_std(&vfla->hdl, &vimc_fla_ctrl_ops,
>>> + V4L2_CID_FLASH_STROBE, 0, 0, 0, 0);
>>> + v4l2_ctrl_new_std(&vfla->hdl, &vimc_fla_ctrl_ops,
>>> + V4L2_CID_FLASH_STROBE_STOP, 0, 0, 0, 0);
>>> + v4l2_ctrl_new_std(&vfla->hdl, &vimc_fla_ctrl_ops,
>>> + V4L2_CID_FLASH_TIMEOUT, 1, 10, 1, 10);
>>> + v4l2_ctrl_new_std(&vfla->hdl, &vimc_fla_ctrl_ops,
>>> + V4L2_CID_FLASH_TORCH_INTENSITY, 0, 255, 1, 255);
>>> + v4l2_ctrl_new_std(&vfla->hdl, &vimc_fla_ctrl_ops,
>>> + V4L2_CID_FLASH_INTENSITY, 0, 255, 1, 255);
>>> + v4l2_ctrl_new_std(&vfla->hdl, &vimc_fla_ctrl_ops,
>>> + V4L2_CID_FLASH_INDICATOR_INTENSITY, 0, 255, 1, 255);
>>> + v4l2_ctrl_new_std(&vfla->hdl, &vimc_fla_ctrl_ops,
>>> + V4L2_CID_FLASH_STROBE_STATUS, 0, 0, 0, 0);
>>
>> It would be nice if this would actually reflect the actual strobe status.
>>
> Regarding the strobe status I was reading the code and find out that
> V4L2_CID_FLASH_STROBE_STATUS is a V4L2_CTRL_FLAG_READ_ONLY
> but it's not a V4L2_CTRL_FLAG_VOLATILE. I found this intriguing. How an
> I suppose to get it if its not volatile? As I understood it changes over time
> if the strobe starts and the timeout expire, isn't it? Shouldn't it be
> volatile
> if so?
A non-volatile read-only control is set deterministically by the the driver.
So the driver calls v4l2_ctrl_s_ctrl() to change the controls value.
A volatile read-only control is one where the value is read from a hardware
register that is continuously changing. E.g. if autogain is on, then the gain
register in a device contains the currently calculated gain, but that might be
changed the next time the register is read.
Regards,
Hans
>
> I've already made a simple implementation were V4L2_CID_FLASH_STROBE_STATUS
> returns after calling V4L2_CID_FLASH_STROBE and becomes false after the
> timeout
> time passes.
>
> Thanks!
>
Re: [PATCH] media: vimc: fla: Add virtual flash subdevice
Hi Hans,
Thanks for the review. I fixed most of the issues you found. Just have
the question below.
On Mon, Sep 2, 2019 at 9:04 AM Hans Verkuil wrote:
>
> > +
> > +int vimc_fla_add(struct vimc_device *vimc, struct vimc_ent_config *vcfg)
> > +{
> > + struct v4l2_device *v4l2_dev = &vimc->v4l2_dev;
> > + struct vimc_fla_device *vfla;
> > + int ret;
> > +
> > + /* Allocate the vfla struct */
> > + vfla = kzalloc(sizeof(*vfla), GFP_KERNEL);
> > + if (!vfla)
> > + return -ENOMEM;
> > +
> > + v4l2_ctrl_handler_init(&vfla->hdl, 4);
> > +
> > + v4l2_ctrl_new_std_menu(&vfla->hdl, &vimc_fla_ctrl_ops,
> > +V4L2_CID_FLASH_LED_MODE,
> > +V4L2_FLASH_LED_MODE_TORCH, ~0x7,
> > +V4L2_FLASH_LED_MODE_NONE);
> > + v4l2_ctrl_new_std_menu(&vfla->hdl, &vimc_fla_ctrl_ops,
> > +V4L2_CID_FLASH_STROBE_SOURCE, 0x1, ~0x3,
> > +V4L2_FLASH_STROBE_SOURCE_SOFTWARE);
> > + v4l2_ctrl_new_std(&vfla->hdl, &vimc_fla_ctrl_ops,
> > + V4L2_CID_FLASH_STROBE, 0, 0, 0, 0);
> > + v4l2_ctrl_new_std(&vfla->hdl, &vimc_fla_ctrl_ops,
> > + V4L2_CID_FLASH_STROBE_STOP, 0, 0, 0, 0);
> > + v4l2_ctrl_new_std(&vfla->hdl, &vimc_fla_ctrl_ops,
> > + V4L2_CID_FLASH_TIMEOUT, 1, 10, 1, 10);
> > + v4l2_ctrl_new_std(&vfla->hdl, &vimc_fla_ctrl_ops,
> > + V4L2_CID_FLASH_TORCH_INTENSITY, 0, 255, 1, 255);
> > + v4l2_ctrl_new_std(&vfla->hdl, &vimc_fla_ctrl_ops,
> > + V4L2_CID_FLASH_INTENSITY, 0, 255, 1, 255);
> > + v4l2_ctrl_new_std(&vfla->hdl, &vimc_fla_ctrl_ops,
> > + V4L2_CID_FLASH_INDICATOR_INTENSITY, 0, 255, 1, 255);
> > + v4l2_ctrl_new_std(&vfla->hdl, &vimc_fla_ctrl_ops,
> > + V4L2_CID_FLASH_STROBE_STATUS, 0, 0, 0, 0);
>
> It would be nice if this would actually reflect the actual strobe status.
>
Regarding the strobe status I was reading the code and find out that
V4L2_CID_FLASH_STROBE_STATUS is a V4L2_CTRL_FLAG_READ_ONLY
but it's not a V4L2_CTRL_FLAG_VOLATILE. I found this intriguing. How an
I suppose to get it if its not volatile? As I understood it changes over time
if the strobe starts and the timeout expire, isn't it? Shouldn't it be volatile
if so?
I've already made a simple implementation were V4L2_CID_FLASH_STROBE_STATUS
returns after calling V4L2_CID_FLASH_STROBE and becomes false after the timeout
time passes.
Thanks!
Re: [PATCH] media: vimc: fla: Add virtual flash subdevice
Hi Lucas, Eduardo, Thank you for the patch! Some comments below: On 9/1/19 11:11 PM, Lucas A. M. Magalhães wrote: > From: Lucas A. M. Magalhaes > > Add a virtual subdevice to simulate the flash control API. > Those are the supported controls: > v4l2-ctl -d /dev/v4l-subdev6 -L > Flash Controls > >led_mode 0x009c0901 (menu) : min=0 max=2 default=0 > value=0 > 0: Off > 1: Flash > 2: Torch > strobe_source 0x009c0902 (menu) : min=0 max=1 default=0 > value=0 > 0: Software > 1: External > strobe 0x009c0903 (button) : flags=write-only, > execute-on-write > stop_strobe 0x009c0904 (button) : flags=write-only, > execute-on-write > strobe_status 0x009c0905 (bool) : default=0 value=0 > flags=read-only > strobe_timeout 0x009c0906 (int): min=1 max=10 step=1 > default=10 value=10 >intensity_flash_mode 0x009c0907 (int): min=0 max=255 step=1 > default=255 value=255 >intensity_torch_mode 0x009c0908 (int): min=0 max=255 step=1 > default=255 value=255 > intensity_indicator 0x009c0909 (int): min=0 max=255 step=1 > default=255 value=255 > faults 0x009c090a (bitmask): max=0x0002 > default=0x value=0x > > Co-authored-by: Eduardo Barretto > Signed-off-by: Eduardo Barretto > Signed-off-by: Lucas A. M. Magalhães > > --- > Hi, > > This patch depends on the patch series > "Collapse vimc into single monolithic driver" version 3. > > I tested it using the v4l2-ctl and the v4l2-compliance. Apparently the > compliance > doesn't test any of the standard flash controls. However I got this error: > > test VIDIOC_G/S/TRY_EXT_CTRLS: OK > fail: v4l2-test-controls.cpp(830): subscribe event for control 'Flash > Controls' failed > > Is it really mandatory to implement the event mechanism? > > Here is the full output of the v4l2-compliance > > root@(none):/# /usr/local/bin/v4l2-compliance -d /dev/v4l-subdev6 > v4l2-compliance SHA: b393a5408383b7341883857dfda78537f2f85ef6, 64 bits > > Compliance test for vimc device /dev/v4l-subdev6: > > Media Driver Info: > Driver name : vimc > Model: VIMC MDEV > Serial : > Bus info : platform:vimc > Media version: 5.3.0 > Hardware revision: 0x (0) > Driver version : 5.3.0 > Interface Info: > ID : 0x0339 > Type : V4L Sub-Device > Entity Info: > ID : 0x001c (28) > Name : Flash Controller > Function : Flash Controller > > Required ioctls: > test MC information (see 'Media Driver Info' above): OK > > Allow for multiple opens: > test second /dev/v4l-subdev6 open: OK > test for unlimited opens: OK > > Debug ioctls: > [ 342.293254] Flash Controller: = START STATUS > > [ 342.293945] Flash Controller: == END STATUS > == > test VIDIOC_LOG_STATUS: OK (Not Supported) > > Input ioctls: > test VIDIOC_G/S_TUNER/ENUM_FREQ_BANDS: OK (Not Supported) > test VIDIOC_G/S_FREQUENCY: OK (Not Supported) > test VIDIOC_S_HW_FREQ_SEEK: OK (Not Supported) > test VIDIOC_ENUMAUDIO: OK (Not Supported) > test VIDIOC_G/S/ENUMINPUT: OK (Not Supported) > test VIDIOC_G/S_AUDIO: OK (Not Supported) > Inputs: 0 Audio Inputs: 0 Tuners: 0 > > Output ioctls: > test VIDIOC_G/S_MODULATOR: OK (Not Supported) > test VIDIOC_G/S_FREQUENCY: OK (Not Supported) > test VIDIOC_ENUMAUDOUT: OK (Not Supported) > test VIDIOC_G/S/ENUMOUTPUT: OK (Not Supported) > test VIDIOC_G/S_AUDOUT: OK (Not Supported) > Outputs: 0 Audio Outputs: 0 Modulators: 0 > > Input/Output configuration ioctls: > test VIDIOC_ENUM/G/S/QUERY_STD: OK (Not Supported) > test VIDIOC_ENUM/G/S/QUERY_DV_TIMINGS: OK (Not Supported) > test VIDIOC_DV_TIMINGS_CAP: OK (Not Supported) > test VIDIOC_G/S_EDID: OK (Not Supported) > > Control ioctls: > test VIDIOC_QUERY_EXT_CTRL/QUERYMENU: OK > test VIDIOC_QUERYCTRL: OK > test VIDIOC_G/S_CTRL: OK > test VIDIOC_G/S/TRY_EXT_CTRLS: OK > fail: v4l2-test-controls.cpp(830): subscribe event for > control 'Flash Controls' failed > test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: FAIL > test VIDIOC_G/S_JPEGCOMP: OK (Not Supported) > > ontrols: 11 Private Controls: 0 > > Format ioctls: > test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK (Not Supported) > test VIDIOC_G/S_PARM: OK (Not Supported) > test VIDIOC_G_FBUF: OK (
[PATCH] media: vimc: fla: Add virtual flash subdevice
From: Lucas A. M. Magalhaes Add a virtual subdevice to simulate the flash control API. Those are the supported controls: v4l2-ctl -d /dev/v4l-subdev6 -L Flash Controls led_mode 0x009c0901 (menu) : min=0 max=2 default=0 value=0 0: Off 1: Flash 2: Torch strobe_source 0x009c0902 (menu) : min=0 max=1 default=0 value=0 0: Software 1: External strobe 0x009c0903 (button) : flags=write-only, execute-on-write stop_strobe 0x009c0904 (button) : flags=write-only, execute-on-write strobe_status 0x009c0905 (bool) : default=0 value=0 flags=read-only strobe_timeout 0x009c0906 (int): min=1 max=10 step=1 default=10 value=10 intensity_flash_mode 0x009c0907 (int): min=0 max=255 step=1 default=255 value=255 intensity_torch_mode 0x009c0908 (int): min=0 max=255 step=1 default=255 value=255 intensity_indicator 0x009c0909 (int): min=0 max=255 step=1 default=255 value=255 faults 0x009c090a (bitmask): max=0x0002 default=0x value=0x Co-authored-by: Eduardo Barretto Signed-off-by: Eduardo Barretto Signed-off-by: Lucas A. M. Magalhães --- Hi, This patch depends on the patch series "Collapse vimc into single monolithic driver" version 3. I tested it using the v4l2-ctl and the v4l2-compliance. Apparently the compliance doesn't test any of the standard flash controls. However I got this error: test VIDIOC_G/S/TRY_EXT_CTRLS: OK fail: v4l2-test-controls.cpp(830): subscribe event for control 'Flash Controls' failed Is it really mandatory to implement the event mechanism? Here is the full output of the v4l2-compliance root@(none):/# /usr/local/bin/v4l2-compliance -d /dev/v4l-subdev6 v4l2-compliance SHA: b393a5408383b7341883857dfda78537f2f85ef6, 64 bits Compliance test for vimc device /dev/v4l-subdev6: Media Driver Info: Driver name : vimc Model: VIMC MDEV Serial : Bus info : platform:vimc Media version: 5.3.0 Hardware revision: 0x (0) Driver version : 5.3.0 Interface Info: ID : 0x0339 Type : V4L Sub-Device Entity Info: ID : 0x001c (28) Name : Flash Controller Function : Flash Controller Required ioctls: test MC information (see 'Media Driver Info' above): OK Allow for multiple opens: test second /dev/v4l-subdev6 open: OK test for unlimited opens: OK Debug ioctls: [ 342.293254] Flash Controller: = START STATUS [ 342.293945] Flash Controller: == END STATUS == test VIDIOC_LOG_STATUS: OK (Not Supported) Input ioctls: test VIDIOC_G/S_TUNER/ENUM_FREQ_BANDS: OK (Not Supported) test VIDIOC_G/S_FREQUENCY: OK (Not Supported) test VIDIOC_S_HW_FREQ_SEEK: OK (Not Supported) test VIDIOC_ENUMAUDIO: OK (Not Supported) test VIDIOC_G/S/ENUMINPUT: OK (Not Supported) test VIDIOC_G/S_AUDIO: OK (Not Supported) Inputs: 0 Audio Inputs: 0 Tuners: 0 Output ioctls: test VIDIOC_G/S_MODULATOR: OK (Not Supported) test VIDIOC_G/S_FREQUENCY: OK (Not Supported) test VIDIOC_ENUMAUDOUT: OK (Not Supported) test VIDIOC_G/S/ENUMOUTPUT: OK (Not Supported) test VIDIOC_G/S_AUDOUT: OK (Not Supported) Outputs: 0 Audio Outputs: 0 Modulators: 0 Input/Output configuration ioctls: test VIDIOC_ENUM/G/S/QUERY_STD: OK (Not Supported) test VIDIOC_ENUM/G/S/QUERY_DV_TIMINGS: OK (Not Supported) test VIDIOC_DV_TIMINGS_CAP: OK (Not Supported) test VIDIOC_G/S_EDID: OK (Not Supported) Control ioctls: test VIDIOC_QUERY_EXT_CTRL/QUERYMENU: OK test VIDIOC_QUERYCTRL: OK test VIDIOC_G/S_CTRL: OK test VIDIOC_G/S/TRY_EXT_CTRLS: OK fail: v4l2-test-controls.cpp(830): subscribe event for control 'Flash Controls' failed test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: FAIL test VIDIOC_G/S_JPEGCOMP: OK (Not Supported) ontrols: 11 Private Controls: 0 Format ioctls: test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK (Not Supported) test VIDIOC_G/S_PARM: OK (Not Supported) test VIDIOC_G_FBUF: OK (Not Supported) test VIDIOC_G_FMT: OK (Not Supported) test VIDIOC_TRY_FMT: OK (Not Supported) test VIDIOC_S_FMT: OK (Not Supported) test VIDIOC_G_SLICED_VBI_CAP: OK (Not Supported) test Cropping: OK (Not Supported) test Composing: OK (Not Supported) test Scaling: OK (Not Supported) Codec ioctls: test VIDIOC_
