Skip to contentSkip to conversation

drm/i915: set AUD_CONFIG N_value_index for DisplayPort

PATCH In review Wu Fengguang 3 participants 14 messages Jan 9, 2012 – Jan 29, 2012

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 drm/i915: set AUD_CONFIG N_value_index for DisplayPort

It should be programmed to "0" for HDMI or "1" for DisplayPort.

This enables DisplayPort audio for

- HP EliteBook 8460p
(whose BIOS does not set the N_value_index bit for us)

- DisplayPort monitor hot plugged after boot (otherwise most BIOS will fill the N_value_index bit for us)

---
 drivers/gpu/drm/i915/i915_reg.h      |    3 +++
 drivers/gpu/drm/i915/intel_display.c |    5 +++++
 2 files changed, 8 insertions(+)
drivers/gpu/drm/i915/i915_reg.h +3 -0
@@ -3582,4 +3582,7 @@
 #define CPT_AUD_CNTL_ST_A		0xE50B4
 #define CPT_AUD_CNTRL_ST2		0xE50C0
 
+#define IBX_AUD_CONFIG_A		0xe2000
+#define CPT_AUD_CONFIG_A		0xe5000
+
 #endif /* _I915_REG_H_ */
drivers/gpu/drm/i915/intel_display.c +5 -0
@@ -5908,15 +5908,18 @@ static void ironlake_write_eld(struct dr
 	uint32_t i;
 	int len;
 	int hdmiw_hdmiedid;
+	int aud_config;
 	int aud_cntl_st;
 	int aud_cntrl_st2;
 
 	if (HAS_PCH_IBX(connector->dev)) {
 		hdmiw_hdmiedid = IBX_HDMIW_HDMIEDID_A;
+		aud_config = IBX_AUD_CONFIG_A;
 		aud_cntl_st = IBX_AUD_CNTL_ST_A;
 		aud_cntrl_st2 = IBX_AUD_CNTL_ST2;
 	} else {
 		hdmiw_hdmiedid = CPT_HDMIW_HDMIEDID_A;
+		aud_config = CPT_AUD_CONFIG_A;
 		aud_cntl_st = CPT_AUD_CNTL_ST_A;
 		aud_cntrl_st2 = CPT_AUD_CNTRL_ST2;
 	}
@@ -5924,6 +5927,7 @@ static void ironlake_write_eld(struct dr
 	i = to_intel_crtc(crtc)->pipe;
 	hdmiw_hdmiedid += i * 0x100;
 	aud_cntl_st += i * 0x100;
+	aud_config += i * 0x100;
 
 	DRM_DEBUG_DRIVER("ELD on pipe %c\n", pipe_name(i));
 
@@ -5943,6 +5947,7 @@ static void ironlake_write_eld(struct dr
 	if (intel_pipe_has_type(crtc, INTEL_OUTPUT_DISPLAYPORT)) {
 		DRM_DEBUG_DRIVER("ELD: DisplayPort detected\n");
 		eld[5] |= (1 << 2);	/* Conn_Type, 0x1 = DisplayPort */
+		I915_WRITE(aud_config, 1 << 29); /* N value index, 0x1 = DP */
 	}
 
 	if (intel_eld_uptodate(connector,
#1 lore raw

drivers/gpu/drm/i915/i915_reg.h line 3586

Quoted 2 lines · Wu Fengguang
drivers/gpu/drm/i915/i915_reg.h +2 -0
+#define IBX_AUD_CONFIG_A		0xe2000
+#define CPT_AUD_CONFIG_A		0xe5000

These register addresses match the docs.

drivers/gpu/drm/i915/intel_display.c static void ironlake_write_eld(struct dr · line 5950

Quoted 1 line
drivers/gpu/drm/i915/intel_display.c +1 -0
+		I915_WRITE(aud_config, 1 << 29); /* N value index, 0x1 = DP */

Please don't use constants here, instead add #defines for all of the bits in this new register, in case someone else needs to use the register later.

Do we also need to program the pixel clock values in this register?

Signature · 1 line
[email protected]

_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

author Tested-by Robert Lemaire
#2 lore raw
Quoted 7 lines · Keith Packard

Review

Quoted 2 lines · Wu Fengguang
(quoted diff) +2 -0
+#define IBX_AUD_CONFIG_A		0xe2000
+#define CPT_AUD_CONFIG_A		0xe5000

These register addresses match the docs.

Quoted 5 lines

Review

Quoted 1 line
(quoted diff) +1 -0
+		I915_WRITE(aud_config, 1 << 29); /* N value index, 0x1 = DP */

Please don't use constants here, instead add #defines for all of the bits in this new register, in case someone else needs to use the register later.

OK. Updated patch to use macro as follows.

Quoted 1 line

Do we also need to program the pixel clock values in this register?

The pixel clock value is only for HDMI. I'd not touch this as long as it works fine ;)

Thanks,
Fengguang
---
Subject: drm/i915: set AUD_CONFIG N_value_index for DisplayPort
Date: Fri Jan 06 14:41:31 CST 2012

It should be programmed to "0" for HDMI or "1" for DisplayPort.

This enables DisplayPort audio for

- HP EliteBook 8460p
(whose BIOS does not set the N_value_index bit for us)

- DisplayPort monitor hot plugged after boot (otherwise most BIOS will fill the N_value_index bit for us)

---
 drivers/gpu/drm/i915/i915_reg.h      |   12 ++++++++++++
 drivers/gpu/drm/i915/intel_display.c |    5 +++++
 2 files changed, 17 insertions(+)
drivers/gpu/drm/i915/i915_reg.h +12 -0
@@ -3582,4 +3582,16 @@
 #define CPT_AUD_CNTL_ST_A		0xE50B4
 #define CPT_AUD_CNTRL_ST2		0xE50C0
 
+#define IBX_AUD_CONFIG_A			0xe2000
+#define CPT_AUD_CONFIG_A			0xe5000
+#define   AUD_CONFIG_N_VALUE_INDEX		(1 << 29)
+#define   AUD_CONFIG_N_PROG_ENABLE		(1 << 28)
+#define   AUD_CONFIG_UPPER_N_SHIFT		20
+#define   AUD_CONFIG_UPPER_N_VALUE		(0xff << 20)
+#define   AUD_CONFIG_LOWER_N_SHIFT		4
+#define   AUD_CONFIG_LOWER_N_VALUE		(0xfff << 4)
+#define   AUD_CONFIG_PIXEL_CLOCK_HDMI_SHIFT	16
+#define   AUD_CONFIG_PIXEL_CLOCK_HDMI		(0xf << 16)
+#define   AUD_CONFIG_DISABLE_NCTS		(1 << 3)
+
 #endif /* _I915_REG_H_ */
drivers/gpu/drm/i915/intel_display.c +5 -0
@@ -5908,15 +5908,18 @@ static void ironlake_write_eld(struct dr
 	uint32_t i;
 	int len;
 	int hdmiw_hdmiedid;
+	int aud_config;
 	int aud_cntl_st;
 	int aud_cntrl_st2;
 
 	if (HAS_PCH_IBX(connector->dev)) {
 		hdmiw_hdmiedid = IBX_HDMIW_HDMIEDID_A;
+		aud_config = IBX_AUD_CONFIG_A;
 		aud_cntl_st = IBX_AUD_CNTL_ST_A;
 		aud_cntrl_st2 = IBX_AUD_CNTL_ST2;
 	} else {
 		hdmiw_hdmiedid = CPT_HDMIW_HDMIEDID_A;
+		aud_config = CPT_AUD_CONFIG_A;
 		aud_cntl_st = CPT_AUD_CNTL_ST_A;
 		aud_cntrl_st2 = CPT_AUD_CNTRL_ST2;
 	}
@@ -5924,6 +5927,7 @@ static void ironlake_write_eld(struct dr
 	i = to_intel_crtc(crtc)->pipe;
 	hdmiw_hdmiedid += i * 0x100;
 	aud_cntl_st += i * 0x100;
+	aud_config += i * 0x100;
 
 	DRM_DEBUG_DRIVER("ELD on pipe %c\n", pipe_name(i));
 
@@ -5943,6 +5947,7 @@ static void ironlake_write_eld(struct dr
 	if (intel_pipe_has_type(crtc, INTEL_OUTPUT_DISPLAYPORT)) {
 		DRM_DEBUG_DRIVER("ELD: DisplayPort detected\n");
 		eld[5] |= (1 << 2);	/* Conn_Type, 0x1 = DisplayPort */
+		I915_WRITE(aud_config, AUD_CONFIG_N_VALUE_INDEX); /* 0x1 = DP */
 	}
 
 	if (intel_eld_uptodate(connector,
#3 lore raw

drivers/gpu/drm/i915/intel_display.c static void ironlake_write_eld(struct dr · line 5951

Quoted 6 lines · Wu Fengguang
drivers/gpu/drm/i915/intel_display.c +1 -0
@@ -5943,6 +5947,7 @@ static void ironlake_write_eld(struct dr
 	if (intel_pipe_has_type(crtc, INTEL_OUTPUT_DISPLAYPORT)) {
 		DRM_DEBUG_DRIVER("ELD: DisplayPort detected\n");
 		eld[5] |= (1 << 2);	/* Conn_Type, 0x1 = DisplayPort */
+		I915_WRITE(aud_config, AUD_CONFIG_N_VALUE_INDEX); /* 0x1 = DP */
 	}

Do we need to clear this bit in the HDMI case? Or do we just trust the BIOS to either leave this bit zero or set it correctly?

Signature · 1 line
[email protected]
#4 lore raw
Quoted 11 lines · Keith Packard

drivers/gpu/drm/i915/intel_display.c static void ironlake_write_eld(struct dr · line 5951

Quoted 6 lines · Wu Fengguang
drivers/gpu/drm/i915/intel_display.c +1 -0
@@ -5943,6 +5947,7 @@ static void ironlake_write_eld(struct dr
 	if (intel_pipe_has_type(crtc, INTEL_OUTPUT_DISPLAYPORT)) {
 		DRM_DEBUG_DRIVER("ELD: DisplayPort detected\n");
 		eld[5] |= (1 << 2);	/* Conn_Type, 0x1 = DisplayPort */
+		I915_WRITE(aud_config, AUD_CONFIG_N_VALUE_INDEX); /* 0x1 = DP */
 	}

Do we need to clear this bit in the HDMI case? Or do we just trust the BIOS to either leave this bit zero or set it correctly?

I tried booting

1) with HDMI monitor plugged
2) plug HDMI monitor after BIOS boot

In both cases, I get the same AUD_CONFIG values for the host/sink matrix

                RX-V1800        SONY TV
ivybridge       0x00000000      0x00000000
ironlake        0x00000000      0x00000000

HDMI audio is working fine in all cases. So I guess it's fine to leave HDMI as (unconfigured) 0.

Thanks,
Fengguang

#5 lore raw
Quoted 18 lines · Wu Fengguang
Quoted 11 lines · Keith Packard

drivers/gpu/drm/i915/intel_display.c static void ironlake_write_eld(struct dr · line 5951

Quoted 6 lines · Wu Fengguang
drivers/gpu/drm/i915/intel_display.c +1 -0
@@ -5943,6 +5947,7 @@ static void ironlake_write_eld(struct dr
 	if (intel_pipe_has_type(crtc, INTEL_OUTPUT_DISPLAYPORT)) {
 		DRM_DEBUG_DRIVER("ELD: DisplayPort detected\n");
 		eld[5] |= (1 << 2);	/* Conn_Type, 0x1 = DisplayPort */
+		I915_WRITE(aud_config, AUD_CONFIG_N_VALUE_INDEX); /* 0x1 = DP */
 	}

Do we need to clear this bit in the HDMI case? Or do we just trust the BIOS to either leave this bit zero or set it correctly?

I tried booting

1) with HDMI monitor plugged
2) plug HDMI monitor after BIOS boot

Quoted 8 lines

In both cases, I get the same AUD_CONFIG values for the host/sink matrix

                RX-V1800        SONY TV
ivybridge       0x00000000      0x00000000
ironlake        0x00000000      0x00000000

HDMI audio is working fine in all cases. So I guess it's fine to leave HDMI as (unconfigured) 0.

Keith, does this address your concern and this patch is r-b: Keith or do we want an

} else {
	I915_WRITE(aud_config, 0);
}

for paranoia?
-Daniel

Signature · 3 lines
Daniel Vetter
Mail: [email protected]
Mobile: +41 (0)79 365 57 48
#6 lore raw
Quoted 8 lines · Daniel Vetter

Keith, does this address your concern and this patch is r-b: Keith or do we want an

} else {
	I915_WRITE(aud_config, 0);
}

for paranoia?

I think we want this added, just to be sure we set the configuration correctly in all cases; it's easy to imagine moving from DP to HDMI and leaving this bit set.

Signature · 1 line
[email protected]
author Tested-by Robert Lemaire
#7 lore raw
Quoted 9 lines · Keith Packard
Quoted 7 lines · Daniel Vetter

Keith, does this address your concern and this patch is r-b: Keith or do we want an

} else {
	I915_WRITE(aud_config, 0);
}
Quoted 5 lines
Quoted 1 line

for paranoia?

I think we want this added, just to be sure we set the configuration correctly in all cases; it's easy to imagine moving from DP to HDMI and leaving this bit set.

Ah good point! Here is the updated patch.

---
Subject: drm/i915: set AUD_CONFIG N_value_index for DisplayPort
Date: Fri Jan 06 14:41:31 CST 2012

It should be programmed to "0" for HDMI or "1" for DisplayPort.

This enables DisplayPort audio for

- HP EliteBook 8460p
(whose BIOS does not set the N_value_index bit for us)

- DisplayPort monitor hot plugged after boot (otherwise most BIOS will fill the N_value_index bit for us)

---
 drivers/gpu/drm/i915/i915_reg.h      |   12 ++++++++++++
 drivers/gpu/drm/i915/intel_display.c |    8 +++++++-
 2 files changed, 19 insertions(+), 1 deletion(-)
drivers/gpu/drm/i915/i915_reg.h +12 -0
@@ -3582,4 +3582,16 @@
 #define CPT_AUD_CNTL_ST_A		0xE50B4
 #define CPT_AUD_CNTRL_ST2		0xE50C0
 
+#define IBX_AUD_CONFIG_A			0xe2000
+#define CPT_AUD_CONFIG_A			0xe5000
+#define   AUD_CONFIG_N_VALUE_INDEX		(1 << 29)
+#define   AUD_CONFIG_N_PROG_ENABLE		(1 << 28)
+#define   AUD_CONFIG_UPPER_N_SHIFT		20
+#define   AUD_CONFIG_UPPER_N_VALUE		(0xff << 20)
+#define   AUD_CONFIG_LOWER_N_SHIFT		4
+#define   AUD_CONFIG_LOWER_N_VALUE		(0xfff << 4)
+#define   AUD_CONFIG_PIXEL_CLOCK_HDMI_SHIFT	16
+#define   AUD_CONFIG_PIXEL_CLOCK_HDMI		(0xf << 16)
+#define   AUD_CONFIG_DISABLE_NCTS		(1 << 3)
+
 #endif /* _I915_REG_H_ */
drivers/gpu/drm/i915/intel_display.c +7 -1
@@ -5908,15 +5908,18 @@ static void ironlake_write_eld(struct dr
 	uint32_t i;
 	int len;
 	int hdmiw_hdmiedid;
+	int aud_config;
 	int aud_cntl_st;
 	int aud_cntrl_st2;
 
 	if (HAS_PCH_IBX(connector->dev)) {
 		hdmiw_hdmiedid = IBX_HDMIW_HDMIEDID_A;
+		aud_config = IBX_AUD_CONFIG_A;
 		aud_cntl_st = IBX_AUD_CNTL_ST_A;
 		aud_cntrl_st2 = IBX_AUD_CNTL_ST2;
 	} else {
 		hdmiw_hdmiedid = CPT_HDMIW_HDMIEDID_A;
+		aud_config = CPT_AUD_CONFIG_A;
 		aud_cntl_st = CPT_AUD_CNTL_ST_A;
 		aud_cntrl_st2 = CPT_AUD_CNTRL_ST2;
 	}
@@ -5924,6 +5927,7 @@ static void ironlake_write_eld(struct dr
 	i = to_intel_crtc(crtc)->pipe;
 	hdmiw_hdmiedid += i * 0x100;
 	aud_cntl_st += i * 0x100;
+	aud_config += i * 0x100;
 
 	DRM_DEBUG_DRIVER("ELD on pipe %c\n", pipe_name(i));
 
@@ -5943,7 +5947,9 @@ static void ironlake_write_eld(struct dr
 	if (intel_pipe_has_type(crtc, INTEL_OUTPUT_DISPLAYPORT)) {
 		DRM_DEBUG_DRIVER("ELD: DisplayPort detected\n");
 		eld[5] |= (1 << 2);	/* Conn_Type, 0x1 = DisplayPort */
-	}
+		I915_WRITE(aud_config, AUD_CONFIG_N_VALUE_INDEX); /* 0x1 = DP */
+	} else
+		I915_WRITE(aud_config, 0);
 
 	if (intel_eld_uptodate(connector,
 			       aud_cntrl_st2, eldv,
Reviewed-by
#8 lore raw
Quoted 26 lines · Wu Fengguang
Quoted 9 lines · Keith Packard
Quoted 7 lines · Daniel Vetter

Keith, does this address your concern and this patch is r-b: Keith or do we want an

} else {
	I915_WRITE(aud_config, 0);
}
Quoted 5 lines
Quoted 1 line

for paranoia?

I think we want this added, just to be sure we set the configuration correctly in all cases; it's easy to imagine moving from DP to HDMI and leaving this bit set.

Ah good point! Here is the updated patch.

---
Subject: drm/i915: set AUD_CONFIG N_value_index for DisplayPort
Date: Fri Jan 06 14:41:31 CST 2012

It should be programmed to "0" for HDMI or "1" for DisplayPort.

This enables DisplayPort audio for

Quoted 8 lines

- HP EliteBook 8460p
(whose BIOS does not set the N_value_index bit for us)

- DisplayPort monitor hot plugged after boot (otherwise most BIOS will fill the N_value_index bit for us)

Quoted 64 lines
---
 drivers/gpu/drm/i915/i915_reg.h      |   12 ++++++++++++
 drivers/gpu/drm/i915/intel_display.c |    8 +++++++-
 2 files changed, 19 insertions(+), 1 deletion(-)
drivers/gpu/drm/i915/i915_reg.h +12 -0
@@ -3582,4 +3582,16 @@
 #define CPT_AUD_CNTL_ST_A		0xE50B4
 #define CPT_AUD_CNTRL_ST2		0xE50C0
 
+#define IBX_AUD_CONFIG_A			0xe2000
+#define CPT_AUD_CONFIG_A			0xe5000
+#define   AUD_CONFIG_N_VALUE_INDEX		(1 << 29)
+#define   AUD_CONFIG_N_PROG_ENABLE		(1 << 28)
+#define   AUD_CONFIG_UPPER_N_SHIFT		20
+#define   AUD_CONFIG_UPPER_N_VALUE		(0xff << 20)
+#define   AUD_CONFIG_LOWER_N_SHIFT		4
+#define   AUD_CONFIG_LOWER_N_VALUE		(0xfff << 4)
+#define   AUD_CONFIG_PIXEL_CLOCK_HDMI_SHIFT	16
+#define   AUD_CONFIG_PIXEL_CLOCK_HDMI		(0xf << 16)
+#define   AUD_CONFIG_DISABLE_NCTS		(1 << 3)
+
 #endif /* _I915_REG_H_ */
drivers/gpu/drm/i915/intel_display.c +7 -1
@@ -5908,15 +5908,18 @@ static void ironlake_write_eld(struct dr
 	uint32_t i;
 	int len;
 	int hdmiw_hdmiedid;
+	int aud_config;
 	int aud_cntl_st;
 	int aud_cntrl_st2;
 
 	if (HAS_PCH_IBX(connector->dev)) {
 		hdmiw_hdmiedid = IBX_HDMIW_HDMIEDID_A;
+		aud_config = IBX_AUD_CONFIG_A;
 		aud_cntl_st = IBX_AUD_CNTL_ST_A;
 		aud_cntrl_st2 = IBX_AUD_CNTL_ST2;
 	} else {
 		hdmiw_hdmiedid = CPT_HDMIW_HDMIEDID_A;
+		aud_config = CPT_AUD_CONFIG_A;
 		aud_cntl_st = CPT_AUD_CNTL_ST_A;
 		aud_cntrl_st2 = CPT_AUD_CNTRL_ST2;
 	}
@@ -5924,6 +5927,7 @@ static void ironlake_write_eld(struct dr
 	i = to_intel_crtc(crtc)->pipe;
 	hdmiw_hdmiedid += i * 0x100;
 	aud_cntl_st += i * 0x100;
+	aud_config += i * 0x100;
 
 	DRM_DEBUG_DRIVER("ELD on pipe %c\n", pipe_name(i));
 
@@ -5943,7 +5947,9 @@ static void ironlake_write_eld(struct dr
 	if (intel_pipe_has_type(crtc, INTEL_OUTPUT_DISPLAYPORT)) {
 		DRM_DEBUG_DRIVER("ELD: DisplayPort detected\n");
 		eld[5] |= (1 << 2);	/* Conn_Type, 0x1 = DisplayPort */
-	}
+		I915_WRITE(aud_config, AUD_CONFIG_N_VALUE_INDEX); /* 0x1 = DP */
+	} else
+		I915_WRITE(aud_config, 0);
 
 	if (intel_eld_uptodate(connector,
 			       aud_cntrl_st2, eldv,
Signature · 1 line
[email protected]

_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

#9 lore raw
In reply to Wu Fengguang
Quoted 26 lines · Wu Fengguang
Quoted 9 lines · Keith Packard
Quoted 7 lines · Daniel Vetter

Keith, does this address your concern and this patch is r-b: Keith or do we want an

} else {
	I915_WRITE(aud_config, 0);
}
Quoted 5 lines
Quoted 1 line

for paranoia?

I think we want this added, just to be sure we set the configuration correctly in all cases; it's easy to imagine moving from DP to HDMI and leaving this bit set.

Ah good point! Here is the updated patch.

---
Subject: drm/i915: set AUD_CONFIG N_value_index for DisplayPort
Date: Fri Jan 06 14:41:31 CST 2012

It should be programmed to "0" for HDMI or "1" for DisplayPort.

This enables DisplayPort audio for

Quoted 8 lines

- HP EliteBook 8460p
(whose BIOS does not set the N_value_index bit for us)

- DisplayPort monitor hot plugged after boot (otherwise most BIOS will fill the N_value_index bit for us)

I've tried to apply this patch but it has a conflict in i915_reg.h (with the SOL reset stuff). Care to rebase and resend? Also, git patches as produced by git format-patch highly preferred.

Thanks, Daniel

Signature · 3 lines
Daniel Vetter
Mail: [email protected]
Mobile: +41 (0)79 365 57 48
#10 lore raw

PATCH 1/1 drm/i915: set AUD_CONFIG N_value_index for DisplayPort

It should be programmed to "0" for HDMI or "1" for DisplayPort.

This enables DisplayPort audio for

- HP EliteBook 8460p
(whose BIOS does not set the N_value_index bit for us)

- DisplayPort monitor hot plugged after boot (otherwise most BIOS will fill the N_value_index bit for us)

---
 drivers/gpu/drm/i915/i915_reg.h      |   12 ++++++++++++
 drivers/gpu/drm/i915/intel_display.c |    8 +++++++-
 2 files changed, 19 insertions(+), 1 deletion(-)
Quoted 3 lines

I've tried to apply this patch but it has a conflict in i915_reg.h (with the SOL reset stuff). Care to rebase and resend? Also, git patches as produced by git format-patch highly preferred.

Daniel, this is now based on linux-next. I use quilt, hope it also works for you :)

drivers/gpu/drm/i915/i915_reg.h +12 -0
@@ -3742,4 +3742,16 @@
  */
 #define GEN7_SO_WRITE_OFFSET(n)		(0x5280 + (n) * 4)
 
+#define IBX_AUD_CONFIG_A			0xe2000
+#define CPT_AUD_CONFIG_A			0xe5000
+#define   AUD_CONFIG_N_VALUE_INDEX		(1 << 29)
+#define   AUD_CONFIG_N_PROG_ENABLE		(1 << 28)
+#define   AUD_CONFIG_UPPER_N_SHIFT		20
+#define   AUD_CONFIG_UPPER_N_VALUE		(0xff << 20)
+#define   AUD_CONFIG_LOWER_N_SHIFT		4
+#define   AUD_CONFIG_LOWER_N_VALUE		(0xfff << 4)
+#define   AUD_CONFIG_PIXEL_CLOCK_HDMI_SHIFT	16
+#define   AUD_CONFIG_PIXEL_CLOCK_HDMI		(0xf << 16)
+#define   AUD_CONFIG_DISABLE_NCTS		(1 << 3)
+
 #endif /* _I915_REG_H_ */
drivers/gpu/drm/i915/intel_display.c +7 -1
@@ -6072,15 +6072,18 @@ static void ironlake_write_eld(struct dr
 	uint32_t i;
 	int len;
 	int hdmiw_hdmiedid;
+	int aud_config;
 	int aud_cntl_st;
 	int aud_cntrl_st2;
 
 	if (HAS_PCH_IBX(connector->dev)) {
 		hdmiw_hdmiedid = IBX_HDMIW_HDMIEDID_A;
+		aud_config = IBX_AUD_CONFIG_A;
 		aud_cntl_st = IBX_AUD_CNTL_ST_A;
 		aud_cntrl_st2 = IBX_AUD_CNTL_ST2;
 	} else {
 		hdmiw_hdmiedid = CPT_HDMIW_HDMIEDID_A;
+		aud_config = CPT_AUD_CONFIG_A;
 		aud_cntl_st = CPT_AUD_CNTL_ST_A;
 		aud_cntrl_st2 = CPT_AUD_CNTRL_ST2;
 	}
@@ -6088,6 +6091,7 @@ static void ironlake_write_eld(struct dr
 	i = to_intel_crtc(crtc)->pipe;
 	hdmiw_hdmiedid += i * 0x100;
 	aud_cntl_st += i * 0x100;
+	aud_config += i * 0x100;
 
 	DRM_DEBUG_DRIVER("ELD on pipe %c\n", pipe_name(i));
 
@@ -6107,7 +6111,9 @@ static void ironlake_write_eld(struct dr
 	if (intel_pipe_has_type(crtc, INTEL_OUTPUT_DISPLAYPORT)) {
 		DRM_DEBUG_DRIVER("ELD: DisplayPort detected\n");
 		eld[5] |= (1 << 2);	/* Conn_Type, 0x1 = DisplayPort */
-	}
+		I915_WRITE(aud_config, AUD_CONFIG_N_VALUE_INDEX); /* 0x1 = DP */
+	} else
+		I915_WRITE(aud_config, 0);
 
 	if (intel_eld_uptodate(connector,
 			       aud_cntrl_st2, eldv,
#11 lore raw
Quoted 15 lines · Wu Fengguang

It should be programmed to "0" for HDMI or "1" for DisplayPort.

This enables DisplayPort audio for

- HP EliteBook 8460p
 (whose BIOS does not set the N_value_index bit for us)

- DisplayPort monitor hot plugged after boot (otherwise most BIOS will fill the N_value_index bit for us)

---
 drivers/gpu/drm/i915/i915_reg.h      |   12 ++++++++++++ drivers/gpu/drm/i915/intel_display.c |    8 +++++++-

Quoted 8 lines
 2 files changed, 19 insertions(+), 1 deletion(-)
Quoted 3 lines

I've tried to apply this patch but it has a conflict in i915_reg.h (with the SOL reset stuff). Care to rebase and resend? Also, git patches as produced by git format-patch highly preferred.

Daniel, this is now based on linux-next. I use quilt, hope it also works for you :)

On a quick check this patch is missing the hunk to actually write the aud configuration into the register. I presume that's not intentional, so can you please resend?

Thanks, Daniel

Signature · 2 lines
Daniel Vetter
[email protected] - +41 (0) 79 364 57 48 - http://blog.ffwll.ch
author Tested-by Robert Lemaire
#12 lore raw
Quoted 6 lines
Quoted 2 lines

Daniel, this is now based on linux-next. I use quilt, hope it also works for you :)

On a quick check this patch is missing the hunk to actually write the aud configuration into the register. I presume that's not intentional, so can you please resend?

This is weird, my outbox has the below chunk. Anyway here is the attached patch.

@@ -6107,7 +6111,9 @@ static void ironlake_write_eld(struct dr

        if (intel_pipe_has_type(crtc, INTEL_OUTPUT_DISPLAYPORT)) {
                DRM_DEBUG_DRIVER("ELD: DisplayPort detected\n");
                eld[5] |= (1 << 2);     /* Conn_Type, 0x1 = DisplayPort */

- }
+ I915_WRITE(aud_config, AUD_CONFIG_N_VALUE_INDEX); /* 0x1 = DP */
+ } else
+ I915_WRITE(aud_config, 0);

Thanks,
Fengguang

Subject: drm/i915: set AUD_CONFIG N_value_index for DisplayPort
Date: Fri Jan 06 14:41:31 CST 2012

It should be programmed to "0" for HDMI or "1" for DisplayPort.

This enables DisplayPort audio for

- HP EliteBook 8460p
(whose BIOS does not set the N_value_index bit for us)

- DisplayPort monitor hot plugged after boot (otherwise most BIOS will fill the N_value_index bit for us)

---
 drivers/gpu/drm/i915/i915_reg.h      |   12 ++++++++++++
 drivers/gpu/drm/i915/intel_display.c |    8 +++++++-
 2 files changed, 19 insertions(+), 1 deletion(-)
drivers/gpu/drm/i915/i915_reg.h +12 -0
@@ -3742,4 +3742,16 @@
  */
 #define GEN7_SO_WRITE_OFFSET(n)		(0x5280 + (n) * 4)
 
+#define IBX_AUD_CONFIG_A			0xe2000
+#define CPT_AUD_CONFIG_A			0xe5000
+#define   AUD_CONFIG_N_VALUE_INDEX		(1 << 29)
+#define   AUD_CONFIG_N_PROG_ENABLE		(1 << 28)
+#define   AUD_CONFIG_UPPER_N_SHIFT		20
+#define   AUD_CONFIG_UPPER_N_VALUE		(0xff << 20)
+#define   AUD_CONFIG_LOWER_N_SHIFT		4
+#define   AUD_CONFIG_LOWER_N_VALUE		(0xfff << 4)
+#define   AUD_CONFIG_PIXEL_CLOCK_HDMI_SHIFT	16
+#define   AUD_CONFIG_PIXEL_CLOCK_HDMI		(0xf << 16)
+#define   AUD_CONFIG_DISABLE_NCTS		(1 << 3)
+
 #endif /* _I915_REG_H_ */
drivers/gpu/drm/i915/intel_display.c +7 -1
@@ -6072,15 +6072,18 @@ static void ironlake_write_eld(struct dr
 	uint32_t i;
 	int len;
 	int hdmiw_hdmiedid;
+	int aud_config;
 	int aud_cntl_st;
 	int aud_cntrl_st2;
 
 	if (HAS_PCH_IBX(connector->dev)) {
 		hdmiw_hdmiedid = IBX_HDMIW_HDMIEDID_A;
+		aud_config = IBX_AUD_CONFIG_A;
 		aud_cntl_st = IBX_AUD_CNTL_ST_A;
 		aud_cntrl_st2 = IBX_AUD_CNTL_ST2;
 	} else {
 		hdmiw_hdmiedid = CPT_HDMIW_HDMIEDID_A;
+		aud_config = CPT_AUD_CONFIG_A;
 		aud_cntl_st = CPT_AUD_CNTL_ST_A;
 		aud_cntrl_st2 = CPT_AUD_CNTRL_ST2;
 	}
@@ -6088,6 +6091,7 @@ static void ironlake_write_eld(struct dr
 	i = to_intel_crtc(crtc)->pipe;
 	hdmiw_hdmiedid += i * 0x100;
 	aud_cntl_st += i * 0x100;
+	aud_config += i * 0x100;
 
 	DRM_DEBUG_DRIVER("ELD on pipe %c\n", pipe_name(i));
 
@@ -6107,7 +6111,9 @@ static void ironlake_write_eld(struct dr
 	if (intel_pipe_has_type(crtc, INTEL_OUTPUT_DISPLAYPORT)) {
 		DRM_DEBUG_DRIVER("ELD: DisplayPort detected\n");
 		eld[5] |= (1 << 2);	/* Conn_Type, 0x1 = DisplayPort */
-	}
+		I915_WRITE(aud_config, AUD_CONFIG_N_VALUE_INDEX); /* 0x1 = DP */
+	} else
+		I915_WRITE(aud_config, 0);
 
 	if (intel_eld_uptodate(connector,
 			       aud_cntrl_st2, eldv,

_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

#13 lore raw
Quoted 20 lines · Wu Fengguang
Quoted 6 lines
Quoted 2 lines

Daniel, this is now based on linux-next. I use quilt, hope it also works for you :)

On a quick check this patch is missing the hunk to actually write the aud configuration into the register. I presume that's not intentional, so can you please resend?

This is weird, my outbox has the below chunk. Anyway here is the attached patch.

drivers/gpu/drm/i915/intel_display.c +3 -1
@@ -6107,7 +6111,9 @@ static void ironlake_write_eld(struct dr
        if (intel_pipe_has_type(crtc, INTEL_OUTPUT_DISPLAYPORT)) {
                DRM_DEBUG_DRIVER("ELD: DisplayPort detected\n");
                eld[5] |= (1 << 2);     /* Conn_Type, 0x1 = DisplayPort */
-       }
+               I915_WRITE(aud_config, AUD_CONFIG_N_VALUE_INDEX); /* 0x1 = DP */
+       } else
+               I915_WRITE(aud_config, 0);
 
 Thanks,
Quoted 1 line

Fengguang

Yeah, the hunk is here, too ... Looks like I've failed with wrestling the quilt patch a bit, sorry for the noise. But I think I've got the handle on that now. So queued for -next, thanks for the patch.

Yours, Daniel

Signature · 3 lines
Daniel Vetter
Mail: [email protected]
Mobile: +41 (0)79 365 57 48