Re: [Intel-gfx] [PATCH] drm/i915: set AUD_CONFIG N_value_index for DisplayPort

2012-01-17 Thread Daniel Vetter
On Tue, Jan 17, 2012 at 07:08:09AM +0800, Wu Fengguang wrote:
 On Mon, Jan 16, 2012 at 12:44:43PM -0800, Keith Packard wrote:
  On Mon, 16 Jan 2012 21:26:18 +0100, Daniel Vetter dan...@ffwll.ch wrote:
  
   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.
 
 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)
 
 Tested-by: Robert Lemaire rlema...@suse.com
 Signed-off-by: Wu Fengguang fengguang...@intel.com

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
-- 
Daniel Vetter
Mail: dan...@ffwll.ch
Mobile: +41 (0)79 365 57 48
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH] drm/i915: set AUD_CONFIG N_value_index for DisplayPort

2012-01-16 Thread Wu Fengguang
On Thu, Jan 12, 2012 at 09:33:34AM -0800, Keith Packard wrote:
 On Tue, 10 Jan 2012 13:45:19 +0800, Wu Fengguang fengguang...@intel.com 
 wrote:
 
  @@ -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-V1800SONY TV
ivybridge   0x  0x
ironlake0x  0x

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

Thanks,
Fengguang
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH] drm/i915: set AUD_CONFIG N_value_index for DisplayPort

2012-01-16 Thread Daniel Vetter
On Mon, Jan 16, 2012 at 04:02:53PM +0800, Wu Fengguang wrote:
 On Thu, Jan 12, 2012 at 09:33:34AM -0800, Keith Packard wrote:
  On Tue, 10 Jan 2012 13:45:19 +0800, Wu Fengguang fengguang...@intel.com 
  wrote:
  
   @@ -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-V1800SONY TV
 ivybridge   0x  0x
 ironlake0x  0x
 
 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
-- 
Daniel Vetter
Mail: dan...@ffwll.ch
Mobile: +41 (0)79 365 57 48
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH] drm/i915: set AUD_CONFIG N_value_index for DisplayPort

2012-01-16 Thread Keith Packard
On Mon, 16 Jan 2012 21:26:18 +0100, Daniel Vetter dan...@ffwll.ch wrote:

 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.

-- 
keith.pack...@intel.com


pgpKdOY4wu2v3.pgp
Description: PGP signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH] drm/i915: set AUD_CONFIG N_value_index for DisplayPort

2012-01-16 Thread Wu Fengguang
On Mon, Jan 16, 2012 at 12:44:43PM -0800, Keith Packard wrote:
 On Mon, 16 Jan 2012 21:26:18 +0100, Daniel Vetter dan...@ffwll.ch wrote:
 
  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.

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)

Tested-by: Robert Lemaire rlema...@suse.com
Signed-off-by: Wu Fengguang fengguang...@intel.com
---
 drivers/gpu/drm/i915/i915_reg.h  |   12 
 drivers/gpu/drm/i915/intel_display.c |8 +++-
 2 files changed, 19 insertions(+), 1 deletion(-)

--- linux.orig/drivers/gpu/drm/i915/i915_reg.h  2012-01-07 23:11:10.0 
+0800
+++ linux/drivers/gpu/drm/i915/i915_reg.h   2012-01-10 13:20:17.0 
+0800
@@ -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_SHIFT16
+#define   AUD_CONFIG_PIXEL_CLOCK_HDMI  (0xf  16)
+#define   AUD_CONFIG_DISABLE_NCTS  (1  3)
+
 #endif /* _I915_REG_H_ */
--- linux.orig/drivers/gpu/drm/i915/intel_display.c 2012-01-07 
23:11:10.0 +0800
+++ linux/drivers/gpu/drm/i915/intel_display.c  2012-01-17 07:06:29.0 
+0800
@@ -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,
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH] drm/i915: set AUD_CONFIG N_value_index for DisplayPort

2012-01-12 Thread Keith Packard
On Tue, 10 Jan 2012 13:45:19 +0800, Wu Fengguang fengguang...@intel.com wrote:

 @@ -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?

-- 
keith.pack...@intel.com


pgpEHh8aTzD0f.pgp
Description: PGP signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH] drm/i915: set AUD_CONFIG N_value_index for DisplayPort

2012-01-09 Thread 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)

Signed-off-by: Wu Fengguang fengguang...@intel.com
---
 drivers/gpu/drm/i915/i915_reg.h  |3 +++
 drivers/gpu/drm/i915/intel_display.c |5 +
 2 files changed, 8 insertions(+)

--- linux.orig/drivers/gpu/drm/i915/i915_reg.h  2012-01-06 15:00:55.0 
+0800
+++ linux/drivers/gpu/drm/i915/i915_reg.h   2012-01-06 15:04:10.0 
+0800
@@ -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_ */
--- linux.orig/drivers/gpu/drm/i915/intel_display.c 2012-01-06 
15:05:47.0 +0800
+++ linux/drivers/gpu/drm/i915/intel_display.c  2012-01-06 16:07:59.0 
+0800
@@ -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,
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH] drm/i915: set AUD_CONFIG N_value_index for DisplayPort

2012-01-09 Thread Keith Packard
On Mon, 9 Jan 2012 21:17:17 +0800, Wu Fengguang fengguang...@intel.com wrote:

 +#define IBX_AUD_CONFIG_A 0xe2000
 +#define CPT_AUD_CONFIG_A 0xe5000

These register addresses match the docs.

 + 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?

-- 
keith.pack...@intel.com


pgpdT0dxISChD.pgp
Description: PGP signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH] drm/i915: set AUD_CONFIG N_value_index for DisplayPort

2012-01-09 Thread Wu Fengguang
On Mon, Jan 09, 2012 at 09:22:47AM -0800, Keith Packard wrote:
 On Mon, 9 Jan 2012 21:17:17 +0800, Wu Fengguang fengguang...@intel.com 
 wrote:
 
  +#define IBX_AUD_CONFIG_A   0xe2000
  +#define CPT_AUD_CONFIG_A   0xe5000
 
 These register addresses match the docs.
 
  +   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.

 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)

Tested-by: Robert Lemaire rlema...@suse.com
Signed-off-by: Wu Fengguang fengguang...@intel.com
---
 drivers/gpu/drm/i915/i915_reg.h  |   12 
 drivers/gpu/drm/i915/intel_display.c |5 +
 2 files changed, 17 insertions(+)

--- linux.orig/drivers/gpu/drm/i915/i915_reg.h  2012-01-07 23:11:10.0 
+0800
+++ linux/drivers/gpu/drm/i915/i915_reg.h   2012-01-10 13:20:17.0 
+0800
@@ -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_SHIFT16
+#define   AUD_CONFIG_PIXEL_CLOCK_HDMI  (0xf  16)
+#define   AUD_CONFIG_DISABLE_NCTS  (1  3)
+
 #endif /* _I915_REG_H_ */
--- linux.orig/drivers/gpu/drm/i915/intel_display.c 2012-01-07 
23:11:10.0 +0800
+++ linux/drivers/gpu/drm/i915/intel_display.c  2012-01-10 13:23:25.0 
+0800
@@ -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,
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx