Re: [PATCH v14 7/7] media: nuvoton: Add driver for NPCM video capture and encode engine

Hi Hans,

Thanks for the reply.

quoted

When userspace dequeues the 1st buffer (video->list[0]), it needs to
know the count of HEXTILE rectangles in the buffer,
so after dequeuing the buffer it will call this control to get the
rect count (video->rect[0]). And when a buffer is dequeued,
npcm_video_buf_finish() will be called, in which the buffer index (in
this example, buffer index = 0) will be stored to video->vb_index.
Then when userspace calls this control, npcm_video_get_volatile_ctrl()
will return the rect count of vb_index = 0.
In this way, I think userspace is always reading the correct control's
value even if userspace is slow.
Does it make sense to you or is there anything I missed?

Ah, I don't think I have ever seen anyone use buf_finish in that way!

Very inventive, and perfectly legal. Actually a very nice idea!

So, with that in mind there are still some things that need to change.

First of all, you can drop the 'VOLATILE' flag from the control, instead
just call v4l2_ctrl_s_ctrl() from buf_finish() to update the control.
And in stop_streaming the control value should probably be set to 0.

The use of volatile for a control is a last resort, and in this case it
is not volatile at all.

Secondly, this behavior has to be documented: in buf_finish add a comment
along the lines of: "This callback is called when the buffer is dequeued,
so update this control with the number of rectangles."

And where the control is defined, refer to buf_finish to explain where it
is set.

Finally the user-facing documentation has to be updated (npcm-video.rst)
to explain this behavior.

OK. Will drop the VOLATILE flag and update comment/document in the next version.

Regards,
Marvin