On Sun, 2014-08-17 at 21:03 +0600, Alexander E. Patrakov wrote: > 2014-08-17 17:52 GMT+06:00 Tanu Kaskinen <tanu.kaskinen at linux.intel.com>: > > The analog-output path should be used only when more specific outputs > > don't exist, so usually it should be suppressed, and currently that > > doesn't happen. One practical problem caused by that is that the > > analog-output port may get chosen during boot if headphones are > > plugged in, because analog-output has higher priority than headphones > > and the speaker port is unavailable[1]. That particular problem could > > maybe be fixed by decreasing the analog-output priority, or by > > tweaking module-switch-on-port-available, but this time I decided to > > fix the problem of analog-output appearing when it's redundant. > > > > [1] https://bugs.freedesktop.org/show_bug.cgi?id=74609 > > > > Tanu Kaskinen (2): > > alsa-mixer: Fix path subset detection > > alsa-mixer: Ignore some elements in the analog-output path > > > > src/modules/alsa/alsa-mixer.c | 26 +++++++++---------- > > src/modules/alsa/mixer/paths/analog-output.conf | 34 ------------------------- > > 2 files changed, 13 insertions(+), 47 deletions(-) > > I have looked at and tested both patches on my laptop. > > Patch 1/2 gets a full ACK. It makes mixer element handling consistent > with the existing code for jack handling. > Patch 2/2, when applied on top of 1/2, successfully kills the unwanted > Analog output port on my laptop. However, I have not verified that the > set of to-be-removed elements in this patch is indeed minimal and > sufficient. So this patch only gets a "possibly-incomplete ACK". Let's define what "minimal" and "sufficient" mean. The patch is minimal if it only removes elements/jacks that prevent subset detection from working in some scenarios. It appears that "Jack Line Out Phantom" doesn't necessarily have to be removed, because analog-output-lineout (the only other path that uses that jack) configures the jack the same way. However, there's no real reason why analog-output should care about "Jack Line Out Phantom", so a minimal patch isn't really the goal here. Rather than asking "does this element/jack prevent subset detection from working", a better question is "is there some path that should be a superset of analog-output that marks the element/jack as required for that path". analog-output-lineout should be a superset of analog-output, and "Jack Line Out Phantom" is required for that path. "Required" here doesn't have the usual meaning of the word, instead it means that the element/jack is marked as "required-any = any", which doesn't mean that the element/jack is strictly necessary, only that at least one of the elements and jacks marked as "required-any = any" must be present. Sorry for confusing language, I couldn't figure out a better way to express myself than redefining the word "required"... The patch doesn't meet the revised criteria either, and that should be fixed. The patch removes "Element Master Mono", which is not required by any superset paths, but it is required by analog-output-mono, which I believe shouldn't be a superset of analog-output. I shouldn't have removed "Element Master Mono". The patch also removes "Element Line HP Swap", which isn't required by any path. I think the patch is correct in this case, and the real bug is that the paths that care about Line HP Swap don't mark it as required. The patch is sufficient if none of the elements left in the file prevent subset detection from working in any scenario. The patch doesn't meet this criteria if there are any should-be-superset paths that either don't have all the elements and jacks that are in analog-output or some of the elements and jacks have different configuration that prevents the subset detection from working. I haven't checked that the patch is correct in this sense. > Maybe it is a good idea to write a test that verifies that the > analog-output path is indeed a subset of all mixer paths that should > kill it? Yes, that's probably a good idea. I'll try to write such test (it may take quite a bit of work - I think I'll need to write a mock implementation of the alsa-lib interface to make the test hardware-independent). -- Tanu
