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

[Sowjanya Komatineni]

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