[PATCH 3/4] ALSA: hda/cirrus: Add jack detect interrupt support from CS42L42 companion codec.

2021-03-03 Thread Vitaly Rodionov
In the case of CS8409 we do not have unsol events from NID's 0x24 and 0x34
where hs mic and hp are connected. Companion codec CS42L42 will generate
interrupt via gpio 4 to notify jack events. We have to overwrite standard
snd_hda_jack_unsol_event(), read CS42L42 jack detect status registers and
then notify status via generic snd_hda_jack_unsol_event() call.

Tested on DELL Inspiron-3500, DELL Inspiron-3501, DELL Inspiron-3505.

Signed-off-by: Vitaly Rodionov 
---
 sound/pci/hda/patch_cirrus.c | 335 +--
 1 file changed, 324 insertions(+), 11 deletions(-)

diff --git a/sound/pci/hda/patch_cirrus.c b/sound/pci/hda/patch_cirrus.c
index 9d664982544f..fa417f7adc7d 100644
--- a/sound/pci/hda/patch_cirrus.c
+++ b/sound/pci/hda/patch_cirrus.c
@@ -9,6 +9,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -38,6 +39,13 @@ struct cs_spec {
/* for MBP SPDIF control */
int (*spdif_sw_put)(struct snd_kcontrol *kcontrol,
struct snd_ctl_elem_value *ucontrol);
+
+   unsigned int cs42l42_hp_jack_in:1;
+   unsigned int cs42l42_mic_jack_in:1;
+
+   /* verb exec op override */
+   int (*exec_verb)(struct hdac_device *dev, unsigned int cmd,
+unsigned int flags, unsigned int *res);
 };
 
 /* available models with CS420x */
@@ -1229,6 +1237,13 @@ static int patch_cs4213(struct hda_codec *codec)
 #define CS8409_CS42L42_SPK_PIN_NID 0x2c
 #define CS8409_CS42L42_AMIC_PIN_NID0x34
 #define CS8409_CS42L42_DMIC_PIN_NID0x44
+#define CS8409_CS42L42_DMIC_ADC_PIN_NID0x22
+
+#define CS42L42_HSDET_AUTO_DONE0x02
+#define CS42L42_HSTYPE_MASK0x03
+
+#define CS42L42_JACK_INSERTED  0x0C
+#define CS42L42_JACK_REMOVED   0x00
 
 #define GPIO3_INT (1 << 3)
 #define GPIO4_INT (1 << 4)
@@ -1243,6 +1258,8 @@ static int patch_cs4213(struct hda_codec *codec)
 #define CIR_I2C_QWRITE 0x005D
 #define CIR_I2C_QREAD  0x005E
 
+static struct mutex cs8409_i2c_mux;
+
 struct cs8409_i2c_param {
unsigned int addr;
unsigned int reg;
@@ -1433,6 +1450,7 @@ static const struct cs8409_i2c_param 
cs42l42_init_reg_seq[] = {
{ 0x1C03, 0xC0 },
{ 0x1105, 0x00 },
{ 0x1112, 0xC0 },
+   { 0x1101, 0x02 },
{} /* Terminator */
 };
 
@@ -1580,21 +1598,180 @@ static void cs8409_cs42l42_reset(struct hda_codec 
*codec)
/* wait ~10ms */
usleep_range(1, 15000);
 
-   /* Clear interrupts status */
+   mutex_lock(&cs8409_i2c_mux);
+
+   /* Clear interrupts, by reading interrupt status registers */
cs8409_i2c_read(codec, CS42L42_I2C_ADDR, 0x1308, 1);
cs8409_i2c_read(codec, CS42L42_I2C_ADDR, 0x1309, 1);
cs8409_i2c_read(codec, CS42L42_I2C_ADDR, 0x130A, 1);
cs8409_i2c_read(codec, CS42L42_I2C_ADDR, 0x130F, 1);
 
+   mutex_unlock(&cs8409_i2c_mux);
+
+}
+
+/* Configure CS42L42 slave codec for jack autodetect */
+static int cs8409_cs42l42_enable_jack_detect(struct hda_codec *codec)
+{
+   mutex_lock(&cs8409_i2c_mux);
+
+   /* Set TIP_SENSE_EN for analog front-end of tip sense. */
+   cs8409_i2c_write(codec, CS42L42_I2C_ADDR, 0x1b70, 0x0020, 1);
+   /* Clear WAKE# */
+   cs8409_i2c_write(codec, CS42L42_I2C_ADDR, 0x1b71, 0x0001, 1);
+   /* Wait ~2.5ms */
+   usleep_range(2500, 3000);
+   /* Set mode WAKE# output follows the combination logic directly */
+   cs8409_i2c_write(codec, CS42L42_I2C_ADDR, 0x1b71, 0x0020, 1);
+   /* Clear interrupts status */
+   cs8409_i2c_read(codec, CS42L42_I2C_ADDR, 0x130f, 1);
+   cs8409_i2c_read(codec, CS42L42_I2C_ADDR, 0x1b7b, 1);
+   /* Enable interrupt */
+   cs8409_i2c_write(codec, CS42L42_I2C_ADDR, 0x1320, 0x03, 1);
+   cs8409_i2c_write(codec, CS42L42_I2C_ADDR, 0x1b79, 0x00, 1);
+
+   mutex_unlock(&cs8409_i2c_mux);
+
+   return 0;
+}
+
+/* Enable and run CS42L42 slave codec jack auto detect */
+static void cs8409_cs42l42_run_jack_detect(struct hda_codec *codec)
+{
+   mutex_lock(&cs8409_i2c_mux);
+
+   /* Clear interrupts */
+   cs8409_i2c_read(codec, CS42L42_I2C_ADDR, 0x1308, 1);
+   cs8409_i2c_read(codec, CS42L42_I2C_ADDR, 0x1b77, 1);
+
+   cs8409_i2c_write(codec, CS42L42_I2C_ADDR, 0x1102, 0x87, 1);
+   cs8409_i2c_write(codec, CS42L42_I2C_ADDR, 0x1f06, 0x86, 1);
+   cs8409_i2c_write(codec, CS42L42_I2C_ADDR, 0x1b74, 0x07, 1);
+   cs8409_i2c_write(codec, CS42L42_I2C_ADDR, 0x131b, 0x01, 1);
+   cs8409_i2c_write(codec, CS42L42_I2C_ADDR, 0x1120, 0x80, 1);
+   /* Wait ~110ms*/
+   usleep_range(11, 20);
+   cs8409_i2c_write(codec, CS42L42_I2C_ADDR, 0x111f, 0x77, 1);
+   cs8409_i2c_write(codec, CS42L42_I2C_ADDR, 0x1120, 0xc0, 1);
+   /* Wait ~10ms */
+   usleep_range(1, 25000);
+
+   mutex_unlock(&cs8409_i2c_mux);
+
 }
 
 static void cs8409_cs42l42_reg_setup(struct hda_codec *codec)
 {
const struct cs8409_i2c_p

[PATCH 3/4] ALSA: hda/cirrus: Add jack detect interrupt support from CS42L42 companion codec.

2021-03-03 Thread Vitaly Rodionov
In the case of CS8409 we do not have unsol events from NID's 0x24 and 0x34
where hs mic and hp are connected. Companion codec CS42L42 will generate
interrupt via gpio 4 to notify jack events. We have to overwrite standard
snd_hda_jack_unsol_event(), read CS42L42 jack detect status registers and
then notify status via generic snd_hda_jack_unsol_event() call.

Tested on DELL Inspiron-3500, DELL Inspiron-3501, DELL Inspiron-3505.

Signed-off-by: Vitaly Rodionov 
---
 sound/pci/hda/patch_cirrus.c | 335 +--
 1 file changed, 324 insertions(+), 11 deletions(-)

diff --git a/sound/pci/hda/patch_cirrus.c b/sound/pci/hda/patch_cirrus.c
index 9d664982544f..fa417f7adc7d 100644
--- a/sound/pci/hda/patch_cirrus.c
+++ b/sound/pci/hda/patch_cirrus.c
@@ -9,6 +9,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -38,6 +39,13 @@ struct cs_spec {
/* for MBP SPDIF control */
int (*spdif_sw_put)(struct snd_kcontrol *kcontrol,
struct snd_ctl_elem_value *ucontrol);
+
+   unsigned int cs42l42_hp_jack_in:1;
+   unsigned int cs42l42_mic_jack_in:1;
+
+   /* verb exec op override */
+   int (*exec_verb)(struct hdac_device *dev, unsigned int cmd,
+unsigned int flags, unsigned int *res);
 };
 
 /* available models with CS420x */
@@ -1229,6 +1237,13 @@ static int patch_cs4213(struct hda_codec *codec)
 #define CS8409_CS42L42_SPK_PIN_NID 0x2c
 #define CS8409_CS42L42_AMIC_PIN_NID0x34
 #define CS8409_CS42L42_DMIC_PIN_NID0x44
+#define CS8409_CS42L42_DMIC_ADC_PIN_NID0x22
+
+#define CS42L42_HSDET_AUTO_DONE0x02
+#define CS42L42_HSTYPE_MASK0x03
+
+#define CS42L42_JACK_INSERTED  0x0C
+#define CS42L42_JACK_REMOVED   0x00
 
 #define GPIO3_INT (1 << 3)
 #define GPIO4_INT (1 << 4)
@@ -1243,6 +1258,8 @@ static int patch_cs4213(struct hda_codec *codec)
 #define CIR_I2C_QWRITE 0x005D
 #define CIR_I2C_QREAD  0x005E
 
+static struct mutex cs8409_i2c_mux;
+
 struct cs8409_i2c_param {
unsigned int addr;
unsigned int reg;
@@ -1433,6 +1450,7 @@ static const struct cs8409_i2c_param 
cs42l42_init_reg_seq[] = {
{ 0x1C03, 0xC0 },
{ 0x1105, 0x00 },
{ 0x1112, 0xC0 },
+   { 0x1101, 0x02 },
{} /* Terminator */
 };
 
@@ -1580,21 +1598,180 @@ static void cs8409_cs42l42_reset(struct hda_codec 
*codec)
/* wait ~10ms */
usleep_range(1, 15000);
 
-   /* Clear interrupts status */
+   mutex_lock(&cs8409_i2c_mux);
+
+   /* Clear interrupts, by reading interrupt status registers */
cs8409_i2c_read(codec, CS42L42_I2C_ADDR, 0x1308, 1);
cs8409_i2c_read(codec, CS42L42_I2C_ADDR, 0x1309, 1);
cs8409_i2c_read(codec, CS42L42_I2C_ADDR, 0x130A, 1);
cs8409_i2c_read(codec, CS42L42_I2C_ADDR, 0x130F, 1);
 
+   mutex_unlock(&cs8409_i2c_mux);
+
+}
+
+/* Configure CS42L42 slave codec for jack autodetect */
+static int cs8409_cs42l42_enable_jack_detect(struct hda_codec *codec)
+{
+   mutex_lock(&cs8409_i2c_mux);
+
+   /* Set TIP_SENSE_EN for analog front-end of tip sense. */
+   cs8409_i2c_write(codec, CS42L42_I2C_ADDR, 0x1b70, 0x0020, 1);
+   /* Clear WAKE# */
+   cs8409_i2c_write(codec, CS42L42_I2C_ADDR, 0x1b71, 0x0001, 1);
+   /* Wait ~2.5ms */
+   usleep_range(2500, 3000);
+   /* Set mode WAKE# output follows the combination logic directly */
+   cs8409_i2c_write(codec, CS42L42_I2C_ADDR, 0x1b71, 0x0020, 1);
+   /* Clear interrupts status */
+   cs8409_i2c_read(codec, CS42L42_I2C_ADDR, 0x130f, 1);
+   cs8409_i2c_read(codec, CS42L42_I2C_ADDR, 0x1b7b, 1);
+   /* Enable interrupt */
+   cs8409_i2c_write(codec, CS42L42_I2C_ADDR, 0x1320, 0x03, 1);
+   cs8409_i2c_write(codec, CS42L42_I2C_ADDR, 0x1b79, 0x00, 1);
+
+   mutex_unlock(&cs8409_i2c_mux);
+
+   return 0;
+}
+
+/* Enable and run CS42L42 slave codec jack auto detect */
+static void cs8409_cs42l42_run_jack_detect(struct hda_codec *codec)
+{
+   mutex_lock(&cs8409_i2c_mux);
+
+   /* Clear interrupts */
+   cs8409_i2c_read(codec, CS42L42_I2C_ADDR, 0x1308, 1);
+   cs8409_i2c_read(codec, CS42L42_I2C_ADDR, 0x1b77, 1);
+
+   cs8409_i2c_write(codec, CS42L42_I2C_ADDR, 0x1102, 0x87, 1);
+   cs8409_i2c_write(codec, CS42L42_I2C_ADDR, 0x1f06, 0x86, 1);
+   cs8409_i2c_write(codec, CS42L42_I2C_ADDR, 0x1b74, 0x07, 1);
+   cs8409_i2c_write(codec, CS42L42_I2C_ADDR, 0x131b, 0x01, 1);
+   cs8409_i2c_write(codec, CS42L42_I2C_ADDR, 0x1120, 0x80, 1);
+   /* Wait ~110ms*/
+   usleep_range(11, 20);
+   cs8409_i2c_write(codec, CS42L42_I2C_ADDR, 0x111f, 0x77, 1);
+   cs8409_i2c_write(codec, CS42L42_I2C_ADDR, 0x1120, 0xc0, 1);
+   /* Wait ~10ms */
+   usleep_range(1, 25000);
+
+   mutex_unlock(&cs8409_i2c_mux);
+
 }
 
 static void cs8409_cs42l42_reg_setup(struct hda_codec *codec)
 {
const struct cs8409_i2c_p

Re: [PATCH 3/4] ALSA: hda/cirrus: Add jack detect interrupt support from CS42L42 companion codec.

2021-03-04 Thread Takashi Iwai
On Wed, 03 Mar 2021 19:29:58 +0100,
Vitaly Rodionov wrote:
> @@ -1243,6 +1258,8 @@ static int patch_cs4213(struct hda_codec *codec)
>  #define CIR_I2C_QWRITE   0x005D
>  #define CIR_I2C_QREAD0x005E
>  
> +static struct mutex cs8409_i2c_mux;

Any reason that this must be the global mutex?  I suppose it can fit
in own spec->i2c_mutex instead?


> +static void cs8409_cs42l42_cap_sync_hook(struct hda_codec *codec,
> +  struct snd_kcontrol *kcontrol,
> +  struct snd_ctl_elem_value *ucontrol)
> +{
> + struct cs_spec *spec = codec->spec;
> + unsigned int curval, expval;
> + /* CS8409 DMIC Pin only allows the setting of the Stream Parameters in
> +  * Power State D0. When a headset is unplugged, and the path is 
> switched to
> +  * the DMIC, the Stream is restarted with the new ADC, but this is done 
> in
> +  * Power State D3. Restart the Stream now DMIC is in D0.
> +  */
> + if (spec->gen.cur_adc == CS8409_CS42L42_DMIC_ADC_PIN_NID) {
> + curval = snd_hda_codec_read(codec, spec->gen.cur_adc,
> + 0, AC_VERB_GET_CONV, 0);
> + expval = (spec->gen.cur_adc_stream_tag << 4) | 0;
> + if (curval != expval) {
> + codec_dbg(codec, "%s Restarting Stream after DMIC 
> switch\n", __func__);
> + __snd_hda_codec_cleanup_stream(codec, 
> spec->gen.cur_adc, 1);
> + snd_hda_codec_setup_stream(codec, spec->gen.cur_adc,
> +spec->gen.cur_adc_stream_tag, 0,
> +spec->gen.cur_adc_format);

Hrm, this looks a big scary.  We may need to reconsider how to handle
this better later, but it's OK as long as you've tested with this
code...

> +static int cs8409_cs42l42_init(struct hda_codec *codec)
> +{
> + int ret = 0;
> +
> + ret = snd_hda_gen_init(codec);
> +
> + if (!ret) {
> + /* On Dell platforms with suspend D3 mode support we
> +  * have to re-initialise cs8409 bridge and companion
> +  * cs42l42 codec
> +  */
> + snd_hda_sequence_write(codec, cs8409_cs42l42_init_verbs);
> + snd_hda_sequence_write(codec, cs8409_cs42l42_add_verbs);
> +
> + cs8409_cs42l42_hw_init(codec);

Ah... the init stuff at resume appears finally here.  This part should
be in the second patch instead.

> +static int cs8409_cs42l42_exec_verb(struct hdac_device *dev,
> + unsigned int cmd, unsigned int flags, unsigned int *res)
> +{
> + struct hda_codec *codec = container_of(dev, struct hda_codec, core);
> + struct cs_spec *spec = codec->spec;
> +
> + unsigned int nid = 0;
> + unsigned int verb = 0;

The blank line above should be removed.

> + case CS8409_CS42L42_HP_PIN_NID:
> + if (verb == AC_VERB_GET_PIN_SENSE) {
> + *res = 
> (spec->cs42l42_hp_jack_in)?AC_PINSENSE_PRESENCE:0;

The spaces are needed around operators.
Similar coding-style issues are seen other places.  Please try to run
scripts/checkpatch.pl.


thanks,

Takashi


Re: [PATCH 3/4] ALSA: hda/cirrus: Add jack detect interrupt support from CS42L42 companion codec.

2021-03-04 Thread Vitaly Rodionov

On 04/03/2021 1:45 pm, Takashi Iwai wrote:

On Wed, 03 Mar 2021 19:29:58 +0100,
Vitaly Rodionov wrote:

@@ -1243,6 +1258,8 @@ static int patch_cs4213(struct hda_codec *codec)
  #define CIR_I2C_QWRITE0x005D
  #define CIR_I2C_QREAD 0x005E
  
+static struct mutex cs8409_i2c_mux;

Any reason that this must be the global mutex?  I suppose it can fit
in own spec->i2c_mutex instead?

No,  there is no reason to have global mutex, will move it out to spec.




+static void cs8409_cs42l42_cap_sync_hook(struct hda_codec *codec,
+struct snd_kcontrol *kcontrol,
+struct snd_ctl_elem_value *ucontrol)
+{
+   struct cs_spec *spec = codec->spec;
+   unsigned int curval, expval;
+   /* CS8409 DMIC Pin only allows the setting of the Stream Parameters in
+* Power State D0. When a headset is unplugged, and the path is 
switched to
+* the DMIC, the Stream is restarted with the new ADC, but this is done 
in
+* Power State D3. Restart the Stream now DMIC is in D0.
+*/
+   if (spec->gen.cur_adc == CS8409_CS42L42_DMIC_ADC_PIN_NID) {
+   curval = snd_hda_codec_read(codec, spec->gen.cur_adc,
+   0, AC_VERB_GET_CONV, 0);
+   expval = (spec->gen.cur_adc_stream_tag << 4) | 0;
+   if (curval != expval) {
+   codec_dbg(codec, "%s Restarting Stream after DMIC 
switch\n", __func__);
+   __snd_hda_codec_cleanup_stream(codec, 
spec->gen.cur_adc, 1);
+   snd_hda_codec_setup_stream(codec, spec->gen.cur_adc,
+  spec->gen.cur_adc_stream_tag, 0,
+  spec->gen.cur_adc_format);

Hrm, this looks a big scary.  We may need to reconsider how to handle
this better later, but it's OK as long as you've tested with this
code...


We have been thinking about this code, and we have some ideas , it was 
tested by us, DELL and Canonical and works.


But we would like to change it a bit later, and handle it in a more 
generic way.





+static int cs8409_cs42l42_init(struct hda_codec *codec)
+{
+   int ret = 0;
+
+   ret = snd_hda_gen_init(codec);
+
+   if (!ret) {
+   /* On Dell platforms with suspend D3 mode support we
+* have to re-initialise cs8409 bridge and companion
+* cs42l42 codec
+*/
+   snd_hda_sequence_write(codec, cs8409_cs42l42_init_verbs);
+   snd_hda_sequence_write(codec, cs8409_cs42l42_add_verbs);
+
+   cs8409_cs42l42_hw_init(codec);

Ah... the init stuff at resume appears finally here.  This part should
be in the second patch instead.

Yes, moving this into second patch.



+static int cs8409_cs42l42_exec_verb(struct hdac_device *dev,
+   unsigned int cmd, unsigned int flags, unsigned int *res)
+{
+   struct hda_codec *codec = container_of(dev, struct hda_codec, core);
+   struct cs_spec *spec = codec->spec;
+
+   unsigned int nid = 0;
+   unsigned int verb = 0;

The blank line above should be removed.


+   case CS8409_CS42L42_HP_PIN_NID:
+   if (verb == AC_VERB_GET_PIN_SENSE) {
+   *res = 
(spec->cs42l42_hp_jack_in)?AC_PINSENSE_PRESENCE:0;

The spaces are needed around operators.
Similar coding-style issues are seen other places.  Please try to run
scripts/checkpatch.pl.


Fixed, and checked with scripts/checkpatch.pl. Base has 19 warnings. 
This patch will not introduce any new.





thanks,

Takashi


Thanks,

Vitaly