Hi
On Wed, Jun 4, 2014 at 2:20 PM, Jani Nikula <jani.nikula@xxxxxxxxxxxxxxx> wrote:
> On Wed, 04 Jun 2014, David Herrmann <dh.herrmann@xxxxxxxxx> 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 <jani.nikula@xxxxxxxxxxxxxxx> wrote:
> On Wed, 04 Jun 2014, David Herrmann <dh.herrmann@xxxxxxxxx> wrote:
>> Hi
>>
>> On Wed, Jun 4, 2014 at 12:57 AM, Daniel Vetter <daniel.vetter@xxxxxxxx> wrote:
>>> From: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
>>>
>>> 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?id=67813
>>> Cc: David Herrmann <dh.herrmann@xxxxxxxxx>
>>> Cc: Jean-Christophe Plagniol-Villard <plagnioj@xxxxxxxxxxxx>
>>> Cc: Tomi Valkeinen <tomi.valkeinen@xxxxxx>
>>> Cc: linux-fbdev@xxxxxxxxxxxxxxx
>>> Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> (v1)
>>> Signed-off-by: Daniel Vetter <daniel.vetter@xxxxxxxx>
>>> ---
>>> 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@xxxxxxxxxxxxxxxxxxxxx
>> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>
> --
> Jani Nikula, Intel Open Source Technology Center
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel
