On Thu, Mar 5, 2015 at 5:06 AM, Archit Taneja <architt@xxxxxxxxxxxxxx> 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@xxxxxxxx> 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@xxxxxxxxxxxxxx> >>>>> 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? BR, -R > Archit > > -- > 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-arm-msm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
