RE: [PATCH 3/4] drm/exynos: added userptr feature.

[Inki Dae <inki.dae@samsung.com>]

> -----Original Message-----
> From: Dave Airlie [mailto:airlied@gmail.com]
> Sent: Wednesday, May 16, 2012 6:23 PM
> To: Rob Clark
> Cc: Inki Dae; kyungmin.park@samsung.com; sw0312.kim@samsung.com; dri-
> devel@lists.freedesktop.org
> Subject: Re: [PATCH 3/4] drm/exynos: added userptr feature.
> 
> On Tue, May 15, 2012 at 8:34 AM, Rob Clark <rob.clark@linaro.org> wrote:
> > On Mon, Apr 23, 2012 at 7:43 AM, Inki Dae <inki.dae@samsung.com> wrote:
> >> this feature could be used to use memory region allocated by malloc()
> in user
> >> mode and mmaped memory region allocated by other memory allocators.
> userptr
> >> interface can identify memory type through vm_flags value and would get
> >> pages or page frame numbers to user space appropriately.
> >
> > I apologize for being a little late to jump in on this thread, but...
> >
> > I must confess to not being a huge fan of userptr.  It really is
> > opening a can of worms, and seems best avoided if at all possible.
> > I'm not entirely sure the use-case for which you require this, but I
> > wonder if there isn't an alternative way?   I mean, the main case I
> > could think of for mapping userspace memory would be something like
> > texture upload.  But could that be handled in an alternative way,
> > something like a pwrite or texture_upload API, which could temporarily
> > pin the userspace memory, kick off a dma from that user buffer to a
> > proper GEM buffer, and then unpin the user buffer when the DMA
> > completes?
> 
> I'm with Rob on this, I really hate the userptr idea, and my problem
> with letting it into exynos is it sets a benchmark for others to do
> things the same way. I'm still not 100% sure how its going to be used
> even with all your explainations.
> 
> Since we've agreed only the X server can access the interface, it
> makes 0 sense to me to exist at all, as the X server can avoid malloc
> memory for all objects it accesses.
> 
> I don't think pixman is at the level where you should be acceleration
> it directly. I thought the point of pixman was a fast SW engine, not
> something to be trunked down to a hw engine. The idea being you use
> cairo and backend it onto something.
> 

For more understanding, PIXMAN draws something on shmem and next id of the
shmem is sent to X and next X gets user address to the id and next EXA's gpu
backend does BitBLT using gpu hardware. However, the gpu backend doesn't
aware of the user address so the purpose of using userptr is to import the
shmem into a gem object for gpu to aware of the memory region as source. so
pixman would use SW engine as is.

> I know ssp had some ideas for making pixman be able to do hw accel,
> but userptr doesn't seem like the proper solution, it seems like a
> hack that needs a lot more VM work to operate properly, and by setting
> a precedent for one GPU driver, I'll have 20 implementations of this
> from ARM vendors and nobody will ever go back and fix things properly.
> 

I'm not sure that this is a hack or not but thing similar to this is being
used at via driver. you can refer to via_lock_all_dma_pages function of
via_dmablit.c file and this driver also uses get_user_pages() to lock all
the pages to the user space for DMA to access the memory region, maybe for
BitBLT. And I really wonder you think just using get_user_pages() is a hack
or importing user address into a gem object. there may be my
misunderstanding so give me any comments.

Thanks,
Inki Dae

> So I'm really not sure the best way to move this forward, maybe a very
> clear set of use cases of where stuff plugs into this, and why dma-buf
> or some other method isn't sufficient, but I'm having trouble getting
> past the fact its setting a dangerous precedent.
> 
> Dave.