On Sun, 01 Sep 2019 21:27:37 +0200, Sergey 'Jin' Bostandzhyan wrote:
Other than that, the approach with UCM profile would be simpler. It won't need anything else than what you already showed (pincfg and GPIO) in the driver side.
Simpler...yes and no :) From what I have seen, all "default" Pulse profiles are replaced by the UCM, meaning that if I wanted them, I'd have to replicate all of them in my conf. It would work though.
You just need to override codec->card->longname to some unique string and use it as UCM profile name. Check alc1220_fixup_gb_dual_codecs() as an example.
{0x01, AC_VERB_SET_GPIO_DIRECTION, 0x02}
Actually this must be paired with the corresponding bit of GPIO_DATA, too. Is the bit 0x02 of GPIO_DATA set or cleared? Usually setting it turns on the amp, but sometimes inverted.
If I understood everything correctly, then the bit is set, meaning that the GPIO signal is configured as output.
The GPIO_DIRECTION specifies whether input or output, and the actual value is passed via GPIO_DATA. Both have to be specified, not only one.
[...]
set(0x01, 0x717, 0x02) # 0x01071702 (SET_GPIO_DIRECTION)
This needs the paired SET_GPIO_DATA for a proper operation. Your analysis implicit assumes some default value that is already set by the hardware.
If I understand it correctly, then "some value" is zero on my hardware:
# hda-verb /dev/snd/hwC0D0 0x01 GET_GPIO_DATA 0x02 nid = 0x1, verb = 0xf15, param = 0x2 value = 0x0
Meanwhile I also figured out that /proc/asound/card0/codec#0 is providing this info as well:
IO[1]: enable=0, dir=1, wake=0, sticky=0, data=0, unsol=0
So the value seems to be 0 and I can add an explicit SET_GPIO_DATA verb quirk to set it in addition to SET_GPIO_DIRECTION, right?
Yes. You need to set SET_GPIO_MASK=0x02, SET_GPIO_DIRECTION=0x02 and SET_GPIO_DATA=0x00 for that bit.
If the above is a yes, could you please point me to the right way of getting triggered upon jack state changes in the driver, since automute hooks seem not to trigger while something is playing?
This isn't easy, as repeatedly mentioned, since the parser assumes only one role for each pin except for the input/output switching. In your case the pin shares for two outputs to be switched, and this isn't implemented at all.
I think we talked a bit past each other, based on your reply I understand that you were assuming that I am still trying to solve it in the parser in some generic way, which as you indeed repeatedly mentioned would not be easy.
At the same time I do not rule out the possibility, that I simply do not see the "bigger picture" and that you have already accounted for the proposal that follows :)
Given that this is my first driver hacking adventure, I was aiming for a much more localized solution, so I was not going to add generic support for shared pins. Once I learned how to subscribe to jack state changes I was able to implement the idea that I had inside the model quirk:
static void alc662_aspire_ethos_mute_speakers(struct hda_codec *codec, struct hda_jack_callback *cb) { /* surround speakers muted automatically when headphones are * plugged in, we'll mute/unmute the remaining channels manually: * 0x15 - front left/front right * 0x18 - front center/ LFE * */ if (snd_hda_jack_detect_state(codec, cb->nid) == HDA_JACK_PRESENT) { snd_hda_set_pin_ctl_cache(codec, 0x15, 0); snd_hda_set_pin_ctl_cache(codec, 0x18, 0); } else { snd_hda_set_pin_ctl_cache(codec, 0x15, PIN_OUT); snd_hda_set_pin_ctl_cache(codec, 0x18, PIN_OUT); } }
static void alc662_fixup_aspire_ethos_hp(struct hda_codec *codec, const struct hda_fixup *fix, int action) { if (action == HDA_FIXUP_ACT_PRE_PROBE) { if (is_jack_detectable(codec, 0x1b)) { snd_hda_jack_detect_enable_callback(codec, 0x1b, alc662_aspire_ethos_mute_speakers); } } }
This was the idea that I have been referring to, when I was talking about "muting in the driver" after I learned that proper automuting based on hp pins was not possible as you indeed repeatedly stated.
The above seems to work quite well for me and does exactly what I want, PulseAudio presents all the autogenerated profiles and handles mic and line jacks itself, at the same time all unwanted speakers get muted as soon as I plug in my headphones into the jack pin that is shared with my surround speakers. Of course Pulse does not "know" anything about the headphones and does not switch profiles, but I don't mind since the user experience is as expected.
Hm, OK, this amount of changes are acceptable. The hardware behavior itself is weird, and we have already tricky code, so it's no problem to keep some yet another tricky code as long as it's described enough in the comments and the changelog.
My earlier attempt was to send the pin widget control verbs directly, however then the pin got reconnected as soon as playback started. This does not happen when I use snd_hda_set_pin_ctl_cache(), but I am not quite sure about the cache, should I use the _cache function or the uncached one?
This should work, AFAIK. The *_set_pin_ctl_cache() remembers the last written value, as its name stands. That's restored again at the PM resume, for example.
The PM resume does re-trigger the jack detection callback, so it'll be written up again in anyway, though.
Another thing I am not sure about is, if I somehow disrupt power management by doing what I do? I saw that for instance restore_shutup_pins() does modify those connections as well and I would basically overwrite whatever it did in the case that the user plugs/unplugs the headphones.
This should be fine as-is. The shutup_pins() clears pins temporarily and the pins are resumed to the cached values in return.
One thing to be improved would be to drop the surround jack control. Adjust the pin config to different value with the fixed pin connection, so that the auto-parser won't create the "Surround Jack" control. This isn't needed by PA or else, otherwise it may be confusing.
thanks,
Takashi