[Mesa-dev] [RFC libdrm] intel: Add support for softpin

[Chris Wilson]

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