Re: [PATCH] drm: msm: Fix build when legacy fbdev support isn't set

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





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@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


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
--
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




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux