All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/5] Stripe control functionality
@ 2019-01-11 17:22 Sameer Pujar
  2019-01-11 17:22 ` [PATCH v2 1/5] ALSA: hda: add verbs for stripe control Sameer Pujar
                   ` (5 more replies)
  0 siblings, 6 replies; 12+ messages in thread
From: Sameer Pujar @ 2019-01-11 17:22 UTC (permalink / raw)
  To: perex, tiwai, broonie; +Cc: alsa-devel, Sameer Pujar

Background
==========

Azalia Controller fetches command and audio data via DMA and send them
to codec through SDO(Serial Data Out) lines. SDO is for outboung and it
broadcasts to all codecs. There is atleast one SDO line present, but
there can be multiple SDO lines supported for extended bandwidth. This
is essential when a platform supports multiple hdmi/dp sinks and there
is a requirement for higher resolution audio playback. In such cases
simultaneous audio playback data can be striped across multiple SDOs.

Global Capabilities(GCAP) Register indicates the capabilities of the
controller. Bits 2:1 of GCAP can be read to know the number of supported
SDO lines (below is from HD audio spec)
  00: 1 SDO
  01: 2 SDO
  10: 4 SDO
  11: Reserved

Stripe control verb reports and controls the stripe capability of an
Audio Output Converter. This verb needs to be implemented only for an
audio output converter and only if the stripe bit of the Audio Widget
Capabilities parameter is 1. 
Stripe control: get code(0xf24) and set code(0x724)

Change log
==========

v1-->v2:
-------- 
  Patch 1: "ALSA: hda: add verbs for stripe control"
    * no change

  Patch 2: "ALSA: hda: Add api to program stripe control bits"
    * no change

  Patch 3: "ALSA: hda: add register offset for stripe control"
    * added mask for stripe control programming

  Patch 4: "ALSA: hda: program stripe bits for controller"
    * used stripe control mask instead of hard coded value

  Patch 5: "ALSA: hda: program stripe control for codec"
    * added conditional check to know if striping is supported.
      If supported, then only stripe verb is implemented.

===========

Sameer Pujar (5):
  ALSA: hda: add verbs for stripe control
  ALSA: hda: Add api to program stripe control bits
  ALSA: hda: add register offset for stripe control
  ALSA: hda: program stripe bits for controller
  ALSA: hda: program stripe control for codec

 include/sound/hda_register.h |  2 ++
 include/sound/hda_verbs.h    |  2 ++
 include/sound/hdaudio.h      |  3 +++
 sound/hda/hdac_stream.c      | 40 ++++++++++++++++++++++++++++++++++++++++
 sound/pci/hda/patch_hdmi.c   | 10 +++++++++-
 5 files changed, 56 insertions(+), 1 deletion(-)

-- 
2.7.4


-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information.  Any unauthorized review, use, disclosure or distribution
is prohibited.  If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v2 1/5] ALSA: hda: add verbs for stripe control
  2019-01-11 17:22 [PATCH v2 0/5] Stripe control functionality Sameer Pujar
@ 2019-01-11 17:22 ` Sameer Pujar
  2019-01-11 17:22 ` [PATCH v2 2/5] ALSA: hda: Add api to program stripe control bits Sameer Pujar
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 12+ messages in thread
From: Sameer Pujar @ 2019-01-11 17:22 UTC (permalink / raw)
  To: perex, tiwai, broonie; +Cc: alsa-devel, Sameer Pujar

Controllers can support multiple Serial Data Out(SDO) lines, for
extended outbound bandwidth, to pump data to all codecs on the link.
Codecs can sample data present on SDO.

Add verbs AC_VERB_GET_STRIPE_CONTROL and AC_VERB_SET_STRIPE_CONTROL
These can be used to program usage of SDO lines for codec.

Signed-off-by: Sameer Pujar <spujar@nvidia.com>
Reviewed-by: Mohan Kumar D <mkumard@nvidia.com>
Reviewed-by: Ravindra Lokhande <rlokhande@nvidia.com>
---
 include/sound/hda_verbs.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/sound/hda_verbs.h b/include/sound/hda_verbs.h
index 2a8573a..e36b775 100644
--- a/include/sound/hda_verbs.h
+++ b/include/sound/hda_verbs.h
@@ -66,6 +66,7 @@ enum {
 #define AC_VERB_GET_CONFIG_DEFAULT		0x0f1c
 /* f20: AFG/MFG */
 #define AC_VERB_GET_SUBSYSTEM_ID		0x0f20
+#define AC_VERB_GET_STRIPE_CONTROL		0x0f24
 #define AC_VERB_GET_CVT_CHAN_COUNT		0x0f2d
 #define AC_VERB_GET_HDMI_DIP_SIZE		0x0f2e
 #define AC_VERB_GET_HDMI_ELDD			0x0f2f
@@ -110,6 +111,7 @@ enum {
 #define AC_VERB_SET_CONFIG_DEFAULT_BYTES_3	0x71f
 #define AC_VERB_SET_EAPD				0x788
 #define AC_VERB_SET_CODEC_RESET			0x7ff
+#define AC_VERB_SET_STRIPE_CONTROL		0x724
 #define AC_VERB_SET_CVT_CHAN_COUNT		0x72d
 #define AC_VERB_SET_HDMI_DIP_INDEX		0x730
 #define AC_VERB_SET_HDMI_DIP_DATA		0x731
-- 
2.7.4


-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information.  Any unauthorized review, use, disclosure or distribution
is prohibited.  If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2 2/5] ALSA: hda: Add api to program stripe control bits
  2019-01-11 17:22 [PATCH v2 0/5] Stripe control functionality Sameer Pujar
  2019-01-11 17:22 ` [PATCH v2 1/5] ALSA: hda: add verbs for stripe control Sameer Pujar
@ 2019-01-11 17:22 ` Sameer Pujar
  2019-01-11 17:55   ` Pierre-Louis Bossart
  2019-01-11 17:22 ` [PATCH v2 3/5] ALSA: hda: add register offset for stripe control Sameer Pujar
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 12+ messages in thread
From: Sameer Pujar @ 2019-01-11 17:22 UTC (permalink / raw)
  To: perex, tiwai, broonie; +Cc: alsa-devel, Sameer Pujar

Controllers and codecs can support striping of audio out across
multiple SDO lines. The number of supported SDO lines can be
specific to chip. GCAP register can be read to know the maximum
supported SDO lines.

snd_hdac_get_stream_stripe_ctl() is exposed to program stripe bits
on controller and codec side.
stripe value: 0 for 1SDO, 1 for 2SDO, 2 for 4SDO lines, etc.,

Signed-off-by: Sameer Pujar <spujar@nvidia.com>
Reviewed-by: Mohan Kumar D <mkumard@nvidia.com>
Reviewed-by: Ravindra Lokhande <rlokhande@nvidia.com>
---
 include/sound/hdaudio.h |  3 +++
 sound/hda/hdac_stream.c | 34 ++++++++++++++++++++++++++++++++++
 2 files changed, 37 insertions(+)

diff --git a/include/sound/hdaudio.h b/include/sound/hdaudio.h
index b4fa1c7..45f944d 100644
--- a/include/sound/hdaudio.h
+++ b/include/sound/hdaudio.h
@@ -539,6 +539,9 @@ void snd_hdac_stream_sync(struct hdac_stream *azx_dev, bool start,
 			  unsigned int streams);
 void snd_hdac_stream_timecounter_init(struct hdac_stream *azx_dev,
 				      unsigned int streams);
+int snd_hdac_get_stream_stripe_ctl(struct hdac_bus *bus,
+				struct snd_pcm_substream *substream);
+
 /*
  * macros for easy use
  */
diff --git a/sound/hda/hdac_stream.c b/sound/hda/hdac_stream.c
index eee4223..b403b05 100644
--- a/sound/hda/hdac_stream.c
+++ b/sound/hda/hdac_stream.c
@@ -13,6 +13,40 @@
 #include "trace.h"
 
 /**
+ * snd_hdac_get_stream_stripe_ctl - get stripe control value
+ * @bus: HD-audio core bus
+ * @substream: PCM substream
+ */
+int snd_hdac_get_stream_stripe_ctl(struct hdac_bus *bus,
+				   struct snd_pcm_substream *substream)
+{
+	struct snd_pcm_runtime *runtime = substream->runtime;
+	unsigned int channels = runtime->channels,
+		     rate = runtime->rate,
+		     bits_per_sample = runtime->sample_bits,
+		     max_sdo_lines, value, sdo_line;
+
+	/* T_AZA_GCAP_NSDO is 1:2 bitfields in GCAP */
+	max_sdo_lines = snd_hdac_chip_readl(bus, GCAP) & AZX_GCAP_NSDO;
+
+	/* following is from HD audio spec */
+	for (sdo_line = max_sdo_lines; sdo_line > 0; sdo_line >>= 1) {
+		if (rate > 48000)
+			value = (channels * bits_per_sample *
+					(rate / 48000)) / sdo_line;
+		else
+			value = channels * bits_per_sample / sdo_line;
+
+		if (value >= 8)
+			break;
+	}
+
+	/* stripe value: 0 for 1SDO, 1 for 2SDO, 2 for 4SDO lines */
+	return sdo_line >> 1;
+}
+EXPORT_SYMBOL_GPL(snd_hdac_get_stream_stripe_ctl);
+
+/**
  * snd_hdac_stream_init - initialize each stream (aka device)
  * @bus: HD-audio core bus
  * @azx_dev: HD-audio core stream object to initialize
-- 
2.7.4


-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information.  Any unauthorized review, use, disclosure or distribution
is prohibited.  If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2 3/5] ALSA: hda: add register offset for stripe control
  2019-01-11 17:22 [PATCH v2 0/5] Stripe control functionality Sameer Pujar
  2019-01-11 17:22 ` [PATCH v2 1/5] ALSA: hda: add verbs for stripe control Sameer Pujar
  2019-01-11 17:22 ` [PATCH v2 2/5] ALSA: hda: Add api to program stripe control bits Sameer Pujar
@ 2019-01-11 17:22 ` Sameer Pujar
  2019-01-11 17:22 ` [PATCH v2 4/5] ALSA: hda: program stripe bits for controller Sameer Pujar
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 12+ messages in thread
From: Sameer Pujar @ 2019-01-11 17:22 UTC (permalink / raw)
  To: perex, tiwai, broonie; +Cc: alsa-devel, Sameer Pujar

bits 16:17 in SD_CTL register refer to stripe control. Added an
offset register(AZX_REG_SD_CTL_3B) to have exclusive read/write
of corresponding register byte. This helps to avoid unnecessary
32-bit read/write of SD_CTL whenever only stripe or other bits of
corresponding byte need to be updated. Also HD audio spec defines
SD_CTL as 3 byte register.

SD_CTL_STRIPE_MASK(0x3) can be used for stripe control programming
and when updating AZX_REG_SD_CTL_3B.

Signed-off-by: Sameer Pujar <spujar@nvidia.com>
Reviewed-by: Mohan Kumar D <mkumard@nvidia.com>
Reviewed-by: Ravindra Lokhande <rlokhande@nvidia.com>
---
 include/sound/hda_register.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/sound/hda_register.h b/include/sound/hda_register.h
index 2ab39fb..0fd3929 100644
--- a/include/sound/hda_register.h
+++ b/include/sound/hda_register.h
@@ -79,6 +79,7 @@ enum { SDI0, SDI1, SDI2, SDI3, SDO0, SDO1, SDO2, SDO3 };
 
 /* stream register offsets from stream base */
 #define AZX_REG_SD_CTL			0x00
+#define AZX_REG_SD_CTL_3B		0x02 /* 3rd byte of SD_CTL register */
 #define AZX_REG_SD_STS			0x03
 #define AZX_REG_SD_LPIB			0x04
 #define AZX_REG_SD_CBL			0x08
@@ -165,6 +166,7 @@ enum { SDI0, SDI1, SDI2, SDI3, SDO0, SDO1, SDO2, SDO3 };
 #define SD_INT_COMPLETE		0x04	/* completion interrupt */
 #define SD_INT_MASK		(SD_INT_DESC_ERR|SD_INT_FIFO_ERR|\
 				 SD_INT_COMPLETE)
+#define SD_CTL_STRIPE_MASK	0x3	/* stripe control mask */
 
 /* SD_STS */
 #define SD_STS_FIFO_READY	0x20	/* FIFO ready */
-- 
2.7.4


-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information.  Any unauthorized review, use, disclosure or distribution
is prohibited.  If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2 4/5] ALSA: hda: program stripe bits for controller
  2019-01-11 17:22 [PATCH v2 0/5] Stripe control functionality Sameer Pujar
                   ` (2 preceding siblings ...)
  2019-01-11 17:22 ` [PATCH v2 3/5] ALSA: hda: add register offset for stripe control Sameer Pujar
@ 2019-01-11 17:22 ` Sameer Pujar
  2019-01-11 17:22 ` [PATCH v2 5/5] ALSA: hda: program stripe control for codec Sameer Pujar
  2019-01-11 18:02 ` [PATCH v2 0/5] Stripe control functionality Pierre-Louis Bossart
  5 siblings, 0 replies; 12+ messages in thread
From: Sameer Pujar @ 2019-01-11 17:22 UTC (permalink / raw)
  To: perex, tiwai, broonie; +Cc: alsa-devel, Sameer Pujar

Platforms having multiple hdmi/dp sinks require higher bandwidth
to support simultaneous playbacks of higher resolution. If hda
controller supports multiple SDO lines, STRIPE can be used to
indicate how many of the SDO lines the stream should be striped
across.

During stream start stripe control bits are programmed to use given
number of sdo lines and the same is cleared during stream stop.

Signed-off-by: Sameer Pujar <spujar@nvidia.com>
Reviewed-by: Mohan Kumar D <mkumard@nvidia.com>
Reviewed-by: Ravindra Lokhande <rlokhande@nvidia.com>
---
 sound/hda/hdac_stream.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/sound/hda/hdac_stream.c b/sound/hda/hdac_stream.c
index b403b05..7b71da0 100644
--- a/sound/hda/hdac_stream.c
+++ b/sound/hda/hdac_stream.c
@@ -82,6 +82,7 @@ EXPORT_SYMBOL_GPL(snd_hdac_stream_init);
 void snd_hdac_stream_start(struct hdac_stream *azx_dev, bool fresh_start)
 {
 	struct hdac_bus *bus = azx_dev->bus;
+	int stripe_ctl;
 
 	trace_snd_hdac_stream_start(bus, azx_dev);
 
@@ -91,6 +92,10 @@ void snd_hdac_stream_start(struct hdac_stream *azx_dev, bool fresh_start)
 
 	/* enable SIE */
 	snd_hdac_chip_updatel(bus, INTCTL, 0, 1 << azx_dev->index);
+	/* set stripe control */
+	stripe_ctl = snd_hdac_get_stream_stripe_ctl(bus, azx_dev->substream);
+	snd_hdac_stream_updateb(azx_dev, SD_CTL_3B, SD_CTL_STRIPE_MASK,
+				stripe_ctl);
 	/* set DMA start and interrupt mask */
 	snd_hdac_stream_updateb(azx_dev, SD_CTL,
 				0, SD_CTL_DMA_START | SD_INT_MASK);
@@ -107,6 +112,7 @@ void snd_hdac_stream_clear(struct hdac_stream *azx_dev)
 	snd_hdac_stream_updateb(azx_dev, SD_CTL,
 				SD_CTL_DMA_START | SD_INT_MASK, 0);
 	snd_hdac_stream_writeb(azx_dev, SD_STS, SD_INT_MASK); /* to be sure */
+	snd_hdac_stream_updateb(azx_dev, SD_CTL_3B, SD_CTL_STRIPE_MASK, 0);
 	azx_dev->running = false;
 }
 EXPORT_SYMBOL_GPL(snd_hdac_stream_clear);
-- 
2.7.4


-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information.  Any unauthorized review, use, disclosure or distribution
is prohibited.  If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2 5/5] ALSA: hda: program stripe control for codec
  2019-01-11 17:22 [PATCH v2 0/5] Stripe control functionality Sameer Pujar
                   ` (3 preceding siblings ...)
  2019-01-11 17:22 ` [PATCH v2 4/5] ALSA: hda: program stripe bits for controller Sameer Pujar
@ 2019-01-11 17:22 ` Sameer Pujar
  2019-01-11 18:02 ` [PATCH v2 0/5] Stripe control functionality Pierre-Louis Bossart
  5 siblings, 0 replies; 12+ messages in thread
From: Sameer Pujar @ 2019-01-11 17:22 UTC (permalink / raw)
  To: perex, tiwai, broonie; +Cc: alsa-devel, Sameer Pujar

Program codec stripe through AC_VERB_SET_STRIPE_CONTROL to use multiple
sdo lines if supported. Audio needs to be striped across number of sdo
lines for simultaneous playbacks of higher resolutions to work.
This needs to be implemented only for an Audio Output Converter and only
if the stripe bit(AC_WCAP_STRIPE) of Audio Widget Capabilities parameter
is 1.

Signed-off-by: Sameer Pujar <spujar@nvidia.com>
Reviewed-by: Mohan Kumar D <mkumard@nvidia.com>
Reviewed-by: Ravindra Lokhande <rlokhande@nvidia.com>
---
 sound/pci/hda/patch_hdmi.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
index 46f88dc..73d7042f 100644
--- a/sound/pci/hda/patch_hdmi.c
+++ b/sound/pci/hda/patch_hdmi.c
@@ -1865,7 +1865,7 @@ static int generic_hdmi_playback_pcm_prepare(struct hda_pcm_stream *hinfo,
 	hda_nid_t pin_nid;
 	struct snd_pcm_runtime *runtime = substream->runtime;
 	bool non_pcm;
-	int pinctl;
+	int pinctl, stripe;
 	int err = 0;
 
 	mutex_lock(&spec->pcm_lock);
@@ -1909,6 +1909,14 @@ static int generic_hdmi_playback_pcm_prepare(struct hda_pcm_stream *hinfo,
 	per_pin->channels = substream->runtime->channels;
 	per_pin->setup = true;
 
+	if (get_wcaps(codec, cvt_nid) & AC_WCAP_STRIPE) {
+		stripe = snd_hdac_get_stream_stripe_ctl(&codec->bus->core,
+							substream);
+		snd_hda_codec_write(codec, cvt_nid, 0,
+				    AC_VERB_SET_STRIPE_CONTROL,
+				    stripe);
+	}
+
 	hdmi_setup_audio_infoframe(codec, per_pin, non_pcm);
 	mutex_unlock(&per_pin->lock);
 	if (spec->dyn_pin_out) {
-- 
2.7.4


-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information.  Any unauthorized review, use, disclosure or distribution
is prohibited.  If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 2/5] ALSA: hda: Add api to program stripe control bits
  2019-01-11 17:22 ` [PATCH v2 2/5] ALSA: hda: Add api to program stripe control bits Sameer Pujar
@ 2019-01-11 17:55   ` Pierre-Louis Bossart
  2019-01-12  6:47     ` Sameer Pujar
  0 siblings, 1 reply; 12+ messages in thread
From: Pierre-Louis Bossart @ 2019-01-11 17:55 UTC (permalink / raw)
  To: Sameer Pujar, perex, tiwai, broonie; +Cc: alsa-devel


> +
> +	/* following is from HD audio spec */
> +	for (sdo_line = max_sdo_lines; sdo_line > 0; sdo_line >>= 1) {
> +		if (rate > 48000)
> +			value = (channels * bits_per_sample *
> +					(rate / 48000)) / sdo_line;
> +		else
> +			value = channels * bits_per_sample / sdo_line;
(channels * bit_per_sample) / sdo_line as in the spec?

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 0/5] Stripe control functionality
  2019-01-11 17:22 [PATCH v2 0/5] Stripe control functionality Sameer Pujar
                   ` (4 preceding siblings ...)
  2019-01-11 17:22 ` [PATCH v2 5/5] ALSA: hda: program stripe control for codec Sameer Pujar
@ 2019-01-11 18:02 ` Pierre-Louis Bossart
  2019-01-12  6:31   ` Sameer Pujar
  5 siblings, 1 reply; 12+ messages in thread
From: Pierre-Louis Bossart @ 2019-01-11 18:02 UTC (permalink / raw)
  To: Sameer Pujar, perex, tiwai, broonie; +Cc: alsa-devel


On 1/11/19 11:22 AM, Sameer Pujar wrote:
> Background
> ==========
>
> Azalia Controller fetches command and audio data via DMA and send them
> to codec through SDO(Serial Data Out) lines. SDO is for outboung and it
> broadcasts to all codecs. There is atleast one SDO line present, but
> there can be multiple SDO lines supported for extended bandwidth. This
> is essential when a platform supports multiple hdmi/dp sinks and there
> is a requirement for higher resolution audio playback. In such cases
> simultaneous audio playback data can be striped across multiple SDOs.
>
> Global Capabilities(GCAP) Register indicates the capabilities of the
> controller. Bits 2:1 of GCAP can be read to know the number of supported
> SDO lines (below is from HD audio spec)
>    00: 1 SDO
>    01: 2 SDO
>    10: 4 SDO
>    11: Reserved
>
> Stripe control verb reports and controls the stripe capability of an
> Audio Output Converter. This verb needs to be implemented only for an
> audio output converter and only if the stripe bit of the Audio Widget
> Capabilities parameter is 1.
> Stripe control: get code(0xf24) and set code(0x724)

the series look ok (one minor comment on operator precedence) and 
aligned with my understanding of the HDaudio 1.0a spec.

That said you made no mention of the Stripe bit  (figure 86) and fields 
22:20 (Stripe capability) in Figure 75, so it's not clear to me if the 
support added in this patchset is sufficient or if there is additional 
logic to be set.

There is also a difference between what the controller supports and the 
actual board layout, so it's not clear if the GCAP are really the raw 
capabilities or the ones filtered by BIOS folks to reflect the actual 
platform hardware implementation.

-Pierre


_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 0/5] Stripe control functionality
  2019-01-11 18:02 ` [PATCH v2 0/5] Stripe control functionality Pierre-Louis Bossart
@ 2019-01-12  6:31   ` Sameer Pujar
  2019-01-14 17:40     ` Pierre-Louis Bossart
  0 siblings, 1 reply; 12+ messages in thread
From: Sameer Pujar @ 2019-01-12  6:31 UTC (permalink / raw)
  To: Pierre-Louis Bossart, perex, tiwai, broonie; +Cc: alsa-devel


On 1/11/2019 11:32 PM, Pierre-Louis Bossart wrote:
>
> On 1/11/19 11:22 AM, Sameer Pujar wrote:
>> Background
>> ==========
>>
>> Azalia Controller fetches command and audio data via DMA and send them
>> to codec through SDO(Serial Data Out) lines. SDO is for outboung and it
>> broadcasts to all codecs. There is atleast one SDO line present, but
>> there can be multiple SDO lines supported for extended bandwidth. This
>> is essential when a platform supports multiple hdmi/dp sinks and there
>> is a requirement for higher resolution audio playback. In such cases
>> simultaneous audio playback data can be striped across multiple SDOs.
>>
>> Global Capabilities(GCAP) Register indicates the capabilities of the
>> controller. Bits 2:1 of GCAP can be read to know the number of supported
>> SDO lines (below is from HD audio spec)
>>    00: 1 SDO
>>    01: 2 SDO
>>    10: 4 SDO
>>    11: Reserved
>>
>> Stripe control verb reports and controls the stripe capability of an
>> Audio Output Converter. This verb needs to be implemented only for an
>> audio output converter and only if the stripe bit of the Audio Widget
>> Capabilities parameter is 1.
>> Stripe control: get code(0xf24) and set code(0x724)
>
> the series look ok (one minor comment on operator precedence) and 
> aligned with my understanding of the HDaudio 1.0a spec.
>
> That said you made no mention of the Stripe bit  (figure 86) and 
> fields 22:20 (Stripe capability) in Figure 75, so it's not clear to me 
> if the support added in this patchset is sufficient or if there is 
> additional logic to be set.
>
Stripe bit is part of Audio Widget Capability parameter and is mentioned 
above in the last section of background. Patch-5(v2) checks for the 
stripe bit, before sending stripe control verb.
As far as stripe control register is concerned (22:20), I don't think 
any SW programming is required for this.
> There is also a difference between what the controller supports and 
> the actual board layout, so it's not clear if the GCAP are really the 
> raw capabilities or the ones filtered by BIOS folks to reflect the 
> actual platform hardware implementation.
>
> -Pierre
>
>
-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information.  Any unauthorized review, use, disclosure or distribution
is prohibited.  If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 2/5] ALSA: hda: Add api to program stripe control bits
  2019-01-11 17:55   ` Pierre-Louis Bossart
@ 2019-01-12  6:47     ` Sameer Pujar
  2019-01-14 17:28       ` Pierre-Louis Bossart
  0 siblings, 1 reply; 12+ messages in thread
From: Sameer Pujar @ 2019-01-12  6:47 UTC (permalink / raw)
  To: Pierre-Louis Bossart, perex, tiwai, broonie; +Cc: alsa-devel


On 1/11/2019 11:25 PM, Pierre-Louis Bossart wrote:
>
>> +
>> +    /* following is from HD audio spec */
>> +    for (sdo_line = max_sdo_lines; sdo_line > 0; sdo_line >>= 1) {
>> +        if (rate > 48000)
>> +            value = (channels * bits_per_sample *
>> +                    (rate / 48000)) / sdo_line;
>> +        else
>> +            value = channels * bits_per_sample / sdo_line;
> (channels * bit_per_sample) / sdo_line as in the spec?

Though I have not used explicit braces, but as per operator precedence 
it will be treated the same way as it is mentioned in spec.
Let me know if you want this to be explicitly mentioned.

-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information.  Any unauthorized review, use, disclosure or distribution
is prohibited.  If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 2/5] ALSA: hda: Add api to program stripe control bits
  2019-01-12  6:47     ` Sameer Pujar
@ 2019-01-14 17:28       ` Pierre-Louis Bossart
  0 siblings, 0 replies; 12+ messages in thread
From: Pierre-Louis Bossart @ 2019-01-14 17:28 UTC (permalink / raw)
  To: Sameer Pujar, perex, tiwai, broonie; +Cc: alsa-devel


>>> +    /* following is from HD audio spec */
>>> +    for (sdo_line = max_sdo_lines; sdo_line > 0; sdo_line >>= 1) {
>>> +        if (rate > 48000)
>>> +            value = (channels * bits_per_sample *
>>> +                    (rate / 48000)) / sdo_line;
>>> +        else
>>> +            value = channels * bits_per_sample / sdo_line;
>> (channels * bit_per_sample) / sdo_line as in the spec?
>
> Though I have not used explicit braces, but as per operator precedence 
> it will be treated the same way as it is mentioned in spec.
> Let me know if you want this to be explicitly mentioned.

If you implement a spec it's better to either follow it verbatim or add 
a comment when you optimize parts away, that way reviewers understand 
your intent without having to refer to email archives. if you want to 
resend a v3 please add my tag for the series:

Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

Thanks!


_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 0/5] Stripe control functionality
  2019-01-12  6:31   ` Sameer Pujar
@ 2019-01-14 17:40     ` Pierre-Louis Bossart
  0 siblings, 0 replies; 12+ messages in thread
From: Pierre-Louis Bossart @ 2019-01-14 17:40 UTC (permalink / raw)
  To: Sameer Pujar, perex, tiwai, broonie; +Cc: alsa-devel


>>> Stripe control verb reports and controls the stripe capability of an
>>> Audio Output Converter. This verb needs to be implemented only for an
>>> audio output converter and only if the stripe bit of the Audio Widget
>>> Capabilities parameter is 1.
>>> Stripe control: get code(0xf24) and set code(0x724)
>>
>> the series look ok (one minor comment on operator precedence) and 
>> aligned with my understanding of the HDaudio 1.0a spec.
>>
>> That said you made no mention of the Stripe bit  (figure 86) and 
>> fields 22:20 (Stripe capability) in Figure 75, so it's not clear to 
>> me if the support added in this patchset is sufficient or if there is 
>> additional logic to be set.
>>
> Stripe bit is part of Audio Widget Capability parameter and is 
> mentioned above in the last section of background. Patch-5(v2) checks 
> for the stripe bit, before sending stripe control verb.

Ah yes, I thought this patchset added stripe control starting from 
scratch but some definitions were already present. Thanks for the precision.

> As far as stripe control register is concerned (22:20), I don't think 
> any SW programming is required for this.

Yes correct. this is on GET only so something the codec needs to worry 
about, not the driver.

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2019-01-14 17:40 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-11 17:22 [PATCH v2 0/5] Stripe control functionality Sameer Pujar
2019-01-11 17:22 ` [PATCH v2 1/5] ALSA: hda: add verbs for stripe control Sameer Pujar
2019-01-11 17:22 ` [PATCH v2 2/5] ALSA: hda: Add api to program stripe control bits Sameer Pujar
2019-01-11 17:55   ` Pierre-Louis Bossart
2019-01-12  6:47     ` Sameer Pujar
2019-01-14 17:28       ` Pierre-Louis Bossart
2019-01-11 17:22 ` [PATCH v2 3/5] ALSA: hda: add register offset for stripe control Sameer Pujar
2019-01-11 17:22 ` [PATCH v2 4/5] ALSA: hda: program stripe bits for controller Sameer Pujar
2019-01-11 17:22 ` [PATCH v2 5/5] ALSA: hda: program stripe control for codec Sameer Pujar
2019-01-11 18:02 ` [PATCH v2 0/5] Stripe control functionality Pierre-Louis Bossart
2019-01-12  6:31   ` Sameer Pujar
2019-01-14 17:40     ` Pierre-Louis Bossart

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.