Re: [PATCH] intel: add prime interface for getting/setting a prime bo. (v3)

[Chris Wilson <chris@chris-wilson.co.uk>]

On Mon, 16 Jul 2012 02:51:17 +0100, Dave Airlie <airlied@gmail.com> wrote:
> This adds interfaces for the X driver to use to create a
> prime handle from a buffer, and create a bo from a handle.
> 
> v2: use Chris's suggested naming (well from at least for consistency)
> v3: git commit --amend fail
> 
> Signed-off-by: Dave Airlie <airlied@redhat.com>
> ---
>  intel/intel_bufmgr.h     |    4 ++++
>  intel/intel_bufmgr_gem.c |   46 ++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 50 insertions(+)
> 
> diff --git a/intel/intel_bufmgr.h b/intel/intel_bufmgr.h
> index 9b3a483..2167e43 100644
> --- a/intel/intel_bufmgr.h
> +++ b/intel/intel_bufmgr.h
> @@ -192,6 +192,10 @@ 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);
>  
> +int drm_intel_bo_gem_export_to_prime(drm_intel_bo *bo, int *prime_fd);
> +drm_intel_bo *drm_intel_bo_gem_create_from_prime(drm_intel_bufmgr *bufmgr,
> +						int prime_fd, int size);
> +
>  /* 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 12a3197..7966924 100644
> --- a/intel/intel_bufmgr_gem.c
> +++ b/intel/intel_bufmgr_gem.c
> @@ -2413,6 +2413,52 @@ drm_intel_gem_bo_get_tiling(drm_intel_bo *bo, uint32_t * tiling_mode,
>  	return 0;
>  }
>  
> +drm_intel_bo *
> +drm_intel_bo_gem_create_from_prime(drm_intel_bufmgr *bufmgr, int prime_fd, int size)
> +{
> +	drm_intel_bufmgr_gem *bufmgr_gem = (drm_intel_bufmgr_gem *) bufmgr;
> +	int ret;
> +	uint32_t handle;
> +	drm_intel_bo_gem *bo_gem;
> +	ret = drmPrimeFDToHandle(bufmgr_gem->fd, prime_fd, &handle);
> +	if (ret) {
> +	  fprintf(stderr,"ret is %d %d\n", ret, errno);
> +		return NULL;
> +	}
> +
> +	bo_gem = calloc(1, sizeof(*bo_gem));
> +	if (!bo_gem)
> +		return NULL;
> +
> +	bo_gem->bo.size = size;
> +	bo_gem->name = "prime";
> +	atomic_set(&bo_gem->refcount, 1);
> +	bo_gem->validate_index = -1;
> +	bo_gem->reloc_tree_fences = 0;
> +	bo_gem->used_as_reloc_target = false;
> +	bo_gem->has_error = false;
> +	bo_gem->reusable = false;
> +
> +	bo_gem->bo.handle = handle;
> +	bo_gem->bo.bufmgr = bufmgr;

Can you group the 3 bo_gem->bo initialisation together, and move the
bo_gem->gem_handle initialisation closer. Perhaps:

  bo_gem->bo =

  bo_gem->gem_handle =
  bo_gem->* =

> +
> +	DRMINITLISTHEAD(&bo_gem->name_list);
> +	DRMINITLISTHEAD(&bo_gem->vma_list);
> +
> +	bo_gem->gem_handle = handle;

Comparing with create_from_name(), we should query the tiling at this
point - DRM_IOCTL_i915_GEM_GET_TILING.

That's the only thing missing that I could spot.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre