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@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? 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 -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- 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
