From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751720AbdBUKBQ (ORCPT ); Tue, 21 Feb 2017 05:01:16 -0500 Received: from stcim.de ([78.46.73.102]:51419 "EHLO stcim.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750992AbdBUKBH (ORCPT ); Tue, 21 Feb 2017 05:01:07 -0500 Date: Tue, 21 Feb 2017 11:00:59 +0100 From: Stefan Lengfeld To: Maxime Ripard Cc: Daniel Vetter , David Airlie , Sean Paul , Jani Nikula , linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org Subject: Re: [PATCH v3 2/2] drm/fb-helper: implement ioctl FBIO_WAITFORVSYNC Message-ID: <20170221100059.GD17643@sill.h.stcim.de> References: MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: X-PGP-Key: https://stefanchrist.eu/personal/Stefan_Lengfeld_0xE44A23B289092311.asc User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Maxime, On Wed, Feb 15, 2017 at 05:19:09PM +0100, Maxime Ripard wrote: > From: Stefan Christ > Maybe change the author here. Only the boilerplate code looks my original patch. The real code is your work ;-) > Implement legacy framebuffer ioctl FBIO_WAITFORVSYNC in the generic > framebuffer emulation driver. Legacy framebuffer users like non kms/drm > based OpenGL(ES)/EGL implementations may require the ioctl to > synchronize drawing or buffer flip for double buffering. It is tested on > the i.MX6. > > Code is based on > https://github.com/Xilinx/linux-xlnx/blob/master/drivers/gpu/drm/xilinx/xilinx_drm_fb.c#L196 > > Signed-off-by: Stefan Christ > Signed-off-by: Maxime Ripard > --- > drivers/gpu/drm/drm_fb_helper.c | 63 ++++++++++++++++++++++++++++++++++- > include/drm/drm_fb_helper.h | 12 +++++- > 2 files changed, 74 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c > index c6de87abaca8..15ee9641c725 100644 > --- a/drivers/gpu/drm/drm_fb_helper.c > +++ b/drivers/gpu/drm/drm_fb_helper.c > @@ -1240,6 +1240,69 @@ int drm_fb_helper_setcmap(struct fb_cmap *cmap, struct fb_info *info) > EXPORT_SYMBOL(drm_fb_helper_setcmap); > > /** > + * drm_fb_helper_ioctl - legacy ioctl implementation > + * @info: fbdev registered by the helper > + * @cmd: ioctl command > + * @arg: ioctl argument > + * > + * A helper to implement the standard fbdev ioctl. Only > + * FBIO_WAITFORVSYNC is implemented for now. > + */ > +int drm_fb_helper_ioctl(struct fb_info *info, unsigned int cmd, > + unsigned long arg) > +{ > + struct drm_fb_helper *fb_helper = info->par; > + struct drm_device *dev = fb_helper->dev; > + struct drm_mode_set *mode_set; > + struct drm_crtc *crtc; > + u32 karg; > + int ret = 0; > + > + mutex_lock(&dev->mode_config.mutex); > + if (!drm_fb_helper_is_bound(fb_helper)) { > + ret = -EBUSY; > + goto unlock; > + } > + > + switch (cmd) { > + case FBIO_WAITFORVSYNC: > + if (get_user(karg, (__u32 __user *)arg)) { > + ret = -EFAULT; > + goto unlock; > + } > + > + if (karg >= fb_helper->crtc_count) { > + ret = -EINVAL; > + goto unlock; > + } Ville Syrjälä said [1]: FBIO_WAITFORVSYNC takes the crtc as a parmeter, so I'm not sure we want to do this for all the crtcs. Though what that crtc means for fb is rather poorly defined. Don't think it takes the crtc as a parameter in 'arg'. When you look at the existing fb_ioctl implementations in the directory drivers/video/fbdev/, the argument 'arg' is either ignored or must be '0'. Furthmore most exiting userspace code just passes the value "0" as the argument. For example DirectFB: static const int zero = 0; [...] if (ioctl( dfb_fbdev->fd, FBIO_WAITFORVSYNC, &zero )) I see two options here: 1/ Just wait for the first vsync event on the first enabled crtc. This is a good approximation for the old framebuffer API. It has no concept of multiple crtcs with concurrenctly running scan out processes. (Maybe apart from extra vendor implementations). So if there are really more than one active crtcs in the system, bad luck with framebuffer API. There will be tearing. Userspace has to use the new DRM/KMS API. 2/ Wait for a single vsync event on all active crtcs as Ville Syrjälä described here [2] in the optimal implementation. Kind regards / Mit freundlichen Grüßen Stefan Lengfeld [1] https://lists.freedesktop.org/archives/dri-devel/2017-February/132617.html [2] https://lists.freedesktop.org/archives/dri-devel/2017-February/132820.html From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stefan Lengfeld Subject: Re: [PATCH v3 2/2] drm/fb-helper: implement ioctl FBIO_WAITFORVSYNC Date: Tue, 21 Feb 2017 11:00:59 +0100 Message-ID: <20170221100059.GD17643@sill.h.stcim.de> References: Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Received: from stcim.de (stcim.de [IPv6:2a01:4f8:120:4065::2]) by gabe.freedesktop.org (Postfix) with ESMTPS id D1A846E60C for ; Tue, 21 Feb 2017 10:01:09 +0000 (UTC) Content-Disposition: inline In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Maxime Ripard Cc: linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, Daniel Vetter List-Id: dri-devel@lists.freedesktop.org SGkgTWF4aW1lLAoKT24gV2VkLCBGZWIgMTUsIDIwMTcgYXQgMDU6MTk6MDlQTSArMDEwMCwgTWF4 aW1lIFJpcGFyZCB3cm90ZToKPiBGcm9tOiBTdGVmYW4gQ2hyaXN0IDxzLmNocmlzdEBwaHl0ZWMu ZGU+Cj4gCgpNYXliZSBjaGFuZ2UgdGhlIGF1dGhvciBoZXJlLiBPbmx5IHRoZSBib2lsZXJwbGF0 ZSBjb2RlIGxvb2tzIG15IG9yaWdpbmFsCnBhdGNoLiBUaGUgcmVhbCBjb2RlIGlzIHlvdXIgd29y ayA7LSkKCj4gSW1wbGVtZW50IGxlZ2FjeSBmcmFtZWJ1ZmZlciBpb2N0bCBGQklPX1dBSVRGT1JW U1lOQyBpbiB0aGUgZ2VuZXJpYwo+IGZyYW1lYnVmZmVyIGVtdWxhdGlvbiBkcml2ZXIuIExlZ2Fj eSBmcmFtZWJ1ZmZlciB1c2VycyBsaWtlIG5vbiBrbXMvZHJtCj4gYmFzZWQgT3BlbkdMKEVTKS9F R0wgaW1wbGVtZW50YXRpb25zIG1heSByZXF1aXJlIHRoZSBpb2N0bCB0bwo+IHN5bmNocm9uaXpl IGRyYXdpbmcgb3IgYnVmZmVyIGZsaXAgZm9yIGRvdWJsZSBidWZmZXJpbmcuIEl0IGlzIHRlc3Rl ZCBvbgo+IHRoZSBpLk1YNi4KPiAKPiBDb2RlIGlzIGJhc2VkIG9uCj4gICAgIGh0dHBzOi8vZ2l0 aHViLmNvbS9YaWxpbngvbGludXgteGxueC9ibG9iL21hc3Rlci9kcml2ZXJzL2dwdS9kcm0veGls aW54L3hpbGlueF9kcm1fZmIuYyNMMTk2Cj4gCj4gU2lnbmVkLW9mZi1ieTogU3RlZmFuIENocmlz dCA8cy5jaHJpc3RAcGh5dGVjLmRlPgo+IFNpZ25lZC1vZmYtYnk6IE1heGltZSBSaXBhcmQgPG1h eGltZS5yaXBhcmRAZnJlZS1lbGVjdHJvbnMuY29tPgo+IC0tLQo+ICBkcml2ZXJzL2dwdS9kcm0v ZHJtX2ZiX2hlbHBlci5jIHwgNjMgKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKy0K PiAgaW5jbHVkZS9kcm0vZHJtX2ZiX2hlbHBlci5oICAgICB8IDEyICsrKysrLQo+ICAyIGZpbGVz IGNoYW5nZWQsIDc0IGluc2VydGlvbnMoKyksIDEgZGVsZXRpb24oLSkKPiAKPiBkaWZmIC0tZ2l0 IGEvZHJpdmVycy9ncHUvZHJtL2RybV9mYl9oZWxwZXIuYyBiL2RyaXZlcnMvZ3B1L2RybS9kcm1f ZmJfaGVscGVyLmMKPiBpbmRleCBjNmRlODdhYmFjYTguLjE1ZWU5NjQxYzcyNSAxMDA2NDQKPiAt LS0gYS9kcml2ZXJzL2dwdS9kcm0vZHJtX2ZiX2hlbHBlci5jCj4gKysrIGIvZHJpdmVycy9ncHUv ZHJtL2RybV9mYl9oZWxwZXIuYwo+IEBAIC0xMjQwLDYgKzEyNDAsNjkgQEAgaW50IGRybV9mYl9o ZWxwZXJfc2V0Y21hcChzdHJ1Y3QgZmJfY21hcCAqY21hcCwgc3RydWN0IGZiX2luZm8gKmluZm8p Cj4gIEVYUE9SVF9TWU1CT0woZHJtX2ZiX2hlbHBlcl9zZXRjbWFwKTsKPiAgCj4gIC8qKgo+ICsg KiBkcm1fZmJfaGVscGVyX2lvY3RsIC0gbGVnYWN5IGlvY3RsIGltcGxlbWVudGF0aW9uCj4gKyAq IEBpbmZvOiBmYmRldiByZWdpc3RlcmVkIGJ5IHRoZSBoZWxwZXIKPiArICogQGNtZDogaW9jdGwg Y29tbWFuZAo+ICsgKiBAYXJnOiBpb2N0bCBhcmd1bWVudAo+ICsgKgo+ICsgKiBBIGhlbHBlciB0 byBpbXBsZW1lbnQgdGhlIHN0YW5kYXJkIGZiZGV2IGlvY3RsLiBPbmx5Cj4gKyAqIEZCSU9fV0FJ VEZPUlZTWU5DIGlzIGltcGxlbWVudGVkIGZvciBub3cuCj4gKyAqLwo+ICtpbnQgZHJtX2ZiX2hl bHBlcl9pb2N0bChzdHJ1Y3QgZmJfaW5mbyAqaW5mbywgdW5zaWduZWQgaW50IGNtZCwKPiArCQkJ dW5zaWduZWQgbG9uZyBhcmcpCj4gK3sKPiArCXN0cnVjdCBkcm1fZmJfaGVscGVyICpmYl9oZWxw ZXIgPSBpbmZvLT5wYXI7Cj4gKwlzdHJ1Y3QgZHJtX2RldmljZSAqZGV2ID0gZmJfaGVscGVyLT5k ZXY7Cj4gKwlzdHJ1Y3QgZHJtX21vZGVfc2V0ICptb2RlX3NldDsKPiArCXN0cnVjdCBkcm1fY3J0 YyAqY3J0YzsKPiArCXUzMiBrYXJnOwo+ICsJaW50IHJldCA9IDA7Cj4gKwo+ICsJbXV0ZXhfbG9j aygmZGV2LT5tb2RlX2NvbmZpZy5tdXRleCk7Cj4gKwlpZiAoIWRybV9mYl9oZWxwZXJfaXNfYm91 bmQoZmJfaGVscGVyKSkgewo+ICsJCXJldCA9IC1FQlVTWTsKPiArCQlnb3RvIHVubG9jazsKPiAr CX0KPiArCj4gKwlzd2l0Y2ggKGNtZCkgewo+ICsJY2FzZSBGQklPX1dBSVRGT1JWU1lOQzoKPiAr CQlpZiAoZ2V0X3VzZXIoa2FyZywgKF9fdTMyIF9fdXNlciAqKWFyZykpIHsKPiArCQkJcmV0ID0g LUVGQVVMVDsKPiArCQkJZ290byB1bmxvY2s7Cj4gKwkJfQo+ICsKPiArCQlpZiAoa2FyZyA+PSBm Yl9oZWxwZXItPmNydGNfY291bnQpIHsKPiArCQkJcmV0ID0gLUVJTlZBTDsKPiArCQkJZ290byB1 bmxvY2s7Cj4gKwkJfQoKVmlsbGUgU3lyasOkbMOkIHNhaWQgWzFdOgoKICAgIEZCSU9fV0FJVEZP UlZTWU5DIHRha2VzIHRoZSBjcnRjIGFzIGEgcGFybWV0ZXIsIHNvIEknbSBub3Qgc3VyZSB3ZSB3 YW50CiAgICB0byBkbyB0aGlzIGZvciBhbGwgdGhlIGNydGNzLiBUaG91Z2ggd2hhdCB0aGF0IGNy dGMgbWVhbnMgZm9yIGZiIGlzCiAgICByYXRoZXIgcG9vcmx5IGRlZmluZWQuCgpEb24ndCB0aGlu ayBpdCB0YWtlcyB0aGUgY3J0YyBhcyBhIHBhcmFtZXRlciBpbiAnYXJnJy4gV2hlbiB5b3UgbG9v ayBhdCB0aGUKZXhpc3RpbmcgZmJfaW9jdGwgaW1wbGVtZW50YXRpb25zIGluIHRoZSBkaXJlY3Rv cnkgZHJpdmVycy92aWRlby9mYmRldi8sIHRoZQphcmd1bWVudCAnYXJnJyBpcyBlaXRoZXIgaWdu b3JlZCBvciBtdXN0IGJlICcwJy4KCkZ1cnRobW9yZSBtb3N0IGV4aXRpbmcgdXNlcnNwYWNlIGNv ZGUganVzdCBwYXNzZXMgdGhlIHZhbHVlICIwIiBhcyB0aGUKYXJndW1lbnQuIEZvciBleGFtcGxl IERpcmVjdEZCOgoKICAgICBzdGF0aWMgY29uc3QgaW50IHplcm8gPSAwOwogICAgIFsuLi5dCiAg ICAgaWYgKGlvY3RsKCBkZmJfZmJkZXYtPmZkLCBGQklPX1dBSVRGT1JWU1lOQywgJnplcm8gKSkK Ckkgc2VlIHR3byBvcHRpb25zIGhlcmU6IAoKMS8gSnVzdCB3YWl0IGZvciB0aGUgZmlyc3QgdnN5 bmMgZXZlbnQgb24gdGhlIGZpcnN0IGVuYWJsZWQgY3J0Yy4gVGhpcyBpcwogICBhIGdvb2QgYXBw cm94aW1hdGlvbiBmb3IgdGhlIG9sZCBmcmFtZWJ1ZmZlciBBUEkuIEl0IGhhcyBubyBjb25jZXB0 IG9mCiAgIG11bHRpcGxlIGNydGNzIHdpdGggY29uY3VycmVuY3RseSBydW5uaW5nIHNjYW4gb3V0 IHByb2Nlc3Nlcy4gKE1heWJlIGFwYXJ0CiAgIGZyb20gZXh0cmEgdmVuZG9yIGltcGxlbWVudGF0 aW9ucykuIFNvIGlmIHRoZXJlIGFyZSByZWFsbHkgbW9yZSB0aGFuIG9uZSBhY3RpdmUKICAgY3J0 Y3MgaW4gdGhlIHN5c3RlbSwgYmFkIGx1Y2sgd2l0aCBmcmFtZWJ1ZmZlciBBUEkuIFRoZXJlIHdp bGwgYmUgdGVhcmluZy4KICAgVXNlcnNwYWNlIGhhcyB0byB1c2UgdGhlIG5ldyBEUk0vS01TIEFQ SS4KCjIvIFdhaXQgZm9yIGEgc2luZ2xlIHZzeW5jIGV2ZW50IG9uIGFsbCBhY3RpdmUgY3J0Y3Mg YXMgVmlsbGUgU3lyasOkbMOkIAogICBkZXNjcmliZWQgaGVyZSBbMl0gaW4gdGhlIG9wdGltYWwg aW1wbGVtZW50YXRpb24uCgpLaW5kIHJlZ2FyZHMgLyBNaXQgZnJldW5kbGljaGVuIEdyw7zDn2Vu CglTdGVmYW4gTGVuZ2ZlbGQKClsxXSBodHRwczovL2xpc3RzLmZyZWVkZXNrdG9wLm9yZy9hcmNo aXZlcy9kcmktZGV2ZWwvMjAxNy1GZWJydWFyeS8xMzI2MTcuaHRtbApbMl0gaHR0cHM6Ly9saXN0 cy5mcmVlZGVza3RvcC5vcmcvYXJjaGl2ZXMvZHJpLWRldmVsLzIwMTctRmVicnVhcnkvMTMyODIw Lmh0bWwKX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KZHJp LWRldmVsIG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBz Oi8vbGlzdHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vZHJpLWRldmVsCg==