From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH 5/9] drm/exynos/crtc: fix framebuffer reference sequence Date: Fri, 12 Sep 2014 10:57:10 +0200 Message-ID: <20140912085710.GD4740@phenom.ffwll.local> References: <1410268573-2297-1-git-send-email-a.hajda@samsung.com> <1410268573-2297-6-git-send-email-a.hajda@samsung.com> <5412B02A.6040609@samsung.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Content-Disposition: inline In-Reply-To: <5412B02A.6040609@samsung.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Inki Dae Cc: "moderated list:ARM/S5P EXYNOS AR..." , Seung-Woo Kim , open list , dri-devel@lists.freedesktop.org, Andrzej Hajda , Kyungmin Park , Marek Szyprowski List-Id: linux-samsung-soc@vger.kernel.org T24gRnJpLCBTZXAgMTIsIDIwMTQgYXQgMDU6MzQ6NTBQTSArMDkwMCwgSW5raSBEYWUgd3JvdGU6 Cj4gSGkgQW5kcnplaiwKPiAKPiBPbiAyMDE064WEIDA57JuUIDA57J28IDIyOjE2LCBBbmRyemVq IEhhamRhIHdyb3RlOgo+ID4gQWRkaW5nIHJlZmVyZW5jZSB0byBmcmFtZWJ1ZmZlciBzaG91bGQg YmUgYWNjb21wYW5pZWQgd2l0aCByZW1vdmluZwo+ID4gcmVmZXJlbmNlIHRvIG9sZCBmcmFtZWJ1 ZmZlciBhc3NpZ25lZCB0byB0aGUgcGxhbmUuCj4gPiBUaGlzIHBhdGNoIHJlbW92ZXMgZm9sbG93 aW5nIHdhcm5pbmc6Cj4gPiAKPiA+IFsgICA5NS4wMzgwMTddIFdBUk5JTkc6IENQVTogMSBQSUQ6 IDMwNjcgYXQgZHJpdmVycy9ncHUvZHJtL2RybV9jcnRjLmM6NTExNSBkcm1fbW9kZV9jb25maWdf Y2xlYW51cCsweDI1OC8weDI2OCgpCj4gPiBbICAgOTUuMDQ4MDg2XSBNb2R1bGVzIGxpbmtlZCBp bjoKPiA+IFsgICA5NS4wNTE0MzBdIENQVTogMSBQSUQ6IDMwNjcgQ29tbTogYmFzaCBUYWludGVk OiBHICAgICAgICBXICAgICAgMy4xNi4wLTExMzU1LWc3YTZlY2E1LWRpcnR5ICMzMDE1Cj4gPiBb ICAgOTUuMDYwMDU4XSBbPGMwMDEzZTkwPl0gKHVud2luZF9iYWNrdHJhY2UpIGZyb20gWzxjMDAx MTEyOD5dIChzaG93X3N0YWNrKzB4MTAvMHgxNCkKPiA+IFsgICA5NS4wNjc3NjZdIFs8YzAwMTEx Mjg+XSAoc2hvd19zdGFjaykgZnJvbSBbPGMwNGE1ZGM0Pl0gKGR1bXBfc3RhY2srMHg3MC8weGJj KQo+ID4gWyAgIDk1LjA3NDk1M10gWzxjMDRhNWRjND5dIChkdW1wX3N0YWNrKSBmcm9tIFs8YzAw MjE3ODQ+XSAod2Fybl9zbG93cGF0aF9jb21tb24rMHg2NC8weDg4KQo+ID4gWyAgIDk1LjA4MzAw NV0gWzxjMDAyMTc4ND5dICh3YXJuX3Nsb3dwYXRoX2NvbW1vbikgZnJvbSBbPGMwMDIxN2M0Pl0g KHdhcm5fc2xvd3BhdGhfbnVsbCsweDFjLzB4MjQpCj4gPiBbICAgOTUuMDkxNzgwXSBbPGMwMDIx N2M0Pl0gKHdhcm5fc2xvd3BhdGhfbnVsbCkgZnJvbSBbPGMwMjc1ZmEwPl0gKGRybV9tb2RlX2Nv bmZpZ19jbGVhbnVwKzB4MjU4LzB4MjY4KQo+ID4gWyAgIDk1LjEwMDk4M10gWzxjMDI3NWZhMD5d IChkcm1fbW9kZV9jb25maWdfY2xlYW51cCkgZnJvbSBbPGMwMjgxNzI0Pl0gKGV4eW5vc19kcm1f dW5sb2FkKzB4MzgvMHg1MCkKPiA+IFsgICA5NS4xMDk5MTVdIFs8YzAyODE3MjQ+XSAoZXh5bm9z X2RybV91bmxvYWQpIGZyb20gWzxjMDI2ZTI0OD5dIChkcm1fZGV2X3VucmVnaXN0ZXIrMHgyNC8w eDk4KQo+ID4gWyAgIDk1LjExODQxNF0gWzxjMDI2ZTI0OD5dIChkcm1fZGV2X3VucmVnaXN0ZXIp IGZyb20gWzxjMDI2ZWNiMD5dIChkcm1fcHV0X2RldisweDI4LzB4NjQpCj4gPiBbICAgOTUuMTI2 NDEyXSBbPGMwMjZlY2IwPl0gKGRybV9wdXRfZGV2KSBmcm9tIFs8YzAyOTQ3YzQ+XSAodGFrZV9k b3duX21hc3RlcisweDI0LzB4NDQpCj4gPiBbICAgOTUuMTM0MjE4XSBbPGMwMjk0N2M0Pl0gKHRh a2VfZG93bl9tYXN0ZXIpIGZyb20gWzxjMDI5NDg3MD5dIChjb21wb25lbnRfZGVsKzB4OGMvMHhj OCkKPiA+IFsgICA5NS4xNDIyMDFdIFs8YzAyOTQ4NzA+XSAoY29tcG9uZW50X2RlbCkgZnJvbSBb PGMwMjg2YzEwPl0gKGV4eW5vc19kc2lfcmVtb3ZlKzB4MTgvMHgyYykKPiA+IFsgICA5NS4xNTAy OTRdIFs8YzAyODZjMTA+XSAoZXh5bm9zX2RzaV9yZW1vdmUpIGZyb20gWzxjMDI5OWUwND5dIChw bGF0Zm9ybV9kcnZfcmVtb3ZlKzB4MTgvMHgxYykKPiA+IFsgICA5NS4xNTg4NzJdIFs8YzAyOTll MDQ+XSAocGxhdGZvcm1fZHJ2X3JlbW92ZSkgZnJvbSBbPGMwMjk4N2E0Pl0gKF9fZGV2aWNlX3Jl bGVhc2VfZHJpdmVyKzB4NzAvMHhjNCkKPiA+IFsgICA5NS4xNjc5ODFdIFs8YzAyOTg3YTQ+XSAo X19kZXZpY2VfcmVsZWFzZV9kcml2ZXIpIGZyb20gWzxjMDI5ODgxOD5dIChkZXZpY2VfcmVsZWFz ZV9kcml2ZXIrMHgyMC8weDJjKQo+ID4gWyAgIDk1LjE3NzI2OF0gWzxjMDI5ODgxOD5dIChkZXZp Y2VfcmVsZWFzZV9kcml2ZXIpIGZyb20gWzxjMDI5NzlkMD5dICh1bmJpbmRfc3RvcmUrMHg1Yy8w eDk0KQo+ID4gWyAgIDk1LjE4NTU5N10gWzxjMDI5NzlkMD5dICh1bmJpbmRfc3RvcmUpIGZyb20g WzxjMDI5NzI3OD5dIChkcnZfYXR0cl9zdG9yZSsweDIwLzB4MmMpCj4gPiBbICAgOTUuMTkzMzIz XSBbPGMwMjk3Mjc4Pl0gKGRydl9hdHRyX3N0b3JlKSBmcm9tIFs8YzAxMmFhOTA+XSAoc3lzZnNf a2Zfd3JpdGUrMHg0Yy8weDUwKQo+ID4gWyAgIDk1LjIwMTIyNF0gWzxjMDEyYWE5MD5dIChzeXNm c19rZl93cml0ZSkgZnJvbSBbPGMwMTI5ZTk0Pl0gKGtlcm5mc19mb3Bfd3JpdGUrMHhjNC8weDE4 NCkKPiA+IFsgICA5NS4yMDkzOTNdIFs8YzAxMjllOTQ+XSAoa2VybmZzX2ZvcF93cml0ZSkgZnJv bSBbPGMwMGQwM2VjPl0gKHZmc193cml0ZSsweGEwLzB4MWE4KQo+ID4gWyAgIDk1LjIxNzExMV0g WzxjMDBkMDNlYz5dICh2ZnNfd3JpdGUpIGZyb20gWzxjMDBkMDdmOD5dIChTeVNfd3JpdGUrMHg0 MC8weDhjKQo+ID4gWyAgIDk1LjIyNDE0Nl0gWzxjMDBkMDdmOD5dIChTeVNfd3JpdGUpIGZyb20g WzxjMDAwZTY2MD5dIChyZXRfZmFzdF9zeXNjYWxsKzB4MC8weDQ4KQo+ID4gCj4gPiBTaWduZWQt b2ZmLWJ5OiBBbmRyemVqIEhhamRhIDxhLmhhamRhQHNhbXN1bmcuY29tPgo+ID4gLS0tCj4gPiAg ZHJpdmVycy9ncHUvZHJtL2V4eW5vcy9leHlub3NfZHJtX2NydGMuYyB8IDYgKysrKysrCj4gPiAg MSBmaWxlIGNoYW5nZWQsIDYgaW5zZXJ0aW9ucygrKQo+ID4gCj4gPiBkaWZmIC0tZ2l0IGEvZHJp dmVycy9ncHUvZHJtL2V4eW5vcy9leHlub3NfZHJtX2NydGMuYyBiL2RyaXZlcnMvZ3B1L2RybS9l eHlub3MvZXh5bm9zX2RybV9jcnRjLmMKPiA+IGluZGV4IGI2OGU1OGYuLmJkZTE5ZjQgMTAwNjQ0 Cj4gPiAtLS0gYS9kcml2ZXJzL2dwdS9kcm0vZXh5bm9zL2V4eW5vc19kcm1fY3J0Yy5jCj4gPiAr KysgYi9kcml2ZXJzL2dwdS9kcm0vZXh5bm9zL2V4eW5vc19kcm1fY3J0Yy5jCj4gPiBAQCAtMTQ1 LDEwICsxNDUsMTYgQEAgZXh5bm9zX2RybV9jcnRjX21vZGVfc2V0KHN0cnVjdCBkcm1fY3J0YyAq Y3J0Yywgc3RydWN0IGRybV9kaXNwbGF5X21vZGUgKm1vZGUsCj4gPiAgCWlmIChyZXQpCj4gPiAg CQlyZXR1cm4gcmV0Owo+ID4gIAo+ID4gKwkvKiB3ZSBuZWVkIHRvIHVucmVmZXJlbmNlIGN1cnJl bnQgZmIgYWZ0ZXIgcmVwbGFjaW5nIGl0IHdpdGggbmV3IG9uZSAqLwo+ID4gKwlvbGRfZmIgPSBw bGFuZS0+ZmI7Cj4gPiArCj4gPiAgCXBsYW5lLT5jcnRjID0gY3J0YzsKPiA+ICAJcGxhbmUtPmZi ID0gY3J0Yy0+cHJpbWFyeS0+ZmI7Cj4gPiAgCWRybV9mcmFtZWJ1ZmZlcl9yZWZlcmVuY2UocGxh bmUtPmZiKTsKPiA+ICAKPiA+ICsJaWYgKG9sZF9mYikKPiA+ICsJCWRybV9mcmFtZWJ1ZmZlcl91 bnJlZmVyZW5jZShvbGRfZmIpOwo+IAo+IFRoaXMgdGltZSB3b3VsZCBiZSBhIGdvb2QgY2hhbmNl IHRoYXQgd2UgY2FuIGNvbnNpZGVyIGRybSBmbGlwIHF1ZXVlIHRvCj4gbWFrZSBzdXJlIHRoYXQg d2hvbGUgbWVtb3J5IHJlZ2lvbiB0byBvbGRfZmIgaXMgc2Nhbm5lZCBvdXQgY29tcGxldGVseQo+ IGJlZm9yZSBkcm9wcGluZyBhIHJlZmVyZW5jZSBvZiBvbGRfZmIuIHRoZSByZWZlcmVuY2Ugb2Yg b2xkX2ZiIHNob3VsZCBiZQo+IGRyb3BwZWQgYXQgaXJxIGhhbmRsZXIgb2YgZWFjaCBjcnRjIGRl dmljZXMsIGZpbWQgYW5kIG1peGVyLgoKR2VuZXJhbGx5IGl0J3Mgbm90IGEgZ29vZCBpZGVhIHRv IGRyb3AgZmIgcmVmZXJlbmNlcyBmcm9tIGlycSBjb250ZXh0LApzaW5jZSBpZiB5b3UgYWN0dWFs bHkgZHJvcCB0aGUgbGFzdCByZWZlcmVuY2UgaXQnbGwgYmxvdyB1cDogZmIgY2xlYW51cApuZWVk cyBhIGJ1bmNoIG9mIG11dGV4ZXMuCgpBbHNvIHRoZSBkcm0gY29yZSByZWFsbHkgc2hvdWxkIGJl IHRha2luZyBjYXJlIG9mIHRoaXMgZm9yIHlvdSwgeW91IG9ubHkKbmVlZCB0byBncmFiIHJlZmVy ZW5jZXMgeW91cnNlbGYgZm9yIGFzeW5jIGZsaXBzIGlmIHlvdSB3YW50IHRoZSBidWZmZXIgdG8K c3Vydml2ZSBhIGJpdC4gY3J0Y19tb2RlX3NldCBoYXMgbm90IG5lZWQgZm9yIHRoaXMuIEkgZXhw ZWN0IHRoYXQgdGhlCnJlZmNvdW50aW5nIGJ1ZyBpcyBzb21ld2hlcmUgZWxzZSwgYXQgbGVhc3Qg ZnJvbSBteSBleHBlcmllbmNlIGNoYXNpbmcKc3VjaCBpc3N1ZXMgaW4gaTkxNSA7LSkKLURhbmll bAotLSAKRGFuaWVsIFZldHRlcgpTb2Z0d2FyZSBFbmdpbmVlciwgSW50ZWwgQ29ycG9yYXRpb24K KzQxICgwKSA3OSAzNjUgNTcgNDggLSBodHRwOi8vYmxvZy5mZndsbC5jaApfX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fXwpkcmktZGV2ZWwgbWFpbGluZyBsaXN0 CmRyaS1kZXZlbEBsaXN0cy5mcmVlZGVza3RvcC5vcmcKaHR0cDovL2xpc3RzLmZyZWVkZXNrdG9w Lm9yZy9tYWlsbWFuL2xpc3RpbmZvL2RyaS1kZXZlbAo= From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752947AbaILI4u (ORCPT ); Fri, 12 Sep 2014 04:56:50 -0400 Received: from mail-wg0-f46.google.com ([74.125.82.46]:38872 "EHLO mail-wg0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751282AbaILI4q (ORCPT ); Fri, 12 Sep 2014 04:56:46 -0400 Date: Fri, 12 Sep 2014 10:57:10 +0200 From: Daniel Vetter To: Inki Dae Cc: Andrzej Hajda , "moderated list:ARM/S5P EXYNOS AR..." , Seung-Woo Kim , open list , dri-devel@lists.freedesktop.org, Kyungmin Park , Marek Szyprowski Subject: Re: [PATCH 5/9] drm/exynos/crtc: fix framebuffer reference sequence Message-ID: <20140912085710.GD4740@phenom.ffwll.local> Mail-Followup-To: Inki Dae , Andrzej Hajda , "moderated list:ARM/S5P EXYNOS AR..." , Seung-Woo Kim , open list , dri-devel@lists.freedesktop.org, Kyungmin Park , Marek Szyprowski References: <1410268573-2297-1-git-send-email-a.hajda@samsung.com> <1410268573-2297-6-git-send-email-a.hajda@samsung.com> <5412B02A.6040609@samsung.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <5412B02A.6040609@samsung.com> X-Operating-System: Linux phenom 3.15.0-rc3+ User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, Sep 12, 2014 at 05:34:50PM +0900, Inki Dae wrote: > Hi Andrzej, > > On 2014년 09월 09일 22:16, Andrzej Hajda wrote: > > Adding reference to framebuffer should be accompanied with removing > > reference to old framebuffer assigned to the plane. > > This patch removes following warning: > > > > [ 95.038017] WARNING: CPU: 1 PID: 3067 at drivers/gpu/drm/drm_crtc.c:5115 drm_mode_config_cleanup+0x258/0x268() > > [ 95.048086] Modules linked in: > > [ 95.051430] CPU: 1 PID: 3067 Comm: bash Tainted: G W 3.16.0-11355-g7a6eca5-dirty #3015 > > [ 95.060058] [] (unwind_backtrace) from [] (show_stack+0x10/0x14) > > [ 95.067766] [] (show_stack) from [] (dump_stack+0x70/0xbc) > > [ 95.074953] [] (dump_stack) from [] (warn_slowpath_common+0x64/0x88) > > [ 95.083005] [] (warn_slowpath_common) from [] (warn_slowpath_null+0x1c/0x24) > > [ 95.091780] [] (warn_slowpath_null) from [] (drm_mode_config_cleanup+0x258/0x268) > > [ 95.100983] [] (drm_mode_config_cleanup) from [] (exynos_drm_unload+0x38/0x50) > > [ 95.109915] [] (exynos_drm_unload) from [] (drm_dev_unregister+0x24/0x98) > > [ 95.118414] [] (drm_dev_unregister) from [] (drm_put_dev+0x28/0x64) > > [ 95.126412] [] (drm_put_dev) from [] (take_down_master+0x24/0x44) > > [ 95.134218] [] (take_down_master) from [] (component_del+0x8c/0xc8) > > [ 95.142201] [] (component_del) from [] (exynos_dsi_remove+0x18/0x2c) > > [ 95.150294] [] (exynos_dsi_remove) from [] (platform_drv_remove+0x18/0x1c) > > [ 95.158872] [] (platform_drv_remove) from [] (__device_release_driver+0x70/0xc4) > > [ 95.167981] [] (__device_release_driver) from [] (device_release_driver+0x20/0x2c) > > [ 95.177268] [] (device_release_driver) from [] (unbind_store+0x5c/0x94) > > [ 95.185597] [] (unbind_store) from [] (drv_attr_store+0x20/0x2c) > > [ 95.193323] [] (drv_attr_store) from [] (sysfs_kf_write+0x4c/0x50) > > [ 95.201224] [] (sysfs_kf_write) from [] (kernfs_fop_write+0xc4/0x184) > > [ 95.209393] [] (kernfs_fop_write) from [] (vfs_write+0xa0/0x1a8) > > [ 95.217111] [] (vfs_write) from [] (SyS_write+0x40/0x8c) > > [ 95.224146] [] (SyS_write) from [] (ret_fast_syscall+0x0/0x48) > > > > Signed-off-by: Andrzej Hajda > > --- > > drivers/gpu/drm/exynos/exynos_drm_crtc.c | 6 ++++++ > > 1 file changed, 6 insertions(+) > > > > diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.c b/drivers/gpu/drm/exynos/exynos_drm_crtc.c > > index b68e58f..bde19f4 100644 > > --- a/drivers/gpu/drm/exynos/exynos_drm_crtc.c > > +++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.c > > @@ -145,10 +145,16 @@ exynos_drm_crtc_mode_set(struct drm_crtc *crtc, struct drm_display_mode *mode, > > if (ret) > > return ret; > > > > + /* we need to unreference current fb after replacing it with new one */ > > + old_fb = plane->fb; > > + > > plane->crtc = crtc; > > plane->fb = crtc->primary->fb; > > drm_framebuffer_reference(plane->fb); > > > > + if (old_fb) > > + drm_framebuffer_unreference(old_fb); > > This time would be a good chance that we can consider drm flip queue to > make sure that whole memory region to old_fb is scanned out completely > before dropping a reference of old_fb. the reference of old_fb should be > dropped at irq handler of each crtc devices, fimd and mixer. Generally it's not a good idea to drop fb references from irq context, since if you actually drop the last reference it'll blow up: fb cleanup needs a bunch of mutexes. Also the drm core really should be taking care of this for you, you only need to grab references yourself for async flips if you want the buffer to survive a bit. crtc_mode_set has not need for this. I expect that the refcounting bug is somewhere else, at least from my experience chasing such issues in i915 ;-) -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch