diff mbox

[v6,03/15] ASoC: hdac_hdmi: Enable DP1.2 and all converters/pins

Message ID 1455243375-16067-4-git-send-email-subhransu.s.prusty@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Subhransu S. Prusty Feb. 12, 2016, 2:16 a.m. UTC 
Skylake supports 3 pin and 3 converter widgets. But by default
only one converter and pin widget are enabled. In skylake
platform the DP port is on a different port which is not enabled
by default. To enable playback on DP port, enable all pin and
converter widget by sending a vendor VERB for a vendor widget to
set required bits.

As we are enabling the DP support enable the DP1.2 feature as well.

Enabling DP1.2 and all widget changes are copied from patch_hdmi.c.

Signed-off-by: Subhransu S. Prusty <subhransu.s.prusty@intel.com>
Signed-off-by: Vinod Koul <vinod.koul@intel.com>
---
 sound/soc/codecs/hdac_hdmi.c | 43 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 43 insertions(+)

Comments

Mark Brown Feb. 15, 2016, 8:10 p.m. UTC | #1 
On Fri, Feb 12, 2016 at 07:46:03AM +0530, Subhransu S. Prusty wrote:

> +static void hdac_hdmi_skl_enable_all_pins(struct hdac_device *hdac)
> +{
> +	unsigned int vendor_param;
> +
> +	vendor_param = snd_hdac_codec_read(hdac, INTEL_VENDOR_NID, 0,
> +				INTEL_GET_VENDOR_VERB, 0);
> +	if (vendor_param == -1 || vendor_param & INTEL_EN_ALL_PIN_CVTS)
> +		return;
> +
> +	vendor_param |= INTEL_EN_ALL_PIN_CVTS;
> +	vendor_param = snd_hdac_codec_read(hdac, INTEL_VENDOR_NID, 0,
> +				INTEL_SET_VENDOR_VERB, vendor_param);
> +	if (vendor_param == -1)
> +		return;
> +}

So to enable the pins we do a read?  That seems...  innovative.  :/
Takashi Iwai Feb. 15, 2016, 10:31 p.m. UTC | #2 
On Mon, 15 Feb 2016 21:10:49 +0100,
Mark Brown wrote:
> 
> On Fri, Feb 12, 2016 at 07:46:03AM +0530, Subhransu S. Prusty wrote:
> 
> > +static void hdac_hdmi_skl_enable_all_pins(struct hdac_device *hdac)
> > +{
> > +	unsigned int vendor_param;
> > +
> > +	vendor_param = snd_hdac_codec_read(hdac, INTEL_VENDOR_NID, 0,
> > +				INTEL_GET_VENDOR_VERB, 0);
> > +	if (vendor_param == -1 || vendor_param & INTEL_EN_ALL_PIN_CVTS)
> > +		return;
> > +
> > +	vendor_param |= INTEL_EN_ALL_PIN_CVTS;
> > +	vendor_param = snd_hdac_codec_read(hdac, INTEL_VENDOR_NID, 0,
> > +				INTEL_SET_VENDOR_VERB, vendor_param);
> > +	if (vendor_param == -1)
> > +		return;
> > +}
> 
> So to enable the pins we do a read?  That seems...  innovative.  :/

It's a weird nature of HD-audio verb handling.  While *_write() just
sends the verb asynchronously, *_read() sends the verb, does sync and
read-back the return value.  But both read and write may handle the
same verb.


Takashi
Mark Brown Feb. 16, 2016, 2 a.m. UTC | #3 
On Mon, Feb 15, 2016 at 11:31:48PM +0100, Takashi Iwai wrote:
> Mark Brown wrote:
> > On Fri, Feb 12, 2016 at 07:46:03AM +0530, Subhransu S. Prusty wrote:

> > > +	vendor_param |= INTEL_EN_ALL_PIN_CVTS;
> > > +	vendor_param = snd_hdac_codec_read(hdac, INTEL_VENDOR_NID, 0,
> > > +				INTEL_SET_VENDOR_VERB, vendor_param);
> > > +	if (vendor_param == -1)
> > > +		return;
> > > +}

> > So to enable the pins we do a read?  That seems...  innovative.  :/

> It's a weird nature of HD-audio verb handling.  While *_write() just
> sends the verb asynchronously, *_read() sends the verb, does sync and
> read-back the return value.  But both read and write may handle the
> same verb.

The above one seems especially odd, we do the read and then essentially
ignore the value (the difference in handling is nonexistant).
Takashi Iwai Feb. 16, 2016, 8:16 a.m. UTC | #4 
On Tue, 16 Feb 2016 03:00:26 +0100,
Mark Brown wrote:
> 
> On Mon, Feb 15, 2016 at 11:31:48PM +0100, Takashi Iwai wrote:
> > Mark Brown wrote:
> > > On Fri, Feb 12, 2016 at 07:46:03AM +0530, Subhransu S. Prusty wrote:
> 
> > > > +	vendor_param |= INTEL_EN_ALL_PIN_CVTS;
> > > > +	vendor_param = snd_hdac_codec_read(hdac, INTEL_VENDOR_NID, 0,
> > > > +				INTEL_SET_VENDOR_VERB, vendor_param);
> > > > +	if (vendor_param == -1)
> > > > +		return;
> > > > +}
> 
> > > So to enable the pins we do a read?  That seems...  innovative.  :/
> 
> > It's a weird nature of HD-audio verb handling.  While *_write() just
> > sends the verb asynchronously, *_read() sends the verb, does sync and
> > read-back the return value.  But both read and write may handle the
> > same verb.
> 
> The above one seems especially odd, we do the read and then essentially
> ignore the value (the difference in handling is nonexistant).

There is a difference -- it does sync.

I don't know whether the sync is mandatory in this case, though.


Takashi
diff mbox

Patch

diff --git a/sound/soc/codecs/hdac_hdmi.c b/sound/soc/codecs/hdac_hdmi.c
index d22fa6b..b2e5e54 100644
--- a/sound/soc/codecs/hdac_hdmi.c
+++ b/sound/soc/codecs/hdac_hdmi.c
@@ -629,6 +629,46 @@  static int hdac_hdmi_add_pin(struct hdac_ext_device *edev, hda_nid_t nid)
 	return 0;
 }
 
+#define INTEL_VENDOR_NID 0x08
+#define INTEL_GET_VENDOR_VERB 0xf81
+#define INTEL_SET_VENDOR_VERB 0x781
+#define INTEL_EN_DP12			0x02 /* enable DP 1.2 features */
+#define INTEL_EN_ALL_PIN_CVTS	0x01 /* enable 2nd & 3rd pins and convertors */
+
+static void hdac_hdmi_skl_enable_all_pins(struct hdac_device *hdac)
+{
+	unsigned int vendor_param;
+
+	vendor_param = snd_hdac_codec_read(hdac, INTEL_VENDOR_NID, 0,
+				INTEL_GET_VENDOR_VERB, 0);
+	if (vendor_param == -1 || vendor_param & INTEL_EN_ALL_PIN_CVTS)
+		return;
+
+	vendor_param |= INTEL_EN_ALL_PIN_CVTS;
+	vendor_param = snd_hdac_codec_read(hdac, INTEL_VENDOR_NID, 0,
+				INTEL_SET_VENDOR_VERB, vendor_param);
+	if (vendor_param == -1)
+		return;
+}
+
+static void hdac_hdmi_skl_enable_dp12(struct hdac_device *hdac)
+{
+	unsigned int vendor_param;
+
+	vendor_param = snd_hdac_codec_read(hdac, INTEL_VENDOR_NID, 0,
+				INTEL_GET_VENDOR_VERB, 0);
+	if (vendor_param == -1 || vendor_param & INTEL_EN_DP12)
+		return;
+
+	/* enable DP1.2 mode */
+	vendor_param |= INTEL_EN_DP12;
+	vendor_param = snd_hdac_codec_read(hdac, INTEL_VENDOR_NID, 0,
+				INTEL_SET_VENDOR_VERB, vendor_param);
+	if (vendor_param == -1)
+		return;
+
+}
+
 /*
  * Parse all nodes and store the cvt/pin nids in array
  * Add one time initialization for pin and cvt widgets
@@ -641,6 +681,9 @@  static int hdac_hdmi_parse_and_map_nid(struct hdac_ext_device *edev)
 	struct hdac_hdmi_priv *hdmi = edev->private_data;
 	int ret;
 
+	hdac_hdmi_skl_enable_all_pins(hdac);
+	hdac_hdmi_skl_enable_dp12(hdac);
+
 	num_nodes = snd_hdac_get_sub_nodes(hdac, hdac->afg, &nid);
 	if (!nid || num_nodes <= 0) {
 		dev_warn(&hdac->dev, "HDMI: failed to get afg sub nodes\n");