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

From: Daniel Vetter <daniel@ffwll.ch>
To: Archit Taneja <architt@codeaurora.org>
Cc: dri-devel@lists.freedesktop.org
Subject: Re: [PATCH RESEND v2 06/25] drm/fb_helper: Create a wrapper for remove_conflicting_framebuffers
Date: Tue, 28 Jul 2015 20:46:24 +0200	[thread overview]
Message-ID: <20150728184624.GK16722@phenom.ffwll.local> (raw)
In-Reply-To: <1437557300-3280-7-git-send-email-architt@codeaurora.org>

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

Thanks, Daniel
> +
>  static int setcolreg(struct drm_crtc *crtc, u16 red, u16 green,
>  		     u16 blue, u16 regno, struct fb_info *info)
>  {
> diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h
> index ef32500..cbdc69d 100644
> --- a/include/drm/drm_fb_helper.h
> +++ b/include/drm/drm_fb_helper.h
> @@ -168,6 +168,10 @@ void drm_fb_helper_cfb_imageblit(struct fb_info *info,
>  
>  void drm_fb_helper_set_suspend(struct drm_fb_helper *fb_helper, int state);
>  
> +int drm_fb_helper_remove_conflicting_framebuffers(struct apertures_struct *a,
> +						  const char *name,
> +						  bool primary);
> +
>  int drm_fb_helper_setcmap(struct fb_cmap *cmap, struct fb_info *info);
>  
>  int drm_fb_helper_hotplug_event(struct drm_fb_helper *fb_helper);
> -- 
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> hosted by The Linux Foundation
> 

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