On Thu, 2013-08-08 at 10:30 +0300, Tanu Kaskinen wrote: > On Thu, 2013-08-08 at 12:44 +0530, Arun Raghavan wrote: [...] > > > Some policy module asked the sink to suspend with the IDLE cause, and > > > that operation was not allowed, so I think it makes sense to tell the > > > policy module that its request was denied. > > > > I guess we can signal an error. From the core's perspective, we're not > > really going to see anything dealing with the error meaningfully (other > > than a debug print). > > > > > > As for chained suspend causes, the intention of this patch was to only > > > > ignore idle suspend. If something else wants to force a suspend, I think > > > > it should be honoured. That is, the check should be filter out the IDLE > > > > cause if cause is IDLE+<something else>. > > > > > > I think we shouldn't prepare for IDLE+<something else>, because we don't > > > need to and it would bring some complications as I explained in the > > > previous mail. > > > > Is the complication the decision about what to do when there are two > > causes? I think it makes sense to mask out IDLE if it's there and > > continue in this case but maybe you're thinking of some case that I'm > > not. > > I think it would be odd to fail if the cause is IDLE, but silently > ignore the IDLE part if it's IDLE+FOO. I'm not saying that completely > failing IDLE+FOO would be any better alternative - both alternatives > seem bad to me, so I'd like to avoid choosing by forbidding multi-cause > suspending altogether. The choice is messy only because we're looking at this differently - I'm looking at IDLE cause as something the core can ignore whereas you'd like it to either be honoured or register a failure. With my approach, we have a meaningful way to say that ... pa_sink_suspend(sink, true, PA_SUSPEND_IDLE | PA_SUSPEND_USER); ... is the same as ... pa_sink_suspend(sink, true, PA_SUSPEND_IDLE); pa_sink_suspend(sink, true, PA_SUSPEND_USER); Nevertheless, if you're not convinced, I'm fine with commenting the quirkiness w.r.t. chaining IDLE with other causes in this case and leaving the behaviour as is in the patch. -- Arun
