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 13:02:01 -0700 |
| |
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.
> >> >>>> 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.
| |