Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1750930AbbF3H4i (ORCPT ); Tue, 30 Jun 2015 03:56:38 -0400 Received: from smtp.codeaurora.org ([198.145.29.96]:51175 "EHLO smtp.codeaurora.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751612AbbF3H4a (ORCPT ); Tue, 30 Jun 2015 03:56:30 -0400 Message-ID: <55924BA6.4050309@codeaurora.org> Date: Tue, 30 Jun 2015 13:26:22 +0530 From: Archit Taneja User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.6.0 MIME-Version: 1.0 To: Daniel Vetter , "Clark, Rob" , Dave Airlie , Thierry Reding , Philipp Zabel , Benjamin Gaignard , dri-devel , Linux Kernel Mailing List , linux-arm-msm , Jani Nikula , Linux Fbdev development list , Tomi Valkeinen Subject: Re: [RFC 1/6] drm: Add top level Kconfig option for DRM fbdev emulation References: <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> <55126F32.90404@codeaurora.org> <20150325092111.GT1349@phenom.ffwll.local> In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 6283 Lines: 162 Hi, On 06/30/2015 12:40 PM, Daniel Vetter wrote: > Any updates on this or too much distractions? I really think doing > this would be awesome for the drm subsystem, instead of reinventing > this wheel for each driver. I'd started on this again. I've got more free time now than before, so I should have something in a week or so. I agree it will help a lot (there are already two new drivers since we started discussing this!) Archit > -Daniel > > On Wed, Mar 25, 2015 at 10:21 AM, Daniel Vetter wrote: >> On Wed, Mar 25, 2015 at 01:47:54PM +0530, Archit Taneja wrote: >>> 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? >> >> That's actually an option I considered, but I hoped we could do it with >> less work. If this indeed works and you can create the patch that would be >> awesome. >> >> Thanks, Daniel >> -- >> Daniel Vetter >> Software Engineer, Intel Corporation >> http://blog.ffwll.ch > > > -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/