Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751767AbbF3Ibt (ORCPT ); Tue, 30 Jun 2015 04:31:49 -0400 Received: from mail-la0-f53.google.com ([209.85.215.53]:35110 "EHLO mail-la0-f53.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752080AbbF3Ibh convert rfc822-to-8bit (ORCPT ); Tue, 30 Jun 2015 04:31:37 -0400 MIME-Version: 1.0 In-Reply-To: <55924BA6.4050309@codeaurora.org> 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> Date: Tue, 30 Jun 2015 10:31:35 +0200 Message-ID: Subject: Re: [RFC 1/6] drm: Add top level Kconfig option for DRM fbdev emulation From: Benjamin Gaignard To: Archit Taneja 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 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8BIT Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 7324 Lines: 226 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 ? 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 -- Benjamin Gaignard Graphic Working Group Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog -- 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/