| Message ID | 1387214365-9625-1-git-send-email-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Dec 16, 2013 at 05:19:25PM +0000, Chris Wilson wrote: > Touching the VGA resources on an IVB EFI machine causes hard hangs when > we then kick out the efifb. Ouch. > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Like I've said on irc I think we want a select DUMMY_CONSOLE here, at least if none of the core developers objects. And we also need this (or something similar) to make Paulo happy, this should fix unclaimed register disasters due to vgacon accessing the dead vga ranges when unloading our driver on hsw while pc8/D3 is in effect. Paulo, can you please give this a spin - you need to make sure that CONFIG_DUMMY_CONSOLE is set though, so that needs some magic. -Daniel > --- > drivers/gpu/drm/i915/i915_dma.c | 29 +++++++++++++++++++++++++++++ > drivers/video/console/dummycon.c | 1 + > 2 files changed, 30 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c > index 6de3a43e3acf..837d1a69ca52 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> > @@ -1444,6 +1446,27 @@ static int i915_kick_out_firmware_fb(struct drm_i915_private *dev_priv) > } > #endif > > +static int i915_kick_out_vgacon(struct drm_i915_private *dev_priv) > +{ > + int ret; > + > +#if !defined(CONFIG_VGA_CONSOLE) > + return 0; > +#endif > + > +#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); > + console_unlock(); > + > + return ret; > +} > + > static void i915_dump_device_info(struct drm_i915_private *dev_priv) > { > const struct intel_device_info *info = dev_priv->info; > @@ -1557,6 +1580,12 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags) > goto out_regs; > > 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; > + } > + > ret = i915_kick_out_firmware_fb(dev_priv); > if (ret) { > DRM_ERROR("failed to remove conflicting framebuffer drivers\n"); > 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); > -- > 1.8.5.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
2013/12/16 Daniel Vetter <daniel@ffwll.ch>: > On Mon, Dec 16, 2013 at 05:19:25PM +0000, Chris Wilson wrote: >> Touching the VGA resources on an IVB EFI machine causes hard hangs when >> we then kick out the efifb. Ouch. Please take a look at arch/x86/kernel/setup.c, at the end of function setup_arch: #ifdef CONFIG_VT #if defined(CONFIG_VGA_CONSOLE) if (!efi_enabled(EFI_BOOT) || (efi_mem_type(0xa0000) != EFI_CONVENTIONAL_MEMORY)) conswitchp = &vga_con; #elif defined(CONFIG_DUMMY_CONSOLE) conswitchp = &dummy_con; #endif #endif Shouldn't this code already be doing what you want on IVB? Perhaps we could improve it to do what you want? >> >> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > Like I've said on irc I think we want a select DUMMY_CONSOLE here, at > least if none of the core developers objects. And we also need this (or > something similar) to make Paulo happy, this should fix unclaimed register > disasters due to vgacon accessing the dead vga ranges when unloading our > driver on hsw while pc8/D3 is in effect. > > Paulo, can you please give this a spin - you need to make sure that > CONFIG_DUMMY_CONSOLE is set though, so that needs some magic. I teste the patch, it does the required magic to solve the remaining problem :) > -Daniel > >> --- >> drivers/gpu/drm/i915/i915_dma.c | 29 +++++++++++++++++++++++++++++ >> drivers/video/console/dummycon.c | 1 + >> 2 files changed, 30 insertions(+) >> >> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c >> index 6de3a43e3acf..837d1a69ca52 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> >> @@ -1444,6 +1446,27 @@ static int i915_kick_out_firmware_fb(struct drm_i915_private *dev_priv) >> } >> #endif >> >> +static int i915_kick_out_vgacon(struct drm_i915_private *dev_priv) >> +{ >> + int ret; >> + >> +#if !defined(CONFIG_VGA_CONSOLE) >> + return 0; >> +#endif >> + >> +#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); >> + console_unlock(); >> + What if the dummy console is already in place? Shouldn't we somehow take a look at the conswitchip variable or something like that? Otherwise doing module_reload many times will make us load dummy_con many times, no? >> + return ret; >> +} >> + >> static void i915_dump_device_info(struct drm_i915_private *dev_priv) >> { >> const struct intel_device_info *info = dev_priv->info; >> @@ -1557,6 +1580,12 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags) >> goto out_regs; >> >> 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"); (note: for some reason this patch doesn't apply directly on -nightly due to differences on this chunk) >> + goto out_gtt; >> + } >> + >> ret = i915_kick_out_firmware_fb(dev_priv); >> if (ret) { >> DRM_ERROR("failed to remove conflicting framebuffer drivers\n"); >> 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); >> -- >> 1.8.5.1 >> >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Daniel Vetter > Software Engineer, Intel Corporation > +41 (0) 79 365 57 48 - http://blog.ffwll.ch
On Mon, Dec 16, 2013 at 06:07:14PM -0200, Paulo Zanoni wrote: > 2013/12/16 Daniel Vetter <daniel@ffwll.ch>: > > On Mon, Dec 16, 2013 at 05:19:25PM +0000, Chris Wilson wrote: > >> Touching the VGA resources on an IVB EFI machine causes hard hangs when > >> we then kick out the efifb. Ouch. > > Please take a look at arch/x86/kernel/setup.c, at the end of function > setup_arch: > > #ifdef CONFIG_VT > #if defined(CONFIG_VGA_CONSOLE) > if (!efi_enabled(EFI_BOOT) || (efi_mem_type(0xa0000) != > EFI_CONVENTIONAL_MEMORY)) > conswitchp = &vga_con; > #elif defined(CONFIG_DUMMY_CONSOLE) > conswitchp = &dummy_con; > #endif > #endif > > Shouldn't this code already be doing what you want on IVB? Perhaps we > could improve it to do what you want? Yes. I jumped to the wrong conclusion, and double checking after uninstalling efifb we default to the dummy console. > >> > >> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > > > Like I've said on irc I think we want a select DUMMY_CONSOLE here, at > > least if none of the core developers objects. And we also need this (or > > something similar) to make Paulo happy, this should fix unclaimed register > > disasters due to vgacon accessing the dead vga ranges when unloading our > > driver on hsw while pc8/D3 is in effect. > > > > Paulo, can you please give this a spin - you need to make sure that > > CONFIG_DUMMY_CONSOLE is set though, so that needs some magic. > > I teste the patch, it does the required magic to solve the remaining problem :) Care to write a real changelog describing the problem this actually solves then? -Chris
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c index 6de3a43e3acf..837d1a69ca52 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> @@ -1444,6 +1446,27 @@ static int i915_kick_out_firmware_fb(struct drm_i915_private *dev_priv) } #endif +static int i915_kick_out_vgacon(struct drm_i915_private *dev_priv) +{ + int ret; + +#if !defined(CONFIG_VGA_CONSOLE) + return 0; +#endif + +#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); + console_unlock(); + + return ret; +} + static void i915_dump_device_info(struct drm_i915_private *dev_priv) { const struct intel_device_info *info = dev_priv->info; @@ -1557,6 +1580,12 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags) goto out_regs; 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; + } + ret = i915_kick_out_firmware_fb(dev_priv); if (ret) { DRM_ERROR("failed to remove conflicting framebuffer drivers\n"); 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);
Touching the VGA resources on an IVB EFI machine causes hard hangs when we then kick out the efifb. Ouch. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> --- drivers/gpu/drm/i915/i915_dma.c | 29 +++++++++++++++++++++++++++++ drivers/video/console/dummycon.c | 1 + 2 files changed, 30 insertions(+)