On Thursday, September 11, 2008 11:03 pm Nick Piggin wrote:
> 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?
Wasn't sure if it was ok to zap the whole VMA in the case we didn't touch it
all.
>
> > 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?
Yeah I can remove them for now; I was expecting we might use them for
debugging or other fun stuff at some point.
> > +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?
Hm, yeah that would be a little easier. :)
> > +
> > + 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).
Ah ok, I'll add that.
> > +
> > +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...
Yeah, same as above. I'll kill them for now since we're not using them.
>
> > @@ -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.
Great, thanks a lot for looking. Now if we can just get your ack on the other
GEM bits we'll be good to go. :)
Thanks,
--
Jesse Barnes, Intel Open Source Technology Center
-------------------------------------------------------------------------
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
