On Thu, Sep 11, 2008 at 06:16:21PM -0700, Jesse Barnes wrote:
> Here's an updated set of GTT mapping patches against Eric's drm-gem-merge
> branch from earlier today and DRM master for the libdrm bits. I fixed up a
> couple of bugs, but it looks like UXA/EXA still don't work against GEM even
> w/o these changes applied. I'm not quite sure about the zap_vma_ptes code,
> that'll need review from one of the MM guys (Nick can you take a look at the
> i915-gtt-mapping patch?).
>
> Other than that, I think the code is looking fairly complete, so at this
> point
> we just need to stabilize GEM and play with using GTT mapping for the general
> dri_bo_map case to see if we want to do it by default.
Well I had a quick look, and the zap_vma_ptes call seems fine. It will just
basically undo the effects of all the vm_insert_pfn calls on that vma, right?
Anything in particular you were worried about?
> return 0;
> }
>
> +void i915_gem_vm_open(struct vm_area_struct *vma)
> +{
> +}
> +
> +void i915_gem_vm_close(struct vm_area_struct *vma)
> +{
> +}
Just a nitpick but can you just skip these if they are empty?
> +int i915_gem_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
> +{
> + struct drm_gem_object *obj = vma->vm_private_data;
> + struct drm_device *dev = obj->dev;
> + struct drm_i915_gem_object *obj_priv = obj->driver_private;
> + unsigned long page_offset;
> + unsigned long pfn;
> + int ret = 0;
> +
> + page_offset = ((unsigned long)vmf->virtual_address - vma->vm_start) >>
> + PAGE_SHIFT;
vmf->pgoff?
> +
> + DRM_ERROR("faulting in object %d offset 0x%08lx\n", obj->name,
> + page_offset);
> +
> + /* Now bind it into the GTT */
> + if (i915_gem_object_bind_to_gtt(obj, obj_priv->gtt_alignment))
> + return VM_FAULT_SIGBUS;
> +
> + vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);
> + pfn = ((dev->agp->base + obj_priv->gtt_offset) >> PAGE_SHIFT) +
> + page_offset;
> + /* Finally, remap it using the new GTT offset */
> + ret = vm_insert_pfn(vma, (unsigned long)vmf->virtual_address, pfn);
> +
> + return ret == -EAGAIN ? VM_FAULT_OOM : VM_FAULT_NOPAGE;
> +}
Should better check return from vm_inesrt_pfn, maybe?
-ENOMEM should give VM_FAULT_OOM
-EFAULT would be a bug in the driver probably, but VM_FAULT_SIGBUS maybe
-EBUSY might be a bug (or maybe it can happen if multiple threads fault
the same address, in which case VM_FAULT_NOPAGE is fine).
> +
> +int i915_gem_vm_mkwrite(struct vm_area_struct *vma, struct page *page)
> +{
> + return 0;
> +}
> +
> +int i915_gem_vm_access(struct vm_area_struct *vma, unsigned long addr,
> + void *buf, int len, int write)
> +{
> + return 0;
> +}
Same for these...
> @@ -940,6 +1029,10 @@ i915_gem_object_unbind(struct drm_gem_object *obj)
>
> BUG_ON(obj_priv->active);
>
> + /* blow away mappings if mapped through GTT */
> + if (vma)
> + zap_vma_ptes(vma, vma->vm_start, vma->vm_end - vma->vm_start);
Seems fine to me.
-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/
--
_______________________________________________
Dri-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/dri-devel
