On Wed, May 30, 2012 at 05:40:13PM +0200, Laurent Pinchart wrote:
> Hi Sascha,
>
> Thank you for the patch. I've successfully tested the helper with the new SH
> Mobile DRM driver. Just a couple of comments below in addition to Lars'
> comments (this is not a full review, just details that caught my attention
> when comparing the code with my implementation, and trying to use it).
>
> > +/*
> > + * drm_gem_cma_mmap - (struct file_operation)->mmap callback function
> > + */
> > +int drm_gem_cma_mmap(struct file *filp, struct vm_area_struct *vma)
> > +{
> > + int ret;
> > +
> > + ret = drm_gem_mmap(filp, vma);
> > + if (ret)
> > + return ret;
> > +
> > + vma->vm_flags &= ~VM_PFNMAP;
> > + vma->vm_flags |= VM_MIXEDMAP;
>
> Why is this a mixed map ?
Honestly, I don't know. This is copied from the exynos driver. I don't
know much about these flags, so if you think something else is better
here than you're probably right ;)
>
> > +
> > + return ret;
> > +}
> > +EXPORT_SYMBOL_GPL(drm_gem_cma_mmap);
>
> My implementation maps the whole buffer in one go at mmap time, not page by
> page at page fault time. Isn't that more efficient when mapping frame buffer
> memory ?
I remember Alan has mentioned this in an earlier review. I'll update
the patch accordingly.
I will fix this and the other things you mentioned and repost.
Thanks
Sascha
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel
