Setting myself up for a late night crying session once again. Most of the people reading this probably know the history and reasons for the patches. If not, you can search the intel-gfx mailing list to try to learn more. I won't recap the whole thing here, and instead let the patches speak for themselves.
Instead a brief review of what's here, and what's happened. Mostly, these badly need testing and review. I've carried these around for so long now, and seen so many different failures, I'm quite paranoid they still aren't perfect. Also, I've spent almost all of the time working on this in the kernel, so there is bound to be simple errors in the other stuff.
I've run these on various workloads and saw nothing worth mentioning.
0-16: kernel patches 17-21: libdrm patches (wait render patch is temporary) 22-24: inetl-gpu-tools 25: mesa
kernel git://people.freedesktop.org/~bwidawsk/drm-intel context_support_rev2
libdrm git://people.freedesktop.org/~bwidawsk/drm context_support_rev2
i-g-t git://people.freedesktop.org/~bwidawsk/intel-gpu-tools context_support
The patches have already been sucked in to the kernel. So we need a placeholder for this IOCTL until the libdrm wait render patches land. --- include/drm/i915_drm.h | 1 + 1 file changed, 1 insertion(+)
diff --git a/include/drm/i915_drm.h b/include/drm/i915_drm.h index af3ce17..6d9a9f1 100644 --- a/include/drm/i915_drm.h +++ b/include/drm/i915_drm.h @@ -191,6 +191,7 @@ typedef struct _drm_i915_sarea { #define DRM_I915_GEM_EXECBUFFER2 0x29 #define DRM_I915_GET_SPRITE_COLORKEY 0x2a #define DRM_I915_SET_SPRITE_COLORKEY 0x2b +#define DRM_I915_GEM_WAIT 0x2c
#define DRM_IOCTL_I915_INIT DRM_IOW( DRM_COMMAND_BASE + DRM_I915_INIT, drm_i915_init_t) #define DRM_IOCTL_I915_FLUSH DRM_IO ( DRM_COMMAND_BASE + DRM_I915_FLUSH)
Signed-off-by: Ben Widawsky ben@bwidawsk.net --- include/drm/i915_drm.h | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-)
diff --git a/include/drm/i915_drm.h b/include/drm/i915_drm.h index 6d9a9f1..0ca2d4f 100644 --- a/include/drm/i915_drm.h +++ b/include/drm/i915_drm.h @@ -192,6 +192,8 @@ typedef struct _drm_i915_sarea { #define DRM_I915_GET_SPRITE_COLORKEY 0x2a #define DRM_I915_SET_SPRITE_COLORKEY 0x2b #define DRM_I915_GEM_WAIT 0x2c +#define DRM_I915_GEM_CONTEXT_CREATE 0x2d +#define DRM_I915_GEM_CONTEXT_DESTROY 0x2e
#define DRM_IOCTL_I915_INIT DRM_IOW( DRM_COMMAND_BASE + DRM_I915_INIT, drm_i915_init_t) #define DRM_IOCTL_I915_FLUSH DRM_IO ( DRM_COMMAND_BASE + DRM_I915_FLUSH) @@ -236,6 +238,9 @@ typedef struct _drm_i915_sarea { #define DRM_IOCTL_I915_SET_SPRITE_COLORKEY DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_SET_SPRITE_COLORKEY, struct drm_intel_sprite_colorkey) #define DRM_IOCTL_I915_GET_SPRITE_COLORKEY DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_SET_SPRITE_COLORKEY, struct drm_intel_sprite_colorkey)
+#define DRM_IOCTL_I915_GEM_CONTEXT_CREATE DRM_IOWR (DRM_COMMAND_BASE + DRM_I915_GEM_CONTEXT_CREATE, struct drm_i915_gem_context_create) +#define DRM_IOCTL_I915_GEM_CONTEXT_DESTROY DRM_IOW (DRM_COMMAND_BASE + DRM_I915_GEM_CONTEXT_DESTROY, struct drm_i915_gem_context_destroy) + /* Allow drivers to submit batchbuffers directly to hardware, relying * on the security mechanisms provided by hardware. */ @@ -647,13 +652,19 @@ struct drm_i915_gem_execbuffer2 { #define I915_EXEC_CONSTANTS_ABSOLUTE (1<<6) #define I915_EXEC_CONSTANTS_REL_SURFACE (2<<6) /* gen4/5 only */ __u64 flags; - __u64 rsvd1; + __u64 rsvd1; /* now used for context info */ __u64 rsvd2; };
/** Resets the SO write offset registers for transform feedback on gen7. */ #define I915_EXEC_GEN7_SOL_RESET (1<<8)
+#define I915_EXEC_CONTEXT_ID_MASK (0xffffffff) +#define i915_execbuffer2_set_context_id(eb2, context) \ + (eb2).rsvd1 = context & I915_EXEC_CONTEXT_ID_MASK +#define i915_execbuffer2_get_context_id(eb2) \ + ((eb2).rsvd1 & I915_EXEC_CONTEXT_ID_MASK) + struct drm_i915_gem_pin { /** Handle of the buffer to be pinned. */ __u32 handle; @@ -877,4 +888,15 @@ struct drm_intel_sprite_colorkey { __u32 flags; };
+struct drm_i915_gem_context_create { + /* output: id of new context*/ + __u32 ctx_id; + __u32 pad; +}; + +struct drm_i915_gem_context_destroy { + __u32 ctx_id; + __u32 pad; +}; + #endif /* _I915_DRM_H_ */
Add an opaque type representing a HW context.
Signed-off-by: Ben Widawsky ben@bwidawsk.net --- intel/intel_bufmgr.h | 1 + intel/intel_bufmgr_priv.h | 5 +++++ 2 files changed, 6 insertions(+)
diff --git a/intel/intel_bufmgr.h b/intel/intel_bufmgr.h index c197abc..83a43cb 100644 --- a/intel/intel_bufmgr.h +++ b/intel/intel_bufmgr.h @@ -41,6 +41,7 @@ struct drm_clip_rect;
typedef struct _drm_intel_bufmgr drm_intel_bufmgr; +typedef struct _drm_intel_context drm_intel_context; typedef struct _drm_intel_bo drm_intel_bo;
struct _drm_intel_bo { diff --git a/intel/intel_bufmgr_priv.h b/intel/intel_bufmgr_priv.h index 0b62520..2592d42 100644 --- a/intel/intel_bufmgr_priv.h +++ b/intel/intel_bufmgr_priv.h @@ -280,6 +280,11 @@ struct _drm_intel_bufmgr { int debug; };
+struct _drm_intel_context { + unsigned int ctx_id; + struct _drm_intel_bufmgr *bufmgr; +}; + #define ALIGN(value, alignment) ((value + alignment - 1) & ~(alignment - 1)) #define ROUND_UP_TO(x, y) (((x) + (y) - 1) / (y) * (y)) #define ROUND_UP_TO_MB(x) ROUND_UP_TO((x), 1024*1024)
To support this we extract the common execbuf2 functionality to be called with, or without contexts.
The context'd execbuf does not support some of the dri1 stuff.
Signed-off-by: Ben Widawsky ben@bwidawsk.net --- intel/intel_bufmgr.h | 5 +++++ intel/intel_bufmgr_gem.c | 32 +++++++++++++++++++++++++------- 2 files changed, 30 insertions(+), 7 deletions(-)
diff --git a/intel/intel_bufmgr.h b/intel/intel_bufmgr.h index 83a43cb..20338c7 100644 --- a/intel/intel_bufmgr.h +++ b/intel/intel_bufmgr.h @@ -186,6 +186,11 @@ int drm_intel_get_pipe_from_crtc_id(drm_intel_bufmgr *bufmgr, int crtc_id); int drm_intel_get_aperture_sizes(int fd, size_t *mappable, size_t *total); int drm_intel_bufmgr_gem_get_devid(drm_intel_bufmgr *bufmgr);
+drm_intel_context *drm_intel_gem_context_create(drm_intel_bufmgr *bufmgr); +void drm_intel_gem_context_destroy(drm_intel_context *ctx); +int drm_intel_gem_bo_context_exec(drm_intel_bo *bo, drm_intel_context *ctx, + int used, unsigned int flags); + /* drm_intel_bufmgr_fake.c */ drm_intel_bufmgr *drm_intel_bufmgr_fake_init(int fd, unsigned long low_offset, diff --git a/intel/intel_bufmgr_gem.c b/intel/intel_bufmgr_gem.c index b776d2f..e74ac0a 100644 --- a/intel/intel_bufmgr_gem.c +++ b/intel/intel_bufmgr_gem.c @@ -2134,9 +2134,9 @@ drm_intel_gem_bo_exec(drm_intel_bo *bo, int used, }
static int -drm_intel_gem_bo_mrb_exec2(drm_intel_bo *bo, int used, - drm_clip_rect_t *cliprects, int num_cliprects, int DR4, - unsigned int flags) +do_exec2(drm_intel_bo *bo, int used, drm_intel_context *ctx, + drm_clip_rect_t *cliprects, int num_cliprects, int DR4, + unsigned int flags) { drm_intel_bufmgr_gem *bufmgr_gem = (drm_intel_bufmgr_gem *)bo->bufmgr; struct drm_i915_gem_execbuffer2 execbuf; @@ -2178,7 +2178,10 @@ drm_intel_gem_bo_mrb_exec2(drm_intel_bo *bo, int used, execbuf.DR1 = 0; execbuf.DR4 = DR4; execbuf.flags = flags; - execbuf.rsvd1 = 0; + if (ctx == NULL) + i915_execbuffer2_set_context_id(execbuf, 0); + else + i915_execbuffer2_set_context_id(execbuf, ctx->ctx_id); execbuf.rsvd2 = 0;
aub_exec(bo, flags, used); @@ -2226,9 +2229,24 @@ drm_intel_gem_bo_exec2(drm_intel_bo *bo, int used, drm_clip_rect_t *cliprects, int num_cliprects, int DR4) { - return drm_intel_gem_bo_mrb_exec2(bo, used, - cliprects, num_cliprects, DR4, - I915_EXEC_RENDER); + return do_exec2(bo, used, NULL, cliprects, num_cliprects, DR4, + I915_EXEC_RENDER); +} + +static int +drm_intel_gem_bo_mrb_exec2(drm_intel_bo *bo, int used, + drm_clip_rect_t *cliprects, int num_cliprects, int DR4, + unsigned int flags) +{ + return do_exec2(bo, used, NULL, cliprects, num_cliprects, DR4, + flags); +} + +int +drm_intel_gem_bo_context_exec(drm_intel_bo *bo, drm_intel_context *ctx, + int used, unsigned int flags) +{ + return do_exec2(bo, used, ctx, NULL, 0, 0, flags); }
static int
Signed-off-by: Ben Widawsky ben@bwidawsk.net --- intel/intel_decode.c | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-)
diff --git a/intel/intel_decode.c b/intel/intel_decode.c index bf23706..5f90a47 100644 --- a/intel/intel_decode.c +++ b/intel/intel_decode.c @@ -139,6 +139,22 @@ instr_out(struct drm_intel_decode *ctx, unsigned int index, }
static int +decode_MI_SET_CONTEXT(struct drm_intel_decode *ctx) +{ + uint32_t data = ctx->data[1]; + if (ctx->gen > 7) + return 1; + + instr_out(ctx, 0, "MI_SET_CONTEXT\n"); + instr_out(ctx, 1, "gtt offset = 0x%x%s%s\n", + data & ~0xfff, + data & (1<<1)? ", Force Restore": "", + data & (1<<0)? ", Restore Inhibit": ""); + + return 2; +} + +static int decode_MI_WAIT_FOR_EVENT(struct drm_intel_decode *ctx) { const char *cc_wait; @@ -233,7 +249,7 @@ decode_mi(struct drm_intel_decode *ctx) { 0x00, 0, 1, 1, "MI_NOOP" }, { 0x11, 0x3f, 2, 2, "MI_OVERLAY_FLIP" }, { 0x07, 0, 1, 1, "MI_REPORT_HEAD" }, - { 0x18, 0x3f, 2, 2, "MI_SET_CONTEXT" }, + { 0x18, 0x3f, 2, 2, "MI_SET_CONTEXT", decode_MI_SET_CONTEXT }, { 0x20, 0x3f, 3, 4, "MI_STORE_DATA_IMM" }, { 0x21, 0x3f, 3, 4, "MI_STORE_DATA_INDEX" }, { 0x24, 0x3f, 3, 3, "MI_STORE_REGISTER_MEM" },
Based off of a patch from Ken Graunke. I just modified it for a more modern mesa (also don't allow contexts on blit ring).
Signed-off-by: Ben Widawsky ben@bwidawsk.net --- src/mesa/drivers/dri/i965/brw_context.c | 1 + src/mesa/drivers/dri/i965/brw_vtbl.c | 5 ++++- src/mesa/drivers/dri/intel/intel_batchbuffer.c | 9 +++++++-- src/mesa/drivers/dri/intel/intel_context.c | 2 ++ src/mesa/drivers/dri/intel/intel_context.h | 2 +- 5 files changed, 15 insertions(+), 4 deletions(-)
diff --git a/src/mesa/drivers/dri/i965/brw_context.c b/src/mesa/drivers/dri/i965/brw_context.c index f7073cd..d4159c7 100644 --- a/src/mesa/drivers/dri/i965/brw_context.c +++ b/src/mesa/drivers/dri/i965/brw_context.c @@ -298,6 +298,7 @@ brwCreateContext(int api,
brw->prim_restart.in_progress = false; brw->prim_restart.enable_cut_index = false; + intel->hw_ctx = drm_intel_gem_context_create(intel->bufmgr);
brw_init_state( brw );
diff --git a/src/mesa/drivers/dri/i965/brw_vtbl.c b/src/mesa/drivers/dri/i965/brw_vtbl.c index 5699749..d9fd2cb 100644 --- a/src/mesa/drivers/dri/i965/brw_vtbl.c +++ b/src/mesa/drivers/dri/i965/brw_vtbl.c @@ -170,7 +170,10 @@ static void brw_new_batch( struct intel_context *intel ) * This is probably not as severe as on 915, since almost all of our state * is just in referenced buffers. */ - brw->state.dirty.brw |= BRW_NEW_CONTEXT | BRW_NEW_BATCH; + if (intel->hw_ctx == NULL) + brw->state.dirty.brw |= BRW_NEW_CONTEXT; + + brw->state.dirty.brw |= BRW_NEW_BATCH;
/* Assume that the last command before the start of our batch was a * primitive, for safety. diff --git a/src/mesa/drivers/dri/intel/intel_batchbuffer.c b/src/mesa/drivers/dri/intel/intel_batchbuffer.c index 76a69f7..7ba141d 100644 --- a/src/mesa/drivers/dri/intel/intel_batchbuffer.c +++ b/src/mesa/drivers/dri/intel/intel_batchbuffer.c @@ -188,8 +188,13 @@ do_flush_locked(struct intel_context *intel) if (ret == 0) { if (unlikely(INTEL_DEBUG & DEBUG_AUB) && intel->vtbl.annotate_aub) intel->vtbl.annotate_aub(intel); - ret = drm_intel_bo_mrb_exec(batch->bo, 4*batch->used, NULL, 0, 0, - flags); + if (intel->hw_ctx == NULL || batch->is_blit) { + ret = drm_intel_bo_mrb_exec(batch->bo, 4*batch->used, NULL, 0, 0, + flags); + } else { + ret = drm_intel_gem_bo_context_exec(batch->bo, intel->hw_ctx, + 4 * batch->used, flags); + } } }
diff --git a/src/mesa/drivers/dri/intel/intel_context.c b/src/mesa/drivers/dri/intel/intel_context.c index 9deb4ca..46c2492 100644 --- a/src/mesa/drivers/dri/intel/intel_context.c +++ b/src/mesa/drivers/dri/intel/intel_context.c @@ -593,6 +593,8 @@ intelInitContext(struct intel_context *intel, if (intelScreen->bufmgr == NULL) return false;
+ intel->hw_ctx = NULL; + /* Can't rely on invalidate events, fall back to glViewport hack */ if (!driContextPriv->driScreenPriv->dri2.useInvalidate) { intel->saved_viewport = functions->Viewport; diff --git a/src/mesa/drivers/dri/intel/intel_context.h b/src/mesa/drivers/dri/intel/intel_context.h index cc3ee0d..c026fea 100644 --- a/src/mesa/drivers/dri/intel/intel_context.h +++ b/src/mesa/drivers/dri/intel/intel_context.h @@ -226,7 +226,7 @@ struct intel_context int urb_size;
struct intel_batchbuffer batch; - + drm_intel_context *hw_ctx; drm_intel_bo *first_post_swapbuffers_batch; bool need_throttle; bool no_batch_wrap;
On 4 June 2012 14:43, Ben Widawsky ben@bwidawsk.net wrote:
Based off of a patch from Ken Graunke. I just modified it for a more modern mesa (also don't allow contexts on blit ring).
Signed-off-by: Ben Widawsky ben@bwidawsk.net
src/mesa/drivers/dri/i965/brw_context.c | 1 + src/mesa/drivers/dri/i965/brw_vtbl.c | 5 ++++- src/mesa/drivers/dri/intel/intel_batchbuffer.c | 9 +++++++-- src/mesa/drivers/dri/intel/intel_context.c | 2 ++ src/mesa/drivers/dri/intel/intel_context.h | 2 +- 5 files changed, 15 insertions(+), 4 deletions(-)
diff --git a/src/mesa/drivers/dri/i965/brw_context.c b/src/mesa/drivers/dri/i965/brw_context.c index f7073cd..d4159c7 100644 --- a/src/mesa/drivers/dri/i965/brw_context.c +++ b/src/mesa/drivers/dri/i965/brw_context.c @@ -298,6 +298,7 @@ brwCreateContext(int api,
brw->prim_restart.in_progress = false; brw->prim_restart.enable_cut_index = false;
intel->hw_ctx = drm_intel_gem_context_create(intel->bufmgr);
brw_init_state( brw );
diff --git a/src/mesa/drivers/dri/i965/brw_vtbl.c b/src/mesa/drivers/dri/i965/brw_vtbl.c index 5699749..d9fd2cb 100644 --- a/src/mesa/drivers/dri/i965/brw_vtbl.c +++ b/src/mesa/drivers/dri/i965/brw_vtbl.c @@ -170,7 +170,10 @@ static void brw_new_batch( struct intel_context *intel ) * This is probably not as severe as on 915, since almost all of our state * is just in referenced buffers. */
- brw->state.dirty.brw |= BRW_NEW_CONTEXT | BRW_NEW_BATCH;
- if (intel->hw_ctx == NULL)
brw->state.dirty.brw |= BRW_NEW_CONTEXT;- brw->state.dirty.brw |= BRW_NEW_BATCH;
The comment above this change ("Mark all context state as needing to be re-emitted.") is no longer accurate. Perhaps change it to something like this?
"If the kernel supports hardware contexts, then most hardware state is preserved between batches; we only need to re-emit state that is required to be in every batch. Otherwise we need to re-emit all the state that would otherwise be stored in the context (which for all intents and purposes means everything)."
Also, I think it would be ok to delete the comment "This is probably not as severe as on 915 ... referenced buffers"--that comment is mostly just a rationalization for not having implemented hardware context support earlier, and not a very convincing one at that :)
/* Assume that the last command before the start of our batch was a * primitive, for safety. diff --git a/src/mesa/drivers/dri/intel/intel_batchbuffer.c b/src/mesa/drivers/dri/intel/intel_batchbuffer.c index 76a69f7..7ba141d 100644 --- a/src/mesa/drivers/dri/intel/intel_batchbuffer.c +++ b/src/mesa/drivers/dri/intel/intel_batchbuffer.c @@ -188,8 +188,13 @@ do_flush_locked(struct intel_context *intel) if (ret == 0) { if (unlikely(INTEL_DEBUG & DEBUG_AUB) && intel->vtbl.annotate_aub) intel->vtbl.annotate_aub(intel);
ret = drm_intel_bo_mrb_exec(batch->bo, 4*batch->used, NULL, 0, 0,flags);
if (intel->hw_ctx == NULL || batch->is_blit) {ret = drm_intel_bo_mrb_exec(batch->bo, 4*batch->used, NULL, 0,0,
flags);} else {ret = drm_intel_gem_bo_context_exec(batch->bo, intel->hw_ctx,4 * batch->used, flags);} }diff --git a/src/mesa/drivers/dri/intel/intel_context.c b/src/mesa/drivers/dri/intel/intel_context.c index 9deb4ca..46c2492 100644 --- a/src/mesa/drivers/dri/intel/intel_context.c +++ b/src/mesa/drivers/dri/intel/intel_context.c @@ -593,6 +593,8 @@ intelInitContext(struct intel_context *intel, if (intelScreen->bufmgr == NULL) return false;
- intel->hw_ctx = NULL;
- /* Can't rely on invalidate events, fall back to glViewport hack */ if (!driContextPriv->driScreenPriv->dri2.useInvalidate) { intel->saved_viewport = functions->Viewport;
diff --git a/src/mesa/drivers/dri/intel/intel_context.h b/src/mesa/drivers/dri/intel/intel_context.h index cc3ee0d..c026fea 100644 --- a/src/mesa/drivers/dri/intel/intel_context.h +++ b/src/mesa/drivers/dri/intel/intel_context.h @@ -226,7 +226,7 @@ struct intel_context int urb_size;
struct intel_batchbuffer batch;
- drm_intel_context *hw_ctx; drm_intel_bo *first_post_swapbuffers_batch; bool need_throttle; bool no_batch_wrap;
-- 1.7.10.2
Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
I'm kind of surprised to see a call to drm_intel_gem_context_create(), but no call anywhere to a clean-up function that destroys the context. Was that an oversight, or is there a reason why it's unnecessary? If it's the latter, a comment in brw_destroy_context() would be helpful.
On Mon, 4 Jun 2012 16:01:54 -0700 Paul Berry stereotype441@gmail.com wrote:
On 4 June 2012 14:43, Ben Widawsky ben@bwidawsk.net wrote:
Based off of a patch from Ken Graunke. I just modified it for a more modern mesa (also don't allow contexts on blit ring).
Signed-off-by: Ben Widawsky ben@bwidawsk.net
src/mesa/drivers/dri/i965/brw_context.c | 1 + src/mesa/drivers/dri/i965/brw_vtbl.c | 5 ++++- src/mesa/drivers/dri/intel/intel_batchbuffer.c | 9 +++++++-- src/mesa/drivers/dri/intel/intel_context.c | 2 ++ src/mesa/drivers/dri/intel/intel_context.h | 2 +- 5 files changed, 15 insertions(+), 4 deletions(-)
diff --git a/src/mesa/drivers/dri/i965/brw_context.c b/src/mesa/drivers/dri/i965/brw_context.c index f7073cd..d4159c7 100644 --- a/src/mesa/drivers/dri/i965/brw_context.c +++ b/src/mesa/drivers/dri/i965/brw_context.c @@ -298,6 +298,7 @@ brwCreateContext(int api,
brw->prim_restart.in_progress = false; brw->prim_restart.enable_cut_index = false;
intel->hw_ctx = drm_intel_gem_context_create(intel->bufmgr);
brw_init_state( brw );
diff --git a/src/mesa/drivers/dri/i965/brw_vtbl.c b/src/mesa/drivers/dri/i965/brw_vtbl.c index 5699749..d9fd2cb 100644 --- a/src/mesa/drivers/dri/i965/brw_vtbl.c +++ b/src/mesa/drivers/dri/i965/brw_vtbl.c @@ -170,7 +170,10 @@ static void brw_new_batch( struct intel_context *intel ) * This is probably not as severe as on 915, since almost all of our state * is just in referenced buffers. */
- brw->state.dirty.brw |= BRW_NEW_CONTEXT | BRW_NEW_BATCH;
- if (intel->hw_ctx == NULL)
brw->state.dirty.brw |= BRW_NEW_CONTEXT;- brw->state.dirty.brw |= BRW_NEW_BATCH;
The comment above this change ("Mark all context state as needing to be re-emitted.") is no longer accurate. Perhaps change it to something like this?
"If the kernel supports hardware contexts, then most hardware state is preserved between batches; we only need to re-emit state that is required to be in every batch. Otherwise we need to re-emit all the state that would otherwise be stored in the context (which for all intents and purposes means everything)."
Also, I think it would be ok to delete the comment "This is probably not as severe as on 915 ... referenced buffers"--that comment is mostly just a rationalization for not having implemented hardware context support earlier, and not a very convincing one at that :)
/* Assume that the last command before the start of our batch was a * primitive, for safety. diff --git a/src/mesa/drivers/dri/intel/intel_batchbuffer.c b/src/mesa/drivers/dri/intel/intel_batchbuffer.c index 76a69f7..7ba141d 100644 --- a/src/mesa/drivers/dri/intel/intel_batchbuffer.c +++ b/src/mesa/drivers/dri/intel/intel_batchbuffer.c @@ -188,8 +188,13 @@ do_flush_locked(struct intel_context *intel) if (ret == 0) { if (unlikely(INTEL_DEBUG & DEBUG_AUB) && intel->vtbl.annotate_aub) intel->vtbl.annotate_aub(intel);
ret = drm_intel_bo_mrb_exec(batch->bo, 4*batch->used, NULL, 0, 0,flags);
if (intel->hw_ctx == NULL || batch->is_blit) {ret = drm_intel_bo_mrb_exec(batch->bo, 4*batch->used, NULL, 0,0,
flags);} else {ret = drm_intel_gem_bo_context_exec(batch->bo, intel->hw_ctx,4 * batch->used, flags);} }diff --git a/src/mesa/drivers/dri/intel/intel_context.c b/src/mesa/drivers/dri/intel/intel_context.c index 9deb4ca..46c2492 100644 --- a/src/mesa/drivers/dri/intel/intel_context.c +++ b/src/mesa/drivers/dri/intel/intel_context.c @@ -593,6 +593,8 @@ intelInitContext(struct intel_context *intel, if (intelScreen->bufmgr == NULL) return false;
- intel->hw_ctx = NULL;
- /* Can't rely on invalidate events, fall back to glViewport hack */ if (!driContextPriv->driScreenPriv->dri2.useInvalidate) { intel->saved_viewport = functions->Viewport;
diff --git a/src/mesa/drivers/dri/intel/intel_context.h b/src/mesa/drivers/dri/intel/intel_context.h index cc3ee0d..c026fea 100644 --- a/src/mesa/drivers/dri/intel/intel_context.h +++ b/src/mesa/drivers/dri/intel/intel_context.h @@ -226,7 +226,7 @@ struct intel_context int urb_size;
struct intel_batchbuffer batch;
- drm_intel_context *hw_ctx; drm_intel_bo *first_post_swapbuffers_batch; bool need_throttle; bool no_batch_wrap;
-- 1.7.10.2
Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
I'm kind of surprised to see a call to drm_intel_gem_context_create(), but no call anywhere to a clean-up function that destroys the context. Was that an oversight, or is there a reason why it's unnecessary? If it's the latter, a comment in brw_destroy_context() would be helpful.
Destroying a context is not required, though it is desirable. The context will be destroyed when the DRM file handle is closed. I think I had that info in a comment somewhere, but I forget exactly where it is. I'd propose mesa people find a good place for destroy as the best solution :-)
On 06/04/2012 10:53 PM, Dave Airlie wrote:
I've run these on various workloads and saw nothing worth mentioning.
Nothing at all? no speedups, slowdowns, etc
why should we merge all this code then :-)
Dave.
Preserving hardware state across batches is going to be necessary for:
* Transform feedback in the presence of geometry shaders
Right now, Mesa counts the number of primitives emitted on the CPU, relies on the kernel resetting the register to 0, and offset values in software. With GS, we obviously can't count primitives on the CPU.
* Primitive restart in hardware
and seems to be increasingly necessary for new features as we move forward. So we'd like to get it in place; we can cut more state uploads and tune Mesa further once it's there.
On Tue, 05 Jun 2012 00:08:10 -0700 Kenneth Graunke kenneth@whitecape.org wrote:
On 06/04/2012 10:53 PM, Dave Airlie wrote:
I've run these on various workloads and saw nothing worth mentioning.
Nothing at all? no speedups, slowdowns, etc
why should we merge all this code then :-)
Dave.
Preserving hardware state across batches is going to be necessary for:
Transform feedback in the presence of geometry shaders
Right now, Mesa counts the number of primitives emitted on the CPU, relies on the kernel resetting the register to 0, and offset values in software. With GS, we obviously can't count primitives on the CPU.
Primitive restart in hardware
and seems to be increasingly necessary for new features as we move forward. So we'd like to get it in place; we can cut more state uploads and tune Mesa further once it's there.
Also, my testing was far from comprehensive.
On 06/04/2012 02:42 PM, Ben Widawsky wrote:
Setting myself up for a late night crying session once again. Most of the people reading this probably know the history and reasons for the patches. If not, you can search the intel-gfx mailing list to try to learn more. I won't recap the whole thing here, and instead let the patches speak for themselves.
Instead a brief review of what's here, and what's happened. Mostly, these badly need testing and review. I've carried these around for so long now, and seen so many different failures, I'm quite paranoid they still aren't perfect. Also, I've spent almost all of the time working on this in the kernel, so there is bound to be simple errors in the other stuff.
I've run these on various workloads and saw nothing worth mentioning.
0-16: kernel patches
17-21: libdrm patches (wait render patch is temporary)
Patches 17-21 look fine. Reviewed-by: Kenneth Graunke kenneth@whitecape.org
22-24: intel-gpu-tools 25: mesa
Patch 25 looks fine too. Feel free to apply some combination of: Reviewed-by: Kenneth Graunke kenneth@whitecape.org Signed-off-by: Kenneth Graunke kenneth@whitecape.org
I do like Paul's comment updates, so I suggest merging those.
kernel git://people.freedesktop.org/~bwidawsk/drm-intel context_support_rev2
libdrm git://people.freedesktop.org/~bwidawsk/drm context_support_rev2
i-g-t git://people.freedesktop.org/~bwidawsk/intel-gpu-tools context_support
On Tue, 05 Jun 2012 16:37:20 -0700 Kenneth Graunke kenneth@whitecape.org wrote:
On 06/04/2012 02:42 PM, Ben Widawsky wrote:
Setting myself up for a late night crying session once again. Most of the people reading this probably know the history and reasons for the patches. If not, you can search the intel-gfx mailing list to try to learn more. I won't recap the whole thing here, and instead let the patches speak for themselves.
Instead a brief review of what's here, and what's happened. Mostly, these badly need testing and review. I've carried these around for so long now, and seen so many different failures, I'm quite paranoid they still aren't perfect. Also, I've spent almost all of the time working on this in the kernel, so there is bound to be simple errors in the other stuff.
I've run these on various workloads and saw nothing worth mentioning.
0-16: kernel patches
17-21: libdrm patches (wait render patch is temporary)
Patches 17-21 look fine. Reviewed-by: Kenneth Graunke kenneth@whitecape.org
22-24: intel-gpu-tools 25: mesa
Patch 25 looks fine too. Feel free to apply some combination of: Reviewed-by: Kenneth Graunke kenneth@whitecape.org Signed-off-by: Kenneth Graunke kenneth@whitecape.org
I do like Paul's comment updates, so I suggest merging those.
Ken, would you do these for me on patch 25? It's really your patch anyhow, I just made ever so slight modifications :-)
kernel git://people.freedesktop.org/~bwidawsk/drm-intel context_support_rev2
libdrm git://people.freedesktop.org/~bwidawsk/drm context_support_rev2
i-g-t git://people.freedesktop.org/~bwidawsk/intel-gpu-tools context_support
On Mon, Jun 04, 2012 at 02:42:40PM -0700, Ben Widawsky wrote:
Setting myself up for a late night crying session once again. Most of the people reading this probably know the history and reasons for the patches. If not, you can search the intel-gfx mailing list to try to learn more. I won't recap the whole thing here, and instead let the patches speak for themselves.
Instead a brief review of what's here, and what's happened. Mostly, these badly need testing and review. I've carried these around for so long now, and seen so many different failures, I'm quite paranoid they still aren't perfect. Also, I've spent almost all of the time working on this in the kernel, so there is bound to be simple errors in the other stuff.
I've run these on various workloads and saw nothing worth mentioning.
Ok, my little attempt at in-situ bikeshedding failed, so I've just slurped in your patches directly and only applied the minimal change to get rid of object->context_id.
Please commit the i-g-t testcase so that qa can start testing this aspa.
Cheers, Daniel
dri-devel@lists.freedesktop.org
-
Ben Widawsky -
Daniel Vetter -
Dave Airlie -
Kenneth Graunke -
Paul Berry