From mboxrd@z Thu Jan 1 00:00:00 1970 From: Archit Taneja Subject: Re: [PATCH] drm: msm: Fix build when legacy fbdev support isn't set Date: Thu, 05 Mar 2015 15:36:27 +0530 Message-ID: <54F82AA3.4040902@codeaurora.org> References: <1424687361-17787-1-git-send-email-architt@codeaurora.org> <20150223140903.GY24485@phenom.ffwll.local> <20150223153940.GZ24485@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: <20150223153940.GZ24485@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: Rob Clark , "dri-devel@lists.freedesktop.org" , linux-arm-msm , Linux Kernel Mailing List List-Id: linux-arm-msm@vger.kernel.org Ck9uIDAyLzIzLzIwMTUgMDk6MDkgUE0sIERhbmllbCBWZXR0ZXIgd3JvdGU6Cj4gT24gTW9uLCBG ZWIgMjMsIDIwMTUgYXQgMTA6MDM6MjFBTSAtMDUwMCwgUm9iIENsYXJrIHdyb3RlOgo+PiBPbiBN b24sIEZlYiAyMywgMjAxNSBhdCA5OjA5IEFNLCBEYW5pZWwgVmV0dGVyIDxkYW5pZWxAZmZ3bGwu Y2g+IHdyb3RlOgo+Pj4gT24gTW9uLCBGZWIgMjMsIDIwMTUgYXQgMDg6MzM6MzZBTSAtMDUwMCwg Um9iIENsYXJrIHdyb3RlOgo+Pj4+IE9uIE1vbiwgRmViIDIzLCAyMDE1IGF0IDU6MjkgQU0sIEFy Y2hpdCBUYW5lamEgPGFyY2hpdHRAY29kZWF1cm9yYS5vcmc+IHdyb3RlOgo+Pj4+PiBUaGUgRFJN X0tNU19GQl9IRUxQRVIgY29uZmlnIGlzIHNlbGVjdGVkIG9ubHkgd2hlbiBEUk1fTVNNX0ZCREVW IGNvbmZpZyBpcwo+Pj4+PiBzZWxlY3RlZC4gVGhlIGRyaXZlciBhY2Nlc3NlcyBkcm1fZmJfaGVs cGVyXyogZnVuY3Rpb25zIGV2ZW4gd2hlbiBsZWdhY3kgZmJkZXYKPj4+Pj4gc3VwcG9ydCBpcyBk aXNhYmxlZCBpbiBtc20uIFdyYXAgYXJvdW5kIHRoZXNlIGZ1bmN0aW9ucyB3aXRoICNpZmRlZiBj aGVja3MgdG8KPj4+Pj4gcHJldmVudCBidWlsZCBicmVhay4KPj4+Pgo+Pj4+IGhtbSwgcGVyaGFw cyByYXRoZXIgdGhhbiBzb2x2aW5nIHRoaXMgaW4gZWFjaCBkcml2ZXIsIHdlIHNob3VsZCBkbwo+ Pj4+IHNvbWUgc3R1YiB2ZXJzaW9ucyBvZiB0aG9zZSBmYi1oZWxwZXIgZnhucz8KPj4+Pgo+Pj4+ IFRoZXJlIGFyZSBhdCBsZWFzdCBvbmUgb3IgdHdvIG90aGVyIGRyaXZlcnMgdGhhdCBjYW4gYnVp bGQgd2l0aG91dAo+Pj4+IGZiZGV2LCBhbmQgSSBndWVzcyBtb3JlIGdvaW5nIGZvcndhcmQuLgo+ Pj4KPj4+IEl0J3Mgbm90IHF1aXRlIHRoYXQgZWFzeSBzaW5jZSB5b3UgYWxzbyBoYXZlIHRvIHN0 YXJ0L3N0b3AgdGhlIHZ0Cj4+PiBzdWJzeXN0ZW0gYXQgdGhlIHJpZ2h0IHBvaW50IGluIHRpbWUg aW4geW91ciBvd24gZHJpdmVyLiBTZWUKPj4+IGludGVsX2ZiZGV2X3NldF9zdXNwZW5kLiBJZiB5 b3UgZG9uJ3QgZG8gdGhhdCB0aGVyZSdzIG5vIHN5bmNocm9uaXphdGlvbgo+Pj4gYmV0d2VlbiBm YmNvbiBzaHV0dGluZyBkb3duL3Jlc3VtaW5nIGFuZCB5b3VyIGRyaXZlciwgd2hpY2ggaW4gdGhl IGJlc3QKPj4+IGNhc2UgbWVhbnMgZmJjb24gZG9lcyBzb21lIHdyaXRlcyB0byBub3doZXJlIGFu ZCB3b3JzdCBjYXNlIG1lYW5zIHlvdXIKPj4+IGNoaXAgZGllcyAobW1pbyB0byBvZmZsaW5lIGNo aXAgYmxvY2tzKSBvciB3cml0ZXMgZ28gdG8gc29tZXdoZXJlIHJhbmRvbQo+Pj4gaW4gc3lzdGVt IG1lbW9yeSAoaW9tbXUgY29udGFpbnMgc29tZSBzdGFsZSBwdGVzIHNpbmNlIG5vdCB5ZXQgZnVs bHkKPj4+IHJlc3RvcmVkLCBtb3JlIGFuIGlzc3VlIHdpdGggaGliZXJuYXRlKS4KPj4KPj4gSSBn dWVzcyBJIGRvbid0IGZ1bGx5IGZvbGxvdyB0aGUgdnQvZmJjb24gaW50ZXJhY3Rpb24gaWYgdGhl cmUgaXMgbm8KPj4gZmJkZXYgZHJpdmVyLi4uICBidXQgdGhlbiBhZ2FpbiBJIGRvbid0IGhhdmUg dmVzYWZiL2VmaWZiIHRvIGNvbnRlbmQKPj4gd2l0aCwgc28gSSdtIGFzc3VtaW5nIHNvbWV0aGlu ZyB0byBkbyB3aXRoIHRoYXQuLgo+Cj4gSXQncyB0aGUgb3RoZXIgd2F5IHJvdW5kOiBUaGVyZSdz IGludGVyYWN0aW9uIHdoZW4gd2UgaGF2ZSBmYmRldiBlbmFibGVkCj4gYmV5b25kIGp1c3QgY2Fs bGluZyBhIGZldyBmYmRldiBoZWxwZXIgZnVuY3Rpb25zLiBBbmQgd2Ugc2hvdWxkIGNvbXBpbGUK PiB0aGF0IG91dCB0b28gc2luY2UgdGhlIGNvbnNvbGVfbG9jayBpcyB3YXkgdG9vIGV2aWwgOy0p Cj4KPiBPbmx5IHdpdGggdGhlc2UgYWRkaXRpb25hbCAjaWZkZWZzIGlzIGk5MTUgY29tcGxldGVs eSBjb25zb2xlX2xvY2sgZnJlZSBpZgo+IHlvdSBkaXNhYmxlIGk5MTUgZmJkZXYgc3VwcG9ydC4g SnVzdCBzdHViYmluZyBvdXQgdGhlIGZiZGV2IGhlbHBlcgo+IGZ1bmN0aW9ucyBpcyBub3QgZW5v dWdoLgo+Cj4+PiBBbmQgYmVjYXVzZSB0aGUgY29uc29sZV9sb2NrIGlzIG1hc3NpdmVseSBjb250 ZW5kZWQgd2UgZG8gdGhhdCBpbiBhIGFzeW5jCj4+PiB3b3JrZXIgaW4gaTkxNS4KPj4+Cj4+PiBC dXQgYW55d2F5IEkgYWdyZWUgaXQgd291bGQgc3RpbGwgc2ltcGx5IGRyaXZlcnMgcXVpdGUgYSBi aXQgaWYgd2UnZCBoYXZlCj4+PiBzdXBwb3J0IGZvciBkdW1teSBmYiBoZWxwZXJzIGluIHRoZSBj b3JlLCBtYXliZSB3aXRoIGFuIGV4cGxpY2l0IEtjb25maWcuCj4+PiBUaGVuIGRyaXZlcnMgY291 bGQgc3dpdGNoIHRvIHVzaW5nIHRoYXQgZm9yIHRoZSBhZGRpdGlvbmFsICNpZmRlZiAobGlrZQo+ Pj4gdGhlIHZ0IHN0dWZmIGk5MTUgZG9lcykgYW5kIG90aGVyd2lzZSByZWx5IHVwb24gZHVtbXkg c3RhdGljIGlubGluZS4gVGhhdAo+Pj4gd291bGQgZ2l2ZSB1cyBmYmRldi1sZXNzIHN1cHBvcnQg Zm9yIG1vc3QgZHJpdmVycyBmb3IgZnJlZSwgd2hpY2ggaXMga2luZGEKPj4+IG5lYXQuCj4+Cj4+ IEkgZ3Vlc3MgYXQgbGVhc3QgZm9yIGFsbCB0aGUgYXJtIGRyaXZlcnMsIGxpZmUgd2l0aG91dCBm YmRldiBkb2Vzbid0Cj4+IGhhdmUgdGhlc2UgZXh0cmEgY29tcGxpY2F0aW9ucywgc28gYXQgbGVh c3QgdGhleSBjb3VsZCB1c2Ugc3R1YnMuLgo+Cj4gRG9lcyB0aGUgcHJvYmxlbSBzb3VuZCBtb3Jl IHRyaWNreSB3aXRoIHRoZSBhYm92ZSBjbGFyaWZpY2F0aW9uPyBJZiB5b3UKPiBkb24ndCBkbyB0 aGUgZmJfc2V0X3N1c3BlbmQgY2FsbCB0aGVuIEkgZXhwZWN0IHlvdSdsbCBoYXZlIHNvbWUKPiBp bnRlcmVzdGluZyBwcm9ibGVtcy4KPgo+PiBQbHVzLCBJIGtpbmQgb2YgZXhwZWN0IHBob25lL3Rh YmxldC9jaHJvbWVib29rIHR5cGUgc3R1ZmYgd291bGQgbGVhZAo+PiB0aGUgY2hhcmdlIGludG8g YW4gZmJkZXYtbGVzcyB3b3JsZC4uCj4KPiBZZWFoLCB0aGF0J3MgYW5vdGhlciByZWFzb24gdG8g c3VwcG9ydCBmYmRldi1sZXNzIGluIHRoZSBoZWxwZXJzIGluc3RlYWQKPiBvZiBlYWNoIGRyaXZl ci4KCkkgd2FzIHRyeWluZyB0byBjcmVhdGUgYSBwYXRjaCB3aXRoIHRoZSBpZGVhIGFib3ZlLiBU aGlzIHdvcmtzIHdlbGwgaWYgCndlIHdhbnQgdGhlIGtlcm5lbCB0byBzdXBwb3J0IG9ubHkgb25l IERSTSBkcml2ZXIuIElmIHRoZSBrZXJuZWwgCnN1cHBvcnRzIG11bHRpcGxlIHBsYXRmb3JtcyBh bmQgb25lIERSTSBkcml2ZXIgc2V0cyBpdHMgY29uZmlnIHRvIGVuYWJsZSAKbGVnYWN5IGZiZGV2 IGFuZCBhbm90aGVyIGRvZXNuJ3QsIHdlIHN0aWxsIGVuZCB1cCBidWlsZGluZyB0aGUgZmJkZXYg CmhlbHBlciBmdW5jcy4gRHJpdmVycyBidWlsdCB3aXRob3V0IGxlZ2FjeSBmYmRldiB3b3VsZCBu ZWVkIHRvIGJlIHZlcnkgCnN0cmljdChjaGVjayBmb3IgcHJpdi0+ZmJkZXYgbm90IE5VTEwpIHRv IHByZXZlbnQgY2FsbGluZyB0aGVtLgoKVGhlIGZiIGNtYSBoZWxwZXIgYWxzbyBhZGRzIHRvIHRo ZSBkaWZmaWN1bHRpZXMuIFRoZSBjbWEgaGVscGVyIHNlZW1zIHRvIApoYXZlIHNvbWUgZnVuY3Rp b25zIHRoYXQgcHJvdmlkZSBsZWdhY3kgZmJkZXYgc3VwcG9ydCBhbmQgb3RoZXJzIHRoYXQgCmFs bG93IGFsbG9jYXRpb24gb2YgZHJtX2ZyYW1lYnVmZmVycyBhbmQgZ2VtIG9iamVjdHMuIFdlJ2Qg bmVlZCB0byBiZSAKY2FyZWZ1bCBhYm91dCBvdXIgc3R1YiBmdW5jdGlvbnMgbm90IG1lc3Npbmcg dXAgdGhlIGRyaXZlcnMgdXNpbmcgdGhlIGZiIApjbWEgaGVscGVycy4KClJhdGhlciB0aGFuIGNy ZWF0aW5nIGZiIGhlbHBlciBzdHViIGZ1bmN0aW9ucywgbWF5YmUgd2UgY291bGQgaGVscCBlYWNo IApkcm0gZHJpdmVyIGNyZWF0ZSB0d28gdmFyaWFudHMgb2YgZnVuY3Rpb25zIG5lZWRlZCBieSBk cm0gY29yZShsaWtlIApvdXRwdXRfcG9sbF9jaGFuZ2VkIGFuZCBkZXZfbGFzdGNsb3NlKSwgb25l IHZhcmlhbnQgc3VwcG9ydGluZyBsZWdhY3kgCmZiZGV2LCBhbmQgdGhlIG90aGVyIG5vdD8KCkFy Y2hpdAoKLS0gClF1YWxjb21tIElubm92YXRpb24gQ2VudGVyLCBJbmMuIGlzIGEgbWVtYmVyIG9m IENvZGUgQXVyb3JhIEZvcnVtLAphIExpbnV4IEZvdW5kYXRpb24gQ29sbGFib3JhdGl2ZSBQcm9q ZWN0Cl9fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fCmRyaS1k ZXZlbCBtYWlsaW5nIGxpc3QKZHJpLWRldmVsQGxpc3RzLmZyZWVkZXNrdG9wLm9yZwpodHRwOi8v bGlzdHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vZHJpLWRldmVsCg== From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754843AbbCEKGg (ORCPT ); Thu, 5 Mar 2015 05:06:36 -0500 Received: from smtp.codeaurora.org ([198.145.29.96]:56340 "EHLO smtp.codeaurora.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753486AbbCEKGd (ORCPT ); Thu, 5 Mar 2015 05:06:33 -0500 Message-ID: <54F82AA3.4040902@codeaurora.org> Date: Thu, 05 Mar 2015 15:36:27 +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: Rob Clark , "dri-devel@lists.freedesktop.org" , linux-arm-msm , Linux Kernel Mailing List Subject: Re: [PATCH] drm: msm: Fix build when legacy fbdev support isn't set References: <1424687361-17787-1-git-send-email-architt@codeaurora.org> <20150223140903.GY24485@phenom.ffwll.local> <20150223153940.GZ24485@phenom.ffwll.local> In-Reply-To: <20150223153940.GZ24485@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 02/23/2015 09:09 PM, Daniel Vetter wrote: > On Mon, Feb 23, 2015 at 10:03:21AM -0500, Rob Clark wrote: >> On Mon, Feb 23, 2015 at 9:09 AM, Daniel Vetter wrote: >>> On Mon, Feb 23, 2015 at 08:33:36AM -0500, Rob Clark wrote: >>>> On Mon, Feb 23, 2015 at 5:29 AM, Archit Taneja wrote: >>>>> The DRM_KMS_FB_HELPER config is selected only when DRM_MSM_FBDEV config is >>>>> selected. The driver accesses drm_fb_helper_* functions even when legacy fbdev >>>>> support is disabled in msm. Wrap around these functions with #ifdef checks to >>>>> prevent build break. >>>> >>>> hmm, perhaps rather than solving this in each driver, we should do >>>> some stub versions of those fb-helper fxns? >>>> >>>> There are at least one or two other drivers that can build without >>>> fbdev, and I guess more going forward.. >>> >>> It's not quite that easy since you also have to start/stop the vt >>> subsystem at the right point in time in your own driver. See >>> intel_fbdev_set_suspend. If you don't do that there's no synchronization >>> between fbcon shutting down/resuming and your driver, which in the best >>> case means fbcon does some writes to nowhere and worst case means your >>> chip dies (mmio to offline chip blocks) or writes go to somewhere random >>> in system memory (iommu contains some stale ptes since not yet fully >>> restored, more an issue with hibernate). >> >> I guess I don't fully follow the vt/fbcon interaction if there is no >> fbdev driver... but then again I don't have vesafb/efifb to contend >> with, so I'm assuming something to do with that.. > > It's the other way round: There's interaction when we have fbdev enabled > beyond just calling a few fbdev helper functions. And we should compile > that out too since the console_lock is way too evil ;-) > > Only with these additional #ifdefs is i915 completely console_lock free if > you disable i915 fbdev support. Just stubbing out the fbdev helper > functions is not enough. > >>> And because the console_lock is massively contended we do that in a async >>> worker in i915. >>> >>> But anyway I agree it would still simply drivers quite a bit if we'd have >>> support for dummy fb helpers in the core, maybe with an explicit Kconfig. >>> Then drivers could switch to using that for the additional #ifdef (like >>> the vt stuff i915 does) and otherwise rely upon dummy static inline. That >>> would give us fbdev-less support for most drivers for free, which is kinda >>> neat. >> >> I guess at least for all the arm drivers, life without fbdev doesn't >> have these extra complications, so at least they could use stubs.. > > Does the problem sound more tricky with the above clarification? If you > don't do the fb_set_suspend call then I expect you'll have some > interesting problems. > >> Plus, I kind of expect phone/tablet/chromebook type stuff would lead >> the charge into an fbdev-less world.. > > Yeah, that's another reason to support fbdev-less in the helpers instead > of each driver. I was trying to create a patch with the idea above. This works well if we want the kernel to support only one DRM driver. If the kernel supports multiple platforms and one DRM driver sets its config to enable legacy fbdev and another doesn't, we still end up building the fbdev helper funcs. Drivers built without legacy fbdev would need to be very strict(check for priv->fbdev not NULL) to prevent calling them. The fb cma helper also adds to the difficulties. The cma helper seems to have some functions that provide legacy fbdev support and others that allow allocation of drm_framebuffers and gem objects. We'd need to be careful about our stub functions not messing up the drivers using the fb cma helpers. Rather than creating fb helper stub functions, maybe we could help each drm driver create two variants of functions needed by drm core(like output_poll_changed and dev_lastclose), one variant supporting legacy fbdev, and the other not? Archit -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project