From mboxrd@z Thu Jan 1 00:00:00 1970 From: Mark yao Subject: Re: [PATCH] drm/rockchip: Return -EBUSY if there's already a pending flip event Date: Thu, 31 Mar 2016 09:25:43 +0800 Message-ID: <56FC7C97.8000506@rock-chips.com> References: <1459345705-4473-1-git-send-email-tomeu.vizoso@collabora.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8"; Format="flowed" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: <1459345705-4473-1-git-send-email-tomeu.vizoso@collabora.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Tomeu Vizoso , linux-kernel@vger.kernel.org Cc: linux-rockchip@lists.infradead.org, dri-devel@lists.freedesktop.org, linux-arm-kernel@lists.infradead.org List-Id: linux-rockchip.vger.kernel.org T24gMjAxNuW5tDAz5pyIMzDml6UgMjE6NDgsIFRvbWV1IFZpem9zbyB3cm90ZToKPiBBcyBwZXIg dGhlIGRvY3MsIGF0b21pY19jb21taXQgc2hvdWxkIHJldHVybiAtRUJVU1kgImlmIGFuIGFzeWNu aHJvbm91cwo+IHVwZGF0ZWQgaXMgcmVxdWVzdGVkIGFuZCB0aGVyZSBpcyBhbiBlYXJsaWVyIHVw ZGF0ZWQgcGVuZGluZyIuCj4KPiBBbHNvIHdhaXQgZm9yIHRoZSBwZW5kaW5nIGV2ZW50IHRvIGNv bXBsZXRlIHdoZW4gYSBzeW5jIHVwZGF0ZSBpcwo+IHJlcXVlc3RlZC4KPgo+IFNpZ25lZC1vZmYt Ynk6IFRvbWV1IFZpem9zbyA8dG9tZXUudml6b3NvQGNvbGxhYm9yYS5jb20+Cj4gLS0tCj4gICBk cml2ZXJzL2dwdS9kcm0vcm9ja2NoaXAvcm9ja2NoaXBfZHJtX2Rydi5oIHwgIDEgKwo+ICAgZHJp dmVycy9ncHUvZHJtL3JvY2tjaGlwL3JvY2tjaGlwX2RybV9mYi5jICB8IDE0ICsrKysrKysrKysr KystCj4gICBkcml2ZXJzL2dwdS9kcm0vcm9ja2NoaXAvcm9ja2NoaXBfZHJtX3ZvcC5jIHwgIDYg KysrKysrCj4gICAzIGZpbGVzIGNoYW5nZWQsIDIwIGluc2VydGlvbnMoKyksIDEgZGVsZXRpb24o LSkKPgo+IGRpZmYgLS1naXQgYS9kcml2ZXJzL2dwdS9kcm0vcm9ja2NoaXAvcm9ja2NoaXBfZHJt X2Rydi5oIGIvZHJpdmVycy9ncHUvZHJtL3JvY2tjaGlwL3JvY2tjaGlwX2RybV9kcnYuaAo+IGlu ZGV4IDM1MjlmNjkyZWRiOC4uMzc5NTJlZWZkMzNkIDEwMDY0NAo+IC0tLSBhL2RyaXZlcnMvZ3B1 L2RybS9yb2NrY2hpcC9yb2NrY2hpcF9kcm1fZHJ2LmgKPiArKysgYi9kcml2ZXJzL2dwdS9kcm0v cm9ja2NoaXAvcm9ja2NoaXBfZHJtX2Rydi5oCj4gQEAgLTY5LDYgKzY5LDcgQEAgaW50IHJvY2tj aGlwX3JlZ2lzdGVyX2NydGNfZnVuY3Moc3RydWN0IGRybV9jcnRjICpjcnRjLAo+ICAgdm9pZCBy b2NrY2hpcF91bnJlZ2lzdGVyX2NydGNfZnVuY3Moc3RydWN0IGRybV9jcnRjICpjcnRjKTsKPiAg IGludCByb2NrY2hpcF9kcm1fY3J0Y19tb2RlX2NvbmZpZyhzdHJ1Y3QgZHJtX2NydGMgKmNydGMs IGludCBjb25uZWN0b3JfdHlwZSwKPiAgIAkJCQkgIGludCBvdXRfbW9kZSk7Cj4gK2Jvb2wgcm9j a2NoaXBfZHJtX2NydGNfaGFzX3BlbmRpbmdfZXZlbnQoc3RydWN0IGRybV9jcnRjICpjcnRjKTsK PiAgIGludCByb2NrY2hpcF9kcm1fZG1hX2F0dGFjaF9kZXZpY2Uoc3RydWN0IGRybV9kZXZpY2Ug KmRybV9kZXYsCj4gICAJCQkJICAgc3RydWN0IGRldmljZSAqZGV2KTsKPiAgIHZvaWQgcm9ja2No aXBfZHJtX2RtYV9kZXRhY2hfZGV2aWNlKHN0cnVjdCBkcm1fZGV2aWNlICpkcm1fZGV2LAo+IGRp ZmYgLS1naXQgYS9kcml2ZXJzL2dwdS9kcm0vcm9ja2NoaXAvcm9ja2NoaXBfZHJtX2ZiLmMgYi9k cml2ZXJzL2dwdS9kcm0vcm9ja2NoaXAvcm9ja2NoaXBfZHJtX2ZiLmMKPiBpbmRleCAzYjhmNjUy Njk4ZjguLjAyOGZkMmY4ZjQ3YiAxMDA2NDQKPiAtLS0gYS9kcml2ZXJzL2dwdS9kcm0vcm9ja2No aXAvcm9ja2NoaXBfZHJtX2ZiLmMKPiArKysgYi9kcml2ZXJzL2dwdS9kcm0vcm9ja2NoaXAvcm9j a2NoaXBfZHJtX2ZiLmMKPiBAQCAtMjgwLDcgKzI4MCwxOSBAQCBpbnQgcm9ja2NoaXBfZHJtX2F0 b21pY19jb21taXQoc3RydWN0IGRybV9kZXZpY2UgKmRldiwKPiAgIHsKPiAgIAlzdHJ1Y3Qgcm9j a2NoaXBfZHJtX3ByaXZhdGUgKnByaXZhdGUgPSBkZXYtPmRldl9wcml2YXRlOwo+ICAgCXN0cnVj dCByb2NrY2hpcF9hdG9taWNfY29tbWl0ICpjb21taXQgPSAmcHJpdmF0ZS0+Y29tbWl0Owo+IC0J aW50IHJldDsKPiArCXN0cnVjdCBkcm1fY3J0Y19zdGF0ZSAqY3J0Y19zdGF0ZTsKPiArCXN0cnVj dCBkcm1fY3J0YyAqY3J0YzsKPiArCWludCBpLCByZXQ7Cj4gKwo+ICsJZm9yX2VhY2hfY3J0Y19p bl9zdGF0ZShzdGF0ZSwgY3J0YywgY3J0Y19zdGF0ZSwgaSkgewo+ICsJCWlmIChjcnRjLT5zdGF0 ZS0+ZXZlbnQgfHwKPiArCQkgICAgcm9ja2NoaXBfZHJtX2NydGNfaGFzX3BlbmRpbmdfZXZlbnQo Y3J0YykpIHsKPiArCQkJaWYgKGFzeW5jKQo+ICsJCQkJcmV0dXJuIC1FQlVTWTsKPiArCQkJZWxz ZQo+ICsJCQkJcm9ja2NoaXBfY3J0Y193YWl0X2Zvcl91cGRhdGUoY3J0Yyk7Cgo+ICsJCX0KPiAr CX0KPiAgIAo+ICAgCXJldCA9IGRybV9hdG9taWNfaGVscGVyX3ByZXBhcmVfcGxhbmVzKGRldiwg c3RhdGUpOwo+ICAgCWlmIChyZXQpCj4gZGlmZiAtLWdpdCBhL2RyaXZlcnMvZ3B1L2RybS9yb2Nr Y2hpcC9yb2NrY2hpcF9kcm1fdm9wLmMgYi9kcml2ZXJzL2dwdS9kcm0vcm9ja2NoaXAvcm9ja2No aXBfZHJtX3ZvcC5jCj4gaW5kZXggZTIxMThlNjIzNDViLi41MjQwYmM4ZmUwODggMTAwNjQ0Cj4g LS0tIGEvZHJpdmVycy9ncHUvZHJtL3JvY2tjaGlwL3JvY2tjaGlwX2RybV92b3AuYwo+ICsrKyBi L2RyaXZlcnMvZ3B1L2RybS9yb2NrY2hpcC9yb2NrY2hpcF9kcm1fdm9wLmMKPiBAQCAtODQ4LDYg Kzg0OCwxMiBAQCBpbnQgcm9ja2NoaXBfZHJtX2NydGNfbW9kZV9jb25maWcoc3RydWN0IGRybV9j cnRjICpjcnRjLAo+ICAgfQo+ICAgRVhQT1JUX1NZTUJPTF9HUEwocm9ja2NoaXBfZHJtX2NydGNf bW9kZV9jb25maWcpOwo+ICAgCj4gK2Jvb2wgcm9ja2NoaXBfZHJtX2NydGNfaGFzX3BlbmRpbmdf ZXZlbnQoc3RydWN0IGRybV9jcnRjICpjcnRjKQo+ICt7Cj4gKwlyZXR1cm4gdG9fdm9wKGNydGMp LT5ldmVudDsKPiArfQo+ICtFWFBPUlRfU1lNQk9MX0dQTChyb2NrY2hpcF9kcm1fY3J0Y19oYXNf cGVuZGluZ19ldmVudCk7Cj4gKwoKTm8gbmVlZCB0byBkbyBFWFBPUlRfU1lNQk9MX0dQTCwgcm9j a2NoaXBfZHJtX3ZvcC5jIGFuZCByb2NrY2hpcF9kcm1fZmIuYyB3aWxsIGJ1aWxkIGludG8gc2Ft ZSBvYmplY3QuCgoKPiAgIHN0YXRpYyBpbnQgdm9wX2NydGNfZW5hYmxlX3ZibGFuayhzdHJ1Y3Qg ZHJtX2NydGMgKmNydGMpCj4gICB7Cj4gICAJc3RydWN0IHZvcCAqdm9wID0gdG9fdm9wKGNydGMp OwpIaSBUb21ldQoKU2luY2Ugcm9ja2NoaXAgYXRvbWljIGFzeW5jIHVzZSAic2VyaWFsaXplIG91 dHN0YW5kaW5nIGFzeW5jaHJvbm91cyAKY29tbWl0cyIsIGFzeW5jIGNvbW1pdCB3aWxsIHdhaXQg b24gImZsdXNoX3dvcmsoJmNvbW1pdC0+d29yayk7IiB0byAKZW5zdXJlIGxhc3QgcGVuZGluZyBl dmVudCBpcyBkb25lLCBzbyB2b3AtPmV2ZW50IHNob3VsZCBiZSBOVUxMIGFmdGVyIApmbHVzaC4g aWYgbm90LCBpdCBzaG91bGQgYmUgYSBCVUcuCgpBbnl3YXksIHNlcmlhbGl6ZSBhc3luYyBjb21t aXRzIGlzIG5vdCBhIGdvb2Qgd2F5LCBidXQgSSBkb24ndCB0aGluayAKeW91ciBwYXRjaCBpcyBi ZXR0ZXIgd2F5LgoKSSB0aGluayBtYXliZSB3ZSBjYW4gY2hlY2sgdGhlICJjb21taXQtPndvcmsi IHN0YXR1cyhmaW5pc2ggb3IgcnVubmluZykgCnRvIGp1ZGdlIHJldHVybiBCVVNZIG9yIG5vdCwg aW5zdGVhZCBvZiBmbHVzaCBpdC4KClRoYW5rcy4KCi0tIArvvK1hcmsgWWFvCgoKX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KZHJpLWRldmVsIG1haWxpbmcg bGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlzdHMuZnJlZWRl c2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vZHJpLWRldmVsCg== From mboxrd@z Thu Jan 1 00:00:00 1970 From: mark.yao@rock-chips.com (Mark yao) Date: Thu, 31 Mar 2016 09:25:43 +0800 Subject: [PATCH] drm/rockchip: Return -EBUSY if there's already a pending flip event In-Reply-To: <1459345705-4473-1-git-send-email-tomeu.vizoso@collabora.com> References: <1459345705-4473-1-git-send-email-tomeu.vizoso@collabora.com> Message-ID: <56FC7C97.8000506@rock-chips.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On 2016?03?30? 21:48, Tomeu Vizoso wrote: > As per the docs, atomic_commit should return -EBUSY "if an asycnhronous > updated is requested and there is an earlier updated pending". > > Also wait for the pending event to complete when a sync update is > requested. > > Signed-off-by: Tomeu Vizoso > --- > drivers/gpu/drm/rockchip/rockchip_drm_drv.h | 1 + > drivers/gpu/drm/rockchip/rockchip_drm_fb.c | 14 +++++++++++++- > drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 6 ++++++ > 3 files changed, 20 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h > index 3529f692edb8..37952eefd33d 100644 > --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h > @@ -69,6 +69,7 @@ int rockchip_register_crtc_funcs(struct drm_crtc *crtc, > void rockchip_unregister_crtc_funcs(struct drm_crtc *crtc); > int rockchip_drm_crtc_mode_config(struct drm_crtc *crtc, int connector_type, > int out_mode); > +bool rockchip_drm_crtc_has_pending_event(struct drm_crtc *crtc); > int rockchip_drm_dma_attach_device(struct drm_device *drm_dev, > struct device *dev); > void rockchip_drm_dma_detach_device(struct drm_device *drm_dev, > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c > index 3b8f652698f8..028fd2f8f47b 100644 > --- a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c > @@ -280,7 +280,19 @@ int rockchip_drm_atomic_commit(struct drm_device *dev, > { > struct rockchip_drm_private *private = dev->dev_private; > struct rockchip_atomic_commit *commit = &private->commit; > - int ret; > + struct drm_crtc_state *crtc_state; > + struct drm_crtc *crtc; > + int i, ret; > + > + for_each_crtc_in_state(state, crtc, crtc_state, i) { > + if (crtc->state->event || > + rockchip_drm_crtc_has_pending_event(crtc)) { > + if (async) > + return -EBUSY; > + else > + rockchip_crtc_wait_for_update(crtc); > + } > + } > > ret = drm_atomic_helper_prepare_planes(dev, state); > if (ret) > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c > index e2118e62345b..5240bc8fe088 100644 > --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c > @@ -848,6 +848,12 @@ int rockchip_drm_crtc_mode_config(struct drm_crtc *crtc, > } > EXPORT_SYMBOL_GPL(rockchip_drm_crtc_mode_config); > > +bool rockchip_drm_crtc_has_pending_event(struct drm_crtc *crtc) > +{ > + return to_vop(crtc)->event; > +} > +EXPORT_SYMBOL_GPL(rockchip_drm_crtc_has_pending_event); > + No need to do EXPORT_SYMBOL_GPL, rockchip_drm_vop.c and rockchip_drm_fb.c will build into same object. > static int vop_crtc_enable_vblank(struct drm_crtc *crtc) > { > struct vop *vop = to_vop(crtc); Hi Tomeu Since rockchip atomic async use "serialize outstanding asynchronous commits", async commit will wait on "flush_work(&commit->work);" to ensure last pending event is done, so vop->event should be NULL after flush. if not, it should be a BUG. Anyway, serialize async commits is not a good way, but I don't think your patch is better way. I think maybe we can check the "commit->work" status(finish or running) to judge return BUSY or not, instead of flush it. Thanks. -- ?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 S1754314AbcCaBZw (ORCPT ); Wed, 30 Mar 2016 21:25:52 -0400 Received: from regular1.263xmail.com ([211.150.99.137]:57087 "EHLO regular1.263xmail.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751215AbcCaBZv (ORCPT ); Wed, 30 Mar 2016 21:25:51 -0400 X-263anti-spam: BIG:0;ABS:1;DNS:0;ATT:0;SPF:S;KSV:0; X-MAIL-GRAY: 0 X-MAIL-DELIVERY: 1 X-ABS-CHECKED: 1 X-SKE-CHECKED: 1 X-ADDR-CHECKED: 0 X-KSVirus-check: 0 X-RL-SENDER: mark.yao@rock-chips.com X-FST-TO: linux-rockchip@lists.infradead.org X-SENDER-IP: 58.22.7.114 X-LOGIN-NAME: mark.yao@rock-chips.com X-UNIQUE-TAG: <73a396ceaa7bea60aa90e7645712bda0> X-ATTACHMENT-NUM: 0 X-DNS-TYPE: 0 Message-ID: <56FC7C97.8000506@rock-chips.com> Date: Thu, 31 Mar 2016 09:25:43 +0800 From: Mark yao User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.8.0 MIME-Version: 1.0 To: Tomeu Vizoso , linux-kernel@vger.kernel.org CC: David Airlie , Heiko Stuebner , dri-devel@lists.freedesktop.org, linux-arm-kernel@lists.infradead.org, linux-rockchip@lists.infradead.org Subject: Re: [PATCH] drm/rockchip: Return -EBUSY if there's already a pending flip event References: <1459345705-4473-1-git-send-email-tomeu.vizoso@collabora.com> In-Reply-To: <1459345705-4473-1-git-send-email-tomeu.vizoso@collabora.com> 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年03月30日 21:48, Tomeu Vizoso wrote: > As per the docs, atomic_commit should return -EBUSY "if an asycnhronous > updated is requested and there is an earlier updated pending". > > Also wait for the pending event to complete when a sync update is > requested. > > Signed-off-by: Tomeu Vizoso > --- > drivers/gpu/drm/rockchip/rockchip_drm_drv.h | 1 + > drivers/gpu/drm/rockchip/rockchip_drm_fb.c | 14 +++++++++++++- > drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 6 ++++++ > 3 files changed, 20 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h > index 3529f692edb8..37952eefd33d 100644 > --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h > @@ -69,6 +69,7 @@ int rockchip_register_crtc_funcs(struct drm_crtc *crtc, > void rockchip_unregister_crtc_funcs(struct drm_crtc *crtc); > int rockchip_drm_crtc_mode_config(struct drm_crtc *crtc, int connector_type, > int out_mode); > +bool rockchip_drm_crtc_has_pending_event(struct drm_crtc *crtc); > int rockchip_drm_dma_attach_device(struct drm_device *drm_dev, > struct device *dev); > void rockchip_drm_dma_detach_device(struct drm_device *drm_dev, > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c > index 3b8f652698f8..028fd2f8f47b 100644 > --- a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c > @@ -280,7 +280,19 @@ int rockchip_drm_atomic_commit(struct drm_device *dev, > { > struct rockchip_drm_private *private = dev->dev_private; > struct rockchip_atomic_commit *commit = &private->commit; > - int ret; > + struct drm_crtc_state *crtc_state; > + struct drm_crtc *crtc; > + int i, ret; > + > + for_each_crtc_in_state(state, crtc, crtc_state, i) { > + if (crtc->state->event || > + rockchip_drm_crtc_has_pending_event(crtc)) { > + if (async) > + return -EBUSY; > + else > + rockchip_crtc_wait_for_update(crtc); > + } > + } > > ret = drm_atomic_helper_prepare_planes(dev, state); > if (ret) > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c > index e2118e62345b..5240bc8fe088 100644 > --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c > @@ -848,6 +848,12 @@ int rockchip_drm_crtc_mode_config(struct drm_crtc *crtc, > } > EXPORT_SYMBOL_GPL(rockchip_drm_crtc_mode_config); > > +bool rockchip_drm_crtc_has_pending_event(struct drm_crtc *crtc) > +{ > + return to_vop(crtc)->event; > +} > +EXPORT_SYMBOL_GPL(rockchip_drm_crtc_has_pending_event); > + No need to do EXPORT_SYMBOL_GPL, rockchip_drm_vop.c and rockchip_drm_fb.c will build into same object. > static int vop_crtc_enable_vblank(struct drm_crtc *crtc) > { > struct vop *vop = to_vop(crtc); Hi Tomeu Since rockchip atomic async use "serialize outstanding asynchronous commits", async commit will wait on "flush_work(&commit->work);" to ensure last pending event is done, so vop->event should be NULL after flush. if not, it should be a BUG. Anyway, serialize async commits is not a good way, but I don't think your patch is better way. I think maybe we can check the "commit->work" status(finish or running) to judge return BUSY or not, instead of flush it. Thanks. -- Ｍark Yao