[PATCH 1/2] drm/radeon/audio: don't set speaker allocation on DCE3.2

[PATCH 1/2] drm/radeon/audio: don't set speaker allocation on DCE3.2

[Alex Deucher]

It causes hangs on some asics.

Bug:
https://bugs.freedesktop.org/show_bug.cgi?id=70439

Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/radeon/r600_hdmi.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/radeon/r600_hdmi.c b/drivers/gpu/drm/radeon/r600_hdmi.c
index 5b72931..06022e3 100644
--- a/drivers/gpu/drm/radeon/r600_hdmi.c
+++ b/drivers/gpu/drm/radeon/r600_hdmi.c
@@ -309,6 +309,9 @@ static void dce3_2_afmt_write_speaker_allocation(struct drm_encoder *encoder)
 	u8 *sadb;
 	int sad_count;
 
+	/* XXX: setting this register causes hangs on some asics */
+	return;
+
 	list_for_each_entry(connector, &encoder->dev->mode_config.connector_list, head) {
 		if (connector->encoder == encoder)
 			radeon_connector = to_radeon_connector(connector);
-- 
1.8.3.1

[PATCH 2/2] drm/radeon: rework audio option

[Alex Deucher]

In 3.12 I changed audio to be enabled by default,
but you still had to turn it on via xrandr.  This
was confusing to users so change it to minic the
previous behavior:

- audio option is set to -1 (auto) by default which is
  the current 3.12 behavior (audio is enabled but requires
  xrandr to turn it on.
- if audio = 1, the audio is enabled without needing
  to mess with xrandr (previous behavior)
- audio = 0 disables audio

It retains the new feature of allowing the user to enable
audio on the fly with xrandr, but turns audio on
automatically if radeon.audio=1 is set which is what
most users expect.

Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/radeon/atombios_encoders.c | 52 ++++++++++++++++++++----------
 drivers/gpu/drm/radeon/radeon_connectors.c | 33 ++++++++++++-------
 drivers/gpu/drm/radeon/radeon_drv.c        |  4 +--
 3 files changed, 58 insertions(+), 31 deletions(-)

diff --git a/drivers/gpu/drm/radeon/atombios_encoders.c b/drivers/gpu/drm/radeon/atombios_encoders.c
index 2a761f2..5e891b2 100644
--- a/drivers/gpu/drm/radeon/atombios_encoders.c
+++ b/drivers/gpu/drm/radeon/atombios_encoders.c
@@ -707,24 +707,37 @@ atombios_get_encoder_mode(struct drm_encoder *encoder)
 	switch (connector->connector_type) {
 	case DRM_MODE_CONNECTOR_DVII:
 	case DRM_MODE_CONNECTOR_HDMIB: /* HDMI-B is basically DL-DVI; analog works fine */
-		if ((radeon_connector->audio == RADEON_AUDIO_ENABLE) ||
-		    (drm_detect_hdmi_monitor(radeon_connector->edid) &&
-		     (radeon_connector->audio == RADEON_AUDIO_AUTO)))
-			return ATOM_ENCODER_MODE_HDMI;
-		else if (radeon_connector->use_digital)
+		if (radeon_audio != 0) {
+			if (radeon_connector->use_digital &&
+			    (radeon_connector->audio == RADEON_AUDIO_ENABLE))
+				return ATOM_ENCODER_MODE_HDMI;
+			else if (drm_detect_hdmi_monitor(radeon_connector->edid) &&
+				 (radeon_connector->audio == RADEON_AUDIO_AUTO))
+				return ATOM_ENCODER_MODE_HDMI;
+			else if (radeon_connector->use_digital)
+				return ATOM_ENCODER_MODE_DVI;
+			else
+				return ATOM_ENCODER_MODE_CRT;
+		} else if (radeon_connector->use_digital) {
 			return ATOM_ENCODER_MODE_DVI;
-		else
+		} else {
 			return ATOM_ENCODER_MODE_CRT;
+		}
 		break;
 	case DRM_MODE_CONNECTOR_DVID:
 	case DRM_MODE_CONNECTOR_HDMIA:
 	default:
-		if ((radeon_connector->audio == RADEON_AUDIO_ENABLE) ||
-		    (drm_detect_hdmi_monitor(radeon_connector->edid) &&
-		     (radeon_connector->audio == RADEON_AUDIO_AUTO)))
-			return ATOM_ENCODER_MODE_HDMI;
-		else
+		if (radeon_audio != 0) {
+			if (radeon_connector->audio == RADEON_AUDIO_ENABLE)
+				return ATOM_ENCODER_MODE_HDMI;
+			else if (drm_detect_hdmi_monitor(radeon_connector->edid) &&
+				 (radeon_connector->audio == RADEON_AUDIO_AUTO))
+				return ATOM_ENCODER_MODE_HDMI;
+			else
+				return ATOM_ENCODER_MODE_DVI;
+		} else {
 			return ATOM_ENCODER_MODE_DVI;
+		}
 		break;
 	case DRM_MODE_CONNECTOR_LVDS:
 		return ATOM_ENCODER_MODE_LVDS;
@@ -732,14 +745,19 @@ atombios_get_encoder_mode(struct drm_encoder *encoder)
 	case DRM_MODE_CONNECTOR_DisplayPort:
 		dig_connector = radeon_connector->con_priv;
 		if ((dig_connector->dp_sink_type == CONNECTOR_OBJECT_ID_DISPLAYPORT) ||
-		    (dig_connector->dp_sink_type == CONNECTOR_OBJECT_ID_eDP))
+		    (dig_connector->dp_sink_type == CONNECTOR_OBJECT_ID_eDP)) {
 			return ATOM_ENCODER_MODE_DP;
-		else if ((radeon_connector->audio == RADEON_AUDIO_ENABLE) ||
-			 (drm_detect_hdmi_monitor(radeon_connector->edid) &&
-			  (radeon_connector->audio == RADEON_AUDIO_AUTO)))
-			return ATOM_ENCODER_MODE_HDMI;
-		else
+		} else if (radeon_audio != 0) {
+			if (radeon_connector->audio == RADEON_AUDIO_ENABLE)
+				return ATOM_ENCODER_MODE_HDMI;
+			else if (drm_detect_hdmi_monitor(radeon_connector->edid) &&
+				 (radeon_connector->audio == RADEON_AUDIO_AUTO))
+				return ATOM_ENCODER_MODE_HDMI;
+			else
+				return ATOM_ENCODER_MODE_DVI;
+		} else {
 			return ATOM_ENCODER_MODE_DVI;
+		}
 		break;
 	case DRM_MODE_CONNECTOR_eDP:
 		return ATOM_ENCODER_MODE_DP;
diff --git a/drivers/gpu/drm/radeon/radeon_connectors.c b/drivers/gpu/drm/radeon/radeon_connectors.c
index 79159b5..6456573 100644
--- a/drivers/gpu/drm/radeon/radeon_connectors.c
+++ b/drivers/gpu/drm/radeon/radeon_connectors.c
@@ -1658,9 +1658,12 @@ radeon_add_atom_connector(struct drm_device *dev,
 			drm_object_attach_property(&radeon_connector->base.base,
 						      rdev->mode_info.underscan_vborder_property,
 						      0);
-			drm_object_attach_property(&radeon_connector->base.base,
-						   rdev->mode_info.audio_property,
-						   RADEON_AUDIO_DISABLE);
+			if (radeon_audio != 0)
+				drm_object_attach_property(&radeon_connector->base.base,
+							   rdev->mode_info.audio_property,
+							   (radeon_audio == 1) ?
+							   RADEON_AUDIO_AUTO :
+							   RADEON_AUDIO_DISABLE);
 			subpixel_order = SubPixelHorizontalRGB;
 			connector->interlace_allowed = true;
 			if (connector_type == DRM_MODE_CONNECTOR_HDMIB)
@@ -1754,10 +1757,12 @@ radeon_add_atom_connector(struct drm_device *dev,
 							      rdev->mode_info.underscan_vborder_property,
 							      0);
 			}
-			if (ASIC_IS_DCE2(rdev)) {
+			if (ASIC_IS_DCE2(rdev) && (radeon_audio != 0)) {
 				drm_object_attach_property(&radeon_connector->base.base,
-							      rdev->mode_info.audio_property,
-							      RADEON_AUDIO_DISABLE);
+							   rdev->mode_info.audio_property,
+							   (radeon_audio == 1) ?
+							   RADEON_AUDIO_AUTO :
+							   RADEON_AUDIO_DISABLE);
 			}
 			if (connector_type == DRM_MODE_CONNECTOR_DVII) {
 				radeon_connector->dac_load_detect = true;
@@ -1799,10 +1804,12 @@ radeon_add_atom_connector(struct drm_device *dev,
 							      rdev->mode_info.underscan_vborder_property,
 							      0);
 			}
-			if (ASIC_IS_DCE2(rdev)) {
+			if (ASIC_IS_DCE2(rdev) && (radeon_audio != 0)) {
 				drm_object_attach_property(&radeon_connector->base.base,
-							      rdev->mode_info.audio_property,
-							      RADEON_AUDIO_DISABLE);
+							   rdev->mode_info.audio_property,
+							   (radeon_audio == 1) ?
+							   RADEON_AUDIO_AUTO :
+							   RADEON_AUDIO_DISABLE);
 			}
 			subpixel_order = SubPixelHorizontalRGB;
 			connector->interlace_allowed = true;
@@ -1843,10 +1850,12 @@ radeon_add_atom_connector(struct drm_device *dev,
 							      rdev->mode_info.underscan_vborder_property,
 							      0);
 			}
-			if (ASIC_IS_DCE2(rdev)) {
+			if (ASIC_IS_DCE2(rdev) && (radeon_audio != 0)) {
 				drm_object_attach_property(&radeon_connector->base.base,
-							      rdev->mode_info.audio_property,
-							      RADEON_AUDIO_DISABLE);
+							   rdev->mode_info.audio_property,
+							   (radeon_audio == 1) ?
+							   RADEON_AUDIO_AUTO :
+							   RADEON_AUDIO_DISABLE);
 			}
 			connector->interlace_allowed = true;
 			/* in theory with a DP to VGA converter... */
diff --git a/drivers/gpu/drm/radeon/radeon_drv.c b/drivers/gpu/drm/radeon/radeon_drv.c
index cdd12dc..9c14a1b 100644
--- a/drivers/gpu/drm/radeon/radeon_drv.c
+++ b/drivers/gpu/drm/radeon/radeon_drv.c
@@ -153,7 +153,7 @@ int radeon_benchmarking = 0;
 int radeon_testing = 0;
 int radeon_connector_table = 0;
 int radeon_tv = 1;
-int radeon_audio = 1;
+int radeon_audio = -1;
 int radeon_disp_priority = 0;
 int radeon_hw_i2c = 0;
 int radeon_pcie_gen2 = -1;
@@ -196,7 +196,7 @@ module_param_named(connector_table, radeon_connector_table, int, 0444);
 MODULE_PARM_DESC(tv, "TV enable (0 = disable)");
 module_param_named(tv, radeon_tv, int, 0444);
 
-MODULE_PARM_DESC(audio, "Audio enable (1 = enable)");
+MODULE_PARM_DESC(audio, "Audio enable (-1 = auto, 0 = disable, 1 = enable)");
 module_param_named(audio, radeon_audio, int, 0444);
 
 MODULE_PARM_DESC(disp_priority, "Display Priority (0 = auto, 1 = normal, 2 = high)");
-- 
1.8.3.1

Re: [PATCH 1/2] drm/radeon/audio: don't set speaker allocation on DCE3.2

[Christian König]

Am 17.10.2013 22:22, schrieb Alex Deucher:
> It causes hangs on some asics.
>
> Bug:
> https://bugs.freedesktop.org/show_bug.cgi?id=70439
>
> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>

Both are: Reviewed-by: Christian König <christian.koenig@amd.com>

> ---
>   drivers/gpu/drm/radeon/r600_hdmi.c | 3 +++
>   1 file changed, 3 insertions(+)
>
> diff --git a/drivers/gpu/drm/radeon/r600_hdmi.c b/drivers/gpu/drm/radeon/r600_hdmi.c
> index 5b72931..06022e3 100644
> --- a/drivers/gpu/drm/radeon/r600_hdmi.c
> +++ b/drivers/gpu/drm/radeon/r600_hdmi.c
> @@ -309,6 +309,9 @@ static void dce3_2_afmt_write_speaker_allocation(struct drm_encoder *encoder)
>   	u8 *sadb;
>   	int sad_count;
>   
> +	/* XXX: setting this register causes hangs on some asics */
> +	return;
> +
>   	list_for_each_entry(connector, &encoder->dev->mode_config.connector_list, head) {
>   		if (connector->encoder == encoder)
>   			radeon_connector = to_radeon_connector(connector);

Re: [PATCH 2/2] drm/radeon: rework audio option

[Rafał Miłecki]

2013/10/17 Alex Deucher <alexdeucher@gmail.com>:
> In 3.12 I changed audio to be enabled by default,
> but you still had to turn it on via xrandr.  This
> was confusing to users so change it to minic the
> previous behavior:
>
> - audio option is set to -1 (auto) by default which is
>   the current 3.12 behavior (audio is enabled but requires
>   xrandr to turn it on.
> - if audio = 1, the audio is enabled without needing
>   to mess with xrandr (previous behavior)
> - audio = 0 disables audio

I like this!

Re: [PATCH 1/2] drm/radeon/audio: don't set speaker allocation on DCE3.2

[Rafał Miłecki]

2013/10/17 Alex Deucher <alexdeucher@gmail.com>:
> It causes hangs on some asics.
>
> Bug:
> https://bugs.freedesktop.org/show_bug.cgi?id=70439

I noticed some hangs on BARTS too, let me test this solution on DCE5.
Maybe it's not just DCE3.2.