linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Laura Abbott <labbott@redhat.com>
To: Chris Wilson <chris@chris-wilson.co.uk>,
	Daniel Vetter <daniel.vetter@ffwll.ch>,
	Sean Paul <seanpaul@chromium.org>,
	dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org,
	Sumit Semwal <sumit.semwal@linaro.org>
Subject: Re: [RFC PATCH 2/2] drm/vgem: Enable dmabuf import interfaces
Date: Fri, 7 Apr 2017 09:18:30 -0700	[thread overview]
Message-ID: <b8b1cd0a-f190-935f-9707-8062a64a1d21@redhat.com> (raw)
In-Reply-To: <20170407073957.GT10496@nuc-i3427.alporthouse.com>

On 04/07/2017 12:39 AM, Chris Wilson wrote:
> On Thu, Apr 06, 2017 at 04:18:33PM -0700, Laura Abbott wrote:
>>
>> Enable the GEM dma-buf import interfaces in addition to the export
>> interfaces. This lets vgem be used as a test source for other allocators
>> (e.g. Ion).
>>
>> +int vgem_gem_get_pages(struct drm_vgem_gem_object *obj)
>> +{
>> +	struct page **pages;
>> +
>> +	if (obj->pages)
>> +		return 0;
>> +
>> +	pages = drm_gem_get_pages(&obj->base);
>> +	if (IS_ERR(pages)) {
>> +		return PTR_ERR(pages);
>> +	}
>> +
>> +	obj->pages = pages;
> 
> That's a significant loss in functionality (it requires the entire
> object to fit into physical memory at the same time and requires a large
> vmalloc for 32b systems), for what? vgem only has the ability to mmap
> and export a fd -- both of which you already have if you have the dmabuf
> fd. The only extra interface is the signaling, which does not yet have a
> direct correspondence on dmabuf.
> 
> (An obvious way to keep both would be to move the get_pages to importing
> and then differentiate in the faulthandler where to get the page from.)
> 

Thanks for pointing this out. I'll look to keep the existing behavior.

> Can you provide more details on how you are using vgem to justify the
> changes? I'm also not particularly happy in losing testing of a virtual
> platform device from our CI.
> -Chris
> 

There exists a test module in the Ion directory
(drivers/staging/android/ion/ion_test.c) today but it's not actually
Ion specific. It registers a platform device and then provides an
ioctl interface to perform a dma_buf attach and map. I proposed
moving this to a generic dma-buf test module
(https://marc.info/?l=dri-devel&m=148952187004611&w=2) and Daniel
suggested that this was roughly what vgem was supposed to do.

Adding the import methods for vgem covers all of what the
Ion test was doing and we don't have to invent yet another ioctl
interface and framework for attaching and mapping. 

Can you clarify about what you mean about 'virtual platform device'?


Thanks,
Laura

  reply	other threads:[~2017-04-07 16:18 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-04-06 23:18 [RFC PATCH 0/2] dmabuf import interfaces for vgem Laura Abbott
2017-04-06 23:18 ` [RFC PATCH 1/2] drm/vgem: Add a dummy platform device Laura Abbott
2017-04-06 23:18 ` [RFC PATCH 2/2] drm/vgem: Enable dmabuf import interfaces Laura Abbott
2017-04-07  7:39   ` Chris Wilson
2017-04-07 16:18     ` Laura Abbott [this message]
2017-04-07 16:58       ` Chris Wilson
2017-04-07 17:58         ` Laura Abbott
2017-04-07 21:22           ` Daniel Vetter

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=b8b1cd0a-f190-935f-9707-8062a64a1d21@redhat.com \
    --to=labbott@redhat.com \
    --cc=chris@chris-wilson.co.uk \
    --cc=daniel.vetter@ffwll.ch \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=seanpaul@chromium.org \
    --cc=sumit.semwal@linaro.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).