Re: [PATCH 00/20] prime/flink fixes and related stuff

[Daniel Vetter <daniel@ffwll.ch>]

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 <inki.dae@samsung.com> wrote:
> > >
> > > 2013/7/16 Daniel Vetter <daniel.vetter@ffwll.ch>
> > >>
> > >> 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