Re: [RFC 1/6] drm: Add top level Kconfig option for DRM fbdev emulation

[Archit Taneja]

On 03/10/2015 05:47 PM, Daniel Vetter wrote:
> On Tue, Mar 10, 2015 at 03:52:41PM +0530, Archit Taneja wrote:
> > On 03/10/2015 03:35 PM, Daniel Vetter wrote:
> > > On Tue, Mar 10, 2015 at 03:22:49PM +0530, Archit Taneja wrote:
> > > > On 03/10/2015 03:17 PM, Daniel Vetter wrote:
> > > > > On Tue, Mar 10, 2015 at 03:11:28PM +0530, Archit Taneja wrote:
> > > > > > diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> > > > > > index 151a050..38f83a0 100644
> > > > > > --- a/drivers/gpu/drm/Kconfig
> > > > > > +++ b/drivers/gpu/drm/Kconfig
> > > > > > @@ -40,6 +40,24 @@ config DRM_KMS_FB_HELPER
> > > > > >   	help
> > > > > >   	  FBDEV helpers for KMS drivers.
> > > > > >
> > > > > > +config DRM_FBDEV_EMULATION
> > > > > > +	bool "Enable legacy fbdev support for your modesetting driver"
> > > > > > +	depends on DRM
> > > > > > +	select DRM_KMS_HELPER
> > > > > > +	select DRM_KMS_FB_HELPER
> > > > > > +	select FB_SYS_FILLRECT
> > > > > > +	select FB_SYS_COPYAREA
> > > > > > +	select FB_SYS_IMAGEBLIT
> > > > > > +	select FB_SYS_FOPS
> > > > > > +	select FB_CFB_FILLRECT
> > > > > > +	select FB_CFB_COPYAREA
> > > > > > +	select FB_CFB_IMAGEBLIT
> > > > > > +	default y
> > > > > > +	help
> > > > > > +	  Choose this option if you have a need for the legacy fbdev
> > > > > > +	  support. Note that this support also provide the linux console
> > > > > > +	  support on top of your modesetting driver.
> > > > >
> > > > > Maybe clarify that for linux console support you also need
> > > > > CONFIG_FRAMEBUFFER_CONSOLE? fbdev alone isn't enough.
> > > >
> > > > DRM_KMS_FB_HELPER selects that for us, right?
> > >
> > > Hm right I've missed that. Reminds me that you need one more patch at the
> > > end to remove all the various select DRM_KMS_FB_HELPER from all drm
> > > drivers. Otherwise this knob here won't work by default if you e.g. select
> > > radeon. In general we can't mix explicit options with menu entries with a
> > > select.
> >
> > I was trying that out. Removing DRM_KMS_FB_HELPER and having
> > DRM_FBDEV_EMULATION disabled breaks drivers which use FB stuff internally in
> > their respective xyz_fbdev.c files.
>
> But with the stubbed out functions that should work, right? Why doesn't
> it?

There are still calls to functions from fb core like fb_set_suspend and 
register_framebuffer which aren't covered by the drm fb helper functions.

> > Are you saying that we should remove DRM_KMS_FB_HELPER for such drivers and
> > replace them with 'select DRM_FBDEV_EMULATION'?
> >
> > Another option would be to provide #ifdef DRM_FBDEV_EMULATION wrap-arounds
> > for fb related function calls/declarations in each driver, something that's
> > already done for i915 and msm drivers.
>
> The problem with the patch as-is the massive amounts of selects the FB
> helper still has. We need to get rid of them so that when you disable
> fbdev emulation you can indeed disable fbcon and the entire fbdev
> subsystem. I've thought that remove the hidden symbol DRM_KMS_FB_HELPER
> and moving all the selects to DRM_FBDEV_EMULATION should work out? If that
> doesn't work we need to look again how to better stub things out I think.

That's true. Also, as Jani pointed out, maybe it isn't the best idea to 
make every kms driver select DRM_FBDEV_EMULATION?

Maybe the drivers that are currently built with fbdev emulation by 
default can add a 'depends on DRM_FBDEV_EMULATION'? Since the config 
defaults to y, the drivers should run as is. Later, we could take up 
each driver and build the fb stuff only when DRM_FBDEV_EMULATION is set, 
like how we do for i915 and msm?

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