Rationale for BYTCR defaults in the kernel's bytcr_rt5640 machine driver ?

Rationale for BYTCR defaults in the kernel's bytcr_rt5640 machine driver ?

[Hans de Goede]

Hi Pierre-Louis,

I'm finally wrapping up my rt5640 jack-detect work, as
such I'm currently testing speaker + headphones + internal mic +
headset-mic + jack-detect functionality on the 10 different
x86 devices with a rt5640 codec which I've gathered.

2 of them stand out in that they use the BYTCR SoC, but don't
have the ACPI table for detecting if SSP0 AIF1 or AIF2
should be used. Currently the driver defaults to AIF2
in this case.

As the somewhat wildcard-ish DMI quirk for boards where
the sys_vendor is "Insyde" (which applies to a lot of
generic designs) shows:

         {
                 .callback = byt_rt5640_quirk_cb,
                 .matches = {
                         DMI_MATCH(DMI_SYS_VENDOR, "Insyde"),
                 },
                 .driver_data = (void *)(BYT_RT5640_IN3_MAP |
                                         BYT_RT5640_MCLK_EN |
                                         BYT_RT5640_SSP0_AIF1),

         },

AIF1 seems to be a better default both models without
the ACPI table which I've here, a HP pavilion X2 and a
Toshiba Click Mini L9W-B need a quirk to use AIF1.

So I was wondering if there was a specific rationale for the
AIF2 default and if it would not be better to change the default
to AIF1 (which will unfortunately bring a chance of regressions) ?

Regards,

Hans

Re: Rationale for BYTCR defaults in the kernel's bytcr_rt5640 machine driver ?

[Pierre-Louis Bossart]

On 05/01/2018 02:21 PM, Hans de Goede wrote:
> Hi Pierre-Louis,
>
> I'm finally wrapping up my rt5640 jack-detect work, as
> such I'm currently testing speaker + headphones + internal mic +
> headset-mic + jack-detect functionality on the 10 different
> x86 devices with a rt5640 codec which I've gathered.
>
> 2 of them stand out in that they use the BYTCR SoC, but don't
> have the ACPI table for detecting if SSP0 AIF1 or AIF2
> should be used. Currently the driver defaults to AIF2
> in this case.
>
> As the somewhat wildcard-ish DMI quirk for boards where
> the sys_vendor is "Insyde" (which applies to a lot of
> generic designs) shows:
>
>         {
>                 .callback = byt_rt5640_quirk_cb,
>                 .matches = {
>                         DMI_MATCH(DMI_SYS_VENDOR, "Insyde"),
>                 },
>                 .driver_data = (void *)(BYT_RT5640_IN3_MAP |
>                                         BYT_RT5640_MCLK_EN |
>                                         BYT_RT5640_SSP0_AIF1),
>
>         },
>
> AIF1 seems to be a better default both models without
> the ACPI table which I've here, a HP pavilion X2 and a
> Toshiba Click Mini L9W-B need a quirk to use AIF1.
>
> So I was wondering if there was a specific rationale for the
> AIF2 default and if it would not be better to change the default
> to AIF1 (which will unfortunately bring a chance of regressions) ?

yes there was a rationale: all the initial Baytrail designs had 
connections between SSP2-AIF1 and SSP0-AIF2. IIRC this was because there 
was a desire to support 44.1kHz on one SSP and 48kHz on the other.
So when BYT-CR was defined with SSP2 removed, most designs kept the 
existing SSP0-AIF2 link, except of course when they didn't...

I don't have a magic solution here, when the BIOS doesn't provide 
information it's difficult to make an educated guess. If you have 
statistical evidence that more devices without the BIOS information are 
based on AIF2 I don't mind changing it, alternatively we could keep the 
default and when things don't work users can still use a module 
parameter to override the default and get AIF1 to work


_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: Rationale for BYTCR defaults in the kernel's bytcr_rt5640 machine driver ?

[Hans de Goede]

Hi,

On 01-05-18 22:29, Pierre-Louis Bossart wrote:
> 
> 
> On 05/01/2018 02:21 PM, Hans de Goede wrote:
>> Hi Pierre-Louis,
>>
>> I'm finally wrapping up my rt5640 jack-detect work, as
>> such I'm currently testing speaker + headphones + internal mic +
>> headset-mic + jack-detect functionality on the 10 different
>> x86 devices with a rt5640 codec which I've gathered.
>>
>> 2 of them stand out in that they use the BYTCR SoC, but don't
>> have the ACPI table for detecting if SSP0 AIF1 or AIF2
>> should be used. Currently the driver defaults to AIF2
>> in this case.
>>
>> As the somewhat wildcard-ish DMI quirk for boards where
>> the sys_vendor is "Insyde" (which applies to a lot of
>> generic designs) shows:
>>
>>         {
>>                 .callback = byt_rt5640_quirk_cb,
>>                 .matches = {
>>                         DMI_MATCH(DMI_SYS_VENDOR, "Insyde"),
>>                 },
>>                 .driver_data = (void *)(BYT_RT5640_IN3_MAP |
>>                                         BYT_RT5640_MCLK_EN |
>>                                         BYT_RT5640_SSP0_AIF1),
>>
>>         },
>>
>> AIF1 seems to be a better default both models without
>> the ACPI table which I've here, a HP pavilion X2 and a
>> Toshiba Click Mini L9W-B need a quirk to use AIF1.
>>
>> So I was wondering if there was a specific rationale for the
>> AIF2 default and if it would not be better to change the default
>> to AIF1 (which will unfortunately bring a chance of regressions) ?
> 
> yes there was a rationale: all the initial Baytrail designs had connections between SSP2-AIF1 and SSP0-AIF2. IIRC this was because there was a desire to support 44.1kHz on one SSP and 48kHz on the other.
> So when BYT-CR was defined with SSP2 removed, most designs kept the existing SSP0-AIF2 link, except of course when they didn't...
> 
> I don't have a magic solution here, when the BIOS doesn't provide information it's difficult to make an educated guess. If you have statistical evidence that more devices without the BIOS information are based on AIF2 I don't mind changing it, alternatively we could keep the default and when things don't work users can still use a module parameter to override the default and get AIF1 to work

Thanks for the info. Given that there was a good reason for the default, we
should probably just keep it as is.

It looks like I'm going to need a lot of quirks anyways for the jack-detect
stuff as well as for selecting the right input for the internal mic.

It seems that there are no correct defaults here, I've an aprox 50/50%
split for JD1IN4P vs JD2IN4N and als a 45/45/10% split for using
IN1/IN3/DMIC1 for the internal mic. So it looks like it is simply going
to be 1 DMI quirk per model :|

Regards,

Hans
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: Rationale for BYTCR defaults in the kernel's bytcr_rt5640 machine driver ?

[Pierre-Louis Bossart]

On 5/3/18 3:13 AM, Hans de Goede wrote:
> Hi,
> 
> On 01-05-18 22:29, Pierre-Louis Bossart wrote:
>>
>>
>> On 05/01/2018 02:21 PM, Hans de Goede wrote:
>>> Hi Pierre-Louis,
>>>
>>> I'm finally wrapping up my rt5640 jack-detect work, as
>>> such I'm currently testing speaker + headphones + internal mic +
>>> headset-mic + jack-detect functionality on the 10 different
>>> x86 devices with a rt5640 codec which I've gathered.
>>>
>>> 2 of them stand out in that they use the BYTCR SoC, but don't
>>> have the ACPI table for detecting if SSP0 AIF1 or AIF2
>>> should be used. Currently the driver defaults to AIF2
>>> in this case.
>>>
>>> As the somewhat wildcard-ish DMI quirk for boards where
>>> the sys_vendor is "Insyde" (which applies to a lot of
>>> generic designs) shows:
>>>
>>>         {
>>>                 .callback = byt_rt5640_quirk_cb,
>>>                 .matches = {
>>>                         DMI_MATCH(DMI_SYS_VENDOR, "Insyde"),
>>>                 },
>>>                 .driver_data = (void *)(BYT_RT5640_IN3_MAP |
>>>                                         BYT_RT5640_MCLK_EN |
>>>                                         BYT_RT5640_SSP0_AIF1),
>>>
>>>         },
>>>
>>> AIF1 seems to be a better default both models without
>>> the ACPI table which I've here, a HP pavilion X2 and a
>>> Toshiba Click Mini L9W-B need a quirk to use AIF1.
>>>
>>> So I was wondering if there was a specific rationale for the
>>> AIF2 default and if it would not be better to change the default
>>> to AIF1 (which will unfortunately bring a chance of regressions) ?
>>
>> yes there was a rationale: all the initial Baytrail designs had 
>> connections between SSP2-AIF1 and SSP0-AIF2. IIRC this was because 
>> there was a desire to support 44.1kHz on one SSP and 48kHz on the other.
>> So when BYT-CR was defined with SSP2 removed, most designs kept the 
>> existing SSP0-AIF2 link, except of course when they didn't...
>>
>> I don't have a magic solution here, when the BIOS doesn't provide 
>> information it's difficult to make an educated guess. If you have 
>> statistical evidence that more devices without the BIOS information 
>> are based on AIF2 I don't mind changing it, alternatively we could 
>> keep the default and when things don't work users can still use a 
>> module parameter to override the default and get AIF1 to work
> 
> Thanks for the info. Given that there was a good reason for the default, we
> should probably just keep it as is.
> 
> It looks like I'm going to need a lot of quirks anyways for the jack-detect
> stuff as well as for selecting the right input for the internal mic.
> 
> It seems that there are no correct defaults here, I've an aprox 50/50%
> split for JD1IN4P vs JD2IN4N and als a 45/45/10% split for using
> IN1/IN3/DMIC1 for the internal mic. So it looks like it is simply going
> to be 1 DMI quirk per model :|

I am afraid you are right. hardware folks make use of every possible 
combination of (AIF, mic interface, JD method, button press method)

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel