[PATCH] drm/vgem: implement virtual GEM

[Thomas Hellstrom]

On 05/21/2015 11:13 AM, Daniel Vetter wrote:
> On Thu, Apr 02, 2015 at 10:30:53AM +0200, Thomas Hellstrom wrote:
>> On 11/25/2014 02:08 AM, Zachary Reizner wrote:
>>> After looking into removing platform_device, I found that using
>>> dma_buf_attach with a NULL device always returns an error, thereby
>>> preventing me from using VGEM for import and mmap. The solution seems
>>> to be to skip using dma_buf_attach, and instead use dma_buf_mmap when
>>> user-space tries to mmap a gem object that was imported into VGEM. The
>>> drawback to this approach is that most drivers stub their
>>> dma_buf_ops->mmap implementation. Presumably mmap could be implemented
>>> for the drivers that this would make sense for. Are there any
>>> comments, questions, or concerns for this proposed solution?
>> I see now that this driver has entered -next, and I'm sorry this comment
>> didn't arrive before. I simply missed this discussion :(
>>
>> My biggest concern, as stated many many times before, is that dma-buf
>> mmap is a horrible interface for incoherent drivers, and for drivers
>> that use odd format (tiled) dma-bufs, basically since it doesn't supply
>> a dirtied region. Therefore (correct me if I'm wrong) there has been an
>> agreement that for purposes outside of ARM SOC, we should simply not
>> implement dma-buf mmap for other uses than for internal driver use.
>>
>> So assume a real driver implements dma-buf mmap, but it is crawling due
>> to coherency- or untiling / tiling operations. How do you tell a generic
>> user of the vgem driver *NOT* to mmap for performance reasons? Or is
>> this driver only intended for ARM SOC systems?
> Seconded. Somehow I thought we've pulled in vgem to support software
> rendering like llvmpipe, and I remember that that's been the original
> justification. TIL that that's indeed not the case and google is
> splattering their cros tree with dma_buf->mmap implementations this is
> obviously not the case and the intention really seems to be to use
> dma_buf->mmap and vgem as the generic interface to expose buffer objects
> of real drivers to software rendering.
>
> Given that neither vgem nor dma_buf->mmap has any sane concept of handling
> coherency I'm really unhappy about this and tempted to just submit the
> revert for vgem before 4.1 ships. I'll chat with relevant people a bit
> more. Worse I chatted with Stephane today and he brushed this off as
> not-my-problem and if this hurts intel intel should fix this. That's not
> how a proper usptream interface is getting designd, and coherency handling
> is an even more serious problem on arm an virtual hw like vmwgfx.

So given how this has turned out, my opinion is that before a usable
generic mmap of accelerated buffer objects
goes upstream, there should be a proper interface to request regions
present and to dirty regions. It seems to me that so far
all use-cases are for one- or two-dimensional access so it should be
sufficient to start with that and add other access
modes later on. Now this is no guarantee that people won't request and
dirty the whole dma-buf on each access, but at least
that would make people think, and if things become slow it's pretty
clear where the problem is.

I'm all for delaying vgem until we have such an interface in place.

Thanks,
Thomas




>  On intel
> (well at least big core thanks to the huge coherent cache fabric) this is
> mostly a non-issue, except that the patch in the cros tree obviously gets
> things wrong.
>
> Decently pissed tbh.
>
> Cheers, Daniel