Re: [RFC 3/6] drm/i915: Remove local fbdev emulation Kconfig option

[Daniel Vetter <daniel@ffwll.ch>]

On Tue, Mar 10, 2015 at 03:11:30PM +0530, Archit Taneja wrote:
> DRM_I915_FBDEV config is currently used to enable/disable fbdev emulation for
> the i915 kms driver.
> 
> Replace this with the top level DRM_FBDEV_EMULATION config option. Using this
> config lets us also prevent wrapping around drm_fb_helper_* calls with #ifdefs
> in certain places.
> 
> The #ifdef in drm_i915_private struct adding/removing members intel_fbdev and
> fbdev_suspend_work has been removed. This helps us use stub drm helper functions
> at not much cost.
> 
> Signed-off-by: Archit Taneja <architt@codeaurora.org>
> ---
>  drivers/gpu/drm/i915/Kconfig         | 15 ---------------
>  drivers/gpu/drm/i915/Makefile        |  4 ++--
>  drivers/gpu/drm/i915/i915_debugfs.c  |  2 +-
>  drivers/gpu/drm/i915/i915_drv.h      |  2 --
>  drivers/gpu/drm/i915/intel_display.c | 10 ++++------
>  drivers/gpu/drm/i915/intel_dp_mst.c  | 14 ++++++++------
>  drivers/gpu/drm/i915/intel_drv.h     |  3 +--
>  drivers/gpu/drm/i915/intel_fbdev.c   |  7 -------
>  8 files changed, 16 insertions(+), 41 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig
> index 74acca9..bd9ccfc 100644
> --- a/drivers/gpu/drm/i915/Kconfig
> +++ b/drivers/gpu/drm/i915/Kconfig
> @@ -45,21 +45,6 @@ config DRM_I915_KMS
>  
>  	  If in doubt, say "Y".
>  
> -config DRM_I915_FBDEV
> -	bool "Enable legacy fbdev support for the modesetting intel driver"
> -	depends on DRM_I915
> -	select DRM_KMS_FB_HELPER
> -	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 the intel modesetting driver.
> -
> -	  If in doubt, say "Y".
> -
>  config DRM_I915_PRELIMINARY_HW_SUPPORT
>  	bool "Enable preliminary support for prerelease Intel hardware by default"
>  	depends on DRM_I915
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index f019225..3b3325d 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -56,8 +56,8 @@ i915-y += intel_audio.o \
>  	  intel_psr.o \
>  	  intel_sideband.o \
>  	  intel_sprite.o
> -i915-$(CONFIG_ACPI)		+= intel_acpi.o intel_opregion.o
> -i915-$(CONFIG_DRM_I915_FBDEV)	+= intel_fbdev.o
> +i915-$(CONFIG_ACPI)			+= intel_acpi.o intel_opregion.o
> +i915-$(CONFIG_DRM_FBDEV_EMULATION)	+= intel_fbdev.o
>  
>  # modesetting output/encoder code
>  i915-y += dvo_ch7017.o \
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index e8b18e5..0c8bcd7 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -1772,7 +1772,7 @@ static int i915_gem_framebuffer_info(struct seq_file *m, void *data)
>  	struct intel_fbdev *ifbdev = NULL;
>  	struct intel_framebuffer *fb;
>  
> -#ifdef CONFIG_DRM_I915_FBDEV
> +#ifdef CONFIG_DRM_FBDEV_EMULATION
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  
>  	ifbdev = dev_priv->fbdev;
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 8727086..e511fa4 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1819,11 +1819,9 @@ struct drm_i915_private {
>  
>  	struct drm_i915_gem_object *vlv_pctx;
>  
> -#ifdef CONFIG_DRM_I915_FBDEV
>  	/* list of fbdev register on this device */
>  	struct intel_fbdev *fbdev;
>  	struct work_struct fbdev_suspend_work;
> -#endif
>  
>  	struct drm_property *broadcast_rgb_property;
>  	struct drm_property *force_audio_property;
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index e730789..9cf13e6 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -8562,7 +8562,6 @@ static struct drm_framebuffer *
>  mode_fits_in_fbdev(struct drm_device *dev,
>  		   struct drm_display_mode *mode)
>  {
> -#ifdef CONFIG_DRM_I915_FBDEV
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	struct drm_i915_gem_object *obj;
>  	struct drm_framebuffer *fb;
> @@ -8585,9 +8584,6 @@ mode_fits_in_fbdev(struct drm_device *dev,
>  		return NULL;
>  
>  	return fb;
> -#else
> -	return NULL;
> -#endif

Won't this now blow up if there's no fbdev? Tbh I prefer a patch to just
replace the CONFIG_ #define and then maybe a 2nd one on top to clean
things up if you feel like. Same for the other changes in this patch.
-Daniel

>  }
>  
>  bool intel_get_load_detect_pipe(struct drm_connector *connector,
> @@ -12807,11 +12803,13 @@ intel_user_framebuffer_create(struct drm_device *dev,
>  	return intel_framebuffer_create(dev, mode_cmd, obj);
>  }
>  
> -#ifndef CONFIG_DRM_I915_FBDEV
>  static inline void intel_fbdev_output_poll_changed(struct drm_device *dev)
>  {
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +
> +	if (dev_priv->fbdev)
> +		drm_fb_helper_hotplug_event(&dev_priv->fbdev->helper);
>  }
> -#endif
>  
>  static const struct drm_mode_config_funcs intel_mode_funcs = {
>  	.fb_create = intel_user_framebuffer_create,
> diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c
> index 9f67a37..fe69df0 100644
> --- a/drivers/gpu/drm/i915/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/intel_dp_mst.c
> @@ -376,18 +376,20 @@ static bool intel_dp_mst_get_hw_state(struct intel_connector *connector)
>  
>  static void intel_connector_add_to_fbdev(struct intel_connector *connector)
>  {
> -#ifdef CONFIG_DRM_I915_FBDEV
>  	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
> -	drm_fb_helper_add_one_connector(&dev_priv->fbdev->helper, &connector->base);
> -#endif
> +
> +	if (dev_priv->fbdev)
> +		drm_fb_helper_add_one_connector(&dev_priv->fbdev->helper,
> +			&connector->base);
>  }
>  
>  static void intel_connector_remove_from_fbdev(struct intel_connector *connector)
>  {
> -#ifdef CONFIG_DRM_I915_FBDEV
>  	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
> -	drm_fb_helper_remove_one_connector(&dev_priv->fbdev->helper, &connector->base);
> -#endif
> +
> +	if (dev_priv->fbdev)
> +		drm_fb_helper_remove_one_connector(&dev_priv->fbdev->helper,
> +			&connector->base);
>  }
>  
>  static struct drm_connector *intel_dp_add_mst_connector(struct drm_dp_mst_topology_mgr *mgr, struct drm_dp_mst_port *port, const char *pathprop)
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index eef79cc..6920da2 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1062,12 +1062,11 @@ void intel_dvo_init(struct drm_device *dev);
>  
>  
>  /* legacy fbdev emulation in intel_fbdev.c */
> -#ifdef CONFIG_DRM_I915_FBDEV
> +#ifdef CONFIG_DRM_FBDEV_EMULATION
>  extern int intel_fbdev_init(struct drm_device *dev);
>  extern void intel_fbdev_initial_config(void *data, async_cookie_t cookie);
>  extern void intel_fbdev_fini(struct drm_device *dev);
>  extern void intel_fbdev_set_suspend(struct drm_device *dev, int state, bool synchronous);
> -extern void intel_fbdev_output_poll_changed(struct drm_device *dev);
>  extern void intel_fbdev_restore_mode(struct drm_device *dev);
>  #else
>  static inline int intel_fbdev_init(struct drm_device *dev)
> diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c
> index 3001a867..656ec1c 100644
> --- a/drivers/gpu/drm/i915/intel_fbdev.c
> +++ b/drivers/gpu/drm/i915/intel_fbdev.c
> @@ -762,13 +762,6 @@ void intel_fbdev_set_suspend(struct drm_device *dev, int state, bool synchronous
>  	console_unlock();
>  }
>  
> -void intel_fbdev_output_poll_changed(struct drm_device *dev)
> -{
> -	struct drm_i915_private *dev_priv = dev->dev_private;
> -	if (dev_priv->fbdev)
> -		drm_fb_helper_hotplug_event(&dev_priv->fbdev->helper);
> -}
> -
>  void intel_fbdev_restore_mode(struct drm_device *dev)
>  {
>  	int ret;
> -- 
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> hosted by The Linux Foundation
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch