From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754118AbdBITGR (ORCPT ); Thu, 9 Feb 2017 14:06:17 -0500 Received: from mail-wm0-f65.google.com ([74.125.82.65]:36204 "EHLO mail-wm0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752301AbdBITGM (ORCPT ); Thu, 9 Feb 2017 14:06:12 -0500 Date: Thu, 9 Feb 2017 20:06:08 +0100 From: Daniel Vetter To: Daniel Stone Cc: Maxime Ripard , Daniel Vetter , David Airlie , Jani Nikula , Sean Paul , Stefan Christ , Linux Kernel Mailing List , dri-devel Subject: Re: [PATCH v2 2/2] drm/fb_helper: implement ioctl FBIO_WAITFORVSYNC Message-ID: <20170209190608.lrogobvi4bhg2blp@phenom.ffwll.local> Mail-Followup-To: Daniel Stone , Maxime Ripard , Daniel Vetter , David Airlie , Jani Nikula , Sean Paul , Stefan Christ , Linux Kernel Mailing List , dri-devel References: <20170209170111.mmp7bpjy6qfl5kow@phenom.ffwll.local> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Operating-System: Linux phenom 4.8.0-1-amd64 User-Agent: NeoMutt/20161126 (1.7.1) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Feb 09, 2017 at 05:38:26PM +0000, Daniel Stone wrote: > Hi, > > On 9 February 2017 at 17:01, Daniel Vetter wrote: > > On Thu, Feb 02, 2017 at 11:31:57AM +0100, Maxime Ripard wrote: > >> +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; > >> + unsigned int i; > >> + int ret = 0; > >> + > >> + drm_modeset_lock_all(dev); > >> + if (!drm_fb_helper_is_bound(fb_helper)) { > >> + drm_modeset_unlock_all(dev); > >> + return -EBUSY; > >> + } > >> + > >> + switch (cmd) { > >> + case FBIO_WAITFORVSYNC: > >> + for (i = 0; i < fb_helper->crtc_count; i++) { > >> + struct drm_mode_set *mode_set; > >> + struct drm_crtc *crtc; > >> + > >> + mode_set = &fb_helper->crtc_info[i].mode_set; > >> + crtc = mode_set->crtc; > >> + > >> + /* > >> + * Only call drm_crtc_wait_one_vblank for crtcs that > >> + * are currently enabled. > >> + */ > >> + if (!crtc->enabled) > >> + continue; > > > > This needs locking > > More locking than the drm_modeset_lock_all() above the switch? Ouch. :) Oh, I didn't spot that one. Well, that one should be removed imo, we dont want any vblank wait path to hold these locks, it looks bad. Otoh we still need to grab the dev->mode_config.mutex, because that's also the fbdev lock. There's a plan somewhere to give the fbdev helpers their own lock, so switching to just mode_config.mutex + the changes I suggestd would be good. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH v2 2/2] drm/fb_helper: implement ioctl FBIO_WAITFORVSYNC Date: Thu, 9 Feb 2017 20:06:08 +0100 Message-ID: <20170209190608.lrogobvi4bhg2blp@phenom.ffwll.local> References: <20170209170111.mmp7bpjy6qfl5kow@phenom.ffwll.local> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Received: from mail-wm0-x244.google.com (mail-wm0-x244.google.com [IPv6:2a00:1450:400c:c09::244]) by gabe.freedesktop.org (Postfix) with ESMTPS id BA2416EB90 for ; Thu, 9 Feb 2017 19:06:12 +0000 (UTC) Received: by mail-wm0-x244.google.com with SMTP id v77so4272726wmv.0 for ; Thu, 09 Feb 2017 11:06:12 -0800 (PST) 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: Daniel Stone Cc: Linux Kernel Mailing List , dri-devel , Daniel Vetter , Maxime Ripard , Stefan Christ List-Id: dri-devel@lists.freedesktop.org T24gVGh1LCBGZWIgMDksIDIwMTcgYXQgMDU6Mzg6MjZQTSArMDAwMCwgRGFuaWVsIFN0b25lIHdy b3RlOgo+IEhpLAo+IAo+IE9uIDkgRmVicnVhcnkgMjAxNyBhdCAxNzowMSwgRGFuaWVsIFZldHRl ciA8ZGFuaWVsQGZmd2xsLmNoPiB3cm90ZToKPiA+IE9uIFRodSwgRmViIDAyLCAyMDE3IGF0IDEx OjMxOjU3QU0gKzAxMDAsIE1heGltZSBSaXBhcmQgd3JvdGU6Cj4gPj4gK2ludCBkcm1fZmJfaGVs cGVyX2lvY3RsKHN0cnVjdCBmYl9pbmZvICppbmZvLCB1bnNpZ25lZCBpbnQgY21kLCB1bnNpZ25l ZCBsb25nIGFyZykKPiA+PiArewo+ID4+ICsgICAgIHN0cnVjdCBkcm1fZmJfaGVscGVyICpmYl9o ZWxwZXIgPSBpbmZvLT5wYXI7Cj4gPj4gKyAgICAgc3RydWN0IGRybV9kZXZpY2UgKmRldiA9IGZi X2hlbHBlci0+ZGV2Owo+ID4+ICsgICAgIHVuc2lnbmVkIGludCBpOwo+ID4+ICsgICAgIGludCBy ZXQgPSAwOwo+ID4+ICsKPiA+PiArICAgICBkcm1fbW9kZXNldF9sb2NrX2FsbChkZXYpOwo+ID4+ ICsgICAgIGlmICghZHJtX2ZiX2hlbHBlcl9pc19ib3VuZChmYl9oZWxwZXIpKSB7Cj4gPj4gKyAg ICAgICAgICAgICBkcm1fbW9kZXNldF91bmxvY2tfYWxsKGRldik7Cj4gPj4gKyAgICAgICAgICAg ICByZXR1cm4gLUVCVVNZOwo+ID4+ICsgICAgIH0KPiA+PiArCj4gPj4gKyAgICAgc3dpdGNoIChj bWQpIHsKPiA+PiArICAgICBjYXNlIEZCSU9fV0FJVEZPUlZTWU5DOgo+ID4+ICsgICAgICAgICAg ICAgZm9yIChpID0gMDsgaSA8IGZiX2hlbHBlci0+Y3J0Y19jb3VudDsgaSsrKSB7Cj4gPj4gKyAg ICAgICAgICAgICAgICAgICAgIHN0cnVjdCBkcm1fbW9kZV9zZXQgKm1vZGVfc2V0Owo+ID4+ICsg ICAgICAgICAgICAgICAgICAgICBzdHJ1Y3QgZHJtX2NydGMgKmNydGM7Cj4gPj4gKwo+ID4+ICsg ICAgICAgICAgICAgICAgICAgICBtb2RlX3NldCA9ICZmYl9oZWxwZXItPmNydGNfaW5mb1tpXS5t b2RlX3NldDsKPiA+PiArICAgICAgICAgICAgICAgICAgICAgY3J0YyA9IG1vZGVfc2V0LT5jcnRj Owo+ID4+ICsKPiA+PiArICAgICAgICAgICAgICAgICAgICAgLyoKPiA+PiArICAgICAgICAgICAg ICAgICAgICAgICogT25seSBjYWxsIGRybV9jcnRjX3dhaXRfb25lX3ZibGFuayBmb3IgY3J0Y3Mg dGhhdAo+ID4+ICsgICAgICAgICAgICAgICAgICAgICAgKiBhcmUgY3VycmVudGx5IGVuYWJsZWQu Cj4gPj4gKyAgICAgICAgICAgICAgICAgICAgICAqLwo+ID4+ICsgICAgICAgICAgICAgICAgICAg ICBpZiAoIWNydGMtPmVuYWJsZWQpCj4gPj4gKyAgICAgICAgICAgICAgICAgICAgICAgICAgICAg Y29udGludWU7Cj4gPgo+ID4gVGhpcyBuZWVkcyBsb2NraW5nCj4gCj4gTW9yZSBsb2NraW5nIHRo YW4gdGhlIGRybV9tb2Rlc2V0X2xvY2tfYWxsKCkgYWJvdmUgdGhlIHN3aXRjaD8gT3VjaC4gOikK Ck9oLCBJIGRpZG4ndCBzcG90IHRoYXQgb25lLiBXZWxsLCB0aGF0IG9uZSBzaG91bGQgYmUgcmVt b3ZlZCBpbW8sIHdlIGRvbnQKd2FudCBhbnkgdmJsYW5rIHdhaXQgcGF0aCB0byBob2xkIHRoZXNl IGxvY2tzLCBpdCBsb29rcyBiYWQuIE90b2ggd2Ugc3RpbGwKbmVlZCB0byBncmFiIHRoZSBkZXYt Pm1vZGVfY29uZmlnLm11dGV4LCBiZWNhdXNlIHRoYXQncyBhbHNvIHRoZSBmYmRldgpsb2NrLiBU aGVyZSdzIGEgcGxhbiBzb21ld2hlcmUgdG8gZ2l2ZSB0aGUgZmJkZXYgaGVscGVycyB0aGVpciBv d24gbG9jaywKc28gc3dpdGNoaW5nIHRvIGp1c3QgbW9kZV9jb25maWcubXV0ZXggKyB0aGUgY2hh bmdlcyBJIHN1Z2dlc3RkIHdvdWxkIGJlCmdvb2QuCi1EYW5pZWwKLS0gCkRhbmllbCBWZXR0ZXIK U29mdHdhcmUgRW5naW5lZXIsIEludGVsIENvcnBvcmF0aW9uCmh0dHA6Ly9ibG9nLmZmd2xsLmNo Cl9fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fCmRyaS1kZXZl bCBtYWlsaW5nIGxpc3QKZHJpLWRldmVsQGxpc3RzLmZyZWVkZXNrdG9wLm9yZwpodHRwczovL2xp c3RzLmZyZWVkZXNrdG9wLm9yZy9tYWlsbWFuL2xpc3RpbmZvL2RyaS1kZXZlbAo=