On Thu, 2013-08-08 at 14:19 +0530, Arun Raghavan wrote: > 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. Ok, please keep the behavior as it was in the patch. -- Tanu
