From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH 00/20] prime/flink fixes and related stuff Date: Mon, 5 Aug 2013 09:43:11 +0200 Message-ID: <20130805074311.GY22035@phenom.ffwll.local> References: <1373958731-4132-1-git-send-email-daniel.vetter@ffwll.ch> <06e601ce917f$e2672e10$a7358a30$%dae@samsung.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail-wg0-f42.google.com (mail-wg0-f42.google.com [74.125.82.42]) by gabe.freedesktop.org (Postfix) with ESMTP id 93251E633A for ; Mon, 5 Aug 2013 00:43:05 -0700 (PDT) Received: by mail-wg0-f42.google.com with SMTP id j13so1110607wgh.1 for ; Mon, 05 Aug 2013 00:43:04 -0700 (PDT) Content-Disposition: inline In-Reply-To: <06e601ce917f$e2672e10$a7358a30$%dae@samsung.com> 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: Inki Dae Cc: 'Daniel Vetter' , 'DRI Development' List-Id: dri-devel@lists.freedesktop.org On Mon, Aug 05, 2013 at 11:02:48AM +0900, Inki Dae wrote: > > > > -----Original Message----- > > From: Daniel Vetter [mailto:daniel.vetter@ffwll.ch] > > Sent: Monday, August 05, 2013 2:42 AM > > To: Inki Dae > > Cc: DRI Development > > Subject: Re: [PATCH 00/20] prime/flink fixes and related stuff > > > > On Sat, Jul 27, 2013 at 11:22 AM, Inki Dae wrote: > > > > > > 2013/7/16 Daniel Vetter > > >> > > >> Hi all, > > >> > > >> This patch series is my 2nd real stab at fixing up the locking issues > > >> around our > > >> two buffer sharing mechanisms in gem: flink and prime. > > >> > > >> I think the approach taken here is much better than my first stab, and > > it > > >> also > > >> seems to no longer leak buffers ;-) There some assorted cleanup and > > prep > > >> work > > >> (and one i915 fix) thrown into the mix, it's all stuff I've stumbled > > over > > >> while > > >> digging through the code. > > >> > > >> Open issues left in prime-land after these patches: > > >> - exynos probably wants a similar patch to "drm/i915: explicit store > > base > > >> gem > > >> object in dma_buf->priv". The current code should be correct, but > > it's a > > >> bit > > > > > > > > > How about using stuffs of drm_prime instead of specific ones? Seem like > > that > > > we could replace specific dmabuf stuffs with common ones of drm_prime, > > at > > > least in case of Exynos: i.e. each driver can export a gem to a dmabuf > > > through drm_gem_prime_export function of drm_prime instead of specific > > one. > > > By doing so, I think we could remove duplicated codes of drivers, > > specific > > > dmabuf stuffs. I'm not sure but it seems like that there is any reason > > you > > > try to use existing stuffs with a little change: maybe the stuffs of > > > drm_prime couldn't be used for all drm drivers commonly. > > > > I have to admit that I didn't really understand your email, mostly > > since you talk about "stuff" and "stuffs" and I didn't really grok > > which piece of code you actually mean. Can you pls elaborate, maybe > > taking the functions for exynos as an example? > > You have posted one patch that makes i915/exynos drivers use > drm_gem_dmabuf_release function commonly. But my opinion is that we could > use not only drm_gem_dmabuf_release function but also other functions of > drm_prime commonly. I think that could be done simply like below, > > In drivers/gpu/drm/i915/i915_dev.c, > Static struct drm_driver driver = { > ... > .gem_prime_export = drm_gem_prime_export, > .gem_prime_import = drm_gem_prime_import, > ... > > We are using driver specific dma_buf_ops, i915_dmabuf_ops for i915 and > exynos_dmabuf_ops for exynos. So my opinion is to use > drm_gem_prime_dmabuf_ops of drm_prime instead. However, for this, we have to > change specific export and import functions to exported drm_prim's ones, > drm_gem_prime_export and drm_gem_prime_import. And I thought maybe you > caught that but you used only drm_gem_dmabuf_release function among them of > drm_prime if there is no my missing point. So I guess maybe there is any > reason in doing so. Ah, now I understand. Yeah, there's probably some room to share more code, otoh prime is still rather new so I prefer to keep code separate if it's not obviously the same code. That makes experimentation a bit easier. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch