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

[Rob Clark]

On Wed, May 16, 2012 at 4:20 AM, Inki Dae <inki.dae at samsung.com> wrote:
>
>
>> -----Original Message-----
>> From: Dave Airlie [mailto:airlied at gmail.com]
>> Sent: Wednesday, May 16, 2012 6:23 PM
>> To: Rob Clark
>> Cc: Inki Dae; kyungmin.park at samsung.com; sw0312.kim at samsung.com; dri-
>> devel at 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 at linaro.org> wrote:
>> > On Mon, Apr 23, 2012 at 7:43 AM, Inki Dae <inki.dae at 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.

if this is all just for X/EXA, wouldn't it make more sense to handle
the operation at the EXA layer before falling back to sw (which would
go to pixman)?

>> 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.

My objection is not really the get_user_pages(), but rather converting
that into a GEM object, which might exist for longer period of time,
be exported to dmabuf and passed to other driver, etc.  (Not to
mention you don't really have control if userspace does free(ptr)
while the GEM object still exists..)