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

[Daniel Vetter]

On Mon, Mar 09, 2015 at 02:24:09PM +0530, Archit Taneja wrote:
> 
> 
> On 03/09/2015 01:14 PM, Daniel Vetter wrote:
> >On Mon, Mar 09, 2015 at 11:27:06AM +0530, Archit Taneja wrote:
> >>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:
> >>>>>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.
> >
> >See Rob's idea quoted above: Imo you should just do the conditional code
> >in the fbdev emulation helper in drm_fb_helper.c, not in drivers. There's
> >some additional code in i915 (and maybe also msm) to compile out (mostly
> >around taking/dropping console_lock) and we want to keep that. But that
> >should also just be using the same new config option. So:
> >
> >- Add new config option DRM_FBDEV_EMULATION (default y for backwards
> >   compat). Use that to stub out helpers in drm_fb_helper.c.
> >
> >- Replace the msm/i915 specific options with the new, i.e. remove it from
> >   Kconfig and use DRM_FBDEV_EMULATION in the code instead of the msm/i915
> >   specific one.
> >
> >fbdev-less support for all other drivers will just magically happen
> >without the need for driver-specific code (except for the special locking
> >in i915 and similar things maybe).
> 
> That sounds good. I wasn't totally sure about fbdev-less support working out
> of the box on devices that use the fb CMA helpers. The drm_fbdev_cma_* funcs
> internally use the fb helper functions. Stubbing the fb helpers out might
> result in some corner cases.
> 
> However, after a quick look at that the cma helpers, it looks like it take
> all the necessary precautions to prevent uninitialized accesses. Although,
> it would be ideal to stub out the drm_fbdev_cma_* helper functions too since
> they don't serve any purpose when fbdev emulation is disabled.

I think you could do that as part of the fbdev less work, but in a
separate patch. Like Rob says the important part is that we don't set up
an fbdev emulation. Taking care of surrounding code like cma (or the
special stuff we have in i915) is just icing on the cake.
-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-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/