From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754057AbcDYIcA (ORCPT ); Mon, 25 Apr 2016 04:32:00 -0400 Received: from mail-wm0-f43.google.com ([74.125.82.43]:38820 "EHLO mail-wm0-f43.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753860AbcDYIb6 (ORCPT ); Mon, 25 Apr 2016 04:31:58 -0400 Date: Mon, 25 Apr 2016 10:31:54 +0200 From: Daniel Vetter To: Emil Velikov Cc: Noralf =?iso-8859-1?Q?Tr=F8nnes?= , ML dri-devel , linux-fbdev , Laurent Pinchart , Tomi Valkeinen , "Linux-Kernel@Vger. Kernel. Org" Subject: Re: [PATCH 2/8] drm/udl: Change drm_fb_helper_sys_*() calls to sys_*() Message-ID: <20160425083154.GJ2510@phenom.ffwll.local> Mail-Followup-To: Emil Velikov , Noralf =?iso-8859-1?Q?Tr=F8nnes?= , ML dri-devel , linux-fbdev , Laurent Pinchart , Tomi Valkeinen , "Linux-Kernel@Vger. Kernel. Org" References: <1461165929-11344-1-git-send-email-noralf@tronnes.org> <1461165929-11344-3-git-send-email-noralf@tronnes.org> <20160420174241.GP2510@phenom.ffwll.local> <5717C742.7080807@tronnes.org> <20160421072847.GW2510@phenom.ffwll.local> <57191988.3020904@tronnes.org> <20160422082415.GG2510@phenom.ffwll.local> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: X-Operating-System: Linux phenom 4.4.0-1-amd64 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 On Sun, Apr 24, 2016 at 11:16:34AM +0100, Emil Velikov wrote: > On 22 April 2016 at 09:24, Daniel Vetter wrote: > > On Thu, Apr 21, 2016 at 08:18:48PM +0200, Noralf Trønnes wrote: > >> > >> Den 21.04.2016 09:28, skrev Daniel Vetter: > >> >On Wed, Apr 20, 2016 at 08:15:30PM +0200, Noralf Trønnes wrote: > >> >>Den 20.04.2016 19:42, skrev Daniel Vetter: > >> >>>On Wed, Apr 20, 2016 at 05:25:23PM +0200, Noralf Trønnes wrote: > >> >>>>Now that drm_fb_helper gets deferred io support, the > >> >>>>drm_fb_helper_sys_{fillrect,copyarea,imageblit} functions will schedule > >> >>>>the worker that calls the deferred_io callback. This will break this > >> >>>>driver so use the sys_{fillrect,copyarea,imageblit} functions directly. > >> >>>> > >> >>>>Signed-off-by: Noralf Trønnes > >> >>>I think this intermediately breaks the build, if you disable fbdev > >> >>>support. That's now supported in the fbdev helpers core generically across > >> >>>all drivers. > >> >>> > >> >>>Not sure how to best fix this up, since the only way would be to squash > >> >>>these patches, plus generic deferred io plus the conversion patches for > >> >>>udl/qxl all into one. Tricky. > >> >>Yes you're right, I missed that. > >> >>How about this: > >> >>#ifdef CONFIG_FB > >> >> sys_fillrect(info, rect); > >> >>#endif > >> >> > >> >>The later patch will then remove this ugliness... > >> >Yeah I think we have to bite the bullet and take this temporary ugliness > >> >:( > >> > >> Turns out the #ifdef isn't necessary since FB is always selected. > >> > >> Both udl and qxl have this: > >> select DRM_KMS_HELPER > >> select DRM_KMS_FB_HELPER > >> > >> And then we have: > >> > >> config DRM_KMS_HELPER > >> tristate > >> depends on DRM > >> > >> config DRM_KMS_FB_HELPER > >> bool > >> depends on DRM_KMS_HELPER > >> select FB > >> ... > >> select FB_SYS_FILLRECT > >> select FB_SYS_COPYAREA > >> select FB_SYS_IMAGEBLIT > > > > Hm ... the thing that actually builds fbdev emulation is > > DRM_FBDEV_EMULATION, and you can disable that. Otoh the select FB stuff > > seems to be at the wrong level and probably should be moved. > > > > But indeed I tried doing this and it's an impossible config. I guess I > > need to type a patch to ditch all these selects from drivers ;-) > That's a great idea imho. You're volunteered? ;-) > Note that some drivers still partially use FB directly - radeon comes > to mind. Last time I've looked there was no particular reason for > that, as relevant fb_helper already exists. > In general I'm wondering if one cannot check-in a new cocci script > with each function that get factored out. Even if they don't produce > nice (enough) patches they will serve as a nice warning/sanity check, > right ? > > If anyone is wondering "Why should we bother?", mostly because people > (too) often base their work before said functionality/helper has > landed. There's also that we cannot catch everything during review ;-) Maybe we could even piggy-pack on top of the cocci project. They maintain a large repo of cocci scripts somewhere, we could use those. Otoh the big work is in looking at fallout and acting upon it, which needs someone with copious amounts of free time. I think that's the hard part. Wrt old stuff slipping through review: Generally when someone does a subsystem wide refactor that kind of stuff gets caught again. So I'm not too worried. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Date: Mon, 25 Apr 2016 08:31:54 +0000 Subject: Re: [PATCH 2/8] drm/udl: Change drm_fb_helper_sys_*() calls to sys_*() Message-Id: <20160425083154.GJ2510@phenom.ffwll.local> List-Id: References: <1461165929-11344-1-git-send-email-noralf@tronnes.org> <1461165929-11344-3-git-send-email-noralf@tronnes.org> <20160420174241.GP2510@phenom.ffwll.local> <5717C742.7080807@tronnes.org> <20160421072847.GW2510@phenom.ffwll.local> <57191988.3020904@tronnes.org> <20160422082415.GG2510@phenom.ffwll.local> In-Reply-To: MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable To: Emil Velikov Cc: linux-fbdev , "Linux-Kernel@Vger. Kernel. Org" , ML dri-devel , Tomi Valkeinen , Laurent Pinchart On Sun, Apr 24, 2016 at 11:16:34AM +0100, Emil Velikov wrote: > On 22 April 2016 at 09:24, Daniel Vetter wrote: > > On Thu, Apr 21, 2016 at 08:18:48PM +0200, Noralf Tr=F8nnes wrote: > >> > >> Den 21.04.2016 09:28, skrev Daniel Vetter: > >> >On Wed, Apr 20, 2016 at 08:15:30PM +0200, Noralf Tr=F8nnes wrote: > >> >>Den 20.04.2016 19:42, skrev Daniel Vetter: > >> >>>On Wed, Apr 20, 2016 at 05:25:23PM +0200, Noralf Tr=F8nnes wrote: > >> >>>>Now that drm_fb_helper gets deferred io support, the > >> >>>>drm_fb_helper_sys_{fillrect,copyarea,imageblit} functions will sch= edule > >> >>>>the worker that calls the deferred_io callback. This will break th= is > >> >>>>driver so use the sys_{fillrect,copyarea,imageblit} functions dire= ctly. > >> >>>> > >> >>>>Signed-off-by: Noralf Tr=F8nnes > >> >>>I think this intermediately breaks the build, if you disable fbdev > >> >>>support. That's now supported in the fbdev helpers core generically= across > >> >>>all drivers. > >> >>> > >> >>>Not sure how to best fix this up, since the only way would be to sq= uash > >> >>>these patches, plus generic deferred io plus the conversion patches= for > >> >>>udl/qxl all into one. Tricky. > >> >>Yes you're right, I missed that. > >> >>How about this: > >> >>#ifdef CONFIG_FB > >> >> sys_fillrect(info, rect); > >> >>#endif > >> >> > >> >>The later patch will then remove this ugliness... > >> >Yeah I think we have to bite the bullet and take this temporary uglin= ess > >> >:( > >> > >> Turns out the #ifdef isn't necessary since FB is always selected. > >> > >> Both udl and qxl have this: > >> select DRM_KMS_HELPER > >> select DRM_KMS_FB_HELPER > >> > >> And then we have: > >> > >> config DRM_KMS_HELPER > >> tristate > >> depends on DRM > >> > >> config DRM_KMS_FB_HELPER > >> bool > >> depends on DRM_KMS_HELPER > >> select FB > >> ... > >> select FB_SYS_FILLRECT > >> select FB_SYS_COPYAREA > >> select FB_SYS_IMAGEBLIT > > > > Hm ... the thing that actually builds fbdev emulation is > > DRM_FBDEV_EMULATION, and you can disable that. Otoh the select FB stuff > > seems to be at the wrong level and probably should be moved. > > > > But indeed I tried doing this and it's an impossible config. I guess I > > need to type a patch to ditch all these selects from drivers ;-) > That's a great idea imho. You're volunteered? ;-) > Note that some drivers still partially use FB directly - radeon comes > to mind. Last time I've looked there was no particular reason for > that, as relevant fb_helper already exists. > In general I'm wondering if one cannot check-in a new cocci script > with each function that get factored out. Even if they don't produce > nice (enough) patches they will serve as a nice warning/sanity check, > right ? >=20 > If anyone is wondering "Why should we bother?", mostly because people > (too) often base their work before said functionality/helper has > landed. There's also that we cannot catch everything during review ;-) Maybe we could even piggy-pack on top of the cocci project. They maintain a large repo of cocci scripts somewhere, we could use those. Otoh the big work is in looking at fallout and acting upon it, which needs someone with copious amounts of free time. I think that's the hard part. Wrt old stuff slipping through review: Generally when someone does a subsystem wide refactor that kind of stuff gets caught again. So I'm not too worried. -Daniel --=20 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 2/8] drm/udl: Change drm_fb_helper_sys_*() calls to sys_*() Date: Mon, 25 Apr 2016 10:31:54 +0200 Message-ID: <20160425083154.GJ2510@phenom.ffwll.local> References: <1461165929-11344-1-git-send-email-noralf@tronnes.org> <1461165929-11344-3-git-send-email-noralf@tronnes.org> <20160420174241.GP2510@phenom.ffwll.local> <5717C742.7080807@tronnes.org> <20160421072847.GW2510@phenom.ffwll.local> <57191988.3020904@tronnes.org> <20160422082415.GG2510@phenom.ffwll.local> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Received: from mail-wm0-x22a.google.com (mail-wm0-x22a.google.com [IPv6:2a00:1450:400c:c09::22a]) by gabe.freedesktop.org (Postfix) with ESMTPS id 3707C6E021 for ; Mon, 25 Apr 2016 08:31:59 +0000 (UTC) Received: by mail-wm0-x22a.google.com with SMTP id n3so115051609wmn.0 for ; Mon, 25 Apr 2016 01:31:59 -0700 (PDT) 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: Emil Velikov Cc: linux-fbdev , "Linux-Kernel@Vger. Kernel. Org" , ML dri-devel , Tomi Valkeinen , Laurent Pinchart List-Id: dri-devel@lists.freedesktop.org T24gU3VuLCBBcHIgMjQsIDIwMTYgYXQgMTE6MTY6MzRBTSArMDEwMCwgRW1pbCBWZWxpa292IHdy b3RlOgo+IE9uIDIyIEFwcmlsIDIwMTYgYXQgMDk6MjQsIERhbmllbCBWZXR0ZXIgPGRhbmllbEBm ZndsbC5jaD4gd3JvdGU6Cj4gPiBPbiBUaHUsIEFwciAyMSwgMjAxNiBhdCAwODoxODo0OFBNICsw MjAwLCBOb3JhbGYgVHLDuG5uZXMgd3JvdGU6Cj4gPj4KPiA+PiBEZW4gMjEuMDQuMjAxNiAwOToy OCwgc2tyZXYgRGFuaWVsIFZldHRlcjoKPiA+PiA+T24gV2VkLCBBcHIgMjAsIDIwMTYgYXQgMDg6 MTU6MzBQTSArMDIwMCwgTm9yYWxmIFRyw7hubmVzIHdyb3RlOgo+ID4+ID4+RGVuIDIwLjA0LjIw MTYgMTk6NDIsIHNrcmV2IERhbmllbCBWZXR0ZXI6Cj4gPj4gPj4+T24gV2VkLCBBcHIgMjAsIDIw MTYgYXQgMDU6MjU6MjNQTSArMDIwMCwgTm9yYWxmIFRyw7hubmVzIHdyb3RlOgo+ID4+ID4+Pj5O b3cgdGhhdCBkcm1fZmJfaGVscGVyIGdldHMgZGVmZXJyZWQgaW8gc3VwcG9ydCwgdGhlCj4gPj4g Pj4+PmRybV9mYl9oZWxwZXJfc3lzX3tmaWxscmVjdCxjb3B5YXJlYSxpbWFnZWJsaXR9IGZ1bmN0 aW9ucyB3aWxsIHNjaGVkdWxlCj4gPj4gPj4+PnRoZSB3b3JrZXIgdGhhdCBjYWxscyB0aGUgZGVm ZXJyZWRfaW8gY2FsbGJhY2suIFRoaXMgd2lsbCBicmVhayB0aGlzCj4gPj4gPj4+PmRyaXZlciBz byB1c2UgdGhlIHN5c197ZmlsbHJlY3QsY29weWFyZWEsaW1hZ2VibGl0fSBmdW5jdGlvbnMgZGly ZWN0bHkuCj4gPj4gPj4+Pgo+ID4+ID4+Pj5TaWduZWQtb2ZmLWJ5OiBOb3JhbGYgVHLDuG5uZXMg PG5vcmFsZkB0cm9ubmVzLm9yZz4KPiA+PiA+Pj5JIHRoaW5rIHRoaXMgaW50ZXJtZWRpYXRlbHkg YnJlYWtzIHRoZSBidWlsZCwgaWYgeW91IGRpc2FibGUgZmJkZXYKPiA+PiA+Pj5zdXBwb3J0LiBU aGF0J3Mgbm93IHN1cHBvcnRlZCBpbiB0aGUgZmJkZXYgaGVscGVycyBjb3JlIGdlbmVyaWNhbGx5 IGFjcm9zcwo+ID4+ID4+PmFsbCBkcml2ZXJzLgo+ID4+ID4+Pgo+ID4+ID4+Pk5vdCBzdXJlIGhv dyB0byBiZXN0IGZpeCB0aGlzIHVwLCBzaW5jZSB0aGUgb25seSB3YXkgd291bGQgYmUgdG8gc3F1 YXNoCj4gPj4gPj4+dGhlc2UgcGF0Y2hlcywgcGx1cyBnZW5lcmljIGRlZmVycmVkIGlvIHBsdXMg dGhlIGNvbnZlcnNpb24gcGF0Y2hlcyBmb3IKPiA+PiA+Pj51ZGwvcXhsIGFsbCBpbnRvIG9uZS4g VHJpY2t5Lgo+ID4+ID4+WWVzIHlvdSdyZSByaWdodCwgSSBtaXNzZWQgdGhhdC4KPiA+PiA+Pkhv dyBhYm91dCB0aGlzOgo+ID4+ID4+I2lmZGVmIENPTkZJR19GQgo+ID4+ID4+ICAgICAgICAgc3lz X2ZpbGxyZWN0KGluZm8sIHJlY3QpOwo+ID4+ID4+I2VuZGlmCj4gPj4gPj4KPiA+PiA+PlRoZSBs YXRlciBwYXRjaCB3aWxsIHRoZW4gcmVtb3ZlIHRoaXMgdWdsaW5lc3MuLi4KPiA+PiA+WWVhaCBJ IHRoaW5rIHdlIGhhdmUgdG8gYml0ZSB0aGUgYnVsbGV0IGFuZCB0YWtlIHRoaXMgdGVtcG9yYXJ5 IHVnbGluZXNzCj4gPj4gPjooCj4gPj4KPiA+PiBUdXJucyBvdXQgdGhlICNpZmRlZiBpc24ndCBu ZWNlc3Nhcnkgc2luY2UgRkIgaXMgYWx3YXlzIHNlbGVjdGVkLgo+ID4+Cj4gPj4gQm90aCB1ZGwg YW5kIHF4bCBoYXZlIHRoaXM6Cj4gPj4gICAgICAgICBzZWxlY3QgRFJNX0tNU19IRUxQRVIKPiA+ PiAgICAgICAgIHNlbGVjdCBEUk1fS01TX0ZCX0hFTFBFUgo+ID4+Cj4gPj4gQW5kIHRoZW4gd2Ug aGF2ZToKPiA+Pgo+ID4+IGNvbmZpZyBEUk1fS01TX0hFTFBFUgo+ID4+ICAgICAgICAgdHJpc3Rh dGUKPiA+PiAgICAgICAgIGRlcGVuZHMgb24gRFJNCj4gPj4KPiA+PiBjb25maWcgRFJNX0tNU19G Ql9IRUxQRVIKPiA+PiAgICAgICAgIGJvb2wKPiA+PiAgICAgICAgIGRlcGVuZHMgb24gRFJNX0tN U19IRUxQRVIKPiA+PiAgICAgICAgIHNlbGVjdCBGQgo+ID4+ICAgICAgICAgLi4uCj4gPj4gICAg ICAgICBzZWxlY3QgRkJfU1lTX0ZJTExSRUNUCj4gPj4gICAgICAgICBzZWxlY3QgRkJfU1lTX0NP UFlBUkVBCj4gPj4gICAgICAgICBzZWxlY3QgRkJfU1lTX0lNQUdFQkxJVAo+ID4KPiA+IEhtIC4u LiB0aGUgdGhpbmcgdGhhdCBhY3R1YWxseSBidWlsZHMgZmJkZXYgZW11bGF0aW9uIGlzCj4gPiBE Uk1fRkJERVZfRU1VTEFUSU9OLCBhbmQgeW91IGNhbiBkaXNhYmxlIHRoYXQuIE90b2ggdGhlIHNl bGVjdCBGQiBzdHVmZgo+ID4gc2VlbXMgdG8gYmUgYXQgdGhlIHdyb25nIGxldmVsIGFuZCBwcm9i YWJseSBzaG91bGQgYmUgbW92ZWQuCj4gPgo+ID4gQnV0IGluZGVlZCBJIHRyaWVkIGRvaW5nIHRo aXMgYW5kIGl0J3MgYW4gaW1wb3NzaWJsZSBjb25maWcuIEkgZ3Vlc3MgSQo+ID4gbmVlZCB0byB0 eXBlIGEgcGF0Y2ggdG8gZGl0Y2ggYWxsIHRoZXNlIHNlbGVjdHMgZnJvbSBkcml2ZXJzIDstKQo+ IFRoYXQncyBhIGdyZWF0IGlkZWEgaW1oby4KCllvdSdyZSB2b2x1bnRlZXJlZD8gOy0pCgo+IE5v dGUgdGhhdCBzb21lIGRyaXZlcnMgc3RpbGwgcGFydGlhbGx5IHVzZSBGQiBkaXJlY3RseSAtIHJh ZGVvbiBjb21lcwo+IHRvIG1pbmQuIExhc3QgdGltZSBJJ3ZlIGxvb2tlZCB0aGVyZSB3YXMgbm8g cGFydGljdWxhciByZWFzb24gZm9yCj4gdGhhdCwgYXMgcmVsZXZhbnQgZmJfaGVscGVyIGFscmVh ZHkgZXhpc3RzLgo+IEluIGdlbmVyYWwgSSdtIHdvbmRlcmluZyBpZiBvbmUgY2Fubm90IGNoZWNr LWluIGEgbmV3IGNvY2NpIHNjcmlwdAo+IHdpdGggZWFjaCBmdW5jdGlvbiB0aGF0IGdldCBmYWN0 b3JlZCBvdXQuIEV2ZW4gaWYgdGhleSBkb24ndCBwcm9kdWNlCj4gbmljZSAoZW5vdWdoKSBwYXRj aGVzIHRoZXkgd2lsbCBzZXJ2ZSBhcyBhIG5pY2Ugd2FybmluZy9zYW5pdHkgY2hlY2ssCj4gcmln aHQgPwo+IAo+IElmIGFueW9uZSBpcyB3b25kZXJpbmcgIldoeSBzaG91bGQgd2UgYm90aGVyPyIs IG1vc3RseSBiZWNhdXNlIHBlb3BsZQo+ICh0b28pIG9mdGVuIGJhc2UgdGhlaXIgd29yayBiZWZv cmUgc2FpZCBmdW5jdGlvbmFsaXR5L2hlbHBlciBoYXMKPiBsYW5kZWQuIFRoZXJlJ3MgYWxzbyB0 aGF0IHdlIGNhbm5vdCBjYXRjaCBldmVyeXRoaW5nIGR1cmluZyByZXZpZXcgOy0pCgpNYXliZSB3 ZSBjb3VsZCBldmVuIHBpZ2d5LXBhY2sgb24gdG9wIG9mIHRoZSBjb2NjaSBwcm9qZWN0LiBUaGV5 IG1haW50YWluCmEgbGFyZ2UgcmVwbyBvZiBjb2NjaSBzY3JpcHRzIHNvbWV3aGVyZSwgd2UgY291 bGQgdXNlIHRob3NlLiBPdG9oIHRoZSBiaWcKd29yayBpcyBpbiBsb29raW5nIGF0IGZhbGxvdXQg YW5kIGFjdGluZyB1cG9uIGl0LCB3aGljaCBuZWVkcyBzb21lb25lIHdpdGgKY29waW91cyBhbW91 bnRzIG9mIGZyZWUgdGltZS4gSSB0aGluayB0aGF0J3MgdGhlIGhhcmQgcGFydC4KCldydCBvbGQg c3R1ZmYgc2xpcHBpbmcgdGhyb3VnaCByZXZpZXc6IEdlbmVyYWxseSB3aGVuIHNvbWVvbmUgZG9l cyBhCnN1YnN5c3RlbSB3aWRlIHJlZmFjdG9yIHRoYXQga2luZCBvZiBzdHVmZiBnZXRzIGNhdWdo dCBhZ2Fpbi4gU28gSSdtIG5vdAp0b28gd29ycmllZC4KLURhbmllbAotLSAKRGFuaWVsIFZldHRl cgpTb2Z0d2FyZSBFbmdpbmVlciwgSW50ZWwgQ29ycG9yYXRpb24KaHR0cDovL2Jsb2cuZmZ3bGwu Y2gKX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KZHJpLWRl dmVsIG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8v bGlzdHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vZHJpLWRldmVsCg==