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

From: Archit Taneja
Date: Tue Mar 10 2015 - 06:11:14 EST




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@xxxxxxxxxxxxxx>
---
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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/