Re: [Intel-gfx] [PATCH] Added a lower limit for the watermark setting
Am 19.11.2013 17:41, schrieb Daniel Vetter:
On Tue, Nov 19, 2013 at 05:15:09PM +0100, Thomas Richter wrote:
Hi Daniel, dear intel experts,
please find a patch attached that finally addresses the display
flicker on i830 chipsets. This
patch adds a lower watermark setting in intel_watermark_params{},
but keeps it zero for
all but the i830 chipsets. The necessary new defines are in i915_reg.h.
I think we ned to split out the gen2/3 single/dual pipe watermark code a
bit better from everything else. A bugfix for i830M shouldn't really
affect snb ;-)
Actually, the fun part is that it does not because all the lower limits
are zero for all other architectures.
I've pushed out my current (and rather broken) wip branch with my idea on
the take to
http://cgit.freedesktop.org/~danvet/drm/log/?h=for-thomas
Could you please help me here how to apply it? I'm not very experienced
with git, and it does not seem to fit to the sources of
intel_pm.c I have. Do I first need to instruct git to download another
branch? I'm currently at drm-intel-nightly.
Thanks,
Thomas
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] intel: make sure VG_CLEAR() will always do memory clear
If valgrind is not available, current VG_CLEAR() would just ignore
memory clear operation which might make invalid ioctl argument. So
make sure VG_CLEAR() will always clear memory.
---
intel/intel_bufmgr_gem.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/intel/intel_bufmgr_gem.c b/intel/intel_bufmgr_gem.c
index df6fcec..389f73a 100644
--- a/intel/intel_bufmgr_gem.c
+++ b/intel/intel_bufmgr_gem.c
@@ -74,7 +74,7 @@
#define VG(x)
#endif
-#define VG_CLEAR(s) VG(memset(s, 0, sizeof(s)))
+#define VG_CLEAR(s) (memset(s, 0, sizeof(s)))
#define DBG(...) do { \
if (bufmgr_gem-bufmgr.debug) \
--
1.8.4.3
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] Added a lower limit for the watermark setting
On Wed, Nov 20, 2013 at 10:18 AM, Thomas Richter t...@math.tu-berlin.de wrote: I think we ned to split out the gen2/3 single/dual pipe watermark code a bit better from everything else. A bugfix for i830M shouldn't really affect snb ;-) Actually, the fun part is that it does not because all the lower limits are zero for all other architectures. What I've meant to say is that I want to split up the watermark code more anyway, so that there's no need to fill in the 0 all over the place where we don't care one bit. I.e. all the gen4+ platforms. I've pushed out my current (and rather broken) wip branch with my idea on the take to http://cgit.freedesktop.org/~danvet/drm/log/?h=for-thomas Could you please help me here how to apply it? I'm not very experienced with git, and it does not seem to fit to the sources of intel_pm.c I have. Do I first need to instruct git to download another branch? I'm currently at drm-intel-nightly. It's more just to read through the patches for ideas. Atm the branch doesn't even compile - I'll try to clean it up and beat it into shape in the next few days. The core idea is to have a minimal wm of 4 (lowest burst setting) and then set the actual burst setting to the watermark rounded down to the next multiple of 4, up to the recommended value in Bspec (which is 16 fifo cachelines). But I've fumbled the job a bit and broke a few things ;-) -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] Added a lower limit for the watermark setting
Am 20.11.2013 10:27, schrieb Daniel Vetter: What I've meant to say is that I want to split up the watermark code more anyway, so that there's no need to fill in the 0 all over the place where we don't care one bit. I.e. all the gen4+ platforms. Ok - well, I guess I cannot judge whether that's necessary or required. Given that the i830 is the only chipset that comes with a unified FIFO, it might be actually necessary. I've pushed out my current (and rather broken) wip branch with my idea on the take to http://cgit.freedesktop.org/~danvet/drm/log/?h=for-thomas Could you please help me here how to apply it? I'm not very experienced with git, and it does not seem to fit to the sources of intel_pm.c I have. Do I first need to instruct git to download another branch? I'm currently at drm-intel-nightly. It's more just to read through the patches for ideas. Atm the branch doesn't even compile - I'll try to clean it up and beat it into shape in the next few days. The core idea is to have a minimal wm of 4 (lowest burst setting) and then set the actual burst setting to the watermark rounded down to the next multiple of 4, up to the recommended value in Bspec (which is 16 fifo cachelines). But I've fumbled the job a bit and broke a few things ;-) Ah, that explains something, thank you. Probably two things: The way how the current code is organized, the wm handling of the i830 is handled in two functions, not one. In i830_update_wm and i9xx_update_wm. The former seems to be used during bootstrap when only one pipe is active, the latter as soon as I activate the second pipe. This is probably needlessly complex, and it would be better to have this just in one place, not two (found that when manually adding a lower limit as first attempt). The second is concerned the lower limit: Four is not enough, six gives a reasonably stable image most the time, but if scrolling wildly, still some hickups. Eight is better, but still not perfect. If you try really, really hard, one can still provoke a crash if a video overlay is active. Unclear why this is - if it happens, the screen seems to go blank for just one frame, then recovers. Given that the plane pointers are only shadow registers that are automatically updated by the chip on a vblank, I have no good idea why that happens. I also see that your code seems to use a modified formula for the latency. Given that I cannot compile the code, it's at this time hard to say whether that's better or worse. The current one is too pessimistic. I tried to find the minimum permissible latency and find that for a pixel clock of 135Mhz, the watermark should be at most 30, and for a pixel clock of 108 Mhz, the watermark should be at most 32. From this one can compute the latencies in ns. If I did everything right, I get values of approximately 1185 ns and 1066 ns, thus approximately in the same ball park. A value of 1500 should likely be fine, but requires testing on additional hardware (I've currently no access to the Fujistu, the other laptop with a i830 chipset and the strange DVO). BTW, even with the maximal watermarks (minimum latency), I do get hickups if I try really really hard. So they are likely provoked by something else, not the wrong watermark. Thanks! Thomas ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 5/9] drm/i915: print object bindings in debugfs
On Tue, Nov 19, 2013 at 09:37:16AM -0800, Rodrigo Vivi wrote: On Tue, Nov 19, 2013 at 5:52 AM, Chris Wilson ch...@chris-wilson.co.uk wrote: On Mon, Nov 18, 2013 at 06:32:34PM -0800, Rodrigo Vivi wrote: From: Daniel Vetter daniel.vet...@ffwll.ch This is useful when we only have aliasing ppgtt and want to figure out what exactly is accesssible and what not. Paulo can somehow overwrite the fbcon framebuffer with the blitter on his hsw machine ... v2: Actually make it compile. Cc: Paulo Zanoni przan...@gmail.com Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch Signed-off-by: Rodrigo Vivi rodrigo.v...@gmail.com This is still too ugly. at least should be the opposite pp before g and complemented with a tt in the end. (bindings: ) (bindings: g) (bindings: pp) (bindings: gpp) -Chris But what are all real possible options? nothing, gtt or ppgtt? nothing, gtt, ppggt, both. -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: make sure VG_CLEAR() will always do memory clear
On Wed, Nov 20, 2013 at 05:22:48PM +0800, Zhenyu Wang wrote: If valgrind is not available, current VG_CLEAR() would just ignore memory clear operation which might make invalid ioctl argument. So make sure VG_CLEAR() will always clear memory. --- intel/intel_bufmgr_gem.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/intel/intel_bufmgr_gem.c b/intel/intel_bufmgr_gem.c index df6fcec..389f73a 100644 --- a/intel/intel_bufmgr_gem.c +++ b/intel/intel_bufmgr_gem.c @@ -74,7 +74,7 @@ #define VG(x) #endif -#define VG_CLEAR(s) VG(memset(s, 0, sizeof(s))) +#define VG_CLEAR(s) (memset(s, 0, sizeof(s))) VG_CLEAR() is really just for valgrind. If you need to set some specific variable/field to 0 then you need to set it to 0 and not rely on VG_CLEAR() to do it for you. What's the actual issue you have? -- Damien ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: setup workarounds on reset
Daniel Vetter dan...@ffwll.ch writes: Hi Daniel, On Mon, Nov 18, 2013 at 04:34:44PM +0200, Mika Kuoppala wrote: Large parts of hw initialization is behind per gen clock gating functions. Including workarounds. Call intel_modeset_init_hw after reset so that we set these up correctly. Signed-off-by: Mika Kuoppala mika.kuopp...@intel.com --- drivers/gpu/drm/i915/i915_drv.c |2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index c2e00ed..2908f7f 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -798,6 +798,8 @@ int i915_reset(struct drm_device *dev) drm_irq_uninstall(dev); drm_irq_install(dev); intel_hpd_init(dev); + +intel_modeset_init_hw(dev); Currently the idea is that w/as which get nuked by the gt reset should be put into the respective ring_init function in intel_ringbuffer.c. This disdinction is important since init_clock_gating gets called fairly early in the resume/load sequence before most of the gem stuff is set up. And the w/a in the ring_init functions are carefully ordered wrt the ring (re) enabling. So which bit/register is the culprit here? Here is output from the test on ivb, after drv_hangman: FAIL WaDisableEarlyCull:ivb OK WaDisableBackToBackFlipFix:ivb FAIL WaDisablePSDDualDispatchEnable:ivb FAIL WaDisableRHWOptimizationForRenderHang:ivb FAIL WaApplyL3ControlAndL3ChickenMode:ivb OK WaForceL3Serialization:ivb OK WaDisableRCZUnitClockGating:ivb OK WaCatErrorRejectionIssue:ivb FAIL WaVSRefCountFullforceMissDisable:ivb FAIL WaDisable4x2SubspanOptimization:ivb 10 workarounds tested, 4 passed, 6 failed Test assertion failure function main, file drv_workarounds.c:119: Failed assertion: check_workarounds(ivb_workarounds[0], ivb) == 0 Test can be found in here: https://github.com/mkuoppal/intel-gpu-tools/tree/was -Mika ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: setup workarounds on reset
On Wed, Nov 20, 2013 at 12:47 PM, Mika Kuoppala mika.kuopp...@linux.intel.com wrote: FAIL WaDisableEarlyCull:ivb OK WaDisableBackToBackFlipFix:ivb FAIL WaDisablePSDDualDispatchEnable:ivb FAIL WaDisableRHWOptimizationForRenderHang:ivb FAIL WaApplyL3ControlAndL3ChickenMode:ivb OK WaForceL3Serialization:ivb OK WaDisableRCZUnitClockGating:ivb OK WaCatErrorRejectionIssue:ivb FAIL WaVSRefCountFullforceMissDisable:ivb FAIL WaDisable4x2SubspanOptimization:ivb Looks like just a bunch of render related registers, so could make sense to move them. I guess the bigger question is what happens if we use per-ring reset. Do we loose all the same register settings as for a full reset or is there some fancy split? Can you please check this out with your per-ring reset patches if possible? Thanks, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/5] drm/i915: retrieve current fb config into new plane_config structure at init
On Wed, Nov 13, 2013 at 10:20:45AM -0800, Jesse Barnes wrote:
Read out the current plane configuration at init time into a new
plane_config structure. This allows us to track any existing
framebuffers attached to the plane and potentially re-use them in our
fbdev code for a smooth handoff.
Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org
---
drivers/gpu/drm/i915/i915_drv.h | 3 +
drivers/gpu/drm/i915/intel_display.c | 113
++-
drivers/gpu/drm/i915/intel_drv.h | 14 +
3 files changed, 128 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 4377523..db6821a 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -364,6 +364,7 @@ struct drm_i915_error_state {
};
struct intel_crtc_config;
+struct intel_plane_config;
struct intel_crtc;
struct intel_limit;
struct dpll;
@@ -402,6 +403,8 @@ struct drm_i915_display_funcs {
* fills out the pipe-config with the hw state. */
bool (*get_pipe_config)(struct intel_crtc *,
struct intel_crtc_config *);
+ void (*get_plane_config)(struct intel_crtc *,
+ struct intel_plane_config *);
int (*crtc_mode_set)(struct drm_crtc *crtc,
int x, int y,
struct drm_framebuffer *old_fb);
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index d4cc00c..c3c4fc3 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2002,6 +2002,27 @@ unsigned long intel_gen4_compute_page_offset(int *x,
int *y,
}
}
+int intel_format_to_fourcc(int format)
+{
+ switch (format) {
+ case DISPPLANE_8BPP:
+ return DRM_FORMAT_C8;
+ case DISPPLANE_BGRX555:
+ return DRM_FORMAT_ARGB1555;
^
X
+ case DISPPLANE_BGRX565:
+ return DRM_FORMAT_RGB565;
+ default:
+ case DISPPLANE_BGRX888:
+ return DRM_FORMAT_XRGB;
+ case DISPPLANE_RGBX888:
+ return DRM_FORMAT_XBGR;
+ case DISPPLANE_BGRX101010:
+ return DRM_FORMAT_XRGB2101010;
+ case DISPPLANE_RGBX101010:
+ return DRM_FORMAT_XBGR2101010;
+ }
+}
+
static int i9xx_update_plane(struct drm_crtc *crtc, struct drm_framebuffer
*fb,
int x, int y)
{
@@ -5463,6 +5484,81 @@ intel_framebuffer_pitch_for_width(int width, int bpp,
bool tiled)
return ALIGN(pitch, 64);
}
+static void i9xx_get_plane_config(struct intel_crtc *crtc,
+ struct intel_plane_config *plane_config)
+{
+ struct drm_device *dev = crtc-base.dev;
+ struct drm_i915_private *dev_priv = dev-dev_private;
+ int pipe = crtc-pipe, plane = crtc-plane;
+ u32 val;
+
+ val = I915_READ(DSPCNTR(plane));
+
+ if (INTEL_INFO(dev)-gen = 4)
+ if (val DISPPLANE_TILED)
+ plane_config-tiled = true;
+
+ plane_config-pixel_format = val DISPPLANE_PIXFORMAT_MASK;
+
+ switch (plane_config-pixel_format) {
+ case DISPPLANE_8BPP:
+ case DISPPLANE_YUV422:
+ plane_config-bpp = 8;
+ break;
+ case DISPPLANE_BGRX555:
+ case DISPPLANE_BGRX565:
+ case DISPPLANE_BGRA555:
+ plane_config-bpp = 16;
+ break;
+ case DISPPLANE_BGRX888:
+ case DISPPLANE_BGRA888:
+ case DISPPLANE_RGBX888:
+ case DISPPLANE_RGBA888:
+ case DISPPLANE_RGBX101010:
+ case DISPPLANE_RGBA101010:
+ case DISPPLANE_BGRX101010:
+ plane_config-bpp = 32;
+ break;
+ }
Maybe just intel_format_to_fourcc()+drm_format_plane_cpp() or something.
Just to avoid duplicating essentially the same code.
+
+ if (INTEL_INFO(dev)-gen = 4) {
+ if (plane_config-tiled)
+ plane_config-offset = I915_READ(DSPTILEOFF(plane));
+ else
+ plane_config-offset = I915_READ(DSPLINOFF(plane));
+ plane_config-base = I915_READ(DSPSURF(plane)) 0xf000;
+ } else {
+ plane_config-base = I915_READ(DSPADDR(plane));
+ }
+
+ val = I915_READ(PIPESRC(pipe));
+ plane_config-pipe_width = ((val 16) 0xfff) + 1;
+ plane_config-pipe_height = ((val 0) 0xfff) + 1;
+
+ val = I915_READ(HTOTAL(pipe));
+ plane_config-fb_width = (val 0x) + 1;
+ val = I915_READ(VTOTAL(pipe));
+ plane_config-fb_height = (val 0x) + 1;
Why make the fb the size of htotal/vtotal? pipe src size should be all
we need.
+
+ DRM_DEBUG_KMS(pipe/plane %d/%d with fb: size=%dx%d@%d, offset=%x\n,
+ pipe, plane, plane_config-fb_width,
+ plane_config-fb_height,
[Intel-gfx] [PATCH] drm/i915: Fix gen3 self-refresh watermarks
This regression has been introduced in commit 4fe8590a921d0b2e36e542dbfa89a8c5993f5a3f Author: Ville Syrjälä ville.syrj...@linux.intel.com Date: Wed Sep 4 18:25:22 2013 +0300 drm/i915: Use adjusted_mode appropriately when computing watermarks I guess we should renable the enabled local variable into something a notch more descriptive, but that's something for -next. The effect on my i945gme netbook is pretty severe amounts of underruns - usually the very first pixel gets used for the entire screeen. Cc: Ville Syrjälä ville.syrj...@linux.intel.com Cc: Damien Lespiau damien.lesp...@intel.com Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch --- drivers/gpu/drm/i915/intel_pm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 172efa0bfb86..3cc757ff60ee 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -1625,7 +1625,7 @@ static void i9xx_update_wm(struct drm_crtc *unused_crtc) to_intel_crtc(enabled)-config.adjusted_mode; int clock = adjusted_mode-crtc_clock; int htotal = adjusted_mode-htotal; - int hdisplay = to_intel_crtc(crtc)-config.pipe_src_w; + int hdisplay = to_intel_crtc(enabled)-config.pipe_src_w; int pixel_size = enabled-fb-bits_per_pixel / 8; unsigned long line_time_us; int entries; -- 1.8.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Fix gen3 self-refresh watermarks
On Wed, Nov 20, 2013 at 03:02:10PM +0100, Daniel Vetter wrote: This regression has been introduced in commit 4fe8590a921d0b2e36e542dbfa89a8c5993f5a3f Author: Ville Syrjälä ville.syrj...@linux.intel.com Date: Wed Sep 4 18:25:22 2013 +0300 drm/i915: Use adjusted_mode appropriately when computing watermarks I guess we should renable the enabled local variable into something a notch more descriptive, but that's something for -next. The effect on my i945gme netbook is pretty severe amounts of underruns - usually the very first pixel gets used for the entire screeen. Cc: Ville Syrjälä ville.syrj...@linux.intel.com Cc: Damien Lespiau damien.lesp...@intel.com Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch Dang. Copy paste fail on my part :( The fix looks good. Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com --- drivers/gpu/drm/i915/intel_pm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 172efa0bfb86..3cc757ff60ee 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -1625,7 +1625,7 @@ static void i9xx_update_wm(struct drm_crtc *unused_crtc) to_intel_crtc(enabled)-config.adjusted_mode; int clock = adjusted_mode-crtc_clock; int htotal = adjusted_mode-htotal; - int hdisplay = to_intel_crtc(crtc)-config.pipe_src_w; + int hdisplay = to_intel_crtc(enabled)-config.pipe_src_w; int pixel_size = enabled-fb-bits_per_pixel / 8; unsigned long line_time_us; int entries; -- 1.8.1.4 -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Fix gen3 self-refresh watermarks
On Wed, Nov 20, 2013 at 04:47:14PM +0200, Ville Syrjälä wrote: On Wed, Nov 20, 2013 at 03:02:10PM +0100, Daniel Vetter wrote: This regression has been introduced in commit 4fe8590a921d0b2e36e542dbfa89a8c5993f5a3f Author: Ville Syrjälä ville.syrj...@linux.intel.com Date: Wed Sep 4 18:25:22 2013 +0300 drm/i915: Use adjusted_mode appropriately when computing watermarks I guess we should renable the enabled local variable into something a notch more descriptive, but that's something for -next. The effect on my i945gme netbook is pretty severe amounts of underruns - usually the very first pixel gets used for the entire screeen. Cc: Ville Syrjälä ville.syrj...@linux.intel.com Cc: Damien Lespiau damien.lesp...@intel.com Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch Dang. Copy paste fail on my part :( The fix looks good. Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com Queued for -next, thanks for the review. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 3/3] tests/gem_reset_stats: check non root access to reset_stats
Getting global reset count needs to be tested with root and
non root access.
Signed-off-by: Mika Kuoppala mika.kuopp...@intel.com
---
tests/gem_reset_stats.c | 70 +--
1 file changed, 61 insertions(+), 9 deletions(-)
diff --git a/tests/gem_reset_stats.c b/tests/gem_reset_stats.c
index 9e52b60..5252833 100644
--- a/tests/gem_reset_stats.c
+++ b/tests/gem_reset_stats.c
@@ -637,6 +637,17 @@ static void test_close_pending(void)
close(fd);
}
+static void drop_root(void)
+{
+ igt_assert(getuid() == 0);
+
+ igt_assert(setgid(2) == 0);
+ igt_assert(setuid(2) == 0);
+
+ igt_assert(getgid() == 2);
+ igt_assert(getuid() == 2);
+}
+
static void __test_count(const bool create_ctx)
{
int fd, h, ctx;
@@ -661,9 +672,21 @@ static void __test_count(const bool create_ctx)
assert_reset_status(fd, ctx, RS_BATCH_ACTIVE);
c2 = get_reset_count(fd, ctx);
igt_assert(c2 = 0);
-
igt_assert(c2 == (c1 + 1));
+ igt_fork(child, 1) {
+ drop_root();
+
+ c2 = get_reset_count(fd, ctx);
+
+ if (ctx == 0)
+ igt_assert(c2 == -EPERM);
+ else
+ igt_assert(c2 == 0);
+ }
+
+ igt_waitchildren();
+
gem_close(fd, h);
if (create_ctx)
@@ -710,16 +733,50 @@ static int _test_params(int fd, int ctx, uint32_t flags,
uint32_t pad)
return 0;
}
-static void test_param_ctx(int fd, int ctx)
+typedef enum { root = 0, user } cap_t;
+
+static void test_param_ctx(const int fd, const int ctx, const cap_t cap)
{
const uint32_t bad = rand() + 1;
- igt_assert(_test_params(fd, ctx, 0, 0) == 0);
+ if (ctx == 0) {
+ if (cap == root)
+ igt_assert(_test_params(fd, ctx, 0, 0) == 0);
+ else
+ igt_assert(_test_params(fd, ctx, 0, 0) == -EPERM);
+ }
+
igt_assert(_test_params(fd, ctx, 0, bad) == -EINVAL);
igt_assert(_test_params(fd, ctx, bad, 0) == -EINVAL);
igt_assert(_test_params(fd, ctx, bad, bad) == -EINVAL);
}
+static void check_params(const int fd, const int ctx, cap_t cap)
+{
+ igt_assert(ioctl(fd, GET_RESET_STATS_IOCTL, 0) == -1);
+ igt_assert(_test_params(fd, 0xbadbad, 0, 0) == -ENOENT);
+
+ test_param_ctx(fd, 0, cap);
+ test_param_ctx(fd, ctx, cap);
+}
+
+static void _test_param(const int fd, const int ctx)
+{
+ check_params(fd, ctx, root);
+
+ igt_fork(child, 1) {
+ check_params(fd, ctx, root);
+
+ drop_root();
+
+ check_params(fd, ctx, user);
+ }
+
+ check_params(fd, ctx, root);
+
+ igt_waitchildren();
+}
+
static void test_params(void)
{
int fd, ctx;
@@ -728,12 +785,7 @@ static void test_params(void)
igt_assert(fd = 0);
ctx = context_create(fd);
- igt_assert(ioctl(fd, GET_RESET_STATS_IOCTL, 0) == -1);
-
- igt_assert(_test_params(fd, 0xbadbad, 0, 0) == -ENOENT);
-
- test_param_ctx(fd, 0);
- test_param_ctx(fd, ctx);
+ _test_param(fd, ctx);
close(fd);
}
--
1.7.9.5
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/3] tests/gem_reset_stats: stop rings after injecting hang
To make driver report a simulated hang in dmesg.
Suggested-by: Daniel Vetter daniel.vet...@ffwll.ch
Signed-off-by: Mika Kuoppala mika.kuopp...@intel.com
---
tests/gem_reset_stats.c | 18 ++
1 file changed, 18 insertions(+)
diff --git a/tests/gem_reset_stats.c b/tests/gem_reset_stats.c
index a7497f3..9e52b60 100644
--- a/tests/gem_reset_stats.c
+++ b/tests/gem_reset_stats.c
@@ -42,6 +42,7 @@
#include intel_batchbuffer.h
#include intel_gpu_tools.h
#include rendercopy.h
+#include igt_debugfs.h
#define RS_NO_ERROR 0
#define RS_BATCH_ACTIVE (1 0)
@@ -73,6 +74,8 @@ struct local_drm_i915_gem_context_destroy {
#define CONTEXT_DESTROY_IOCTL DRM_IOWR(DRM_COMMAND_BASE + 0x2e, struct
local_drm_i915_gem_context_destroy)
#define GET_RESET_STATS_IOCTL DRM_IOWR(DRM_COMMAND_BASE + 0x32, struct
local_drm_i915_reset_stats)
+static igt_debugfs_t dfs;
+
static uint32_t context_create(int fd)
{
struct local_drm_i915_gem_context_create create;
@@ -192,6 +195,17 @@ static int exec_valid(int fd, int ctx)
return exec.handle;
}
+static void stop_rings(void)
+{
+ int fd;
+
+ fd = igt_debugfs_open(dfs, i915_ring_stop, O_WRONLY);
+ igt_assert(fd = 0);
+
+ igt_assert(write(fd, 0xff, 4) == 4);
+ close(fd);
+}
+
#define BUFSIZE (4 * 1024)
#define ITEMS (BUFSIZE 2)
@@ -284,6 +298,8 @@ static int inject_hang(int fd, int ctx)
free(buf);
+ stop_rings();
+
return exec.handle;
}
@@ -743,6 +759,8 @@ igt_main
igt_skip(Kernel is too old, or contexts not supported:
%s\n,
strerror(errno));
+ assert(igt_debugfs_init(dfs) == 0);
+
close(fd);
}
--
1.7.9.5
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/3] tests/gem_reset_stats: add support for BDW+
For BDW+, there BATCH_BUFFER_START is 3 * 32bits in length and length needs to be encoded into the opcode. Suggested-by: Damien Lespiau damien.lesp...@intel.com Signed-off-by: Mika Kuoppala mika.kuopp...@intel.com --- tests/gem_reset_stats.c | 12 +--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/tests/gem_reset_stats.c b/tests/gem_reset_stats.c index 07dfac4..a7497f3 100644 --- a/tests/gem_reset_stats.c +++ b/tests/gem_reset_stats.c @@ -202,9 +202,13 @@ static int inject_hang(int fd, int ctx) uint64_t gtt_off; uint32_t *buf; int roff, i; + unsigned cmd_len = 2; srandom(time(NULL)); + if (intel_gen(intel_get_drm_devid(fd)) = 8) + cmd_len = 3; + buf = malloc(BUFSIZE); igt_assert(buf != NULL); @@ -240,9 +244,11 @@ static int inject_hang(int fd, int ctx) for (i = 0; i ITEMS; i++) buf[i] = MI_NOOP; - roff = random() % (ITEMS - 2); - buf[roff] = MI_BATCH_BUFFER_START; - buf[roff + 1] = gtt_off + (roff 2); + roff = random() % (ITEMS - cmd_len); + buf[roff] = MI_BATCH_BUFFER_START | (cmd_len - 2); + buf[roff + 1] = (gtt_off 0xfffc) + (roff 2); + if (cmd_len == 3) + buf[roff + 2] = gtt_off 0xull; #ifdef VERBOSE printf(loop injected at 0x%lx (off 0x%x, bo_start 0x%lx, bo_end 0x%lx)\n, -- 1.7.9.5 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PULL] drm-intel-fixes
Hi Dave, Just a small pile of fixes for bugs and a few regressions. I'm still trying to track down a driver load hang on my g33 (which infuriatingly doesn't happen when loading the module manually after boot), somehow bisecting loves to go astray on this one :( And there's a (harmless) locking WARN in the suspend code due to one of Jesse's vlv backlight rework patches. Otherwise nothing outstanding afaik. Cheers, Daniel The following changes since commit ad40f83f5a89f6d723fd4db424b531f8dd7d3b49: Merge branch 'drm-next-3.13' of git://people.freedesktop.org/~agd5f/linux into drm-next (2013-11-14 09:53:15 +1000) are available in the git repository at: git://people.freedesktop.org/~danvet/drm-intel tags/drm-intel-fixes-2013-11-20 for you to fetch changes up to f727b490efd0941a8d720fd07012dcb7f0740f77: drm/i915: Fix gen3 self-refresh watermarks (2013-11-20 15:52:52 +0100) Chris Wilson (2): drm/i915: Hold pc8 lock around toggling pc8.gpu_idle drm/i915: Do not enable package C8 on unsupported hardware Daniel Vetter (7): drm/i915: flush cursors harder Partially revert drm/i915: tune the RC6 threshold for stability drm/i915: restore the early forcewake cleanup drm/i915/tv: add -get_config callback drm/i915: encoder-get_config is no longer optional drm/i915: Replicate BIOS eDP bpp clamping hack for hsw drm/i915: Fix gen3 self-refresh watermarks Duncan Laurie (1): i915: Use 120MHz LVDS SSC clock for gen5/gen6/gen7 Jani Nikula (1): drm/i915/dp: set sink to power down mode on dp disable Jesse Barnes (1): x86/early quirk: use gen6 stolen detection for VLV arch/x86/kernel/early-quirks.c | 4 ++-- drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/intel_bios.c| 7 ++- drivers/gpu/drm/i915/intel_ddi.c | 20 drivers/gpu/drm/i915/intel_display.c | 33 +++-- drivers/gpu/drm/i915/intel_dp.c | 2 +- drivers/gpu/drm/i915/intel_pm.c | 4 ++-- drivers/gpu/drm/i915/intel_tv.c | 8 drivers/gpu/drm/i915/intel_uncore.c | 26 ++ 9 files changed, 81 insertions(+), 24 deletions(-) -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] intel: make sure VG_CLEAR() will always do memory clear
On 2013.11.20 11:36:20 +, Damien Lespiau wrote: On Wed, Nov 20, 2013 at 05:22:48PM +0800, Zhenyu Wang wrote: If valgrind is not available, current VG_CLEAR() would just ignore memory clear operation which might make invalid ioctl argument. So make sure VG_CLEAR() will always clear memory. --- intel/intel_bufmgr_gem.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/intel/intel_bufmgr_gem.c b/intel/intel_bufmgr_gem.c index df6fcec..389f73a 100644 --- a/intel/intel_bufmgr_gem.c +++ b/intel/intel_bufmgr_gem.c @@ -74,7 +74,7 @@ #define VG(x) #endif -#define VG_CLEAR(s) VG(memset(s, 0, sizeof(s))) +#define VG_CLEAR(s) (memset(s, 0, sizeof(s))) VG_CLEAR() is really just for valgrind. If you need to set some specific variable/field to 0 then you need to set it to 0 and not rely on VG_CLEAR() to do it for you. ok, in valgrind case it does memory clear for ioctl args which I think is good behavior in non-valgrind case too. What's the actual issue you have? During testing on recent get reset status ioctl, if !HAVE_VALGRIND, stats argument is not cleared, which failed in kernel check. -- Open Source Technology Center, Intel ltd. $gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827 signature.asc Description: Digital signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/vlv: Add VLV specific force wake routines.
On Wed, Nov 20, 2013 at 7:00 AM, S, Deepak deepa...@intel.com wrote: - have you done measurements on this? given how infrequently we ought to be waking the wells when they're idle, and how long we generally keep them awake, is this a real power win? [Deepak] By Individually controlling the wells we observed around 100mW - 200mW saving in different scenarios (GL Beanchmark Media playback). This kind of information is really important and should be part of the commit message. This rule holds generally for performance/power tuning work - the commit message should at least mention the order of magnitude of the improvements seen and which workloads have been tested. If you're afraid to disclose confidential information (e.g. for power savings) just make the language fuzzy enough, e.g. here We've seen power savings in the lower sub-1W range on workloads that only neeed on of the power wells, e.g. glbenchmark, media playback. Thanks, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/vlv: Add VLV specific force wake routines.
Thanks Jesse, I wil split the patches and send it for review. Thanks Deepak -Original Message- From: Jesse Barnes [mailto:jbar...@virtuousgeek.org] Sent: Wednesday, November 20, 2013 9:35 PM To: S, Deepak Cc: intel-gfx@lists.freedesktop.org Subject: Re: [Intel-gfx] [PATCH] drm/i915/vlv: Add VLV specific force wake routines. On Wed, 20 Nov 2013 06:00:24 + S, Deepak deepa...@intel.com wrote: Hi Jesse, Thanks for the review. Below is my response. - why not use the callback to __vlv_force_wake_* from gen6_gt_force_wake_*? i.e. why is VLV special here? [Deepak] Gen6 has a single power well whereas the VLV is has spate wells. This was the reason for the separate function - having a new gen7_media_force_wake function may be better than passing an engine around, and would touch fewer pieces of code [Deepak] Even Gen7 is also as single Power Well. Having common function between gen7 and vlv might be difficult to individually handle the wells. - have you done measurements on this? given how infrequently we ought to be waking the wells when they're idle, and how long we generally keep them awake, is this a real power win? [Deepak] By Individually controlling the wells we observed around 100mW - 200mW saving in different scenarios (GL Beanchmark Media playback). wow, that's a significant savings. Can you split the patch into one that adds the power well arg, and another that adds the VLV support for the split? That would make it easier to review. Thanks, -- Jesse Barnes, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] intel: Use memset instead of VG_CLEAR
From: Ian Romanick ian.d.roman...@intel.com The ioctl expects that certain fields will be zeroed, so we should allow the helper function to actually work in non-Valgrind builds. Signed-off-by: Ian Romanick ian.d.roman...@intel.com Reported-by: Zhenyu Wang zhen...@linux.intel.com Cc: Damien Lespiau damien.lesp...@intel.com Cc: Daniel Vetter daniel.vet...@ffwll.ch --- intel/intel_bufmgr_gem.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/intel/intel_bufmgr_gem.c b/intel/intel_bufmgr_gem.c index df6fcec..c11ed45 100644 --- a/intel/intel_bufmgr_gem.c +++ b/intel/intel_bufmgr_gem.c @@ -3033,7 +3033,7 @@ drm_intel_get_reset_stats(drm_intel_context *ctx, if (ctx == NULL) return -EINVAL; - VG_CLEAR(stats); + memset(stats, 0, sizeof(stats)); bufmgr_gem = (drm_intel_bufmgr_gem *)ctx-bufmgr; stats.ctx_id = ctx-ctx_id; -- 1.8.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] intel: Use memset instead of VG_CLEAR
On Wed, Nov 20, 2013 at 08:38:38AM -0800, Ian Romanick wrote: From: Ian Romanick ian.d.roman...@intel.com The ioctl expects that certain fields will be zeroed, so we should allow the helper function to actually work in non-Valgrind builds. Signed-off-by: Ian Romanick ian.d.roman...@intel.com Reported-by: Zhenyu Wang zhen...@linux.intel.com Cc: Damien Lespiau damien.lesp...@intel.com Cc: Daniel Vetter daniel.vet...@ffwll.ch Reviewed-by: Daniel Vetter daniel.vet...@ffwll.ch --- intel/intel_bufmgr_gem.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/intel/intel_bufmgr_gem.c b/intel/intel_bufmgr_gem.c index df6fcec..c11ed45 100644 --- a/intel/intel_bufmgr_gem.c +++ b/intel/intel_bufmgr_gem.c @@ -3033,7 +3033,7 @@ drm_intel_get_reset_stats(drm_intel_context *ctx, if (ctx == NULL) return -EINVAL; - VG_CLEAR(stats); + memset(stats, 0, sizeof(stats)); bufmgr_gem = (drm_intel_bufmgr_gem *)ctx-bufmgr; stats.ctx_id = ctx-ctx_id; -- 1.8.1.4 -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 5/9] drm/i915: print object bindings in debugfs
On Wed, Nov 20, 2013 at 3:03 AM, Chris Wilson ch...@chris-wilson.co.uk wrote: On Tue, Nov 19, 2013 at 09:37:16AM -0800, Rodrigo Vivi wrote: On Tue, Nov 19, 2013 at 5:52 AM, Chris Wilson ch...@chris-wilson.co.uk wrote: On Mon, Nov 18, 2013 at 06:32:34PM -0800, Rodrigo Vivi wrote: From: Daniel Vetter daniel.vet...@ffwll.ch This is useful when we only have aliasing ppgtt and want to figure out what exactly is accesssible and what not. Paulo can somehow overwrite the fbcon framebuffer with the blitter on his hsw machine ... v2: Actually make it compile. Cc: Paulo Zanoni przan...@gmail.com Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch Signed-off-by: Rodrigo Vivi rodrigo.v...@gmail.com This is still too ugly. at least should be the opposite pp before g and complemented with a tt in the end. (bindings: ) (bindings: g) (bindings: pp) (bindings: gpp) -Chris But what are all real possible options? nothing, gtt or ppgtt? nothing, gtt, ppggt, both. so, forget what I said... what about this below? seq_printf(m, (bindings: %s%s), obj-has_global_gtt_mapping ? gtt : , obj-has_aliasing_ppgtt_mapping ? ppgtt : ); (bindings: ) (bindings: gtt) (bindings: ppgtt) (bindings: gtt ppgtt) with or without this bikeshed feel free to use: Reviewed-by: Rodrigo Vivi rodrigo.v...@gmail.com -Chris -- Chris Wilson, Intel Open Source Technology Centre -- Rodrigo Vivi Blog: http://blog.vivi.eng.br ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] intel: Use memset instead of VG_CLEAR
On Wed, Nov 20, 2013 at 08:38:38AM -0800, Ian Romanick wrote: From: Ian Romanick ian.d.roman...@intel.com The ioctl expects that certain fields will be zeroed, so we should allow the helper function to actually work in non-Valgrind builds. Signed-off-by: Ian Romanick ian.d.roman...@intel.com Reported-by: Zhenyu Wang zhen...@linux.intel.com Cc: Damien Lespiau damien.lesp...@intel.com Cc: Daniel Vetter daniel.vet...@ffwll.ch I was thinking that I missed it in the (lidrm) review, but it's actually a newer patch that introduces the checks. Lesson learned for next ioctl reviews (kernel), have a better pass on the input validation and think about rejecting reserved values. Reviewed-by: Damien Lespiau damien.lesp...@intel.com --- intel/intel_bufmgr_gem.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/intel/intel_bufmgr_gem.c b/intel/intel_bufmgr_gem.c index df6fcec..c11ed45 100644 --- a/intel/intel_bufmgr_gem.c +++ b/intel/intel_bufmgr_gem.c @@ -3033,7 +3033,7 @@ drm_intel_get_reset_stats(drm_intel_context *ctx, if (ctx == NULL) return -EINVAL; - VG_CLEAR(stats); + memset(stats, 0, sizeof(stats)); bufmgr_gem = (drm_intel_bufmgr_gem *)ctx-bufmgr; stats.ctx_id = ctx-ctx_id; -- 1.8.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] intel: make sure VG_CLEAR() will always do memory clear
On Wed, Nov 20, 2013 at 11:53:54PM +0800, Zhenyu Wang wrote: VG_CLEAR() is really just for valgrind. If you need to set some specific variable/field to 0 then you need to set it to 0 and not rely on VG_CLEAR() to do it for you. ok, in valgrind case it does memory clear for ioctl args which I think is good behavior in non-valgrind case too. Ian just submitted a patch that memset the whole structure. What's the actual issue you have? During testing on recent get reset status ioctl, if !HAVE_VALGRIND, stats argument is not cleared, which failed in kernel check. Right, so the fix is (was) to zero the fields checked by the kernel explicitely, not change the VG() macro, which is just used in testing (and it should this way). HTH, -- Damien ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/2] drm/i915: Report all GTFIFODBG errors
On Mon, Nov 18, 2013 at 05:13:19PM +0200, Ville Syrjälä wrote: On Thu, Nov 14, 2013 at 07:09:48PM +0200, Ville Syrjälä wrote: On Thu, Nov 14, 2013 at 02:54:10PM +0200, Mika Kuoppala wrote: ville.syrj...@linux.intel.com writes: From: Ville Syrjälä ville.syrj...@linux.intel.com On VLV GTFIFODBG has more bits. Just report them all. Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com --- drivers/gpu/drm/i915/i915_reg.h | 5 - drivers/gpu/drm/i915/intel_uncore.c | 5 ++--- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 849e595..e8f47de 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -4852,7 +4852,10 @@ #defineFORCEWAKE_MT_ENABLE (15) #define GTFIFODBG 0x12 -#defineGT_FIFO_CPU_ERROR_MASK 7 +#defineGT_FIFO_SBDROPERR (16) +#defineGT_FIFO_BLOBDROPERR (15) +#defineGT_FIFO_SB_READ_ABORTERR(14) +#defineGT_FIFO_DROPERR (13) #defineGT_FIFO_OVFERR (12) #defineGT_FIFO_IAWRERR (11) #defineGT_FIFO_IARDERR (10) diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c index 0edabbb..a9849ab 100644 --- a/drivers/gpu/drm/i915/intel_uncore.c +++ b/drivers/gpu/drm/i915/intel_uncore.c @@ -121,9 +121,8 @@ static void gen6_gt_check_fifodbg(struct drm_i915_private *dev_priv) u32 gtfifodbg; gtfifodbg = __raw_i915_read32(dev_priv, GTFIFODBG); - if (WARN(gtfifodbg GT_FIFO_CPU_ERROR_MASK, -MMIO read or write has been dropped %x\n, gtfifodbg)) - __raw_i915_write32(dev_priv, GTFIFODBG, GT_FIFO_CPU_ERROR_MASK); + if (WARN(gtfifodbg, GT wake FIFO error 0x%x\n, gtfifodbg)) I think you still need mask, there are ro fields != 0 in the same register. Which bits? VLV has those seven low bits, others just three low bits AFAICS. OK, so the problem is that bspec seems to list some bogus junk for these registers. The gunit register HAS is what I used to write these patches. Someone with a VLV on their hands should double check whether real hardware agrees with the gunit register HAS. Any volunteers? Imre had a look on his VLV the other day, and the register contents seemed to match the Gunit register HAS. So I think these patches should be doing the right thing. -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] i915 driver gpu hung kernel 3.11
Hi Stephen, On Tue, 19 November 2013 Stephen Clark sclar...@earthlink.net wrote: Thanks for the response. I have subscribed to the intel-gfx list. I didn't post the error_state file since it huge. It's best to submit a but report on bugs.freedesktop.org and attach the error_state there (compressed if needed) - repeating the information you provided in this thread. Without the error_state chances of getting some developer look at it and have a chance of understanding the cause are small. If they can reproduce it's a bonus. Once you have done so, replying with a reference to the bug might help people who find your report in mailing list archives. Bruno I was trying to play Myst Online using wine-1.3.24. I get started and start moving my avatar fairly quickly I get the error. I have built the latest X, mesa etc from the git repo and loaded the latest kernel but still have the problem, though now my screen doesn't lose horizontal sync like it used to before I uppgraded X etc. Below is a lspci of my laptop. 00:00.0 Host bridge: Intel Corporation Mobile 945GM/PM/GMS, 943/940GML and 945GT Express Memory Controller Hub (rev 03) 00:02.0 VGA compatible controller: Intel Corporation Mobile 945GM/GMS, 943/940GML Express Integrated Graphics Controller (rev 03) 00:02.1 Display controller: Intel Corporation Mobile 945GM/GMS/GME, 943/940GML Express Integrated Graphics Controller (rev 03) 00:1b.0 Audio device: Intel Corporation N10/ICH 7 Family High Definition Audio Controller (rev 02) 00:1c.0 PCI bridge: Intel Corporation N10/ICH 7 Family PCI Express Port 1 (rev 02) 00:1c.1 PCI bridge: Intel Corporation N10/ICH 7 Family PCI Express Port 2 (rev 02) 00:1c.2 PCI bridge: Intel Corporation N10/ICH 7 Family PCI Express Port 3 (rev 02) 00:1d.0 USB Controller: Intel Corporation N10/ICH 7 Family USB UHCI Controller #1 (rev 02) 00:1d.1 USB Controller: Intel Corporation N10/ICH 7 Family USB UHCI Controller #2 (rev 02) 00:1d.2 USB Controller: Intel Corporation N10/ICH 7 Family USB UHCI Controller #3 (rev 02) 00:1d.3 USB Controller: Intel Corporation N10/ICH 7 Family USB UHCI Controller #4 (rev 02) 00:1d.7 USB Controller: Intel Corporation N10/ICH 7 Family USB2 EHCI Controller (rev 02) 00:1e.0 PCI bridge: Intel Corporation 82801 Mobile PCI Bridge (rev e2) 00:1f.0 ISA bridge: Intel Corporation 82801GBM (ICH7-M) LPC Interface Bridge (rev 02) 00:1f.2 IDE interface: Intel Corporation 82801GBM/GHM (ICH7 Family) SATA IDE Controller (rev 02) 00:1f.3 SMBus: Intel Corporation N10/ICH 7 Family SMBus Controller (rev 02) 03:00.0 Network controller: Intel Corporation PRO/Wireless 3945ABG [Golan] Network Connection (rev 02) 05:01.0 FireWire (IEEE 1394): Ricoh Co Ltd R5C832 IEEE 1394 Controller 05:01.1 SD Host controller: Ricoh Co Ltd R5C822 SD/SDIO/MMC/MS/MSPro Host Adapter (rev 19) 05:01.2 System peripheral: Ricoh Co Ltd R5C843 MMC Host Controller (rev 01) 05:01.3 System peripheral: Ricoh Co Ltd R5C592 Memory Stick Bus Host Adapter (rev 0a) 05:07.0 Ethernet controller: Realtek Semiconductor Co., Ltd. RTL-8110SC/8169SC Gigabit Ethernet (rev 10) On 11/18/2013 12:41 PM, Bruno Prémont wrote: Hi Stephen, You may want to CC intel-gfx@lists.freedesktop.org for i915 issues (even if you are not subscribed and you mail will wait for a moderator to let it go through). In case of intel GPU hangs you should at least include /sys/kernel/debug/dri/0/i915_error_state, probably submitting as a bug report on bugs.freedesktop.org due to its size. If you have any indication on what triggers the hang, please add! Bruno On Sun, 17 November 2013 Stephen Clarksclar...@earthlink.net wrote: Hi List, I am getting this in kernel 3.11 x86_64 Nov 17 18:56:19 joker4 kernel: [drm:i915_hangcheck_elapsed] *ERROR* stuck on render ring Nov 17 18:56:19 joker4 kernel: [drm] capturing error event; look for more information in /sys/kernel/debug/dri/0/i915_error_state Nov 17 18:56:19 joker4 kernel: swapper/1: page allocation failure: order:6, mode:0x200020 Nov 17 18:56:19 joker4 kernel: CPU: 1 PID: 0 Comm: swapper/1 Not tainted 3.11.6-1.el6.elrepo.x86_64 #1 Nov 17 18:56:19 joker4 kernel: Hardware name: To Be Filled By O.E.M. Z96F/Z96F, BIOS 080012 08/29/2006 Nov 17 18:56:19 joker4 kernel: 0006 8800b73038e0 815f7f89 0010 Nov 17 18:56:19 joker4 kernel: 00200020 8800b7303970 8114243d 8800b778ab28 Nov 17 18:56:19 joker4 kernel: 0031 8800b7789000 00060002 Nov 17 18:56:19 joker4 kernel: Call Trace: Nov 17 18:56:19 joker4 kernel:IRQ [815f7f89] dump_stack+0x49/0x60 Nov 17 18:56:19 joker4 kernel: [8114243d] warn_alloc_failed+0xfd/0x160 Nov 17 18:56:19 joker4 kernel: [8114e98c] ? wakeup_kswapd+0x10c/0x140 Nov 17 18:56:19 joker4 kernel: [811455ae]
Re: [Intel-gfx] i915 driver gpu hung kernel 3.11
Hi Bruno, I have tested the latest kernel and X, mesa etc, but am still using wine-1.3.24. I am working on upgrading that. If I still have the error I will file a bug report at bugs.freedesktop.org. I already have a login because of the same problem happening with Myst 5, but it was never resolved. Do you know if there is a comprehensive set of test I can run to make sure my hardware is OK. When I run dxdiag under wine it passes all tests, but then when trying to play Myst online or Myst 5 I get the gpu hung situation. Anyway thanks for taking the time to respond. Regards, Steve On 11/20/2013 12:26 PM, Bruno Prémont wrote: Hi Stephen, On Tue, 19 November 2013 Stephen Clarksclar...@earthlink.net wrote: Thanks for the response. I have subscribed to the intel-gfx list. I didn't post the error_state file since it huge. It's best to submit a but report on bugs.freedesktop.org and attach the error_state there (compressed if needed) - repeating the information you provided in this thread. Without the error_state chances of getting some developer look at it and have a chance of understanding the cause are small. If they can reproduce it's a bonus. Once you have done so, replying with a reference to the bug might help people who find your report in mailing list archives. Bruno I was trying to play Myst Online using wine-1.3.24. I get started and start moving my avatar fairly quickly I get the error. I have built the latest X, mesa etc from the git repo and loaded the latest kernel but still have the problem, though now my screen doesn't lose horizontal sync like it used to before I uppgraded X etc. Below is a lspci of my laptop. 00:00.0 Host bridge: Intel Corporation Mobile 945GM/PM/GMS, 943/940GML and 945GT Express Memory Controller Hub (rev 03) 00:02.0 VGA compatible controller: Intel Corporation Mobile 945GM/GMS, 943/940GML Express Integrated Graphics Controller (rev 03) 00:02.1 Display controller: Intel Corporation Mobile 945GM/GMS/GME, 943/940GML Express Integrated Graphics Controller (rev 03) 00:1b.0 Audio device: Intel Corporation N10/ICH 7 Family High Definition Audio Controller (rev 02) 00:1c.0 PCI bridge: Intel Corporation N10/ICH 7 Family PCI Express Port 1 (rev 02) 00:1c.1 PCI bridge: Intel Corporation N10/ICH 7 Family PCI Express Port 2 (rev 02) 00:1c.2 PCI bridge: Intel Corporation N10/ICH 7 Family PCI Express Port 3 (rev 02) 00:1d.0 USB Controller: Intel Corporation N10/ICH 7 Family USB UHCI Controller #1 (rev 02) 00:1d.1 USB Controller: Intel Corporation N10/ICH 7 Family USB UHCI Controller #2 (rev 02) 00:1d.2 USB Controller: Intel Corporation N10/ICH 7 Family USB UHCI Controller #3 (rev 02) 00:1d.3 USB Controller: Intel Corporation N10/ICH 7 Family USB UHCI Controller #4 (rev 02) 00:1d.7 USB Controller: Intel Corporation N10/ICH 7 Family USB2 EHCI Controller (rev 02) 00:1e.0 PCI bridge: Intel Corporation 82801 Mobile PCI Bridge (rev e2) 00:1f.0 ISA bridge: Intel Corporation 82801GBM (ICH7-M) LPC Interface Bridge (rev 02) 00:1f.2 IDE interface: Intel Corporation 82801GBM/GHM (ICH7 Family) SATA IDE Controller (rev 02) 00:1f.3 SMBus: Intel Corporation N10/ICH 7 Family SMBus Controller (rev 02) 03:00.0 Network controller: Intel Corporation PRO/Wireless 3945ABG [Golan] Network Connection (rev 02) 05:01.0 FireWire (IEEE 1394): Ricoh Co Ltd R5C832 IEEE 1394 Controller 05:01.1 SD Host controller: Ricoh Co Ltd R5C822 SD/SDIO/MMC/MS/MSPro Host Adapter (rev 19) 05:01.2 System peripheral: Ricoh Co Ltd R5C843 MMC Host Controller (rev 01) 05:01.3 System peripheral: Ricoh Co Ltd R5C592 Memory Stick Bus Host Adapter (rev 0a) 05:07.0 Ethernet controller: Realtek Semiconductor Co., Ltd. RTL-8110SC/8169SC Gigabit Ethernet (rev 10) On 11/18/2013 12:41 PM, Bruno Prémont wrote: Hi Stephen, You may want to CC intel-gfx@lists.freedesktop.org for i915 issues (even if you are not subscribed and you mail will wait for a moderator to let it go through). In case of intel GPU hangs you should at least include /sys/kernel/debug/dri/0/i915_error_state, probably submitting as a bug report on bugs.freedesktop.org due to its size. If you have any indication on what triggers the hang, please add! Bruno On Sun, 17 November 2013 Stephen Clarksclar...@earthlink.net wrote: Hi List, I am getting this in kernel 3.11 x86_64 Nov 17 18:56:19 joker4 kernel: [drm:i915_hangcheck_elapsed] *ERROR* stuck on render ring Nov 17 18:56:19 joker4 kernel: [drm] capturing error event; look for more information in /sys/kernel/debug/dri/0/i915_error_state Nov 17 18:56:19 joker4 kernel: swapper/1: page allocation failure: order:6, mode:0x200020 Nov 17 18:56:19 joker4 kernel: CPU: 1 PID: 0 Comm: swapper/1 Not tainted 3.11.6-1.el6.elrepo.x86_64 #1 Nov 17 18:56:19 joker4 kernel: Hardware name: To Be Filled By O.E.M. Z96F/Z96F, BIOS 080012 08/29/2006 Nov 17 18:56:19 joker4 kernel: 0006 8800b73038e0 815f7f89 0010 Nov 17 18:56:19 joker4 kernel: 00200020 8800b7303970
Re: [Intel-gfx] [PATCH 3/3] tests/gem_reset_stats: check non root access to reset_stats
On Wed, Nov 20, 2013 at 04:58:17PM +0200, Mika Kuoppala wrote:
Getting global reset count needs to be tested with root and
non root access.
Signed-off-by: Mika Kuoppala mika.kuopp...@intel.com
All merged to igt, thanks for the patches.
-Daniel
---
tests/gem_reset_stats.c | 70
+--
1 file changed, 61 insertions(+), 9 deletions(-)
diff --git a/tests/gem_reset_stats.c b/tests/gem_reset_stats.c
index 9e52b60..5252833 100644
--- a/tests/gem_reset_stats.c
+++ b/tests/gem_reset_stats.c
@@ -637,6 +637,17 @@ static void test_close_pending(void)
close(fd);
}
+static void drop_root(void)
+{
+ igt_assert(getuid() == 0);
+
+ igt_assert(setgid(2) == 0);
+ igt_assert(setuid(2) == 0);
+
+ igt_assert(getgid() == 2);
+ igt_assert(getuid() == 2);
+}
+
static void __test_count(const bool create_ctx)
{
int fd, h, ctx;
@@ -661,9 +672,21 @@ static void __test_count(const bool create_ctx)
assert_reset_status(fd, ctx, RS_BATCH_ACTIVE);
c2 = get_reset_count(fd, ctx);
igt_assert(c2 = 0);
-
igt_assert(c2 == (c1 + 1));
+ igt_fork(child, 1) {
+ drop_root();
+
+ c2 = get_reset_count(fd, ctx);
+
+ if (ctx == 0)
+ igt_assert(c2 == -EPERM);
+ else
+ igt_assert(c2 == 0);
+ }
+
+ igt_waitchildren();
+
gem_close(fd, h);
if (create_ctx)
@@ -710,16 +733,50 @@ static int _test_params(int fd, int ctx, uint32_t
flags, uint32_t pad)
return 0;
}
-static void test_param_ctx(int fd, int ctx)
+typedef enum { root = 0, user } cap_t;
+
+static void test_param_ctx(const int fd, const int ctx, const cap_t cap)
{
const uint32_t bad = rand() + 1;
- igt_assert(_test_params(fd, ctx, 0, 0) == 0);
+ if (ctx == 0) {
+ if (cap == root)
+ igt_assert(_test_params(fd, ctx, 0, 0) == 0);
+ else
+ igt_assert(_test_params(fd, ctx, 0, 0) == -EPERM);
+ }
+
igt_assert(_test_params(fd, ctx, 0, bad) == -EINVAL);
igt_assert(_test_params(fd, ctx, bad, 0) == -EINVAL);
igt_assert(_test_params(fd, ctx, bad, bad) == -EINVAL);
}
+static void check_params(const int fd, const int ctx, cap_t cap)
+{
+ igt_assert(ioctl(fd, GET_RESET_STATS_IOCTL, 0) == -1);
+ igt_assert(_test_params(fd, 0xbadbad, 0, 0) == -ENOENT);
+
+ test_param_ctx(fd, 0, cap);
+ test_param_ctx(fd, ctx, cap);
+}
+
+static void _test_param(const int fd, const int ctx)
+{
+ check_params(fd, ctx, root);
+
+ igt_fork(child, 1) {
+ check_params(fd, ctx, root);
+
+ drop_root();
+
+ check_params(fd, ctx, user);
+ }
+
+ check_params(fd, ctx, root);
+
+ igt_waitchildren();
+}
+
static void test_params(void)
{
int fd, ctx;
@@ -728,12 +785,7 @@ static void test_params(void)
igt_assert(fd = 0);
ctx = context_create(fd);
- igt_assert(ioctl(fd, GET_RESET_STATS_IOCTL, 0) == -1);
-
- igt_assert(_test_params(fd, 0xbadbad, 0, 0) == -ENOENT);
-
- test_param_ctx(fd, 0);
- test_param_ctx(fd, ctx);
+ _test_param(fd, ctx);
close(fd);
}
--
1.7.9.5
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 0/4] xf86-video-intel DRI3 and Present patch series
Here's a series of patches which provide DRI3 and Present support in the Intel 2D driver. The first two patches pave the way by synthesizing 64-bit vblank counters and extending the DRM event handling to allow for both DRI2 and DRI3 events. Then there's a patch to add DRI2 and miSyncShm support followed by a patch to add Present support. [PATCH 1/4] Support 64-bit MSC values. Handle kernel vageries about [PATCH 2/4] Restructure DRM event handling. [PATCH 3/4] Add DRI3 and miSyncShm support [PATCH 4/4] Add Present extension support -keith ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 4/4] Add Present extension support
Signed-off-by: Keith Packard kei...@keithp.com --- configure.ac| 15 ++ src/uxa/Makefile.am | 6 + src/uxa/intel.h | 15 ++ src/uxa/intel_driver.c | 4 + src/uxa/intel_present.c | 406 5 files changed, 446 insertions(+) create mode 100644 src/uxa/intel_present.c diff --git a/configure.ac b/configure.ac index 13b9970..8d881a8 100644 --- a/configure.ac +++ b/configure.ac @@ -277,6 +277,7 @@ XORG_DRIVER_CHECK_EXT(RENDER, renderproto) XORG_DRIVER_CHECK_EXT(XF86DRI, xextproto x11) XORG_DRIVER_CHECK_EXT(DPMSExtension, xextproto) XORG_DRIVER_CHECK_EXT(DRI3, dri3proto) +XORG_DRIVER_CHECK_EXT(PRESENT, presentproto) # Obtain compiler/linker options for the driver dependencies PKG_CHECK_MODULES(DRM, [libdrm = 2.4.20]) # libdrm_intel is checked separately @@ -477,6 +478,19 @@ AC_MSG_CHECKING([whether to include DRI3 support]) AM_CONDITIONAL(DRI3, test x$DRI3 = xyes) AC_MSG_RESULT([$DRI3]) +if test x$PRESENT != xno; then + save_CFLAGS=$CFLAGS + CFLAGS=$XORG_CFLAGS $PRESENT_CFLAGS + AC_CHECK_DECL(PRESENT, + [PRESENT=yes], [PRESENT=no], + [#include xorg-server.h]) + CFLAGS=$save_CFLAGS +fi +echo 'PRESENT is now ' $PRESENT +AC_MSG_CHECKING([whether to include Present support]) +AM_CONDITIONAL(PRESENT, test x$PRESENT = xyes) +AC_MSG_RESULT([$PRESENT]) + AC_CHECK_HEADERS([X11/extensions/dpmsconst.h]) AC_MSG_CHECKING([whether to include UXA support]) @@ -731,6 +745,7 @@ echo Additional debugging support?$debug_msg echo Support for Kernel Mode Setting? $KMS echo Support for legacy User Mode Setting (for i810)? $UMS echo Support for Direct Rendering Infrastructure:$dri_msg +echo Support for Present extension? $PRESENT echo Support for Xv motion compensation (XvMC and libXvMC):$xvmc_msg echo Build additional tools and utilities?$tools_msg if test -n $xp_msg; then diff --git a/src/uxa/Makefile.am b/src/uxa/Makefile.am index 3c9e693..1f6f942 100644 --- a/src/uxa/Makefile.am +++ b/src/uxa/Makefile.am @@ -87,6 +87,12 @@ libuxa_la_SOURCES += \ $(NULL) endif +if PRESENT +libuxa_la_SOURCES += \ + intel_present.c \ + $(NULL) +endif + if XVMC AM_CFLAGS += -I$(top_srcdir)/xvmc libuxa_la_SOURCES += \ diff --git a/src/uxa/intel.h b/src/uxa/intel.h index c3d00f4..48711e4 100644 --- a/src/uxa/intel.h +++ b/src/uxa/intel.h @@ -742,4 +742,19 @@ intel_sync_init(ScreenPtr screen); void intel_sync_close(ScreenPtr screen); +/* + * intel_present.c + */ + +#if 0 +#define DebugPresent(x) ErrorF x +#else +#define DebugPresent(x) +#endif + +#if PRESENT +Bool +intel_present_screen_init(ScreenPtr screen); +#endif + #endif /* _I830_H_ */ diff --git a/src/uxa/intel_driver.c b/src/uxa/intel_driver.c index 12c7b34..c760ff6 100644 --- a/src/uxa/intel_driver.c +++ b/src/uxa/intel_driver.c @@ -1050,6 +1050,10 @@ I830ScreenInit(SCREEN_INIT_ARGS_DECL) if (intel-XvEnabled) I830InitVideo(screen); +#if PRESENT +intel_present_screen_init(screen); +#endif + #if DRI3 intel_dri3_screen_init(screen); #endif diff --git a/src/uxa/intel_present.c b/src/uxa/intel_present.c new file mode 100644 index 000..297497b --- /dev/null +++ b/src/uxa/intel_present.c @@ -0,0 +1,406 @@ +/* + * Copyright © 2013 Keith Packard + * + * Permission to use, copy, modify, distribute, and sell this software and its + * documentation for any purpose is hereby granted without fee, provided that + * the above copyright notice appear in all copies and that both that copyright + * notice and this permission notice appear in supporting documentation, and + * that the name of the copyright holders not be used in advertising or + * publicity pertaining to distribution of the software without specific, + * written prior permission. The copyright holders make no representations + * about the suitability of this software for any purpose. It is provided as + * is without express or implied warranty. + * + * THE COPYRIGHT HOLDERS DISCLAIM ALL WARRANTIES WITH REGARD TO THIS SOFTWARE, + * INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS, IN NO + * EVENT SHALL THE COPYRIGHT HOLDERS BE LIABLE FOR ANY SPECIAL, INDIRECT OR + * CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM LOSS OF USE, + * DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER + * TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR PERFORMANCE + * OF THIS SOFTWARE. + */ + +#ifdef HAVE_CONFIG_H +#include config.h +#endif + +#include stdio.h +#include string.h +#include assert.h +#include sys/types.h +#include sys/stat.h +#include sys/ioctl.h +#include unistd.h +#include fcntl.h +#include sys/time.h +#include time.h +#include errno.h + +#include xf86.h +#include xf86_OSproc.h + +#include xf86Pci.h +#include xf86drm.h + +#include windowstr.h +#include shadow.h +#include fb.h + +#include intel.h +#include i830_reg.h + +#include
[Intel-gfx] [PATCH 2/4] Restructure DRM event handling.
This refactors the drm interrupt handling logic quite a bit, both to
allow for either DRI2 or Present handlers, but also to eliminate
passing pointers through the kernel. Passing pointers left the kernel
holding the only reference to some internal X server data structures.
After a server reset, the X server would end up using stale pointers
stored in those structures. Using simple integers makes it possible to
empty the queue of pending interrupt data and then ignore the stale
kernel data.
Signed-off-by: Keith Packard kei...@keithp.com
---
src/uxa/intel.h | 31 -
src/uxa/intel_display.c | 316 ++--
src/uxa/intel_dri.c | 99 ---
3 files changed, 390 insertions(+), 56 deletions(-)
diff --git a/src/uxa/intel.h b/src/uxa/intel.h
index f05b160..922b208 100644
--- a/src/uxa/intel.h
+++ b/src/uxa/intel.h
@@ -395,6 +395,25 @@ extern void
intel_mode_disable_unused_functions(ScrnInfoPtr scrn);
extern void intel_mode_remove_fb(intel_screen_private *intel);
extern void intel_mode_close(intel_screen_private *intel);
extern void intel_mode_fini(intel_screen_private *intel);
+extern int intel_mode_read_drm_events(intel_screen_private *intel);
+
+typedef void (*intel_drm_handler_proc)(ScrnInfoPtr scrn,
+ xf86CrtcPtr crtc,
+ uint64_t seq,
+ uint64_t usec,
+ void *data);
+
+typedef void (*intel_drm_abort_proc)(ScrnInfoPtr scrn,
+ xf86CrtcPtr crtc,
+ void *data);
+
+extern uint32_t intel_drm_queue_alloc(ScrnInfoPtr scrn, xf86CrtcPtr crtc, void
*data, intel_drm_handler_proc handler, intel_drm_abort_proc abort);
+extern void intel_drm_abort(ScrnInfoPtr scrn, Bool (*match)(void *data, void
*match_data), void *match_data);
+
+/* struct intel_mode *
+ intel_page_flip_handler(void *event_data); */
+
+
extern int intel_get_pipe_from_crtc_id(drm_intel_bufmgr *bufmgr, xf86CrtcPtr
crtc);
extern int intel_crtc_id(xf86CrtcPtr crtc);
@@ -422,6 +441,12 @@ typedef void (*DRI2SwapEventPtr)(ClientPtr client, void
*data, int type,
CARD64 ust, CARD64 msc, CARD64 sbc);
#endif
+typedef void (*intel_pageflip_handler_proc) (uint64_t frame,
+ uint64_t usec,
+ void *data);
+
+typedef void (*intel_pageflip_abort_proc) (void *data);
+
typedef struct _DRI2FrameEvent {
struct intel_screen_private *intel;
@@ -444,7 +469,11 @@ typedef struct _DRI2FrameEvent {
extern Bool intel_do_pageflip(intel_screen_private *intel,
dri_bo *new_front,
- DRI2FrameEventPtr flip_info, int ref_crtc_hw_id);
+ int ref_crtc_hw_id,
+ Bool async,
+ void *pageflip_data,
+ intel_pageflip_handler_proc pageflip_handler,
+ intel_pageflip_abort_proc pageflip_abort);
static inline intel_screen_private *
intel_get_screen_private(ScrnInfoPtr scrn)
diff --git a/src/uxa/intel_display.c b/src/uxa/intel_display.c
index 09cd48f..e6cc07a 100644
--- a/src/uxa/intel_display.c
+++ b/src/uxa/intel_display.c
@@ -61,6 +61,23 @@
#define KNOWN_MODE_FLAGS ((114)-1)
+struct intel_drm_queue {
+struct xorg_listlist;
+xf86CrtcPtr crtc;
+uint32_tseq;
+void*data;
+ScrnInfoPtr scrn;
+intel_drm_handler_proc handler;
+intel_drm_abort_procabort;
+};
+
+static void
+intel_drm_abort_scrn(ScrnInfoPtr scrn);
+
+static uint32_t intel_drm_seq;
+
+static struct xorg_list intel_drm_queue;
+
struct intel_mode {
int fd;
uint32_t fb_id;
@@ -68,7 +85,6 @@ struct intel_mode {
int cpp;
drmEventContext event_context;
- DRI2FrameEventPtr flip_info;
int old_fb_id;
int flip_count;
uint64_t fe_msc;
@@ -76,6 +92,10 @@ struct intel_mode {
struct list outputs;
struct list crtcs;
+
+void *pageflip_data;
+intel_pageflip_handler_proc pageflip_handler;
+intel_pageflip_abort_proc pageflip_abort;
};
struct intel_pageflip {
@@ -536,6 +556,7 @@ intel_crtc_apply(xf86CrtcPtr crtc)
if (scrn-pScreen)
xf86_reload_cursors(scrn-pScreen);
+intel_drm_abort_scrn(scrn);
done:
free(output_ids);
@@ -1138,11 +1159,23 @@ intel_output_dpms(xf86OutputPtr output, int dpms)
dpms);
intel_output-dpms_mode = dpms;
drmModeFreeProperty(props);
- return;
+break;
}
[Intel-gfx] [PATCH 3/4] Add DRI3 and miSyncShm support
Signed-off-by: Keith Packard kei...@keithp.com
---
configure.ac | 14
src/uxa/Makefile.am| 7 ++
src/uxa/intel.h| 17 +
src/uxa/intel_dri3.c | 184 +
src/uxa/intel_driver.c | 13
src/uxa/intel_sync.c | 109 +
src/uxa/intel_uxa.c| 1 +
7 files changed, 345 insertions(+)
create mode 100644 src/uxa/intel_dri3.c
create mode 100644 src/uxa/intel_sync.c
diff --git a/configure.ac b/configure.ac
index 0783d61..13b9970 100644
--- a/configure.ac
+++ b/configure.ac
@@ -276,6 +276,7 @@ XORG_DRIVER_CHECK_EXT(RANDR, randrproto)
XORG_DRIVER_CHECK_EXT(RENDER, renderproto)
XORG_DRIVER_CHECK_EXT(XF86DRI, xextproto x11)
XORG_DRIVER_CHECK_EXT(DPMSExtension, xextproto)
+XORG_DRIVER_CHECK_EXT(DRI3, dri3proto)
# Obtain compiler/linker options for the driver dependencies
PKG_CHECK_MODULES(DRM, [libdrm = 2.4.20]) # libdrm_intel is checked separately
@@ -463,6 +464,19 @@ else
UXA=no
fi
+if test x$DRI3 != xno; then
+ save_CFLAGS=$CFLAGS
+ CFLAGS=$XORG_CFLAGS $DRM_CFLAGS $DRI_CFLAGS $DRI3_CFLAGS
+ AC_CHECK_DECL(DRI3,
+ [DRI3=yes], [DRI3=no],
+ [#include xorg-server.h])
+ CFLAGS=$save_CFLAGS
+ dri_msg=$dri_msg DRI3
+fi
+AC_MSG_CHECKING([whether to include DRI3 support])
+AM_CONDITIONAL(DRI3, test x$DRI3 = xyes)
+AC_MSG_RESULT([$DRI3])
+
AC_CHECK_HEADERS([X11/extensions/dpmsconst.h])
AC_MSG_CHECKING([whether to include UXA support])
diff --git a/src/uxa/Makefile.am b/src/uxa/Makefile.am
index 971ac21..3c9e693 100644
--- a/src/uxa/Makefile.am
+++ b/src/uxa/Makefile.am
@@ -80,6 +80,13 @@ libuxa_la_LIBADD += \
$(NULL)
endif
+if DRI3
+libuxa_la_SOURCES += \
+ intel_dri3.c \
+ intel_sync.c \
+ $(NULL)
+endif
+
if XVMC
AM_CFLAGS += -I$(top_srcdir)/xvmc
libuxa_la_SOURCES += \
diff --git a/src/uxa/intel.h b/src/uxa/intel.h
index 922b208..c3d00f4 100644
--- a/src/uxa/intel.h
+++ b/src/uxa/intel.h
@@ -59,6 +59,7 @@ SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
#include xf86xv.h
#include xf86Crtc.h
#include xf86RandR12.h
+#include misync.h
#include xorg-server.h
#include pciaccess.h
@@ -352,6 +353,11 @@ typedef struct intel_screen_private {
InputHandlerProc uevent_handler;
#endif
Bool has_prime_vmap_flush;
+
+SyncScreenFuncsRec save_sync_screen_funcs;
+
+void (*flush_rendering)(struct intel_screen_private *intel);
+
} intel_screen_private;
#define INTEL_INFO(intel) ((intel)-info)
@@ -519,6 +525,9 @@ void I830DRI2FrameEventHandler(unsigned int frame, unsigned
int tv_sec,
void I830DRI2FlipEventHandler(unsigned int frame, unsigned int tv_sec,
unsigned int tv_usec, DRI2FrameEventPtr
flip_info);
+/* intel_dri3.c */
+Bool intel_dri3_screen_init(ScreenPtr screen);
+
extern Bool intel_crtc_on(xf86CrtcPtr crtc);
int intel_crtc_to_pipe(xf86CrtcPtr crtc);
@@ -725,4 +734,12 @@ static inline Bool intel_pixmap_is_offscreen(PixmapPtr
pixmap)
return priv priv-offscreen;
}
+#if DRI3
+Bool
+intel_sync_init(ScreenPtr screen);
+#endif
+
+void
+intel_sync_close(ScreenPtr screen);
+
#endif /* _I830_H_ */
diff --git a/src/uxa/intel_dri3.c b/src/uxa/intel_dri3.c
new file mode 100644
index 000..99ac9d5
--- /dev/null
+++ b/src/uxa/intel_dri3.c
@@ -0,0 +1,184 @@
+/*
+ * Copyright © 2013 Keith Packard
+ *
+ * Permission to use, copy, modify, distribute, and sell this software and its
+ * documentation for any purpose is hereby granted without fee, provided that
+ * the above copyright notice appear in all copies and that both that copyright
+ * notice and this permission notice appear in supporting documentation, and
+ * that the name of the copyright holders not be used in advertising or
+ * publicity pertaining to distribution of the software without specific,
+ * written prior permission. The copyright holders make no representations
+ * about the suitability of this software for any purpose. It is provided as
+ * is without express or implied warranty.
+ *
+ * THE COPYRIGHT HOLDERS DISCLAIM ALL WARRANTIES WITH REGARD TO THIS SOFTWARE,
+ * INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS, IN NO
+ * EVENT SHALL THE COPYRIGHT HOLDERS BE LIABLE FOR ANY SPECIAL, INDIRECT OR
+ * CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM LOSS OF USE,
+ * DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER
+ * TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR PERFORMANCE
+ * OF THIS SOFTWARE.
+ */
+
+#ifdef HAVE_CONFIG_H
+#include config.h
+#endif
+
+#include stdio.h
+#include string.h
+#include assert.h
+#include sys/types.h
+#include sys/stat.h
+#include sys/ioctl.h
+#include unistd.h
+#include fcntl.h
+#include sys/time.h
+#include time.h
+#include errno.h
+
+#include xf86.h
+#include xf86_OSproc.h
+
+#include xf86Pci.h
+#include xf86drm.h
+
+#include
[Intel-gfx] [PATCH 1/4] Support 64-bit MSC values. Handle kernel vageries about MSC reporting
The kernel sometimes reports bogus MSC values, especially when
suspending and resuming the machine. Deal with this by tracking an
offset to ensure that the MSC seen by applications increases
monotonically, and at a reasonable pace.
Also, provide a full 64 bits of MSC value by noticing wrapping and
tracking the high 32-bits of MSC separately.
Signed-off-by: Keith Packard kei...@keithp.com
---
src/uxa/intel.h | 9
src/uxa/intel_display.c | 118 -
src/uxa/intel_dri.c | 125
3 files changed, 156 insertions(+), 96 deletions(-)
diff --git a/src/uxa/intel.h b/src/uxa/intel.h
index 131f18c..f05b160 100644
--- a/src/uxa/intel.h
+++ b/src/uxa/intel.h
@@ -401,6 +401,15 @@ extern int intel_crtc_id(xf86CrtcPtr crtc);
extern int intel_output_dpms_status(xf86OutputPtr output);
extern void intel_copy_fb(ScrnInfoPtr scrn);
+int
+intel_get_crtc_msc_ust(ScrnInfoPtr scrn, xf86CrtcPtr crtc, uint64_t *msc,
uint64_t *ust);
+
+uint32_t
+intel_crtc_msc_to_sequence(ScrnInfoPtr scrn, xf86CrtcPtr crtc, uint64_t
expect);
+
+uint64_t
+intel_sequence_to_crtc_msc(xf86CrtcPtr crtc, uint32_t sequence);
+
enum DRI2FrameEventType {
DRI2_SWAP,
DRI2_SWAP_CHAIN,
diff --git a/src/uxa/intel_display.c b/src/uxa/intel_display.c
index 3c2f964..09cd48f 100644
--- a/src/uxa/intel_display.c
+++ b/src/uxa/intel_display.c
@@ -71,9 +71,8 @@ struct intel_mode {
DRI2FrameEventPtr flip_info;
int old_fb_id;
int flip_count;
- unsigned int fe_frame;
- unsigned int fe_tv_sec;
- unsigned int fe_tv_usec;
+ uint64_t fe_msc;
+uint64_t fe_usec;
struct list outputs;
struct list crtcs;
@@ -97,6 +96,9 @@ struct intel_crtc {
struct list link;
PixmapPtr scanout_pixmap;
uint32_t scanout_fb_id;
+int32_t vblank_offset;
+uint32_t msc_prev;
+uint64_t msc_high;
};
struct intel_property {
@@ -1647,9 +1649,8 @@ intel_do_pageflip(intel_screen_private *intel,
* Also, flips queued on disabled or incorrectly configured displays
* may never complete; this is a configuration error.
*/
- mode-fe_frame = 0;
- mode-fe_tv_sec = 0;
- mode-fe_tv_usec = 0;
+ mode-fe_msc = 0;
+ mode-fe_usec = 0;
for (i = 0; i config-num_crtc; i++) {
if (!intel_crtc_on(config-crtc[i]))
@@ -1705,6 +1706,102 @@ static const xf86CrtcConfigFuncsRec
intel_xf86crtc_config_funcs = {
intel_xf86crtc_resize
};
+static uint32_t pipe_select(int pipe)
+{
+ if (pipe 1)
+ return pipe DRM_VBLANK_HIGH_CRTC_SHIFT;
+ else if (pipe 0)
+ return DRM_VBLANK_SECONDARY;
+ else
+ return 0;
+}
+
+/*
+ * Get the current msc/ust value from the kernel
+ */
+static int
+intel_get_msc_ust(ScrnInfoPtr scrn, xf86CrtcPtr crtc, uint32_t *msc, uint64_t
*ust)
+{
+ intel_screen_private*intel = intel_get_screen_private(scrn);
+drmVBlank vbl;
+int ret;
+
+/* Get current count */
+vbl.request.type = DRM_VBLANK_RELATIVE |
pipe_select(intel_crtc_to_pipe(crtc));
+vbl.request.sequence = 0;
+vbl.request.signal = 0;
+ret = drmWaitVBlank(intel-drmSubFD, vbl);
+if (ret) {
+*msc = 0;
+*ust = 0;
+return BadMatch;
+} else {
+*msc = vbl.reply.sequence;
+*ust = (CARD64) vbl.reply.tval_sec * 100 +
vbl.reply.tval_usec;
+}
+return Success;
+}
+
+/*
+ * Convert a 32-bit kernel MSC sequence number to a 64-bit local sequence
+ * number, adding in the vblank_offset and high 32 bits, and dealing
+ * with 64-bit wrapping
+ */
+uint64_t
+intel_sequence_to_crtc_msc(xf86CrtcPtr crtc, uint32_t sequence)
+{
+ struct intel_crtc *intel_crtc = crtc-driver_private;
+sequence += intel_crtc-vblank_offset;
+
+if ((int32_t) (sequence - intel_crtc-msc_prev) -0x4000)
+intel_crtc-msc_high += 0x1L;
+intel_crtc-msc_prev = sequence;
+return intel_crtc-msc_high + sequence;
+}
+
+/*
+ * Get the current 64-bit adjust MSC and UST value
+ */
+int
+intel_get_crtc_msc_ust(ScrnInfoPtr scrn, xf86CrtcPtr crtc, uint64_t *msc,
uint64_t *ust)
+{
+uint32_tsequence;
+int ret;
+
+ret = intel_get_msc_ust(scrn, crtc, sequence, ust);
+*msc = intel_sequence_to_crtc_msc(crtc, sequence);
+return ret;
+}
+
+/*
+ * Convert a 64-bit adjusted MSC value into a 32-bit kernel sequence number,
+ * removing the high 32 bits and subtracting out the vblank_offset term.
+ *
+ * This also updates the vblank_offset when it notices that the value should
+ * change.
+ */
+uint32_t
+intel_crtc_msc_to_sequence(ScrnInfoPtr scrn, xf86CrtcPtr crtc,
Re: [Intel-gfx] i915 driver gpu hung kernel 3.11
Hi Stephen, On Wed, 20 November 2013 Stephen Clark sclar...@earthlink.net wrote: Hi Bruno, I have tested the latest kernel and X, mesa etc, but am still using wine-1.3.24. I am working on upgrading that. If I still have the error I will file a bug report at bugs.freedesktop.org. I already have a login because of the same problem happening with Myst 5, but it was never resolved. If you add an error_state file to the bug you should have rather good chance to get it solved (of course mentioning the various software versions in use - mesa, libdrm, xf86-video-intel, wine, kernel). Do you know if there is a comprehensive set of test I can run to make sure my hardware is OK. When I run dxdiag under wine it passes all tests, but then when trying to play Myst online or Myst 5 I get the gpu hung situation. I've not heard of a comprehensive test suite though. It probably is a bug in the driver (libdrm, mesa or xf86-video-intel). I think I've identified your bug as #32582 for the Myth 5 hang. As Chris Wilson has already replied to it, it's maybe just a matter of re-testing with current software, mentioning those versions (and including error_state). If you get no feedback you usually have a good chance attracting attention to the bug(s) by showing up on #intel-gfx IRC channel on freenode and referring to the bug (and stay around long enough to catch possible replies - an be prepared to apply patches and recompile/test to verify if a proposed fix helps). If there are multiple games hanging the GPU (via Wine) they might even all trigger the same issue, thus having error_state for both will be an advantage. Bruno Anyway thanks for taking the time to respond. Regards, Steve ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] AVI infoframes: default aspect ratio/VIC for CEA modes
Folks,
A newbie question - When filling in an HDMI AVI infoframe, how does one
correctly determine the default picture aspect ratio (and VIC) for CEA modes
that support multiple (4:3 and 16:9) aspect ratios. 720x576p for example,
corresponds to VIC 17 or 18:
/* 17 - 720x576@50Hz */
{ DRM_MODE(720x576, DRM_MODE_TYPE_DRIVER, 27000, 720, 732,
796, 864, 0, 576, 581, 586, 625, 0,
DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC),
.vrefresh = 50, },
/* 18 - 720x576@50Hz */
{ DRM_MODE(720x576, DRM_MODE_TYPE_DRIVER, 27000, 720, 732,
796, 864, 0, 576, 581, 586, 625, 0,
DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC),
.vrefresh = 50, },
Should the picture aspect ratio information be derived from sink EDID (from
detailed/cvt/standard timings)? .. possibly in drm_add_edid_modes() and store
the picture aspect ratio in drm_display_mode perhaps, for later use? Trying to
get a better understanding of how this usually works.
As an aside, to match VIC correctly, shouldn't drm_match_cea_mode() take into
account picture aspect ratio as well?
Cheers,
Tushar
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] AVI infoframes: default aspect ratio/VIC for CEA modes
On Wed, Nov 20, 2013 at 10:11:55PM +, Damien Lespiau wrote:
On Wed, Nov 20, 2013 at 09:48:26PM +, Gohad, Tushar wrote:
Folks,
A newbie question - When filling in an HDMI AVI infoframe, how does
one correctly determine the default picture aspect ratio (and VIC)
for CEA modes that support multiple (4:3 and 16:9) aspect ratios.
720x576p for example, corresponds to VIC 17 or 18:
/* 17 - 720x576@50Hz */
{ DRM_MODE(720x576, DRM_MODE_TYPE_DRIVER, 27000, 720, 732,
796, 864, 0, 576, 581, 586, 625, 0,
DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC),
.vrefresh = 50, },
/* 18 - 720x576@50Hz */
{ DRM_MODE(720x576, DRM_MODE_TYPE_DRIVER, 27000, 720, 732,
796, 864, 0, 576, 581, 586, 625, 0,
DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC),
.vrefresh = 50, },
Should the picture aspect ratio information be derived from sink
EDID (from detailed/cvt/standard timings)? .. possibly in
drm_add_edid_modes() and store the picture aspect ratio in
drm_display_mode perhaps, for later use? Trying to get a better
understanding of how this usually works.
Oh no! someone finally discovered it! Yes, we are totally missing the
picture aspect ratio information from the CEA modes. It's been on my
TODO list for a couple of month but not exactly high priority. If we
want to get a stab a it, we'll review the patches :)
I realized that I did not actually answer the question. The CEA modes
come with their defined aspect ratio, it's part of the CEA 861 standard.
In this case VIC 17 is 4:3, VIC 18 is 16:9.
--
Damien
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 01/10] drm/i915: Grab struct_mutex around all FBC updates
On Wed, Nov 06, 2013 at 11:02:16PM +0200, ville.syrj...@linux.intel.com wrote: From: Ville Syrjälä ville.syrj...@linux.intel.com We need some protection for the FBC state, and since struct_mutex is it currently in most places, make sure all FBC update/disable calles are protected by it. Why don't you create a new mutex only for fbc update? Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com --- drivers/gpu/drm/i915/intel_display.c | 10 ++ 1 file changed, 10 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 12cf362..bce6e07 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -3576,8 +3576,10 @@ static void haswell_crtc_disable_planes(struct drm_crtc *crtc) drm_vblank_off(dev, pipe); /* FBC must be disabled before disabling the plane on HSW. */ + mutex_lock(dev-struct_mutex); if (dev_priv-fbc.plane == plane) intel_disable_fbc(dev); + mutex_unlock(dev-struct_mutex); hsw_disable_ips(intel_crtc); @@ -3717,8 +3719,10 @@ static void ironlake_crtc_disable(struct drm_crtc *crtc) intel_crtc_wait_for_pending_flips(crtc); drm_vblank_off(dev, pipe); + mutex_lock(dev-struct_mutex); if (dev_priv-fbc.plane == plane) intel_disable_fbc(dev); + mutex_unlock(dev-struct_mutex); intel_crtc_update_cursor(crtc, false); intel_disable_planes(crtc); @@ -4102,7 +4106,9 @@ static void valleyview_crtc_enable(struct drm_crtc *crtc) intel_enable_planes(crtc); intel_crtc_update_cursor(crtc, true); + mutex_lock(dev-struct_mutex); intel_update_fbc(dev); + mutex_unlock(dev-struct_mutex); for_each_encoder_on_crtc(dev, crtc, encoder) encoder-enable(encoder); @@ -4146,7 +4152,9 @@ static void i9xx_crtc_enable(struct drm_crtc *crtc) /* Give the overlay scaler a chance to enable if it's on this pipe */ intel_crtc_dpms_overlay(intel_crtc, true); + mutex_lock(dev-struct_mutex); intel_update_fbc(dev); + mutex_unlock(dev-struct_mutex); for_each_encoder_on_crtc(dev, crtc, encoder) encoder-enable(encoder); @@ -4210,7 +4218,9 @@ static void i9xx_crtc_disable(struct drm_crtc *crtc) intel_crtc-active = false; intel_update_watermarks(crtc); + mutex_lock(dev-struct_mutex); intel_update_fbc(dev); + mutex_unlock(dev-struct_mutex); } static void i9xx_crtc_off(struct drm_crtc *crtc) -- 1.8.1.5 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 04/10] drm/i915: Limit FBC flush to post batch flush
On Wed, Nov 06, 2013 at 11:02:19PM +0200, ville.syrj...@linux.intel.com wrote: From: Ville Syrjälä ville.syrj...@linux.intel.com Don't issue the FBC nuke/cache clean command when invalidate_domains!=0. Double negative almost confused me, but all right here. Reviewed-by: Rodrigo Vivi rodrigo.v...@gmail.com That would indicate that we're not being called for the post-batch flush. Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com --- drivers/gpu/drm/i915/intel_ringbuffer.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index e32c08a..752f208 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -354,7 +354,7 @@ gen7_render_ring_flush(struct intel_ring_buffer *ring, intel_ring_emit(ring, 0); intel_ring_advance(ring); - if (flush_domains) + if (!invalidate_domains flush_domains) return gen7_ring_fbc_flush(ring, FBC_REND_NUKE); return 0; @@ -1837,7 +1837,7 @@ static int gen6_ring_flush(struct intel_ring_buffer *ring, } intel_ring_advance(ring); - if (IS_GEN7(dev) flush) + if (IS_GEN7(dev) !invalidate flush) return gen7_ring_fbc_flush(ring, FBC_REND_CACHE_CLEAN); return 0; -- 1.8.1.5 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 05/10] drm/i915: Emit SRM after the MSG_FBC_REND_STATE LRI
Reviewed-by: Rodrigo Vivi rodrigo.v...@gmail.com On Wed, Nov 06, 2013 at 11:02:20PM +0200, ville.syrj...@linux.intel.com wrote: From: Ville Syrjälä ville.syrj...@linux.intel.com The spec tells us that we need to emit an SRM after the LRI to MSG_FBC_REND_STATE. Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com --- drivers/gpu/drm/i915/i915_reg.h | 1 + drivers/gpu/drm/i915/intel_ringbuffer.c | 6 -- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 0719c8b..7a4d3e1 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -235,6 +235,7 @@ */ #define MI_LOAD_REGISTER_IMM(x) MI_INSTR(0x22, 2*x-1) #define MI_STORE_REGISTER_MEM(x) MI_INSTR(0x24, 2*x-1) +#define MI_SRM_LRM_GLOBAL_GTT (122) #define MI_FLUSH_DW MI_INSTR(0x26, 1) /* for GEN6 */ #define MI_FLUSH_DW_STORE_INDEX(121) #define MI_INVALIDATE_TLB (118) diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index 752f208..4649bf5 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -285,14 +285,16 @@ static int gen7_ring_fbc_flush(struct intel_ring_buffer *ring, u32 value) if (!ring-fbc_dirty) return 0; - ret = intel_ring_begin(ring, 4); + ret = intel_ring_begin(ring, 6); if (ret) return ret; - intel_ring_emit(ring, MI_NOOP); /* WaFbcNukeOn3DBlt:ivb/hsw */ intel_ring_emit(ring, MI_LOAD_REGISTER_IMM(1)); intel_ring_emit(ring, MSG_FBC_REND_STATE); intel_ring_emit(ring, value); + intel_ring_emit(ring, MI_STORE_REGISTER_MEM(1) | MI_SRM_LRM_GLOBAL_GTT); + intel_ring_emit(ring, MSG_FBC_REND_STATE); + intel_ring_emit(ring, ring-scratch.gtt_offset + 256); intel_ring_advance(ring); ring-fbc_dirty = false; -- 1.8.1.5 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3 06/10] drm/i915: Implement LRI based FBC tracking
On Wed, Nov 06, 2013 at 11:02:21PM +0200, ville.syrj...@linux.intel.com wrote:
From: Ville Syrjälä ville.syrj...@linux.intel.com
As per the SNB and HSW PM guides, we should enable FBC render/blitter
tracking only during batches targetting the front buffer.
You improved things a lot here, but I'm just not convinced this is tracking
only and all front buffer touches.
On SNB we must also update the FBC render tracking address whenever it
changes. And since the register in question is stored in the context,
we need to make sure we reload it with correct data after context
switches.
On IVB/HSW we use the render nuke mechanism, so no render tracking
address updates are needed. Hoever on the blitter side we need to
enable the blitter tracking like on SNB, and in addition we need
to issue the cache clean messages, which we already did.
v2: Introduce intel_fb_obj_has_fbc()
Fix crtc locking around crtc-fb access
Drop a hunk that was included by accident in v1
Set fbc_address_dirty=false not true after emitting the LRI
v3: Now that fbc hangs on to the fb intel_fb_obj_has_fbc() doesn't
need to upset lockdep anymore
Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
---
drivers/gpu/drm/i915/i915_gem_context.c| 7
drivers/gpu/drm/i915/i915_gem_execbuffer.c | 31
drivers/gpu/drm/i915/intel_display.c | 17 +++--
drivers/gpu/drm/i915/intel_drv.h | 1 +
drivers/gpu/drm/i915/intel_ringbuffer.c| 58
+-
drivers/gpu/drm/i915/intel_ringbuffer.h| 2 ++
6 files changed, 113 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c
b/drivers/gpu/drm/i915/i915_gem_context.c
index 72a3df3..d438ea1 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -404,6 +404,13 @@ mi_set_context(struct intel_ring_buffer *ring,
intel_ring_advance(ring);
+ /*
+ * FBC RT address is stored in the context, so we may have just
+ * restored it to an old value. Make sure we emit a new LRI
+ * to update the address.
+ */
+ ring-fbc_address_dirty = true;
+
return ret;
}
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 885d595..db25158 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -886,6 +886,35 @@ validate_exec_list(struct drm_i915_gem_exec_object2
*exec,
}
static void
+i915_gem_execbuffer_mark_fbc_dirty(struct intel_ring_buffer *ring,
+struct list_head *vmas)
+{
+ struct i915_vma *vma;
+ struct drm_i915_gem_object *fbc_obj = NULL;
+ u32 fbc_address = -1;
+
+ list_for_each_entry(vma, vmas, exec_list) {
+ struct drm_i915_gem_object *obj = vma-obj;
+
+ if (obj-base.pending_write_domain
+ intel_fb_obj_has_fbc(obj)) {
+ WARN_ON(fbc_obj fbc_obj != obj);
+ fbc_obj = obj;
+ }
+ }
+
+ if (fbc_obj)
+ fbc_address = i915_gem_obj_ggtt_offset(fbc_obj);
+
+ /* need to nuke/cache_clean on IVB+? */
+ ring-fbc_dirty = fbc_obj != NULL;
+
+ /* need to update FBC tracking? */
+ ring-fbc_address_dirty = fbc_address != ring-fbc_address;
+ ring-fbc_address = fbc_address;
+}
+
+static void
i915_gem_execbuffer_move_to_active(struct list_head *vmas,
struct intel_ring_buffer *ring)
{
@@ -1150,6 +1179,8 @@ i915_gem_do_execbuffer(struct drm_device *dev, void
*data,
if (flags I915_DISPATCH_SECURE !batch_obj-has_global_gtt_mapping)
i915_gem_gtt_bind_object(batch_obj, batch_obj-cache_level);
+ i915_gem_execbuffer_mark_fbc_dirty(ring, eb-vmas);
+
ret = i915_gem_execbuffer_move_to_gpu(ring, eb-vmas);
if (ret)
goto err;
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index bce6e07..c29e9d4 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -8074,6 +8074,21 @@ void intel_mark_idle(struct drm_device *dev)
gen6_rps_idle(dev-dev_private);
}
+bool intel_fb_obj_has_fbc(struct drm_i915_gem_object *obj)
+{
+ struct drm_device *dev = obj-base.dev;
+ struct drm_i915_private *dev_priv = dev-dev_private;
+
+ /* check for potential scanout */
+ if (!obj-pin_display)
+ return false;
+
+ if (!dev_priv-fbc.fb)
+ return false;
+
+ return to_intel_framebuffer(dev_priv-fbc.fb)-obj == obj;
+}
+
void intel_mark_fb_busy(struct drm_i915_gem_object *obj,
struct intel_ring_buffer *ring)
{
@@ -8091,8 +8106,6 @@ void intel_mark_fb_busy(struct drm_i915_gem_object *obj,
Re: [Intel-gfx] [PATCH v2 08/10] drm/i915: Don't write ILK/IVB_FBC_RT_BASE directly
On Wed, Nov 06, 2013 at 11:02:23PM +0200, ville.syrj...@linux.intel.com wrote: From: Ville Syrjälä ville.syrj...@linux.intel.com We use LRIs to enable/disable the render tracking as needed. So leave ILK_FBC_RT_BASE alone when enabling FBC on SNB. Shouldn't this be part of patch 6 While at it, kill the IVB_FBC_RT_BASE completely since we don't use render tracking on IVB+. and this a new patch? TODO: Make ILK use the LRI mechanism too? v2: Drop the IVB_FBC_RT_BASE write too Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com --- drivers/gpu/drm/i915/intel_pm.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 3f600ba..4f5293e7 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -212,7 +212,8 @@ static void ironlake_enable_fbc(struct drm_crtc *crtc, (stall_watermark DPFC_RECOMP_STALL_WM_SHIFT) | (interval DPFC_RECOMP_TIMER_COUNT_SHIFT)); I915_WRITE(ILK_DPFC_FENCE_YOFF, crtc-y); - I915_WRITE(ILK_FBC_RT_BASE, i915_gem_obj_ggtt_offset(obj) | ILK_FBC_RT_VALID); + if (IS_GEN5(dev)) + I915_WRITE(ILK_FBC_RT_BASE, i915_gem_obj_ggtt_offset(obj) | ILK_FBC_RT_VALID); /* enable it... */ I915_WRITE(ILK_DPFC_CONTROL, dpfc_ctl | DPFC_CTL_EN); @@ -257,8 +258,6 @@ static void gen7_enable_fbc(struct drm_crtc *crtc, struct drm_i915_gem_object *obj = intel_fb-obj; struct intel_crtc *intel_crtc = to_intel_crtc(crtc); - I915_WRITE(IVB_FBC_RT_BASE, i915_gem_obj_ggtt_offset(obj)); - I915_WRITE(ILK_DPFC_CONTROL, DPFC_CTL_EN | DPFC_CTL_LIMIT_1X | IVB_DPFC_CTL_FENCE_EN | intel_crtc-plane IVB_DPFC_CTL_PLANE_SHIFT); -- 1.8.1.5 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 07/10] drm/i915: Kill sandybridge_blit_fbc_update()
On Wed, Nov 06, 2013 at 11:02:22PM +0200, ville.syrj...@linux.intel.com wrote:
From: Ville Syrjälä ville.syrj...@linux.intel.com
No longer needed since the LRIs do the work.
Shouldn't this be part of previous patch?
v2: Rebased due to hunk getting dropped from an ealier patch
Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
---
drivers/gpu/drm/i915/intel_pm.c | 23 ---
1 file changed, 23 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index dc65bb4..3f600ba 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -187,26 +187,6 @@ static bool g4x_fbc_enabled(struct drm_device *dev)
return I915_READ(DPFC_CONTROL) DPFC_CTL_EN;
}
-static void sandybridge_blit_fbc_update(struct drm_device *dev)
-{
- struct drm_i915_private *dev_priv = dev-dev_private;
- u32 blt_ecoskpd;
-
- /* Make sure blitter notifies FBC of writes */
- gen6_gt_force_wake_get(dev_priv);
- blt_ecoskpd = I915_READ(GEN6_BLITTER_ECOSKPD);
- blt_ecoskpd |= GEN6_BLITTER_FBC_NOTIFY
- GEN6_BLITTER_LOCK_SHIFT;
- I915_WRITE(GEN6_BLITTER_ECOSKPD, blt_ecoskpd);
- blt_ecoskpd |= GEN6_BLITTER_FBC_NOTIFY;
- I915_WRITE(GEN6_BLITTER_ECOSKPD, blt_ecoskpd);
- blt_ecoskpd = ~(GEN6_BLITTER_FBC_NOTIFY
- GEN6_BLITTER_LOCK_SHIFT);
- I915_WRITE(GEN6_BLITTER_ECOSKPD, blt_ecoskpd);
- POSTING_READ(GEN6_BLITTER_ECOSKPD);
- gen6_gt_force_wake_put(dev_priv);
-}
-
static void ironlake_enable_fbc(struct drm_crtc *crtc,
struct drm_framebuffer *fb,
unsigned long interval)
@@ -240,7 +220,6 @@ static void ironlake_enable_fbc(struct drm_crtc *crtc,
I915_WRITE(SNB_DPFC_CTL_SA,
SNB_CPU_FENCE_ENABLE | obj-fence_reg);
I915_WRITE(DPFC_CPU_FENCE_OFFSET, crtc-y);
- sandybridge_blit_fbc_update(dev);
}
DRM_DEBUG_KMS(enabled fbc on plane %c\n,
plane_name(intel_crtc-plane));
@@ -297,8 +276,6 @@ static void gen7_enable_fbc(struct drm_crtc *crtc,
SNB_CPU_FENCE_ENABLE | obj-fence_reg);
I915_WRITE(DPFC_CPU_FENCE_OFFSET, crtc-y);
- sandybridge_blit_fbc_update(dev);
-
DRM_DEBUG_KMS(enabled fbc on plane %d\n, intel_crtc-plane);
}
--
1.8.1.5
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 10/10] drm/i915: Use plane_name() in gen7_enable_fbc()
Reviewed-by: Rodrigo Vivi rodrigo.v...@gmail.com On Wed, Nov 06, 2013 at 11:02:25PM +0200, ville.syrj...@linux.intel.com wrote: From: Ville Syrjälä ville.syrj...@linux.intel.com All the other .enable_fbc() funcs use plane_name(). Make gen7_enable_fbc() do the same. Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com --- drivers/gpu/drm/i915/intel_pm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 4f5293e7..48f6eda 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -275,7 +275,7 @@ static void gen7_enable_fbc(struct drm_crtc *crtc, SNB_CPU_FENCE_ENABLE | obj-fence_reg); I915_WRITE(DPFC_CPU_FENCE_OFFSET, crtc-y); - DRM_DEBUG_KMS(enabled fbc on plane %d\n, intel_crtc-plane); + DRM_DEBUG_KMS(enabled fbc on plane %c\n, plane_name(intel_crtc-plane)); } bool intel_fbc_enabled(struct drm_device *dev) -- 1.8.1.5 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3 06/10] drm/i915: Implement LRI based FBC tracking
On Wed, Nov 20, 2013 at 02:55:57PM -0800, Rodrigo Vivi wrote:
On Wed, Nov 06, 2013 at 11:02:21PM +0200, ville.syrj...@linux.intel.com wrote:
static void
+i915_gem_execbuffer_mark_fbc_dirty(struct intel_ring_buffer *ring,
+ struct list_head *vmas)
+{
+ struct i915_vma *vma;
+ struct drm_i915_gem_object *fbc_obj = NULL;
+ u32 fbc_address = -1;
+
+ list_for_each_entry(vma, vmas, exec_list) {
+ struct drm_i915_gem_object *obj = vma-obj;
+
+ if (obj-base.pending_write_domain
+ intel_fb_obj_has_fbc(obj)) {
+ WARN_ON(fbc_obj fbc_obj != obj);
+ fbc_obj = obj;
+ }
+ }
+
+ if (fbc_obj)
+ fbc_address = i915_gem_obj_ggtt_offset(fbc_obj);
+
+ /* need to nuke/cache_clean on IVB+? */
+ ring-fbc_dirty = fbc_obj != NULL;
+
+ /* need to update FBC tracking? */
+ ring-fbc_address_dirty = fbc_address != ring-fbc_address;
+ ring-fbc_address = fbc_address;
There's a risk here that we restart the execbuffer and on the second
pass we no longer treat the fbc_address as requiring an update.
ring-fb_address_dirty |= fbc_address != ring-fbc_address
wins for paranoia, and also makes the ordering with set_context a
non-issue.
-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] AVI infoframes: default aspect ratio/VIC for CEA modes
On Wed, Nov 20, 2013 at 09:48:26PM +, Gohad, Tushar wrote:
Folks,
When filling in an HDMI AVI infoframe, how does
one correctly determine the default picture aspect ratio (and VIC)
for CEA modes that support multiple (4:3 and 16:9) aspect ratios.
720x576p for example, corresponds to VIC 17 or 18:
/* 17 - 720x576@50Hz */
{ DRM_MODE(720x576, DRM_MODE_TYPE_DRIVER, 27000, 720,
732,
796, 864, 0, 576, 581, 586, 625, 0,
DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC),
.vrefresh = 50, },
/* 18 - 720x576@50Hz */
{ DRM_MODE(720x576, DRM_MODE_TYPE_DRIVER, 27000, 720,
732,
796, 864, 0, 576, 581, 586, 625, 0,
DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC),
.vrefresh = 50, },
Should the picture aspect ratio information be derived from sink
EDID (from detailed/cvt/standard timings)? .. possibly in
drm_add_edid_modes() and store the picture aspect ratio in
drm_display_mode perhaps, for later use? Trying to get a better
understanding of how this usually works.
Oh no! someone finally discovered it! Yes, we are totally missing the
picture aspect ratio information from the CEA modes. It's been on my
TODO list for a couple of month but not exactly high priority. If we
want to get a stab a it, we'll review the patches :)
Thanks Damien. Sure, I can come up with something quick. Is the idea then to
store aspect ratio information in drm_display_mode, possibly as a separate
member or as a hint that's part of mode-flags?
I realized that I did not actually answer the question. The CEA modes come
with their defined aspect ratio, it's part of the CEA 861 standard.
In this case VIC 17 is 4:3, VIC 18 is 16:9.
I understand the VIC/aspect ratio correlation, however in order to set the VIC
correctly, don't we need to first determine the picture aspect ratio? My
question really is, how do we select the aspect ratio for the ambiguous
cases, such as 720x576 which neither truly 4:3 nor 16:9?
Section 7.2.2 of CEA-861-D spec reads -
7.2.2 Order of Dual-Aspect Ratio Detailed Timing Descriptors
Source devices that do not support the AVI InfoFrame (e.g., DVI sources) shall
consider the first EDID
descriptor of any dual-aspect ratio timing to be the display-assumed aspect
ratio for that timing ...
How do we figure out the display-assumed aspect ratio?
I see that drm_add_edid_modes() enumerates detailed, CVT, standard,
established, CEA modes. Should picture aspect ratio determination be a part of
that process, based on the EDID info?
Cheers,
Tushar
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3 06/10] drm/i915: Implement LRI based FBC tracking
ops, I just noticed that by mistake I replied the v1-series
but I actually looked to v2 seires... Sorry about that
On Wed, Nov 20, 2013 at 3:17 PM, Chris Wilson ch...@chris-wilson.co.uk wrote:
On Wed, Nov 20, 2013 at 02:55:57PM -0800, Rodrigo Vivi wrote:
On Wed, Nov 06, 2013 at 11:02:21PM +0200, ville.syrj...@linux.intel.com
wrote:
static void
+i915_gem_execbuffer_mark_fbc_dirty(struct intel_ring_buffer *ring,
+ struct list_head *vmas)
+{
+ struct i915_vma *vma;
+ struct drm_i915_gem_object *fbc_obj = NULL;
+ u32 fbc_address = -1;
+
+ list_for_each_entry(vma, vmas, exec_list) {
+ struct drm_i915_gem_object *obj = vma-obj;
+
+ if (obj-base.pending_write_domain
+ intel_fb_obj_has_fbc(obj)) {
+ WARN_ON(fbc_obj fbc_obj != obj);
+ fbc_obj = obj;
+ }
+ }
+
+ if (fbc_obj)
+ fbc_address = i915_gem_obj_ggtt_offset(fbc_obj);
+
+ /* need to nuke/cache_clean on IVB+? */
+ ring-fbc_dirty = fbc_obj != NULL;
+
+ /* need to update FBC tracking? */
+ ring-fbc_address_dirty = fbc_address != ring-fbc_address;
+ ring-fbc_address = fbc_address;
There's a risk here that we restart the execbuffer and on the second
pass we no longer treat the fbc_address as requiring an update.
ring-fb_address_dirty |= fbc_address != ring-fbc_address
wins for paranoia, and also makes the ordering with set_context a
non-issue.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
--
Rodrigo Vivi
Blog: http://blog.vivi.eng.br
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3 06/10] drm/i915: Implement LRI based FBC tracking
actually just ignore my last msg... alternate between gmail and mutt
confused me...
On Wed, Nov 6, 2013 at 1:02 PM, ville.syrj...@linux.intel.com wrote:
From: Ville Syrjälä ville.syrj...@linux.intel.com
As per the SNB and HSW PM guides, we should enable FBC render/blitter
tracking only during batches targetting the front buffer.
On SNB we must also update the FBC render tracking address whenever it
changes. And since the register in question is stored in the context,
we need to make sure we reload it with correct data after context
switches.
On IVB/HSW we use the render nuke mechanism, so no render tracking
address updates are needed. Hoever on the blitter side we need to
enable the blitter tracking like on SNB, and in addition we need
to issue the cache clean messages, which we already did.
v2: Introduce intel_fb_obj_has_fbc()
Fix crtc locking around crtc-fb access
Drop a hunk that was included by accident in v1
Set fbc_address_dirty=false not true after emitting the LRI
v3: Now that fbc hangs on to the fb intel_fb_obj_has_fbc() doesn't
need to upset lockdep anymore
Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
---
drivers/gpu/drm/i915/i915_gem_context.c| 7
drivers/gpu/drm/i915/i915_gem_execbuffer.c | 31
drivers/gpu/drm/i915/intel_display.c | 17 +++--
drivers/gpu/drm/i915/intel_drv.h | 1 +
drivers/gpu/drm/i915/intel_ringbuffer.c| 58
+-
drivers/gpu/drm/i915/intel_ringbuffer.h| 2 ++
6 files changed, 113 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c
b/drivers/gpu/drm/i915/i915_gem_context.c
index 72a3df3..d438ea1 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -404,6 +404,13 @@ mi_set_context(struct intel_ring_buffer *ring,
intel_ring_advance(ring);
+ /*
+* FBC RT address is stored in the context, so we may have just
+* restored it to an old value. Make sure we emit a new LRI
+* to update the address.
+*/
+ ring-fbc_address_dirty = true;
+
return ret;
}
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 885d595..db25158 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -886,6 +886,35 @@ validate_exec_list(struct drm_i915_gem_exec_object2
*exec,
}
static void
+i915_gem_execbuffer_mark_fbc_dirty(struct intel_ring_buffer *ring,
+ struct list_head *vmas)
+{
+ struct i915_vma *vma;
+ struct drm_i915_gem_object *fbc_obj = NULL;
+ u32 fbc_address = -1;
+
+ list_for_each_entry(vma, vmas, exec_list) {
+ struct drm_i915_gem_object *obj = vma-obj;
+
+ if (obj-base.pending_write_domain
+ intel_fb_obj_has_fbc(obj)) {
+ WARN_ON(fbc_obj fbc_obj != obj);
+ fbc_obj = obj;
+ }
+ }
+
+ if (fbc_obj)
+ fbc_address = i915_gem_obj_ggtt_offset(fbc_obj);
+
+ /* need to nuke/cache_clean on IVB+? */
+ ring-fbc_dirty = fbc_obj != NULL;
+
+ /* need to update FBC tracking? */
+ ring-fbc_address_dirty = fbc_address != ring-fbc_address;
+ ring-fbc_address = fbc_address;
+}
+
+static void
i915_gem_execbuffer_move_to_active(struct list_head *vmas,
struct intel_ring_buffer *ring)
{
@@ -1150,6 +1179,8 @@ i915_gem_do_execbuffer(struct drm_device *dev, void
*data,
if (flags I915_DISPATCH_SECURE
!batch_obj-has_global_gtt_mapping)
i915_gem_gtt_bind_object(batch_obj, batch_obj-cache_level);
+ i915_gem_execbuffer_mark_fbc_dirty(ring, eb-vmas);
+
ret = i915_gem_execbuffer_move_to_gpu(ring, eb-vmas);
if (ret)
goto err;
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index bce6e07..c29e9d4 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -8074,6 +8074,21 @@ void intel_mark_idle(struct drm_device *dev)
gen6_rps_idle(dev-dev_private);
}
+bool intel_fb_obj_has_fbc(struct drm_i915_gem_object *obj)
+{
+ struct drm_device *dev = obj-base.dev;
+ struct drm_i915_private *dev_priv = dev-dev_private;
+
+ /* check for potential scanout */
+ if (!obj-pin_display)
+ return false;
+
+ if (!dev_priv-fbc.fb)
+ return false;
+
+ return to_intel_framebuffer(dev_priv-fbc.fb)-obj == obj;
+}
+
void intel_mark_fb_busy(struct drm_i915_gem_object *obj,
struct intel_ring_buffer *ring)
{
@@ -8091,8 +8106,6 @@ void intel_mark_fb_busy(struct
Re: [Intel-gfx] AVI infoframes: default aspect ratio/VIC for CEA modes
On Wed, Nov 20, 2013 at 11:45:03PM +, Gohad, Tushar wrote:
On Wed, Nov 20, 2013 at 09:48:26PM +, Gohad, Tushar wrote:
Folks,
When filling in an HDMI AVI infoframe, how does
one correctly determine the default picture aspect ratio (and VIC)
for CEA modes that support multiple (4:3 and 16:9) aspect ratios.
720x576p for example, corresponds to VIC 17 or 18:
/* 17 - 720x576@50Hz */
{ DRM_MODE(720x576, DRM_MODE_TYPE_DRIVER, 27000, 720,
732,
796, 864, 0, 576, 581, 586, 625, 0,
DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC),
.vrefresh = 50, },
/* 18 - 720x576@50Hz */
{ DRM_MODE(720x576, DRM_MODE_TYPE_DRIVER, 27000, 720,
732,
796, 864, 0, 576, 581, 586, 625, 0,
DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC),
.vrefresh = 50, },
Should the picture aspect ratio information be derived from sink
EDID (from detailed/cvt/standard timings)? .. possibly in
drm_add_edid_modes() and store the picture aspect ratio in
drm_display_mode perhaps, for later use? Trying to get a better
understanding of how this usually works.
Oh no! someone finally discovered it! Yes, we are totally missing the
picture aspect ratio information from the CEA modes. It's been on my
TODO list for a couple of month but not exactly high priority. If we
want to get a stab a it, we'll review the patches :)
Thanks Damien. Sure, I can come up with something quick. Is the idea
then to store aspect ratio information in drm_display_mode, possibly
as a separate member or as a hint that's part of mode-flags?
It seems natural to extend those flags to describe the picture aspect
ratio (that why dri-devel is in Cc., touching core DRM).
I realized that I did not actually answer the question. The CEA
modes come with their defined aspect ratio, it's part of the CEA 861
standard. In this case VIC 17 is 4:3, VIC 18 is 16:9.
I understand the VIC/aspect ratio correlation, however in order to set
the VIC correctly, don't we need to first determine the picture aspect
ratio? My question really is, how do we select the aspect ratio for
the ambiguous cases, such as 720x576 which neither truly 4:3 nor 16:9?
If we expose both to user-space (through the flags), user-space is
responsible for picking one. One detail (out of many, I'm sure) is that,
today, we accept modes without any picture aspect ratio information. We
need to keep that behaviour working.
Section 7.2.2 of CEA-861-D spec reads -
7.2.2 Order of Dual-Aspect Ratio Detailed Timing Descriptors
Source devices that do not support the AVI InfoFrame (e.g., DVI
sources) shall consider the first EDID descriptor of any dual-aspect
ratio timing to be the display-assumed aspect ratio for that timing
...
How do we figure out the display-assumed aspect ratio?
It seems that CEA-861-E rewords that sentence:
A sink not capable of receiving AVI InfoFrames shall only declare video
formats with different video timings in its EDID data structure unless
the sink declares it is capable of displaying a video timing in either
picture aspect ratio.
This relates to 4.1:
For a display device to simultaneously support both formats, the source needs
a way to let the display device know the picture aspect ratio in which the
video should be displayed. A DTV shall list only one picture aspect ratio of
any dual-aspect ratio timing unless it is capable of receiving and decoding the
AVI InfoFrame defined in Section 6.
However, it is possible for a DTV that has no support for the AVI InfoFrame to
still support both aspect ratios of such formats as a user programmable option.
In that case, the EDID Detailed Timing Descriptor could be modified during
operation to reflect the selected picture aspect ratio and the change could be
signaled to the source (e.g. with Hot Plug Detect on DVI or HDMI). The effects
on the EDID data structure are explained in Section 7.2.2. See Table 4 for
Video ID Code and Aspect Ratios.
Still not super, super clear. Worst case scenario, we could only expose the
first (preferred) mode when several modes have the same timings but different
Picture AR on DVI and call it a day.
I see that drm_add_edid_modes() enumerates detailed, CVT, standard,
established, CEA modes. Should picture aspect ratio determination be
a part of that process, based on the EDID info?
Right, so I guess you'd need to add the proper flags to the modes as
you're parsing them. May prove a bit tricky.
--
Damien
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] intel: make sure VG_CLEAR() will always do memory clear
On 2013.11.20 16:59:22 +, Damien Lespiau wrote: Right, so the fix is (was) to zero the fields checked by the kernel explicitely, not change the VG() macro, which is just used in testing (and it should this way). That's fine. Thanks. -- Open Source Technology Center, Intel ltd. $gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827 signature.asc Description: Digital signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] AVI infoframes: default aspect ratio/VIC for CEA modes
On Wed, Nov 20, 2013 at 11:45:03PM +, Gohad, Tushar wrote:
On Wed, Nov 20, 2013 at 09:48:26PM +, Gohad, Tushar wrote:
Folks,
When filling in an HDMI AVI infoframe, how does one correctly
determine the default picture aspect ratio (and VIC) for CEA
modes that support multiple (4:3 and 16:9) aspect ratios.
720x576p for example, corresponds to VIC 17 or 18:
/* 17 - 720x576@50Hz */
{ DRM_MODE(720x576, DRM_MODE_TYPE_DRIVER, 27000, 720,
732,
796, 864, 0, 576, 581, 586, 625, 0,
DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC),
.vrefresh = 50, },
/* 18 - 720x576@50Hz */
{ DRM_MODE(720x576, DRM_MODE_TYPE_DRIVER, 27000, 720,
732,
796, 864, 0, 576, 581, 586, 625, 0,
DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC),
.vrefresh = 50, },
Should the picture aspect ratio information be derived from
sink EDID (from detailed/cvt/standard timings)? .. possibly in
drm_add_edid_modes() and store the picture aspect ratio in
drm_display_mode perhaps, for later use? Trying to get a better
understanding of how this usually works.
Oh no! someone finally discovered it! Yes, we are totally missing
the picture aspect ratio information from the CEA modes. It's been
on my TODO list for a couple of month but not exactly high
priority. If we want to get a stab a it, we'll review the patches
:)
Thanks Damien. Sure, I can come up with something quick. Is the idea
then to store aspect ratio information in drm_display_mode, possibly
as a separate member or as a hint that's part of mode-flags?
It seems natural to extend those flags to describe the picture aspect ratio
(that
why dri-devel is in Cc., touching core DRM).
Cc: dri-devel
To start with we can use a single bit in drm_display_mode-flags to distinguish
16:9 vs 4:3.
I realized that I did not actually answer the question. The CEA
modes come with their defined aspect ratio, it's part of the CEA 861
standard. In this case VIC 17 is 4:3, VIC 18 is 16:9.
I understand the VIC/aspect ratio correlation, however in order to set
the VIC correctly, don't we need to first determine the picture aspect
ratio? My question really is, how do we select the aspect ratio for
the ambiguous cases, such as 720x576 which neither truly 4:3 nor 16:9?
If we expose both to user-space (through the flags), user-space is responsible
for picking one. One detail (out of many, I'm sure) is that, today, we accept
modes without any picture aspect ratio information. We need to keep that
behaviour working.
Exactly the reason for the term default aspect ratio in original question.
:) I didn't see a way to set/force an aspect ratio via KMS or other methods
(xrandr). Is this for historical reasons?
I see that drm_add_edid_modes() enumerates detailed, CVT, standard,
established, CEA modes. Should picture aspect ratio determination be
a part of that process, based on the EDID info?
Right, so I guess you'd need to add the proper flags to the modes as you're
parsing them. May prove a bit tricky.
Okay, so we seem to agree that we should set the default picture aspect ratio
as we are parsing the EDID timings info.
Comments from other folks on any pitfalls of this approach?
Section 7.2.2 of CEA-861-D spec reads -
7.2.2 Order of Dual-Aspect Ratio Detailed Timing Descriptors
Source devices that do not support the AVI InfoFrame (e.g., DVI
sources) shall consider the first EDID descriptor of any dual-aspect
ratio timing to be the display-assumed aspect ratio for that timing
...
How do we figure out the display-assumed aspect ratio?
It seems that CEA-861-E rewords that sentence:
A sink not capable of receiving AVI InfoFrames shall only declare video
formats with different video timings in its EDID data structure unless the
sink
declares it is capable of displaying a video timing in either picture aspect
ratio.
This relates to 4.1:
For a display device to simultaneously support both formats, the source
needs a way to let the display device know the picture aspect ratio in which
the video should be displayed. A DTV shall list only one picture aspect ratio
of
any dual-aspect ratio timing unless it is capable of receiving and decoding
the
AVI InfoFrame defined in Section 6.
However, it is possible for a DTV that has no support for the AVI InfoFrame to
still support both aspect ratios of such formats as a user programmable
option.
In that case, the EDID Detailed Timing Descriptor could be modified during
operation to reflect the selected picture aspect ratio and the change could be
signaled to the source (e.g. with Hot Plug Detect on DVI or HDMI). The effects
on the EDID data structure are explained in Section 7.2.2. See Table 4 for
[Intel-gfx] [PATCH v2 rebased] ACPI / video: Add systems that should favor native backlight interface
On 11/21/2013 04:56 AM, Igor Gnatenko wrote:
Any news here? If no - I think we need re-send patch as new..
Since the v2 patch can't apply cleanly on top of pm's -next tree, I
think it's worth a re-send, so here it comes.
---
Subject: [PATCH] ACPI / video: Add systems that should favor native backlight
interface
From: Aaron Lu aaron...@intel.com
Date: Thu, 21 Nov 2013 11:24:48 +0800
Some system's ACPI video backlight control interface is broken and the
native backlight control interface should be used by default. This patch
sets the use_native_backlight parameter to true for those systems so
that video backlight control interface will not be created. To be
specific, the ThinkPad T430s/X230, Lenovo Yoga 13, Dell Inspiron 7520
and Acer Aspire 5733Z are added here, if they appear in some other DMI
table before, they are removed from there.
Note that the user specified kernel cmdline option will always have the
highest priority, i.e. if use_native_backlight=0 is specified and the
system is in the DMI table, the video module will not skip registering
backlight interface for it.
Thinkpad T430s:
Reported-by: Theodore Tso ty...@mit.edu
Reported-and-tested-by: Peter Weber b...@ttyhoney.com
Reference: https://bugzilla.kernel.org/show_bug.cgi?id=51231
Thinkpad X230:
Reported-and-tested-by: Igor Gnatenko i.gnatenko.br...@gmail.com
Reference: https://bugzilla.kernel.org/show_bug.cgi?id=51231
Lenovo Yoga 13:
Reported-by: Lennart Poettering lenn...@poettering.net
Reported-and-tested-by: Kevin Smith thirdwig...@gmail.com
Reference: https://bugzilla.kernel.org/show_bug.cgi?id=63811
Dell Inspiron 7520:
Reported-by: Rinat Ibragimov ibragimovri...@mail.ru
Acer Aspire 5733Z:
Reported-by: sov.i...@mail.ru
Reference: https://bugzilla.kernel.org/show_bug.cgi?id=62941
Signed-off-by: Aaron Lu aaron...@intel.com
---
drivers/acpi/blacklist.c| 8 --
drivers/acpi/video.c| 65 +
drivers/acpi/video_detect.c | 8 --
3 files changed, 60 insertions(+), 21 deletions(-)
diff --git a/drivers/acpi/blacklist.c b/drivers/acpi/blacklist.c
index 078c4f7fe2dd..2b6a76b6d59a 100644
--- a/drivers/acpi/blacklist.c
+++ b/drivers/acpi/blacklist.c
@@ -261,14 +261,6 @@ static struct dmi_system_id acpi_osi_dmi_table[]
__initdata = {
},
{
.callback = dmi_disable_osi_win8,
- .ident = Dell Inspiron 15R SE,
- .matches = {
-DMI_MATCH(DMI_SYS_VENDOR, Dell Inc.),
-DMI_MATCH(DMI_PRODUCT_NAME, Inspiron 7520),
- },
- },
- {
- .callback = dmi_disable_osi_win8,
.ident = ThinkPad Edge E530,
.matches = {
DMI_MATCH(DMI_SYS_VENDOR, LENOVO),
diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
index 995e91bcb97b..7dc6071a04b6 100644
--- a/drivers/acpi/video.c
+++ b/drivers/acpi/video.c
@@ -82,11 +82,12 @@ static bool allow_duplicates;
module_param(allow_duplicates, bool, 0644);
/*
- * For Windows 8 systems: if set ture and the GPU driver has
- * registered a backlight interface, skip registering ACPI video's.
+ * For Windows 8 systems: used to decide if video module
+ * should skip registering backlight interface of its own.
*/
-static bool use_native_backlight = false;
-module_param(use_native_backlight, bool, 0644);
+static int use_native_backlight_param = -1;
+module_param_named(use_native_backlight, use_native_backlight_param, int,
0444);
+static bool use_native_backlight_dmi = false;
static int register_count;
static struct mutex video_list_lock;
@@ -232,9 +233,17 @@ static int acpi_video_get_next_level(struct
acpi_video_device *device,
static int acpi_video_switch_brightness(struct acpi_video_device *device,
int event);
+static bool acpi_video_use_native_backlight(void)
+{
+ if (use_native_backlight_param != -1)
+ return use_native_backlight_param;
+ else
+ return use_native_backlight_dmi;
+}
+
static bool acpi_video_verify_backlight_support(void)
{
- if (acpi_osi_is_win8() use_native_backlight
+ if (acpi_osi_is_win8() acpi_video_use_native_backlight()
backlight_device_registered(BACKLIGHT_RAW))
return false;
return acpi_video_backlight_support();
@@ -399,6 +408,12 @@ static int __init video_set_bqc_offset(const struct
dmi_system_id *d)
return 0;
}
+static int __init video_set_use_native_backlight(const struct dmi_system_id *d)
+{
+ use_native_backlight_dmi = true;
+ return 0;
+}
+
static struct dmi_system_id video_dmi_table[] __initdata = {
/*
* Broken _BQC workaround
http://bugzilla.kernel.org/show_bug.cgi?id=13121
@@ -443,6 +458,46 @@ static struct dmi_system_id video_dmi_table[] __initdata =
{
DMI_MATCH(DMI_PRODUCT_NAME, Aspire 7720),
},
},
+ {
+.callback = video_set_use_native_backlight,
+
