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 16:14:42 -0700 |
| |
On 4/30/20 3:19 PM, Sowjanya Komatineni wrote: > > On 4/30/20 3:16 PM, Sowjanya Komatineni wrote: >> >> On 4/30/20 2:53 PM, Sowjanya Komatineni wrote: >>> >>> On 4/30/20 2:37 PM, Sowjanya Komatineni wrote: >>>> >>>> On 4/30/20 2:26 PM, Sowjanya Komatineni wrote: >>>>> >>>>> On 4/30/20 2:17 PM, Dmitry Osipenko wrote: >>>>>> 30.04.2020 23:02, Sowjanya Komatineni пишет: >>>>>>> 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 >>>>>>> Also to be clear, hardware sees next frame start event prior to >>>>>>> previous >>>>>>> frame mw_ack event as mw_ack event happens after frame end. So once >>>>>>> initial buffer got queued to done list to finish processes, while >>>>>>> waiting for mw_ack next frame start happens and pushes next >>>>>>> buffer to >>>>>>> done list. >>>>>> What about this variant: >>>>>> >>>>>> 1. "start" thread wakes up to start capture >>>>>> >>>>>> 2. system freezing activates >>>>>> >>>>>> 3. "finish" thread wakes up and freezes >>>>> >>>>> finish thread will wake up only when done list is not >>>>> empty/kthread_stop/wake even from capture start thread. >>>>> >>>>> Also when I said will allow try_to_free when done list is empty I >>>>> meant to have this at end of capture_done() in finish thread >>>>> >>>>>> >>>>>> 4. "start" thread issues capture and freezes >>>>>> >>>>>> And again, I assume that system's freezing should wake >>>>>> wait_event_interruptible(), otherwise it won't be possible to freeze >>>>>> idling threads at all and freezing should fail (IIUC). >>>>> >>>>> Based on kernel doc on freezing, looks like when we mark thread as >>>>> freezable, freeze state happens when we explicitly call >>>>> try_to_freeze. >>>>> >>>>> I don't think its other way where freeze causes >>>>> wait_event_interruptible to wake up. >>> >>> Based on my understanding when we mark thread as freezable, >>> >>> with wait_event_freezable() - after wait event, it invokes >>> try_to_freeze(). So frozen state enters unconditionally with this. >>> >>> with wait_event_interruptible - we do try_to_freeze when its safe to >>> enter frozen state. >>> >>> https://www.kernel.org/doc/Documentation/power/freezing-of-tasks.txt >>> >> Sorry correction. When system tries to freeze tasks looks like it >> will sending signal to thread and wake up happens when signal is sent >> to thread and freezable thread should invoke try_to_free when its >> safe to free > > freeze_task() sends fake signal > > https://elixir.bootlin.com/linux/v5.7-rc2/source/kernel/freezer.c#L115 > >> >>> >>>>> >>>>>> And in this case synchronization between start/finish threads >>>>>> should be >>>>>> needed in regards to freezing. >>>>> >>>>> Was thinking to have counter to track outstanding frame w.r.t >>>>> single shot issue b/w start and finish and allow to freeze only >>>>> when no outstanding frames in process. >>>>> >>>>> This will make sure freeze will not happen when any buffers are in >>>>> progress >>>>> >>>>>> Note that this could be a wrong assumption, I'm not closely familiar >>>>>> with how freezer works. >>>> >>>> kthread_start can unconditionally allow try_to_freeze before start >>>> of frame capture >>>> >>>> We can compute captures inflight w.r.t single shot issued during >>>> capture start and finished frames by kthread_finish and allow >>>> kthread_finish to freeze only when captures inflight is 0. >>>> >>>> This allows freeze to happen b/w frames but not in middle of frame > will have caps inflight check in v12 to allow freeze finish thread > only when no captures are in progress
try_to_freeze() returns thread frozen state and looks like we can use this in kthread finish to allow finish thread to freeze only when kthread_start is already frozen and no buffers in progress/initiated for capture.
| |