[Intel-gfx] [PATCH] drm/i915: add support for physical memory objects

[Jesse Barnes]

On Tuesday, December 30, 2008 2:19 am Dave Airlie wrote:
> From: Dave Airlie <airlied at linux.ie>
>
> This is an initial patch to do support for objects which needs physical
> contiguous main ram, cursors and overlay registers on older chipsets.
>
> These objects are bound on cursor bin, like pinning, and we copy
> the data to/from the backing store object into the real one on
> attach/detach.

> +
> +/*
> + * Create a physically contiguous memory object for this object
> + * e.g. for cursor + overlay regs
> + */
> +int i915_gem_init_phys_object(struct drm_device *dev,
> +			      int id, int size)
> +{
> +	drm_i915_private_t *dev_priv = dev->dev_private;
> +	struct drm_i915_gem_phys_object *phys_obj;
> +	int ret;
> +
> +	if (dev_priv->mm.phys_objs[id] || !size)
> +		return 0;
> +
> +	phys_obj = drm_calloc(1, sizeof(struct drm_i915_gem_phys_object),
> DRM_MEM_DRIVER); +	if (!phys_obj)
> +		return -ENOMEM;
> +
> +	phys_obj->id = id;
> +
> +	phys_obj->handle = drm_pci_alloc(dev, size, 0, 0xffffffff);
> +	if (!phys_obj->handle) {
> +		ret = -ENOMEM;
> +		goto kfree_obj;
> +	}
> +#ifdef CONFIG_X86
> +	set_memory_wc((unsigned long)phys_obj->handle->vaddr,
> phys_obj->handle->size / PAGE_SIZE); +#endif
> +
> +	dev_priv->mm.phys_objs[id] = phys_obj;
> +
> +	return 0;
> +kfree_obj:
> +	drm_free(phys_obj, sizeof(struct drm_i915_gem_phys_object),
> DRM_MEM_DRIVER); +	return ret;
> +}

Yes, this is much better than the old code (agp->base + gtt_offset != phys addr duh).

> +
> +void i915_gem_free_phys_object(struct drm_device *dev, int id)
> +{
> +	drm_i915_private_t *dev_priv = dev->dev_private;
> +	struct drm_i915_gem_phys_object *phys_obj;
> +
> +	if (!dev_priv->mm.phys_objs[id])
> +		return;
> +
> +	phys_obj = dev_priv->mm.phys_objs[id];
> +	if (phys_obj->cur_obj) {
> +		i915_gem_detach_phys_object(dev, phys_obj->cur_obj);
> +	}
> +
> +#ifdef CONFIG_X86
> +	set_memory_wb((unsigned long)phys_obj->handle->vaddr,
> phys_obj->handle->size / PAGE_SIZE); +#endif
> +	drm_pci_free(dev, phys_obj->handle);
> +	kfree(phys_obj);
> +	dev_priv->mm.phys_objs[id] = NULL;
> +}
> +
> +void i915_gem_free_all_phys_object(struct drm_device *dev)
> +{
> +	int i;
> +
> +	for (i = 0; i < I915_MAX_PHYS_OBJECT; i++)
> +		i915_gem_free_phys_object(dev, i);
> +}
> +
> +void i915_gem_detach_phys_object(struct drm_device *dev,
> +				 struct drm_gem_object *obj)
> +{
> +	struct drm_i915_gem_object *obj_priv;
> +	int i;
> +	int ret;
> +	int page_count;
> +
> +	obj_priv = obj->driver_private;
> +	if (!obj_priv->phys_obj)
> +		return;
> +
> +	ret = i915_gem_object_get_page_list(obj);
> +	if (ret)
> +		goto out;
> +
> +	page_count = obj->size / PAGE_SIZE;
> +
> +	for (i = 0; i < page_count; i++) {
> +		char *dst = kmap_atomic(obj_priv->page_list[i], KM_USER0);
> +		char *src = obj_priv->phys_obj->handle->vaddr + (i * PAGE_SIZE);
> +
> +		memcpy(dst, src, PAGE_SIZE);
> +		kunmap_atomic(dst, KM_USER0);
> +	}
> +	drm_clflush_pages(obj_priv->page_list, page_count);
> +	drm_agp_chipset_flush(dev);
> +out:
> +	obj_priv->phys_obj->cur_obj = NULL;
> +	obj_priv->phys_obj = NULL;
> +}
> +
> +int
> +i915_gem_attach_phys_object(struct drm_device *dev,
> +			    struct drm_gem_object *obj, int id)
> +{
> +	drm_i915_private_t *dev_priv = dev->dev_private;
> +	struct drm_i915_gem_object *obj_priv;
> +	int ret = 0;
> +	int page_count;
> +	int i;
> +
> +	if (id > I915_MAX_PHYS_OBJECT)
> +		return -EINVAL;
> +
> +	obj_priv = obj->driver_private;
> +
> +	if (obj_priv->phys_obj) {
> +		if (obj_priv->phys_obj->id == id)
> +			return 0;
> +		i915_gem_detach_phys_object(dev, obj);
> +	}
> +
> +
> +	/* create a new object */
> +	if (!dev_priv->mm.phys_objs[id]) {
> +		ret = i915_gem_init_phys_object(dev, id,
> +						obj->size);
> +		if (ret)
> +			goto out;
> +	}
> +
> +	/* bind to the object */
> +	obj_priv->phys_obj = dev_priv->mm.phys_objs[id];
> +	obj_priv->phys_obj->cur_obj = obj;
> +
> +	ret = i915_gem_object_get_page_list(obj);
> +	if (ret)
> +		goto out;
> +
> +	page_count = obj->size / PAGE_SIZE;
> +
> +	for (i = 0; i < page_count; i++) {
> +		char *src = kmap_atomic(obj_priv->page_list[i], KM_USER0);
> +		char *dst = obj_priv->phys_obj->handle->vaddr + (i * PAGE_SIZE);
> +
> +		memcpy(dst, src, PAGE_SIZE);
> +		kunmap_atomic(src, KM_USER0);
> +	}
> +
> +	return 0;
> +out:
> +	return ret;
> +}
> +
> +static int
> +i915_gem_phys_pwrite(struct drm_device *dev, struct drm_gem_object *obj,
> +		     struct drm_i915_gem_pwrite *args,
> +		     struct drm_file *file_priv)
> +{
> +	struct drm_i915_gem_object *obj_priv = obj->driver_private;
> +	void *obj_addr;
> +	int ret;
> +	char __user *user_data;
> +
> +	user_data = (char __user *) (uintptr_t) args->data_ptr;
> +	obj_addr = obj_priv->phys_obj->handle->vaddr + args->offset;
> +
> +	DRM_ERROR("obj_addr %p, %lld\n", obj_addr, args->size);
> +	ret = copy_from_user(obj_addr, user_data, args->size);
> +	if (ret)
> +		return -EFAULT;
> +
> +	drm_agp_chipset_flush(dev);
> +	return 0;
> +}
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c index 1204d26..5556e20 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -1017,17 +1017,23 @@ static int intel_crtc_cursor_set(struct drm_crtc
> *crtc, return -ENOMEM;
>  	}
>
> -	if (dev_priv->cursor_needs_physical) {
> -		addr = dev->agp->base + obj_priv->gtt_offset;
> -	} else {
> +	/* we only need to pin inside GTT if cursor is non-phy */
> +	if (!dev_priv->cursor_needs_physical) {
> +		ret = i915_gem_object_pin(bo, PAGE_SIZE);
> +		if (ret) {
> +			DRM_ERROR("failed to pin cursor bo\n");
> +			drm_gem_object_unreference(bo);
> +			return ret;
> +		}
>  		addr = obj_priv->gtt_offset;
> -	}
> -
> -	ret = i915_gem_object_pin(bo, PAGE_SIZE);
> -	if (ret) {
> -		DRM_ERROR("failed to pin cursor bo\n");
> -		drm_gem_object_unreference(bo);
> -		return ret;
> +	} else {
> +		ret = i915_gem_attach_phys_object(dev, bo, (pipe == 0) ?
> I915_GEM_PHYS_CURSOR_0 : I915_GEM_PHYS_CURSOR_1); +		if (ret) {
> +			DRM_ERROR("failed to attach phys object\n");
> +			drm_gem_object_unreference(bo);
> +			return ret;
> +		}
> +		addr = obj_priv->phys_obj->handle->busaddr;
>  	}

And it looks like this catches the fact that we pinned *after* looking at
gtt_offset, which was wrong.  So that's good.


We'll also need the below to fix a thinko in modeset_init:

Correct cursor_needs_physical initialization.  The existing test is either
missing some parens or should be split out like the 2D driver (this patch
takes the latter approach to make it easier to compare).

Signed-off-by: Jesse Barnes <jbarnes at virtuousgeek.org>

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 44f20bf..87932b7 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -938,11 +938,14 @@ static int i915_load_modeset_init(struct drm_device *dev)
 
 	DRM_DEBUG("*** fb base 0x%08lx\n", dev->mode_config.fb_base);
 
-	if (IS_MOBILE(dev) || (IS_I9XX(dev) && !IS_I965G(dev) && !IS_G33(dev)))
+	if (IS_MOBILE(dev) || IS_I9XX(dev))
 		dev_priv->cursor_needs_physical = true;
 	else
 		dev_priv->cursor_needs_physical = false;
 
+	if (IS_I965G(dev) || IS_G33(dev))
+		dev_priv->cursor_needs_physical = false;
+
 	ret = i915_probe_agp(dev, &agp_size, &prealloc_size);
 	if (ret)
 		goto kfree_devname;