Re: [PATCH RESEND v2 06/25] drm/fb_helper: Create a wrapper for remove_conflicting_framebuffers

[Daniel Vetter <daniel@ffwll.ch>]

On Wed, Jul 29, 2015 at 09:50:56AM +0530, Archit Taneja wrote:
> 
> 
> On 07/29/2015 12:19 AM, Daniel Vetter wrote:
> >On Tue, Jul 28, 2015 at 08:46:24PM +0200, Daniel Vetter wrote:
> >>On Wed, Jul 22, 2015 at 02:58:01PM +0530, Archit Taneja wrote:
> >>>Some drm drivers call remove_conflicting_framebuffers. Create a
> >>>drm_fb_helper function that wraps around these calls.
> >>>
> >>>This is part of an effort to prevent drm drivers from calling fbdev
> >>>functions directly, in order to make fbdev emulation a top level drm
> >>>option.
> >>>
> >>>v2:
> >>>- Added kerneldocs
> >>>- Follow the drm way of aligning of arguments in func definitions
> >>>
> >>>Signed-off-by: Archit Taneja <architt@codeaurora.org>
> >>>---
> >>>  drivers/gpu/drm/drm_fb_helper.c | 15 +++++++++++++++
> >>>  include/drm/drm_fb_helper.h     |  4 ++++
> >>>  2 files changed, 19 insertions(+)
> >>>
> >>>diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> >>>index 9620aa5..86e4e2c 100644
> >>>--- a/drivers/gpu/drm/drm_fb_helper.c
> >>>+++ b/drivers/gpu/drm/drm_fb_helper.c
> >>>@@ -894,6 +894,21 @@ void drm_fb_helper_set_suspend(struct drm_fb_helper *fb_helper, int state)
> >>>  }
> >>>  EXPORT_SYMBOL(drm_fb_helper_set_suspend);
> >>>
> >>>+/**
> >>>+ * drm_fb_helper_remove_conflicting_framebuffers - wrapper around
> >>>+ *						remove_conflicting_framebuffers
> >>>+ * @fb_helper: driver-allocated fbdev helper
> >>>+ *
> >>>+ * A wrapper around remove_conflicting_framebuffers implemented by fbdev core
> >>>+ */
> >>>+int drm_fb_helper_remove_conflicting_framebuffers(struct apertures_struct *a,
> >>>+						  const char *name,
> >>>+						  bool primary)
> >>>+{
> >>>+	return remove_conflicting_framebuffers(a, name, primary);
> >>>+}
> >>>+EXPORT_SYMBOL(drm_fb_helper_remove_conflicting_framebuffers);
> >>
> >>Chris Wilson reported on irc that if he builds with I915_FBDEV=n then
> >>there's a missing symbol problem here because i915 wants to kick out other
> >>framebuffers (well we have to for correctness) even when fbdev emulation
> >>is disabled. If you look at i915_dma.c you'll see that the call to
> >>remove_conflicting_framebuffers is conditional upon CONFIG_FB and not
> >>CONFIG_I915_FBDEV.
> >>
> >>I think the proper solution here would be to provide a static inline
> >>helper for remove_conflicting_framebuffer itself when CONFIG_FB=n, since a
> >>all drivers who call this still want to call this even when fbdev
> >>emulation is disabled.
> >>
> >>Plan B would be to move this into core drm and make the static inline one
> >>selected for CONFIG_FB=n. If you do that then maybe also rename it to
> >>drm_remove_conflicting_framebuffer since this really is only tangetial to
> >>fbdev emulation.
> >>
> >>Anyway current patch unfortunately won't work since we really must remove
> >>other framebuffers even in the FB=y, I915_FBDEV=n case. So I dropped it
> >>from drm-misc for now. Unfortunately that means all the driver conversions
> >>won't apply anymore either :(
> >
> >Ok I didn't have to drop all of the driver specific patches since all the
> >arm ones don't have calls to remove_conflicting_framebuffers.
> 
> I can re-spin the x86 ones and make them stick to the original
> remove_conflicting_framebuffers call. This way we can at least use the
> other helpers.

Yeah that would work. btw for the kerneldoc I guess it would be easier if
you just provide a fixup patch, changed my mind there.
-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