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

Sowjanya Komatineni
2020-05-03T00:04:05Z

On 5/2/20 3:46 PM, Sowjanya Komatineni wrote:
>
> On 5/2/20 1:48 PM, Dmitry Osipenko wrote:
>> 02.05.2020 19:55, Sowjanya Komatineni пишет:
>>> Also stop stream ioctl request happens during suspend where both
>>> threads
>>> will be stopped properly. done thread stop happens only after finishing
>>> all outstanding buffers.
>> Do you mean that V4L core takes care of stopping the streami on suspend
>> and re-starting it on resume from suspend?
>>
>>> Stop stream request happens from streaming applications so even without
>>> driver suspend/resume implementation currently, streaming will be
>>> stopped prior to system  suspend where both threads will be stopped
>>> properly (after finishing out standing buffers) and will be resumed by
>>> application on system resume
>> All userspace is frozen on suspend. System suspension is transparent for
>> userspace applications. I'm not sure what you're meaning here.
>>
>>> Also tested suspending while streaming with this unconditional
>>> freeze, I
>>> don't see any issue as application stops stream where v4l_streamoff
>>> gets
>>> executed during suspend and on resume streaming starts where
>>> v4l_streamon happens.
>>>
>>> So, I don't see any issue with existing implementation of unconditional
>>> freeze.
>> I don't understand why freezing is needed at all if V4L core takes care
>> of stopping the stream on suspend, what is the point? If there is no
>> real point, then let's make threads non-freezable and done with that.
>
> video device fops unlocked_ioctl is set to video_ioctl2() in vi driver.
>
> video device fops unlocked_ioctl gets executed with stream off cmd
> during suspend and stream on cmd during resume which eventually calls
> v4l_streamoff and v4l_streamon during system suspend/resume.
>
> My understanding to have freezable threads is during system suspend
> user space applications are frozen prior to kernel freeze and during
> suspend when opened video character device node gets closed these
> ioctl gets invoked and stream off during suspend and stream on during
> resume happens. So probably we still need to use freezable threads to
> sync with user space application when frozen before really entering
> suspend.
>
> Will wait for Thierry/Hans comment to correct if my above
> understanding is wrong and help clarify if we need freezable threads
> at all in this case...
>
> Note: I see other drivers using freezable threads for capture drivers.

I see only couple of media drivers using freezable threads.

Also, referring to below article probably we don't need freezable threads.

https://lwn.net/Articles/662703/

Probably we can then remove thread as freezable...

>
>
> Assuming we use freezable threads, I was saying we don't need
> conditional try_to_freeze() like you pointed because even if finish
> thread freeze happens prior to frame capture initiated by start
> thread, vi hardware will still continue to update this single ongoing
> buffer and will finish max within 200ms and actually there is no
> direct processing of this done by finish thread itself except that it
> returns buffers back when done and in this case it returns back when
> unfreeze/wake up happens.
>
> So, I don't see any issue of unconditional try_to_freeze() even with
> freezable threads.
>
> Thanks
>
> Sowjanya
>
>