From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752575AbcDXKQj (ORCPT ); Sun, 24 Apr 2016 06:16:39 -0400 Received: from mail-wm0-f46.google.com ([74.125.82.46]:37440 "EHLO mail-wm0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752444AbcDXKQg convert rfc822-to-8bit (ORCPT ); Sun, 24 Apr 2016 06:16:36 -0400 MIME-Version: 1.0 In-Reply-To: <20160422082415.GG2510@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> Date: Sun, 24 Apr 2016 11:16:34 +0100 Message-ID: Subject: Re: [PATCH 2/8] drm/udl: Change drm_fb_helper_sys_*() calls to sys_*() From: Emil Velikov To: =?UTF-8?Q?Noralf_Tr=C3=B8nnes?= , ML dri-devel , linux-fbdev , Laurent Pinchart , Tomi Valkeinen , "Linux-Kernel@Vger. Kernel. Org" Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8BIT Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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. 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 ;-) Regards, Emil From mboxrd@z Thu Jan 1 00:00:00 1970 From: Emil Velikov Date: Sun, 24 Apr 2016 10:16:34 +0000 Subject: Re: [PATCH 2/8] drm/udl: Change drm_fb_helper_sys_*() calls to sys_*() Message-Id: 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: <20160422082415.GG2510@phenom.ffwll.local> MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8bit To: =?UTF-8?Q?Noralf_Tr=C3=B8nnes?= , ML dri-devel , linux-fbdev , Laurent Pinchart , Tomi Valkeinen , "Linux-Kernel@Vger. Kernel. Org" 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. 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 ;-) Regards, Emil From mboxrd@z Thu Jan 1 00:00:00 1970 From: Emil Velikov Subject: Re: [PATCH 2/8] drm/udl: Change drm_fb_helper_sys_*() calls to sys_*() Date: Sun, 24 Apr 2016 11:16:34 +0100 Message-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> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Received: from mail-wm0-x22d.google.com (mail-wm0-x22d.google.com [IPv6:2a00:1450:400c:c09::22d]) by gabe.freedesktop.org (Postfix) with ESMTPS id 8EF366E1CF for ; Sun, 24 Apr 2016 10:16:36 +0000 (UTC) Received: by mail-wm0-x22d.google.com with SMTP id u206so85718638wme.1 for ; Sun, 24 Apr 2016 03:16:36 -0700 (PDT) In-Reply-To: <20160422082415.GG2510@phenom.ffwll.local> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: =?UTF-8?Q?Noralf_Tr=C3=B8nnes?= , ML dri-devel , linux-fbdev , Laurent Pinchart , Tomi Valkeinen , "Linux-Kernel@Vger. Kernel. Org" List-Id: dri-devel@lists.freedesktop.org T24gMjIgQXByaWwgMjAxNiBhdCAwOToyNCwgRGFuaWVsIFZldHRlciA8ZGFuaWVsQGZmd2xsLmNo PiB3cm90ZToKPiBPbiBUaHUsIEFwciAyMSwgMjAxNiBhdCAwODoxODo0OFBNICswMjAwLCBOb3Jh bGYgVHLDuG5uZXMgd3JvdGU6Cj4+Cj4+IERlbiAyMS4wNC4yMDE2IDA5OjI4LCBza3JldiBEYW5p ZWwgVmV0dGVyOgo+PiA+T24gV2VkLCBBcHIgMjAsIDIwMTYgYXQgMDg6MTU6MzBQTSArMDIwMCwg Tm9yYWxmIFRyw7hubmVzIHdyb3RlOgo+PiA+PkRlbiAyMC4wNC4yMDE2IDE5OjQyLCBza3JldiBE YW5pZWwgVmV0dGVyOgo+PiA+Pj5PbiBXZWQsIEFwciAyMCwgMjAxNiBhdCAwNToyNToyM1BNICsw MjAwLCBOb3JhbGYgVHLDuG5uZXMgd3JvdGU6Cj4+ID4+Pj5Ob3cgdGhhdCBkcm1fZmJfaGVscGVy IGdldHMgZGVmZXJyZWQgaW8gc3VwcG9ydCwgdGhlCj4+ID4+Pj5kcm1fZmJfaGVscGVyX3N5c197 ZmlsbHJlY3QsY29weWFyZWEsaW1hZ2VibGl0fSBmdW5jdGlvbnMgd2lsbCBzY2hlZHVsZQo+PiA+ Pj4+dGhlIHdvcmtlciB0aGF0IGNhbGxzIHRoZSBkZWZlcnJlZF9pbyBjYWxsYmFjay4gVGhpcyB3 aWxsIGJyZWFrIHRoaXMKPj4gPj4+PmRyaXZlciBzbyB1c2UgdGhlIHN5c197ZmlsbHJlY3QsY29w eWFyZWEsaW1hZ2VibGl0fSBmdW5jdGlvbnMgZGlyZWN0bHkuCj4+ID4+Pj4KPj4gPj4+PlNpZ25l ZC1vZmYtYnk6IE5vcmFsZiBUcsO4bm5lcyA8bm9yYWxmQHRyb25uZXMub3JnPgo+PiA+Pj5JIHRo aW5rIHRoaXMgaW50ZXJtZWRpYXRlbHkgYnJlYWtzIHRoZSBidWlsZCwgaWYgeW91IGRpc2FibGUg ZmJkZXYKPj4gPj4+c3VwcG9ydC4gVGhhdCdzIG5vdyBzdXBwb3J0ZWQgaW4gdGhlIGZiZGV2IGhl bHBlcnMgY29yZSBnZW5lcmljYWxseSBhY3Jvc3MKPj4gPj4+YWxsIGRyaXZlcnMuCj4+ID4+Pgo+ PiA+Pj5Ob3Qgc3VyZSBob3cgdG8gYmVzdCBmaXggdGhpcyB1cCwgc2luY2UgdGhlIG9ubHkgd2F5 IHdvdWxkIGJlIHRvIHNxdWFzaAo+PiA+Pj50aGVzZSBwYXRjaGVzLCBwbHVzIGdlbmVyaWMgZGVm ZXJyZWQgaW8gcGx1cyB0aGUgY29udmVyc2lvbiBwYXRjaGVzIGZvcgo+PiA+Pj51ZGwvcXhsIGFs bCBpbnRvIG9uZS4gVHJpY2t5Lgo+PiA+PlllcyB5b3UncmUgcmlnaHQsIEkgbWlzc2VkIHRoYXQu Cj4+ID4+SG93IGFib3V0IHRoaXM6Cj4+ID4+I2lmZGVmIENPTkZJR19GQgo+PiA+PiAgICAgICAg IHN5c19maWxscmVjdChpbmZvLCByZWN0KTsKPj4gPj4jZW5kaWYKPj4gPj4KPj4gPj5UaGUgbGF0 ZXIgcGF0Y2ggd2lsbCB0aGVuIHJlbW92ZSB0aGlzIHVnbGluZXNzLi4uCj4+ID5ZZWFoIEkgdGhp bmsgd2UgaGF2ZSB0byBiaXRlIHRoZSBidWxsZXQgYW5kIHRha2UgdGhpcyB0ZW1wb3JhcnkgdWds aW5lc3MKPj4gPjooCj4+Cj4+IFR1cm5zIG91dCB0aGUgI2lmZGVmIGlzbid0IG5lY2Vzc2FyeSBz aW5jZSBGQiBpcyBhbHdheXMgc2VsZWN0ZWQuCj4+Cj4+IEJvdGggdWRsIGFuZCBxeGwgaGF2ZSB0 aGlzOgo+PiAgICAgICAgIHNlbGVjdCBEUk1fS01TX0hFTFBFUgo+PiAgICAgICAgIHNlbGVjdCBE Uk1fS01TX0ZCX0hFTFBFUgo+Pgo+PiBBbmQgdGhlbiB3ZSBoYXZlOgo+Pgo+PiBjb25maWcgRFJN X0tNU19IRUxQRVIKPj4gICAgICAgICB0cmlzdGF0ZQo+PiAgICAgICAgIGRlcGVuZHMgb24gRFJN Cj4+Cj4+IGNvbmZpZyBEUk1fS01TX0ZCX0hFTFBFUgo+PiAgICAgICAgIGJvb2wKPj4gICAgICAg ICBkZXBlbmRzIG9uIERSTV9LTVNfSEVMUEVSCj4+ICAgICAgICAgc2VsZWN0IEZCCj4+ICAgICAg ICAgLi4uCj4+ICAgICAgICAgc2VsZWN0IEZCX1NZU19GSUxMUkVDVAo+PiAgICAgICAgIHNlbGVj dCBGQl9TWVNfQ09QWUFSRUEKPj4gICAgICAgICBzZWxlY3QgRkJfU1lTX0lNQUdFQkxJVAo+Cj4g SG0gLi4uIHRoZSB0aGluZyB0aGF0IGFjdHVhbGx5IGJ1aWxkcyBmYmRldiBlbXVsYXRpb24gaXMK PiBEUk1fRkJERVZfRU1VTEFUSU9OLCBhbmQgeW91IGNhbiBkaXNhYmxlIHRoYXQuIE90b2ggdGhl IHNlbGVjdCBGQiBzdHVmZgo+IHNlZW1zIHRvIGJlIGF0IHRoZSB3cm9uZyBsZXZlbCBhbmQgcHJv YmFibHkgc2hvdWxkIGJlIG1vdmVkLgo+Cj4gQnV0IGluZGVlZCBJIHRyaWVkIGRvaW5nIHRoaXMg YW5kIGl0J3MgYW4gaW1wb3NzaWJsZSBjb25maWcuIEkgZ3Vlc3MgSQo+IG5lZWQgdG8gdHlwZSBh IHBhdGNoIHRvIGRpdGNoIGFsbCB0aGVzZSBzZWxlY3RzIGZyb20gZHJpdmVycyA7LSkKVGhhdCdz IGEgZ3JlYXQgaWRlYSBpbWhvLgoKTm90ZSB0aGF0IHNvbWUgZHJpdmVycyBzdGlsbCBwYXJ0aWFs bHkgdXNlIEZCIGRpcmVjdGx5IC0gcmFkZW9uIGNvbWVzCnRvIG1pbmQuIExhc3QgdGltZSBJJ3Zl IGxvb2tlZCB0aGVyZSB3YXMgbm8gcGFydGljdWxhciByZWFzb24gZm9yCnRoYXQsIGFzIHJlbGV2 YW50IGZiX2hlbHBlciBhbHJlYWR5IGV4aXN0cy4KSW4gZ2VuZXJhbCBJJ20gd29uZGVyaW5nIGlm IG9uZSBjYW5ub3QgY2hlY2staW4gYSBuZXcgY29jY2kgc2NyaXB0CndpdGggZWFjaCBmdW5jdGlv biB0aGF0IGdldCBmYWN0b3JlZCBvdXQuIEV2ZW4gaWYgdGhleSBkb24ndCBwcm9kdWNlCm5pY2Ug KGVub3VnaCkgcGF0Y2hlcyB0aGV5IHdpbGwgc2VydmUgYXMgYSBuaWNlIHdhcm5pbmcvc2FuaXR5 IGNoZWNrLApyaWdodCA/CgpJZiBhbnlvbmUgaXMgd29uZGVyaW5nICJXaHkgc2hvdWxkIHdlIGJv dGhlcj8iLCBtb3N0bHkgYmVjYXVzZSBwZW9wbGUKKHRvbykgb2Z0ZW4gYmFzZSB0aGVpciB3b3Jr IGJlZm9yZSBzYWlkIGZ1bmN0aW9uYWxpdHkvaGVscGVyIGhhcwpsYW5kZWQuIFRoZXJlJ3MgYWxz byB0aGF0IHdlIGNhbm5vdCBjYXRjaCBldmVyeXRoaW5nIGR1cmluZyByZXZpZXcgOy0pCgpSZWdh cmRzLApFbWlsCl9fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f CmRyaS1kZXZlbCBtYWlsaW5nIGxpc3QKZHJpLWRldmVsQGxpc3RzLmZyZWVkZXNrdG9wLm9yZwpo dHRwczovL2xpc3RzLmZyZWVkZXNrdG9wLm9yZy9tYWlsbWFuL2xpc3RpbmZvL2RyaS1kZXZlbAo=