On 03/05/2015 09:14 PM, Daniel Vetter wrote: > On Thu, Mar 05, 2015 at 07:10:44AM -0500, Rob Clark wrote: >> On Thu, Mar 5, 2015 at 5:06 AM, Archit Taneja <architt at codeaurora.org> >> wrote: >>> >>> 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 <daniel at ffwll.ch> 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 <architt at >>>>>>> codeaurora.org> >>>>>>> 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? >> >> So one quick thought.. building without fbdev would ideally/eventually >> be a distro level decision, not a driver level decision.. so I think >> it is *eventually* not a problem for it being a global drm level >> decision. The only problem is right now some drivers support no-fbdev >> config, and some do not. Maybe it is worth fixing that? > > Yeah, if we get fbdev emulation Kconfig option then I think i915 and msm > should remove their own options and just use that. There's really no need > to have this per-driver, this is a question of what userspace expects and > so per-distro, independant of the driver. > -Daniel >
Okay. If I understand right. We need to do something like this: - Create a new Kconfig option that lets us emulate fbdev - For drivers that already have a config for fbdev emulation, replace it with our new emulation config. - For drivers that assume fbdev emulation by default, select our new emulation config in their respective Kconfigs. Does this sound okay? I suppose this could be the first step. Later we'd need to work on each driver to work with and without the fbdev emulation Kconfig option. Archit -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
