[Intel-gfx] [PATCH 1/2] drm: support routines for HDMI/DP ELD
ELD (EDID-Like Data) describes to the HDMI/DP audio driver the audio
capabilities of the plugged monitor.
This adds drm_edid_to_eld() for converting EDID to ELD. The converted
ELD will be saved in a new drm_connector.eld[128] data field. This is
necessary because the graphics driver will need to fixup some of the
data fields (eg. HDMI/DP connection type, AV sync delay) before writing
to the hardware ELD buffer. drm_av_sync_delay() will help the graphics
drivers dynamically compute the AV sync delay for fixing-up the ELD.
ELD selection policy: it's possible for one encoder to be associated
with multiple connectors (ie. monitors), in which case the first found
ELD will be returned by drm_select_eld(). This policy may not be
suitable for all users, but let's start it simple first.
The impact of ELD selection policy: assume there are two monitors, one
supports stereo playback and the other has 8-channel output; cloned
display mode is used, so that the two monitors are associated with the
same internal encoder. If only the stereo playback capability is reported,
the user won't be able to start 8-channel playback; if the 8-channel ELD
is reported, then user space applications may send 8-channel samples
down, however the user may actually be listening to the 2-channel
monitor and not connecting speakers to the 8-channel monitor.
According to James, many TVs will either refuse the display anything or
pop-up an OSD warning whenever they receive hdmi audio which they cannot
handle. Eventually we will require configurability and/or per-monitor
audio control even when the video is cloned.
CC: Zhao Yakui yakui.z...@intel.com
CC: Wang Zhenyu zhenyu.z.w...@intel.com
CC: Jeremy Bush contractfrombe...@gmail.com
CC: Christopher White c.wh...@pulseforce.com
CC: Pierre-Louis Bossart pierre-louis.boss...@intel.com
CC: Paul Menzel paulepan...@users.sourceforge.net
CC: James Cloos cl...@jhcloos.com
CC: Chris Wilson ch...@chris-wilson.co.uk
Signed-off-by: Ben Skeggs bske...@redhat.com
Signed-off-by: Wu Fengguang fengguang...@intel.com
---
drivers/gpu/drm/drm_edid.c | 171 +++
include/drm/drm_edid.h |9 +
2 files changed, 180 insertions(+)
--- linux-next.orig/include/drm/drm_crtc.h 2011-09-04 09:42:19.0
+0800
+++ linux-next/include/drm/drm_crtc.h 2011-09-05 09:00:25.0 +0800
@@ -466,6 +466,8 @@ enum drm_connector_force {
/* DACs should rarely do this without a lot of testing */
#define DRM_CONNECTOR_POLL_DISCONNECT (1 2)
+#define MAX_ELD_BYTES 128
+
/**
* drm_connector - central DRM connector control structure
* @crtc: CRTC this connector is currently connected to, NULL if none
@@ -523,6 +525,13 @@ struct drm_connector {
uint32_t force_encoder_id;
struct drm_encoder *encoder; /* currently active encoder */
+ /* EDID bits */
+ uint8_t eld[MAX_ELD_BYTES];
+ bool dvi_dual;
+ int max_tmds_clock; /* in MHz */
+ bool latency_present[2];
+ int video_latency[2]; /* [0]: progressive, [1]: interlaced */
+ int audio_latency[2];
int null_edid_counter; /* needed to workaround some HW bugs where we
get all 0s */
};
--- linux-next.orig/include/drm/drm_edid.h 2011-09-05 12:35:36.0
+0800
+++ linux-next/include/drm/drm_edid.h 2011-09-05 12:36:10.0 +0800
@@ -230,4 +230,13 @@ struct edid {
#define EDID_PRODUCT_ID(e) ((e)-prod_code[0] | ((e)-prod_code[1] 8))
+struct drm_encoder;
+struct drm_connector;
+struct drm_display_mode;
+void drm_edid_to_eld(struct drm_connector *connector, struct edid *edid);
+int drm_av_sync_delay(struct drm_connector *connector,
+ struct drm_display_mode *mode);
+struct drm_connector *drm_select_eld(struct drm_encoder *encoder,
+struct drm_display_mode *mode);
+
#endif /* __DRM_EDID_H__ */
--- linux-next.orig/drivers/gpu/drm/drm_edid.c 2011-09-05 12:35:36.0
+0800
+++ linux-next/drivers/gpu/drm/drm_edid.c 2011-09-05 12:36:10.0
+0800
@@ -1319,6 +1319,7 @@ add_detailed_modes(struct drm_connector
#define HDMI_IDENTIFIER 0x000C03
#define AUDIO_BLOCK0x01
#define VENDOR_BLOCK0x03
+#define SPEAKER_BLOCK 0x04
#define EDID_BASIC_AUDIO (1 6)
/**
@@ -1347,6 +1348,176 @@ u8 *drm_find_cea_extension(struct edid *
}
EXPORT_SYMBOL(drm_find_cea_extension);
+static void
+parse_hdmi_vsdb(struct drm_connector *connector, uint8_t *db)
+{
+ connector-eld[5] |= (db[6] 7) 1; /* Supports_AI */
+
+ connector-dvi_dual = db[6] 1;
+ connector-max_tmds_clock = db[7] * 5;
+
+ connector-latency_present[0] = db[8] 7;
+ connector-latency_present[1] = (db[8] 6) 1;
+ connector-video_latency[0] = db[9];
+ connector-audio_latency[0] = db[10];
+ connector-video_latency[1] = db[11];
+ connector-audio_latency[1] = db[12];
+
+ DRM_LOG_KMS(HDMI: DVI dual %d,
+ max TMDS clock %d,
+
[Intel-gfx] [PATCH 2/2] drm/i915: pass ELD to HDMI/DP audio driver
Add ELD support for Intel Eaglelake, IbexPeak/Ironlake,
SandyBridge/CougarPoint and IvyBridge/PantherPoint chips.
ELD (EDID-Like Data) describes to the HDMI/DP audio driver the audio
capabilities of the plugged monitor. It's built and passed to audio
driver in 2 steps:
(1) at get_modes time, parse EDID and save ELD to drm_connector.eld[]
(2) at mode_set time, write drm_connector.eld[] to the Transcoder's hw
ELD buffer and set the ELD_valid bit to inform HDMI/DP audio driver
This patch is tested OK on G45/HDMI, IbexPeak/HDMI and IvyBridge/HDMI+DP.
Test scheme: plug in the HDMI/DP monitor, and run
cat /proc/asound/card0/eld*
to check if the monitor name, HDMI/DP type, etc. show up correctly.
Minor imperfection: the GEN5_AUD_CNTL_ST/DIP_Port_Select field always
reads 0 (reserved). Without knowing the port number, I worked it around
by setting the ELD_valid bit for ALL the three ports. It's tested to not
be a problem, because the audio driver will find invalid ELD data and
hence rightfully abort, even when it sees the ELD_valid indicator.
Thanks to Zhenyu and Pierre-Louis for a lot of valuable help and testing.
CC: Zhao Yakui yakui.z...@intel.com
CC: Wang Zhenyu zhenyu.z.w...@intel.com
CC: Jeremy Bush contractfrombe...@gmail.com
CC: Christopher White c.wh...@pulseforce.com
CC: Pierre-Louis Bossart pierre-louis.boss...@intel.com
CC: Paul Menzel paulepan...@users.sourceforge.net
Signed-off-by: Wu Fengguang fengguang...@intel.com
---
drivers/gpu/drm/i915/i915_drv.h |2
drivers/gpu/drm/i915/i915_reg.h | 25
drivers/gpu/drm/i915/intel_display.c | 131 -
drivers/gpu/drm/i915/intel_dp.c |6 -
drivers/gpu/drm/i915/intel_drv.h |2
drivers/gpu/drm/i915/intel_hdmi.c|3
drivers/gpu/drm/i915/intel_modes.c |2
7 files changed, 169 insertions(+), 2 deletions(-)
--- linux-next.orig/drivers/gpu/drm/i915/intel_hdmi.c 2011-09-05
14:01:48.0 +0800
+++ linux-next/drivers/gpu/drm/i915/intel_hdmi.c2011-09-05
14:02:57.0 +0800
@@ -245,8 +245,11 @@ static void intel_hdmi_mode_set(struct d
sdvox |= HDMI_MODE_SELECT;
if (intel_hdmi-has_audio) {
+ DRM_DEBUG_DRIVER(Enabling HDMI audio on pipe %c\n,
+pipe_name(intel_crtc-pipe));
sdvox |= SDVO_AUDIO_ENABLE;
sdvox |= SDVO_NULL_PACKETS_DURING_VSYNC;
+ intel_write_eld(encoder, adjusted_mode);
}
if (intel_crtc-pipe == 1) {
--- linux-next.orig/drivers/gpu/drm/i915/intel_display.c2011-09-05
14:01:48.0 +0800
+++ linux-next/drivers/gpu/drm/i915/intel_display.c 2011-09-05
14:02:57.0 +0800
@@ -31,6 +31,7 @@
#include linux/kernel.h
#include linux/slab.h
#include linux/vgaarb.h
+#include drm/drm_edid.h
#include drmP.h
#include intel_drv.h
#include i915_drm.h
@@ -5667,6 +5668,131 @@ static int intel_crtc_mode_set(struct dr
return ret;
}
+static void g4x_write_eld(struct drm_connector *connector,
+ struct drm_crtc *crtc)
+{
+ struct drm_i915_private *dev_priv = connector-dev-dev_private;
+ uint8_t *eld = connector-eld;
+ uint32_t eldv;
+ uint32_t len;
+ uint32_t i;
+
+ i = I915_READ(G4X_AUD_VID_DID);
+
+ if (i == INTEL_AUDIO_DEVBLC || i == INTEL_AUDIO_DEVCL)
+ eldv = G4X_ELDV_DEVCL_DEVBLC;
+ else
+ eldv = G4X_ELDV_DEVCTG;
+
+ i = I915_READ(G4X_AUD_CNTL_ST);
+ i = ~(eldv | G4X_ELD_ADDR);
+ len = (i 9) 0x1f; /* ELD buffer size */
+ I915_WRITE(G4X_AUD_CNTL_ST, i);
+
+ if (!eld[0])
+ return;
+
+ len = min_t(uint8_t, eld[2], len);
+ DRM_DEBUG_DRIVER(ELD size %d\n, len);
+ for (i = 0; i len; i++)
+ I915_WRITE(G4X_HDMIW_HDMIEDID, *((uint32_t *)eld + i));
+
+ i = I915_READ(G4X_AUD_CNTL_ST);
+ i |= eldv;
+ I915_WRITE(G4X_AUD_CNTL_ST, i);
+}
+
+static void ironlake_write_eld(struct drm_connector *connector,
+struct drm_crtc *crtc)
+{
+ struct drm_i915_private *dev_priv = connector-dev-dev_private;
+ uint8_t *eld = connector-eld;
+ uint32_t eldv;
+ uint32_t i;
+ int len;
+ int hdmiw_hdmiedid;
+ int aud_cntl_st;
+ int aud_cntrl_st2;
+
+ if (IS_IVYBRIDGE(connector-dev)) {
+ hdmiw_hdmiedid = GEN7_HDMIW_HDMIEDID_A;
+ aud_cntl_st = GEN7_AUD_CNTRL_ST_A;
+ aud_cntrl_st2 = GEN7_AUD_CNTRL_ST2;
+ } else {
+ hdmiw_hdmiedid = GEN5_HDMIW_HDMIEDID_A;
+ aud_cntl_st = GEN5_AUD_CNTL_ST_A;
+ aud_cntrl_st2 = GEN5_AUD_CNTL_ST2;
+ }
+
+ i = to_intel_crtc(crtc)-pipe;
+ hdmiw_hdmiedid += i * 0x100;
+ aud_cntl_st += i * 0x100;
+
+ DRM_DEBUG_DRIVER(ELD on pipe %c\n, pipe_name(i));
+
+ i = I915_READ(aud_cntl_st);
+
Re: [Intel-gfx] [PATCH 2/3] drm/i915: close PM interrupt masking races in the rps work func
On Sun, Sep 04, 2011 at 09:38:56PM +, Ben Widawsky wrote: Oops, you're totally right, I think I meant: - I915_WRITE(GEN6_PMIMR, pm_imr ~pm_iir); + I915_WRITE(GEN6_PMIMR, dev_priv-pm_iir); Imo still racy without the irqsafe rps_lock around it. gcc is free to compile that into a separate load and store which the irq handler can get in between and change dev_priv-pm_iir and PMIMR. The race is now only one instruction wide, though ;-) -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 2/2] drm/i915: pass ELD to HDMI/DP audio driver
I'd like to do more cleanups:
+ int aud_cntl_st;
+ int aud_cntrl_st2;
s/aud_cntrl_st2/aud_cntl_st2/
+ if (IS_IVYBRIDGE(connector-dev)) {
+ hdmiw_hdmiedid = GEN7_HDMIW_HDMIEDID_A;
+ aud_cntl_st = GEN7_AUD_CNTRL_ST_A;
+ aud_cntrl_st2 = GEN7_AUD_CNTRL_ST2;
+ } else {
+ hdmiw_hdmiedid = GEN5_HDMIW_HDMIEDID_A;
+ aud_cntl_st = GEN5_AUD_CNTL_ST_A;
+ aud_cntrl_st2 = GEN5_AUD_CNTL_ST2;
+ }
+
+ i = to_intel_crtc(crtc)-pipe;
+ hdmiw_hdmiedid += i * 0x100;
+ aud_cntl_st += i * 0x100;
+
+ DRM_DEBUG_DRIVER(ELD on pipe %c\n, pipe_name(i));
[...]
+ len = min_t(uint8_t, eld[2], 21); /* 84 bytes of hw ELD buffer */
+ DRM_DEBUG_DRIVER(ELD size %d\n, len);
Merge the above two DRM_DEBUG_DRIVER() calls into:
+ DRM_DEBUG_DRIVER(writing %d ELD dword(s) on pipe %c\n,
+len, pipe_name(to_intel_crtc(crtc)-pipe));
+ for (i = 0; i len; i++)
+ I915_WRITE(hdmiw_hdmiedid, *((uint32_t *)eld + i));
Thanks,
Fengguang
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/3] drm/i915: close PM interrupt masking races in the rps work func
On Mon, 5 Sep 2011 08:38:07 +0200 Daniel Vetter dan...@ffwll.ch wrote: On Sun, Sep 04, 2011 at 09:38:56PM +, Ben Widawsky wrote: Oops, you're totally right, I think I meant: - I915_WRITE(GEN6_PMIMR, pm_imr ~pm_iir); + I915_WRITE(GEN6_PMIMR, dev_priv-pm_iir); Imo still racy without the irqsafe rps_lock around it. gcc is free to compile that into a separate load and store which the irq handler can get in between and change dev_priv-pm_iir and PMIMR. The race is now only one instruction wide, though ;-) -Daniel You are absolutely correct. The modification to GEN6_PMIMR must be within the protection of rps_lock. Ben ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/2 v2] drm/i915: pass ELD to HDMI/DP audio driver
Add ELD support for Intel Eaglelake, IbexPeak/Ironlake,
SandyBridge/CougarPoint and IvyBridge/PantherPoint chips.
ELD (EDID-Like Data) describes to the HDMI/DP audio driver the audio
capabilities of the plugged monitor. It's built and passed to audio
driver in 2 steps:
(1) at get_modes time, parse EDID and save ELD to drm_connector.eld[]
(2) at mode_set time, write drm_connector.eld[] to the Transcoder's hw
ELD buffer and set the ELD_valid bit to inform HDMI/DP audio driver
This patch is tested OK on G45/HDMI, IbexPeak/HDMI and IvyBridge/HDMI+DP.
Test scheme: plug in the HDMI/DP monitor, and run
cat /proc/asound/card0/eld*
to check if the monitor name, HDMI/DP type, etc. show up correctly.
Minor imperfection: the GEN5_AUD_CNTL_ST/DIP_Port_Select field always
reads 0 (reserved). Without knowing the port number, I worked it around
by setting the ELD_valid bit for ALL the three ports. It's tested to not
be a problem, because the audio driver will either find unpresent
monitor or invalid ELD data and abort rightfully even when it sees the
ELD_valid indicator.
Thanks to Zhenyu and Pierre-Louis for a lot of valuable help and testing.
CC: Zhao Yakui yakui.z...@intel.com
CC: Wang Zhenyu zhenyu.z.w...@intel.com
CC: Jeremy Bush contractfrombe...@gmail.com
CC: Christopher White c.wh...@pulseforce.com
CC: Paul Menzel paulepan...@users.sourceforge.net
CC: Pierre-Louis Bossart pierre-louis.boss...@intel.com
Signed-off-by: Wu Fengguang fengguang...@intel.com
---
drivers/gpu/drm/i915/i915_drv.h |2
drivers/gpu/drm/i915/i915_reg.h | 25
drivers/gpu/drm/i915/intel_display.c | 131 -
drivers/gpu/drm/i915/intel_dp.c |6 -
drivers/gpu/drm/i915/intel_drv.h |2
drivers/gpu/drm/i915/intel_hdmi.c|3
drivers/gpu/drm/i915/intel_modes.c |2
7 files changed, 169 insertions(+), 2 deletions(-)
--- linux-next.orig/drivers/gpu/drm/i915/intel_hdmi.c 2011-09-05
14:12:08.0 +0800
+++ linux-next/drivers/gpu/drm/i915/intel_hdmi.c2011-09-05
14:12:31.0 +0800
@@ -245,8 +245,11 @@ static void intel_hdmi_mode_set(struct d
sdvox |= HDMI_MODE_SELECT;
if (intel_hdmi-has_audio) {
+ DRM_DEBUG_DRIVER(Enabling HDMI audio on pipe %c\n,
+pipe_name(intel_crtc-pipe));
sdvox |= SDVO_AUDIO_ENABLE;
sdvox |= SDVO_NULL_PACKETS_DURING_VSYNC;
+ intel_write_eld(encoder, adjusted_mode);
}
if (intel_crtc-pipe == 1) {
--- linux-next.orig/drivers/gpu/drm/i915/intel_display.c2011-09-05
14:12:08.0 +0800
+++ linux-next/drivers/gpu/drm/i915/intel_display.c 2011-09-05
14:46:21.0 +0800
@@ -31,6 +31,7 @@
#include linux/kernel.h
#include linux/slab.h
#include linux/vgaarb.h
+#include drm/drm_edid.h
#include drmP.h
#include intel_drv.h
#include i915_drm.h
@@ -5667,6 +5668,131 @@ static int intel_crtc_mode_set(struct dr
return ret;
}
+static void g4x_write_eld(struct drm_connector *connector,
+ struct drm_crtc *crtc)
+{
+ struct drm_i915_private *dev_priv = connector-dev-dev_private;
+ uint8_t *eld = connector-eld;
+ uint32_t eldv;
+ uint32_t len;
+ uint32_t i;
+
+ i = I915_READ(G4X_AUD_VID_DID);
+
+ if (i == INTEL_AUDIO_DEVBLC || i == INTEL_AUDIO_DEVCL)
+ eldv = G4X_ELDV_DEVCL_DEVBLC;
+ else
+ eldv = G4X_ELDV_DEVCTG;
+
+ i = I915_READ(G4X_AUD_CNTL_ST);
+ i = ~(eldv | G4X_ELD_ADDR);
+ len = (i 9) 0x1f; /* ELD buffer size */
+ I915_WRITE(G4X_AUD_CNTL_ST, i);
+
+ if (!eld[0])
+ return;
+
+ len = min_t(uint8_t, eld[2], len);
+ DRM_DEBUG_DRIVER(writing %d ELD dword(s) on pipe %c\n,
+len, pipe_name(to_intel_crtc(crtc)-pipe));
+ for (i = 0; i len; i++)
+ I915_WRITE(G4X_HDMIW_HDMIEDID, *((uint32_t *)eld + i));
+
+ i = I915_READ(G4X_AUD_CNTL_ST);
+ i |= eldv;
+ I915_WRITE(G4X_AUD_CNTL_ST, i);
+}
+
+static void ironlake_write_eld(struct drm_connector *connector,
+struct drm_crtc *crtc)
+{
+ struct drm_i915_private *dev_priv = connector-dev-dev_private;
+ uint8_t *eld = connector-eld;
+ uint32_t eldv;
+ uint32_t i;
+ int len;
+ int aud_cntl_st;
+ int aud_cntl_st2;
+ int hdmiw_hdmiedid;
+
+ if (IS_IVYBRIDGE(connector-dev)) {
+ aud_cntl_st = GEN7_AUD_CNTRL_ST_A;
+ aud_cntl_st2 = GEN7_AUD_CNTRL_ST2;
+ hdmiw_hdmiedid = GEN7_HDMIW_HDMIEDID_A;
+ } else {
+ aud_cntl_st = GEN5_AUD_CNTL_ST_A;
+ aud_cntl_st2 = GEN5_AUD_CNTL_ST2;
+ hdmiw_hdmiedid = GEN5_HDMIW_HDMIEDID_A;
+ }
+
+ i = to_intel_crtc(crtc)-pipe;
+ hdmiw_hdmiedid += i * 0x100;
+ aud_cntl_st += i * 0x100;
Re: [Intel-gfx] [PATCH] drm/i915: Dumb down the semaphore logic
On Sun, 4 Sep 2011 20:52:42 -0700 Ben Widawsky b...@bwidawsk.net wrote: While I think the previous code is correct, it was hard to follow and hard to debug. Since we already have a ring abstraction, might as well use it to handle the semaphore updates and compares. I don't expect this code to make semaphores better or worse, but you never know... Cc: Chris Wilson ch...@chris-wilson.co.uk Cc: Daniel Vetter daniel.vet...@ffwll.ch Cc: Eric Anholt e...@anholt.net Signed-off-by: Ben Widawsky b...@bwidawsk.net I just realized I forgot to use the new invalid #define in the code. I'll resubmit that tomorrow. Ben ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: properly cancel rps_work on module unload
The rps disabling code wasn't properly cancelling outstanding work items. Also add a comment that explains why we're not racing with the work item that could unmask interrupts - that piece of code confused me quite a bit. v2: Ben Widawsky pointed out that the first patch would deadlock (and a few lesser problems). All corrected. Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch --- drivers/gpu/drm/i915/intel_display.c |5 + 1 files changed, 5 insertions(+), 0 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 56a8554..2e425be 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -7484,6 +7484,10 @@ void gen6_disable_rps(struct drm_device *dev) I915_WRITE(GEN6_RPNSWREQ, 1 31); I915_WRITE(GEN6_PMINTRMSK, 0x); I915_WRITE(GEN6_PMIER, 0); + /* Complete PM interrupt masking here doesn't race with the rps work +* item again unmasking PM interrupts because that is using a different +* register (PMIMR) to mask PM interrupts. The only risk is in leaving +* stale bits in PMIIR and PMIMR which gen6_enable_rps will clean up. */ spin_lock_irq(dev_priv-rps_lock); dev_priv-pm_iir = 0; @@ -8478,6 +8482,7 @@ void intel_modeset_cleanup(struct drm_device *dev) * enqueue unpin/hotplug work. */ drm_irq_uninstall(dev); cancel_work_sync(dev_priv-hotplug_work); + cancel_work_sync(dev_priv-rps_work); /* flush any delayed tasks or pending work */ flush_scheduled_work(); -- 1.7.6 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v5] drm/i915: pass ELD to HDMI/DP audio driver
On Mon, 5 Sep 2011 09:14:00 +0800, Wu Fengguang fengguang...@intel.com wrote: On Sun, Sep 04, 2011 at 08:08:37PM +0800, Chris Wilson wrote: On Sun, 4 Sep 2011 05:15:10 +0800, Wu Fengguang fengguang...@intel.com wrote: This patch should be split between adding the core drm functionality to build the ELD and the introduction of i915 support. OK. I didn't do this because I was not sure if it's OK to just add the drm_*() functions without any code to call them.. Right, we don't introduce new interfaces without users - but it is acceptable to say ... which will be used by i915 in a following patch if you want to clarify the need for the interface. It just helps to compartmentalize the damage should we find something goes horribly wrong later. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/3] drm/i915: close PM interrupt masking races in the rps work func
On Sun, 4 Sep 2011 23:51:52 -0700, Ben Widawsky b...@bwidawsk.net wrote: On Mon, 5 Sep 2011 08:38:07 +0200 Daniel Vetter dan...@ffwll.ch wrote: On Sun, Sep 04, 2011 at 09:38:56PM +, Ben Widawsky wrote: Oops, you're totally right, I think I meant: - I915_WRITE(GEN6_PMIMR, pm_imr ~pm_iir); + I915_WRITE(GEN6_PMIMR, dev_priv-pm_iir); Imo still racy without the irqsafe rps_lock around it. gcc is free to compile that into a separate load and store which the irq handler can get in between and change dev_priv-pm_iir and PMIMR. The race is now only one instruction wide, though ;-) -Daniel You are absolutely correct. The modification to GEN6_PMIMR must be within the protection of rps_lock. Right. However, I don't see why we need to hold the mutex though. Why are we touching PMIMR in gen6_enable_rps()? Afaics, it is because gen6_disable_rps() may leave a stale PMIMR (it sets dev_priv-pm_iir to 0, causing the existing work-handler to return early and not touch PMIMR). I believe everything is correct if we clear PMIMR on module load, remove the setting of PMIMR from gen6_enable_rps() and clear PMIMR under the rps lock at the top of the work handler (which both Daniel and I have desired to do... ;-) -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v5] drm/i915: pass ELD to HDMI/DP audio driver
On Mon, Sep 05, 2011 at 07:04:50PM +0800, Chris Wilson wrote: On Mon, 5 Sep 2011 09:14:00 +0800, Wu Fengguang fengguang...@intel.com wrote: On Sun, Sep 04, 2011 at 08:08:37PM +0800, Chris Wilson wrote: On Sun, 4 Sep 2011 05:15:10 +0800, Wu Fengguang fengguang...@intel.com wrote: This patch should be split between adding the core drm functionality to build the ELD and the introduction of i915 support. OK. I didn't do this because I was not sure if it's OK to just add the drm_*() functions without any code to call them.. Right, we don't introduce new interfaces without users - but it is acceptable to say ... which will be used by i915 in a following patch if you want to clarify the need for the interface. It just helps to compartmentalize the damage should we find something goes horribly wrong later. Sounds pretty reasonable. And the logical split may help make a bit easier to review :) Fengguang ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [Xen-devel] Intel IGP VGA-passthrough to Ubuntu 11.04/openSUSE domU doesn't quite work
On Mon, Aug 29, 2011 at 9:36 PM, Konrad Rzeszutek Wilk
konrad.w...@oracle.com wrote:
On Mon, Aug 29, 2011 at 05:50:23PM +0200, Martin Meier wrote:
Hi,
when comparing the dmesgs from a Ubuntu 11.04+xorg-edgers-ppa running on real
hardware ver. running in a HVM-domU, I see this change in dmesg:
real:
[ 2.306326] [drm:intel_wait_for_vblank], vblank wait timed out
[ 2.307140] [drm:gen6_fdi_link_train], FDI_RX_IIR 0x700
[ 2.307143] [drm:gen6_fdi_link_train], FDI train 1 done.
[ 2.307798] [drm:gen6_fdi_link_train], FDI_RX_IIR 0x600
[ 2.307801] [drm:gen6_fdi_link_train], FDI train 2 done.
[ 2.307802] [drm:gen6_fdi_link_train], FDI train done.
domU:
[..]
[ 3.661137] [drm:gen6_fdi_link_train], FDI_RX_IIR 0x0
[ 3.661140] [drm:gen6_fdi_link_train] *ERROR* FDI train 2 fail!
[ 3.661142] [drm:gen6_fdi_link_train], FDI train done.
Hm, this might be a symptom of an earlier error in the boot process I
hadn't noticed last week:
real:
i915 :00:02.0: setting latency timer to 64
[drm:intel_opregion_setup], graphic opregion physical addr: 0xbc8d6018
[drm:intel_opregion_setup], Public ACPI methods supported
[drm:intel_opregion_setup], SWSCI supported
[drm:intel_opregion_setup], ASLE supported
[drm:intel_detect_pch], Found CougarPoint PCH
[drm:intel_parse_bios], Using VBT from OpRegion: $VBT SNB/IVB-DESKTOPd
domU.
i915 :00:02.0: setting latency timer to 64
[drm:intel_opregion_setup], graphic opregion physical addr: 0xbc8d6018
drm:intel_opregion_setup], opregion signature mismatch
i915 :00:02.0: irq 64 for MSI/MSI-X
[drm] Supports vblank timestamp caching Rev 1 (10.10.2010).
[drm] Driver supports precise vblank timestamp query.
[drm:parse_general_definitions], crt_ddc_bus_pin: 5
By searching for opregion signature mismatch I found in intel_opregion.c:
#define OPREGION_SIGNATURE IntelGraphicsMem
[..]
pci_read_config_dword(dev-pdev, PCI_ASLS, asls);
DRM_DEBUG_DRIVER(graphic opregion physical addr: 0x%x\n, asls);
if (asls == 0) {
DRM_DEBUG_DRIVER(ACPI OpRegion not supported!\n);
return -ENOTSUPP;
}
base = acpi_os_ioremap(asls, OPREGION_SIZE);
if (!base)
return -ENOMEM;
if (memcmp(base, OPREGION_SIGNATURE, 16)) {
DRM_DEBUG_DRIVER(opregion signature mismatch\n);
err = -EINVAL;
goto err_out;
}
On the xen side(/var/log/xen/qemu-dm-domU.log) I see:
register_vga_regions: register_vga: igd_opregion = bc8d6018
dm-command: hot insert pass-through pci dev
register_real_device: Assigning real physical device 00:02.0 ...
pt_iomul_init: Error: pt_iomul_init can't open file
/dev/xen/pci_iomul: No such file or directory: 0x0:0x2.0x0
pt_register_regions: IO region registered (size=0x0040 base_addr=0xfe04)
pt_register_regions: IO region registered (size=0x1000 base_addr=0xd00c)
pt_register_regions: IO region registered (size=0x0040 base_addr=0xf001)
register_vga_regions: register_vga: igd_opregion = bc8d6018
pt_msi_setup: msi mapped with pirq 37
pci_intx: intx=1
register_real_device: Real physical device 00:02.0 registered successfuly!
IRQ type = MSI-INTx
igd_pci_read: pci_config_read: 0:0.0: addr=0 len=2 val=8086
igd_pci_read: pci_config_read: 0:0.0: addr=2 len=2 val=0100
pt_iomem_map: e_phys=e000 maddr=d000 type=8 len=268435456
index=2 first_map=1
pt_iomem_map: e_phys=f100 maddr=fe00 type=0 len=4194304
index=0 first_map=1
pt_ioport_map: e_phys=c100 pio_base=f000 len=64 index=4 first_map=1
igd_pci_read: pci_config_read: 0:0.0: addr=0 len=2 val=8086
igd_pci_read: pci_config_read: 0:0.0: addr=2 len=2 val=0100
Is can't open file /dev/xen/pci_iomul a real problem?
I'm not sure where to go from here...
Hardware:
DQ67SW (vt-d enabled)
i5 2400
Display connected via DVI-D / DVI-I+VGA adapter
Software:
domU kernel: 3.1.0-rc3 x86 32 bit
dom0 kernel: 3.0.3
(git://git.kernel.org/pub/scm/linux/kernel/git/konrad/xen.git) x86_64
An you have CONFIG_DMAR enabled?
Yes, CONFIG_DMAR is set to 'y' for the dom0.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: properly cancel rps_work on module unload
On Mon, 5 Sep 2011 10:15:28 +0200 Daniel Vetter daniel.vet...@ffwll.ch wrote: The rps disabling code wasn't properly cancelling outstanding work items. Also add a comment that explains why we're not racing with the work item that could unmask interrupts - that piece of code confused me quite a bit. v2: Ben Widawsky pointed out that the first patch would deadlock (and a few lesser problems). All corrected. Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch Reviewed-by: Ben Widawsky b...@bwidawsk.net This looks good. I think we need to do something in i915_save_state() too though. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] intel_gpu_mon: continous CPU, GPU and power monitoring tool.
From: Eugeni Dodonov eugeni.dodo...@intel.com
This tool is based on ideas from intel_gpu_top, top and powertop
applications, combining them into one continous profiler which generates
human-readable output further processable by gnuplot and similar
applications.
It can be used either in continuous monitoring way (e.g., like top
and powertop, it will run until stopped), or could be used to execute and
profile specific applications with parameters.
Sample execution:
CAIRO_TEST_TARGET=gl intel_gpu_mon -s 100 -o cairo_perf_trace.log \
-b BAT0 cairo-perf-trace gvim
Profiling: /home/eugeni/intel/src/cairo/perf/cairo-perf-trace /tmp/gvim
time user%sys%volts render% ops bits% ops ...
1.00 26.63 1.010 4 7 0 0 ...
2.00 25.12 1.230 0 0 0 0 ...
3.01 25.88 0.500 7 140 0 ...
...
Signed-off-by: Eugeni Dodonov eugeni.dodo...@intel.com
---
tools/Makefile.am |1 +
tools/intel_gpu_mon.c | 582 +
2 files changed, 583 insertions(+), 0 deletions(-)
diff --git a/tools/Makefile.am b/tools/Makefile.am
index 86fafd7..5057495 100644
--- a/tools/Makefile.am
+++ b/tools/Makefile.am
@@ -5,6 +5,7 @@ bin_PROGRAMS = \
intel_bios_dumper \
intel_bios_reader \
intel_error_decode \
+ intel_gpu_mon \
intel_gpu_top \
intel_gpu_time \
intel_gtt \
diff --git a/tools/intel_gpu_mon.c b/tools/intel_gpu_mon.c
new file mode 100644
index 000..9fc4fd3
--- /dev/null
+++ b/tools/intel_gpu_mon.c
@@ -0,0 +1,582 @@
+/*
+ * Copyright © 2007 Intel Corporation
+ * Copyright © 2011 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the Software),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ * DEALINGS IN THE SOFTWARE.
+ *
+ * Authors:
+ *Eric Anholt e...@anholt.net
+ *Eugeni Dodonov eugeni.dodo...@intel.com
+ *
+ */
+
+#include unistd.h
+#include stdlib.h
+#include stdio.h
+#include err.h
+#include string.h
+#include sys/ioctl.h
+#include sys/wait.h
+#include sys/time.h
+#include intel_gpu_tools.h
+#include instdone.h
+
+#define FORCEWAKE 0xA18C
+#define FORCEWAKE_ACK 0x130090
+
+#define SAMPLES_PER_SEC 1
+#define SAMPLES_TO_PERCENT_RATIO(SAMPLES_PER_SEC / 100)
+
+#define MAX_NUM_TOP_BITS100
+
+#define HAS_STATS_REGS(devid) IS_965(devid)
+
+#define BATTERY BAT1
+
+struct top_bit {
+struct instdone_bit *bit;
+int count;
+} top_bits[MAX_NUM_TOP_BITS];
+
+static uint32_t instdone, instdone1;
+static uint32_t devid;
+
+enum stats_counts {
+IA_VERTICES,
+IA_PRIMITIVES,
+VS_INVOCATION,
+GS_INVOCATION,
+GS_PRIMITIVES,
+CL_INVOCATION,
+CL_PRIMITIVES,
+PS_INVOCATION,
+PS_DEPTH,
+STATS_COUNT
+};
+
+const uint32_t stats_regs[STATS_COUNT] = {
+IA_VERTICES_COUNT_QW,
+IA_PRIMITIVES_COUNT_QW,
+VS_INVOCATION_COUNT_QW,
+GS_INVOCATION_COUNT_QW,
+GS_PRIMITIVES_COUNT_QW,
+CL_INVOCATION_COUNT_QW,
+CL_PRIMITIVES_COUNT_QW,
+PS_INVOCATION_COUNT_QW,
+PS_DEPTH_COUNT_QW,
+};
+
+const char *stats_reg_names[STATS_COUNT] = {
+vert f,
+prim f,
+VS inv,
+GS inv,
+GS prim,
+CL inv,
+CL prim,
+PS inv,
+PS dpt,
+};
+
+uint64_t stats[STATS_COUNT];
+uint64_t last_stats[STATS_COUNT];
+
+static int samples_per_sec = SAMPLES_PER_SEC;
+static int samples_to_percent_ratio = (SAMPLES_PER_SEC / 100);
+
+struct cpudata
+{
+unsigned long user, system, idle, nice, total;
+};
+
+struct proc_cpudata
+{
+unsigned long userproc, systemproc, vmem, rmem;
+char pname[255];
+};
+
+struct powerdata
+{
+long rate;
+};
+
+static unsigned long
+gettime(void)
+{
+struct timeval t;
+gettimeofday(t, NULL);
+return (t.tv_usec + (t.tv_sec * 100));
+}
+
+static void
+get_cpu_stat(struct cpudata *cpu)
+{
+FILE *file;
+char temp[10];
Re: [Intel-gfx] [PATCH] intel_gpu_mon: continous CPU, GPU and power monitoring tool.
On Mon, 5 Sep 2011 14:56:42 -0300, Eugeni Dodonov eug...@dodonov.net wrote: From: Eugeni Dodonov eugeni.dodo...@intel.com This tool is based on ideas from intel_gpu_top, top and powertop applications, combining them into one continous profiler which generates human-readable output further processable by gnuplot and similar applications. It can be used either in continuous monitoring way (e.g., like top and powertop, it will run until stopped), or could be used to execute and profile specific applications with parameters. Ok, this looks like a useful extension to intel_gpu_top (until the day arrives when we have a real, integrated profiler!). I don't see the need for a separate utility with lots of code duplicated just for a couple of extra features. Care to respin as a patch against intel_gpu_top? (I want to resist the temptation of making a second perf... especially as I'd like to make all this information available through perf!) -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] intel_gpu_mon: continous CPU, GPU and power monitoring tool.
On Mon, Sep 5, 2011 at 15:22, Chris Wilson ch...@chris-wilson.co.uk wrote: On Mon, 5 Sep 2011 14:56:42 -0300, Eugeni Dodonov eug...@dodonov.net wrote: From: Eugeni Dodonov eugeni.dodo...@intel.com This tool is based on ideas from intel_gpu_top, top and powertop applications, combining them into one continous profiler which generates human-readable output further processable by gnuplot and similar applications. It can be used either in continuous monitoring way (e.g., like top and powertop, it will run until stopped), or could be used to execute and profile specific applications with parameters. Ok, this looks like a useful extension to intel_gpu_top (until the day arrives when we have a real, integrated profiler!). I don't see the need for a separate utility with lots of code duplicated just for a couple of extra features. Care to respin as a patch against intel_gpu_top? (I want to resist the temptation of making a second perf... especially as I'd like to make all this information available through perf!) Sure, will do. -- Eugeni Dodonov http://eugeni.dodonov.net/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Enable dither whenever display bpc frame buffer bpc
We want to enable dithering on any pipe where the frame buffer has more color resolution than the output device. The previous code was incorrectly clamping the frame buffer bpc to the display bpc, effectively disabling dithering all of the time as the computed frame buffer bpc would never be larger than the display bpc. Signed-off-by: Keith Packard kei...@keithp.com Reported-by: Oliver Hartkopp socket...@hartkopp.net Tested-by: Oliver Hartkopp socket...@hartkopp.net --- drivers/gpu/drm/i915/intel_display.c | 10 ++ 1 files changed, 6 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 56a8554..9fb4a40 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -4687,13 +4687,13 @@ static bool intel_choose_pipe_bpp_dither(struct drm_crtc *crtc, bpc = 6; /* min is 18bpp */ break; case 24: - bpc = min((unsigned int)8, display_bpc); + bpc = 8; break; case 30: - bpc = min((unsigned int)10, display_bpc); + bpc = 10; break; case 48: - bpc = min((unsigned int)12, display_bpc); + bpc = 12; break; default: DRM_DEBUG(unsupported depth, assuming 24 bits\n); @@ -4701,10 +4701,12 @@ static bool intel_choose_pipe_bpp_dither(struct drm_crtc *crtc, break; } + display_bpc = min(display_bpc, bpc); + DRM_DEBUG_DRIVER(setting pipe bpc to %d (max display bpc %d)\n, bpc, display_bpc); - *pipe_bpp = bpc * 3; + *pipe_bpp = display_bpc * 3; return display_bpc != bpc; } -- 1.7.5.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 0/6] additional intel_gpu_top profiling features
From: Eugeni Dodonov eugeni.dodo...@intel.com This patchset adds a number of small but useful features into intel_gpu_top tool, as asked by Chris Wilson: - support for getopt to customize internal parameters at runtime (such as number of samplings per second). - possibility of non-interactive execution to collect GPU usage into a log file for future analysis - possibility to profile a specific command, leaving when the profiled command reaches its end of the execution - collection of initial statistics before the profile starts. - account for the time spent within syscalls to provide more appropriate logging and more precise screen updates (e.g., instead of refreshing once a second, we could potentially take way longer than that due to the system calls overhead) Eugeni Dodonov (6): intel_gpu_top: account for time spent in syscalls intel_gpu_top: suport command line parameters and variable samples per second intel_gpu_tool: initial support for non-screen output intel_gpu_top: initialize monitoring statistics at startup intel_gpu_top: support non-interactive mode intel_gpu_top: support profiling user-specified commands man/intel_gpu_top.1 | 22 tools/intel_gpu_top.c | 303 +--- 2 files changed, 280 insertions(+), 45 deletions(-) -- 1.7.6.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/6] intel_gpu_top: account for time spent in syscalls
From: Eugeni Dodonov eugeni.dodo...@intel.com
This allows intel_gpu_top to properly account for time spent inside system
calls. Effectively, with previous implementation, intel_gpu_top could
spent longer than 1s between consecutive measures. This attempts to
minimize the extra time spent while polling for data.
Signed-off-by: Eugeni Dodonov eugeni.dodo...@intel.com
---
tools/intel_gpu_top.c | 53 +---
1 files changed, 41 insertions(+), 12 deletions(-)
diff --git a/tools/intel_gpu_top.c b/tools/intel_gpu_top.c
index e9fbf43..64ce828 100644
--- a/tools/intel_gpu_top.c
+++ b/tools/intel_gpu_top.c
@@ -1,5 +1,6 @@
/*
* Copyright © 2007 Intel Corporation
+ * Copyright © 2011 Intel Corporation
*
* Permission is hereby granted, free of charge, to any person obtaining a
* copy of this software and associated documentation files (the Software),
@@ -22,6 +23,7 @@
*
* Authors:
*Eric Anholt e...@anholt.net
+ *Eugeni Dodonov eugeni.dodo...@intel.com
*
*/
@@ -30,6 +32,7 @@
#include stdio.h
#include err.h
#include sys/ioctl.h
+#include sys/time.h
#include intel_gpu_tools.h
#include instdone.h
@@ -104,6 +107,14 @@ const char *stats_reg_names[STATS_COUNT] = {
uint64_t stats[STATS_COUNT];
uint64_t last_stats[STATS_COUNT];
+static unsigned long
+gettime(void)
+{
+struct timeval t;
+gettimeofday(t, NULL);
+return (t.tv_usec + (t.tv_sec * 100));
+}
+
static int
top_bits_sort(const void *a, const void *b)
{
@@ -362,21 +373,23 @@ static void ring_sample(struct ring *ring)
ring-full += full;
}
-static void ring_print(struct ring *ring)
+static void ring_print(struct ring *ring, unsigned long samples_per_sec)
{
- int percent, len;
+ int samples_to_percent_ratio, percent, len;
if (!ring-size)
return;
- percent = 100 - ring-idle / SAMPLES_TO_PERCENT_RATIO;
+ /* Calculate current value of samples_to_percent_ratio */
+ samples_to_percent_ratio = (ring-idle * 100) / samples_per_sec;
+ percent = 100 - samples_to_percent_ratio;
len = printf(%25s busy: %3d%%: , ring-name, percent);
print_percentage_bar (percent, len);
printf(%24s space: %d/%d (%d%%)\n,
ring-name,
- (int)(ring-full / SAMPLES_PER_SEC),
+ (int)(ring-full / samples_per_sec),
ring-size,
- (int)((ring-full / SAMPLES_TO_PERCENT_RATIO) / ring-size));
+ (int)((ring-full / samples_to_percent_ratio) / ring-size));
}
int main(int argc, char **argv)
@@ -418,18 +431,25 @@ int main(int argc, char **argv)
for (;;) {
int j;
+ unsigned long long t1, ti, tf;
+ unsigned long long def_sleep = 100 / SAMPLES_PER_SEC;
+ unsigned long long last_samples_per_sec = SAMPLES_PER_SEC;
char clear_screen[] = {0x1b, '[', 'H',
0x1b, '[', 'J',
0x0};
int percent;
int len;
+ t1 = gettime();
+
ring_reset(render_ring);
ring_reset(bsd_ring);
ring_reset(bsd6_ring);
ring_reset(blt_ring);
for (i = 0; i SAMPLES_PER_SEC; i++) {
+ long long interval;
+ ti = gettime();
if (IS_965(devid)) {
instdone = INREG(INST_DONE_I965);
instdone1 = INREG(INST_DONE_1);
@@ -443,7 +463,16 @@ int main(int argc, char **argv)
ring_sample(bsd_ring);
ring_sample(bsd6_ring);
ring_sample(blt_ring);
- usleep(100 / SAMPLES_PER_SEC);
+
+ tf = gettime();
+ if (tf - t1 = 100) {
+ /* We are out of sync, bail out */
+ last_samples_per_sec = i+1;
+ break;
+ }
+ interval = def_sleep - (tf - ti);
+ if (interval 0)
+ usleep(interval);
}
if (HAS_STATS_REGS(devid)) {
@@ -477,16 +506,16 @@ int main(int argc, char **argv)
print_clock_info(pci_dev);
- ring_print(render_ring);
- ring_print(bsd_ring);
- ring_print(bsd6_ring);
- ring_print(blt_ring);
+ ring_print(render_ring, last_samples_per_sec);
+ ring_print(bsd_ring, last_samples_per_sec);
+ ring_print(bsd6_ring, last_samples_per_sec);
+ ring_print(blt_ring, last_samples_per_sec);
printf(\n%30s %s\n, task, percent busy);
for (i = 0; i max_lines; i++) {
[Intel-gfx] [PATCH 2/6] intel_gpu_top: suport command line parameters and variable samples per second
From: Eugeni Dodonov eugeni.dodo...@intel.com
This patch adds support for getopt, and adds two default parameters to it:
-h to show usage notes; and -s to allow user to define number of samples
to acquire per second.
Manpage documentation is also adjusted accordingly.
Signed-off-by: Eugeni Dodonov eugeni.dodo...@intel.com
---
man/intel_gpu_top.1 |9
tools/intel_gpu_top.c | 52 +---
2 files changed, 57 insertions(+), 4 deletions(-)
diff --git a/man/intel_gpu_top.1 b/man/intel_gpu_top.1
index 79c9c0e..2cbbec9 100644
--- a/man/intel_gpu_top.1
+++ b/man/intel_gpu_top.1
@@ -4,11 +4,20 @@
.SH NAME
intel_gpu_top \- Display a top-like summary of Intel GPU usage
.SH SYNOPSIS
+.nf
.B intel_gpu_top
+.B intel_gpu_top [ parameters ]
.SH DESCRIPTION
.B intel_gpu_top
is a tool to display usage information of an Intel GPU. It requires root
privilege to map the graphics device.
+.SS Options
+.TP
+.B -s [samples per second]
+number of samples to acquire per second
+.TP
+.B -h
+show usage notes
.PP
Note that idle units are not
displayed, so an entirely idle GPU will only display the ring status and
diff --git a/tools/intel_gpu_top.c b/tools/intel_gpu_top.c
index 64ce828..abe9d4b 100644
--- a/tools/intel_gpu_top.c
+++ b/tools/intel_gpu_top.c
@@ -392,6 +392,23 @@ static void ring_print(struct ring *ring, unsigned long
samples_per_sec)
(int)((ring-full / samples_to_percent_ratio) / ring-size));
}
+static void
+usage(const char *appname)
+{
+ printf(intel_gpu_top - Display a top-like summary of Intel GPU usage\n
+ \n
+ usage: %s [parameters]\n
+ \n
+ The following parameters apply:\n
+ [-s samples] samples per seconds (default
%d)\n
+ [-h] show this help screen\n
+ \n,
+ appname,
+ SAMPLES_PER_SEC
+ );
+ return;
+}
+
int main(int argc, char **argv)
{
struct pci_device *pci_dev;
@@ -408,7 +425,34 @@ int main(int argc, char **argv)
.name = blitter,
.mmio = 0x22030,
};
- int i;
+ int i, ch;
+ int samples_per_sec = SAMPLES_PER_SEC;
+
+ /* Parse options? */
+ while ((ch = getopt(argc, argv, s:h)) != -1)
+ {
+ switch (ch)
+ {
+ case 's': samples_per_sec = atoi(optarg);
+ if (samples_per_sec 100) {
+ fprintf(stderr, Error:
samples per second must be = 100\n);
+ exit(1);
+ }
+ break;
+ case 'h':
+ usage(argv[0]);
+ exit(0);
+ break;
+ default:
+ fprintf(stderr, Invalid flag %c!\n,
(char)optopt);
+ usage(argv[0]);
+ exit(1);
+ break;
+ }
+
+ }
+ argc -= optind;
+ argv += optind;
pci_dev = intel_get_pci_device();
devid = pci_dev-device_id;
@@ -432,8 +476,8 @@ int main(int argc, char **argv)
for (;;) {
int j;
unsigned long long t1, ti, tf;
- unsigned long long def_sleep = 100 / SAMPLES_PER_SEC;
- unsigned long long last_samples_per_sec = SAMPLES_PER_SEC;
+ unsigned long long def_sleep = 100 / samples_per_sec;
+ unsigned long long last_samples_per_sec = samples_per_sec;
char clear_screen[] = {0x1b, '[', 'H',
0x1b, '[', 'J',
0x0};
@@ -447,7 +491,7 @@ int main(int argc, char **argv)
ring_reset(bsd6_ring);
ring_reset(blt_ring);
- for (i = 0; i SAMPLES_PER_SEC; i++) {
+ for (i = 0; i samples_per_sec; i++) {
long long interval;
ti = gettime();
if (IS_965(devid)) {
--
1.7.6.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 3/6] intel_gpu_tool: initial support for non-screen output
From: Eugeni Dodonov eugeni.dodo...@intel.com
This patch adds initial support for non-stdio output, to be used for
non-interactive monitoring.
Signed-off-by: Eugeni Dodonov eugeni.dodo...@intel.com
---
tools/intel_gpu_top.c | 28 +++-
1 files changed, 15 insertions(+), 13 deletions(-)
diff --git a/tools/intel_gpu_top.c b/tools/intel_gpu_top.c
index abe9d4b..edb4a82 100644
--- a/tools/intel_gpu_top.c
+++ b/tools/intel_gpu_top.c
@@ -373,7 +373,8 @@ static void ring_sample(struct ring *ring)
ring-full += full;
}
-static void ring_print(struct ring *ring, unsigned long samples_per_sec)
+static void ring_print(struct ring *ring, unsigned long samples_per_sec,
+ FILE *output)
{
int samples_to_percent_ratio, percent, len;
@@ -383,9 +384,9 @@ static void ring_print(struct ring *ring, unsigned long
samples_per_sec)
/* Calculate current value of samples_to_percent_ratio */
samples_to_percent_ratio = (ring-idle * 100) / samples_per_sec;
percent = 100 - samples_to_percent_ratio;
- len = printf(%25s busy: %3d%%: , ring-name, percent);
+ len = fprintf(output, %25s busy: %3d%%: , ring-name, percent);
print_percentage_bar (percent, len);
- printf(%24s space: %d/%d (%d%%)\n,
+ fprintf(output, %24s space: %d/%d (%d%%)\n,
ring-name,
(int)(ring-full / samples_per_sec),
ring-size,
@@ -427,6 +428,7 @@ int main(int argc, char **argv)
};
int i, ch;
int samples_per_sec = SAMPLES_PER_SEC;
+ FILE *output = stdout;
/* Parse options? */
while ((ch = getopt(argc, argv, s:h)) != -1)
@@ -546,30 +548,30 @@ int main(int argc, char **argv)
if (max_lines = num_instdone_bits)
max_lines = num_instdone_bits;
- printf(%s, clear_screen);
+ fprintf(output, %s, clear_screen);
print_clock_info(pci_dev);
- ring_print(render_ring, last_samples_per_sec);
- ring_print(bsd_ring, last_samples_per_sec);
- ring_print(bsd6_ring, last_samples_per_sec);
- ring_print(blt_ring, last_samples_per_sec);
+ ring_print(render_ring, last_samples_per_sec, output);
+ ring_print(bsd_ring, last_samples_per_sec, output);
+ ring_print(bsd6_ring, last_samples_per_sec, output);
+ ring_print(blt_ring, last_samples_per_sec, output);
- printf(\n%30s %s\n, task, percent busy);
+ fprintf(output, \n%30s %s\n, task, percent busy);
for (i = 0; i max_lines; i++) {
if (top_bits_sorted[i]-count 0) {
percent = (top_bits_sorted[i]-count * 100) /
last_samples_per_sec;
- len = printf(%30s: %3d%%: ,
+ len = fprintf(output, %30s: %3d%%: ,
top_bits_sorted[i]-bit-name,
percent);
print_percentage_bar (percent, len);
} else {
- printf(%*s, PERCENTAGE_BAR_END, );
+ fprintf(output, %*s, PERCENTAGE_BAR_END, );
}
if (i STATS_COUNT HAS_STATS_REGS(devid)) {
- printf(%13s: %llu (%lld/sec),
+ fprintf(output, %13s: %llu (%lld/sec),
stats_reg_names[i],
stats[i],
stats[i] - last_stats[i]);
@@ -578,7 +580,7 @@ int main(int argc, char **argv)
if (!top_bits_sorted[i]-count)
break;
}
- printf(\n);
+ fprintf(output, \n);
}
for (i = 0; i num_instdone_bits; i++) {
--
1.7.6.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 4/6] intel_gpu_top: initialize monitoring statistics at startup
From: Eugeni Dodonov eugeni.dodo...@intel.com
This patch initializes the last_stats[] for registers prior to starting
the monitoring itself. This way, the first measure will already contain
the difference from the previous value instead of non-initialized value.
Signed-off-by: Eugeni Dodonov eugeni.dodo...@intel.com
---
tools/intel_gpu_top.c | 16
1 files changed, 16 insertions(+), 0 deletions(-)
diff --git a/tools/intel_gpu_top.c b/tools/intel_gpu_top.c
index edb4a82..e2dd173 100644
--- a/tools/intel_gpu_top.c
+++ b/tools/intel_gpu_top.c
@@ -475,6 +475,22 @@ int main(int argc, char **argv)
ring_init(blt_ring);
}
+/* Initialize GPU stats */
+if (HAS_STATS_REGS(devid)) {
+for (i = 0; i STATS_COUNT; i++) {
+uint32_t stats_high, stats_low, stats_high_2;
+
+do {
+stats_high = INREG(stats_regs[i] + 4);
+stats_low = INREG(stats_regs[i]);
+stats_high_2 = INREG(stats_regs[i] + 4);
+} while (stats_high != stats_high_2);
+
+last_stats[i] = (uint64_t)stats_high 32 |
+stats_low;
+}
+}
+
for (;;) {
int j;
unsigned long long t1, ti, tf;
--
1.7.6.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 5/6] intel_gpu_top: support non-interactive mode
From: Eugeni Dodonov eugeni.dodo...@intel.com
This patch adds support for non-interactive mode, invoked by running with
'-o output' switch. In this case, no interactive output is being
performed, but the execution statistics are being saved into the output
file.
The output file is generated in both human and gnuplot-readable format.
Unlike interactive mode, where non-supported pipes and non-active
registers are skipped, the content of such pipes and registers is recorded
into the log file to simplify parsing and standardize the list of columns.
Also, unlike interactive mode, the registers are not sorted according to
the usage - this way, their variation over time can be analysed offline.
Signed-off-by: Eugeni Dodonov eugeni.dodo...@intel.com
---
man/intel_gpu_top.1 |3 +
tools/intel_gpu_top.c | 150 +++-
2 files changed, 112 insertions(+), 41 deletions(-)
diff --git a/man/intel_gpu_top.1 b/man/intel_gpu_top.1
index 2cbbec9..bca83f0 100644
--- a/man/intel_gpu_top.1
+++ b/man/intel_gpu_top.1
@@ -16,6 +16,9 @@ privilege to map the graphics device.
.B -s [samples per second]
number of samples to acquire per second
.TP
+.B -o [output file]
+run non-interactively and collect usage statistics to [file]
+.TP
.B -h
show usage notes
.PP
diff --git a/tools/intel_gpu_top.c b/tools/intel_gpu_top.c
index e2dd173..88f7157 100644
--- a/tools/intel_gpu_top.c
+++ b/tools/intel_gpu_top.c
@@ -373,24 +373,39 @@ static void ring_sample(struct ring *ring)
ring-full += full;
}
+static void ring_print_header(FILE *out, struct ring *ring)
+{
+fprintf(out, %.6s%%\tops\t,
+ring-name
+ );
+}
+
static void ring_print(struct ring *ring, unsigned long samples_per_sec,
FILE *output)
{
int samples_to_percent_ratio, percent, len;
- if (!ring-size)
- return;
-
/* Calculate current value of samples_to_percent_ratio */
samples_to_percent_ratio = (ring-idle * 100) / samples_per_sec;
percent = 100 - samples_to_percent_ratio;
- len = fprintf(output, %25s busy: %3d%%: , ring-name, percent);
- print_percentage_bar (percent, len);
- fprintf(output, %24s space: %d/%d (%d%%)\n,
- ring-name,
- (int)(ring-full / samples_per_sec),
- ring-size,
- (int)((ring-full / samples_to_percent_ratio) / ring-size));
+
+ if (output == stdout) {
+ if (!ring-size)
+ return;
+
+ len = fprintf(output, %25s busy: %3d%%: , ring-name,
percent);
+ print_percentage_bar (percent, len);
+ fprintf(output, %24s space: %d/%d (%d%%)\n,
+ ring-name,
+ (int)(ring-full / samples_per_sec),
+ ring-size,
+ (int)((ring-full / samples_to_percent_ratio) /
ring-size));
+ } else {
+ fprintf(output, %3d\t%d\t,
+ (ring-size) ? 100 - ring-idle /
samples_to_percent_ratio : -1,
+ (ring-size) ? (int)(ring-full / samples_per_sec) :
-1
+ );
+ }
}
static void
@@ -402,6 +417,7 @@ usage(const char *appname)
\n
The following parameters apply:\n
[-s samples] samples per seconds (default
%d)\n
+[-o file] output to file (default to stdio)\n
[-h] show this help screen\n
\n,
appname,
@@ -429,9 +445,11 @@ int main(int argc, char **argv)
int i, ch;
int samples_per_sec = SAMPLES_PER_SEC;
FILE *output = stdout;
+ double elapsed_time=0;
+ int print_headers=1;
/* Parse options? */
- while ((ch = getopt(argc, argv, s:h)) != -1)
+ while ((ch = getopt(argc, argv, s:o:h)) != -1)
{
switch (ch)
{
@@ -441,6 +459,13 @@ int main(int argc, char **argv)
exit(1);
}
break;
+ case 'o': output = fopen(optarg, w);
+ if (!output)
+ {
+ perror(fopen);
+ exit(1);
+ }
+ break;
case 'h':
usage(argv[0]);
exit(0);
@@ -493,7 +518,7 @@ int main(int argc, char **argv)
for (;;) {
int j;
- unsigned long long t1, ti, tf;
+ unsigned long long t1, ti, tf, t2;
unsigned long
[Intel-gfx] [PATCH 6/6] intel_gpu_top: support profiling user-specified commands
From: Eugeni Dodonov eugeni.dodo...@intel.com
This patch adds support for running intel_gpu_top to profile specific
commands. The required command will be carried out in separate process,
and main intel_gpu_top will leave when the child process will exit.
Signed-off-by: Eugeni Dodonov eugeni.dodo...@intel.com
---
man/intel_gpu_top.1 | 10
tools/intel_gpu_top.c | 56 -
2 files changed, 65 insertions(+), 1 deletions(-)
diff --git a/man/intel_gpu_top.1 b/man/intel_gpu_top.1
index bca83f0..db2f362 100644
--- a/man/intel_gpu_top.1
+++ b/man/intel_gpu_top.1
@@ -19,8 +19,18 @@ number of samples to acquire per second
.B -o [output file]
run non-interactively and collect usage statistics to [file]
.TP
+.B -e [command to profile]
+execute a command, and leave when it is finished. Note that the entire command
+with all parameters should be included as one parameter.
+.TP
.B -h
show usage notes
+.SH EXAMPLES
+.TP
+intel_gpu_top -o cairo-trace-gvim.log -s 100 -e cairo-perf-trace /tmp/gvim
+will run cairo-perf-trace with /tmp/gvim trace, non-interactively, saving the
+statistics into cairo-trace-gvim.log file, and collecting 100 samples per
+second.
.PP
Note that idle units are not
displayed, so an entirely idle GPU will only display the ring status and
diff --git a/tools/intel_gpu_top.c b/tools/intel_gpu_top.c
index 88f7157..3619d1b 100644
--- a/tools/intel_gpu_top.c
+++ b/tools/intel_gpu_top.c
@@ -33,6 +33,8 @@
#include err.h
#include sys/ioctl.h
#include sys/time.h
+#include sys/wait.h
+#include string.h
#include intel_gpu_tools.h
#include instdone.h
@@ -447,12 +449,17 @@ int main(int argc, char **argv)
FILE *output = stdout;
double elapsed_time=0;
int print_headers=1;
+ pid_t child_pid=-1;
+ int child_stat;
+ char *cmd=NULL;
/* Parse options? */
- while ((ch = getopt(argc, argv, s:o:h)) != -1)
+ while ((ch = getopt(argc, argv, s:o:e:h)) != -1)
{
switch (ch)
{
+ case 'e': cmd = strdup(optarg);
+ break;
case 's': samples_per_sec = atoi(optarg);
if (samples_per_sec 100) {
fprintf(stderr, Error:
samples per second must be = 100\n);
@@ -481,6 +488,37 @@ int main(int argc, char **argv)
argc -= optind;
argv += optind;
+ /* Do we have a command to run? */
+ if (cmd != NULL)
+ {
+ if (output != stdout) {
+ fprintf(output, # Profiling: %s\n, cmd);
+ fflush(output);
+ }
+ child_pid = fork();
+ if (child_pid 0)
+ {
+ perror(fork);
+ exit(1);
+ }
+ else if (child_pid == 0) {
+ int res;
+ res = system(cmd);
+free(cmd);
+ if (res 0)
+ perror(running command);
+ if (output != stdout) {
+ fflush(output);
+ fprintf(output, # %s exited with status %d\n,
cmd, res);
+ fflush(output);
+ }
+ exit(0);
+ }
+else {
+free(cmd);
+}
+ }
+
pci_dev = intel_get_pci_device();
devid = pci_dev-device_id;
intel_get_mmio(pci_dev);
@@ -673,7 +711,23 @@ int main(int argc, char **argv)
if (i STATS_COUNT)
last_stats[i] = stats[i];
}
+
+ /* Check if child has gone */
+ if (child_pid 0)
+ {
+ int res;
+ if ((res = waitpid(child_pid, child_stat, WNOHANG)) ==
-1) {
+ perror(waitpid);
+ exit(1);
+ }
+ if (res == 0)
+ continue;
+ if (WIFEXITED(child_stat))
+ break;
+ }
}
+ fclose(output);
+
return 0;
}
--
1.7.6.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/6] intel_gpu_top: suport command line parameters and variable samples per second
On Mon, 5 Sep 2011 17:19:29 -0300, Eugeni Dodonov eug...@dodonov.net wrote:
From: Eugeni Dodonov eugeni.dodo...@intel.com
This patch adds support for getopt, and adds two default parameters to it:
-h to show usage notes; and -s to allow user to define number of samples
to acquire per second.
Just a minor style issue, otherwise it looks good. All I need is
someway to correlate GPU activity with batches (and especially the
contents of those batches) and with even higher level code and then I'd
be happy. Oh, and integrated with a timeline of CPU activity, of course.
:-)
+ /* Parse options? */
+ while ((ch = getopt(argc, argv, s:h)) != -1)
+ {
+ switch (ch)
+ {
+ case 's': samples_per_sec = atoi(optarg);
In the modules we own, we have adopted the kernel CODING_STYLE as our
standard. 8 space indents, 80 cols line (except where readibility is
improved by going over), braces on the same line as the control flow,
/*
* This style of long comments.
*/
and case statements should being at the same indentation as the switch
and so should the parameters of a multiline function...
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/6] intel_gpu_top: suport command line parameters and variable samples per second
On Mon, Sep 5, 2011 at 18:44, Chris Wilson ch...@chris-wilson.co.uk wrote: In the modules we own, we have adopted the kernel CODING_STYLE as our standard. 8 space indents, 80 cols line (except where readibility is improved by going over), braces on the same line as the control flow, Fixed, thanks! -- Eugeni Dodonov http://eugeni.dodonov.net/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/6] intel_gpu_tool: initial support for non-screen output
Is that really necessary? Isn't intel_gpu_tool logfile enough? signature.asc Description: This is a digitally signed message part. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/6] intel_gpu_tool: initial support for non-screen output
My bad, it is necessary to add file. signature.asc Description: This is a digitally signed message part. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
