[RESEND][PATCH] ASoC: hdac_hdmi: Program pin-port-mux during jack report event

[RESEND][PATCH] ASoC: hdac_hdmi: Program pin-port-mux during jack report event

[Sriram Periyasamy]

During d3/d0 cycle, the connection selection index of all pins points
to the default value. This needs to be restored to ensure audio is
restored after d3/d0 cycle.

So store the connection selection index and program it during jack
report event which gets invoked in cases like d3/d0 cycle, hot plug
detection when multiple displays are connected.

Signed-off-by: Sriram Periyasamy <sriramx.periyasamy@intel.com>
Signed-off-by: Sanyog Kale <sanyog.r.kale@intel.com>
Signed-off-by: Jeeja KP <jeeja.kp@intel.com>
Signed-off-by: Guneshwor Singh <guneshwor.o.singh@intel.com>
Acked-by: Vinod Koul <vkoul@kernel.org>
---
 sound/soc/codecs/hdac_hdmi.c | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/sound/soc/codecs/hdac_hdmi.c b/sound/soc/codecs/hdac_hdmi.c
index 84f7a7a36e4b..7eb5a7138483 100644
--- a/sound/soc/codecs/hdac_hdmi.c
+++ b/sound/soc/codecs/hdac_hdmi.c
@@ -32,6 +32,7 @@
 #include <sound/hda_chmap.h>
 #include "../../hda/local.h"
 #include "hdac_hdmi.h"
+#include <sound/hda_regmap.h>
 
 #define NAME_SIZE	32
 
@@ -83,6 +84,7 @@ struct hdac_hdmi_pin {
 	struct list_head head;
 	hda_nid_t nid;
 	bool mst_capable;
+	int conn_index;
 	struct hdac_hdmi_port *ports;
 	int num_ports;
 	struct hdac_ext_device *edev;
@@ -141,6 +143,9 @@ struct hdac_hdmi_priv {
 
 #define hdev_to_hdmi_priv(_hdev) ((to_ehdac_device(_hdev))->private_data)
 
+static int hdac_hdmi_port_select_set(struct hdac_ext_device *edev,
+					struct hdac_hdmi_port *port);
+
 static struct hdac_hdmi_pcm *
 hdac_hdmi_get_pcm_from_cvt(struct hdac_hdmi_priv *hdmi,
 			   struct hdac_hdmi_cvt *cvt)
@@ -159,6 +164,7 @@ static void hdac_hdmi_jack_report(struct hdac_hdmi_pcm *pcm,
 		struct hdac_hdmi_port *port, bool is_connect)
 {
 	struct hdac_ext_device *edev = port->pin->edev;
+	int cmd, err;
 
 	if (is_connect)
 		snd_soc_dapm_enable_pin(port->dapm, port->jack_pin);
@@ -166,6 +172,29 @@ static void hdac_hdmi_jack_report(struct hdac_hdmi_pcm *pcm,
 		snd_soc_dapm_disable_pin(port->dapm, port->jack_pin);
 
 	if (is_connect) {
+		/* set the device if pin is mst_capable */
+		if (hdac_hdmi_port_select_set(edev, port) < 0) {
+			dev_err(&edev->hdev.dev,
+				"port %d device select fail\n", port->id);
+			return;
+		}
+		/*
+		 * Restore the connection selection index of the
+		 * respective pin.
+		 */
+		if (port->pin->conn_index > 0) {
+			cmd = snd_hdac_regmap_encode_verb(port->pin->nid,
+						AC_VERB_SET_CONNECT_SEL);
+			err = snd_hdac_regmap_write_raw(&edev->hdev, cmd,
+						port->pin->conn_index - 1);
+			if (err < 0) {
+				dev_err(&edev->hdev.dev,
+					"pin %d conn select index fail %d\n",
+					port->pin->nid, err);
+				return;
+			}
+		}
+
 		/*
 		 * Report Jack connect event when a device is connected
 		 * for the first time where same PCM is attached to multiple
@@ -903,6 +932,9 @@ static int hdac_hdmi_set_pin_port_mux(struct snd_kcontrol *kcontrol,
 		}
 	}
 
+	if (ucontrol->value.enumerated.item[0] > 0)
+		port->pin->conn_index =	ucontrol->value.enumerated.item[0];
+
 	/*
 	 * Jack status is not reported during device probe as the
 	 * PCMs are not registered by then. So report it here.
-- 
2.7.4

Re: [RESEND][PATCH] ASoC: hdac_hdmi: Program pin-port-mux during jack report event

[Takashi Iwai]

On Wed, 20 Jun 2018 13:08:21 +0200,
Sriram Periyasamy wrote:
> 
> During d3/d0 cycle, the connection selection index of all pins points
> to the default value. This needs to be restored to ensure audio is
> restored after d3/d0 cycle.
> 
> So store the connection selection index and program it during jack
> report event which gets invoked in cases like d3/d0 cycle, hot plug
> detection when multiple displays are connected.

Hm, I thought *_CONNECT_SEL being cached and restored at resume
already by regmap?

*_DEVICE_SEL is declared as volatile, so you'd need to restore it
manually, yes.


thanks,

Takashi

> 
> Signed-off-by: Sriram Periyasamy <sriramx.periyasamy@intel.com>
> Signed-off-by: Sanyog Kale <sanyog.r.kale@intel.com>
> Signed-off-by: Jeeja KP <jeeja.kp@intel.com>
> Signed-off-by: Guneshwor Singh <guneshwor.o.singh@intel.com>
> Acked-by: Vinod Koul <vkoul@kernel.org>
> ---
>  sound/soc/codecs/hdac_hdmi.c | 32 ++++++++++++++++++++++++++++++++
>  1 file changed, 32 insertions(+)
> 
> diff --git a/sound/soc/codecs/hdac_hdmi.c b/sound/soc/codecs/hdac_hdmi.c
> index 84f7a7a36e4b..7eb5a7138483 100644
> --- a/sound/soc/codecs/hdac_hdmi.c
> +++ b/sound/soc/codecs/hdac_hdmi.c
> @@ -32,6 +32,7 @@
>  #include <sound/hda_chmap.h>
>  #include "../../hda/local.h"
>  #include "hdac_hdmi.h"
> +#include <sound/hda_regmap.h>
>  
>  #define NAME_SIZE	32
>  
> @@ -83,6 +84,7 @@ struct hdac_hdmi_pin {
>  	struct list_head head;
>  	hda_nid_t nid;
>  	bool mst_capable;
> +	int conn_index;
>  	struct hdac_hdmi_port *ports;
>  	int num_ports;
>  	struct hdac_ext_device *edev;
> @@ -141,6 +143,9 @@ struct hdac_hdmi_priv {
>  
>  #define hdev_to_hdmi_priv(_hdev) ((to_ehdac_device(_hdev))->private_data)
>  
> +static int hdac_hdmi_port_select_set(struct hdac_ext_device *edev,
> +					struct hdac_hdmi_port *port);
> +
>  static struct hdac_hdmi_pcm *
>  hdac_hdmi_get_pcm_from_cvt(struct hdac_hdmi_priv *hdmi,
>  			   struct hdac_hdmi_cvt *cvt)
> @@ -159,6 +164,7 @@ static void hdac_hdmi_jack_report(struct hdac_hdmi_pcm *pcm,
>  		struct hdac_hdmi_port *port, bool is_connect)
>  {
>  	struct hdac_ext_device *edev = port->pin->edev;
> +	int cmd, err;
>  
>  	if (is_connect)
>  		snd_soc_dapm_enable_pin(port->dapm, port->jack_pin);
> @@ -166,6 +172,29 @@ static void hdac_hdmi_jack_report(struct hdac_hdmi_pcm *pcm,
>  		snd_soc_dapm_disable_pin(port->dapm, port->jack_pin);
>  
>  	if (is_connect) {
> +		/* set the device if pin is mst_capable */
> +		if (hdac_hdmi_port_select_set(edev, port) < 0) {
> +			dev_err(&edev->hdev.dev,
> +				"port %d device select fail\n", port->id);
> +			return;
> +		}
> +		/*
> +		 * Restore the connection selection index of the
> +		 * respective pin.
> +		 */
> +		if (port->pin->conn_index > 0) {
> +			cmd = snd_hdac_regmap_encode_verb(port->pin->nid,
> +						AC_VERB_SET_CONNECT_SEL);
> +			err = snd_hdac_regmap_write_raw(&edev->hdev, cmd,
> +						port->pin->conn_index - 1);
> +			if (err < 0) {
> +				dev_err(&edev->hdev.dev,
> +					"pin %d conn select index fail %d\n",
> +					port->pin->nid, err);
> +				return;
> +			}
> +		}
> +
>  		/*
>  		 * Report Jack connect event when a device is connected
>  		 * for the first time where same PCM is attached to multiple
> @@ -903,6 +932,9 @@ static int hdac_hdmi_set_pin_port_mux(struct snd_kcontrol *kcontrol,
>  		}
>  	}
>  
> +	if (ucontrol->value.enumerated.item[0] > 0)
> +		port->pin->conn_index =	ucontrol->value.enumerated.item[0];
> +
>  	/*
>  	 * Jack status is not reported during device probe as the
>  	 * PCMs are not registered by then. So report it here.
> -- 
> 2.7.4
> 

Re: [RESEND][PATCH] ASoC: hdac_hdmi: Program pin-port-mux during jack report event

[Sriram Periyasamy]

On Wed, Jun 20, 2018 at 03:01:13PM +0200, Takashi Iwai wrote:
> On Wed, 20 Jun 2018 13:08:21 +0200,
> Sriram Periyasamy wrote:
> > 
> > During d3/d0 cycle, the connection selection index of all pins points
> > to the default value. This needs to be restored to ensure audio is
> > restored after d3/d0 cycle.
> > 
> > So store the connection selection index and program it during jack
> > report event which gets invoked in cases like d3/d0 cycle, hot plug
> > detection when multiple displays are connected.
> 
> Hm, I thought *_CONNECT_SEL being cached and restored at resume
> already by regmap?
> 

Yes, legacy HDA HDMI driver has this method whereas ASoC based HDAC HDMI
driver doesn't use regmap. Hence we needed this solution.

Thanks,
Sriram.

Re: [RESEND][PATCH] ASoC: hdac_hdmi: Program pin-port-mux during jack report event

[Takashi Iwai]

On Thu, 21 Jun 2018 09:54:20 +0200,
Sriram Periyasamy wrote:
> 
> On Wed, Jun 20, 2018 at 03:01:13PM +0200, Takashi Iwai wrote:
> > On Wed, 20 Jun 2018 13:08:21 +0200,
> > Sriram Periyasamy wrote:
> > > 
> > > During d3/d0 cycle, the connection selection index of all pins points
> > > to the default value. This needs to be restored to ensure audio is
> > > restored after d3/d0 cycle.
> > > 
> > > So store the connection selection index and program it during jack
> > > report event which gets invoked in cases like d3/d0 cycle, hot plug
> > > detection when multiple displays are connected.
> > 
> > Hm, I thought *_CONNECT_SEL being cached and restored at resume
> > already by regmap?
> > 
> 
> Yes, legacy HDA HDMI driver has this method whereas ASoC based HDAC HDMI
> driver doesn't use regmap. Hence we needed this solution.

If so, why not just use the standard snd_hdac_codec_write()?


thanks,

Takashi

Re: [RESEND][PATCH] ASoC: hdac_hdmi: Program pin-port-mux during jack report event

[Sriram Periyasamy]

On Thu, Jun 21, 2018 at 10:28:33AM +0200, Takashi Iwai wrote:
> On Thu, 21 Jun 2018 09:54:20 +0200,
> Sriram Periyasamy wrote:
> > 
> > On Wed, Jun 20, 2018 at 03:01:13PM +0200, Takashi Iwai wrote:
> > > On Wed, 20 Jun 2018 13:08:21 +0200,
> > > Sriram Periyasamy wrote:
> > > > 
> > > > During d3/d0 cycle, the connection selection index of all pins points
> > > > to the default value. This needs to be restored to ensure audio is
> > > > restored after d3/d0 cycle.
> > > > 
> > > > So store the connection selection index and program it during jack
> > > > report event which gets invoked in cases like d3/d0 cycle, hot plug
> > > > detection when multiple displays are connected.
> > > 
> > > Hm, I thought *_CONNECT_SEL being cached and restored at resume
> > > already by regmap?
> > > 
> > 
> > Yes, legacy HDA HDMI driver has this method whereas ASoC based HDAC HDMI
> > driver doesn't use regmap. Hence we needed this solution.
> 
> If so, why not just use the standard snd_hdac_codec_write()?
> 

Because snd_hdac_codec_write doesn't power up the device to send the verb
across the link when the device is runtime suspended.

Thanks,
Sriram.

Re: [RESEND][PATCH] ASoC: hdac_hdmi: Program pin-port-mux during jack report event

[Takashi Iwai]

On Thu, 21 Jun 2018 12:45:34 +0200,
Sriram Periyasamy wrote:
> 
> On Thu, Jun 21, 2018 at 10:28:33AM +0200, Takashi Iwai wrote:
> > On Thu, 21 Jun 2018 09:54:20 +0200,
> > Sriram Periyasamy wrote:
> > > 
> > > On Wed, Jun 20, 2018 at 03:01:13PM +0200, Takashi Iwai wrote:
> > > > On Wed, 20 Jun 2018 13:08:21 +0200,
> > > > Sriram Periyasamy wrote:
> > > > > 
> > > > > During d3/d0 cycle, the connection selection index of all pins points
> > > > > to the default value. This needs to be restored to ensure audio is
> > > > > restored after d3/d0 cycle.
> > > > > 
> > > > > So store the connection selection index and program it during jack
> > > > > report event which gets invoked in cases like d3/d0 cycle, hot plug
> > > > > detection when multiple displays are connected.
> > > > 
> > > > Hm, I thought *_CONNECT_SEL being cached and restored at resume
> > > > already by regmap?
> > > > 
> > > 
> > > Yes, legacy HDA HDMI driver has this method whereas ASoC based HDAC HDMI
> > > driver doesn't use regmap. Hence we needed this solution.
> > 
> > If so, why not just use the standard snd_hdac_codec_write()?
> > 
> 
> Because snd_hdac_codec_write doesn't power up the device to send the verb
> across the link when the device is runtime suspended.

Then why hdac_hdmi_port_select_set() would work?  It's using
snd_hdac_codec_write(), too.


Takashi

Re: [RESEND][PATCH] ASoC: hdac_hdmi: Program pin-port-mux during jack report event

[Sriram Periyasamy]

On Thu, Jun 21, 2018 at 01:14:28PM +0200, Takashi Iwai wrote:
> On Thu, 21 Jun 2018 12:45:34 +0200,
> Sriram Periyasamy wrote:
> > 
> > On Thu, Jun 21, 2018 at 10:28:33AM +0200, Takashi Iwai wrote:
> > > On Thu, 21 Jun 2018 09:54:20 +0200,
> > > Sriram Periyasamy wrote:
> > > > 
> > > > On Wed, Jun 20, 2018 at 03:01:13PM +0200, Takashi Iwai wrote:
> > > > > On Wed, 20 Jun 2018 13:08:21 +0200,
> > > > > Sriram Periyasamy wrote:
> > > > > > 
> > > > > > During d3/d0 cycle, the connection selection index of all pins points
> > > > > > to the default value. This needs to be restored to ensure audio is
> > > > > > restored after d3/d0 cycle.
> > > > > > 
> > > > > > So store the connection selection index and program it during jack
> > > > > > report event which gets invoked in cases like d3/d0 cycle, hot plug
> > > > > > detection when multiple displays are connected.
> > > > > 
> > > > > Hm, I thought *_CONNECT_SEL being cached and restored at resume
> > > > > already by regmap?
> > > > > 
> > > > 
> > > > Yes, legacy HDA HDMI driver has this method whereas ASoC based HDAC HDMI
> > > > driver doesn't use regmap. Hence we needed this solution.
> > > 
> > > If so, why not just use the standard snd_hdac_codec_write()?
> > > 
> > 
> > Because snd_hdac_codec_write doesn't power up the device to send the verb
> > across the link when the device is runtime suspended.
> 
> Then why hdac_hdmi_port_select_set() would work?  It's using
> snd_hdac_codec_write(), too.
>

We will check and come back on this.

Thanks,
Sriram.

Re: [RESEND][PATCH] ASoC: hdac_hdmi: Program pin-port-mux during jack report event

[Sriram Periyasamy]

On Fri, Jun 22, 2018 at 11:27:55AM +0530, Sriram Periyasamy wrote:
> On Thu, Jun 21, 2018 at 01:14:28PM +0200, Takashi Iwai wrote:
> > On Thu, 21 Jun 2018 12:45:34 +0200,
> > Sriram Periyasamy wrote:
> > > 
> > > On Thu, Jun 21, 2018 at 10:28:33AM +0200, Takashi Iwai wrote:
> > > > On Thu, 21 Jun 2018 09:54:20 +0200,
> > > > Sriram Periyasamy wrote:
> > > > > 
> > > > > On Wed, Jun 20, 2018 at 03:01:13PM +0200, Takashi Iwai wrote:
> > > > > > On Wed, 20 Jun 2018 13:08:21 +0200,
> > > > > > Sriram Periyasamy wrote:
> > > > > > > 
> > > > > > > During d3/d0 cycle, the connection selection index of all pins points
> > > > > > > to the default value. This needs to be restored to ensure audio is
> > > > > > > restored after d3/d0 cycle.
> > > > > > > 
> > > > > > > So store the connection selection index and program it during jack
> > > > > > > report event which gets invoked in cases like d3/d0 cycle, hot plug
> > > > > > > detection when multiple displays are connected.
> > > > > > 
> > > > > > Hm, I thought *_CONNECT_SEL being cached and restored at resume
> > > > > > already by regmap?
> > > > > > 
> > > > > 
> > > > > Yes, legacy HDA HDMI driver has this method whereas ASoC based HDAC HDMI
> > > > > driver doesn't use regmap. Hence we needed this solution.
> > > > 
> > > > If so, why not just use the standard snd_hdac_codec_write()?
> > > > 
> > > 
> > > Because snd_hdac_codec_write doesn't power up the device to send the verb
> > > across the link when the device is runtime suspended.
> > 
> > Then why hdac_hdmi_port_select_set() would work?  It's using
> > snd_hdac_codec_write(), too.
> >
> 
> We will check and come back on this.
>

In case of MST, it will work since snd_hdac_read_parm_uncached() wakes up
the device and put the device to runtime auto suspend. The auto suspend
delay was helping this snd_hdac_codec_write() pass through.

In case of non-MST, hdac_hdmi_port_select_set() will simply return 0 by
checking the MST capable port->pin->mst_capable. Hence the device is not
runtime resumed and subsequent calls of snd_hdac_codec_write() will fail.

We will post v2 to address this.

Thanks,
Sriram.