Re: [PATCH] drm/i915: Kick out vga console

[Jani Nikula <jani.nikula@linux.intel.com>]

On Wed, 04 Jun 2014, David Herrmann <dh.herrmann@gmail.com> wrote:
> Hi
>
> On Wed, Jun 4, 2014 at 12:57 AM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
>> From: Chris Wilson <chris@chris-wilson.co.uk>
>>
>> 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 <dh.herrmann@gmail.com>
>> Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
>> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
>> Cc: linux-fbdev@vger.kernel.org
>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> (v1)
>> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>> ---
>>  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 <linux/pci.h>
>> +#include <linux/console.h>
>> +#include <linux/vt.h>
>>  #include <linux/vgaarb.h>
>>  #include <linux/acpi.h>
>>  #include <linux/pnp.h>
>> @@ -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