card: Only set active_profile with available profile

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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, 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.

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.

-- 
Luiz Augusto von Dentz


[Index of Archives]     [Linux Audio Users]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux