From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH 1/3] drm: use common drm_gem_dmabuf_release in i915/exynos drivers Date: Thu, 8 Aug 2013 01:21:14 +0200 Message-ID: <20130807232114.GH22035@phenom.ffwll.local> References: <1375866908-5000-1-git-send-email-daniel.vetter@ffwll.ch> <1375866908-5000-2-git-send-email-daniel.vetter@ffwll.ch> <014c01ce9352$1e559690$5b00c3b0$%dae@samsung.com> <52021F05.6070401@samsung.com> <20130807102138.GV22035@phenom.ffwll.local> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail-ea0-f182.google.com (mail-ea0-f182.google.com [209.85.215.182]) by gabe.freedesktop.org (Postfix) with ESMTP id 34EB1E7236 for ; Wed, 7 Aug 2013 16:21:09 -0700 (PDT) Received: by mail-ea0-f182.google.com with SMTP id o10so1117999eaj.27 for ; Wed, 07 Aug 2013 16:21:08 -0700 (PDT) Content-Disposition: inline In-Reply-To: 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: Intel Graphics Development , DRI Development List-Id: dri-devel@lists.freedesktop.org On Wed, Aug 07, 2013 at 09:37:52PM +0900, Inki Dae wrote: > 2013/8/7 Daniel Vetter > > > On Wed, Aug 7, 2013 at 2:01 PM, Inki Dae wrote: > > > > > > > > > 2013/8/7 Daniel Vetter > > >> > > >> On Wed, Aug 07, 2013 at 07:18:45PM +0900, Joonyoung Shim wrote: > > >> > On 08/07/2013 06:55 PM, Daniel Vetter wrote: > > >> > >On Wed, Aug 7, 2013 at 11:40 AM, Inki Dae > > wrote: > > >> > >>>-----Original Message----- > > >> > >>>From: Daniel Vetter [mailto:daniel.vetter@ffwll.ch] > > >> > >>>Sent: Wednesday, August 07, 2013 6:15 PM > > >> > >>>To: DRI Development > > >> > >>>Cc: Intel Graphics Development; Daniel Vetter; Inki Dae > > >> > >>>Subject: [PATCH 1/3] drm: use common drm_gem_dmabuf_release in > > >> > >>> i915/exynos > > >> > >>>drivers > > >> > >>> > > >> > >>>Note that this is slightly tricky since both drivers store their > > >> > >>>native objects in dma_buf->priv. But both also embed the base > > >> > >>>drm_gem_object at the first position, so the implicit cast is ok. > > >> > >>> > > >> > >>>To use the release helper we need to export it, too. > > >> > >>Yeah, may I repost this patch with additional work? We also need to > > >> > >> export > > >> > >>with a gem object instead of specific one like you did. > > >> > > > >> > I think dmabuf stuff of exynos can be replaced to common > > drm_gem_dmabuf. > > >> > Already dmabuf stuff of drm_gem_cma_helper.c was substituted to common > > >> > drm_gem_dmabuf with low-level hook functions to use prime helpers. > > >> > > >> Ah, but that can easily be done on top of this, right? > > > > > > > > > Daniel, could you remove exynos related codes from your patch set? Your > > > patch set would make exynos broken because you didn't consider exporting > > > with a gem object for exynos like [PATCH 3/3] drm/i915: explicit store > > base > > > gem object in dma_buf->priv. So I think your patch set is not complete > > set, > > > and That is why exynos needs the additional work I mentioned above. So I > > > just wanted to repost your patch set + new one. > > > > Nope, my patch should not break exynos since the base gem_object is > > the first member of the exynos object, so we don't have any issues > > > > Ah, right. However, it does not seem like good way. > > > > with upcasting in exynos dma-buf code. The same applies to i915 > > dma-buf code, my follow-up patch just makes the code a bit safer. > > > > > > > > > > > > > > However, I think not only exynos could go to common drm_gem_dmabuf > > directly > > > but also it would make your patch set to be complete set if you remove > > > exynos related codes from your patch set. Otherwise, we have to work > > twice. > > > one is the additional work for resolving exynos broken issue by your > > patch > > > set, and other is to replace existing dmabuf stuff of exynos to common > > > drm_gem_dmabuf. > > > > Yeah np, I'll drop exynos then. > > > > Thanks a lot. :) Ah, I remember again why I want to also convert over exynos to the common dma buf release function: Later patches in my prime locking series will change things in there to avoid a userspace-triggerable oops. If we leave out exynos it'll break rather badly for dma-buf export. I need to think a bit more about what stuff looks like atm, but if I resend those parts I'll include exynos. It's a bit tricky that it still works, but that way you can fix it up without the introduction of a bisect failure point. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch