From mboxrd@z Thu Jan 1 00:00:00 1970 From: Archit Taneja Subject: Re: [RFC 1/6] drm: Add top level Kconfig option for DRM fbdev emulation Date: Fri, 13 Mar 2015 11:55:07 +0530 Message-ID: <550282C3.10903@codeaurora.org> References: <1425980493-27533-1-git-send-email-architt@codeaurora.org> <1425980493-27533-2-git-send-email-architt@codeaurora.org> <20150310094756.GE3800@phenom.ffwll.local> <54FEBEF1.5000103@codeaurora.org> <20150310100547.GG3800@phenom.ffwll.local> <54FEC5F1.90208@codeaurora.org> <20150310121702.GO3800@phenom.ffwll.local> <54FFFAEE.3020301@codeaurora.org> <20150311151731.GJ3800@phenom.ffwll.local> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8"; Format="flowed" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: <20150311151731.GJ3800@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: linux-kernel@vger.kernel.org, linux-arm-msm@vger.kernel.org, Jani Nikula , daniel@ffwll.ch Cc: linux-fbdev@vger.kernel.org, dri-devel@lists.freedesktop.org, tomi.valkeinen@ti.com, benjamin.gaignard@linaro.org, treding@nvidia.com List-Id: linux-arm-msm@vger.kernel.org CgpPbiAwMy8xMS8yMDE1IDA4OjQ3IFBNLCBEYW5pZWwgVmV0dGVyIHdyb3RlOgo+IE9uIFdlZCwg TWFyIDExLCAyMDE1IGF0IDAxOjUxOjAyUE0gKzA1MzAsIEFyY2hpdCBUYW5lamEgd3JvdGU6Cj4+ Cj4+Cj4+IE9uIDAzLzEwLzIwMTUgMDU6NDcgUE0sIERhbmllbCBWZXR0ZXIgd3JvdGU6Cj4+PiBP biBUdWUsIE1hciAxMCwgMjAxNSBhdCAwMzo1Mjo0MVBNICswNTMwLCBBcmNoaXQgVGFuZWphIHdy b3RlOgo+Pj4+IE9uIDAzLzEwLzIwMTUgMDM6MzUgUE0sIERhbmllbCBWZXR0ZXIgd3JvdGU6Cj4+ Pj4+IE9uIFR1ZSwgTWFyIDEwLCAyMDE1IGF0IDAzOjIyOjQ5UE0gKzA1MzAsIEFyY2hpdCBUYW5l amEgd3JvdGU6Cj4+Pj4+PiBPbiAwMy8xMC8yMDE1IDAzOjE3IFBNLCBEYW5pZWwgVmV0dGVyIHdy b3RlOgo+Pj4+Pj4+IE9uIFR1ZSwgTWFyIDEwLCAyMDE1IGF0IDAzOjExOjI4UE0gKzA1MzAsIEFy Y2hpdCBUYW5lamEgd3JvdGU6Cj4+Pj4+Pj4+IGRpZmYgLS1naXQgYS9kcml2ZXJzL2dwdS9kcm0v S2NvbmZpZyBiL2RyaXZlcnMvZ3B1L2RybS9LY29uZmlnCj4+Pj4+Pj4+IGluZGV4IDE1MWEwNTAu LjM4ZjgzYTAgMTAwNjQ0Cj4+Pj4+Pj4+IC0tLSBhL2RyaXZlcnMvZ3B1L2RybS9LY29uZmlnCj4+ Pj4+Pj4+ICsrKyBiL2RyaXZlcnMvZ3B1L2RybS9LY29uZmlnCj4+Pj4+Pj4+IEBAIC00MCw2ICs0 MCwyNCBAQCBjb25maWcgRFJNX0tNU19GQl9IRUxQRVIKPj4+Pj4+Pj4gICAJaGVscAo+Pj4+Pj4+ PiAgIAkgIEZCREVWIGhlbHBlcnMgZm9yIEtNUyBkcml2ZXJzLgo+Pj4+Pj4+Pgo+Pj4+Pj4+PiAr Y29uZmlnIERSTV9GQkRFVl9FTVVMQVRJT04KPj4+Pj4+Pj4gKwlib29sICJFbmFibGUgbGVnYWN5 IGZiZGV2IHN1cHBvcnQgZm9yIHlvdXIgbW9kZXNldHRpbmcgZHJpdmVyIgo+Pj4+Pj4+PiArCWRl cGVuZHMgb24gRFJNCj4+Pj4+Pj4+ICsJc2VsZWN0IERSTV9LTVNfSEVMUEVSCj4+Pj4+Pj4+ICsJ c2VsZWN0IERSTV9LTVNfRkJfSEVMUEVSCj4+Pj4+Pj4+ICsJc2VsZWN0IEZCX1NZU19GSUxMUkVD VAo+Pj4+Pj4+PiArCXNlbGVjdCBGQl9TWVNfQ09QWUFSRUEKPj4+Pj4+Pj4gKwlzZWxlY3QgRkJf U1lTX0lNQUdFQkxJVAo+Pj4+Pj4+PiArCXNlbGVjdCBGQl9TWVNfRk9QUwo+Pj4+Pj4+PiArCXNl bGVjdCBGQl9DRkJfRklMTFJFQ1QKPj4+Pj4+Pj4gKwlzZWxlY3QgRkJfQ0ZCX0NPUFlBUkVBCj4+ Pj4+Pj4+ICsJc2VsZWN0IEZCX0NGQl9JTUFHRUJMSVQKPj4+Pj4+Pj4gKwlkZWZhdWx0IHkKPj4+ Pj4+Pj4gKwloZWxwCj4+Pj4+Pj4+ICsJICBDaG9vc2UgdGhpcyBvcHRpb24gaWYgeW91IGhhdmUg YSBuZWVkIGZvciB0aGUgbGVnYWN5IGZiZGV2Cj4+Pj4+Pj4+ICsJICBzdXBwb3J0LiBOb3RlIHRo YXQgdGhpcyBzdXBwb3J0IGFsc28gcHJvdmlkZSB0aGUgbGludXggY29uc29sZQo+Pj4+Pj4+PiAr CSAgc3VwcG9ydCBvbiB0b3Agb2YgeW91ciBtb2Rlc2V0dGluZyBkcml2ZXIuCj4+Pj4+Pj4KPj4+ Pj4+PiBNYXliZSBjbGFyaWZ5IHRoYXQgZm9yIGxpbnV4IGNvbnNvbGUgc3VwcG9ydCB5b3UgYWxz byBuZWVkCj4+Pj4+Pj4gQ09ORklHX0ZSQU1FQlVGRkVSX0NPTlNPTEU/IGZiZGV2IGFsb25lIGlz bid0IGVub3VnaC4KPj4+Pj4+Cj4+Pj4+PiBEUk1fS01TX0ZCX0hFTFBFUiBzZWxlY3RzIHRoYXQg Zm9yIHVzLCByaWdodD8KPj4+Pj4KPj4+Pj4gSG0gcmlnaHQgSSd2ZSBtaXNzZWQgdGhhdC4gUmVt aW5kcyBtZSB0aGF0IHlvdSBuZWVkIG9uZSBtb3JlIHBhdGNoIGF0IHRoZQo+Pj4+PiBlbmQgdG8g cmVtb3ZlIGFsbCB0aGUgdmFyaW91cyBzZWxlY3QgRFJNX0tNU19GQl9IRUxQRVIgZnJvbSBhbGwg ZHJtCj4+Pj4+IGRyaXZlcnMuIE90aGVyd2lzZSB0aGlzIGtub2IgaGVyZSB3b24ndCB3b3JrIGJ5 IGRlZmF1bHQgaWYgeW91IGUuZy4gc2VsZWN0Cj4+Pj4+IHJhZGVvbi4gSW4gZ2VuZXJhbCB3ZSBj YW4ndCBtaXggZXhwbGljaXQgb3B0aW9ucyB3aXRoIG1lbnUgZW50cmllcyB3aXRoIGEKPj4+Pj4g c2VsZWN0Lgo+Pj4+Cj4+Pj4gSSB3YXMgdHJ5aW5nIHRoYXQgb3V0LiBSZW1vdmluZyBEUk1fS01T X0ZCX0hFTFBFUiBhbmQgaGF2aW5nCj4+Pj4gRFJNX0ZCREVWX0VNVUxBVElPTiBkaXNhYmxlZCBi cmVha3MgZHJpdmVycyB3aGljaCB1c2UgRkIgc3R1ZmYgaW50ZXJuYWxseSBpbgo+Pj4+IHRoZWly IHJlc3BlY3RpdmUgeHl6X2ZiZGV2LmMgZmlsZXMuCj4+Pgo+Pj4gQnV0IHdpdGggdGhlIHN0dWJi ZWQgb3V0IGZ1bmN0aW9ucyB0aGF0IHNob3VsZCB3b3JrLCByaWdodD8gV2h5IGRvZXNuJ3QKPj4+ IGl0Pwo+Pgo+PiBUaGVyZSBhcmUgc3RpbGwgY2FsbHMgdG8gZnVuY3Rpb25zIGZyb20gZmIgY29y ZSBsaWtlIGZiX3NldF9zdXNwZW5kIGFuZAo+PiByZWdpc3Rlcl9mcmFtZWJ1ZmZlciB3aGljaCBh cmVuJ3QgY292ZXJlZCBieSB0aGUgZHJtIGZiIGhlbHBlciBmdW5jdGlvbnMuCj4KPiBIbSwgc291 bmRzIGxpa2Ugd2UgbmVlZCBhbm90aGVyIHBhdGNoIHRvIHN0dWIgb3V0IGZiX3NldF9zdXNwZW5k IHdoZW4KPiBmYmRldiBpc24ndCBlbmFibGVkLiBJcyB0aGVyZSBhbnl0aGluZyBlbHNlPwoKVGhl cmUgYXJlIGEgaGFuZGZ1bCBvZiBmYiBjb3JlIGZ1bmN0aW9ucyB3aGljaCBhcmUgY2FsbGVkIGJ5 IGRybSBkcml2ZXJzOgoKZmJfYWxsb2NfY21hcC9mYl9kZWFsbG9jX2NtYXAKCmZiX3N5c19yZWFk L2ZiX3N5c193cml0ZQoKcmVnaXN0ZXJfZnJhbWVidWZmZXIvdW5yZWdpc3Rlcl9mcmFtZWJ1ZmZl ci91bmxpbmtfZnJhbWVidWZmZXIvCnJlbW92ZV9jb25mbGljdGluZ19mcmFtZWJ1ZmZlcnMKCmZi X3NldF9zdXNwZW5kCgpmYl9kZWZlcnJlZF9pb19pbml0L2ZiX2RlZmVycmVkX2lvX2NsZWFudXAK CmZyYW1lYnVmZmVyX2FsbG9jL2ZyYW1lYnVmZmVyX3JlbGVhc2UKCgo+Cj4+Pj4gQXJlIHlvdSBz YXlpbmcgdGhhdCB3ZSBzaG91bGQgcmVtb3ZlIERSTV9LTVNfRkJfSEVMUEVSIGZvciBzdWNoIGRy aXZlcnMgYW5kCj4+Pj4gcmVwbGFjZSB0aGVtIHdpdGggJ3NlbGVjdCBEUk1fRkJERVZfRU1VTEFU SU9OJz8KPj4+Pgo+Pj4+IEFub3RoZXIgb3B0aW9uIHdvdWxkIGJlIHRvIHByb3ZpZGUgI2lmZGVm IERSTV9GQkRFVl9FTVVMQVRJT04gd3JhcC1hcm91bmRzCj4+Pj4gZm9yIGZiIHJlbGF0ZWQgZnVu Y3Rpb24gY2FsbHMvZGVjbGFyYXRpb25zIGluIGVhY2ggZHJpdmVyLCBzb21ldGhpbmcgdGhhdCdz Cj4+Pj4gYWxyZWFkeSBkb25lIGZvciBpOTE1IGFuZCBtc20gZHJpdmVycy4KPj4+Cj4+PiBUaGUg cHJvYmxlbSB3aXRoIHRoZSBwYXRjaCBhcy1pcyB0aGUgbWFzc2l2ZSBhbW91bnRzIG9mIHNlbGVj dHMgdGhlIEZCCj4+PiBoZWxwZXIgc3RpbGwgaGFzLiBXZSBuZWVkIHRvIGdldCByaWQgb2YgdGhl bSBzbyB0aGF0IHdoZW4geW91IGRpc2FibGUKPj4+IGZiZGV2IGVtdWxhdGlvbiB5b3UgY2FuIGlu ZGVlZCBkaXNhYmxlIGZiY29uIGFuZCB0aGUgZW50aXJlIGZiZGV2Cj4+PiBzdWJzeXN0ZW0uIEkn dmUgdGhvdWdodCB0aGF0IHJlbW92ZSB0aGUgaGlkZGVuIHN5bWJvbCBEUk1fS01TX0ZCX0hFTFBF Ugo+Pj4gYW5kIG1vdmluZyBhbGwgdGhlIHNlbGVjdHMgdG8gRFJNX0ZCREVWX0VNVUxBVElPTiBz aG91bGQgd29yayBvdXQ/IElmIHRoYXQKPj4+IGRvZXNuJ3Qgd29yayB3ZSBuZWVkIHRvIGxvb2sg YWdhaW4gaG93IHRvIGJldHRlciBzdHViIHRoaW5ncyBvdXQgSSB0aGluay4KPj4KPj4gVGhhdCdz IHRydWUuIEFsc28sIGFzIEphbmkgcG9pbnRlZCBvdXQsIG1heWJlIGl0IGlzbid0IHRoZSBiZXN0 IGlkZWEgdG8gbWFrZQo+PiBldmVyeSBrbXMgZHJpdmVyIHNlbGVjdCBEUk1fRkJERVZfRU1VTEFU SU9OPwo+Pgo+PiBNYXliZSB0aGUgZHJpdmVycyB0aGF0IGFyZSBjdXJyZW50bHkgYnVpbHQgd2l0 aCBmYmRldiBlbXVsYXRpb24gYnkgZGVmYXVsdAo+PiBjYW4gYWRkIGEgJ2RlcGVuZHMgb24gRFJN X0ZCREVWX0VNVUxBVElPTic/IFNpbmNlIHRoZSBjb25maWcgZGVmYXVsdHMgdG8geSwKPj4gdGhl IGRyaXZlcnMgc2hvdWxkIHJ1biBhcyBpcy4gTGF0ZXIsIHdlIGNvdWxkIHRha2UgdXAgZWFjaCBk cml2ZXIgYW5kIGJ1aWxkCj4+IHRoZSBmYiBzdHVmZiBvbmx5IHdoZW4gRFJNX0ZCREVWX0VNVUxB VElPTiBpcyBzZXQsIGxpa2UgaG93IHdlIGRvIGZvciBpOTE1Cj4+IGFuZCBtc20/Cj4KPiBZZWFo IHdlIGRlZmluaXRlbHkgY2FuJ3QgbWl4IHNlbGVjdCB3aXRoIGEgdXNlci12aXNpYmxlIG9wdGlv bi4gSSB0aGluayB3ZQo+IGp1c3QgbmVlZCB0byBmaXggdXAgdGhlIHJlbWFpbmluZyBmdW5jdGlv bnMgYW5kIGNyZWF0ZSBzdHVicyBmb3IgdGhlbSBpZgo+IG5lZWRlZCBhbmQgdGhlbiBkcm9wIGFs bCB0aGUgc2VsZWN0cy4gV2VsbCBpbiBEUk1fRkJERVZfRU1VTEFUSU9OIHdlCj4gc2hvdWxkIHN0 aWxsIHNlbGVjdCBmb3IgZmJjb24gc2luY2Ugb3RoZXJ3aXNlIHRvbnMgb2YgYnVnIHJlcG9ydHMg YWJvdXQKPiAid2hlcmUgaXMgbXkgZmJjb24gd2l0aCBrbXM/Ii4KCkknbGwgZ2l2ZSB0aGlzIGEg dHJ5LiBBbHRob3VnaCwgaXQgd291bGQgYmUgYSBiZXR0ZXIgaWRlYSB0byBtYWtlIHRoZSAKZHJp dmVycyBub3QgY2FsbCB0aGVzZSBhdCBhbGwgd2hlbiBmYmRldiBlbXVsYXRpb24gaXNuJ3QgYXNr ZWQgZm9yLiBXaXRoIAp0aGUgc3R1YnMsIGlmIHNvbWVvbmUgZG9lcyB0cnkgdG8gdXNlIHRoZSBk cml2ZXIgd2l0aCAKRFJNX0ZCREVWX0VNVUxBVElPTiBub3Qgc2V0LCB0aGUgd29yc3QgdGhhdCds bCBoYXBwZW4gd291bGQgYmUgdGhhdCB0aGUgCmRyaXZlciBmYWlscyB0byBwcm9iZS4gV2hpY2gg aXNuJ3Qgc28gYmFkLgoKQXJjaGl0CgotLSAKUXVhbGNvbW0gSW5ub3ZhdGlvbiBDZW50ZXIsIElu Yy4gaXMgYSBtZW1iZXIgb2YgQ29kZSBBdXJvcmEgRm9ydW0sCmEgTGludXggRm91bmRhdGlvbiBD b2xsYWJvcmF0aXZlIFByb2plY3QKX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX18KZHJpLWRldmVsIG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRl c2t0b3Aub3JnCmh0dHA6Ly9saXN0cy5mcmVlZGVza3RvcC5vcmcvbWFpbG1hbi9saXN0aW5mby9k cmktZGV2ZWwK From mboxrd@z Thu Jan 1 00:00:00 1970 From: Archit Taneja Date: Fri, 13 Mar 2015 06:37:07 +0000 Subject: Re: [RFC 1/6] drm: Add top level Kconfig option for DRM fbdev emulation Message-Id: <550282C3.10903@codeaurora.org> List-Id: References: <1425980493-27533-1-git-send-email-architt@codeaurora.org> <1425980493-27533-2-git-send-email-architt@codeaurora.org> <20150310094756.GE3800@phenom.ffwll.local> <54FEBEF1.5000103@codeaurora.org> <20150310100547.GG3800@phenom.ffwll.local> <54FEC5F1.90208@codeaurora.org> <20150310121702.GO3800@phenom.ffwll.local> <54FFFAEE.3020301@codeaurora.org> <20150311151731.GJ3800@phenom.ffwll.local> In-Reply-To: <20150311151731.GJ3800@phenom.ffwll.local> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: linux-kernel@vger.kernel.org, linux-arm-msm@vger.kernel.org, Jani Nikula , daniel@ffwll.ch Cc: linux-fbdev@vger.kernel.org, dri-devel@lists.freedesktop.org, tomi.valkeinen@ti.com, benjamin.gaignard@linaro.org, treding@nvidia.com On 03/11/2015 08:47 PM, Daniel Vetter wrote: > On Wed, Mar 11, 2015 at 01:51:02PM +0530, Archit Taneja wrote: >> >> >> On 03/10/2015 05:47 PM, Daniel Vetter wrote: >>> On Tue, Mar 10, 2015 at 03:52:41PM +0530, Archit Taneja wrote: >>>> On 03/10/2015 03:35 PM, Daniel Vetter wrote: >>>>> On Tue, Mar 10, 2015 at 03:22:49PM +0530, Archit Taneja wrote: >>>>>> On 03/10/2015 03:17 PM, Daniel Vetter wrote: >>>>>>> On Tue, Mar 10, 2015 at 03:11:28PM +0530, Archit Taneja wrote: >>>>>>>> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig >>>>>>>> index 151a050..38f83a0 100644 >>>>>>>> --- a/drivers/gpu/drm/Kconfig >>>>>>>> +++ b/drivers/gpu/drm/Kconfig >>>>>>>> @@ -40,6 +40,24 @@ config DRM_KMS_FB_HELPER >>>>>>>> help >>>>>>>> FBDEV helpers for KMS drivers. >>>>>>>> >>>>>>>> +config DRM_FBDEV_EMULATION >>>>>>>> + bool "Enable legacy fbdev support for your modesetting driver" >>>>>>>> + depends on DRM >>>>>>>> + select DRM_KMS_HELPER >>>>>>>> + select DRM_KMS_FB_HELPER >>>>>>>> + select FB_SYS_FILLRECT >>>>>>>> + select FB_SYS_COPYAREA >>>>>>>> + select FB_SYS_IMAGEBLIT >>>>>>>> + select FB_SYS_FOPS >>>>>>>> + select FB_CFB_FILLRECT >>>>>>>> + select FB_CFB_COPYAREA >>>>>>>> + select FB_CFB_IMAGEBLIT >>>>>>>> + default y >>>>>>>> + help >>>>>>>> + Choose this option if you have a need for the legacy fbdev >>>>>>>> + support. Note that this support also provide the linux console >>>>>>>> + support on top of your modesetting driver. >>>>>>> >>>>>>> Maybe clarify that for linux console support you also need >>>>>>> CONFIG_FRAMEBUFFER_CONSOLE? fbdev alone isn't enough. >>>>>> >>>>>> DRM_KMS_FB_HELPER selects that for us, right? >>>>> >>>>> Hm right I've missed that. Reminds me that you need one more patch at the >>>>> end to remove all the various select DRM_KMS_FB_HELPER from all drm >>>>> drivers. Otherwise this knob here won't work by default if you e.g. select >>>>> radeon. In general we can't mix explicit options with menu entries with a >>>>> select. >>>> >>>> I was trying that out. Removing DRM_KMS_FB_HELPER and having >>>> DRM_FBDEV_EMULATION disabled breaks drivers which use FB stuff internally in >>>> their respective xyz_fbdev.c files. >>> >>> But with the stubbed out functions that should work, right? Why doesn't >>> it? >> >> There are still calls to functions from fb core like fb_set_suspend and >> register_framebuffer which aren't covered by the drm fb helper functions. > > Hm, sounds like we need another patch to stub out fb_set_suspend when > fbdev isn't enabled. Is there anything else? There are a handful of fb core functions which are called by drm drivers: fb_alloc_cmap/fb_dealloc_cmap fb_sys_read/fb_sys_write register_framebuffer/unregister_framebuffer/unlink_framebuffer/ remove_conflicting_framebuffers fb_set_suspend fb_deferred_io_init/fb_deferred_io_cleanup framebuffer_alloc/framebuffer_release > >>>> Are you saying that we should remove DRM_KMS_FB_HELPER for such drivers and >>>> replace them with 'select DRM_FBDEV_EMULATION'? >>>> >>>> Another option would be to provide #ifdef DRM_FBDEV_EMULATION wrap-arounds >>>> for fb related function calls/declarations in each driver, something that's >>>> already done for i915 and msm drivers. >>> >>> The problem with the patch as-is the massive amounts of selects the FB >>> helper still has. We need to get rid of them so that when you disable >>> fbdev emulation you can indeed disable fbcon and the entire fbdev >>> subsystem. I've thought that remove the hidden symbol DRM_KMS_FB_HELPER >>> and moving all the selects to DRM_FBDEV_EMULATION should work out? If that >>> doesn't work we need to look again how to better stub things out I think. >> >> That's true. Also, as Jani pointed out, maybe it isn't the best idea to make >> every kms driver select DRM_FBDEV_EMULATION? >> >> Maybe the drivers that are currently built with fbdev emulation by default >> can add a 'depends on DRM_FBDEV_EMULATION'? Since the config defaults to y, >> the drivers should run as is. Later, we could take up each driver and build >> the fb stuff only when DRM_FBDEV_EMULATION is set, like how we do for i915 >> and msm? > > Yeah we definitely can't mix select with a user-visible option. I think we > just need to fix up the remaining functions and create stubs for them if > needed and then drop all the selects. Well in DRM_FBDEV_EMULATION we > should still select for fbcon since otherwise tons of bug reports about > "where is my fbcon with kms?". I'll give this a try. Although, it would be a better idea to make the drivers not call these at all when fbdev emulation isn't asked for. With the stubs, if someone does try to use the driver with DRM_FBDEV_EMULATION not set, the worst that'll happen would be that the driver fails to probe. Which isn't so bad. Archit -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752424AbbCMG00 (ORCPT ); Fri, 13 Mar 2015 02:26:26 -0400 Received: from smtp.codeaurora.org ([198.145.29.96]:40856 "EHLO smtp.codeaurora.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751707AbbCMG0Y (ORCPT ); Fri, 13 Mar 2015 02:26:24 -0400 Message-ID: <550282C3.10903@codeaurora.org> Date: Fri, 13 Mar 2015 11:55:07 +0530 From: Archit Taneja User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.4.0 MIME-Version: 1.0 To: linux-kernel@vger.kernel.org, linux-arm-msm@vger.kernel.org, Jani Nikula , daniel@ffwll.ch CC: robdclark@gmail.com, airlied@linux.ie, treding@nvidia.com, p.zabel@pengutronix.de, benjamin.gaignard@linaro.org, dri-devel@lists.freedesktop.org, linux-fbdev@vger.kernel.org, tomi.valkeinen@ti.com Subject: Re: [RFC 1/6] drm: Add top level Kconfig option for DRM fbdev emulation References: <1425980493-27533-1-git-send-email-architt@codeaurora.org> <1425980493-27533-2-git-send-email-architt@codeaurora.org> <20150310094756.GE3800@phenom.ffwll.local> <54FEBEF1.5000103@codeaurora.org> <20150310100547.GG3800@phenom.ffwll.local> <54FEC5F1.90208@codeaurora.org> <20150310121702.GO3800@phenom.ffwll.local> <54FFFAEE.3020301@codeaurora.org> <20150311151731.GJ3800@phenom.ffwll.local> In-Reply-To: <20150311151731.GJ3800@phenom.ffwll.local> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 03/11/2015 08:47 PM, Daniel Vetter wrote: > On Wed, Mar 11, 2015 at 01:51:02PM +0530, Archit Taneja wrote: >> >> >> On 03/10/2015 05:47 PM, Daniel Vetter wrote: >>> On Tue, Mar 10, 2015 at 03:52:41PM +0530, Archit Taneja wrote: >>>> On 03/10/2015 03:35 PM, Daniel Vetter wrote: >>>>> On Tue, Mar 10, 2015 at 03:22:49PM +0530, Archit Taneja wrote: >>>>>> On 03/10/2015 03:17 PM, Daniel Vetter wrote: >>>>>>> On Tue, Mar 10, 2015 at 03:11:28PM +0530, Archit Taneja wrote: >>>>>>>> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig >>>>>>>> index 151a050..38f83a0 100644 >>>>>>>> --- a/drivers/gpu/drm/Kconfig >>>>>>>> +++ b/drivers/gpu/drm/Kconfig >>>>>>>> @@ -40,6 +40,24 @@ config DRM_KMS_FB_HELPER >>>>>>>> help >>>>>>>> FBDEV helpers for KMS drivers. >>>>>>>> >>>>>>>> +config DRM_FBDEV_EMULATION >>>>>>>> + bool "Enable legacy fbdev support for your modesetting driver" >>>>>>>> + depends on DRM >>>>>>>> + select DRM_KMS_HELPER >>>>>>>> + select DRM_KMS_FB_HELPER >>>>>>>> + select FB_SYS_FILLRECT >>>>>>>> + select FB_SYS_COPYAREA >>>>>>>> + select FB_SYS_IMAGEBLIT >>>>>>>> + select FB_SYS_FOPS >>>>>>>> + select FB_CFB_FILLRECT >>>>>>>> + select FB_CFB_COPYAREA >>>>>>>> + select FB_CFB_IMAGEBLIT >>>>>>>> + default y >>>>>>>> + help >>>>>>>> + Choose this option if you have a need for the legacy fbdev >>>>>>>> + support. Note that this support also provide the linux console >>>>>>>> + support on top of your modesetting driver. >>>>>>> >>>>>>> Maybe clarify that for linux console support you also need >>>>>>> CONFIG_FRAMEBUFFER_CONSOLE? fbdev alone isn't enough. >>>>>> >>>>>> DRM_KMS_FB_HELPER selects that for us, right? >>>>> >>>>> Hm right I've missed that. Reminds me that you need one more patch at the >>>>> end to remove all the various select DRM_KMS_FB_HELPER from all drm >>>>> drivers. Otherwise this knob here won't work by default if you e.g. select >>>>> radeon. In general we can't mix explicit options with menu entries with a >>>>> select. >>>> >>>> I was trying that out. Removing DRM_KMS_FB_HELPER and having >>>> DRM_FBDEV_EMULATION disabled breaks drivers which use FB stuff internally in >>>> their respective xyz_fbdev.c files. >>> >>> But with the stubbed out functions that should work, right? Why doesn't >>> it? >> >> There are still calls to functions from fb core like fb_set_suspend and >> register_framebuffer which aren't covered by the drm fb helper functions. > > Hm, sounds like we need another patch to stub out fb_set_suspend when > fbdev isn't enabled. Is there anything else? There are a handful of fb core functions which are called by drm drivers: fb_alloc_cmap/fb_dealloc_cmap fb_sys_read/fb_sys_write register_framebuffer/unregister_framebuffer/unlink_framebuffer/ remove_conflicting_framebuffers fb_set_suspend fb_deferred_io_init/fb_deferred_io_cleanup framebuffer_alloc/framebuffer_release > >>>> Are you saying that we should remove DRM_KMS_FB_HELPER for such drivers and >>>> replace them with 'select DRM_FBDEV_EMULATION'? >>>> >>>> Another option would be to provide #ifdef DRM_FBDEV_EMULATION wrap-arounds >>>> for fb related function calls/declarations in each driver, something that's >>>> already done for i915 and msm drivers. >>> >>> The problem with the patch as-is the massive amounts of selects the FB >>> helper still has. We need to get rid of them so that when you disable >>> fbdev emulation you can indeed disable fbcon and the entire fbdev >>> subsystem. I've thought that remove the hidden symbol DRM_KMS_FB_HELPER >>> and moving all the selects to DRM_FBDEV_EMULATION should work out? If that >>> doesn't work we need to look again how to better stub things out I think. >> >> That's true. Also, as Jani pointed out, maybe it isn't the best idea to make >> every kms driver select DRM_FBDEV_EMULATION? >> >> Maybe the drivers that are currently built with fbdev emulation by default >> can add a 'depends on DRM_FBDEV_EMULATION'? Since the config defaults to y, >> the drivers should run as is. Later, we could take up each driver and build >> the fb stuff only when DRM_FBDEV_EMULATION is set, like how we do for i915 >> and msm? > > Yeah we definitely can't mix select with a user-visible option. I think we > just need to fix up the remaining functions and create stubs for them if > needed and then drop all the selects. Well in DRM_FBDEV_EMULATION we > should still select for fbcon since otherwise tons of bug reports about > "where is my fbcon with kms?". I'll give this a try. Although, it would be a better idea to make the drivers not call these at all when fbdev emulation isn't asked for. With the stubs, if someone does try to use the driver with DRM_FBDEV_EMULATION not set, the worst that'll happen would be that the driver fails to probe. Which isn't so bad. Archit -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project