From mboxrd@z Thu Jan 1 00:00:00 1970 From: David Herrmann Date: Wed, 04 Jun 2014 13:18:28 +0000 Subject: Re: [PATCH] drm/i915: Kick out vga console Message-Id: List-Id: References: <1401836249-4922-1-git-send-email-daniel.vetter@ffwll.ch> <87sinkeuiu.fsf@intel.com> In-Reply-To: <87sinkeuiu.fsf@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Jani Nikula Cc: "linux-fbdev@vger.kernel.org" , Daniel Vetter , Intel Graphics Development , DRI Development , Tomi Valkeinen , Jean-Christophe Plagniol-Villard Hi On Wed, Jun 4, 2014 at 2:20 PM, Jani Nikula wrote: > On Wed, 04 Jun 2014, David Herrmann wrote: >> You rely on compiler-optimizations here. "dummy_con" is not available >> if !CONFIG_DUMMY_CONSOLE, but you use it. This causes linker-failure >> if dead-code elimination is not done (-O0). > > Nested #ifs too. How about > > #if !defined(CONFIG_VGA_CONSOLE) > static inline int i915_kick_out_vgacon(struct drm_i915_private *dev_priv) > { > return 0; > } > #elif !defined(CONFIG_DUMMY_CONSOLE) > static inline int i915_kick_out_vgacon(struct drm_i915_private *dev_priv) > { > return -ENODEV; > } > #else > static int i915_kick_out_vgacon(struct drm_i915_private *dev_priv) > { > /* ... */ > } > #endif > > in proper kernel style? Or even shorter: #if defined(CONFIG_VGA_CONSOLE) && defined(CONFIG_DUMMY_CONSOLE) static inline int i915_kick_out_vgacon(struct drm_i915_private *dev_priv) { ... } #else static inline int i915_kick_out_vgacon(struct drm_i915_private *dev_priv) { return IS_ENABLED(CONFIG_VGA_CONSOLE) ? -ENODEV : 0; } #endif Thanks David On Wed, Jun 4, 2014 at 2:20 PM, Jani Nikula wrote: > On Wed, 04 Jun 2014, David Herrmann wrote: >> Hi >> >> On Wed, Jun 4, 2014 at 12:57 AM, Daniel Vetter wrote: >>> From: Chris Wilson >>> >>> Touching the VGA resources on an IVB EFI machine causes hard hangs when >>> we then kick out the efifb. Ouch. >>> >>> Apparently this also prevents unclaimed register errors on hsw and >>> hard machine hangs on my i855gm when trying to unbind fbcon. >>> >>> Also, we want this to make I915_FBDEV=n safe. >>> >>> v2: Rebase and pimp commit message. >>> >>> v3: We also need to unregister the vga console, otherwise the unbind >>> of the fb console before module unload might resurrect it again. >>> >>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?idg813 >>> Cc: David Herrmann >>> Cc: Jean-Christophe Plagniol-Villard >>> Cc: Tomi Valkeinen >>> Cc: linux-fbdev@vger.kernel.org >>> Signed-off-by: Chris Wilson (v1) >>> Signed-off-by: Daniel Vetter >>> --- >>> drivers/gpu/drm/i915/i915_dma.c | 34 +++++++++++++++++++++++++++++++++- >>> drivers/video/console/dummycon.c | 1 + >>> drivers/video/console/vgacon.c | 1 + >>> 3 files changed, 35 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c >>> index b9159ade5e85..a4df80740b37 100644 >>> --- a/drivers/gpu/drm/i915/i915_dma.c >>> +++ b/drivers/gpu/drm/i915/i915_dma.c >>> @@ -36,6 +36,8 @@ >>> #include "i915_drv.h" >>> #include "i915_trace.h" >>> #include >>> +#include >>> +#include >>> #include >>> #include >>> #include >>> @@ -1450,6 +1452,29 @@ static void i915_kick_out_firmware_fb(struct drm_i915_private *dev_priv) >>> } >>> #endif >>> >>> +static int i915_kick_out_vgacon(struct drm_i915_private *dev_priv) >>> +{ >>> +#if !defined(CONFIG_VGA_CONSOLE) >>> + return 0; >>> +#else >>> + int ret; >>> + >>> +#if !defined(CONFIG_DUMMY_CONSOLE) >>> + return -ENODEV; >>> +#endif >>> + >>> + DRM_INFO("Replacing VGA console driver\n"); >>> + >>> + console_lock(); >>> + ret = do_take_over_console(&dummy_con, 0, MAX_NR_CONSOLES - 1, 1); >> >> You rely on compiler-optimizations here. "dummy_con" is not available >> if !CONFIG_DUMMY_CONSOLE, but you use it. This causes linker-failure >> if dead-code elimination is not done (-O0). > > Nested #ifs too. How about > > #if !defined(CONFIG_VGA_CONSOLE) > static inline int i915_kick_out_vgacon(struct drm_i915_private *dev_priv) > { > return 0; > } > #elif !defined(CONFIG_DUMMY_CONSOLE) > static inline int i915_kick_out_vgacon(struct drm_i915_private *dev_priv) > { > return -ENODEV; > } > #else > static int i915_kick_out_vgacon(struct drm_i915_private *dev_priv) > { > /* ... */ > } > #endif > > in proper kernel style? > > BR, > Jani. > > >> >> Thanks >> David >> >>> + if (ret = 0) >>> + ret = do_unregister_con_driver(&vga_con); >>> + console_unlock(); >>> + >>> + return ret; >>> +#endif >>> +} >>> + >>> static void i915_dump_device_info(struct drm_i915_private *dev_priv) >>> { >>> const struct intel_device_info *info = &dev_priv->info; >>> @@ -1623,8 +1648,15 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags) >>> if (ret) >>> goto out_regs; >>> >>> - if (drm_core_check_feature(dev, DRIVER_MODESET)) >>> + if (drm_core_check_feature(dev, DRIVER_MODESET)) { >>> + ret = i915_kick_out_vgacon(dev_priv); >>> + if (ret) { >>> + DRM_ERROR("failed to remove conflicting VGA console\n"); >>> + goto out_gtt; >>> + } >>> + >>> i915_kick_out_firmware_fb(dev_priv); >>> + } >>> >>> pci_set_master(dev->pdev); >>> >>> diff --git a/drivers/video/console/dummycon.c b/drivers/video/console/dummycon.c >>> index b63860f7beab..40bec8d64b0a 100644 >>> --- a/drivers/video/console/dummycon.c >>> +++ b/drivers/video/console/dummycon.c >>> @@ -77,3 +77,4 @@ const struct consw dummy_con = { >>> .con_set_palette = DUMMY, >>> .con_scrolldelta = DUMMY, >>> }; >>> +EXPORT_SYMBOL_GPL(dummy_con); >>> diff --git a/drivers/video/console/vgacon.c b/drivers/video/console/vgacon.c >>> index 9d8feac67637..84acd6223dc5 100644 >>> --- a/drivers/video/console/vgacon.c >>> +++ b/drivers/video/console/vgacon.c >>> @@ -1440,5 +1440,6 @@ const struct consw vga_con = { >>> .con_build_attr = vgacon_build_attr, >>> .con_invert_region = vgacon_invert_region, >>> }; >>> +EXPORT_SYMBOL(vga_con); >>> >>> MODULE_LICENSE("GPL"); >>> -- >>> 1.8.1.4 >>> >> _______________________________________________ >> dri-devel mailing list >> dri-devel@lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/dri-devel > > -- > Jani Nikula, Intel Open Source Technology Center