From mboxrd@z Thu Jan 1 00:00:00 1970 From: Thomas Hellstrom Subject: Re: [PATCH 0/3] embed drm_gem_object into radeon_bo Date: Mon, 15 Nov 2010 21:48:06 +0100 Message-ID: <4CE19C86.4010509@shipmail.org> References: <1289681854-15505-1-git-send-email-daniel.vetter@ffwll.ch> <4CE0E05C.7030507@shipmail.org> <20101115184555.GA3484@viiv.ffwll.ch> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: Received: from smtp-outbound-1.vmware.com (smtp-outbound-1.vmware.com [65.115.85.69]) by gabe.freedesktop.org (Postfix) with ESMTP id 29A659ED67 for ; Mon, 15 Nov 2010 12:48:20 -0800 (PST) In-Reply-To: <20101115184555.GA3484@viiv.ffwll.ch> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org Errors-To: dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org To: Daniel Vetter Cc: Daniel Vetter , dri-devel@lists.freedesktop.org List-Id: dri-devel@lists.freedesktop.org On 11/15/2010 07:45 PM, Daniel Vetter wrote: > Hi Thomas, > > Thanks for your comments about ttm and vmwgfx. Some of my own ideas about > where this might all be heading below. > > On Mon, Nov 15, 2010 at 08:25:16AM +0100, Thomas Hellstrom wrote: > >> Hi, Daniel, >> >> My main concerns previously for embedding GEM objects as user-space >> references for TTM has been twofold and implementation specific. >> >> 1) The locking has been using global mutexes where local spin- or >> RCU locks have been more appropriate. It looks like this has finally >> been / is finally going to be addressed. >> > gem object lookup / insert has always (iirc) been protected by a spinlock. > drm/i915 is just a bit inefficient with lookups, hence this spinlock > showing up in profiling. > > >> 2) The gem object is too specialized for general purpose use: >> a) The shmem member has no natural use with TTM except possibly as a >> persistent swap storage, but in an ideal world, TTM would talk to >> the swap cache directly so in the longer term there is no need for >> the shmem object at all. >> > Chris Wilson is working on a gem_vm for i915 that employs a address_space > per bo and directly manages swap entries and has its own page allocator > (actualy cflushed page cache). I haven't yet looked into this closely > (especially the ttm part), but this might (at least in parts) be shareable > between i915 and ttm. At least it gets rid of the shmfs inode in struct > drm_gem_object! > > >> b) Implementations may want to use other user-space visible objects >> than buffer objects: >> For example fence objects or CPU synchronization objects. The common >> traits of / operations on these are user-space visibility, >> inter-process visibility, refcounting and destruction when the >> relevant file is closed. >> >> Hence a class that provides only the user-space handles, naming >> (flink), refcounting and registering with a file handle would be the >> best choice of a "base" class. Traditional Gem objects could derive >> from those and provide any extra *generic* members needed for buffer >> objects. >> >> This doesn't really affect your work though. Just some comments on >> why vmwgfx don't use GEM objects by default and how they could be >> made optimal for TTM-aware drivers. >> > Yeah, I've noticed that vmwgfx is the (sometimes) sole user of many of the > more fancy stuff in ttm. And I also don't like the duplication between gem > and ttm. My plan (i.e. wishful thinking) is to have a common set of helper > functions that can be shared between i915, radeon and nouveau and any > other driver with a gem-like interface (i.e. buffer-objects + execbuffer, > gpu<->cpu sync abstracted away by the kernel). Leaving the cracy stuff for > drivers that need it (vmwgfx). Nothing concrete though (besides a new > testing rig to start hacking on nouveau), I'll intend to simply plow > along, hunting for common patterns. > Sounds good. I actually think the only thing which is currently vmwgfx-specific are the ttm_objects and the ttm_locks. TTM objects could really be replaced by gem objects with the bo-specific part stripped, as discussed above. And then there is the ttm lock to allow concurrent execbufs running. /Thomas > >> Thanks, >> /Thomas >> > Yours, Daniel >