On Tue, Apr 04, 2017 at 05:10:29PM -0700, Kenneth Graunke wrote:
> I'm about to rewrite how relocation handling works, at which point
> drm_bacon_bo_emit_reloc() and drm_bacon_bo_mrb_exec() won't exist
> anymore. This code is already largely not using the batchbuffer
> infrastructure, so just go all the way and handle relocations, the
> validation list, and execbuffer ourselves. That way, we don't have
> to think the weird case where we only have a screen, and no context,
> when redesigning the relocation handling.
> ---
> src/mesa/drivers/dri/i965/intel_screen.c | 38
> ++++++++++++++++++++++++++------
> 1 file changed, 31 insertions(+), 7 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/intel_screen.c
> b/src/mesa/drivers/dri/i965/intel_screen.c
> index 4cecd52f36a..42baadf25e1 100644
> --- a/src/mesa/drivers/dri/i965/intel_screen.c
> +++ b/src/mesa/drivers/dri/i965/intel_screen.c
> @@ -1367,6 +1367,9 @@ static bool
> intel_detect_pipelined_register(struct intel_screen *screen,
> int reg, uint32_t expected_value, bool reset)
> {
> + if (screen->no_hw)
> + return false;
> +
> drm_bacon_bo *results, *bo;
> uint32_t *batch;
> uint32_t offset = 0;
> @@ -1394,11 +1397,14 @@ intel_detect_pipelined_register(struct intel_screen
> *screen,
> /* Save the register's value back to the buffer. */
> *batch++ = MI_STORE_REGISTER_MEM | (3 - 2);
> *batch++ = reg;
> - drm_bacon_bo_emit_reloc(bo, (char *)batch -(char *)bo->virtual,
> - results, offset*sizeof(uint32_t),
> - I915_GEM_DOMAIN_INSTRUCTION,
> - I915_GEM_DOMAIN_INSTRUCTION);
> - *batch++ = ((uint32_t) results->offset64) + offset*sizeof(uint32_t);
> + struct drm_i915_gem_relocation_entry reloc = {
> + .offset = (char *) batch - (char *) bo->virtual,
> + .delta = offset * sizeof(uint32_t),
> + .target_handle = results->handle,
> + .read_domains = I915_GEM_DOMAIN_INSTRUCTION,
> + .write_domain = I915_GEM_DOMAIN_INSTRUCTION,
> + };
> + *batch++ = offset * sizeof(uint32_t);
Better as *batch++ = reloc.presumed_offset + reloc.delta;
gcc should reduce that to the reloc.delta already in the register, and
it demonstrates the execbuf rules more clearly.
> /* And afterwards clear the register */
> if (reset) {
> @@ -1409,8 +1415,26 @@ intel_detect_pipelined_register(struct intel_screen
> *screen,
>
> *batch++ = MI_BATCH_BUFFER_END;
>
> - drm_bacon_bo_mrb_exec(bo, ALIGN((char *)batch - (char *)bo->virtual, 8),
> - I915_EXEC_RENDER);
> + struct drm_i915_gem_exec_object2 exec_objects[2] = {
> + {
> + .handle = results->handle,
> + },
> + {
> + .handle = bo->handle,
> + .relocation_count = 1,
> + .relocs_ptr = (uintptr_t) &reloc,
> + }
> + };
> +
> + struct drm_i915_gem_execbuffer2 execbuf = {
> + .buffers_ptr = (uintptr_t) exec_objects,
> + .buffer_count = 2,
> + .batch_len = ALIGN((char *) batch - (char *) bo->virtual, 8),
> + .flags = I915_EXEC_RENDER,
> + };
> +
> + __DRIscreen *dri_screen = screen->driScrnPriv;
> + drmIoctl(dri_screen->fd, DRM_IOCTL_I915_GEM_EXECBUFFER2, &execbuf);
>
> /* Check whether the value got written. */
> if (drm_bacon_bo_map(results, false) == 0) {
And all the error handling falls out in the wash by simplying disabling
access.
Reviewed-by: Chris Wilson <ch...@chris-wilson.co.uk>
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
