On Wed, Sep 09, 2015 at 04:07:10PM +0200, Michał Winiarski wrote:
> Softpin allows userspace to take greater control of GPU virtual address
> space and eliminates the need of relocations. It can also be used to
> mirror addresses between GPU and CPU (shared virtual memory).
> Calls to drm_intel_bo_emit_reloc are still required to build the list of
> drm_i915_gem_exec_objects at exec time, but no entries in relocs are
> created. Self-relocs don't make any sense for softpinned objects and can
> indicate a programming errors, thus are forbidden. Softpinned objects
> are marked by asterisk in debug dumps.
This patch gets the kernel interface wrong.
> static int
> +drm_intel_gem_bo_add_softpin_target(drm_intel_bo *bo, drm_intel_bo *target_bo)
> +{
> + drm_intel_bufmgr_gem *bufmgr_gem = (drm_intel_bufmgr_gem *) bo->bufmgr;
> + drm_intel_bo_gem *bo_gem = (drm_intel_bo_gem *) bo;
> + drm_intel_bo_gem *target_bo_gem = (drm_intel_bo_gem *) target_bo;
> + if (bo_gem->has_error)
> + return -ENOMEM;
> +
> + if (target_bo_gem->has_error) {
> + bo_gem->has_error = true;
> + return -ENOMEM;
> + }
> +
> + if (!target_bo_gem->is_softpin)
> + return -EINVAL;
> + if (target_bo_gem == bo_gem)
> + return -EINVAL;
> +
> + if (bo_gem->softpin_target_count == bo_gem->softpin_target_size) {
> + int new_size = bo_gem->softpin_target_size * 2;
> + if (new_size == 0)
> + new_size = bufmgr_gem->max_relocs;
> +
> + bo_gem->softpin_target = realloc(bo_gem->softpin_target, new_size *
> + sizeof(drm_intel_bo *));
> + if (!bo_gem->softpin_target)
> + return -ENOMEM;
> +
> + bo_gem->softpin_target_size = new_size;
> + }
> + bo_gem->softpin_target[bo_gem->softpin_target_count] = target_bo;
> + drm_intel_gem_bo_reference(target_bo);
> + bo_gem->softpin_target_count++;
> +
> + return 0;
> +}
Short-circuiting the reloc handling like this is broken without
instructing the kernel via the alternative mechanisms about the special
handling the buffer requires EXEC_OBJECT_WRITE, EXEC_OBJECT_NEEDS_GTT).
> void
> drm_intel_gem_bo_clear_relocs(drm_intel_bo *bo, int start)
> @@ -1998,6 +2084,12 @@ drm_intel_gem_bo_clear_relocs(drm_intel_bo *bo, int start)
> }
> bo_gem->reloc_count = start;
>
> + for (i = 0; i < bo_gem->softpin_target_count; i++) {
> + drm_intel_bo_gem *target_bo_gem = (drm_intel_bo_gem *) bo_gem->softpin_target[i];
> + drm_intel_gem_bo_unreference_locked_timed(&target_bo_gem->bo, time.tv_sec);
> + }
> + bo_gem->softpin_target_count = 0;
This would have better pursued using a batch manager.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel
