[PATCH 0/4] updated patches for KMS audio

[PATCH 0/4] updated patches for KMS audio

[Zhenyu Wang]

I found my check for audio capability within CEA extension might
has some conflict with Adam's patch to add more detailed mode from
CEA ext block. But I haven't found Adam's patch in next tree, so I
just kept my current change.

This fixed incorrect probe for DP monitor's audio. Fengguang helped
to test this set. We got correct DP sound on Eizo EV2333W, and HDMI
audio also worked with no regression found.

Patch is against Chris's drm-intel-next branch.

thanks.

Zhenyu Wang (4):
  drm/edid: add helper function to detect monitor audio capability
  drm/i915: Enable DisplayPort audio
  drm/i915: Enable HDMI audio for monitor with audio support
  drm/i915: add new param to force audio on or off for HDMI/DP port

 drivers/gpu/drm/drm_edid.c        |   92 +++++++++++++++++++++++++++++++------
 drivers/gpu/drm/i915/i915_drv.c   |    3 +
 drivers/gpu/drm/i915/i915_drv.h   |    1 +
 drivers/gpu/drm/i915/intel_dp.c   |   64 +++++++++++++++----------
 drivers/gpu/drm/i915/intel_hdmi.c |   13 ++++--
 include/drm/drm_crtc.h            |    1 +
 6 files changed, 130 insertions(+), 44 deletions(-)

[PATCH 1/4] drm/edid: add helper function to detect monitor audio capability

[Zhenyu Wang]

To help to determine if digital display port needs to enable
audio output or not. This one adds a helper to get monitor's
audio capability via EDID CEA extension block.

Tested-by: Wu Fengguang <fengguang.wu@intel.com>
Signed-off-by: Zhenyu Wang <zhenyuw@linux.intel.com>
---
 drivers/gpu/drm/drm_edid.c |   92 +++++++++++++++++++++++++++++++++++++-------
 include/drm/drm_crtc.h     |    1 +
 2 files changed, 79 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 96e9631..7f356af 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -1268,34 +1268,51 @@ add_detailed_modes(struct drm_connector *connector, struct edid *edid,
 }
 
 #define HDMI_IDENTIFIER 0x000C03
+#define AUDIO_BLOCK	0x01
 #define VENDOR_BLOCK    0x03
+#define EDID_BASIC_AUDIO	(1 << 6)
+
 /**
- * drm_detect_hdmi_monitor - detect whether monitor is hdmi.
- * @edid: monitor EDID information
- *
- * Parse the CEA extension according to CEA-861-B.
- * Return true if HDMI, false if not or unknown.
+ * Search EDID for CEA extension block.
  */
-bool drm_detect_hdmi_monitor(struct edid *edid)
+static u8 *drm_find_cea_extension(struct edid *edid)
 {
-	char *edid_ext = NULL;
-	int i, hdmi_id;
-	int start_offset, end_offset;
-	bool is_hdmi = false;
+	u8 *edid_ext = NULL;
+	int i;
 
 	/* No EDID or EDID extensions */
 	if (edid == NULL || edid->extensions == 0)
-		goto end;
+		return NULL;
 
 	/* Find CEA extension */
 	for (i = 0; i < edid->extensions; i++) {
-		edid_ext = (char *)edid + EDID_LENGTH * (i + 1);
-		/* This block is CEA extension */
-		if (edid_ext[0] == 0x02)
+		edid_ext = (u8 *)edid + EDID_LENGTH * (i + 1);
+		if (edid_ext[0] == CEA_EXT)
 			break;
 	}
 
 	if (i == edid->extensions)
+		return NULL;
+
+	return edid_ext;
+}
+
+/**
+ * drm_detect_hdmi_monitor - detect whether monitor is hdmi.
+ * @edid: monitor EDID information
+ *
+ * Parse the CEA extension according to CEA-861-B.
+ * Return true if HDMI, false if not or unknown.
+ */
+bool drm_detect_hdmi_monitor(struct edid *edid)
+{
+	u8 *edid_ext;
+	int i, hdmi_id;
+	int start_offset, end_offset;
+	bool is_hdmi = false;
+
+	edid_ext = drm_find_cea_extension(edid);
+	if (!edid_ext)
 		goto end;
 
 	/* Data block offset in CEA extension block */
@@ -1326,6 +1343,53 @@ end:
 EXPORT_SYMBOL(drm_detect_hdmi_monitor);
 
 /**
+ * drm_detect_monitor_audio - check monitor audio capability
+ *
+ * Monitor should have CEA extension block.
+ * If monitor has 'basic audio', but no CEA audio blocks, it's 'basic
+ * audio' only. If there is any audio extension block and supported
+ * audio format, assume at least 'basic audio' support, even if 'basic
+ * audio' is not defined in EDID.
+ *
+ */
+bool drm_detect_monitor_audio(struct edid *edid)
+{
+	u8 *edid_ext;
+	int i, j;
+	bool has_audio = false;
+	int start_offset, end_offset;
+
+	edid_ext = drm_find_cea_extension(edid);
+	if (!edid_ext)
+		goto end;
+
+	has_audio = ((edid_ext[3] & EDID_BASIC_AUDIO) != 0);
+
+	if (has_audio) {
+		DRM_DEBUG_KMS("Monitor has basic audio support\n");
+		goto end;
+	}
+
+	/* Data block offset in CEA extension block */
+	start_offset = 4;
+	end_offset = edid_ext[2];
+
+	for (i = start_offset; i < end_offset;
+			i += ((edid_ext[i] & 0x1f) + 1)) {
+		if ((edid_ext[i] >> 5) == AUDIO_BLOCK) {
+			has_audio = true;
+			for (j = 1; j < (edid_ext[i] & 0x1f); j += 3)
+				DRM_DEBUG_KMS("CEA audio format %d\n",
+					      (edid_ext[i + j] >> 3) & 0xf);
+			goto end;
+		}
+	}
+end:
+	return has_audio;
+}
+EXPORT_SYMBOL(drm_detect_monitor_audio);
+
+/**
  * drm_add_edid_modes - add modes from EDID data, if available
  * @connector: connector we're probing
  * @edid: edid data
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index c9f3cc5..284ca90 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -754,6 +754,7 @@ extern int drm_mode_gamma_get_ioctl(struct drm_device *dev,
 extern int drm_mode_gamma_set_ioctl(struct drm_device *dev,
 				    void *data, struct drm_file *file_priv);
 extern bool drm_detect_hdmi_monitor(struct edid *edid);
+extern bool drm_detect_monitor_audio(struct edid *edid);
 extern int drm_mode_page_flip_ioctl(struct drm_device *dev,
 				    void *data, struct drm_file *file_priv);
 extern struct drm_display_mode *drm_cvt_mode(struct drm_device *dev,
-- 
1.7.1

[PATCH 2/4] drm/i915: Enable DisplayPort audio

[Zhenyu Wang]

This will turn on DP audio output by checking monitor's audio
capability.

Tested-by: Wu Fengguang <fengguang.wu@intel.com>
Signed-off-by: Zhenyu Wang <zhenyuw@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c |   61 +++++++++++++++++++++++----------------
 1 files changed, 36 insertions(+), 25 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 208a4ec..81fca1e 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1444,39 +1444,50 @@ intel_dp_detect(struct drm_connector *connector)
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	uint32_t temp, bit;
 	enum drm_connector_status status;
+	struct edid *edid = NULL;
 
 	intel_dp->has_audio = false;
 
-	if (HAS_PCH_SPLIT(dev))
-		return ironlake_dp_detect(connector);
+	if (HAS_PCH_SPLIT(dev)) {
+		status = ironlake_dp_detect(connector);
+	} else {
+		switch (intel_dp->output_reg) {
+		case DP_B:
+			bit = DPB_HOTPLUG_INT_STATUS;
+			break;
+		case DP_C:
+			bit = DPC_HOTPLUG_INT_STATUS;
+			break;
+		case DP_D:
+			bit = DPD_HOTPLUG_INT_STATUS;
+			break;
+		default:
+			return connector_status_unknown;
+		}
 
-	switch (intel_dp->output_reg) {
-	case DP_B:
-		bit = DPB_HOTPLUG_INT_STATUS;
-		break;
-	case DP_C:
-		bit = DPC_HOTPLUG_INT_STATUS;
-		break;
-	case DP_D:
-		bit = DPD_HOTPLUG_INT_STATUS;
-		break;
-	default:
-		return connector_status_unknown;
-	}
+		temp = I915_READ(PORT_HOTPLUG_STAT);
 
-	temp = I915_READ(PORT_HOTPLUG_STAT);
+		if ((temp & bit) == 0)
+			return connector_status_disconnected;
 
-	if ((temp & bit) == 0)
-		return connector_status_disconnected;
+		status = connector_status_disconnected;
+		if (intel_dp_aux_native_read(intel_dp, 0x000, intel_dp->dpcd,
+					     sizeof (intel_dp->dpcd)) == sizeof (intel_dp->dpcd))
+		{
+			if (intel_dp->dpcd[0] != 0)
+				status = connector_status_connected;
+		}
+	}
 
-	status = connector_status_disconnected;
-	if (intel_dp_aux_native_read(intel_dp,
-				     0x000, intel_dp->dpcd,
-				     sizeof (intel_dp->dpcd)) == sizeof (intel_dp->dpcd))
-	{
-		if (intel_dp->dpcd[0] != 0)
-			status = connector_status_connected;
+	if (status == connector_status_connected) {
+		edid = drm_get_edid(connector, intel_dp->base.ddc_bus);
+		if (edid) {
+			intel_dp->has_audio = drm_detect_monitor_audio(edid);
+			connector->display_info.raw_edid = NULL;
+			kfree(edid);
+		}
 	}
+
 	return status;
 }
 
-- 
1.7.1

[PATCH 3/4] drm/i915: Enable HDMI audio for monitor with audio support

[Zhenyu Wang]

Rely on monitor's audio capability to turn on audio output for HDMI.

Tested-by: Wu Fengguang <fengguang.wu@intel.com>
Signed-off-by: Zhenyu Wang <zhenyuw@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_hdmi.c |   12 ++++++++----
 1 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index 783924c..90184e6 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -41,6 +41,7 @@ struct intel_hdmi {
 	struct intel_encoder base;
 	u32 sdvox_reg;
 	bool has_hdmi_sink;
+	bool has_audio;
 };
 
 static struct intel_hdmi *enc_to_intel_hdmi(struct drm_encoder *encoder)
@@ -71,11 +72,12 @@ static void intel_hdmi_mode_set(struct drm_encoder *encoder,
 	if (adjusted_mode->flags & DRM_MODE_FLAG_PHSYNC)
 		sdvox |= SDVO_HSYNC_ACTIVE_HIGH;
 
-	if (intel_hdmi->has_hdmi_sink) {
+	/* Required on CPT */
+	if (intel_hdmi->has_hdmi_sink && HAS_PCH_CPT(dev))
+		sdvox |= HDMI_MODE_SELECT;
+
+	if (intel_hdmi->has_audio)
 		sdvox |= SDVO_AUDIO_ENABLE;
-		if (HAS_PCH_CPT(dev))
-			sdvox |= HDMI_MODE_SELECT;
-	}
 
 	if (intel_crtc->pipe == 1) {
 		if (HAS_PCH_CPT(dev))
@@ -152,12 +154,14 @@ intel_hdmi_detect(struct drm_connector *connector)
 	enum drm_connector_status status = connector_status_disconnected;
 
 	intel_hdmi->has_hdmi_sink = false;
+	intel_hdmi->has_audio = false;
 	edid = drm_get_edid(connector, intel_hdmi->base.ddc_bus);
 
 	if (edid) {
 		if (edid->input & DRM_EDID_INPUT_DIGITAL) {
 			status = connector_status_connected;
 			intel_hdmi->has_hdmi_sink = drm_detect_hdmi_monitor(edid);
+			intel_hdmi->has_audio = drm_detect_monitor_audio(edid);
 		}
 		connector->display_info.raw_edid = NULL;
 		kfree(edid);
-- 
1.7.1

[PATCH 4/4] drm/i915: add new param to force audio on or off for HDMI/DP port

[Zhenyu Wang]

Two reasons to add this param, one is for some AV device requiring
HDMI input for audio, but some device might not have sane EDID info
to enable audio on HDMI port. Another is for testing purpose.

Signed-off-by: Zhenyu Wang <zhenyuw@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c   |    3 +++
 drivers/gpu/drm/i915/i915_drv.h   |    1 +
 drivers/gpu/drm/i915/intel_dp.c   |    3 ++-
 drivers/gpu/drm/i915/intel_hdmi.c |    3 ++-
 4 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index dffc1bc..95277e5 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -48,6 +48,9 @@ module_param_named(powersave, i915_powersave, int, 0400);
 unsigned int i915_lvds_downclock = 0;
 module_param_named(lvds_downclock, i915_lvds_downclock, int, 0400);
 
+unsigned int i915_force_audio = -1;
+module_param_named(force_audio, i915_force_audio, int, 0400);
+
 static struct drm_driver driver;
 extern int intel_agp_enabled;
 
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index b0692c4..495182e 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -844,6 +844,7 @@ extern int i915_max_ioctl;
 extern unsigned int i915_fbpercrtc;
 extern unsigned int i915_powersave;
 extern unsigned int i915_lvds_downclock;
+extern unsigned int i915_force_audio;
 
 extern int i915_suspend(struct drm_device *dev, pm_message_t state);
 extern int i915_resume(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 81fca1e..77b45e7 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -736,7 +736,8 @@ intel_dp_mode_set(struct drm_encoder *encoder, struct drm_display_mode *mode,
 		intel_dp->DP |= DP_PORT_WIDTH_4;
 		break;
 	}
-	if (intel_dp->has_audio)
+	if (i915_force_audio == 1 ||
+	    (i915_force_audio != 0 && intel_dp->has_audio))
 		intel_dp->DP |= DP_AUDIO_OUTPUT_ENABLE;
 
 	memset(intel_dp->link_configuration, 0, DP_LINK_CONFIGURATION_SIZE);
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index 90184e6..20af171 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -76,7 +76,8 @@ static void intel_hdmi_mode_set(struct drm_encoder *encoder,
 	if (intel_hdmi->has_hdmi_sink && HAS_PCH_CPT(dev))
 		sdvox |= HDMI_MODE_SELECT;
 
-	if (intel_hdmi->has_audio)
+	if (i915_force_audio == 1 ||
+	    (i915_force_audio != 0 && intel_hdmi->has_audio))
 		sdvox |= SDVO_AUDIO_ENABLE;
 
 	if (intel_crtc->pipe == 1) {
-- 
1.7.1

Re: [Intel-gfx] [PATCH 1/4] drm/edid: add helper function to detect monitor audio capability

[Chris Wilson]

[Lets see if I have a working vpn connection today...]

On Sun, 19 Sep 2010 14:52:06 +0800, Zhenyu Wang <zhenyuw@linux.intel.com> wrote:
> To help to determine if digital display port needs to enable
> audio output or not. This one adds a helper to get monitor's
> audio capability via EDID CEA extension block.
> 
> Tested-by: Wu Fengguang <fengguang.wu@intel.com>
> Signed-off-by: Zhenyu Wang <zhenyuw@linux.intel.com>

This should be cc'ed for Adam Jackson's attention as well.

> ---
>  drivers/gpu/drm/drm_edid.c |   92 +++++++++++++++++++++++++++++++++++++-------
>  include/drm/drm_crtc.h     |    1 +
>  2 files changed, 79 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 96e9631..7f356af 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -1268,34 +1268,51 @@ add_detailed_modes(struct drm_connector *connector, struct edid *edid,
>  }
>  
>  #define HDMI_IDENTIFIER 0x000C03
> +#define AUDIO_BLOCK	0x01
>  #define VENDOR_BLOCK    0x03
> +#define EDID_BASIC_AUDIO	(1 << 6)
> +
>  /**
> + * drm_detect_monitor_audio - check monitor audio capability
> + *
> + * Monitor should have CEA extension block.
> + * If monitor has 'basic audio', but no CEA audio blocks, it's 'basic
> + * audio' only. If there is any audio extension block and supported
> + * audio format, assume at least 'basic audio' support, even if 'basic
> + * audio' is not defined in EDID.
> + *
> + */
> +bool drm_detect_monitor_audio(struct edid *edid)

drm_edid_has_monitor_audio()? That is a little more self-descriptive.
(I also think drm_detect_hdmi_monitor is poorly named.)

> +{
> +	u8 *edid_ext;
> +	int i, j;
> +	bool has_audio = false;
> +	int start_offset, end_offset;
> +
> +	edid_ext = drm_find_cea_extension(edid);
> +	if (!edid_ext)
> +		goto end;
> +
> +	has_audio = ((edid_ext[3] & EDID_BASIC_AUDIO) != 0);
Too many brackets do not lead to code clarity. ;-)
> +
> +	if (has_audio) {

The last time Adam had a chance to comment on this EDID check, he
suggested that we cannot rely on has_audio being a reliable indicator of
the sink's properties. If the EDID has no audio support, then return
early, otherwise perform the secondary check that extension block contains
audio data.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH 2/4] drm/i915: Enable DisplayPort audio

[Chris Wilson]

On Sun, 19 Sep 2010 14:52:07 +0800, Zhenyu Wang <zhenyuw@linux.intel.com> wrote:
> This will turn on DP audio output by checking monitor's audio
> capability.
> 
> Tested-by: Wu Fengguang <fengguang.wu@intel.com>
> Signed-off-by: Zhenyu Wang <zhenyuw@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c |   61 +++++++++++++++++++++++----------------
>  1 files changed, 36 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 208a4ec..81fca1e 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1444,39 +1444,50 @@ intel_dp_detect(struct drm_connector *connector)
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	uint32_t temp, bit;
>  	enum drm_connector_status status;
> +	struct edid *edid = NULL;
>  
>  	intel_dp->has_audio = false;
>  
> -	if (HAS_PCH_SPLIT(dev))
> -		return ironlake_dp_detect(connector);
> +	if (HAS_PCH_SPLIT(dev)) {
> +		status = ironlake_dp_detect(connector);
> +	} else {
> +		switch (intel_dp->output_reg) {
> +		case DP_B:
> +			bit = DPB_HOTPLUG_INT_STATUS;
> +			break;
> +		case DP_C:
> +			bit = DPC_HOTPLUG_INT_STATUS;
> +			break;
> +		case DP_D:
> +			bit = DPD_HOTPLUG_INT_STATUS;
> +			break;
> +		default:
> +			return connector_status_unknown;
> +		}
Move the switch and register check into a separate function, so
the main detect body becomes:
  if (HAS_PCH_SPLIT(dev))
    status = ironlake_dp_detect(connector);
  else
    status = g4x_dp_detect(connector);
  if (status != connector_status_connected)
    return status;

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [Intel-gfx] [PATCH 1/4] drm/edid: add helper function to detect monitor audio capability

[Zhenyu Wang]

[-- Attachment #1.1: Type: text/plain, Size: 1147 bytes --]

On 2010.09.19 08:38:07 +0100, Chris Wilson wrote:
> 
> This should be cc'ed for Adam Jackson's attention as well.

yep, I think I cc-ed ajax in git-send-mail command line...I'll amend the commit.

> > +bool drm_detect_monitor_audio(struct edid *edid)
> 
> drm_edid_has_monitor_audio()? That is a little more self-descriptive.
> (I also think drm_detect_hdmi_monitor is poorly named.)

yeah, that looks better. thanks.

> The last time Adam had a chance to comment on this EDID check, he
> suggested that we cannot rely on has_audio being a reliable indicator of
> the sink's properties. If the EDID has no audio support, then return
> early, otherwise perform the secondary check that extension block contains
> audio data.

That's also the problem we stuck for quite some time, and until recently
we got reply on this issue. Adam is right, 'basic audio' bit might not be
reliable, so we try to search audio block too, if there's any existence,
we will accept 'basic audio' support as well.

Thanks for the review!

-- 
Open Source Technology Center, Intel ltd.

$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/4] drm/i915: Enable DisplayPort audio

[Zhenyu Wang]

[-- Attachment #1.1: Type: text/plain, Size: 491 bytes --]

On 2010.09.19 08:43:30 +0100, Chris Wilson wrote:
> Move the switch and register check into a separate function, so
> the main detect body becomes:
>   if (HAS_PCH_SPLIT(dev))
>     status = ironlake_dp_detect(connector);
>   else
>     status = g4x_dp_detect(connector);
>   if (status != connector_status_connected)
>     return status;
> 

ok, will do that.

thanks.

-- 
Open Source Technology Center, Intel ltd.

$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 4/4] drm/i915: add new param to force audio on or off for HDMI/DP port

[Chris Wilson]

On Sun, 19 Sep 2010 14:52:09 +0800, Zhenyu Wang <zhenyuw@linux.intel.com> wrote:
> Two reasons to add this param, one is for some AV device requiring
> HDMI input for audio, but some device might not have sane EDID info
> to enable audio on HDMI port. Another is for testing purpose.

I was thinking along the lines of a connector property for force_audio.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH 4/4] drm/i915: add new param to force audio on or off for HDMI/DP port

[Zhenyu Wang]

[-- Attachment #1.1: Type: text/plain, Size: 607 bytes --]

On 2010.09.19 09:14:53 +0100, Chris Wilson wrote:
> On Sun, 19 Sep 2010 14:52:09 +0800, Zhenyu Wang <zhenyuw@linux.intel.com> wrote:
> > Two reasons to add this param, one is for some AV device requiring
> > HDMI input for audio, but some device might not have sane EDID info
> > to enable audio on HDMI port. Another is for testing purpose.
> 
> I was thinking along the lines of a connector property for force_audio.

oh, right, that's much better. sorry, I forgot about it completely. ;)

-- 
Open Source Technology Center, Intel ltd.

$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 4/4] drm/i915: add new param to force audio on or off for HDMI/DP port

[Chris Wilson]

On Sun, 19 Sep 2010 16:22:06 +0800, Zhenyu Wang <zhenyuw@linux.intel.com> wrote:
> On 2010.09.19 09:14:53 +0100, Chris Wilson wrote:
> > On Sun, 19 Sep 2010 14:52:09 +0800, Zhenyu Wang <zhenyuw@linux.intel.com> wrote:
> > > Two reasons to add this param, one is for some AV device requiring
> > > HDMI input for audio, but some device might not have sane EDID info
> > > to enable audio on HDMI port. Another is for testing purpose.
> > 
> > I was thinking along the lines of a connector property for force_audio.
> 
> oh, right, that's much better. sorry, I forgot about it completely. ;)

No worries, I've written up a couple of patches to add the connector
properties on top of your series. So just a couple of minor tweaks and
getting Adam's seal of approval for the core changes and I'll apply the
series to next. Thanks!
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH 0/4] updated patches for KMS audio

[Adam Jackson]

[-- Attachment #1.1: Type: text/plain, Size: 627 bytes --]

On Sun, 2010-09-19 at 14:52 +0800, Zhenyu Wang wrote:
> I found my check for audio capability within CEA extension might
> has some conflict with Adam's patch to add more detailed mode from
> CEA ext block. But I haven't found Adam's patch in next tree, so I
> just kept my current change.
> 
> This fixed incorrect probe for DP monitor's audio. Fengguang helped
> to test this set. We got correct DP sound on Eizo EV2333W, and HDMI
> audio also worked with no regression found.
> 
> Patch is against Chris's drm-intel-next branch.

Ignore my previous r-b.

Reviewed-by: Adam Jackson <ajax@redhat.com>

- ajax

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 0/4] updated patches for KMS audio

[Chris Wilson]

On Sun, 19 Sep 2010 14:52:05 +0800, Zhenyu Wang <zhenyuw@linux.intel.com> wrote:
> I found my check for audio capability within CEA extension might
> has some conflict with Adam's patch to add more detailed mode from
> CEA ext block. But I haven't found Adam's patch in next tree, so I
> just kept my current change.
> 
> This fixed incorrect probe for DP monitor's audio. Fengguang helped
> to test this set. We got correct DP sound on Eizo EV2333W, and HDMI
> audio also worked with no regression found.

Applied. Thanks for the reminder, Zhenyu.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [Intel-gfx] [PATCH 1/4] drm/edid: add helper function to detect monitor audio capability

[Ben Skeggs]

On Sun, 2010-09-19 at 15:56 +0800, Zhenyu Wang wrote:
> On 2010.09.19 08:38:07 +0100, Chris Wilson wrote:
> > 
> > This should be cc'ed for Adam Jackson's attention as well.
> 
> yep, I think I cc-ed ajax in git-send-mail command line...I'll amend the commit.
> 
> > > +bool drm_detect_monitor_audio(struct edid *edid)
> > 
> > drm_edid_has_monitor_audio()? That is a little more self-descriptive.
> > (I also think drm_detect_hdmi_monitor is poorly named.)
> 
> yeah, that looks better. thanks.
> 
> > The last time Adam had a chance to comment on this EDID check, he
> > suggested that we cannot rely on has_audio being a reliable indicator of
> > the sink's properties. If the EDID has no audio support, then return
> > early, otherwise perform the secondary check that extension block contains
> > audio data.
> 
> That's also the problem we stuck for quite some time, and until recently
> we got reply on this issue. Adam is right, 'basic audio' bit might not be
> reliable, so we try to search audio block too, if there's any existence,
> we will accept 'basic audio' support as well.
Also, would you be able to export drm_find_cea_extension(), I have
pending nouveau patches that will require this symbol (to build a HDA
ELD from the EDID data for HDMI/DP audio).

With that, you can add my Reviewed-by :)

Thanks,
Ben.
> 
> Thanks for the review!
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel