Hi Tanu, On Tue, Nov 19, 2013 at 9:37 AM, Tanu Kaskinen <tanu.kaskinen at linux.intel.com> wrote: > On Mon, 2013-11-18 at 16:51 +0200, Luiz Augusto von Dentz wrote: >> Hi Tanu, >> >> On Mon, Nov 18, 2013 at 9:25 AM, Tanu Kaskinen >> <tanu.kaskinen at linux.intel.com> wrote: >> > On Sun, 2013-11-17 at 17:53 +0200, Luiz Augusto von Dentz wrote: >> >> Hi Colin, Tanu, >> >> >> >> On Sun, Nov 17, 2013 at 3:17 PM, Colin Guthrie <gmane at colin.guthr.ie> wrote: >> >> > 'Twas brillig, and Tanu Kaskinen at 16/11/13 08:51 did gyre and gimble: >> >> >> On Fri, 2013-11-15 at 09:31 +0100, Colin Guthrie wrote: >> >> >> I haven't concentrated on the patches enough to even know what >> >> >> problem you're solving - is it just that the assertion makes you >> >> >> nervous, or is there some other benefit. >> >> > >> >> > I'm not solving any specific problem per-se. I'm just looking at several >> >> > bugs where incorrect profiles appear to be selected by default on first >> >> > boot where HDMI is picked over regular analog output (for those alsa >> >> > cards which share a single card for both HDMI and Analog output rather >> >> > than having it presented as separate cards) despite HDMI having a lower >> >> > priority. This lead me to look at this code and got me thinking - this >> >> > is the only reason for the patch. >> >> > >> >> > A further complication to my current bugs is that sometimes Headphone >> >> > *ports* are picked by default on first boot even when they are >> >> > unavailable and again are not the highest priority port. It seems like >> >> > the same problem but in a different, but similar, part of the code. This >> >> > is why the second set of patches applied the same logical fixes to the >> >> > Sink/Source code that Luiz added to the Card profiles. I think it makes >> >> > sense to keep these bits of code approximately in sync regardless of the >> >> > outcome of this discussion - the same logic should apply. >> >> >> >> I can try and apply the same logic to Sink/Source, it just that >> >> overall this kind of policy should be in one place or then remove it >> >> altogether and make sure the list is always ordered by their module so >> >> the core should just pick the first one, the problem we were trying to >> >> solve was that there is no hook for priority changes thus they are >> >> considered static which is probably why we are relying on >> >> pa_card_set_profile_available + >> >> PA_CORE_HOOK_CARD_PROFILE_AVAILABLE_CHANGED to indicate the profiles >> >> changes. >> >> >> >> So perhaps we should came to an agreement where and how we should be >> >> doing this sort of policy, we could perhaps generalize >> >> module-bluetooth-policy, or do we want to continue with per technology >> >> policy? >> > >> > I don't think the policy in module-bluetooth-policy is useful outside >> > Bluetooth. module-bluetooth-policy is mainly concerned with the >> > transport state in its policy, and the profile availability is only used >> > as a proxy for the transport state. I'd be happy if >> > module-bluetooth-policy would be changed so that instead of using the >> > profile states, it would directly track the transport states using the >> > APIs that pa_bluez_discovery provides. But since using the profile >> > states as a proxy works fine too, I'm not really asking anyone to do >> > anything about that. >> > >> > From the proposed changes, I think Colin's patches 1 and 3 make sense. >> > Preferring PA_AVAILABLE_YES over PA_AVAILABLE_UNKNOWN in generic code >> > isn't a good idea. >> >> Im having a hard time to understand why any policy would prefer a >> profile that is less likely to work than another, > > If you have a "bad" (not very well suited for the use case) profile and > a "good" (very well suited for the use case) profile, and the > probability of the "bad" profile working is 100% and the probability of > the "good" profile working is 99%, do you really see no reason to choose > the "good" profile? Well there is nothing to classify a profile as good/bad, is there? Also the chance can be 99% or 1%, so it is not deterministic. My argument is that pa_card_new don't have this information so it should not pretend it knows best and select a profile with available set to unknown. >> but perhaps we are >> not willing the mess with other parts of the code that were using this >> information in a different way. But then why we are bothering >> selecting any profile on pa_card_new? This is a policy decision and it >> seems we are not able to do a generic one so it should not even try, >> pick the first and be done with it. > > Yes, it's a policy decision. I would prefer pa_card_new() to always > choose the "off" profile by default, but that would require work in > policy modules to keep old use cases working. For example, on the first > boot of a PC something else than "off" should be selected for the sound > card, and currently we don't have any policy module that would ensure > that. And nothing fits the "good" profile that works 99% of the time? I guess that should be marked as available 'yes', or perhaps the modules should prefer something when they create the card with pa_card_new_data_set_profile but then we perhaps need to change when PA_CORE_HOOK_CARD_NEW is fired otherwise it might break things like module-card-restore. >> Anyway if pa_card_new do not select 'off' by default it pretty much >> breaks Bluetooth qualification and if I change the priority of 'off' >> profile to force it to be selected it breaks module-bluetooth-policy. > > Can you tell the Bluetooth qualification requirement that is failing? If > it's something HFGW specific, you should be able to implement the > desired policy in module-bluetooth-policy, without modifying > pa_card_new(). The qualification requires that no audio connection should be open in certain tests, but right now pa_card_new will select 'hfgw' as it has the highest priority thus creating a SCO connection. module-bluetooth-policy doesn't use PA_CORE_HOOK_CARD_NEW perhaps you want it to use it to select it to 'off' before that. Still this will only work for modules-bluetooth-policy and internal modules, external policy components are at the mercy of pa_card_new auto selecting the right profile. -- Luiz Augusto von Dentz
