| Message ID | 1529492902-6600-1-git-send-email-sriramx.periyasamy@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 >
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.
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
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.
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
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.
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.
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.