From mboxrd@z Thu Jan 1 00:00:00 1970 From: Mark yao Subject: Re: [PATCH 2/8] drm/rockchip: Get rid of some unnecessary code Date: Sun, 18 Sep 2016 09:50:38 +0800 Message-ID: <57DDF2EE.5060807@rock-chips.com> References: <1473857701-9250-1-git-send-email-tfiga@chromium.org> <1473857701-9250-3-git-send-email-tfiga@chromium.org> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8"; Format="flowed" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: <1473857701-9250-3-git-send-email-tfiga@chromium.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Tomasz Figa , dri-devel@lists.freedesktop.org Cc: linux-kernel@vger.kernel.org, linux-rockchip@lists.infradead.org, linux-arm-kernel@lists.infradead.org List-Id: linux-rockchip.vger.kernel.org T24gMjAxNuW5tDA55pyIMTTml6UgMjA6NTQsIFRvbWFzeiBGaWdhIHdyb3RlOgo+IEN1cnJlbnQg Y29kZSBpbXBsZW1lbnRzIHByZXBhcmVfZmIgYW5kIGNsZWFudXBfZmIgY2FsbGJhY2tzIG9ubHkg dG8KPiBncmFiL3JlbGVhc2UgZmIgcmVmZXJlbmNlcywgd2hpY2ggaXMgYWxyZWFkeSBkb25lIGJ5 IGF0b21pYyBmcmFtZXdvcmsKPiB3aGVuIGNyZWF0aW5nL2Rlc3RyeW9pbmcgcGxhbmUgc3RhdGUu IExldCdzIHJlbW92ZSB0aGVzZQo+IHVudXNlZCBiaXRzLgo+Cj4gU2lnbmVkLW9mZi1ieTogVG9t YXN6IEZpZ2EgPHRmaWdhQGNocm9taXVtLm9yZz4KPiAtLS0KPiAgIGRyaXZlcnMvZ3B1L2RybS9y b2NrY2hpcC9yb2NrY2hpcF9kcm1fdm9wLmMgfCAxOCAtLS0tLS0tLS0tLS0tLS0tLS0KPiAgIDEg ZmlsZSBjaGFuZ2VkLCAxOCBkZWxldGlvbnMoLSkKCkhpIFRvbWFzegoKSSB0aGluayB3ZSBjYW4n dCBnZXQgcmlkIG9mIHRoZSBwcmVwYXJlX2ZiIGFuZCBjbGVhbnVwX2ZiCgpzZWUgdGhlIHJlYXNv bjoKY29tbWl0IDQ0ZDAyMzdhMjYzOTVhYzk0MTYwY2YyM2YzMjc2OTAxM2IzNjU1OTAKQXV0aG9y OiBNYXJrIFlhbyA8bWFyay55YW9Acm9jay1jaGlwcy5jb20+CkRhdGU6ICAgRnJpIEFwciAyOSAx MTozNzoyMCAyMDE2ICswODAwCgogICAgIGRybS9yb2NrY2hpcDogdm9wOiBmaXggaW9tbXUgY3Jh c2ggd2l0aCBhc3luYyBhdG9taWMKCiAgICAgQWZ0ZXIgYXN5bmMgYXRvbWljX2NvbW1pdCBjYWxs YmFjaywgZHJtX2F0b21pY19jbGVhbl9vbGRfZmIgd2lsbAogICAgIGNsZWFuIGFsbCBvbGQgZmIs IGJ1dCBiZWNhdXNlIGFzeW5jLCB0aGUgb2xkIGZiIG1heSBiZSBhbHNvIG9uCiAgICAgdGhlIHZv cCBoYXJkd2FyZSwgZG1hIHdpbGwgYWNjZXNzIHRoZSBvbGQgZmIgYnVmZmVyLCBjbGVhbiBvbGQK ICAgICBmYiB3aWxsIGNhdXNlIGlvbW11IHBhZ2UgZmF1bHQuCgogICAgIFJlZmVyZW5jZSB0aGUg ZmIgYW5kIHVucmVmZXJlbmNlIGl0IHdoZW4gdGhlIGZiIGFjdHVhbGwgc3dhcCBvdXQKICAgICBm cm9tIHZvcCBoYXJkd2FyZS4KCgo+IGRpZmYgLS1naXQgYS9kcml2ZXJzL2dwdS9kcm0vcm9ja2No aXAvcm9ja2NoaXBfZHJtX3ZvcC5jIGIvZHJpdmVycy9ncHUvZHJtL3JvY2tjaGlwL3JvY2tjaGlw X2RybV92b3AuYwo+IGluZGV4IDdlODExY2YuLjY4OTg4YzYgMTAwNjQ0Cj4gLS0tIGEvZHJpdmVy cy9ncHUvZHJtL3JvY2tjaGlwL3JvY2tjaGlwX2RybV92b3AuYwo+ICsrKyBiL2RyaXZlcnMvZ3B1 L2RybS9yb2NrY2hpcC9yb2NrY2hpcF9kcm1fdm9wLmMKPiBAQCAtNjQxLDIyICs2NDEsNiBAQCBz dGF0aWMgdm9pZCB2b3BfcGxhbmVfZGVzdHJveShzdHJ1Y3QgZHJtX3BsYW5lICpwbGFuZSkKPiAg IAlkcm1fcGxhbmVfY2xlYW51cChwbGFuZSk7Cj4gICB9Cj4gICAKPiAtc3RhdGljIGludCB2b3Bf cGxhbmVfcHJlcGFyZV9mYihzdHJ1Y3QgZHJtX3BsYW5lICpwbGFuZSwKPiAtCQkJCXN0cnVjdCBk cm1fcGxhbmVfc3RhdGUgKm5ld19zdGF0ZSkKPiAtewo+IC0JaWYgKHBsYW5lLT5zdGF0ZS0+ZmIp Cj4gLQkJZHJtX2ZyYW1lYnVmZmVyX3JlZmVyZW5jZShwbGFuZS0+c3RhdGUtPmZiKTsKPiAtCj4g LQlyZXR1cm4gMDsKPiAtfQo+IC0KPiAtc3RhdGljIHZvaWQgdm9wX3BsYW5lX2NsZWFudXBfZmIo c3RydWN0IGRybV9wbGFuZSAqcGxhbmUsCj4gLQkJCQkgc3RydWN0IGRybV9wbGFuZV9zdGF0ZSAq b2xkX3N0YXRlKQo+IC17Cj4gLQlpZiAob2xkX3N0YXRlLT5mYikKPiAtCQlkcm1fZnJhbWVidWZm ZXJfdW5yZWZlcmVuY2Uob2xkX3N0YXRlLT5mYik7Cj4gLX0KPiAtCj4gICBzdGF0aWMgaW50IHZv cF9wbGFuZV9hdG9taWNfY2hlY2soc3RydWN0IGRybV9wbGFuZSAqcGxhbmUsCj4gICAJCQkgICBz dHJ1Y3QgZHJtX3BsYW5lX3N0YXRlICpzdGF0ZSkKPiAgIHsKPiBAQCAtODQ5LDggKzgzMyw2IEBA IHN0YXRpYyB2b2lkIHZvcF9wbGFuZV9hdG9taWNfdXBkYXRlKHN0cnVjdCBkcm1fcGxhbmUgKnBs YW5lLAo+ICAgfQo+ICAgCj4gICBzdGF0aWMgY29uc3Qgc3RydWN0IGRybV9wbGFuZV9oZWxwZXJf ZnVuY3MgcGxhbmVfaGVscGVyX2Z1bmNzID0gewo+IC0JLnByZXBhcmVfZmIgPSB2b3BfcGxhbmVf cHJlcGFyZV9mYiwKPiAtCS5jbGVhbnVwX2ZiID0gdm9wX3BsYW5lX2NsZWFudXBfZmIsCj4gICAJ LmF0b21pY19jaGVjayA9IHZvcF9wbGFuZV9hdG9taWNfY2hlY2ssCj4gICAJLmF0b21pY191cGRh dGUgPSB2b3BfcGxhbmVfYXRvbWljX3VwZGF0ZSwKPiAgIAkuYXRvbWljX2Rpc2FibGUgPSB2b3Bf cGxhbmVfYXRvbWljX2Rpc2FibGUsCgoKLS0gCu+8rWFyayBZYW8KCgpfX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fXwpkcmktZGV2ZWwgbWFpbGluZyBsaXN0CmRy aS1kZXZlbEBsaXN0cy5mcmVlZGVza3RvcC5vcmcKaHR0cHM6Ly9saXN0cy5mcmVlZGVza3RvcC5v cmcvbWFpbG1hbi9saXN0aW5mby9kcmktZGV2ZWwK From mboxrd@z Thu Jan 1 00:00:00 1970 From: mark.yao@rock-chips.com (Mark yao) Date: Sun, 18 Sep 2016 09:50:38 +0800 Subject: [PATCH 2/8] drm/rockchip: Get rid of some unnecessary code In-Reply-To: <1473857701-9250-3-git-send-email-tfiga@chromium.org> References: <1473857701-9250-1-git-send-email-tfiga@chromium.org> <1473857701-9250-3-git-send-email-tfiga@chromium.org> Message-ID: <57DDF2EE.5060807@rock-chips.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On 2016?09?14? 20:54, Tomasz Figa wrote: > Current code implements prepare_fb and cleanup_fb callbacks only to > grab/release fb references, which is already done by atomic framework > when creating/destryoing plane state. Let's remove these > unused bits. > > Signed-off-by: Tomasz Figa > --- > drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 18 ------------------ > 1 file changed, 18 deletions(-) Hi Tomasz I think we can't get rid of the prepare_fb and cleanup_fb see the reason: commit 44d0237a26395ac94160cf23f32769013b365590 Author: Mark Yao Date: Fri Apr 29 11:37:20 2016 +0800 drm/rockchip: vop: fix iommu crash with async atomic After async atomic_commit callback, drm_atomic_clean_old_fb will clean all old fb, but because async, the old fb may be also on the vop hardware, dma will access the old fb buffer, clean old fb will cause iommu page fault. Reference the fb and unreference it when the fb actuall swap out from vop hardware. > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c > index 7e811cf..68988c6 100644 > --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c > @@ -641,22 +641,6 @@ static void vop_plane_destroy(struct drm_plane *plane) > drm_plane_cleanup(plane); > } > > -static int vop_plane_prepare_fb(struct drm_plane *plane, > - struct drm_plane_state *new_state) > -{ > - if (plane->state->fb) > - drm_framebuffer_reference(plane->state->fb); > - > - return 0; > -} > - > -static void vop_plane_cleanup_fb(struct drm_plane *plane, > - struct drm_plane_state *old_state) > -{ > - if (old_state->fb) > - drm_framebuffer_unreference(old_state->fb); > -} > - > static int vop_plane_atomic_check(struct drm_plane *plane, > struct drm_plane_state *state) > { > @@ -849,8 +833,6 @@ static void vop_plane_atomic_update(struct drm_plane *plane, > } > > static const struct drm_plane_helper_funcs plane_helper_funcs = { > - .prepare_fb = vop_plane_prepare_fb, > - .cleanup_fb = vop_plane_cleanup_fb, > .atomic_check = vop_plane_atomic_check, > .atomic_update = vop_plane_atomic_update, > .atomic_disable = vop_plane_atomic_disable, -- ?ark Yao From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755660AbcIRBuv (ORCPT ); Sat, 17 Sep 2016 21:50:51 -0400 Received: from regular1.263xmail.com ([211.150.99.137]:58284 "EHLO regular1.263xmail.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755050AbcIRBuu (ORCPT ); Sat, 17 Sep 2016 21:50:50 -0400 X-263anti-spam: KSV:0;BIG:0;ABS:1;DNS:0;ATT:0;SPF:S; X-MAIL-GRAY: 0 X-MAIL-DELIVERY: 1 X-KSVirus-check: 0 X-ABS-CHECKED: 1 X-SKE-CHECKED: 1 X-ADDR-CHECKED4: 1 X-RL-SENDER: mark.yao@rock-chips.com X-FST-TO: djkurtz@chromium.org X-SENDER-IP: 103.29.142.67 X-LOGIN-NAME: mark.yao@rock-chips.com X-UNIQUE-TAG: <4b70aba02c9483809d7a9741dc28a390> X-ATTACHMENT-NUM: 0 X-DNS-TYPE: 0 Subject: Re: [PATCH 2/8] drm/rockchip: Get rid of some unnecessary code To: Tomasz Figa , dri-devel@lists.freedesktop.org References: <1473857701-9250-1-git-send-email-tfiga@chromium.org> <1473857701-9250-3-git-send-email-tfiga@chromium.org> Cc: linux-arm-kernel@lists.infradead.org, linux-rockchip@lists.infradead.org, linux-kernel@vger.kernel.org, Heiko Stuebner , David Airlie , Sean Paul , Daniel Kurtz From: Mark yao Message-ID: <57DDF2EE.5060807@rock-chips.com> Date: Sun, 18 Sep 2016 09:50:38 +0800 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 MIME-Version: 1.0 In-Reply-To: <1473857701-9250-3-git-send-email-tfiga@chromium.org> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 2016年09月14日 20:54, Tomasz Figa wrote: > Current code implements prepare_fb and cleanup_fb callbacks only to > grab/release fb references, which is already done by atomic framework > when creating/destryoing plane state. Let's remove these > unused bits. > > Signed-off-by: Tomasz Figa > --- > drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 18 ------------------ > 1 file changed, 18 deletions(-) Hi Tomasz I think we can't get rid of the prepare_fb and cleanup_fb see the reason: commit 44d0237a26395ac94160cf23f32769013b365590 Author: Mark Yao Date: Fri Apr 29 11:37:20 2016 +0800 drm/rockchip: vop: fix iommu crash with async atomic After async atomic_commit callback, drm_atomic_clean_old_fb will clean all old fb, but because async, the old fb may be also on the vop hardware, dma will access the old fb buffer, clean old fb will cause iommu page fault. Reference the fb and unreference it when the fb actuall swap out from vop hardware. > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c > index 7e811cf..68988c6 100644 > --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c > @@ -641,22 +641,6 @@ static void vop_plane_destroy(struct drm_plane *plane) > drm_plane_cleanup(plane); > } > > -static int vop_plane_prepare_fb(struct drm_plane *plane, > - struct drm_plane_state *new_state) > -{ > - if (plane->state->fb) > - drm_framebuffer_reference(plane->state->fb); > - > - return 0; > -} > - > -static void vop_plane_cleanup_fb(struct drm_plane *plane, > - struct drm_plane_state *old_state) > -{ > - if (old_state->fb) > - drm_framebuffer_unreference(old_state->fb); > -} > - > static int vop_plane_atomic_check(struct drm_plane *plane, > struct drm_plane_state *state) > { > @@ -849,8 +833,6 @@ static void vop_plane_atomic_update(struct drm_plane *plane, > } > > static const struct drm_plane_helper_funcs plane_helper_funcs = { > - .prepare_fb = vop_plane_prepare_fb, > - .cleanup_fb = vop_plane_cleanup_fb, > .atomic_check = vop_plane_atomic_check, > .atomic_update = vop_plane_atomic_update, > .atomic_disable = vop_plane_atomic_disable, -- Ｍark Yao