On 08/10/2014 07:05 PM, Mauro Carvalho Chehab wrote: > Hi Hans, > > Em Sun, 10 Aug 2014 17:16:17 +0200 > Hans Verkuil <hverkuil@xxxxxxxxx> escreveu: > >> Hi Mauro, >> >> The following are just some general remarks regarding PM and au0828, it's >> not specific to this patch series. I'm just brainstorming here... >> >> It's unfortunate that the au0828 isn't using vb2 yet. I would be interested in >> seeing what can be done in vb2 to help implement suspend/resume. Basically vb2 >> has already most (all?) of the information it needs to handle this. Ideally all >> you need to do in the driver is to call vb2_suspend or vb2_resume and vb2 will >> take care of the rest, calling start/stop_streaming as needed. Some work would >> have to be done there to ensure buffers are queued/dequeued to the right queues >> and in the right state. > > I guess one of the problems with any core-provided method is to ensure that > the tuner is initialized and locked before it, in order to avoid filling > buffers from the wrong channel. > > Btw, this is one unsolved problem that we face with PM on media: we > need to assure that resume will follow a precise init order: > - All core subsystems are initialized before everything else; > - The firmwares are loaded; > - The I2C gates are in the right state; > - The tuner is set; > - The gpio's are properly configured; > - The analog and/or the digital demodulator are properly > configured; > > Only after that, the driver (and VB2) can be resumed. Those items are certainly out-of-scope of vb2 and are device specific. > A proper PM support will require lots of changes at the media core, and > likely on other subsystems. We (I, Shuah and some people I'm hiring) > are looking into those issues, but a proper fix with proper core support > will take some time. > >> So vb2 would handle all the DMA/streaming aspects of suspend/resume, thus >> simplifying the driver. > > The changes will be minimal. Just one patch would be affected: > https://patchwork.linuxtv.org/patch/25277/ > > IMHO, for USB drivers, the best strategy for suspend/resume is the one > taken on au0828, e. g. at suspend: > - stop DMA; > - cancel the pending URB; > - stop any pending kthread; > - call vb2_discard_done() to discard any pending buffers > (I think VB1 doesn't have anything similar); With proper vb2 support stop DMA, cancel pending USB and vb2_discard_done would all be done by vb2. The first two would be done in the stop_streaming vb2 op. > > And, at resume, restart them. > > We could provide a core support to cancel the pending URBs, but most > of the stuff are driver-specific, because the core doesn't have > any code to handle USB streams on a generic way (well, gspca has, > but it doesn't cover the DVB specifics). > > I started writing a URB handling abstraction for the tm6000 driver > that I wanted to add at the core, but I didn't finish making it > generic enough. Moving it to core and porting the existing drivers > to use it would take a lot of time and effort. Not sure if it is > worth nowadays. > > What I'm trying to say is that most of the issues related to > suspend/resume aren't at the streaming engine (being VB1, VB2 > or a driver-specific one). Once we fix the main issues subsystem-wide, > then we can have a clearer view if are there anything we could do at > VB2 level. > >> Is it perhaps an idea to convert au0828 to vb2 in order to pursue this further? >> >> Besides, converting to vb2 tends to get rid of a substantial amount of code >> which makes it much easier to work with. > > I don't think that converting au0828 to vb2 would help to improve > the suspend issues. Ok, should do it anyway at long term, as we want > one day to get rid of VB1, but we should take some care with this driver. > It has workarounds for several hardware bugs that cause the stream to > stop working under not well known situations. There are even two > threads there to detect and apply such workarounds when stream > suddenly stops without a (known) reason. These types of weird errors are exactly why I would recommend to port to vb2. The sequence of events is very clear and systematic in vb2, whereas it will drive you bonkers if you try to understand the flow in vb1. I do not trust *any* driver that uses vb1. There is no way drivers can cover all corner cases since vb1 is one of the craziest pieces of code I've ever seen. Especially since it doesn't look crazy at first glance, that's what makes it such a nasty framework. I've ported quite a few drivers by now to vb2 and in all cases things that used to be flaky (or just plain broken) suddenly started working. This may or may not apply to au0828, but it will certainly make the code a lot easier to understand. Particularly crucial steps such as when the streaming starts and stops is very well defined in vb2. Ditto for resource handling (who 'owns' the stream) which is handled by vb2 as well. Just my 5 cents, of course. Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
