[RFC 0/6] drm: Add DRM_FBDEV_EMULATION Kconfig option

[RFC 0/6] drm: Add DRM_FBDEV_EMULATION Kconfig option

[Archit Taneja]

This provides a uniform interface to enable/disable legacy fbdev support for
modesetting drivers, based on the discussion here:

http://lists.freedesktop.org/archives/dri-devel/2015-March/078729.html

This has only been build-tested for a few devices. Mainly looking for comments
for now.

The drivers that provide fbdev emulation by default won't be impacted by
this. However, if we could make all drivers use DRM_FBDEV_EMULATION, it
would clean up individual Kconfigs, and have a centralized place where we
touch FB_* configs.

Archit Taneja (6):
  drm: Add top level Kconfig option for DRM fbdev emulation
  drm/msm: Remove local fbdev emulation Kconfig option
  drm/i915: Remove local fbdev emulation Kconfig option
  drm/tegra: Remove local fbdev emulation Kconfig option
  drm/imx: Remove local fbdev emulation Kconfig option
  drm/sti: Remove local fbdev emulation Kconfig option

 drivers/gpu/drm/Kconfig              |  18 ++++++
 drivers/gpu/drm/Makefile             |   4 ++
 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 --
 drivers/gpu/drm/imx/Kconfig          |   9 ---
 drivers/gpu/drm/imx/imx-drm-core.c   |  10 +--
 drivers/gpu/drm/msm/Kconfig          |  14 ----
 drivers/gpu/drm/msm/Makefile         |   2 +-
 drivers/gpu/drm/msm/msm_drv.c        |   4 +-
 drivers/gpu/drm/sti/Kconfig          |   6 --
 drivers/gpu/drm/sti/sti_drm_drv.c    |   2 +-
 drivers/gpu/drm/tegra/Kconfig        |  12 ----
 drivers/gpu/drm/tegra/drm.c          |  15 +++--
 drivers/gpu/drm/tegra/drm.h          |   8 ---
 drivers/gpu/drm/tegra/fb.c           |  25 ++------
 include/drm/drm_fb_helper.h          | 120 +++++++++++++++++++++++++++++++++++
 22 files changed, 179 insertions(+), 127 deletions(-)

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

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

[Archit Taneja]

Legacy fbdev emulation support via DRM is achieved through KMS FB helpers.
Most modesetting drivers enable provide fbdev emulation by default by selecting
KMS FB helpers. A few provide a separate Kconfig option for the user to enable
or disbale fbdev emulation.

Enabling fbdev emulation is finally a distro-level decision. Having a top level
Kconfig option for fbdev emulation helps by providing a uniform way to
enable/disable fbdev emulation for any modesetting driver. It also lets us
remove unnecessary driver specific Kconfig options that causes bloat.

With a top level Kconfig in place, we can stub out the fb helper functions when
not needed without breaking functionality. Having stub functions also prevents
drivers to require wrapping fb helper function calls with #ifdefs.

DRM_FBDEV_EMULATION defaults to y since many drivers enable fbdev emulation by
default and majority of distributions expect the fbdev interface in the kernel.

For now, this config selects both FB_SYS_* and FB_CFB_* configs as some
modesetting drivers use the former and other the later. This needs to be taken
care of in a better way.

Signed-off-by: Archit Taneja <architt@codeaurora.org>
---
 drivers/gpu/drm/Kconfig     |  18 +++++++
 drivers/gpu/drm/Makefile    |   4 ++
 include/drm/drm_fb_helper.h | 120 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 142 insertions(+)

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.
+
 config DRM_LOAD_EDID_FIRMWARE
 	bool "Allow to specify an EDID data set instead of probing for it"
 	depends on DRM_KMS_HELPER
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index 2c239b9..c1d44b2 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -25,7 +25,11 @@ drm-$(CONFIG_OF) += drm_of.o
 drm_kms_helper-y := drm_crtc_helper.o drm_dp_helper.o drm_probe_helper.o \
 		drm_plane_helper.o drm_dp_mst_topology.o drm_atomic_helper.o
 drm_kms_helper-$(CONFIG_DRM_LOAD_EDID_FIRMWARE) += drm_edid_load.o
+
+ifeq ($(CONFIG_DRM_FBDEV_EMULATION),y)
 drm_kms_helper-$(CONFIG_DRM_KMS_FB_HELPER) += drm_fb_helper.o
+endif
+
 drm_kms_helper-$(CONFIG_DRM_KMS_CMA_HELPER) += drm_fb_cma_helper.o
 
 obj-$(CONFIG_DRM_KMS_HELPER) += drm_kms_helper.o
diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h
index 21b944c..dbfce1a 100644
--- a/include/drm/drm_fb_helper.h
+++ b/include/drm/drm_fb_helper.h
@@ -103,6 +103,7 @@ struct drm_fb_helper {
 	bool delayed_hotplug;
 };
 
+#ifdef CONFIG_DRM_FBDEV_EMULATION
 void drm_fb_helper_prepare(struct drm_device *dev, struct drm_fb_helper *helper,
 			   const struct drm_fb_helper_funcs *funcs);
 int drm_fb_helper_init(struct drm_device *dev,
@@ -139,4 +140,123 @@ drm_pick_cmdline_mode(struct drm_fb_helper_connector *fb_helper_conn,
 int drm_fb_helper_add_one_connector(struct drm_fb_helper *fb_helper, struct drm_connector *connector);
 int drm_fb_helper_remove_one_connector(struct drm_fb_helper *fb_helper,
 				       struct drm_connector *connector);
+#else
+static inline void drm_fb_helper_prepare(struct drm_device *dev,
+					struct drm_fb_helper *helper,
+					const struct drm_fb_helper_funcs *funcs)
+{
+}
+
+static inline int drm_fb_helper_init(struct drm_device *dev,
+		       struct drm_fb_helper *helper, int crtc_count,
+		       int max_conn)
+{
+	return 0;
+}
+
+static inline void drm_fb_helper_fini(struct drm_fb_helper *helper)
+{
+}
+
+static inline int drm_fb_helper_blank(int blank, struct fb_info *info)
+{
+	return 0;
+}
+
+static inline int drm_fb_helper_pan_display(struct fb_var_screeninfo *var,
+					    struct fb_info *info)
+{
+	return 0;
+}
+
+static inline int drm_fb_helper_set_par(struct fb_info *info)
+{
+	return 0;
+}
+
+static inline int drm_fb_helper_check_var(struct fb_var_screeninfo *var,
+					  struct fb_info *info)
+{
+	return 0;
+}
+
+static inline bool
+drm_fb_helper_restore_fbdev_mode_unlocked(struct drm_fb_helper *fb_helper)
+{
+	return true;
+}
+
+static inline void drm_fb_helper_fill_var(struct fb_info *info,
+					  struct drm_fb_helper *fb_helper,
+					  uint32_t fb_width, uint32_t fb_height)
+{
+}
+
+static inline void drm_fb_helper_fill_fix(struct fb_info *info, uint32_t pitch,
+					  uint32_t depth)
+{
+}
+
+static inline int drm_fb_helper_setcmap(struct fb_cmap *cmap,
+					struct fb_info *info)
+{
+	return 0;
+}
+
+static inline int drm_fb_helper_hotplug_event(struct drm_fb_helper *fb_helper)
+{
+	return 0;
+}
+
+static inline int drm_fb_helper_initial_config(struct drm_fb_helper *fb_helper,
+					       int bpp_sel)
+{
+	return 0;
+}
+
+static inline int
+drm_fb_helper_single_add_all_connectors(struct drm_fb_helper *fb_helper)
+{
+	return 0;
+}
+
+static inline int drm_fb_helper_debug_enter(struct fb_info *info)
+{
+	return 0;
+}
+
+static inline int drm_fb_helper_debug_leave(struct fb_info *info)
+{
+	return 0;
+}
+
+static inline struct drm_display_mode *
+drm_has_preferred_mode(struct drm_fb_helper_connector *fb_connector,
+		       int width, int height)
+{
+	return NULL;
+}
+
+static inline struct drm_display_mode *
+drm_pick_cmdline_mode(struct drm_fb_helper_connector *fb_helper_conn,
+		      int width, int height)
+{
+	return NULL;
+}
+
+static inline int
+drm_fb_helper_add_one_connector(struct drm_fb_helper *fb_helper,
+				struct drm_connector *connector)
+{
+	return 0;
+}
+
+static inline int
+drm_fb_helper_remove_one_connector(struct drm_fb_helper *fb_helper,
+				   struct drm_connector *connector)
+{
+	return 0;
+}
+#endif
+
 #endif
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[RFC 2/6] drm/msm: Remove local fbdev emulation Kconfig option

[Archit Taneja]

DRM_MSM_FBDEV config is used to enable/disable fbdev emulation for the msm kms
driver.

Replace this with the top level DRM_FBDEV_EMULATION config option where
applicable. This also prevents build breaks caused by undefined drm_fb_helper_*
functions when legacy fbdev support was disabled.

Signed-off-by: Archit Taneja <architt@codeaurora.org>
---
 drivers/gpu/drm/msm/Kconfig   | 14 --------------
 drivers/gpu/drm/msm/Makefile  |  2 +-
 drivers/gpu/drm/msm/msm_drv.c |  4 ++--
 3 files changed, 3 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/msm/Kconfig b/drivers/gpu/drm/msm/Kconfig
index bacbbb7..d6f3a4f 100644
--- a/drivers/gpu/drm/msm/Kconfig
+++ b/drivers/gpu/drm/msm/Kconfig
@@ -13,20 +13,6 @@ config DRM_MSM
 	help
 	  DRM/KMS driver for MSM/snapdragon.
 
-config DRM_MSM_FBDEV
-	bool "Enable legacy fbdev support for MSM modesetting driver"
-	depends on DRM_MSM
-	select DRM_KMS_FB_HELPER
-	select FB_SYS_FILLRECT
-	select FB_SYS_COPYAREA
-	select FB_SYS_IMAGEBLIT
-	select FB_SYS_FOPS
-	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 MSM modesetting driver.
-
 config DRM_MSM_REGISTER_LOGGING
 	bool "MSM DRM register logging"
 	depends on DRM_MSM
diff --git a/drivers/gpu/drm/msm/Makefile b/drivers/gpu/drm/msm/Makefile
index 674a132..40726ad 100644
--- a/drivers/gpu/drm/msm/Makefile
+++ b/drivers/gpu/drm/msm/Makefile
@@ -48,7 +48,7 @@ msm-y := \
 	msm_rd.o \
 	msm_ringbuffer.o
 
-msm-$(CONFIG_DRM_MSM_FBDEV) += msm_fbdev.o
+msm-$(CONFIG_DRM_FBDEV_EMULATION) += msm_fbdev.o
 msm-$(CONFIG_COMMON_CLK) += mdp/mdp4/mdp4_lvds_pll.o
 
 obj-$(CONFIG_DRM_MSM)	+= msm.o
diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
index a426911..d3e816a 100644
--- a/drivers/gpu/drm/msm/msm_drv.c
+++ b/drivers/gpu/drm/msm/msm_drv.c
@@ -54,7 +54,7 @@ module_param(reglog, bool, 0600);
 #define reglog 0
 #endif
 
-#ifdef CONFIG_DRM_MSM_FBDEV
+#ifdef CONFIG_DRM_FBDEV_EMULATION
 static bool fbdev = true;
 MODULE_PARM_DESC(fbdev, "Enable fbdev compat layer");
 module_param(fbdev, bool, 0600);
@@ -305,7 +305,7 @@ static int msm_load(struct drm_device *dev, unsigned long flags)
 
 	drm_mode_config_reset(dev);
 
-#ifdef CONFIG_DRM_MSM_FBDEV
+#ifdef CONFIG_DRM_FBDEV_EMULATION
 	if (fbdev)
 		priv->fbdev = msm_fbdev_init(dev);
 #endif
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Archit Taneja]

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

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[RFC 4/6] drm/tegra: Remove local fbdev emulation Kconfig option

[Archit Taneja]

DRM_TEGRA_FBDEV config is currently used to enable/disable fbdev emulation for
the tegra 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 tegra_drm struct that adds/removes the terga_fbdev member has been
removed completely. This helps in calling stub drm fb helper functions at not much
cost.

We could clean up fb.c a bit further to reduce the number of #ifdefs, but that's
left for later.

Signed-off-by: Archit Taneja <architt@codeaurora.org>
---
 drivers/gpu/drm/tegra/Kconfig | 12 ------------
 drivers/gpu/drm/tegra/drm.c   | 15 ++++++++++-----
 drivers/gpu/drm/tegra/drm.h   |  8 --------
 drivers/gpu/drm/tegra/fb.c    | 25 ++++++-------------------
 4 files changed, 16 insertions(+), 44 deletions(-)

diff --git a/drivers/gpu/drm/tegra/Kconfig b/drivers/gpu/drm/tegra/Kconfig
index 74d9d62..63ebb15 100644
--- a/drivers/gpu/drm/tegra/Kconfig
+++ b/drivers/gpu/drm/tegra/Kconfig
@@ -16,18 +16,6 @@ config DRM_TEGRA
 
 if DRM_TEGRA
 
-config DRM_TEGRA_FBDEV
-	bool "Enable legacy fbdev support"
-	select DRM_KMS_FB_HELPER
-	select FB_SYS_FILLRECT
-	select FB_SYS_COPYAREA
-	select FB_SYS_IMAGEBLIT
-	default y
-	help
-	  Choose this option if you have a need for the legacy fbdev support.
-	  Note that this support also provides the Linux console on top of
-	  the Tegra modesetting driver.
-
 config DRM_TEGRA_DEBUG
 	bool "NVIDIA Tegra DRM debug support"
 	help
diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c
index 7dd328d..97ebde9 100644
--- a/drivers/gpu/drm/tegra/drm.c
+++ b/drivers/gpu/drm/tegra/drm.c
@@ -104,11 +104,17 @@ static int tegra_atomic_commit(struct drm_device *drm,
 	return 0;
 }
 
+static inline void tegra_fb_output_poll_changed(struct drm_device *drm)
+{
+	struct tegra_drm *tegra = drm->dev_private;
+
+	if (tegra->fbdev)
+		drm_fb_helper_hotplug_event(&tegra->fbdev->base);
+}
+
 static const struct drm_mode_config_funcs tegra_drm_mode_funcs = {
 	.fb_create = tegra_fb_create,
-#ifdef CONFIG_DRM_TEGRA_FBDEV
 	.output_poll_changed = tegra_fb_output_poll_changed,
-#endif
 	.atomic_check = drm_atomic_helper_check,
 	.atomic_commit = tegra_atomic_commit,
 };
@@ -248,11 +254,10 @@ static void tegra_drm_context_free(struct tegra_drm_context *context)
 
 static void tegra_drm_lastclose(struct drm_device *drm)
 {
-#ifdef CONFIG_DRM_TEGRA_FBDEV
 	struct tegra_drm *tegra = drm->dev_private;
 
-	tegra_fbdev_restore_mode(tegra->fbdev);
-#endif
+	if (tegra->fbdev)
+		drm_fb_helper_restore_fbdev_mode_unlocked(&tegra->fbdev->base);
 }
 
 static struct host1x_bo *
diff --git a/drivers/gpu/drm/tegra/drm.h b/drivers/gpu/drm/tegra/drm.h
index 8cb2dfe..3a92413 100644
--- a/drivers/gpu/drm/tegra/drm.h
+++ b/drivers/gpu/drm/tegra/drm.h
@@ -29,12 +29,10 @@ struct tegra_fb {
 	unsigned int num_planes;
 };
 
-#ifdef CONFIG_DRM_TEGRA_FBDEV
 struct tegra_fbdev {
 	struct drm_fb_helper base;
 	struct tegra_fb *fb;
 };
-#endif
 
 struct tegra_drm {
 	struct drm_device *drm;
@@ -45,9 +43,7 @@ struct tegra_drm {
 	struct mutex clients_lock;
 	struct list_head clients;
 
-#ifdef CONFIG_DRM_TEGRA_FBDEV
 	struct tegra_fbdev *fbdev;
-#endif
 
 	unsigned int pitch_align;
 
@@ -263,10 +259,6 @@ int tegra_drm_fb_prepare(struct drm_device *drm);
 void tegra_drm_fb_free(struct drm_device *drm);
 int tegra_drm_fb_init(struct drm_device *drm);
 void tegra_drm_fb_exit(struct drm_device *drm);
-#ifdef CONFIG_DRM_TEGRA_FBDEV
-void tegra_fbdev_restore_mode(struct tegra_fbdev *fbdev);
-void tegra_fb_output_poll_changed(struct drm_device *drm);
-#endif
 
 extern struct platform_driver tegra_dc_driver;
 extern struct platform_driver tegra_dsi_driver;
diff --git a/drivers/gpu/drm/tegra/fb.c b/drivers/gpu/drm/tegra/fb.c
index 397fb34..2eacea2 100644
--- a/drivers/gpu/drm/tegra/fb.c
+++ b/drivers/gpu/drm/tegra/fb.c
@@ -18,7 +18,7 @@ static inline struct tegra_fb *to_tegra_fb(struct drm_framebuffer *fb)
 	return container_of(fb, struct tegra_fb, base);
 }
 
-#ifdef CONFIG_DRM_TEGRA_FBDEV
+#ifdef CONFIG_DRM_FBDEV_EMULATION
 static inline struct tegra_fbdev *to_tegra_fbdev(struct drm_fb_helper *helper)
 {
 	return container_of(helper, struct tegra_fbdev, base);
@@ -181,7 +181,7 @@ unreference:
 	return ERR_PTR(err);
 }
 
-#ifdef CONFIG_DRM_TEGRA_FBDEV
+#ifdef CONFIG_DRM_FBDEV_EMULATION
 static struct fb_ops tegra_fb_ops = {
 	.owner = THIS_MODULE,
 	.fb_fillrect = sys_fillrect,
@@ -371,24 +371,11 @@ static void tegra_fbdev_exit(struct tegra_fbdev *fbdev)
 	tegra_fbdev_free(fbdev);
 }
 
-void tegra_fbdev_restore_mode(struct tegra_fbdev *fbdev)
-{
-	if (fbdev)
-		drm_fb_helper_restore_fbdev_mode_unlocked(&fbdev->base);
-}
-
-void tegra_fb_output_poll_changed(struct drm_device *drm)
-{
-	struct tegra_drm *tegra = drm->dev_private;
-
-	if (tegra->fbdev)
-		drm_fb_helper_hotplug_event(&tegra->fbdev->base);
-}
 #endif
 
 int tegra_drm_fb_prepare(struct drm_device *drm)
 {
-#ifdef CONFIG_DRM_TEGRA_FBDEV
+#ifdef CONFIG_DRM_FBDEV_EMULATION
 	struct tegra_drm *tegra = drm->dev_private;
 
 	tegra->fbdev = tegra_fbdev_create(drm);
@@ -401,7 +388,7 @@ int tegra_drm_fb_prepare(struct drm_device *drm)
 
 void tegra_drm_fb_free(struct drm_device *drm)
 {
-#ifdef CONFIG_DRM_TEGRA_FBDEV
+#ifdef CONFIG_DRM_FBDEV_EMULATION
 	struct tegra_drm *tegra = drm->dev_private;
 
 	tegra_fbdev_free(tegra->fbdev);
@@ -410,7 +397,7 @@ void tegra_drm_fb_free(struct drm_device *drm)
 
 int tegra_drm_fb_init(struct drm_device *drm)
 {
-#ifdef CONFIG_DRM_TEGRA_FBDEV
+#ifdef CONFIG_DRM_FBDEV_EMULATION
 	struct tegra_drm *tegra = drm->dev_private;
 	int err;
 
@@ -425,7 +412,7 @@ int tegra_drm_fb_init(struct drm_device *drm)
 
 void tegra_drm_fb_exit(struct drm_device *drm)
 {
-#ifdef CONFIG_DRM_TEGRA_FBDEV
+#ifdef CONFIG_DRM_FBDEV_EMULATION
 	struct tegra_drm *tegra = drm->dev_private;
 
 	tegra_fbdev_exit(tegra->fbdev);
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

[RFC 5/6] drm/imx: Remove local fbdev emulation Kconfig option

[Archit Taneja]

DRM_IMX_FB_HELPER config is currently used to enable/disable fbdev emulation for
the imx kms driver.

Remove this local config option and use the top level DRM_FBDEV_EMULATION config
option where applicable. Using this config lets us also prevent wrapping around
drm_fb_helper_* calls with #ifdefs in certain places.

We replace the #ifdef in imx_drm_driver_load with CONFIG_DRM_FBDEV_EMULATION.
It's probably okay to get remove the #ifdef itself, but just left it here for
now to be safe. It can be removed after some testing.

Signed-off-by: Archit Taneja <architt@codeaurora.org>
---
 drivers/gpu/drm/imx/Kconfig        |  9 ---------
 drivers/gpu/drm/imx/imx-drm-core.c | 10 +---------
 2 files changed, 1 insertion(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/imx/Kconfig b/drivers/gpu/drm/imx/Kconfig
index 33cdddf..e008960 100644
--- a/drivers/gpu/drm/imx/Kconfig
+++ b/drivers/gpu/drm/imx/Kconfig
@@ -10,15 +10,6 @@ config DRM_IMX
 	help
 	  enable i.MX graphics support
 
-config DRM_IMX_FB_HELPER
-	tristate "provide legacy framebuffer /dev/fb0"
-	select DRM_KMS_CMA_HELPER
-	depends on DRM_IMX
-	help
-	  The DRM framework can provide a legacy /dev/fb0 framebuffer
-	  for your device. This is necessary to get a framebuffer console
-	  and also for applications using the legacy framebuffer API
-
 config DRM_IMX_PARALLEL_DISPLAY
 	tristate "Support for parallel displays"
 	select DRM_PANEL
diff --git a/drivers/gpu/drm/imx/imx-drm-core.c b/drivers/gpu/drm/imx/imx-drm-core.c
index a002f53..5cbcb8a 100644
--- a/drivers/gpu/drm/imx/imx-drm-core.c
+++ b/drivers/gpu/drm/imx/imx-drm-core.c
@@ -60,26 +60,20 @@ EXPORT_SYMBOL_GPL(imx_drm_crtc_id);
 
 static void imx_drm_driver_lastclose(struct drm_device *drm)
 {
-#if IS_ENABLED(CONFIG_DRM_IMX_FB_HELPER)
 	struct imx_drm_device *imxdrm = drm->dev_private;
 
 	if (imxdrm->fbhelper)
 		drm_fbdev_cma_restore_mode(imxdrm->fbhelper);
-#endif
 }
 
 static int imx_drm_driver_unload(struct drm_device *drm)
 {
-#if IS_ENABLED(CONFIG_DRM_IMX_FB_HELPER)
 	struct imx_drm_device *imxdrm = drm->dev_private;
-#endif
 
 	drm_kms_helper_poll_fini(drm);
 
-#if IS_ENABLED(CONFIG_DRM_IMX_FB_HELPER)
 	if (imxdrm->fbhelper)
 		drm_fbdev_cma_fini(imxdrm->fbhelper);
-#endif
 
 	component_unbind_all(drm->dev, drm);
 
@@ -215,11 +209,9 @@ EXPORT_SYMBOL_GPL(imx_drm_encoder_destroy);
 
 static void imx_drm_output_poll_changed(struct drm_device *drm)
 {
-#if IS_ENABLED(CONFIG_DRM_IMX_FB_HELPER)
 	struct imx_drm_device *imxdrm = drm->dev_private;
 
 	drm_fbdev_cma_hotplug_event(imxdrm->fbhelper);
-#endif
 }
 
 static struct drm_mode_config_funcs imx_drm_mode_config_funcs = {
@@ -308,7 +300,7 @@ static int imx_drm_driver_load(struct drm_device *drm, unsigned long flags)
 	 * The fb helper takes copies of key hardware information, so the
 	 * crtcs/connectors/encoders must not change after this point.
 	 */
-#if IS_ENABLED(CONFIG_DRM_IMX_FB_HELPER)
+#ifdef CONFIG_DRM_FBDEV_EMULATION
 	if (legacyfb_depth != 16 && legacyfb_depth != 32) {
 		dev_warn(drm->dev, "Invalid legacyfb_depth.  Defaulting to 16bpp\n");
 		legacyfb_depth = 16;
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

[RFC 6/6] drm/sti: Remove local fbdev emulation Kconfig option

[Archit Taneja]

DRM_STI_FBDEV config is currently used to enable/disable fbdev emulation for
the sti kms driver.

Remove this local config option and use the top level DRM_FBDEV_EMULATION config
option instead where applicable.

We replace the #ifdef in sti_drm_load with CONFIG_DRM_FBDEV_EMULATION. It's
probably okay to get remove the #ifdef itself, but just left it here for now to
be safe. It can be removed after some testing.

Signed-off-by: Archit Taneja <architt@codeaurora.org>
---
 drivers/gpu/drm/sti/Kconfig       | 6 ------
 drivers/gpu/drm/sti/sti_drm_drv.c | 2 +-
 2 files changed, 1 insertion(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/sti/Kconfig b/drivers/gpu/drm/sti/Kconfig
index fbccc10..e3aa5af 100644
--- a/drivers/gpu/drm/sti/Kconfig
+++ b/drivers/gpu/drm/sti/Kconfig
@@ -9,9 +9,3 @@ config DRM_STI
 	select FW_LOADER_USER_HELPER_FALLBACK
 	help
 	  Choose this option to enable DRM on STM stiH41x chipset
-
-config DRM_STI_FBDEV
-	bool "DRM frame buffer device for STMicroelectronics SoC stiH41x Serie"
-	depends on DRM_STI
-	help
-	  Choose this option to enable FBDEV on top of DRM for STM stiH41x chipset
diff --git a/drivers/gpu/drm/sti/sti_drm_drv.c b/drivers/gpu/drm/sti/sti_drm_drv.c
index 5239fa1..90f121d 100644
--- a/drivers/gpu/drm/sti/sti_drm_drv.c
+++ b/drivers/gpu/drm/sti/sti_drm_drv.c
@@ -76,7 +76,7 @@ static int sti_drm_load(struct drm_device *dev, unsigned long flags)
 
 	drm_helper_disable_unused_functions(dev);
 
-#ifdef CONFIG_DRM_STI_FBDEV
+#ifdef CONFIG_DRM_FBDEV_EMULATION
 	drm_fbdev_cma_init(dev, 32,
 		   dev->mode_config.num_crtc,
 		   dev->mode_config.num_connector);
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Daniel Vetter]

On Tue, Mar 10, 2015 at 03:11:28PM +0530, Archit Taneja wrote:
> Legacy fbdev emulation support via DRM is achieved through KMS FB helpers.
> Most modesetting drivers enable provide fbdev emulation by default by selecting
> KMS FB helpers. A few provide a separate Kconfig option for the user to enable
> or disbale fbdev emulation.
> 
> Enabling fbdev emulation is finally a distro-level decision. Having a top level
> Kconfig option for fbdev emulation helps by providing a uniform way to
> enable/disable fbdev emulation for any modesetting driver. It also lets us
> remove unnecessary driver specific Kconfig options that causes bloat.
> 
> With a top level Kconfig in place, we can stub out the fb helper functions when
> not needed without breaking functionality. Having stub functions also prevents
> drivers to require wrapping fb helper function calls with #ifdefs.
> 
> DRM_FBDEV_EMULATION defaults to y since many drivers enable fbdev emulation by
> default and majority of distributions expect the fbdev interface in the kernel.
> 
> For now, this config selects both FB_SYS_* and FB_CFB_* configs as some
> modesetting drivers use the former and other the later. This needs to be taken
> care of in a better way.
> 
> Signed-off-by: Archit Taneja <architt@codeaurora.org>
> ---
>  drivers/gpu/drm/Kconfig     |  18 +++++++
>  drivers/gpu/drm/Makefile    |   4 ++
>  include/drm/drm_fb_helper.h | 120 ++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 142 insertions(+)
> 
> 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.
> +
>  config DRM_LOAD_EDID_FIRMWARE
>  	bool "Allow to specify an EDID data set instead of probing for it"
>  	depends on DRM_KMS_HELPER
> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> index 2c239b9..c1d44b2 100644
> --- a/drivers/gpu/drm/Makefile
> +++ b/drivers/gpu/drm/Makefile
> @@ -25,7 +25,11 @@ drm-$(CONFIG_OF) += drm_of.o
>  drm_kms_helper-y := drm_crtc_helper.o drm_dp_helper.o drm_probe_helper.o \
>  		drm_plane_helper.o drm_dp_mst_topology.o drm_atomic_helper.o
>  drm_kms_helper-$(CONFIG_DRM_LOAD_EDID_FIRMWARE) += drm_edid_load.o
> +
> +ifeq ($(CONFIG_DRM_FBDEV_EMULATION),y)
>  drm_kms_helper-$(CONFIG_DRM_KMS_FB_HELPER) += drm_fb_helper.o
> +endif

This should work as

drm_kms_helper-$CONFIG_DRM_FBDEV_EMULATION += drm_fb_helper.o

since both <modename>-y and <modename>-m are merged together.
-Daniel

> +
>  drm_kms_helper-$(CONFIG_DRM_KMS_CMA_HELPER) += drm_fb_cma_helper.o
>  
>  obj-$(CONFIG_DRM_KMS_HELPER) += drm_kms_helper.o
> diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h
> index 21b944c..dbfce1a 100644
> --- a/include/drm/drm_fb_helper.h
> +++ b/include/drm/drm_fb_helper.h
> @@ -103,6 +103,7 @@ struct drm_fb_helper {
>  	bool delayed_hotplug;
>  };
>  
> +#ifdef CONFIG_DRM_FBDEV_EMULATION
>  void drm_fb_helper_prepare(struct drm_device *dev, struct drm_fb_helper *helper,
>  			   const struct drm_fb_helper_funcs *funcs);
>  int drm_fb_helper_init(struct drm_device *dev,
> @@ -139,4 +140,123 @@ drm_pick_cmdline_mode(struct drm_fb_helper_connector *fb_helper_conn,
>  int drm_fb_helper_add_one_connector(struct drm_fb_helper *fb_helper, struct drm_connector *connector);
>  int drm_fb_helper_remove_one_connector(struct drm_fb_helper *fb_helper,
>  				       struct drm_connector *connector);
> +#else
> +static inline void drm_fb_helper_prepare(struct drm_device *dev,
> +					struct drm_fb_helper *helper,
> +					const struct drm_fb_helper_funcs *funcs)
> +{
> +}
> +
> +static inline int drm_fb_helper_init(struct drm_device *dev,
> +		       struct drm_fb_helper *helper, int crtc_count,
> +		       int max_conn)
> +{
> +	return 0;
> +}
> +
> +static inline void drm_fb_helper_fini(struct drm_fb_helper *helper)
> +{
> +}
> +
> +static inline int drm_fb_helper_blank(int blank, struct fb_info *info)
> +{
> +	return 0;
> +}
> +
> +static inline int drm_fb_helper_pan_display(struct fb_var_screeninfo *var,
> +					    struct fb_info *info)
> +{
> +	return 0;
> +}
> +
> +static inline int drm_fb_helper_set_par(struct fb_info *info)
> +{
> +	return 0;
> +}
> +
> +static inline int drm_fb_helper_check_var(struct fb_var_screeninfo *var,
> +					  struct fb_info *info)
> +{
> +	return 0;
> +}
> +
> +static inline bool
> +drm_fb_helper_restore_fbdev_mode_unlocked(struct drm_fb_helper *fb_helper)
> +{
> +	return true;
> +}
> +
> +static inline void drm_fb_helper_fill_var(struct fb_info *info,
> +					  struct drm_fb_helper *fb_helper,
> +					  uint32_t fb_width, uint32_t fb_height)
> +{
> +}
> +
> +static inline void drm_fb_helper_fill_fix(struct fb_info *info, uint32_t pitch,
> +					  uint32_t depth)
> +{
> +}
> +
> +static inline int drm_fb_helper_setcmap(struct fb_cmap *cmap,
> +					struct fb_info *info)
> +{
> +	return 0;
> +}
> +
> +static inline int drm_fb_helper_hotplug_event(struct drm_fb_helper *fb_helper)
> +{
> +	return 0;
> +}
> +
> +static inline int drm_fb_helper_initial_config(struct drm_fb_helper *fb_helper,
> +					       int bpp_sel)
> +{
> +	return 0;
> +}
> +
> +static inline int
> +drm_fb_helper_single_add_all_connectors(struct drm_fb_helper *fb_helper)
> +{
> +	return 0;
> +}
> +
> +static inline int drm_fb_helper_debug_enter(struct fb_info *info)
> +{
> +	return 0;
> +}
> +
> +static inline int drm_fb_helper_debug_leave(struct fb_info *info)
> +{
> +	return 0;
> +}
> +
> +static inline struct drm_display_mode *
> +drm_has_preferred_mode(struct drm_fb_helper_connector *fb_connector,
> +		       int width, int height)
> +{
> +	return NULL;
> +}
> +
> +static inline struct drm_display_mode *
> +drm_pick_cmdline_mode(struct drm_fb_helper_connector *fb_helper_conn,
> +		      int width, int height)
> +{
> +	return NULL;
> +}
> +
> +static inline int
> +drm_fb_helper_add_one_connector(struct drm_fb_helper *fb_helper,
> +				struct drm_connector *connector)
> +{
> +	return 0;
> +}
> +
> +static inline int
> +drm_fb_helper_remove_one_connector(struct drm_fb_helper *fb_helper,
> +				   struct drm_connector *connector)
> +{
> +	return 0;
> +}
> +#endif
> +
>  #endif
> -- 
> 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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Daniel Vetter]

On Tue, Mar 10, 2015 at 03:11:28PM +0530, Archit Taneja wrote:
> Legacy fbdev emulation support via DRM is achieved through KMS FB helpers.
> Most modesetting drivers enable provide fbdev emulation by default by selecting
> KMS FB helpers. A few provide a separate Kconfig option for the user to enable
> or disbale fbdev emulation.
> 
> Enabling fbdev emulation is finally a distro-level decision. Having a top level
> Kconfig option for fbdev emulation helps by providing a uniform way to
> enable/disable fbdev emulation for any modesetting driver. It also lets us
> remove unnecessary driver specific Kconfig options that causes bloat.
> 
> With a top level Kconfig in place, we can stub out the fb helper functions when
> not needed without breaking functionality. Having stub functions also prevents
> drivers to require wrapping fb helper function calls with #ifdefs.
> 
> DRM_FBDEV_EMULATION defaults to y since many drivers enable fbdev emulation by
> default and majority of distributions expect the fbdev interface in the kernel.
> 
> For now, this config selects both FB_SYS_* and FB_CFB_* configs as some
> modesetting drivers use the former and other the later. This needs to be taken
> care of in a better way.
> 
> Signed-off-by: Archit Taneja <architt@codeaurora.org>
> ---
>  drivers/gpu/drm/Kconfig     |  18 +++++++
>  drivers/gpu/drm/Makefile    |   4 ++
>  include/drm/drm_fb_helper.h | 120 ++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 142 insertions(+)
> 
> 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.
-Daniel

> +
>  config DRM_LOAD_EDID_FIRMWARE
>  	bool "Allow to specify an EDID data set instead of probing for it"
>  	depends on DRM_KMS_HELPER
> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> index 2c239b9..c1d44b2 100644
> --- a/drivers/gpu/drm/Makefile
> +++ b/drivers/gpu/drm/Makefile
> @@ -25,7 +25,11 @@ drm-$(CONFIG_OF) += drm_of.o
>  drm_kms_helper-y := drm_crtc_helper.o drm_dp_helper.o drm_probe_helper.o \
>  		drm_plane_helper.o drm_dp_mst_topology.o drm_atomic_helper.o
>  drm_kms_helper-$(CONFIG_DRM_LOAD_EDID_FIRMWARE) += drm_edid_load.o
> +
> +ifeq ($(CONFIG_DRM_FBDEV_EMULATION),y)
>  drm_kms_helper-$(CONFIG_DRM_KMS_FB_HELPER) += drm_fb_helper.o
> +endif
> +
>  drm_kms_helper-$(CONFIG_DRM_KMS_CMA_HELPER) += drm_fb_cma_helper.o
>  
>  obj-$(CONFIG_DRM_KMS_HELPER) += drm_kms_helper.o
> diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h
> index 21b944c..dbfce1a 100644
> --- a/include/drm/drm_fb_helper.h
> +++ b/include/drm/drm_fb_helper.h
> @@ -103,6 +103,7 @@ struct drm_fb_helper {
>  	bool delayed_hotplug;
>  };
>  
> +#ifdef CONFIG_DRM_FBDEV_EMULATION
>  void drm_fb_helper_prepare(struct drm_device *dev, struct drm_fb_helper *helper,
>  			   const struct drm_fb_helper_funcs *funcs);
>  int drm_fb_helper_init(struct drm_device *dev,
> @@ -139,4 +140,123 @@ drm_pick_cmdline_mode(struct drm_fb_helper_connector *fb_helper_conn,
>  int drm_fb_helper_add_one_connector(struct drm_fb_helper *fb_helper, struct drm_connector *connector);
>  int drm_fb_helper_remove_one_connector(struct drm_fb_helper *fb_helper,
>  				       struct drm_connector *connector);
> +#else
> +static inline void drm_fb_helper_prepare(struct drm_device *dev,
> +					struct drm_fb_helper *helper,
> +					const struct drm_fb_helper_funcs *funcs)
> +{
> +}
> +
> +static inline int drm_fb_helper_init(struct drm_device *dev,
> +		       struct drm_fb_helper *helper, int crtc_count,
> +		       int max_conn)
> +{
> +	return 0;
> +}
> +
> +static inline void drm_fb_helper_fini(struct drm_fb_helper *helper)
> +{
> +}
> +
> +static inline int drm_fb_helper_blank(int blank, struct fb_info *info)
> +{
> +	return 0;
> +}
> +
> +static inline int drm_fb_helper_pan_display(struct fb_var_screeninfo *var,
> +					    struct fb_info *info)
> +{
> +	return 0;
> +}
> +
> +static inline int drm_fb_helper_set_par(struct fb_info *info)
> +{
> +	return 0;
> +}
> +
> +static inline int drm_fb_helper_check_var(struct fb_var_screeninfo *var,
> +					  struct fb_info *info)
> +{
> +	return 0;
> +}
> +
> +static inline bool
> +drm_fb_helper_restore_fbdev_mode_unlocked(struct drm_fb_helper *fb_helper)
> +{
> +	return true;
> +}
> +
> +static inline void drm_fb_helper_fill_var(struct fb_info *info,
> +					  struct drm_fb_helper *fb_helper,
> +					  uint32_t fb_width, uint32_t fb_height)
> +{
> +}
> +
> +static inline void drm_fb_helper_fill_fix(struct fb_info *info, uint32_t pitch,
> +					  uint32_t depth)
> +{
> +}
> +
> +static inline int drm_fb_helper_setcmap(struct fb_cmap *cmap,
> +					struct fb_info *info)
> +{
> +	return 0;
> +}
> +
> +static inline int drm_fb_helper_hotplug_event(struct drm_fb_helper *fb_helper)
> +{
> +	return 0;
> +}
> +
> +static inline int drm_fb_helper_initial_config(struct drm_fb_helper *fb_helper,
> +					       int bpp_sel)
> +{
> +	return 0;
> +}
> +
> +static inline int
> +drm_fb_helper_single_add_all_connectors(struct drm_fb_helper *fb_helper)
> +{
> +	return 0;
> +}
> +
> +static inline int drm_fb_helper_debug_enter(struct fb_info *info)
> +{
> +	return 0;
> +}
> +
> +static inline int drm_fb_helper_debug_leave(struct fb_info *info)
> +{
> +	return 0;
> +}
> +
> +static inline struct drm_display_mode *
> +drm_has_preferred_mode(struct drm_fb_helper_connector *fb_connector,
> +		       int width, int height)
> +{
> +	return NULL;
> +}
> +
> +static inline struct drm_display_mode *
> +drm_pick_cmdline_mode(struct drm_fb_helper_connector *fb_helper_conn,
> +		      int width, int height)
> +{
> +	return NULL;
> +}
> +
> +static inline int
> +drm_fb_helper_add_one_connector(struct drm_fb_helper *fb_helper,
> +				struct drm_connector *connector)
> +{
> +	return 0;
> +}
> +
> +static inline int
> +drm_fb_helper_remove_one_connector(struct drm_fb_helper *fb_helper,
> +				   struct drm_connector *connector)
> +{
> +	return 0;
> +}
> +#endif
> +
>  #endif
> -- 
> 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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Archit Taneja]

On 03/10/2015 03:17 PM, Daniel Vetter wrote:
> On Tue, Mar 10, 2015 at 03:11:28PM +0530, Archit Taneja wrote:
>> Legacy fbdev emulation support via DRM is achieved through KMS FB helpers.
>> Most modesetting drivers enable provide fbdev emulation by default by selecting
>> KMS FB helpers. A few provide a separate Kconfig option for the user to enable
>> or disbale fbdev emulation.
>>
>> Enabling fbdev emulation is finally a distro-level decision. Having a top level
>> Kconfig option for fbdev emulation helps by providing a uniform way to
>> enable/disable fbdev emulation for any modesetting driver. It also lets us
>> remove unnecessary driver specific Kconfig options that causes bloat.
>>
>> With a top level Kconfig in place, we can stub out the fb helper functions when
>> not needed without breaking functionality. Having stub functions also prevents
>> drivers to require wrapping fb helper function calls with #ifdefs.
>>
>> DRM_FBDEV_EMULATION defaults to y since many drivers enable fbdev emulation by
>> default and majority of distributions expect the fbdev interface in the kernel.
>>
>> For now, this config selects both FB_SYS_* and FB_CFB_* configs as some
>> modesetting drivers use the former and other the later. This needs to be taken
>> care of in a better way.
>>
>> Signed-off-by: Archit Taneja <architt@codeaurora.org>
>> ---
>>   drivers/gpu/drm/Kconfig     |  18 +++++++
>>   drivers/gpu/drm/Makefile    |   4 ++
>>   include/drm/drm_fb_helper.h | 120 ++++++++++++++++++++++++++++++++++++++++++++
>>   3 files changed, 142 insertions(+)
>>
>> 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?

Archit
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Archit Taneja]

On 03/10/2015 03:16 PM, Daniel Vetter wrote:
> On Tue, Mar 10, 2015 at 03:11:28PM +0530, Archit Taneja wrote:
>> Legacy fbdev emulation support via DRM is achieved through KMS FB helpers.
>> Most modesetting drivers enable provide fbdev emulation by default by selecting
>> KMS FB helpers. A few provide a separate Kconfig option for the user to enable
>> or disbale fbdev emulation.
>>
>> Enabling fbdev emulation is finally a distro-level decision. Having a top level
>> Kconfig option for fbdev emulation helps by providing a uniform way to
>> enable/disable fbdev emulation for any modesetting driver. It also lets us
>> remove unnecessary driver specific Kconfig options that causes bloat.
>>
>> With a top level Kconfig in place, we can stub out the fb helper functions when
>> not needed without breaking functionality. Having stub functions also prevents
>> drivers to require wrapping fb helper function calls with #ifdefs.
>>
>> DRM_FBDEV_EMULATION defaults to y since many drivers enable fbdev emulation by
>> default and majority of distributions expect the fbdev interface in the kernel.
>>
>> For now, this config selects both FB_SYS_* and FB_CFB_* configs as some
>> modesetting drivers use the former and other the later. This needs to be taken
>> care of in a better way.
>>
>> Signed-off-by: Archit Taneja <architt@codeaurora.org>
>> ---
>>   drivers/gpu/drm/Kconfig     |  18 +++++++
>>   drivers/gpu/drm/Makefile    |   4 ++
>>   include/drm/drm_fb_helper.h | 120 ++++++++++++++++++++++++++++++++++++++++++++
>>   3 files changed, 142 insertions(+)
>>
>> 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.
>> +
>>   config DRM_LOAD_EDID_FIRMWARE
>>   	bool "Allow to specify an EDID data set instead of probing for it"
>>   	depends on DRM_KMS_HELPER
>> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
>> index 2c239b9..c1d44b2 100644
>> --- a/drivers/gpu/drm/Makefile
>> +++ b/drivers/gpu/drm/Makefile
>> @@ -25,7 +25,11 @@ drm-$(CONFIG_OF) += drm_of.o
>>   drm_kms_helper-y := drm_crtc_helper.o drm_dp_helper.o drm_probe_helper.o \
>>   		drm_plane_helper.o drm_dp_mst_topology.o drm_atomic_helper.o
>>   drm_kms_helper-$(CONFIG_DRM_LOAD_EDID_FIRMWARE) += drm_edid_load.o
>> +
>> +ifeq ($(CONFIG_DRM_FBDEV_EMULATION),y)
>>   drm_kms_helper-$(CONFIG_DRM_KMS_FB_HELPER) += drm_fb_helper.o
>> +endif
>
> This should work as
>
> drm_kms_helper-$CONFIG_DRM_FBDEV_EMULATION += drm_fb_helper.o
>
> since both <modename>-y and <modename>-m are merged together.

Ah right. I'll fix that.

Archit

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

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

[Daniel Vetter]

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

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

[Daniel Vetter]

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:
> >>Legacy fbdev emulation support via DRM is achieved through KMS FB helpers.
> >>Most modesetting drivers enable provide fbdev emulation by default by selecting
> >>KMS FB helpers. A few provide a separate Kconfig option for the user to enable
> >>or disbale fbdev emulation.
> >>
> >>Enabling fbdev emulation is finally a distro-level decision. Having a top level
> >>Kconfig option for fbdev emulation helps by providing a uniform way to
> >>enable/disable fbdev emulation for any modesetting driver. It also lets us
> >>remove unnecessary driver specific Kconfig options that causes bloat.
> >>
> >>With a top level Kconfig in place, we can stub out the fb helper functions when
> >>not needed without breaking functionality. Having stub functions also prevents
> >>drivers to require wrapping fb helper function calls with #ifdefs.
> >>
> >>DRM_FBDEV_EMULATION defaults to y since many drivers enable fbdev emulation by
> >>default and majority of distributions expect the fbdev interface in the kernel.
> >>
> >>For now, this config selects both FB_SYS_* and FB_CFB_* configs as some
> >>modesetting drivers use the former and other the later. This needs to be taken
> >>care of in a better way.
> >>
> >>Signed-off-by: Archit Taneja <architt@codeaurora.org>
> >>---
> >>  drivers/gpu/drm/Kconfig     |  18 +++++++
> >>  drivers/gpu/drm/Makefile    |   4 ++
> >>  include/drm/drm_fb_helper.h | 120 ++++++++++++++++++++++++++++++++++++++++++++
> >>  3 files changed, 142 insertions(+)
> >>
> >>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.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Archit Taneja]

On 03/10/2015 03:31 PM, Daniel Vetter wrote:
> 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.

There are checks for dev_priv->fbdev and dev_priv->fbdev->fb being non 
NULL at the start of the function which would prevent that. But you're 
right, the first patch for each device can replace the config, the later 
ones can take advantage of the stub functions.

Archit

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Archit Taneja]

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:
>>>> Legacy fbdev emulation support via DRM is achieved through KMS FB helpers.
>>>> Most modesetting drivers enable provide fbdev emulation by default by selecting
>>>> KMS FB helpers. A few provide a separate Kconfig option for the user to enable
>>>> or disbale fbdev emulation.
>>>>
>>>> Enabling fbdev emulation is finally a distro-level decision. Having a top level
>>>> Kconfig option for fbdev emulation helps by providing a uniform way to
>>>> enable/disable fbdev emulation for any modesetting driver. It also lets us
>>>> remove unnecessary driver specific Kconfig options that causes bloat.
>>>>
>>>> With a top level Kconfig in place, we can stub out the fb helper functions when
>>>> not needed without breaking functionality. Having stub functions also prevents
>>>> drivers to require wrapping fb helper function calls with #ifdefs.
>>>>
>>>> DRM_FBDEV_EMULATION defaults to y since many drivers enable fbdev emulation by
>>>> default and majority of distributions expect the fbdev interface in the kernel.
>>>>
>>>> For now, this config selects both FB_SYS_* and FB_CFB_* configs as some
>>>> modesetting drivers use the former and other the later. This needs to be taken
>>>> care of in a better way.
>>>>
>>>> Signed-off-by: Archit Taneja <architt@codeaurora.org>
>>>> ---
>>>>   drivers/gpu/drm/Kconfig     |  18 +++++++
>>>>   drivers/gpu/drm/Makefile    |   4 ++
>>>>   include/drm/drm_fb_helper.h | 120 ++++++++++++++++++++++++++++++++++++++++++++
>>>>   3 files changed, 142 insertions(+)
>>>>
>>>> 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.

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.

Archit

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RFC 5/6] drm/imx: Remove local fbdev emulation Kconfig option

[Philipp Zabel]

Hi Archit,

thanks for the cleanup!

Am Dienstag, den 10.03.2015, 15:11 +0530 schrieb Archit Taneja:
> DRM_IMX_FB_HELPER config is currently used to enable/disable fbdev emulation for
> the imx kms driver.
> 
> Remove this local config option and use the top level DRM_FBDEV_EMULATION config
> option where applicable. Using this config lets us also prevent wrapping around
> drm_fb_helper_* calls with #ifdefs in certain places.
> 
> We replace the #ifdef in imx_drm_driver_load with CONFIG_DRM_FBDEV_EMULATION.
> It's probably okay to get remove the #ifdef itself, but just left it here for
> now to be safe. It can be removed after some testing.
> 
> Signed-off-by: Archit Taneja <architt@codeaurora.org>

Tested-by: Philipp Zabel <p.zabel@pengutronix.de>
(Both with and without the #ifdef CONFIG_DRM_FBDEV_EMULATION removed.)

Although this is for another patch, I think the legacyfb_depth
module_param should be removed altogether if CONFIG_DRM_FBDEV_EMULATION
is disabled, so maybe that #ifdef should stay.

regards
Philipp

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

[Daniel Vetter]

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?

> 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.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Jani Nikula]

On Tue, 10 Mar 2015, Archit Taneja <architt@codeaurora.org> 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:
>>> 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.
>
> Are you saying that we should remove DRM_KMS_FB_HELPER for such drivers 
> and replace them with 'select DRM_FBDEV_EMULATION'?

Slightly tangential: As a rule of thumb, you should not "select"
anything that is visible in menuconfig or has dependencies [1]. This
will break the build eventually, and attempts to fix it are troublesome
[2].

BR,
Jani.

[1] Documentation/kbuild/kconfig-language.txt
[2] http://mid.gmane.org/1413580403-16225-1-git-send-email-jani.nikula@intel.com


-- 
Jani Nikula, Intel Open Source Technology Center

Re: [RFC 5/6] drm/imx: Remove local fbdev emulation Kconfig option

[Archit Taneja]

On 03/10/2015 04:24 PM, Philipp Zabel wrote:
> Hi Archit,
>
> thanks for the cleanup!
>
> Am Dienstag, den 10.03.2015, 15:11 +0530 schrieb Archit Taneja:
>> DRM_IMX_FB_HELPER config is currently used to enable/disable fbdev emulation for
>> the imx kms driver.
>>
>> Remove this local config option and use the top level DRM_FBDEV_EMULATION config
>> option where applicable. Using this config lets us also prevent wrapping around
>> drm_fb_helper_* calls with #ifdefs in certain places.
>>
>> We replace the #ifdef in imx_drm_driver_load with CONFIG_DRM_FBDEV_EMULATION.
>> It's probably okay to get remove the #ifdef itself, but just left it here for
>> now to be safe. It can be removed after some testing.
>>
>> Signed-off-by: Archit Taneja <architt@codeaurora.org>
>
> Tested-by: Philipp Zabel <p.zabel@pengutronix.de>
> (Both with and without the #ifdef CONFIG_DRM_FBDEV_EMULATION removed.)
>

Thanks for testing it out.

> Although this is for another patch, I think the legacyfb_depth
> module_param should be removed altogether if CONFIG_DRM_FBDEV_EMULATION
> is disabled, so maybe that #ifdef should stay.

I'll create a patch for that for future revs of this patch set.

Archit

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

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

[Archit Taneja]

On 03/10/2015 09:03 PM, Jani Nikula wrote:
> On Tue, 10 Mar 2015, Archit Taneja <architt@codeaurora.org> 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:
>>>> 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.
>>
>> Are you saying that we should remove DRM_KMS_FB_HELPER for such drivers
>> and replace them with 'select DRM_FBDEV_EMULATION'?
>
> Slightly tangential: As a rule of thumb, you should not "select"
> anything that is visible in menuconfig or has dependencies [1]. This
> will break the build eventually, and attempts to fix it are troublesome
> [2].

Thanks, I'll keep this in mind!

Archit

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Benjamin Gaignard]

[-- Attachment #1.1: Type: text/plain, Size: 2541 bytes --]

I was close the send a patch it remove this flag but more inspired by what
Rob has done few weeks ago:
http://cgit.freedesktop.org/~airlied/linux/commit/?h=drm-next&id=e90dfec78ec288d6c89a7b508a5c5d4ae8b7f934


2015-03-10 10:41 GMT+01:00 Archit Taneja <architt@codeaurora.org>:

> DRM_STI_FBDEV config is currently used to enable/disable fbdev emulation
> for
> the sti kms driver.
>
> Remove this local config option and use the top level DRM_FBDEV_EMULATION
> config
> option instead where applicable.
>
> We replace the #ifdef in sti_drm_load with CONFIG_DRM_FBDEV_EMULATION. It's
> probably okay to get remove the #ifdef itself, but just left it here for
> now to
> be safe. It can be removed after some testing.
>
> Signed-off-by: Archit Taneja <architt@codeaurora.org>
> ---
>  drivers/gpu/drm/sti/Kconfig       | 6 ------
>  drivers/gpu/drm/sti/sti_drm_drv.c | 2 +-
>  2 files changed, 1 insertion(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/sti/Kconfig b/drivers/gpu/drm/sti/Kconfig
> index fbccc10..e3aa5af 100644
> --- a/drivers/gpu/drm/sti/Kconfig
> +++ b/drivers/gpu/drm/sti/Kconfig
> @@ -9,9 +9,3 @@ config DRM_STI
>         select FW_LOADER_USER_HELPER_FALLBACK
>         help
>           Choose this option to enable DRM on STM stiH41x chipset
> -
> -config DRM_STI_FBDEV
> -       bool "DRM frame buffer device for STMicroelectronics SoC stiH41x
> Serie"
> -       depends on DRM_STI
> -       help
> -         Choose this option to enable FBDEV on top of DRM for STM stiH41x
> chipset
> diff --git a/drivers/gpu/drm/sti/sti_drm_drv.c
> b/drivers/gpu/drm/sti/sti_drm_drv.c
> index 5239fa1..90f121d 100644
> --- a/drivers/gpu/drm/sti/sti_drm_drv.c
> +++ b/drivers/gpu/drm/sti/sti_drm_drv.c
> @@ -76,7 +76,7 @@ static int sti_drm_load(struct drm_device *dev, unsigned
> long flags)
>
>         drm_helper_disable_unused_functions(dev);
>
> -#ifdef CONFIG_DRM_STI_FBDEV
> +#ifdef CONFIG_DRM_FBDEV_EMULATION
>         drm_fbdev_cma_init(dev, 32,
>                    dev->mode_config.num_crtc,
>                    dev->mode_config.num_connector);
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> hosted by The Linux Foundation
>
>


-- 

Benjamin Gaignard

Graphic Working Group

Linaro.org <http://www.linaro.org/> *│ *Open source software for ARM SoCs

Follow *Linaro: *Facebook <http://www.facebook.com/pages/Linaro> | Twitter
<http://twitter.com/#!/linaroorg> | Blog
<http://www.linaro.org/linaro-blog/>

[-- Attachment #1.2: Type: text/html, Size: 4580 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Daniel Vetter]

On Wed, Mar 11, 2015 at 01:51:02PM +0530, Archit Taneja wrote:
> 
> 
> 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.

Hm, sounds like we need another patch to stub out fb_set_suspend when
fbdev isn't enabled. Is there anything else?

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

Yeah we definitely can't mix select with a user-visible option. I think we
just need to fix up the remaining functions and create stubs for them if
needed and then drop all the selects. Well in DRM_FBDEV_EMULATION we
should still select for fbcon since otherwise tons of bug reports about
"where is my fbcon with kms?".
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Archit Taneja]

On 03/11/2015 08:47 PM, Daniel Vetter wrote:
> On Wed, Mar 11, 2015 at 01:51:02PM +0530, Archit Taneja wrote:
>>
>>
>> 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.
>
> Hm, sounds like we need another patch to stub out fb_set_suspend when
> fbdev isn't enabled. Is there anything else?

There are a handful of fb core functions which are called by drm drivers:

fb_alloc_cmap/fb_dealloc_cmap

fb_sys_read/fb_sys_write

register_framebuffer/unregister_framebuffer/unlink_framebuffer/
remove_conflicting_framebuffers

fb_set_suspend

fb_deferred_io_init/fb_deferred_io_cleanup

framebuffer_alloc/framebuffer_release


>
>>>> 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?
>
> Yeah we definitely can't mix select with a user-visible option. I think we
> just need to fix up the remaining functions and create stubs for them if
> needed and then drop all the selects. Well in DRM_FBDEV_EMULATION we
> should still select for fbcon since otherwise tons of bug reports about
> "where is my fbcon with kms?".

I'll give this a try. Although, it would be a better idea to make the 
drivers not call these at all when fbdev emulation isn't asked for. With 
the stubs, if someone does try to use the driver with 
DRM_FBDEV_EMULATION not set, the worst that'll happen would be that the 
driver fails to probe. Which isn't so bad.

Archit

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Daniel Vetter]

On Fri, Mar 13, 2015 at 11:55:07AM +0530, Archit Taneja wrote:
> 
> 
> On 03/11/2015 08:47 PM, Daniel Vetter wrote:
> >On Wed, Mar 11, 2015 at 01:51:02PM +0530, Archit Taneja wrote:
> >>
> >>
> >>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.
> >
> >Hm, sounds like we need another patch to stub out fb_set_suspend when
> >fbdev isn't enabled. Is there anything else?
> 
> There are a handful of fb core functions which are called by drm drivers:
> 
> fb_alloc_cmap/fb_dealloc_cmap
> 
> fb_sys_read/fb_sys_write
> 
> register_framebuffer/unregister_framebuffer/unlink_framebuffer/
> remove_conflicting_framebuffers
> 
> fb_set_suspend
> 
> fb_deferred_io_init/fb_deferred_io_cleanup
> 
> framebuffer_alloc/framebuffer_release

Hm yeah that's somewhat annoying indeed. What about the following:
1. We move all the #include <linux/fb.h> from drivers into drm_fb_helper.h

2. Then we add stubs for these functions in drm_fb_helper.h, like this

#if defined(CONFIG_FB)
#include <linux/fb.h>
#else

/* static inline stubs for all the fb stuff used by kms drivers */
#endif

Imo this makes sense since kms drivers really have a bit a special
situation with fbdev. They're not full-blown fbdev drivers and can be
useful fully without fbdev.

What do you think?
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Jani Nikula]

On Fri, 13 Mar 2015, Daniel Vetter <daniel@ffwll.ch> wrote:
> #if defined(CONFIG_FB)
> #include <linux/fb.h>
> #else

Side note, #if IS_ENABLED(CONFIG_FB)

BR,
Jani.


-- 
Jani Nikula, Intel Open Source Technology Center

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

[Archit Taneja]

On 03/13/2015 02:36 PM, Daniel Vetter wrote:
> On Fri, Mar 13, 2015 at 11:55:07AM +0530, Archit Taneja wrote:
>>
>>
>> On 03/11/2015 08:47 PM, Daniel Vetter wrote:
>>> On Wed, Mar 11, 2015 at 01:51:02PM +0530, Archit Taneja wrote:
>>>>
>>>>
>>>> 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.
>>>
>>> Hm, sounds like we need another patch to stub out fb_set_suspend when
>>> fbdev isn't enabled. Is there anything else?
>>
>> There are a handful of fb core functions which are called by drm drivers:
>>
>> fb_alloc_cmap/fb_dealloc_cmap
>>
>> fb_sys_read/fb_sys_write
>>
>> register_framebuffer/unregister_framebuffer/unlink_framebuffer/
>> remove_conflicting_framebuffers
>>
>> fb_set_suspend
>>
>> fb_deferred_io_init/fb_deferred_io_cleanup
>>
>> framebuffer_alloc/framebuffer_release
>
> Hm yeah that's somewhat annoying indeed. What about the following:
> 1. We move all the #include <linux/fb.h> from drivers into drm_fb_helper.h
>
> 2. Then we add stubs for these functions in drm_fb_helper.h, like this
>
> #if defined(CONFIG_FB)
> #include <linux/fb.h>
> #else
>
> /* static inline stubs for all the fb stuff used by kms drivers */
> #endif
>
> Imo this makes sense since kms drivers really have a bit a special
> situation with fbdev. They're not full-blown fbdev drivers and can be
> useful fully without fbdev.
>
> What do you think?

This looks good! I'll give it a try.

Archit

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Archit Taneja]

Hi,

On 03/13/2015 02:36 PM, Daniel Vetter wrote:
> On Fri, Mar 13, 2015 at 11:55:07AM +0530, Archit Taneja wrote:
>>
>>
>> On 03/11/2015 08:47 PM, Daniel Vetter wrote:
>>> On Wed, Mar 11, 2015 at 01:51:02PM +0530, Archit Taneja wrote:
>>>>
>>>>
>>>> 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.
>>>
>>> Hm, sounds like we need another patch to stub out fb_set_suspend when
>>> fbdev isn't enabled. Is there anything else?
>>
>> There are a handful of fb core functions which are called by drm drivers:
>>
>> fb_alloc_cmap/fb_dealloc_cmap
>>
>> fb_sys_read/fb_sys_write
>>
>> register_framebuffer/unregister_framebuffer/unlink_framebuffer/
>> remove_conflicting_framebuffers
>>
>> fb_set_suspend
>>
>> fb_deferred_io_init/fb_deferred_io_cleanup
>>
>> framebuffer_alloc/framebuffer_release
>
> Hm yeah that's somewhat annoying indeed. What about the following:
> 1. We move all the #include <linux/fb.h> from drivers into drm_fb_helper.h
>
> 2. Then we add stubs for these functions in drm_fb_helper.h, like this
>
> #if defined(CONFIG_FB)
> #include <linux/fb.h>
> #else
>
> /* static inline stubs for all the fb stuff used by kms drivers */
> #endif
>
> Imo this makes sense since kms drivers really have a bit a special
> situation with fbdev. They're not full-blown fbdev drivers and can be
> useful fully without fbdev.
>

I was trying this out. Removing 'linux/fb.h' and replacing stub fb funcs 
won't really work because struct declarations(like fb_info) also get 
removed.

I considered placing '#if IS_ENABLED(CONFIG_FB)' within linux/fb.h 
itself, but that seemed a bit too intrusive.

This is what I'm currently doing:

- Some funcs, like framebufer_alloc/release, alloc_cmap/dealloc_cmap 
would actually benefit if we have drm fb helpers for them. They are used 
in exactly the same manner by all the drivers.

- For the rest of the functions that are sparsely used, I was 
considering making very simple drm_fb_* wrapper functions. Something like:

void drm_fb_helper_deferred_io_init(struct drm_fb_helper *helper)
{
	if (helper->fbdev)
		fb_deferred_io_init(helper->fbdev);
}

We could have all fb calls called within drm_fb_helper.c, creating 
drm_fb_helper_* stub functions would then be an easier task. What do you 
think?

Archit

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Daniel Vetter]

On Wed, Mar 25, 2015 at 01:47:54PM +0530, Archit Taneja wrote:
> Hi,
> 
> On 03/13/2015 02:36 PM, Daniel Vetter wrote:
> >On Fri, Mar 13, 2015 at 11:55:07AM +0530, Archit Taneja wrote:
> >>
> >>
> >>On 03/11/2015 08:47 PM, Daniel Vetter wrote:
> >>>On Wed, Mar 11, 2015 at 01:51:02PM +0530, Archit Taneja wrote:
> >>>>
> >>>>
> >>>>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.
> >>>
> >>>Hm, sounds like we need another patch to stub out fb_set_suspend when
> >>>fbdev isn't enabled. Is there anything else?
> >>
> >>There are a handful of fb core functions which are called by drm drivers:
> >>
> >>fb_alloc_cmap/fb_dealloc_cmap
> >>
> >>fb_sys_read/fb_sys_write
> >>
> >>register_framebuffer/unregister_framebuffer/unlink_framebuffer/
> >>remove_conflicting_framebuffers
> >>
> >>fb_set_suspend
> >>
> >>fb_deferred_io_init/fb_deferred_io_cleanup
> >>
> >>framebuffer_alloc/framebuffer_release
> >
> >Hm yeah that's somewhat annoying indeed. What about the following:
> >1. We move all the #include <linux/fb.h> from drivers into drm_fb_helper.h
> >
> >2. Then we add stubs for these functions in drm_fb_helper.h, like this
> >
> >#if defined(CONFIG_FB)
> >#include <linux/fb.h>
> >#else
> >
> >/* static inline stubs for all the fb stuff used by kms drivers */
> >#endif
> >
> >Imo this makes sense since kms drivers really have a bit a special
> >situation with fbdev. They're not full-blown fbdev drivers and can be
> >useful fully without fbdev.
> >
> 
> I was trying this out. Removing 'linux/fb.h' and replacing stub fb funcs
> won't really work because struct declarations(like fb_info) also get
> removed.
> 
> I considered placing '#if IS_ENABLED(CONFIG_FB)' within linux/fb.h itself,
> but that seemed a bit too intrusive.
> 
> This is what I'm currently doing:
> 
> - Some funcs, like framebufer_alloc/release, alloc_cmap/dealloc_cmap would
> actually benefit if we have drm fb helpers for them. They are used in
> exactly the same manner by all the drivers.
> 
> - For the rest of the functions that are sparsely used, I was considering
> making very simple drm_fb_* wrapper functions. Something like:
> 
> void drm_fb_helper_deferred_io_init(struct drm_fb_helper *helper)
> {
> 	if (helper->fbdev)
> 		fb_deferred_io_init(helper->fbdev);
> }
> 
> We could have all fb calls called within drm_fb_helper.c, creating
> drm_fb_helper_* stub functions would then be an easier task. What do you
> think?

That's actually an option I considered, but I hoped we could do it with
less work. If this indeed works and you can create the patch that would be
awesome.

Thanks, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

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

[Daniel Vetter]

Any updates on this or too much distractions? I really think doing
this would be awesome for the drm subsystem, instead of reinventing
this wheel for each driver.
-Daniel

On Wed, Mar 25, 2015 at 10:21 AM, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Wed, Mar 25, 2015 at 01:47:54PM +0530, Archit Taneja wrote:
>> Hi,
>>
>> On 03/13/2015 02:36 PM, Daniel Vetter wrote:
>> >On Fri, Mar 13, 2015 at 11:55:07AM +0530, Archit Taneja wrote:
>> >>
>> >>
>> >>On 03/11/2015 08:47 PM, Daniel Vetter wrote:
>> >>>On Wed, Mar 11, 2015 at 01:51:02PM +0530, Archit Taneja wrote:
>> >>>>
>> >>>>
>> >>>>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.
>> >>>
>> >>>Hm, sounds like we need another patch to stub out fb_set_suspend when
>> >>>fbdev isn't enabled. Is there anything else?
>> >>
>> >>There are a handful of fb core functions which are called by drm drivers:
>> >>
>> >>fb_alloc_cmap/fb_dealloc_cmap
>> >>
>> >>fb_sys_read/fb_sys_write
>> >>
>> >>register_framebuffer/unregister_framebuffer/unlink_framebuffer/
>> >>remove_conflicting_framebuffers
>> >>
>> >>fb_set_suspend
>> >>
>> >>fb_deferred_io_init/fb_deferred_io_cleanup
>> >>
>> >>framebuffer_alloc/framebuffer_release
>> >
>> >Hm yeah that's somewhat annoying indeed. What about the following:
>> >1. We move all the #include <linux/fb.h> from drivers into drm_fb_helper.h
>> >
>> >2. Then we add stubs for these functions in drm_fb_helper.h, like this
>> >
>> >#if defined(CONFIG_FB)
>> >#include <linux/fb.h>
>> >#else
>> >
>> >/* static inline stubs for all the fb stuff used by kms drivers */
>> >#endif
>> >
>> >Imo this makes sense since kms drivers really have a bit a special
>> >situation with fbdev. They're not full-blown fbdev drivers and can be
>> >useful fully without fbdev.
>> >
>>
>> I was trying this out. Removing 'linux/fb.h' and replacing stub fb funcs
>> won't really work because struct declarations(like fb_info) also get
>> removed.
>>
>> I considered placing '#if IS_ENABLED(CONFIG_FB)' within linux/fb.h itself,
>> but that seemed a bit too intrusive.
>>
>> This is what I'm currently doing:
>>
>> - Some funcs, like framebufer_alloc/release, alloc_cmap/dealloc_cmap would
>> actually benefit if we have drm fb helpers for them. They are used in
>> exactly the same manner by all the drivers.
>>
>> - For the rest of the functions that are sparsely used, I was considering
>> making very simple drm_fb_* wrapper functions. Something like:
>>
>> void drm_fb_helper_deferred_io_init(struct drm_fb_helper *helper)
>> {
>>       if (helper->fbdev)
>>               fb_deferred_io_init(helper->fbdev);
>> }
>>
>> We could have all fb calls called within drm_fb_helper.c, creating
>> drm_fb_helper_* stub functions would then be an easier task. What do you
>> think?
>
> That's actually an option I considered, but I hoped we could do it with
> less work. If this indeed works and you can create the patch that would be
> awesome.
>
> Thanks, Daniel
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch



-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Archit Taneja]

Hi,

On 06/30/2015 12:40 PM, Daniel Vetter wrote:
> Any updates on this or too much distractions? I really think doing
> this would be awesome for the drm subsystem, instead of reinventing
> this wheel for each driver.

I'd started on this again. I've got more free time now than before, so I 
should have something in a week or so. I agree it will help a lot (there 
are already two new drivers since we started discussing this!)

Archit

> -Daniel
>
> On Wed, Mar 25, 2015 at 10:21 AM, Daniel Vetter <daniel@ffwll.ch> wrote:
>> On Wed, Mar 25, 2015 at 01:47:54PM +0530, Archit Taneja wrote:
>>> Hi,
>>>
>>> On 03/13/2015 02:36 PM, Daniel Vetter wrote:
>>>> On Fri, Mar 13, 2015 at 11:55:07AM +0530, Archit Taneja wrote:
>>>>>
>>>>>
>>>>> On 03/11/2015 08:47 PM, Daniel Vetter wrote:
>>>>>> On Wed, Mar 11, 2015 at 01:51:02PM +0530, Archit Taneja wrote:
>>>>>>>
>>>>>>>
>>>>>>> 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.
>>>>>>
>>>>>> Hm, sounds like we need another patch to stub out fb_set_suspend when
>>>>>> fbdev isn't enabled. Is there anything else?
>>>>>
>>>>> There are a handful of fb core functions which are called by drm drivers:
>>>>>
>>>>> fb_alloc_cmap/fb_dealloc_cmap
>>>>>
>>>>> fb_sys_read/fb_sys_write
>>>>>
>>>>> register_framebuffer/unregister_framebuffer/unlink_framebuffer/
>>>>> remove_conflicting_framebuffers
>>>>>
>>>>> fb_set_suspend
>>>>>
>>>>> fb_deferred_io_init/fb_deferred_io_cleanup
>>>>>
>>>>> framebuffer_alloc/framebuffer_release
>>>>
>>>> Hm yeah that's somewhat annoying indeed. What about the following:
>>>> 1. We move all the #include <linux/fb.h> from drivers into drm_fb_helper.h
>>>>
>>>> 2. Then we add stubs for these functions in drm_fb_helper.h, like this
>>>>
>>>> #if defined(CONFIG_FB)
>>>> #include <linux/fb.h>
>>>> #else
>>>>
>>>> /* static inline stubs for all the fb stuff used by kms drivers */
>>>> #endif
>>>>
>>>> Imo this makes sense since kms drivers really have a bit a special
>>>> situation with fbdev. They're not full-blown fbdev drivers and can be
>>>> useful fully without fbdev.
>>>>
>>>
>>> I was trying this out. Removing 'linux/fb.h' and replacing stub fb funcs
>>> won't really work because struct declarations(like fb_info) also get
>>> removed.
>>>
>>> I considered placing '#if IS_ENABLED(CONFIG_FB)' within linux/fb.h itself,
>>> but that seemed a bit too intrusive.
>>>
>>> This is what I'm currently doing:
>>>
>>> - Some funcs, like framebufer_alloc/release, alloc_cmap/dealloc_cmap would
>>> actually benefit if we have drm fb helpers for them. They are used in
>>> exactly the same manner by all the drivers.
>>>
>>> - For the rest of the functions that are sparsely used, I was considering
>>> making very simple drm_fb_* wrapper functions. Something like:
>>>
>>> void drm_fb_helper_deferred_io_init(struct drm_fb_helper *helper)
>>> {
>>>        if (helper->fbdev)
>>>                fb_deferred_io_init(helper->fbdev);
>>> }
>>>
>>> We could have all fb calls called within drm_fb_helper.c, creating
>>> drm_fb_helper_* stub functions would then be an easier task. What do you
>>> think?
>>
>> That's actually an option I considered, but I hoped we could do it with
>> less work. If this indeed works and you can create the patch that would be
>> awesome.
>>
>> Thanks, Daniel
>> --
>> Daniel Vetter
>> Software Engineer, Intel Corporation
>> http://blog.ffwll.ch
>
>
>

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

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

[Benjamin Gaignard]

Hi,

I think that what have been done by Rob with module_param is also a good idea:
https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/commit/drivers/gpu/drm/msm/msm_drv.c?id=e90dfec78ec288d6c89a7b508a5c5d4ae8b7f934

Can you mix compilation flag and module param ?

Benjamin

2015-06-30 9:56 GMT+02:00 Archit Taneja <architt@codeaurora.org>:
> Hi,
>
> On 06/30/2015 12:40 PM, Daniel Vetter wrote:
>>
>> Any updates on this or too much distractions? I really think doing
>> this would be awesome for the drm subsystem, instead of reinventing
>> this wheel for each driver.
>
>
> I'd started on this again. I've got more free time now than before, so I
> should have something in a week or so. I agree it will help a lot (there are
> already two new drivers since we started discussing this!)
>
> Archit
>
>
>> -Daniel
>>
>> On Wed, Mar 25, 2015 at 10:21 AM, Daniel Vetter <daniel@ffwll.ch> wrote:
>>>
>>> On Wed, Mar 25, 2015 at 01:47:54PM +0530, Archit Taneja wrote:
>>>>
>>>> Hi,
>>>>
>>>> On 03/13/2015 02:36 PM, Daniel Vetter wrote:
>>>>>
>>>>> On Fri, Mar 13, 2015 at 11:55:07AM +0530, Archit Taneja wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>> On 03/11/2015 08:47 PM, Daniel Vetter wrote:
>>>>>>>
>>>>>>> On Wed, Mar 11, 2015 at 01:51:02PM +0530, Archit Taneja wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> 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.
>>>>>>>
>>>>>>>
>>>>>>> Hm, sounds like we need another patch to stub out fb_set_suspend when
>>>>>>> fbdev isn't enabled. Is there anything else?
>>>>>>
>>>>>>
>>>>>> There are a handful of fb core functions which are called by drm
>>>>>> drivers:
>>>>>>
>>>>>> fb_alloc_cmap/fb_dealloc_cmap
>>>>>>
>>>>>> fb_sys_read/fb_sys_write
>>>>>>
>>>>>> register_framebuffer/unregister_framebuffer/unlink_framebuffer/
>>>>>> remove_conflicting_framebuffers
>>>>>>
>>>>>> fb_set_suspend
>>>>>>
>>>>>> fb_deferred_io_init/fb_deferred_io_cleanup
>>>>>>
>>>>>> framebuffer_alloc/framebuffer_release
>>>>>
>>>>>
>>>>> Hm yeah that's somewhat annoying indeed. What about the following:
>>>>> 1. We move all the #include <linux/fb.h> from drivers into
>>>>> drm_fb_helper.h
>>>>>
>>>>> 2. Then we add stubs for these functions in drm_fb_helper.h, like this
>>>>>
>>>>> #if defined(CONFIG_FB)
>>>>> #include <linux/fb.h>
>>>>> #else
>>>>>
>>>>> /* static inline stubs for all the fb stuff used by kms drivers */
>>>>> #endif
>>>>>
>>>>> Imo this makes sense since kms drivers really have a bit a special
>>>>> situation with fbdev. They're not full-blown fbdev drivers and can be
>>>>> useful fully without fbdev.
>>>>>
>>>>
>>>> I was trying this out. Removing 'linux/fb.h' and replacing stub fb funcs
>>>> won't really work because struct declarations(like fb_info) also get
>>>> removed.
>>>>
>>>> I considered placing '#if IS_ENABLED(CONFIG_FB)' within linux/fb.h
>>>> itself,
>>>> but that seemed a bit too intrusive.
>>>>
>>>> This is what I'm currently doing:
>>>>
>>>> - Some funcs, like framebufer_alloc/release, alloc_cmap/dealloc_cmap
>>>> would
>>>> actually benefit if we have drm fb helpers for them. They are used in
>>>> exactly the same manner by all the drivers.
>>>>
>>>> - For the rest of the functions that are sparsely used, I was
>>>> considering
>>>> making very simple drm_fb_* wrapper functions. Something like:
>>>>
>>>> void drm_fb_helper_deferred_io_init(struct drm_fb_helper *helper)
>>>> {
>>>>        if (helper->fbdev)
>>>>                fb_deferred_io_init(helper->fbdev);
>>>> }
>>>>
>>>> We could have all fb calls called within drm_fb_helper.c, creating
>>>> drm_fb_helper_* stub functions would then be an easier task. What do you
>>>> think?
>>>
>>>
>>> That's actually an option I considered, but I hoped we could do it with
>>> less work. If this indeed works and you can create the patch that would
>>> be
>>> awesome.
>>>
>>> Thanks, Daniel
>>> --
>>> Daniel Vetter
>>> Software Engineer, Intel Corporation
>>> http://blog.ffwll.ch
>>
>>
>>
>>
>
> --
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> a Linux Foundation Collaborative Project



-- 
Benjamin Gaignard

Graphic Working Group

Linaro.org │ Open source software for ARM SoCs

Follow Linaro: Facebook | Twitter | Blog
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Daniel Vetter]

On Tue, Jun 30, 2015 at 10:31 AM, Benjamin Gaignard
<benjamin.gaignard@linaro.org> wrote:
> I think that what have been done by Rob with module_param is also a good idea:
> https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/commit/drivers/gpu/drm/msm/msm_drv.c?id=e90dfec78ec288d6c89a7b508a5c5d4ae8b7f934
>
> Can you mix compilation flag and module param ?

Hm, the point of the kconfig is to not require all the legacy baggage
from fbdev. But I guess a module option could be added later on too.
bochs has it too.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Archit Taneja]

Hi,

On 06/30/2015 02:01 PM, Benjamin Gaignard wrote:
> Hi,
>
> I think that what have been done by Rob with module_param is also a good idea:
> https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/commit/drivers/gpu/drm/msm/msm_drv.c?id=e90dfec78ec288d6c89a7b508a5c5d4ae8b7f934
>
> Can you mix compilation flag and module param ?

I guess this won't be hard to do once we have a common fbdev emulation 
config option. We could consider this as a drm top level module param.

I'll keep this in mind.

Thanks,
Archit

>
> Benjamin
>
> 2015-06-30 9:56 GMT+02:00 Archit Taneja <architt@codeaurora.org>:
>> Hi,
>>
>> On 06/30/2015 12:40 PM, Daniel Vetter wrote:
>>>
>>> Any updates on this or too much distractions? I really think doing
>>> this would be awesome for the drm subsystem, instead of reinventing
>>> this wheel for each driver.
>>
>>
>> I'd started on this again. I've got more free time now than before, so I
>> should have something in a week or so. I agree it will help a lot (there are
>> already two new drivers since we started discussing this!)
>>
>> Archit
>>
>>
>>> -Daniel
>>>
>>> On Wed, Mar 25, 2015 at 10:21 AM, Daniel Vetter <daniel@ffwll.ch> wrote:
>>>>
>>>> On Wed, Mar 25, 2015 at 01:47:54PM +0530, Archit Taneja wrote:
>>>>>
>>>>> Hi,
>>>>>
>>>>> On 03/13/2015 02:36 PM, Daniel Vetter wrote:
>>>>>>
>>>>>> On Fri, Mar 13, 2015 at 11:55:07AM +0530, Archit Taneja wrote:
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> On 03/11/2015 08:47 PM, Daniel Vetter wrote:
>>>>>>>>
>>>>>>>> On Wed, Mar 11, 2015 at 01:51:02PM +0530, Archit Taneja wrote:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> 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.
>>>>>>>>
>>>>>>>>
>>>>>>>> Hm, sounds like we need another patch to stub out fb_set_suspend when
>>>>>>>> fbdev isn't enabled. Is there anything else?
>>>>>>>
>>>>>>>
>>>>>>> There are a handful of fb core functions which are called by drm
>>>>>>> drivers:
>>>>>>>
>>>>>>> fb_alloc_cmap/fb_dealloc_cmap
>>>>>>>
>>>>>>> fb_sys_read/fb_sys_write
>>>>>>>
>>>>>>> register_framebuffer/unregister_framebuffer/unlink_framebuffer/
>>>>>>> remove_conflicting_framebuffers
>>>>>>>
>>>>>>> fb_set_suspend
>>>>>>>
>>>>>>> fb_deferred_io_init/fb_deferred_io_cleanup
>>>>>>>
>>>>>>> framebuffer_alloc/framebuffer_release
>>>>>>
>>>>>>
>>>>>> Hm yeah that's somewhat annoying indeed. What about the following:
>>>>>> 1. We move all the #include <linux/fb.h> from drivers into
>>>>>> drm_fb_helper.h
>>>>>>
>>>>>> 2. Then we add stubs for these functions in drm_fb_helper.h, like this
>>>>>>
>>>>>> #if defined(CONFIG_FB)
>>>>>> #include <linux/fb.h>
>>>>>> #else
>>>>>>
>>>>>> /* static inline stubs for all the fb stuff used by kms drivers */
>>>>>> #endif
>>>>>>
>>>>>> Imo this makes sense since kms drivers really have a bit a special
>>>>>> situation with fbdev. They're not full-blown fbdev drivers and can be
>>>>>> useful fully without fbdev.
>>>>>>
>>>>>
>>>>> I was trying this out. Removing 'linux/fb.h' and replacing stub fb funcs
>>>>> won't really work because struct declarations(like fb_info) also get
>>>>> removed.
>>>>>
>>>>> I considered placing '#if IS_ENABLED(CONFIG_FB)' within linux/fb.h
>>>>> itself,
>>>>> but that seemed a bit too intrusive.
>>>>>
>>>>> This is what I'm currently doing:
>>>>>
>>>>> - Some funcs, like framebufer_alloc/release, alloc_cmap/dealloc_cmap
>>>>> would
>>>>> actually benefit if we have drm fb helpers for them. They are used in
>>>>> exactly the same manner by all the drivers.
>>>>>
>>>>> - For the rest of the functions that are sparsely used, I was
>>>>> considering
>>>>> making very simple drm_fb_* wrapper functions. Something like:
>>>>>
>>>>> void drm_fb_helper_deferred_io_init(struct drm_fb_helper *helper)
>>>>> {
>>>>>         if (helper->fbdev)
>>>>>                 fb_deferred_io_init(helper->fbdev);
>>>>> }
>>>>>
>>>>> We could have all fb calls called within drm_fb_helper.c, creating
>>>>> drm_fb_helper_* stub functions would then be an easier task. What do you
>>>>> think?
>>>>
>>>>
>>>> That's actually an option I considered, but I hoped we could do it with
>>>> less work. If this indeed works and you can create the patch that would
>>>> be
>>>> awesome.
>>>>
>>>> Thanks, Daniel
>>>> --
>>>> Daniel Vetter
>>>> Software Engineer, Intel Corporation
>>>> http://blog.ffwll.ch
>>>
>>>
>>>
>>>
>>
>> --
>> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
>> a Linux Foundation Collaborative Project
>
>
>

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Thierry Reding]

[-- Attachment #1.1: Type: text/plain, Size: 956 bytes --]

On Tue, Jun 30, 2015 at 11:04:45AM +0200, Daniel Vetter wrote:
> On Tue, Jun 30, 2015 at 10:31 AM, Benjamin Gaignard
> <benjamin.gaignard@linaro.org> wrote:
> > I think that what have been done by Rob with module_param is also a good idea:
> > https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/commit/drivers/gpu/drm/msm/msm_drv.c?id=e90dfec78ec288d6c89a7b508a5c5d4ae8b7f934
> >
> > Can you mix compilation flag and module param ?
> 
> Hm, the point of the kconfig is to not require all the legacy baggage
> from fbdev. But I guess a module option could be added later on too.
> bochs has it too.

I think the option would primarily be useful for testing. I've often
needed to test modesetting code without fbdev getting in the way and the
only way to currently do that is by recompiling with legacy fbdev
disabled. The option would allow an fbdev-enabled kernel to be used but
disable the fbdev code if desired.

Thierry

[-- Attachment #1.2: Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Rob Clark]

On Fri, Jul 3, 2015 at 8:32 AM, Thierry Reding <thierry.reding@gmail.com> wrote:
> On Tue, Jun 30, 2015 at 11:04:45AM +0200, Daniel Vetter wrote:
>> On Tue, Jun 30, 2015 at 10:31 AM, Benjamin Gaignard
>> <benjamin.gaignard@linaro.org> wrote:
>> > I think that what have been done by Rob with module_param is also a good idea:
>> > https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/commit/drivers/gpu/drm/msm/msm_drv.c?id=e90dfec78ec288d6c89a7b508a5c5d4ae8b7f934
>> >
>> > Can you mix compilation flag and module param ?
>>
>> Hm, the point of the kconfig is to not require all the legacy baggage
>> from fbdev. But I guess a module option could be added later on too.
>> bochs has it too.
>
> I think the option would primarily be useful for testing. I've often
> needed to test modesetting code without fbdev getting in the way and the
> only way to currently do that is by recompiling with legacy fbdev
> disabled. The option would allow an fbdev-enabled kernel to be used but
> disable the fbdev code if desired.

and for debugging.. when things are going wrong it's nice for the
first modeset to not happen under console_lock ;-)

BR,
-R

> Thierry
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[RFC v2 0/6] drm: fb emulation: Step 2: Create a fbdev emulation config option

[Archit Taneja]

This provides a uniform interface to enable/disable legacy fbdev support
for modesetting drivers.

DRM_FBDEV_EMULATION is made available in the top level drm Kconfig. A
driver that wants fbdev emulation using drm_fb_helper can enable this
option.

Apart from 5 drivers (msm, tegra, imx, sti, i915), all enable fbdev
emulation by default. For these drivers, if we explicitly disable fbdev
emulation, the 'hope' is that we will cleanly bail out with an error. We
set DRM_FBDEV_EMULATION to 'y' by default, since most drivers rely on it.


Archit Taneja (6):
  drm: Add top level Kconfig option for DRM fbdev emulation
  drm/msm: Remove local fbdev emulation Kconfig option
  drm/tegra: Remove local fbdev emulation Kconfig option
  drm/imx: Remove local fbdev emulation Kconfig option
  drm/sti: Remove local fbdev emulation Kconfig option
  drm/i915: Remove local fbdev emulation Kconfig option

 drivers/gpu/drm/Kconfig              |  12 +++
 drivers/gpu/drm/Makefile             |   2 +-
 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 --
 drivers/gpu/drm/imx/Kconfig          |   9 --
 drivers/gpu/drm/imx/imx-drm-core.c   |  10 +-
 drivers/gpu/drm/msm/Kconfig          |  14 ---
 drivers/gpu/drm/msm/Makefile         |   2 +-
 drivers/gpu/drm/msm/msm_drv.c        |   8 +-
 drivers/gpu/drm/sti/Kconfig          |   6 --
 drivers/gpu/drm/sti/sti_drm_drv.c    |   2 +-
 drivers/gpu/drm/tegra/Kconfig        |  12 ---
 drivers/gpu/drm/tegra/drm.c          |  15 ++-
 drivers/gpu/drm/tegra/drm.h          |   8 --
 drivers/gpu/drm/tegra/fb.c           |  25 ++---
 include/drm/drm_fb_helper.h          | 192 +++++++++++++++++++++++++++++++++++
 22 files changed, 242 insertions(+), 132 deletions(-)

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

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

[Archit Taneja]

Legacy fbdev emulation support via DRM is achieved through KMS FB helpers.
Most modesetting drivers enable provide fbdev emulation by default by
selecting KMS FB helpers. A few provide a separate Kconfig option for the
user to enable or disbale fbdev emulation.

Enabling fbdev emulation is finally a distro-level decision. Having a top
level Kconfig option for fbdev emulation helps by providing a uniform way
to enable/disable fbdev emulation for any modesetting driver. It also lets
us remove unnecessary driver specific Kconfig options that causes bloat.

With a top level Kconfig in place, we can stub out the fb helper functions
when not needed without breaking functionality. Having stub functions also
prevents drivers to require wrapping fb helper function calls with #ifdefs.

DRM_FBDEV_EMULATION defaults to y since many drivers enable fbdev
emulation by default and majority of distributions expect the fbdev
interface in the kernel.

Signed-off-by: Archit Taneja <architt@codeaurora.org>
---
 drivers/gpu/drm/Kconfig     |  12 +++
 drivers/gpu/drm/Makefile    |   2 +-
 include/drm/drm_fb_helper.h | 192 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 205 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index e373f8a..8fd670b 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -47,6 +47,18 @@ 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
+	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. You'd need this if
+	  you're looking for console support too.
+
 config DRM_LOAD_EDID_FIRMWARE
 	bool "Allow to specify an EDID data set instead of probing for it"
 	depends on DRM_KMS_HELPER
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index 5713d05..8858510 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -23,7 +23,7 @@ drm-$(CONFIG_OF) += drm_of.o
 drm_kms_helper-y := drm_crtc_helper.o drm_dp_helper.o drm_probe_helper.o \
 		drm_plane_helper.o drm_dp_mst_topology.o drm_atomic_helper.o
 drm_kms_helper-$(CONFIG_DRM_LOAD_EDID_FIRMWARE) += drm_edid_load.o
-drm_kms_helper-$(CONFIG_DRM_KMS_FB_HELPER) += drm_fb_helper.o
+drm_kms_helper-$(CONFIG_DRM_FBDEV_EMULATION) += drm_fb_helper.o
 drm_kms_helper-$(CONFIG_DRM_KMS_CMA_HELPER) += drm_fb_cma_helper.o
 
 obj-$(CONFIG_DRM_KMS_HELPER) += drm_kms_helper.o
diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h
index cc19e88..8f972da 100644
--- a/include/drm/drm_fb_helper.h
+++ b/include/drm/drm_fb_helper.h
@@ -122,6 +122,7 @@ struct drm_fb_helper {
 	bool delayed_hotplug;
 };
 
+#ifdef CONFIG_DRM_FBDEV_EMULATION
 void drm_fb_helper_prepare(struct drm_device *dev, struct drm_fb_helper *helper,
 			   const struct drm_fb_helper_funcs *funcs);
 int drm_fb_helper_init(struct drm_device *dev,
@@ -188,4 +189,195 @@ drm_pick_cmdline_mode(struct drm_fb_helper_connector *fb_helper_conn,
 int drm_fb_helper_add_one_connector(struct drm_fb_helper *fb_helper, struct drm_connector *connector);
 int drm_fb_helper_remove_one_connector(struct drm_fb_helper *fb_helper,
 				       struct drm_connector *connector);
+#else
+static inline void drm_fb_helper_prepare(struct drm_device *dev,
+					struct drm_fb_helper *helper,
+					const struct drm_fb_helper_funcs *funcs)
+{
+}
+
+static inline int drm_fb_helper_init(struct drm_device *dev,
+		       struct drm_fb_helper *helper, int crtc_count,
+		       int max_conn)
+{
+	return 0;
+}
+
+static inline void drm_fb_helper_fini(struct drm_fb_helper *helper)
+{
+}
+
+static inline int drm_fb_helper_blank(int blank, struct fb_info *info)
+{
+	return 0;
+}
+
+static inline int drm_fb_helper_pan_display(struct fb_var_screeninfo *var,
+					    struct fb_info *info)
+{
+	return 0;
+}
+
+static inline int drm_fb_helper_set_par(struct fb_info *info)
+{
+	return 0;
+}
+
+static inline int drm_fb_helper_check_var(struct fb_var_screeninfo *var,
+					  struct fb_info *info)
+{
+	return 0;
+}
+
+static inline bool
+drm_fb_helper_restore_fbdev_mode_unlocked(struct drm_fb_helper *fb_helper)
+{
+	return true;
+}
+
+static inline struct fb_info *
+drm_fb_helper_alloc_fbi(struct drm_fb_helper *fb_helper)
+{
+	return NULL;
+}
+
+static inline void drm_fb_helper_unregister_fbi(struct drm_fb_helper *fb_helper)
+{
+}
+static inline void drm_fb_helper_release_fbi(struct drm_fb_helper *fb_helper)
+{
+}
+
+static inline void drm_fb_helper_fill_var(struct fb_info *info,
+					  struct drm_fb_helper *fb_helper,
+					  uint32_t fb_width, uint32_t fb_height)
+{
+}
+
+static inline void drm_fb_helper_fill_fix(struct fb_info *info, uint32_t pitch,
+					  uint32_t depth)
+{
+}
+
+static inline int drm_fb_helper_setcmap(struct fb_cmap *cmap,
+					struct fb_info *info)
+{
+	return 0;
+}
+
+static inline void drm_fb_helper_unlink_fbi(struct drm_fb_helper *fb_helper)
+{
+}
+
+static inline ssize_t drm_fb_helper_sys_read(struct fb_info *info,
+					char __user *buf, size_t count,
+					loff_t *ppos)
+{
+	return -ENODEV;
+}
+
+static inline ssize_t drm_fb_helper_sys_write(struct fb_info *info,
+					const char __user *buf, size_t count,
+					loff_t *ppos)
+{
+	return -ENODEV;
+}
+
+static inline void drm_fb_helper_sys_fillrect(struct fb_info *info,
+		const struct fb_fillrect *rect)
+{
+}
+
+static inline void drm_fb_helper_sys_copyarea(struct fb_info *info,
+		const struct fb_copyarea *area)
+{
+}
+
+static inline void drm_fb_helper_sys_imageblit(struct fb_info *info,
+		const struct fb_image *image)
+{
+}
+
+static inline void drm_fb_helper_cfb_fillrect(struct fb_info *info,
+		const struct fb_fillrect *rect)
+{
+}
+
+static inline void drm_fb_helper_cfb_copyarea(struct fb_info *info,
+		const struct fb_copyarea *area)
+{
+}
+
+static inline void drm_fb_helper_cfb_imageblit(struct fb_info *info,
+		const struct fb_image *image)
+{
+}
+
+static inline void drm_fb_helper_set_suspend(struct drm_fb_helper *fb_helper,
+						int state)
+{
+}
+
+static inline int
+drm_fb_helper_remove_conflicting_framebuffers(struct apertures_struct *a,
+						const char *name, bool primary)
+{
+	return 0;
+}
+
+static inline int drm_fb_helper_hotplug_event(struct drm_fb_helper *fb_helper)
+{
+	return 0;
+}
+
+static inline int drm_fb_helper_initial_config(struct drm_fb_helper *fb_helper,
+					       int bpp_sel)
+{
+	return 0;
+}
+
+static inline int
+drm_fb_helper_single_add_all_connectors(struct drm_fb_helper *fb_helper)
+{
+	return 0;
+}
+
+static inline int drm_fb_helper_debug_enter(struct fb_info *info)
+{
+	return 0;
+}
+
+static inline int drm_fb_helper_debug_leave(struct fb_info *info)
+{
+	return 0;
+}
+
+static inline struct drm_display_mode *
+drm_has_preferred_mode(struct drm_fb_helper_connector *fb_connector,
+		       int width, int height)
+{
+	return NULL;
+}
+
+static inline struct drm_display_mode *
+drm_pick_cmdline_mode(struct drm_fb_helper_connector *fb_helper_conn,
+		      int width, int height)
+{
+	return NULL;
+}
+
+static inline int
+drm_fb_helper_add_one_connector(struct drm_fb_helper *fb_helper,
+				struct drm_connector *connector)
+{
+	return 0;
+}
+
+static inline int
+drm_fb_helper_remove_one_connector(struct drm_fb_helper *fb_helper,
+				   struct drm_connector *connector)
+{
+	return 0;
+}
+#endif
 #endif
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[RFC v2 2/6] drm/msm: Remove local fbdev emulation Kconfig option

[Archit Taneja]

DRM_MSM_FBDEV config is used to enable/disable fbdev emulation for the
msm kms driver.

Replace this with the top level DRM_FBDEV_EMULATION config option where
applicable. This also prevents build breaks caused by undefined
drm_fb_helper_* functions when legacy fbdev support was disabled.

Signed-off-by: Archit Taneja <architt@codeaurora.org>
---
 drivers/gpu/drm/msm/Kconfig   | 14 --------------
 drivers/gpu/drm/msm/Makefile  |  2 +-
 drivers/gpu/drm/msm/msm_drv.c |  8 ++------
 3 files changed, 3 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/msm/Kconfig b/drivers/gpu/drm/msm/Kconfig
index 08ba8d0..ebb03fd 100644
--- a/drivers/gpu/drm/msm/Kconfig
+++ b/drivers/gpu/drm/msm/Kconfig
@@ -13,20 +13,6 @@ config DRM_MSM
 	help
 	  DRM/KMS driver for MSM/snapdragon.
 
-config DRM_MSM_FBDEV
-	bool "Enable legacy fbdev support for MSM modesetting driver"
-	depends on DRM_MSM
-	select DRM_KMS_FB_HELPER
-	select FB_SYS_FILLRECT
-	select FB_SYS_COPYAREA
-	select FB_SYS_IMAGEBLIT
-	select FB_SYS_FOPS
-	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 MSM modesetting driver.
-
 config DRM_MSM_REGISTER_LOGGING
 	bool "MSM DRM register logging"
 	depends on DRM_MSM
diff --git a/drivers/gpu/drm/msm/Makefile b/drivers/gpu/drm/msm/Makefile
index 16a81b9..5b08951 100644
--- a/drivers/gpu/drm/msm/Makefile
+++ b/drivers/gpu/drm/msm/Makefile
@@ -49,7 +49,7 @@ msm-y := \
 	msm_rd.o \
 	msm_ringbuffer.o
 
-msm-$(CONFIG_DRM_MSM_FBDEV) += msm_fbdev.o
+msm-$(CONFIG_DRM_FBDEV_EMULATION) += msm_fbdev.o
 msm-$(CONFIG_COMMON_CLK) += mdp/mdp4/mdp4_lvds_pll.o
 
 msm-$(CONFIG_DRM_MSM_DSI) += dsi/dsi.o \
diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
index b7ef56e..8061bd9 100644
--- a/drivers/gpu/drm/msm/msm_drv.c
+++ b/drivers/gpu/drm/msm/msm_drv.c
@@ -21,11 +21,9 @@
 
 static void msm_fb_output_poll_changed(struct drm_device *dev)
 {
-#ifdef CONFIG_DRM_MSM_FBDEV
 	struct msm_drm_private *priv = dev->dev_private;
 	if (priv->fbdev)
 		drm_fb_helper_hotplug_event(priv->fbdev);
-#endif
 }
 
 static const struct drm_mode_config_funcs mode_config_funcs = {
@@ -56,7 +54,7 @@ module_param(reglog, bool, 0600);
 #define reglog 0
 #endif
 
-#ifdef CONFIG_DRM_MSM_FBDEV
+#ifdef CONFIG_DRM_FBDEV_EMULATION
 static bool fbdev = true;
 MODULE_PARM_DESC(fbdev, "Enable fbdev compat layer");
 module_param(fbdev, bool, 0600);
@@ -353,7 +351,7 @@ static int msm_load(struct drm_device *dev, unsigned long flags)
 
 	drm_mode_config_reset(dev);
 
-#ifdef CONFIG_DRM_MSM_FBDEV
+#ifdef CONFIG_DRM_FBDEV_EMULATION
 	if (fbdev)
 		priv->fbdev = msm_fbdev_init(dev);
 #endif
@@ -421,11 +419,9 @@ static void msm_preclose(struct drm_device *dev, struct drm_file *file)
 
 static void msm_lastclose(struct drm_device *dev)
 {
-#ifdef CONFIG_DRM_MSM_FBDEV
 	struct msm_drm_private *priv = dev->dev_private;
 	if (priv->fbdev)
 		drm_fb_helper_restore_fbdev_mode_unlocked(priv->fbdev);
-#endif
 }
 
 static irqreturn_t msm_irq(int irq, void *arg)
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

[RFC v2 3/6] drm/tegra: Remove local fbdev emulation Kconfig option

[Archit Taneja]

DRM_TEGRA_FBDEV config is currently used to enable/disable fbdev emulation
for the tegra 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

The #ifdef in tegra_drm struct that adds/removes the terga_fbdev member
has been removed completely. This helps in calling stub drm fb helper
functions at not much cost.

We could clean up fb.c a bit further to reduce the number of #ifdefs, but
that's left for later.

Signed-off-by: Archit Taneja <architt@codeaurora.org>
---
 drivers/gpu/drm/tegra/Kconfig | 12 ------------
 drivers/gpu/drm/tegra/drm.c   | 15 ++++++++++-----
 drivers/gpu/drm/tegra/drm.h   |  8 --------
 drivers/gpu/drm/tegra/fb.c    | 25 ++++++-------------------
 4 files changed, 16 insertions(+), 44 deletions(-)

diff --git a/drivers/gpu/drm/tegra/Kconfig b/drivers/gpu/drm/tegra/Kconfig
index 74d9d62..63ebb15 100644
--- a/drivers/gpu/drm/tegra/Kconfig
+++ b/drivers/gpu/drm/tegra/Kconfig
@@ -16,18 +16,6 @@ config DRM_TEGRA
 
 if DRM_TEGRA
 
-config DRM_TEGRA_FBDEV
-	bool "Enable legacy fbdev support"
-	select DRM_KMS_FB_HELPER
-	select FB_SYS_FILLRECT
-	select FB_SYS_COPYAREA
-	select FB_SYS_IMAGEBLIT
-	default y
-	help
-	  Choose this option if you have a need for the legacy fbdev support.
-	  Note that this support also provides the Linux console on top of
-	  the Tegra modesetting driver.
-
 config DRM_TEGRA_DEBUG
 	bool "NVIDIA Tegra DRM debug support"
 	help
diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c
index 427f50c..e884e54 100644
--- a/drivers/gpu/drm/tegra/drm.c
+++ b/drivers/gpu/drm/tegra/drm.c
@@ -104,11 +104,17 @@ static int tegra_atomic_commit(struct drm_device *drm,
 	return 0;
 }
 
+static inline void tegra_fb_output_poll_changed(struct drm_device *drm)
+{
+	struct tegra_drm *tegra = drm->dev_private;
+
+	if (tegra->fbdev)
+		drm_fb_helper_hotplug_event(&tegra->fbdev->base);
+}
+
 static const struct drm_mode_config_funcs tegra_drm_mode_funcs = {
 	.fb_create = tegra_fb_create,
-#ifdef CONFIG_DRM_TEGRA_FBDEV
 	.output_poll_changed = tegra_fb_output_poll_changed,
-#endif
 	.atomic_check = drm_atomic_helper_check,
 	.atomic_commit = tegra_atomic_commit,
 };
@@ -259,11 +265,10 @@ static void tegra_drm_context_free(struct tegra_drm_context *context)
 
 static void tegra_drm_lastclose(struct drm_device *drm)
 {
-#ifdef CONFIG_DRM_TEGRA_FBDEV
 	struct tegra_drm *tegra = drm->dev_private;
 
-	tegra_fbdev_restore_mode(tegra->fbdev);
-#endif
+	if (tegra->fbdev)
+		drm_fb_helper_restore_fbdev_mode_unlocked(&tegra->fbdev->base);
 }
 
 static struct host1x_bo *
diff --git a/drivers/gpu/drm/tegra/drm.h b/drivers/gpu/drm/tegra/drm.h
index 659b2fc..25ee5ea 100644
--- a/drivers/gpu/drm/tegra/drm.h
+++ b/drivers/gpu/drm/tegra/drm.h
@@ -29,12 +29,10 @@ struct tegra_fb {
 	unsigned int num_planes;
 };
 
-#ifdef CONFIG_DRM_TEGRA_FBDEV
 struct tegra_fbdev {
 	struct drm_fb_helper base;
 	struct tegra_fb *fb;
 };
-#endif
 
 struct tegra_drm {
 	struct drm_device *drm;
@@ -45,9 +43,7 @@ struct tegra_drm {
 	struct mutex clients_lock;
 	struct list_head clients;
 
-#ifdef CONFIG_DRM_TEGRA_FBDEV
 	struct tegra_fbdev *fbdev;
-#endif
 
 	unsigned int pitch_align;
 
@@ -263,10 +259,6 @@ int tegra_drm_fb_prepare(struct drm_device *drm);
 void tegra_drm_fb_free(struct drm_device *drm);
 int tegra_drm_fb_init(struct drm_device *drm);
 void tegra_drm_fb_exit(struct drm_device *drm);
-#ifdef CONFIG_DRM_TEGRA_FBDEV
-void tegra_fbdev_restore_mode(struct tegra_fbdev *fbdev);
-void tegra_fb_output_poll_changed(struct drm_device *drm);
-#endif
 
 extern struct platform_driver tegra_dc_driver;
 extern struct platform_driver tegra_dsi_driver;
diff --git a/drivers/gpu/drm/tegra/fb.c b/drivers/gpu/drm/tegra/fb.c
index a2d66f9..1fd595c 100644
--- a/drivers/gpu/drm/tegra/fb.c
+++ b/drivers/gpu/drm/tegra/fb.c
@@ -18,7 +18,7 @@ static inline struct tegra_fb *to_tegra_fb(struct drm_framebuffer *fb)
 	return container_of(fb, struct tegra_fb, base);
 }
 
-#ifdef CONFIG_DRM_TEGRA_FBDEV
+#ifdef CONFIG_DRM_FBDEV_EMULATION
 static inline struct tegra_fbdev *to_tegra_fbdev(struct drm_fb_helper *helper)
 {
 	return container_of(helper, struct tegra_fbdev, base);
@@ -181,7 +181,7 @@ unreference:
 	return ERR_PTR(err);
 }
 
-#ifdef CONFIG_DRM_TEGRA_FBDEV
+#ifdef CONFIG_DRM_FBDEV_EMULATION
 static struct fb_ops tegra_fb_ops = {
 	.owner = THIS_MODULE,
 	.fb_fillrect = drm_fb_helper_sys_fillrect,
@@ -355,24 +355,11 @@ static void tegra_fbdev_exit(struct tegra_fbdev *fbdev)
 	tegra_fbdev_free(fbdev);
 }
 
-void tegra_fbdev_restore_mode(struct tegra_fbdev *fbdev)
-{
-	if (fbdev)
-		drm_fb_helper_restore_fbdev_mode_unlocked(&fbdev->base);
-}
-
-void tegra_fb_output_poll_changed(struct drm_device *drm)
-{
-	struct tegra_drm *tegra = drm->dev_private;
-
-	if (tegra->fbdev)
-		drm_fb_helper_hotplug_event(&tegra->fbdev->base);
-}
 #endif
 
 int tegra_drm_fb_prepare(struct drm_device *drm)
 {
-#ifdef CONFIG_DRM_TEGRA_FBDEV
+#ifdef CONFIG_DRM_FBDEV_EMULATION
 	struct tegra_drm *tegra = drm->dev_private;
 
 	tegra->fbdev = tegra_fbdev_create(drm);
@@ -385,7 +372,7 @@ int tegra_drm_fb_prepare(struct drm_device *drm)
 
 void tegra_drm_fb_free(struct drm_device *drm)
 {
-#ifdef CONFIG_DRM_TEGRA_FBDEV
+#ifdef CONFIG_DRM_FBDEV_EMULATION
 	struct tegra_drm *tegra = drm->dev_private;
 
 	tegra_fbdev_free(tegra->fbdev);
@@ -394,7 +381,7 @@ void tegra_drm_fb_free(struct drm_device *drm)
 
 int tegra_drm_fb_init(struct drm_device *drm)
 {
-#ifdef CONFIG_DRM_TEGRA_FBDEV
+#ifdef CONFIG_DRM_FBDEV_EMULATION
 	struct tegra_drm *tegra = drm->dev_private;
 	int err;
 
@@ -409,7 +396,7 @@ int tegra_drm_fb_init(struct drm_device *drm)
 
 void tegra_drm_fb_exit(struct drm_device *drm)
 {
-#ifdef CONFIG_DRM_TEGRA_FBDEV
+#ifdef CONFIG_DRM_FBDEV_EMULATION
 	struct tegra_drm *tegra = drm->dev_private;
 
 	tegra_fbdev_exit(tegra->fbdev);
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

[RFC v2 4/6] drm/imx: Remove local fbdev emulation Kconfig option

[Archit Taneja]

DRM_IMX_FB_HELPER config is currently used to enable/disable fbdev
emulation for the imx kms driver.

Remove this local config option and use the top level DRM_FBDEV_EMULATION
config option where applicable. Using this config lets us also prevent
wrapping around drm_fb_helper_* calls with #ifdefs in certain places.

We replace the #ifdef in imx_drm_driver_load with
CONFIG_DRM_FBDEV_EMULATION. It's probably okay to get remove the #ifdef
itself, but just left it here for now to be safe. It can be removed after
some testing.

Signed-off-by: Archit Taneja <architt@codeaurora.org>
---
 drivers/gpu/drm/imx/Kconfig        |  9 ---------
 drivers/gpu/drm/imx/imx-drm-core.c | 10 +---------
 2 files changed, 1 insertion(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/imx/Kconfig b/drivers/gpu/drm/imx/Kconfig
index 2b81a41..35ca4f0 100644
--- a/drivers/gpu/drm/imx/Kconfig
+++ b/drivers/gpu/drm/imx/Kconfig
@@ -10,15 +10,6 @@ config DRM_IMX
 	help
 	  enable i.MX graphics support
 
-config DRM_IMX_FB_HELPER
-	tristate "provide legacy framebuffer /dev/fb0"
-	select DRM_KMS_CMA_HELPER
-	depends on DRM_IMX
-	help
-	  The DRM framework can provide a legacy /dev/fb0 framebuffer
-	  for your device. This is necessary to get a framebuffer console
-	  and also for applications using the legacy framebuffer API
-
 config DRM_IMX_PARALLEL_DISPLAY
 	tristate "Support for parallel displays"
 	select DRM_PANEL
diff --git a/drivers/gpu/drm/imx/imx-drm-core.c b/drivers/gpu/drm/imx/imx-drm-core.c
index 74f505b..2beae02 100644
--- a/drivers/gpu/drm/imx/imx-drm-core.c
+++ b/drivers/gpu/drm/imx/imx-drm-core.c
@@ -60,26 +60,20 @@ EXPORT_SYMBOL_GPL(imx_drm_crtc_id);
 
 static void imx_drm_driver_lastclose(struct drm_device *drm)
 {
-#if IS_ENABLED(CONFIG_DRM_IMX_FB_HELPER)
 	struct imx_drm_device *imxdrm = drm->dev_private;
 
 	if (imxdrm->fbhelper)
 		drm_fbdev_cma_restore_mode(imxdrm->fbhelper);
-#endif
 }
 
 static int imx_drm_driver_unload(struct drm_device *drm)
 {
-#if IS_ENABLED(CONFIG_DRM_IMX_FB_HELPER)
 	struct imx_drm_device *imxdrm = drm->dev_private;
-#endif
 
 	drm_kms_helper_poll_fini(drm);
 
-#if IS_ENABLED(CONFIG_DRM_IMX_FB_HELPER)
 	if (imxdrm->fbhelper)
 		drm_fbdev_cma_fini(imxdrm->fbhelper);
-#endif
 
 	component_unbind_all(drm->dev, drm);
 
@@ -215,11 +209,9 @@ EXPORT_SYMBOL_GPL(imx_drm_encoder_destroy);
 
 static void imx_drm_output_poll_changed(struct drm_device *drm)
 {
-#if IS_ENABLED(CONFIG_DRM_IMX_FB_HELPER)
 	struct imx_drm_device *imxdrm = drm->dev_private;
 
 	drm_fbdev_cma_hotplug_event(imxdrm->fbhelper);
-#endif
 }
 
 static struct drm_mode_config_funcs imx_drm_mode_config_funcs = {
@@ -308,7 +300,7 @@ static int imx_drm_driver_load(struct drm_device *drm, unsigned long flags)
 	 * The fb helper takes copies of key hardware information, so the
 	 * crtcs/connectors/encoders must not change after this point.
 	 */
-#if IS_ENABLED(CONFIG_DRM_IMX_FB_HELPER)
+#ifdef CONFIG_DRM_FBDEV_EMULATION
 	if (legacyfb_depth != 16 && legacyfb_depth != 32) {
 		dev_warn(drm->dev, "Invalid legacyfb_depth.  Defaulting to 16bpp\n");
 		legacyfb_depth = 16;
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

[RFC v2 5/6] drm/sti: Remove local fbdev emulation Kconfig option

[Archit Taneja]

DRM_STI_FBDEV config is currently used to enable/disable fbdev emulation
for the sti kms driver.

Remove this local config option and use the top level DRM_FBDEV_EMULATION
config option instead where applicable.

We replace the #ifdef in sti_drm_load with CONFIG_DRM_FBDEV_EMULATION.
It's probably okay to get remove the #ifdef itself, but just left it here
for now to be safe. It can be removed after some testing.

Signed-off-by: Archit Taneja <architt@codeaurora.org>
---
 drivers/gpu/drm/sti/Kconfig       | 6 ------
 drivers/gpu/drm/sti/sti_drm_drv.c | 2 +-
 2 files changed, 1 insertion(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/sti/Kconfig b/drivers/gpu/drm/sti/Kconfig
index fbccc10..e3aa5af 100644
--- a/drivers/gpu/drm/sti/Kconfig
+++ b/drivers/gpu/drm/sti/Kconfig
@@ -9,9 +9,3 @@ config DRM_STI
 	select FW_LOADER_USER_HELPER_FALLBACK
 	help
 	  Choose this option to enable DRM on STM stiH41x chipset
-
-config DRM_STI_FBDEV
-	bool "DRM frame buffer device for STMicroelectronics SoC stiH41x Serie"
-	depends on DRM_STI
-	help
-	  Choose this option to enable FBDEV on top of DRM for STM stiH41x chipset
diff --git a/drivers/gpu/drm/sti/sti_drm_drv.c b/drivers/gpu/drm/sti/sti_drm_drv.c
index 59d558b..36493eb 100644
--- a/drivers/gpu/drm/sti/sti_drm_drv.c
+++ b/drivers/gpu/drm/sti/sti_drm_drv.c
@@ -160,7 +160,7 @@ static int sti_drm_load(struct drm_device *dev, unsigned long flags)
 
 	drm_mode_config_reset(dev);
 
-#ifdef CONFIG_DRM_STI_FBDEV
+#ifdef CONFIG_DRM_FBDEV_EMULATION
 	drm_fbdev_cma_init(dev, 32,
 		   dev->mode_config.num_crtc,
 		   dev->mode_config.num_connector);
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

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

[Archit Taneja]

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

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 b7ddf48..355e43f 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 82bbe3f..bcce317 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1849,7 +1849,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 542fac6..ab5881c 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1798,11 +1798,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 1b61f98..0acd801 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9886,7 +9886,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;
@@ -9909,9 +9908,6 @@ mode_fits_in_fbdev(struct drm_device *dev,
 		return NULL;
 
 	return fb;
-#else
-	return NULL;
-#endif
 }
 
 static int intel_modeset_setup_plane_state(struct drm_atomic_state *state,
@@ -14296,11 +14292,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 6e4cc53..66eaf6f 100644
--- a/drivers/gpu/drm/i915/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/intel_dp_mst.c
@@ -395,18 +395,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 1059283..371b201 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1207,12 +1207,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 4aa21f7..445c35f 100644
--- a/drivers/gpu/drm/i915/intel_fbdev.c
+++ b/drivers/gpu/drm/i915/intel_fbdev.c
@@ -798,13 +798,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

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

[Daniel Vetter]

On Mon, Jul 13, 2015 at 12:12:06PM +0530, Archit Taneja wrote:
> Legacy fbdev emulation support via DRM is achieved through KMS FB helpers.
> Most modesetting drivers enable provide fbdev emulation by default by
> selecting KMS FB helpers. A few provide a separate Kconfig option for the
> user to enable or disbale fbdev emulation.
> 
> Enabling fbdev emulation is finally a distro-level decision. Having a top
> level Kconfig option for fbdev emulation helps by providing a uniform way
> to enable/disable fbdev emulation for any modesetting driver. It also lets
> us remove unnecessary driver specific Kconfig options that causes bloat.
> 
> With a top level Kconfig in place, we can stub out the fb helper functions
> when not needed without breaking functionality. Having stub functions also
> prevents drivers to require wrapping fb helper function calls with #ifdefs.
> 
> DRM_FBDEV_EMULATION defaults to y since many drivers enable fbdev
> emulation by default and majority of distributions expect the fbdev
> interface in the kernel.
> 
> Signed-off-by: Archit Taneja <architt@codeaurora.org>
> ---
>  drivers/gpu/drm/Kconfig     |  12 +++
>  drivers/gpu/drm/Makefile    |   2 +-
>  include/drm/drm_fb_helper.h | 192 ++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 205 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> index e373f8a..8fd670b 100644
> --- a/drivers/gpu/drm/Kconfig
> +++ b/drivers/gpu/drm/Kconfig
> @@ -47,6 +47,18 @@ 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
> +	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. You'd need this if
> +	  you're looking for console support too.

Maybe add an "If unsure say Y." here.

> +
>  config DRM_LOAD_EDID_FIRMWARE
>  	bool "Allow to specify an EDID data set instead of probing for it"
>  	depends on DRM_KMS_HELPER
> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> index 5713d05..8858510 100644
> --- a/drivers/gpu/drm/Makefile
> +++ b/drivers/gpu/drm/Makefile
> @@ -23,7 +23,7 @@ drm-$(CONFIG_OF) += drm_of.o
>  drm_kms_helper-y := drm_crtc_helper.o drm_dp_helper.o drm_probe_helper.o \
>  		drm_plane_helper.o drm_dp_mst_topology.o drm_atomic_helper.o
>  drm_kms_helper-$(CONFIG_DRM_LOAD_EDID_FIRMWARE) += drm_edid_load.o
> -drm_kms_helper-$(CONFIG_DRM_KMS_FB_HELPER) += drm_fb_helper.o
> +drm_kms_helper-$(CONFIG_DRM_FBDEV_EMULATION) += drm_fb_helper.o
>  drm_kms_helper-$(CONFIG_DRM_KMS_CMA_HELPER) += drm_fb_cma_helper.o
>  
>  obj-$(CONFIG_DRM_KMS_HELPER) += drm_kms_helper.o
> diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h
> index cc19e88..8f972da 100644
> --- a/include/drm/drm_fb_helper.h
> +++ b/include/drm/drm_fb_helper.h
> @@ -122,6 +122,7 @@ struct drm_fb_helper {
>  	bool delayed_hotplug;
>  };
>  
> +#ifdef CONFIG_DRM_FBDEV_EMULATION
>  void drm_fb_helper_prepare(struct drm_device *dev, struct drm_fb_helper *helper,
>  			   const struct drm_fb_helper_funcs *funcs);
>  int drm_fb_helper_init(struct drm_device *dev,
> @@ -188,4 +189,195 @@ drm_pick_cmdline_mode(struct drm_fb_helper_connector *fb_helper_conn,
>  int drm_fb_helper_add_one_connector(struct drm_fb_helper *fb_helper, struct drm_connector *connector);
>  int drm_fb_helper_remove_one_connector(struct drm_fb_helper *fb_helper,
>  				       struct drm_connector *connector);
> +#else
> +static inline void drm_fb_helper_prepare(struct drm_device *dev,
> +					struct drm_fb_helper *helper,
> +					const struct drm_fb_helper_funcs *funcs)
> +{
> +}
> +
> +static inline int drm_fb_helper_init(struct drm_device *dev,
> +		       struct drm_fb_helper *helper, int crtc_count,
> +		       int max_conn)
> +{
> +	return 0;
> +}
> +
> +static inline void drm_fb_helper_fini(struct drm_fb_helper *helper)
> +{
> +}
> +
> +static inline int drm_fb_helper_blank(int blank, struct fb_info *info)
> +{
> +	return 0;
> +}
> +
> +static inline int drm_fb_helper_pan_display(struct fb_var_screeninfo *var,
> +					    struct fb_info *info)
> +{
> +	return 0;
> +}
> +
> +static inline int drm_fb_helper_set_par(struct fb_info *info)
> +{
> +	return 0;
> +}
> +
> +static inline int drm_fb_helper_check_var(struct fb_var_screeninfo *var,
> +					  struct fb_info *info)
> +{
> +	return 0;
> +}
> +
> +static inline bool
> +drm_fb_helper_restore_fbdev_mode_unlocked(struct drm_fb_helper *fb_helper)
> +{
> +	return true;
> +}
> +
> +static inline struct fb_info *
> +drm_fb_helper_alloc_fbi(struct drm_fb_helper *fb_helper)
> +{
> +	return NULL;
> +}
> +
> +static inline void drm_fb_helper_unregister_fbi(struct drm_fb_helper *fb_helper)
> +{
> +}
> +static inline void drm_fb_helper_release_fbi(struct drm_fb_helper *fb_helper)
> +{
> +}
> +
> +static inline void drm_fb_helper_fill_var(struct fb_info *info,
> +					  struct drm_fb_helper *fb_helper,
> +					  uint32_t fb_width, uint32_t fb_height)
> +{
> +}
> +
> +static inline void drm_fb_helper_fill_fix(struct fb_info *info, uint32_t pitch,
> +					  uint32_t depth)
> +{
> +}
> +
> +static inline int drm_fb_helper_setcmap(struct fb_cmap *cmap,
> +					struct fb_info *info)
> +{
> +	return 0;
> +}
> +
> +static inline void drm_fb_helper_unlink_fbi(struct drm_fb_helper *fb_helper)
> +{
> +}
> +
> +static inline ssize_t drm_fb_helper_sys_read(struct fb_info *info,
> +					char __user *buf, size_t count,
> +					loff_t *ppos)
> +{
> +	return -ENODEV;
> +}
> +
> +static inline ssize_t drm_fb_helper_sys_write(struct fb_info *info,
> +					const char __user *buf, size_t count,
> +					loff_t *ppos)
> +{
> +	return -ENODEV;
> +}
> +
> +static inline void drm_fb_helper_sys_fillrect(struct fb_info *info,
> +		const struct fb_fillrect *rect)
> +{
> +}
> +
> +static inline void drm_fb_helper_sys_copyarea(struct fb_info *info,
> +		const struct fb_copyarea *area)
> +{
> +}
> +
> +static inline void drm_fb_helper_sys_imageblit(struct fb_info *info,
> +		const struct fb_image *image)
> +{
> +}
> +
> +static inline void drm_fb_helper_cfb_fillrect(struct fb_info *info,
> +		const struct fb_fillrect *rect)
> +{
> +}
> +
> +static inline void drm_fb_helper_cfb_copyarea(struct fb_info *info,
> +		const struct fb_copyarea *area)
> +{
> +}
> +
> +static inline void drm_fb_helper_cfb_imageblit(struct fb_info *info,
> +		const struct fb_image *image)
> +{
> +}
> +
> +static inline void drm_fb_helper_set_suspend(struct drm_fb_helper *fb_helper,
> +						int state)
> +{
> +}
> +
> +static inline int
> +drm_fb_helper_remove_conflicting_framebuffers(struct apertures_struct *a,
> +						const char *name, bool primary)
> +{
> +	return 0;
> +}

The above dummy functions don't exist yet - is this part of a more
extensive patch series to introduce those as drm fb helpers too? Should
probably be dropped for now.
-Daniel

> +
> +static inline int drm_fb_helper_hotplug_event(struct drm_fb_helper *fb_helper)
> +{
> +	return 0;
> +}
> +
> +static inline int drm_fb_helper_initial_config(struct drm_fb_helper *fb_helper,
> +					       int bpp_sel)
> +{
> +	return 0;
> +}
> +
> +static inline int
> +drm_fb_helper_single_add_all_connectors(struct drm_fb_helper *fb_helper)
> +{
> +	return 0;
> +}
> +
> +static inline int drm_fb_helper_debug_enter(struct fb_info *info)
> +{
> +	return 0;
> +}
> +
> +static inline int drm_fb_helper_debug_leave(struct fb_info *info)
> +{
> +	return 0;
> +}
> +
> +static inline struct drm_display_mode *
> +drm_has_preferred_mode(struct drm_fb_helper_connector *fb_connector,
> +		       int width, int height)
> +{
> +	return NULL;
> +}
> +
> +static inline struct drm_display_mode *
> +drm_pick_cmdline_mode(struct drm_fb_helper_connector *fb_helper_conn,
> +		      int width, int height)
> +{
> +	return NULL;
> +}
> +
> +static inline int
> +drm_fb_helper_add_one_connector(struct drm_fb_helper *fb_helper,
> +				struct drm_connector *connector)
> +{
> +	return 0;
> +}
> +
> +static inline int
> +drm_fb_helper_remove_one_connector(struct drm_fb_helper *fb_helper,
> +				   struct drm_connector *connector)
> +{
> +	return 0;
> +}
> +#endif
>  #endif
> -- 
> 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

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

[Daniel Vetter]

On Mon, Jul 13, 2015 at 12:12:11PM +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
> 
> 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

Hm, your new generic fbdev emulation doesn't select the FB helpers we need
here, which probably breaks compilation. I think we just need to add all
these selects to the new core fbev emulation config - most drivers want
them anyway. And even for the drivers that have their own drawing routines
(udl and qxl afaik) the only thing they do is wrap calls to the ->dirty
callback around the cfb helpers. And we probably want to move this to the
fbdev helper library eventually anyway.

> -	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 b7ddf48..355e43f 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 82bbe3f..bcce317 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -1849,7 +1849,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 542fac6..ab5881c 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1798,11 +1798,9 @@ struct drm_i915_private {
>  
>  	struct drm_i915_gem_object *vlv_pctx;
>  
> -#ifdef CONFIG_DRM_I915_FBDEV

Hm for i915 I'd prefer to just replace one Kconfig symbol with the new one
and leave all the #ifdefs around. We try hard to remove all the fbdev
related code completely and that won't be the case when reintroducing some
of the runtime checks like you do. We could do something like

#ifdef CONFIG_DRM_FBDEV_EMULATION
#define HAS_FBDEV(dev_priv) 0
#else
#define HAS_FBDEV(dev_priv) ((dev_priv)->fbdev)
#endif

though. But really doing that is not your job ;-)

Cheers, Daniel

>  	/* 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 1b61f98..0acd801 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -9886,7 +9886,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;
> @@ -9909,9 +9908,6 @@ mode_fits_in_fbdev(struct drm_device *dev,
>  		return NULL;
>  
>  	return fb;
> -#else
> -	return NULL;
> -#endif
>  }
>  
>  static int intel_modeset_setup_plane_state(struct drm_atomic_state *state,
> @@ -14296,11 +14292,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 6e4cc53..66eaf6f 100644
> --- a/drivers/gpu/drm/i915/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/intel_dp_mst.c
> @@ -395,18 +395,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 1059283..371b201 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1207,12 +1207,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 4aa21f7..445c35f 100644
> --- a/drivers/gpu/drm/i915/intel_fbdev.c
> +++ b/drivers/gpu/drm/i915/intel_fbdev.c
> @@ -798,13 +798,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
http://blog.ffwll.ch

Re: [RFC v2 0/6] drm: fb emulation: Step 2: Create a fbdev emulation config option

[Daniel Vetter]

On Mon, Jul 13, 2015 at 12:12:05PM +0530, Archit Taneja wrote:
> This provides a uniform interface to enable/disable legacy fbdev support
> for modesetting drivers.
> 
> DRM_FBDEV_EMULATION is made available in the top level drm Kconfig. A
> driver that wants fbdev emulation using drm_fb_helper can enable this
> option.
> 
> Apart from 5 drivers (msm, tegra, imx, sti, i915), all enable fbdev
> emulation by default. For these drivers, if we explicitly disable fbdev
> emulation, the 'hope' is that we will cleanly bail out with an error. We
> set DRM_FBDEV_EMULATION to 'y' by default, since most drivers rely on it.
> 
> 
> Archit Taneja (6):
>   drm: Add top level Kconfig option for DRM fbdev emulation
>   drm/msm: Remove local fbdev emulation Kconfig option
>   drm/tegra: Remove local fbdev emulation Kconfig option
>   drm/imx: Remove local fbdev emulation Kconfig option
>   drm/sti: Remove local fbdev emulation Kconfig option
>   drm/i915: Remove local fbdev emulation Kconfig option

Ah I guess you can disregard some of my comments here. Since this was
in-reply-to the old thread but step 1 wasn't I've read the patches
out-of-order ;-)
-Daniel

> 
>  drivers/gpu/drm/Kconfig              |  12 +++
>  drivers/gpu/drm/Makefile             |   2 +-
>  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 --
>  drivers/gpu/drm/imx/Kconfig          |   9 --
>  drivers/gpu/drm/imx/imx-drm-core.c   |  10 +-
>  drivers/gpu/drm/msm/Kconfig          |  14 ---
>  drivers/gpu/drm/msm/Makefile         |   2 +-
>  drivers/gpu/drm/msm/msm_drv.c        |   8 +-
>  drivers/gpu/drm/sti/Kconfig          |   6 --
>  drivers/gpu/drm/sti/sti_drm_drv.c    |   2 +-
>  drivers/gpu/drm/tegra/Kconfig        |  12 ---
>  drivers/gpu/drm/tegra/drm.c          |  15 ++-
>  drivers/gpu/drm/tegra/drm.h          |   8 --
>  drivers/gpu/drm/tegra/fb.c           |  25 ++---
>  include/drm/drm_fb_helper.h          | 192 +++++++++++++++++++++++++++++++++++
>  22 files changed, 242 insertions(+), 132 deletions(-)
> 
> -- 
> 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

[PATCH v3] drm: Add top level Kconfig option for DRM fbdev emulation

[Archit Taneja]

Legacy fbdev emulation support via DRM is achieved through KMS FB helpers.
Most modesetting drivers enable provide fbdev emulation by default by
selecting KMS FB helpers. A few provide a separate Kconfig option for the
user to enable or disbale fbdev emulation.

Enabling fbdev emulation is finally a distro-level decision. Having a top
level Kconfig option for fbdev emulation helps by providing a uniform way
to enable/disable fbdev emulation for any modesetting driver. It also lets
us remove unnecessary driver specific Kconfig options that causes bloat.

With a top level Kconfig in place, we can stub out the fb helper functions
when not needed without breaking functionality. Having stub functions also
prevents drivers to require wrapping fb helper function calls with #ifdefs.

DRM_FBDEV_EMULATION defaults to y since many drivers enable fbdev
emulation by default and majority of distributions expect the fbdev
interface in the kernel.

Signed-off-by: Archit Taneja <architt@codeaurora.org>
---
v3:
- Add line in Kconfig "If in doubt, say Y".
- Dropped the rest of the patches in the series below, they will go in after
  acks for each driver.

http://www.spinics.net/lists/dri-devel/msg86084.html

 drivers/gpu/drm/Kconfig     |  13 ++++
 drivers/gpu/drm/Makefile    |   2 +-
 include/drm/drm_fb_helper.h | 185 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 199 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index 35a8c0b..06ae500 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -47,6 +47,19 @@ 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
+	default y
+	help
+	  Choose this option if you have a need for the legacy fbdev
+	  support. Note that this support also provides the linux console
+	  support on top of your modesetting driver.
+
+	  If in doubt, say "Y".
+
 config DRM_LOAD_EDID_FIRMWARE
 	bool "Allow to specify an EDID data set instead of probing for it"
 	depends on DRM_KMS_HELPER
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index 5713d05..8858510 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -23,7 +23,7 @@ drm-$(CONFIG_OF) += drm_of.o
 drm_kms_helper-y := drm_crtc_helper.o drm_dp_helper.o drm_probe_helper.o \
 		drm_plane_helper.o drm_dp_mst_topology.o drm_atomic_helper.o
 drm_kms_helper-$(CONFIG_DRM_LOAD_EDID_FIRMWARE) += drm_edid_load.o
-drm_kms_helper-$(CONFIG_DRM_KMS_FB_HELPER) += drm_fb_helper.o
+drm_kms_helper-$(CONFIG_DRM_FBDEV_EMULATION) += drm_fb_helper.o
 drm_kms_helper-$(CONFIG_DRM_KMS_CMA_HELPER) += drm_fb_cma_helper.o
 
 obj-$(CONFIG_DRM_KMS_HELPER) += drm_kms_helper.o
diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h
index ef32500..dbab462 100644
--- a/include/drm/drm_fb_helper.h
+++ b/include/drm/drm_fb_helper.h
@@ -122,6 +122,7 @@ struct drm_fb_helper {
 	bool delayed_hotplug;
 };
 
+#ifdef CONFIG_DRM_FBDEV_EMULATION
 void drm_fb_helper_prepare(struct drm_device *dev, struct drm_fb_helper *helper,
 			   const struct drm_fb_helper_funcs *funcs);
 int drm_fb_helper_init(struct drm_device *dev,
@@ -185,4 +186,188 @@ drm_pick_cmdline_mode(struct drm_fb_helper_connector *fb_helper_conn,
 int drm_fb_helper_add_one_connector(struct drm_fb_helper *fb_helper, struct drm_connector *connector);
 int drm_fb_helper_remove_one_connector(struct drm_fb_helper *fb_helper,
 				       struct drm_connector *connector);
+#else
+static inline void drm_fb_helper_prepare(struct drm_device *dev,
+					struct drm_fb_helper *helper,
+					const struct drm_fb_helper_funcs *funcs)
+{
+}
+
+static inline int drm_fb_helper_init(struct drm_device *dev,
+		       struct drm_fb_helper *helper, int crtc_count,
+		       int max_conn)
+{
+	return 0;
+}
+
+static inline void drm_fb_helper_fini(struct drm_fb_helper *helper)
+{
+}
+
+static inline int drm_fb_helper_blank(int blank, struct fb_info *info)
+{
+	return 0;
+}
+
+static inline int drm_fb_helper_pan_display(struct fb_var_screeninfo *var,
+					    struct fb_info *info)
+{
+	return 0;
+}
+
+static inline int drm_fb_helper_set_par(struct fb_info *info)
+{
+	return 0;
+}
+
+static inline int drm_fb_helper_check_var(struct fb_var_screeninfo *var,
+					  struct fb_info *info)
+{
+	return 0;
+}
+
+static inline bool
+drm_fb_helper_restore_fbdev_mode_unlocked(struct drm_fb_helper *fb_helper)
+{
+	return true;
+}
+
+static inline struct fb_info *
+drm_fb_helper_alloc_fbi(struct drm_fb_helper *fb_helper)
+{
+	return NULL;
+}
+
+static inline void drm_fb_helper_unregister_fbi(struct drm_fb_helper *fb_helper)
+{
+}
+static inline void drm_fb_helper_release_fbi(struct drm_fb_helper *fb_helper)
+{
+}
+
+static inline void drm_fb_helper_fill_var(struct fb_info *info,
+					  struct drm_fb_helper *fb_helper,
+					  uint32_t fb_width, uint32_t fb_height)
+{
+}
+
+static inline void drm_fb_helper_fill_fix(struct fb_info *info, uint32_t pitch,
+					  uint32_t depth)
+{
+}
+
+static inline int drm_fb_helper_setcmap(struct fb_cmap *cmap,
+					struct fb_info *info)
+{
+	return 0;
+}
+
+static inline void drm_fb_helper_unlink_fbi(struct drm_fb_helper *fb_helper)
+{
+}
+
+static inline ssize_t drm_fb_helper_sys_read(struct fb_info *info,
+					     char __user *buf, size_t count,
+					     loff_t *ppos)
+{
+	return -ENODEV;
+}
+
+static inline ssize_t drm_fb_helper_sys_write(struct fb_info *info,
+					      const char __user *buf,
+					      size_t count, loff_t *ppos)
+{
+	return -ENODEV;
+}
+
+static inline void drm_fb_helper_sys_fillrect(struct fb_info *info,
+					      const struct fb_fillrect *rect)
+{
+}
+
+static inline void drm_fb_helper_sys_copyarea(struct fb_info *info,
+					      const struct fb_copyarea *area)
+{
+}
+
+static inline void drm_fb_helper_sys_imageblit(struct fb_info *info,
+					       const struct fb_image *image)
+{
+}
+
+static inline void drm_fb_helper_cfb_fillrect(struct fb_info *info,
+					      const struct fb_fillrect *rect)
+{
+}
+
+static inline void drm_fb_helper_cfb_copyarea(struct fb_info *info,
+					      const struct fb_copyarea *area)
+{
+}
+
+static inline void drm_fb_helper_cfb_imageblit(struct fb_info *info,
+					       const struct fb_image *image)
+{
+}
+
+static inline void drm_fb_helper_set_suspend(struct drm_fb_helper *fb_helper,
+					     int state)
+{
+}
+
+static inline int drm_fb_helper_hotplug_event(struct drm_fb_helper *fb_helper)
+{
+	return 0;
+}
+
+static inline int drm_fb_helper_initial_config(struct drm_fb_helper *fb_helper,
+					       int bpp_sel)
+{
+	return 0;
+}
+
+static inline int
+drm_fb_helper_single_add_all_connectors(struct drm_fb_helper *fb_helper)
+{
+	return 0;
+}
+
+static inline int drm_fb_helper_debug_enter(struct fb_info *info)
+{
+	return 0;
+}
+
+static inline int drm_fb_helper_debug_leave(struct fb_info *info)
+{
+	return 0;
+}
+
+static inline struct drm_display_mode *
+drm_has_preferred_mode(struct drm_fb_helper_connector *fb_connector,
+		       int width, int height)
+{
+	return NULL;
+}
+
+static inline struct drm_display_mode *
+drm_pick_cmdline_mode(struct drm_fb_helper_connector *fb_helper_conn,
+		      int width, int height)
+{
+	return NULL;
+}
+
+static inline int
+drm_fb_helper_add_one_connector(struct drm_fb_helper *fb_helper,
+				struct drm_connector *connector)
+{
+	return 0;
+}
+
+static inline int
+drm_fb_helper_remove_one_connector(struct drm_fb_helper *fb_helper,
+				   struct drm_connector *connector)
+{
+	return 0;
+}
+#endif
 #endif
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation