Re: [PATCH 3/4] drm/fb-helper: Add module option to disable fbdev emulation

From: Daniel Vetter <daniel@ffwll.ch>
To: Archit Taneja <architt@codeaurora.org>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>,
	Intel Graphics Development <intel-gfx@lists.freedesktop.org>,
	DRI Development <dri-devel@lists.freedesktop.org>
Subject: Re: [PATCH 3/4] drm/fb-helper: Add module option to disable fbdev emulation
Date: Wed, 26 Aug 2015 14:51:39 +0200	[thread overview]
Message-ID: <20150826125139.GN1367@phenom.ffwll.local> (raw)
In-Reply-To: <55DDB11A.60000@codeaurora.org>

On Wed, Aug 26, 2015 at 05:59:14PM +0530, Archit Taneja wrote:
> 
> 
> On 08/26/2015 05:07 PM, Daniel Vetter wrote:
> >On Wed, Aug 26, 2015 at 01:34:58PM +0200, Daniel Vetter wrote:
> >>On Wed, Aug 26, 2015 at 02:14:37PM +0530, Archit Taneja wrote:
> >>>
> >>>
> >>>On 08/26/2015 10:42 AM, Archit Taneja wrote:
> >>>>
> >>>>
> >>>>On 08/25/2015 07:15 PM, Daniel Vetter wrote:
> >>>>>Faster than recompiling.
> >>>>>
> >>>>>Note that restore_fbdev_mode_unlocked is a bit special and the only
> >>>>>one which returns an error code when fbdev isn't there - i915 needs
> >>>>>that one to not fall over with some additional fbcon related restore
> >>>>>code. Everyone else just ignores the return value or only prints a
> >>>>>DRM_DEBUG level message.
> >>>>
> >>>>Reviewed-by:Archit Taneja <architt@codeaurora.org>
> >>>
> >>>
> >>>With the module param, and the drivers should see the following state(
> >>>based on the truth table below):
> >>>
> >>>module param | config option
> >>>    true      |    true       -> real helper funcs called, driver
> >>>                                 allocated drm_fb_helper is correctly
> >>>                                 populated.
> >>>
> >>>    false     |    true       -> real helper funcs called, but bailed
> >>>                                 out early, driver allocated
> >>>                                 drm_fb_helper is non-NULL, but we won't
> >>>                                 end up using it.
> >>
> >>Hm I tried to give drivers the same semantics here as with the stub
> >>functions. Where did I screw up? The goal really was to match the end
> >>result for drivers ...
> >
> >Note that at least for i915 we can't make it perfectly equal since i915
> >compiles out a few more things with FBDEV_EMULATION=n than just the stubs.
> >Long-term we might want to push some of that into helpers too perhaps.
> 
> Ah, I missed looking at this mail.

Yeah we had to go ahead with removing I915_FBDEV since it was causing
trouble if you disable one but not the other. I think i915 is the only
driver where this can happen though, the others with their own fbdev
Kconfig option disable a lot less.

> Yeah, that's what I wanted to mainly point out. It looks okay
> otherwise.
> 
> Since the param is 'true' by default. Things should be okay for all
> drivers. If someone reports an issue with a driver with the above
> combination, we could fix it individually.
> 
> I guess the next step now is to remove the custom config fbdev
> emulation options and module params from drivers that have
> those.
> 
> After that, we could start with scary process of removing the
> CONFIG_FB and CONFIG_DRM_KMS_FB_HELPER from each driver.

Yeah, definitely should do that for 4.4. I'll pull in this one here with
your r-b too, thanks for the feedback.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel