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