2013-09-13 04:00, Tanu Kaskinen skrev: > On Fri, 2013-09-13 at 13:20 +0530, Arun Raghavan wrote: >> On Fri, 2013-08-30 at 15:11 +0200, David Henningsson wrote: >>> On 08/30/2013 09:28 AM, Tanu Kaskinen wrote: >>>> On Fri, 2013-08-30 at 08:16 +0200, David Henningsson wrote: >>>>> On 08/29/2013 04:36 PM, Tanu Kaskinen wrote: >>>>>> Sometimes it would be nice to disable module-suspend-on-idle for >>>>>> specific devices. For me the use case is to keep a HDMI sink running >>>>>> all the time to avoid loss of audio when starting to play a stream to >>>>>> the device (the HDMI receiver eats a bit from the beginning of the >>>>>> stream when the device is opened). This is arguably a hacky solution >>>>>> to the problem, but on the other hand, I think it's very sensible to >>>>>> interpret negative timeout in the module-suspend-on-idle.timeout >>>>>> property as disabling the suspending altogher. This is also how the >>>>>> exit-idle-time configuration option behaves (negative value disables >>>>>> automatic exiting). >>>>> Didn't Arun have another solution for a similar problem? Like a sink >>>>> ALWAYS_RUNNING flag or something. >>>> Yes, he did. That patch was not merged, and I don't know if Arun plans >>>> to still work on it. >>>> >>>> Regardless of Arun's solution, I think it makes sense to support >>>> negative value for the module-suspend-on-idle.timeout property to be >>>> consistent with other timeout parameters. >>>> >>> Hmm, it looks like there already is a module-suspend-on-idle.timeout >>> property and you're just improving its functionality. Then I would >>> typically prefer this over a new sink flag. >>> >>> Even better if the module-suspend-on-idle.timeout was documented though... >>> >>> Arun, would this solve your problem as well? Would it be possible for >>> the two of you to synchronise your efforts? >> Tanu and I discussed this on IRC. I don't think we should use this >> property to do what I was trying to with ALWAYS_RUNNING - the module >> parameters is mechanism, and the always-running-ness is policy. Don't >> want to mix the two. >> >> That said, I've no objection to what the patch is trying to do, per se >> (i.e. extend/standardise the semantics of a device property that we >> already support). > OK, patch pushed. > > For the record, I agree with Arun in that this property shouldn't be > used in the hardware description files (UCM or alsa-mixer files). I > didn't quite understand Arun's explanation about mechanism vs. policy. > My reason for disliking the property is that it's specific to > module-suspend-on-idle, while "idle suspending not recommended" is a > hardware property that shouldn't be tied to any particular module. > I can see your point, but I also like to be pragmatic - if we now have this property tied to this module, and the module can achieve what Arun needs, I wouldn't add another way of doing the same thing, unless it was necessary. That just gives us additional code to maintain and test. Not that I'm so strong about it that I want to block Arun's patch if he really feels the need to add that flag in addition to this patch, it just seems superfluous to me. -- David Henningsson, Canonical Ltd. https://launchpad.net/~diwic
