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: Wed, 25 Mar 2015 13:47:54 +0530 Message-ID: <55126F32.90404@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> <550282C3.10903@codeaurora.org> <20150313090651.GZ3800@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: <20150313090651.GZ3800@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: robdclark@gmail.com, airlied@linux.ie, treding@nvidia.com, p.zabel@pengutronix.de, benjamin.gaignard@linaro.org, dri-devel@lists.freedesktop.org, daniel@ffwll.ch Cc: linux-arm-msm@vger.kernel.org, linux-fbdev@vger.kernel.org, tomi.valkeinen@ti.com, linux-kernel@vger.kernel.org List-Id: linux-arm-msm@vger.kernel.org SGksCgpPbiAwMy8xMy8yMDE1IDAyOjM2IFBNLCBEYW5pZWwgVmV0dGVyIHdyb3RlOgo+IE9uIEZy aSwgTWFyIDEzLCAyMDE1IGF0IDExOjU1OjA3QU0gKzA1MzAsIEFyY2hpdCBUYW5lamEgd3JvdGU6 Cj4+Cj4+Cj4+IE9uIDAzLzExLzIwMTUgMDg6NDcgUE0sIERhbmllbCBWZXR0ZXIgd3JvdGU6Cj4+ PiBPbiBXZWQsIE1hciAxMSwgMjAxNSBhdCAwMTo1MTowMlBNICswNTMwLCBBcmNoaXQgVGFuZWph IHdyb3RlOgo+Pj4+Cj4+Pj4KPj4+PiBPbiAwMy8xMC8yMDE1IDA1OjQ3IFBNLCBEYW5pZWwgVmV0 dGVyIHdyb3RlOgo+Pj4+PiBPbiBUdWUsIE1hciAxMCwgMjAxNSBhdCAwMzo1Mjo0MVBNICswNTMw LCBBcmNoaXQgVGFuZWphIHdyb3RlOgo+Pj4+Pj4gT24gMDMvMTAvMjAxNSAwMzozNSBQTSwgRGFu aWVsIFZldHRlciB3cm90ZToKPj4+Pj4+PiBPbiBUdWUsIE1hciAxMCwgMjAxNSBhdCAwMzoyMjo0 OVBNICswNTMwLCBBcmNoaXQgVGFuZWphIHdyb3RlOgo+Pj4+Pj4+PiBPbiAwMy8xMC8yMDE1IDAz OjE3IFBNLCBEYW5pZWwgVmV0dGVyIHdyb3RlOgo+Pj4+Pj4+Pj4gT24gVHVlLCBNYXIgMTAsIDIw MTUgYXQgMDM6MTE6MjhQTSArMDUzMCwgQXJjaGl0IFRhbmVqYSB3cm90ZToKPj4+Pj4+Pj4+PiBk aWZmIC0tZ2l0IGEvZHJpdmVycy9ncHUvZHJtL0tjb25maWcgYi9kcml2ZXJzL2dwdS9kcm0vS2Nv bmZpZwo+Pj4+Pj4+Pj4+IGluZGV4IDE1MWEwNTAuLjM4ZjgzYTAgMTAwNjQ0Cj4+Pj4+Pj4+Pj4g LS0tIGEvZHJpdmVycy9ncHUvZHJtL0tjb25maWcKPj4+Pj4+Pj4+PiArKysgYi9kcml2ZXJzL2dw dS9kcm0vS2NvbmZpZwo+Pj4+Pj4+Pj4+IEBAIC00MCw2ICs0MCwyNCBAQCBjb25maWcgRFJNX0tN U19GQl9IRUxQRVIKPj4+Pj4+Pj4+PiAgIAloZWxwCj4+Pj4+Pj4+Pj4gICAJICBGQkRFViBoZWxw ZXJzIGZvciBLTVMgZHJpdmVycy4KPj4+Pj4+Pj4+Pgo+Pj4+Pj4+Pj4+ICtjb25maWcgRFJNX0ZC REVWX0VNVUxBVElPTgo+Pj4+Pj4+Pj4+ICsJYm9vbCAiRW5hYmxlIGxlZ2FjeSBmYmRldiBzdXBw b3J0IGZvciB5b3VyIG1vZGVzZXR0aW5nIGRyaXZlciIKPj4+Pj4+Pj4+PiArCWRlcGVuZHMgb24g RFJNCj4+Pj4+Pj4+Pj4gKwlzZWxlY3QgRFJNX0tNU19IRUxQRVIKPj4+Pj4+Pj4+PiArCXNlbGVj dCBEUk1fS01TX0ZCX0hFTFBFUgo+Pj4+Pj4+Pj4+ICsJc2VsZWN0IEZCX1NZU19GSUxMUkVDVAo+ Pj4+Pj4+Pj4+ICsJc2VsZWN0IEZCX1NZU19DT1BZQVJFQQo+Pj4+Pj4+Pj4+ICsJc2VsZWN0IEZC X1NZU19JTUFHRUJMSVQKPj4+Pj4+Pj4+PiArCXNlbGVjdCBGQl9TWVNfRk9QUwo+Pj4+Pj4+Pj4+ ICsJc2VsZWN0IEZCX0NGQl9GSUxMUkVDVAo+Pj4+Pj4+Pj4+ICsJc2VsZWN0IEZCX0NGQl9DT1BZ QVJFQQo+Pj4+Pj4+Pj4+ICsJc2VsZWN0IEZCX0NGQl9JTUFHRUJMSVQKPj4+Pj4+Pj4+PiArCWRl ZmF1bHQgeQo+Pj4+Pj4+Pj4+ICsJaGVscAo+Pj4+Pj4+Pj4+ICsJICBDaG9vc2UgdGhpcyBvcHRp b24gaWYgeW91IGhhdmUgYSBuZWVkIGZvciB0aGUgbGVnYWN5IGZiZGV2Cj4+Pj4+Pj4+Pj4gKwkg IHN1cHBvcnQuIE5vdGUgdGhhdCB0aGlzIHN1cHBvcnQgYWxzbyBwcm92aWRlIHRoZSBsaW51eCBj b25zb2xlCj4+Pj4+Pj4+Pj4gKwkgIHN1cHBvcnQgb24gdG9wIG9mIHlvdXIgbW9kZXNldHRpbmcg ZHJpdmVyLgo+Pj4+Pj4+Pj4KPj4+Pj4+Pj4+IE1heWJlIGNsYXJpZnkgdGhhdCBmb3IgbGludXgg Y29uc29sZSBzdXBwb3J0IHlvdSBhbHNvIG5lZWQKPj4+Pj4+Pj4+IENPTkZJR19GUkFNRUJVRkZF Ul9DT05TT0xFPyBmYmRldiBhbG9uZSBpc24ndCBlbm91Z2guCj4+Pj4+Pj4+Cj4+Pj4+Pj4+IERS TV9LTVNfRkJfSEVMUEVSIHNlbGVjdHMgdGhhdCBmb3IgdXMsIHJpZ2h0Pwo+Pj4+Pj4+Cj4+Pj4+ Pj4gSG0gcmlnaHQgSSd2ZSBtaXNzZWQgdGhhdC4gUmVtaW5kcyBtZSB0aGF0IHlvdSBuZWVkIG9u ZSBtb3JlIHBhdGNoIGF0IHRoZQo+Pj4+Pj4+IGVuZCB0byByZW1vdmUgYWxsIHRoZSB2YXJpb3Vz IHNlbGVjdCBEUk1fS01TX0ZCX0hFTFBFUiBmcm9tIGFsbCBkcm0KPj4+Pj4+PiBkcml2ZXJzLiBP dGhlcndpc2UgdGhpcyBrbm9iIGhlcmUgd29uJ3Qgd29yayBieSBkZWZhdWx0IGlmIHlvdSBlLmcu IHNlbGVjdAo+Pj4+Pj4+IHJhZGVvbi4gSW4gZ2VuZXJhbCB3ZSBjYW4ndCBtaXggZXhwbGljaXQg b3B0aW9ucyB3aXRoIG1lbnUgZW50cmllcyB3aXRoIGEKPj4+Pj4+PiBzZWxlY3QuCj4+Pj4+Pgo+ Pj4+Pj4gSSB3YXMgdHJ5aW5nIHRoYXQgb3V0LiBSZW1vdmluZyBEUk1fS01TX0ZCX0hFTFBFUiBh bmQgaGF2aW5nCj4+Pj4+PiBEUk1fRkJERVZfRU1VTEFUSU9OIGRpc2FibGVkIGJyZWFrcyBkcml2 ZXJzIHdoaWNoIHVzZSBGQiBzdHVmZiBpbnRlcm5hbGx5IGluCj4+Pj4+PiB0aGVpciByZXNwZWN0 aXZlIHh5el9mYmRldi5jIGZpbGVzLgo+Pj4+Pgo+Pj4+PiBCdXQgd2l0aCB0aGUgc3R1YmJlZCBv dXQgZnVuY3Rpb25zIHRoYXQgc2hvdWxkIHdvcmssIHJpZ2h0PyBXaHkgZG9lc24ndAo+Pj4+PiBp dD8KPj4+Pgo+Pj4+IFRoZXJlIGFyZSBzdGlsbCBjYWxscyB0byBmdW5jdGlvbnMgZnJvbSBmYiBj b3JlIGxpa2UgZmJfc2V0X3N1c3BlbmQgYW5kCj4+Pj4gcmVnaXN0ZXJfZnJhbWVidWZmZXIgd2hp Y2ggYXJlbid0IGNvdmVyZWQgYnkgdGhlIGRybSBmYiBoZWxwZXIgZnVuY3Rpb25zLgo+Pj4KPj4+ IEhtLCBzb3VuZHMgbGlrZSB3ZSBuZWVkIGFub3RoZXIgcGF0Y2ggdG8gc3R1YiBvdXQgZmJfc2V0 X3N1c3BlbmQgd2hlbgo+Pj4gZmJkZXYgaXNuJ3QgZW5hYmxlZC4gSXMgdGhlcmUgYW55dGhpbmcg ZWxzZT8KPj4KPj4gVGhlcmUgYXJlIGEgaGFuZGZ1bCBvZiBmYiBjb3JlIGZ1bmN0aW9ucyB3aGlj aCBhcmUgY2FsbGVkIGJ5IGRybSBkcml2ZXJzOgo+Pgo+PiBmYl9hbGxvY19jbWFwL2ZiX2RlYWxs b2NfY21hcAo+Pgo+PiBmYl9zeXNfcmVhZC9mYl9zeXNfd3JpdGUKPj4KPj4gcmVnaXN0ZXJfZnJh bWVidWZmZXIvdW5yZWdpc3Rlcl9mcmFtZWJ1ZmZlci91bmxpbmtfZnJhbWVidWZmZXIvCj4+IHJl bW92ZV9jb25mbGljdGluZ19mcmFtZWJ1ZmZlcnMKPj4KPj4gZmJfc2V0X3N1c3BlbmQKPj4KPj4g ZmJfZGVmZXJyZWRfaW9faW5pdC9mYl9kZWZlcnJlZF9pb19jbGVhbnVwCj4+Cj4+IGZyYW1lYnVm ZmVyX2FsbG9jL2ZyYW1lYnVmZmVyX3JlbGVhc2UKPgo+IEhtIHllYWggdGhhdCdzIHNvbWV3aGF0 IGFubm95aW5nIGluZGVlZC4gV2hhdCBhYm91dCB0aGUgZm9sbG93aW5nOgo+IDEuIFdlIG1vdmUg YWxsIHRoZSAjaW5jbHVkZSA8bGludXgvZmIuaD4gZnJvbSBkcml2ZXJzIGludG8gZHJtX2ZiX2hl bHBlci5oCj4KPiAyLiBUaGVuIHdlIGFkZCBzdHVicyBmb3IgdGhlc2UgZnVuY3Rpb25zIGluIGRy bV9mYl9oZWxwZXIuaCwgbGlrZSB0aGlzCj4KPiAjaWYgZGVmaW5lZChDT05GSUdfRkIpCj4gI2lu Y2x1ZGUgPGxpbnV4L2ZiLmg+Cj4gI2Vsc2UKPgo+IC8qIHN0YXRpYyBpbmxpbmUgc3R1YnMgZm9y IGFsbCB0aGUgZmIgc3R1ZmYgdXNlZCBieSBrbXMgZHJpdmVycyAqLwo+ICNlbmRpZgo+Cj4gSW1v IHRoaXMgbWFrZXMgc2Vuc2Ugc2luY2Uga21zIGRyaXZlcnMgcmVhbGx5IGhhdmUgYSBiaXQgYSBz cGVjaWFsCj4gc2l0dWF0aW9uIHdpdGggZmJkZXYuIFRoZXkncmUgbm90IGZ1bGwtYmxvd24gZmJk ZXYgZHJpdmVycyBhbmQgY2FuIGJlCj4gdXNlZnVsIGZ1bGx5IHdpdGhvdXQgZmJkZXYuCj4KCkkg d2FzIHRyeWluZyB0aGlzIG91dC4gUmVtb3ZpbmcgJ2xpbnV4L2ZiLmgnIGFuZCByZXBsYWNpbmcg c3R1YiBmYiBmdW5jcyAKd29uJ3QgcmVhbGx5IHdvcmsgYmVjYXVzZSBzdHJ1Y3QgZGVjbGFyYXRp b25zKGxpa2UgZmJfaW5mbykgYWxzbyBnZXQgCnJlbW92ZWQuCgpJIGNvbnNpZGVyZWQgcGxhY2lu ZyAnI2lmIElTX0VOQUJMRUQoQ09ORklHX0ZCKScgd2l0aGluIGxpbnV4L2ZiLmggCml0c2VsZiwg YnV0IHRoYXQgc2VlbWVkIGEgYml0IHRvbyBpbnRydXNpdmUuCgpUaGlzIGlzIHdoYXQgSSdtIGN1 cnJlbnRseSBkb2luZzoKCi0gU29tZSBmdW5jcywgbGlrZSBmcmFtZWJ1ZmVyX2FsbG9jL3JlbGVh c2UsIGFsbG9jX2NtYXAvZGVhbGxvY19jbWFwIAp3b3VsZCBhY3R1YWxseSBiZW5lZml0IGlmIHdl IGhhdmUgZHJtIGZiIGhlbHBlcnMgZm9yIHRoZW0uIFRoZXkgYXJlIHVzZWQgCmluIGV4YWN0bHkg dGhlIHNhbWUgbWFubmVyIGJ5IGFsbCB0aGUgZHJpdmVycy4KCi0gRm9yIHRoZSByZXN0IG9mIHRo ZSBmdW5jdGlvbnMgdGhhdCBhcmUgc3BhcnNlbHkgdXNlZCwgSSB3YXMgCmNvbnNpZGVyaW5nIG1h a2luZyB2ZXJ5IHNpbXBsZSBkcm1fZmJfKiB3cmFwcGVyIGZ1bmN0aW9ucy4gU29tZXRoaW5nIGxp a2U6Cgp2b2lkIGRybV9mYl9oZWxwZXJfZGVmZXJyZWRfaW9faW5pdChzdHJ1Y3QgZHJtX2ZiX2hl bHBlciAqaGVscGVyKQp7CglpZiAoaGVscGVyLT5mYmRldikKCQlmYl9kZWZlcnJlZF9pb19pbml0 KGhlbHBlci0+ZmJkZXYpOwp9CgpXZSBjb3VsZCBoYXZlIGFsbCBmYiBjYWxscyBjYWxsZWQgd2l0 aGluIGRybV9mYl9oZWxwZXIuYywgY3JlYXRpbmcgCmRybV9mYl9oZWxwZXJfKiBzdHViIGZ1bmN0 aW9ucyB3b3VsZCB0aGVuIGJlIGFuIGVhc2llciB0YXNrLiBXaGF0IGRvIHlvdSAKdGhpbms/CgpB cmNoaXQKCi0tIApRdWFsY29tbSBJbm5vdmF0aW9uIENlbnRlciwgSW5jLiBpcyBhIG1lbWJlciBv ZiBDb2RlIEF1cm9yYSBGb3J1bSwKYSBMaW51eCBGb3VuZGF0aW9uIENvbGxhYm9yYXRpdmUgUHJv amVjdApfX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fXwpkcmkt ZGV2ZWwgbWFpbGluZyBsaXN0CmRyaS1kZXZlbEBsaXN0cy5mcmVlZGVza3RvcC5vcmcKaHR0cDov L2xpc3RzLmZyZWVkZXNrdG9wLm9yZy9tYWlsbWFuL2xpc3RpbmZvL2RyaS1kZXZlbAo= From mboxrd@z Thu Jan 1 00:00:00 1970 From: Archit Taneja Date: Wed, 25 Mar 2015 08:29:54 +0000 Subject: Re: [RFC 1/6] drm: Add top level Kconfig option for DRM fbdev emulation Message-Id: <55126F32.90404@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> <550282C3.10903@codeaurora.org> <20150313090651.GZ3800@phenom.ffwll.local> In-Reply-To: <20150313090651.GZ3800@phenom.ffwll.local> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: robdclark@gmail.com, airlied@linux.ie, treding@nvidia.com, p.zabel@pengutronix.de, benjamin.gaignard@linaro.org, dri-devel@lists.freedesktop.org, daniel@ffwll.ch Cc: linux-arm-msm@vger.kernel.org, linux-fbdev@vger.kernel.org, tomi.valkeinen@ti.com, linux-kernel@vger.kernel.org Hi, On 03/13/2015 02:36 PM, Daniel Vetter wrote: > On Fri, Mar 13, 2015 at 11:55:07AM +0530, Archit Taneja wrote: >> >> >> 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 > > Hm yeah that's somewhat annoying indeed. What about the following: > 1. We move all the #include from drivers into drm_fb_helper.h > > 2. Then we add stubs for these functions in drm_fb_helper.h, like this > > #if defined(CONFIG_FB) > #include > #else > > /* static inline stubs for all the fb stuff used by kms drivers */ > #endif > > Imo this makes sense since kms drivers really have a bit a special > situation with fbdev. They're not full-blown fbdev drivers and can be > useful fully without fbdev. > I was trying this out. Removing 'linux/fb.h' and replacing stub fb funcs won't really work because struct declarations(like fb_info) also get removed. I considered placing '#if IS_ENABLED(CONFIG_FB)' within linux/fb.h itself, but that seemed a bit too intrusive. This is what I'm currently doing: - Some funcs, like framebufer_alloc/release, alloc_cmap/dealloc_cmap would actually benefit if we have drm fb helpers for them. They are used in exactly the same manner by all the drivers. - For the rest of the functions that are sparsely used, I was considering making very simple drm_fb_* wrapper functions. Something like: void drm_fb_helper_deferred_io_init(struct drm_fb_helper *helper) { if (helper->fbdev) fb_deferred_io_init(helper->fbdev); } We could have all fb calls called within drm_fb_helper.c, creating drm_fb_helper_* stub functions would then be an easier task. What do you think? 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 S1752205AbbCYISI (ORCPT ); Wed, 25 Mar 2015 04:18:08 -0400 Received: from smtp.codeaurora.org ([198.145.29.96]:51266 "EHLO smtp.codeaurora.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752071AbbCYISD (ORCPT ); Wed, 25 Mar 2015 04:18:03 -0400 Message-ID: <55126F32.90404@codeaurora.org> Date: Wed, 25 Mar 2015 13:47:54 +0530 From: Archit Taneja User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.5.0 MIME-Version: 1.0 To: robdclark@gmail.com, airlied@linux.ie, treding@nvidia.com, p.zabel@pengutronix.de, benjamin.gaignard@linaro.org, dri-devel@lists.freedesktop.org, daniel@ffwll.ch CC: linux-kernel@vger.kernel.org, linux-arm-msm@vger.kernel.org, Jani Nikula , 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> <550282C3.10903@codeaurora.org> <20150313090651.GZ3800@phenom.ffwll.local> In-Reply-To: <20150313090651.GZ3800@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 Hi, On 03/13/2015 02:36 PM, Daniel Vetter wrote: > On Fri, Mar 13, 2015 at 11:55:07AM +0530, Archit Taneja wrote: >> >> >> 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 > > Hm yeah that's somewhat annoying indeed. What about the following: > 1. We move all the #include from drivers into drm_fb_helper.h > > 2. Then we add stubs for these functions in drm_fb_helper.h, like this > > #if defined(CONFIG_FB) > #include > #else > > /* static inline stubs for all the fb stuff used by kms drivers */ > #endif > > Imo this makes sense since kms drivers really have a bit a special > situation with fbdev. They're not full-blown fbdev drivers and can be > useful fully without fbdev. > I was trying this out. Removing 'linux/fb.h' and replacing stub fb funcs won't really work because struct declarations(like fb_info) also get removed. I considered placing '#if IS_ENABLED(CONFIG_FB)' within linux/fb.h itself, but that seemed a bit too intrusive. This is what I'm currently doing: - Some funcs, like framebufer_alloc/release, alloc_cmap/dealloc_cmap would actually benefit if we have drm fb helpers for them. They are used in exactly the same manner by all the drivers. - For the rest of the functions that are sparsely used, I was considering making very simple drm_fb_* wrapper functions. Something like: void drm_fb_helper_deferred_io_init(struct drm_fb_helper *helper) { if (helper->fbdev) fb_deferred_io_init(helper->fbdev); } We could have all fb calls called within drm_fb_helper.c, creating drm_fb_helper_* stub functions would then be an easier task. What do you think? Archit -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project