Messages in this thread | | | Subject | Re: [RFC PATCH v11 6/9] media: tegra: Add Tegra210 Video input driver | From | Sowjanya Komatineni <> | Date | Thu, 30 Apr 2020 12:46:13 -0700 |
| |
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
>> 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.
| |