Re: [RFC PATCH v11 6/9] media: tegra: Add Tegra210 Video input driver

From: Sowjanya Komatineni
Date: Thu Apr 30 2020 - 17:23:41 EST



On 4/30/20 12:53 PM, Sowjanya Komatineni wrote:

On 4/30/20 12:46 PM, Sowjanya Komatineni wrote:

On 4/30/20 12:33 PM, Dmitry Osipenko wrote:
30.04.2020 22:09, Sowjanya Komatineni ÐÐÑÐÑ:
On 4/30/20 11:18 AM, Sowjanya Komatineni wrote:
On 4/30/20 10:06 AM, Sowjanya Komatineni wrote:
On 4/30/20 9:29 AM, Sowjanya Komatineni wrote:
On 4/30/20 9:04 AM, Sowjanya Komatineni wrote:
On 4/30/20 7:13 AM, Dmitry Osipenko wrote:
30.04.2020 17:02, Dmitry Osipenko ÐÐÑÐÑ:
30.04.2020 16:56, Dmitry Osipenko ÐÐÑÐÑ:
30.04.2020 01:00, Sowjanya Komatineni ÐÐÑÐÑ:
+static int chan_capture_kthread_finish(void *data)
+{
+ÂÂÂ struct tegra_vi_channel *chan = data;
+ÂÂÂ struct tegra_channel_buffer *buf;
+
+ÂÂÂ set_freezable();
+
+ÂÂÂ while (1) {
+ÂÂÂÂÂÂÂ try_to_freeze();
I guess it won't be great to freeze in the middle of a capture
process, so:
ÂÂÂÂÂÂÂÂ if (list_empty(&chan->done))
ÂÂÂÂÂÂÂÂÂÂÂÂ try_to_freeze();
And here should be some locking protection in order not race with
the
chan_capture_kthread_start because kthread_finish could freeze
before
kthread_start.
Or maybe both start / finish threads should simply be allowed to
freeze
only when both capture and done lists are empty.

if (list_empty(&chan->capture) &&
ÂÂÂÂÂ list_empty(&chan->done))
ÂÂÂÂÂtry_to_freeze();
good to freeze when not in middle of the frame capture but why
should we not allow freeze in between captures?

Other drivers do allow freeze in between frame captures.

I guess we can freeze before dequeue for capture and in finish
thread we can freeze after capture done. This also don't need to
check for list_empty with freeze to allow between frame captures.

Also if we add check for both lists empty, freeze is not allowed as
long as streaming is going on and in case of continuous streaming
freeze will never happen.
To allow freeze b/w frames (but not in middle of a frame),

for capture_start thread, probably we can do unconditional
try_to_freeze()
Is it possible to use wait_event_freezable()?

https://www.kernel.org/doc/Documentation/power/freezing-of-tasks.txt

Will the wait_event_interruptible() be woken up when system freezes?

Based on wait_event_freezable implementation, looks like it similar to wait_event_interruptible + try_to_free() as it does freezable_schedule unlike schedule with wait_event_interruptible.

So using this for capture_start may be ok to allow freeze before start of frame. But can't use for capture_finish as this is same as wait_event_interruptible followed by unconditional try_to_freeze.


for capture_finish thread, at end of capture done we can do
try_to_freeze() only when done list is empty
This doesn't prevent situation where the done-list is empty and the
"finish" thread freezes, in the same time the "start" thread issues new
capture and then freezes too.

1. "start" thread issues capture

2. "finish" thread wakes and waits for the capture to complete

3. "start" thread begins another capture, waits for FRAME_START

4. system freezing activates

5. "finish" thread completes the capture and freezes because done-list
is empty

6. "start" thread gets FRAME_START, issues another capture and freezes

This will not happen as we allow double buffering done list will not be empty till stream stop happens

There will always be 1 outstanding frame in done list

Correction, there will always be 1 outstanding buffer except beginning during beginning of stream.

Except during beginning frames, done list will not be empty for all subsequent streaming process

or probably we should add pending buffers b/w 2 threads w.r.t single shot issues and allow freeze only when no pending frame to cover any corner case b/w done list and capture list.


My understanding is buffer updates/release should not happen after
frozen state. So we should let frame capture of outstanding buffer to
finish before freezing in capture_finish thread.

But for capture_start thread we can unconditionally freeze before
dequeuing next buffer for capture.

With this when both threads are in frozen state and no buffer
updates/captures will happen after frozen state.

I think its not required to finish streaming of all frames completely to
let threads to enter frozen state as streaming can be continuous as well.
Yes, only freezing in the middle of IO should be avoided.

https://lwn.net/Articles/705269/

Hi Dmitry,

Will update in v12 to not allow freeze in middle of a frame capture.

Can you please confirm on above if you agree to allow freeze to
happen in b/w frame captures?

Also as most feedback has been received from you by now, appreciate
if you can provide all in this v11 if you have anything else so we
will not have any new changes after v12.
I'll take another look tomorrow / during weekend and let you know.