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 15:19:00 -0700 |
| |
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 >>> >>>
| |