[PATCH] ASoC: Intel: sof_sdw: Initialize the sof_sdw_quirk with RT711_JD_NULL

[PATCH] ASoC: Intel: sof_sdw: Initialize the sof_sdw_quirk with RT711_JD_NULL

[Chris Chiu]

The jd_src of RT711 which is initialized in rt711/rt711_sdca_init
will be overridden by rt711/rt711_sdca_add_codec_device_props when
the sof_sdw_quirk is not RT711_JD_NULL. It will force the JD mode
to RT711_JD1 and cause confusion while debugging the JD mode of
the boards without quirk. Initialize sof_sdw_quirk with RT711_JD_NULL
to honor the jd_src value in rt711/rt711_sdca init.

Signed-off-by: Chris Chiu <chris.chiu@canonical.com>
---
 sound/soc/intel/boards/sof_sdw.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/soc/intel/boards/sof_sdw.c b/sound/soc/intel/boards/sof_sdw.c
index 6b06248a9327..d05c0565e09c 100644
--- a/sound/soc/intel/boards/sof_sdw.c
+++ b/sound/soc/intel/boards/sof_sdw.c
@@ -15,7 +15,7 @@
 #include "sof_sdw_common.h"
 #include "../../codecs/rt711.h"
 
-unsigned long sof_sdw_quirk = RT711_JD1;
+unsigned long sof_sdw_quirk = RT711_JD_NULL;
 static int quirk_override = -1;
 module_param_named(quirk, quirk_override, int, 0444);
 MODULE_PARM_DESC(quirk, "Board-specific quirk override");
-- 
2.20.1

Re: [PATCH] ASoC: Intel: sof_sdw: Initialize the sof_sdw_quirk with RT711_JD_NULL

[Pierre-Louis Bossart]

On 10/15/21 8:34 AM, Chris Chiu wrote:
> The jd_src of RT711 which is initialized in rt711/rt711_sdca_init
> will be overridden by rt711/rt711_sdca_add_codec_device_props when
> the sof_sdw_quirk is not RT711_JD_NULL. It will force the JD mode
> to RT711_JD1 and cause confusion while debugging the JD mode of
> the boards without quirk. Initialize sof_sdw_quirk with RT711_JD_NULL
> to honor the jd_src value in rt711/rt711_sdca init.

Not able to follow what the "confusion while debugging the JD mode of
the boards without quirk" is. You need a DMI quirk or need to override
the default quirk with the kernel module parameter.

This also has the side effect of breaking ALL existing DMI quirks
implicitly using JD1...


> Signed-off-by: Chris Chiu <chris.chiu@canonical.com>
> ---
>  sound/soc/intel/boards/sof_sdw.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/sound/soc/intel/boards/sof_sdw.c b/sound/soc/intel/boards/sof_sdw.c
> index 6b06248a9327..d05c0565e09c 100644
> --- a/sound/soc/intel/boards/sof_sdw.c
> +++ b/sound/soc/intel/boards/sof_sdw.c
> @@ -15,7 +15,7 @@
>  #include "sof_sdw_common.h"
>  #include "../../codecs/rt711.h"
>  
> -unsigned long sof_sdw_quirk = RT711_JD1;
> +unsigned long sof_sdw_quirk = RT711_JD_NULL;
>  static int quirk_override = -1;
>  module_param_named(quirk, quirk_override, int, 0444);
>  MODULE_PARM_DESC(quirk, "Board-specific quirk override");
> 

Re: [PATCH] ASoC: Intel: sof_sdw: Initialize the sof_sdw_quirk with RT711_JD_NULL

[Chris Chiu]

On Fri, Oct 15, 2021 at 11:08 PM Pierre-Louis Bossart
<pierre-louis.bossart@linux.intel.com> wrote:
>
>
>
> On 10/15/21 8:34 AM, Chris Chiu wrote:
> > The jd_src of RT711 which is initialized in rt711/rt711_sdca_init
> > will be overridden by rt711/rt711_sdca_add_codec_device_props when
> > the sof_sdw_quirk is not RT711_JD_NULL. It will force the JD mode
> > to RT711_JD1 and cause confusion while debugging the JD mode of
> > the boards without quirk. Initialize sof_sdw_quirk with RT711_JD_NULL
> > to honor the jd_src value in rt711/rt711_sdca init.
>
> Not able to follow what the "confusion while debugging the JD mode of
> the boards without quirk" is. You need a DMI quirk or need to override
> the default quirk with the kernel module parameter.
>
The JD mode will be set by rt711/rt711_sdca_init first (which is JD2
as https://github.com/torvalds/linux/blob/master/sound/soc/codecs/rt711.c#L1209.
Then it will be overridden by rt711_add_codec_device_props() while doing
rt711_parse_dt(), which is now always JD1 since the current
sof_sdw_quirk init value.
I'm afraid that JD2 is a more preferable mode rather than JD1. Then we
will have to
maintain a bigger DMI quirk table for more and more coming alderlake machines.

Given the rt711 codec has initialized the jd_src to JD2, The
sof_sdw_quirk should only
override it unless necessary? But now it's forced to override. Or can
we have a more
generic solution for it?

Chris


> This also has the side effect of breaking ALL existing DMI quirks
> implicitly using JD1...
>
>
> > Signed-off-by: Chris Chiu <chris.chiu@canonical.com>
> > ---
> >  sound/soc/intel/boards/sof_sdw.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/sound/soc/intel/boards/sof_sdw.c b/sound/soc/intel/boards/sof_sdw.c
> > index 6b06248a9327..d05c0565e09c 100644
> > --- a/sound/soc/intel/boards/sof_sdw.c
> > +++ b/sound/soc/intel/boards/sof_sdw.c
> > @@ -15,7 +15,7 @@
> >  #include "sof_sdw_common.h"
> >  #include "../../codecs/rt711.h"
> >
> > -unsigned long sof_sdw_quirk = RT711_JD1;
> > +unsigned long sof_sdw_quirk = RT711_JD_NULL;
> >  static int quirk_override = -1;
> >  module_param_named(quirk, quirk_override, int, 0444);
> >  MODULE_PARM_DESC(quirk, "Board-specific quirk override");
> >

Re: [PATCH] ASoC: Intel: sof_sdw: Initialize the sof_sdw_quirk with RT711_JD_NULL

[Pierre-Louis Bossart]

>> On 10/15/21 8:34 AM, Chris Chiu wrote:
>>> The jd_src of RT711 which is initialized in rt711/rt711_sdca_init
>>> will be overridden by rt711/rt711_sdca_add_codec_device_props when
>>> the sof_sdw_quirk is not RT711_JD_NULL. It will force the JD mode
>>> to RT711_JD1 and cause confusion while debugging the JD mode of
>>> the boards without quirk. Initialize sof_sdw_quirk with RT711_JD_NULL
>>> to honor the jd_src value in rt711/rt711_sdca init.
>>
>> Not able to follow what the "confusion while debugging the JD mode of
>> the boards without quirk" is. You need a DMI quirk or need to override
>> the default quirk with the kernel module parameter.
>>
> The JD mode will be set by rt711/rt711_sdca_init first (which is JD2
> as https://github.com/torvalds/linux/blob/master/sound/soc/codecs/rt711.c#L1209.
> Then it will be overridden by rt711_add_codec_device_props() while doing
> rt711_parse_dt(), which is now always JD1 since the current
> sof_sdw_quirk init value.
> I'm afraid that JD2 is a more preferable mode rather than JD1. Then we
> will have to
> maintain a bigger DMI quirk table for more and more coming alderlake machines.
> 
> Given the rt711 codec has initialized the jd_src to JD2, The
> sof_sdw_quirk should only
> override it unless necessary? But now it's forced to override. Or can
> we have a more
> generic solution for it?

I don't mind changing the default if the majority of devices uses JD2.
But your patch doesn't do this cleanly, it breaks all usages of JD1 that
relied on the previous default.

Changing the default for jack detection also doesn't really decrease the
need for quirks in itself, for this we would need to handle the RT715
quirk and also fix the amplifier link id, and also set the HDMI quirk
depending on the platform setting. Bard's working on this, see
https://github.com/thesofproject/linux/pull/3203