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

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

> -----Original Message-----
> From: robdclark@gmail.com [mailto:robdclark@gmail.com] On Behalf Of Rob
> Clark
> Sent: Wednesday, May 16, 2012 9:13 PM
> To: Inki Dae
> Cc: Dave Airlie; kyungmin.park@samsung.com; sw0312.kim@samsung.com; dri-
> devel@lists.freedesktop.org
> Subject: Re: [PATCH 3/4] drm/exynos: added userptr feature.
> 
> On Wed, May 16, 2012 at 4:20 AM, Inki Dae <inki.dae@samsung.com> wrote:
> >
> >
> >> -----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.
> 
> 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)?
> 

Note that PIXMAN is also used as backend of Evas amd the shmem is sent to
EXA backend of X to use gpu.(not same process)

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

Hm...Good point. ok, got it. I gonna stop it forward. Instead, we will
follow the via way for next so do you agree that the via way has no any
problem?

Thanks,
Inki Dae

> From a quick look at the via code, it appears to return while the blit
> is still queued, which I don't completely like but at least the
> pinning and hw use of the userspace buffer is just temporary and not
> able to exist for an indefinite amount of time.
> 
> BR,
> -R
> 
> > 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.
> >
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/dri-devel