| Message ID | 20211015133424.494463-1-chris.chiu@canonical.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ASoC: Intel: sof_sdw: Initialize the sof_sdw_quirk with RT711_JD_NULL | expand |
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"); >
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"); > >
>> 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
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");
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(-)