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

[Sowjanya Komatineni <skomatineni@nvidia.com>]

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
>
>