Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751203AbbF3JHA (ORCPT ); Tue, 30 Jun 2015 05:07:00 -0400 Received: from smtp.codeaurora.org ([198.145.29.96]:54682 "EHLO smtp.codeaurora.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750966AbbF3JGx (ORCPT ); Tue, 30 Jun 2015 05:06:53 -0400 Message-ID: <55925C24.8030909@codeaurora.org> Date: Tue, 30 Jun 2015 14:36:44 +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: Benjamin Gaignard CC: Daniel Vetter , "Clark, Rob" , Dave Airlie , Thierry Reding , Philipp Zabel , 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> <55924BA6.4050309@codeaurora.org> 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: 7763 Lines: 234 Hi, On 06/30/2015 02:01 PM, Benjamin Gaignard wrote: > Hi, > > I think that what have been done by Rob with module_param is also a good idea: > https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/commit/drivers/gpu/drm/msm/msm_drv.c?id=e90dfec78ec288d6c89a7b508a5c5d4ae8b7f934 > > Can you mix compilation flag and module param ? I guess this won't be hard to do once we have a common fbdev emulation config option. We could consider this as a drm top level module param. I'll keep this in mind. Thanks, Archit > > Benjamin > > 2015-06-30 9:56 GMT+02:00 Archit Taneja : >> 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 > > > -- 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/