On Fri, Feb 22, 2019 at 08:16:03AM +0100, Gerd Hoffmann wrote:
> Also rename it and call it automatically from
> drm_fb_helper_remove_conflicting_pci_framebuffers()
Yeah this looks neat.
> Signed-off-by: Gerd Hoffmann <kra...@redhat.com>
> ---
> include/drm/drm_fb_helper.h | 14 +++++++++++---
> include/linux/vgaarb.h | 2 ++
> drivers/gpu/drm/i915/i915_drv.c | 43
> -----------------------------------------
> drivers/gpu/vga/vgaarb.c | 38 ++++++++++++++++++++++++++++++++++++
> 4 files changed, 51 insertions(+), 46 deletions(-)
>
> diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h
> index bb9acea61369..286d58efed5d 100644
> --- a/include/drm/drm_fb_helper.h
> +++ b/include/drm/drm_fb_helper.h
> @@ -36,6 +36,7 @@ struct drm_fb_helper;
> #include <drm/drm_crtc.h>
> #include <drm/drm_device.h>
> #include <linux/kgdb.h>
> +#include <linux/vgaarb.h>
>
> enum mode_set_atomic {
> LEAVE_ATOMIC_MODE_SET,
> @@ -642,11 +643,18 @@
> drm_fb_helper_remove_conflicting_pci_framebuffers(struct pci_dev *pdev,
> int resource_id,
> const char *name)
> {
> + int ret = 0;
> +
> + /*
> + * WARNING: Apparently we must kick fbdev drivers before vgacon,
> + * otherwise the vga fbdev driver falls over.
> + */
> #if IS_REACHABLE(CONFIG_FB)
> - return remove_conflicting_pci_framebuffers(pdev, resource_id, name);
> -#else
> - return 0;
> + ret = remove_conflicting_pci_framebuffers(pdev, resource_id, name);
> #endif
> + if (ret == 0)
> + ret = vga_remove_vgacon(pdev);
> + return ret;
> }
>
> #endif
> diff --git a/include/linux/vgaarb.h b/include/linux/vgaarb.h
> index ee162e3e879b..553b34c8b5f7 100644
> --- a/include/linux/vgaarb.h
> +++ b/include/linux/vgaarb.h
> @@ -125,9 +125,11 @@ extern void vga_put(struct pci_dev *pdev, unsigned int
> rsrc);
> #ifdef CONFIG_VGA_ARB
> extern struct pci_dev *vga_default_device(void);
> extern void vga_set_default_device(struct pci_dev *pdev);
> +extern int vga_remove_vgacon(struct pci_dev *pdev);
> #else
> static inline struct pci_dev *vga_default_device(void) { return NULL; };
> static inline void vga_set_default_device(struct pci_dev *pdev) { };
> +static inline int vga_remove_vgacon(struct pci_dev *pdev) { return 0; };
> #endif
>
> /*
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 6630212f2faf..0e87eef542da 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -757,39 +757,6 @@ static int i915_kick_out_firmware_fb(struct
> drm_i915_private *dev_priv)
> return ret;
> }
>
> -#if !defined(CONFIG_VGA_CONSOLE)
> -static int i915_kick_out_vgacon(struct drm_i915_private *dev_priv)
> -{
> - return 0;
> -}
> -#elif !defined(CONFIG_DUMMY_CONSOLE)
> -static 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)
> -{
> - int ret = 0;
> -
> - DRM_INFO("Replacing VGA console driver\n");
> -
> - console_lock();
> - if (con_is_bound(&vga_con))
> - ret = do_take_over_console(&dummy_con, 0, MAX_NR_CONSOLES - 1,
> 1);
> - if (ret == 0) {
> - ret = do_unregister_con_driver(&vga_con);
> -
> - /* Ignore "already unregistered". */
> - if (ret == -ENODEV)
> - ret = 0;
> - }
> - console_unlock();
> -
> - return ret;
> -}
> -#endif
> -
> static void intel_init_dpio(struct drm_i915_private *dev_priv)
> {
> /*
> @@ -1410,22 +1377,12 @@ static int i915_driver_init_hw(struct
> drm_i915_private *dev_priv)
> if (ret)
> goto err_perf;
>
> - /*
> - * WARNING: Apparently we must kick fbdev drivers before vgacon,
> - * otherwise the vga fbdev driver falls over.
> - */
> ret = i915_kick_out_firmware_fb(dev_priv);
This needs to be replaced with a call to
drm_fb_helper_remove_conflicting_pci_framebuffers, because the above
wrapper hasn't been converted yet. Otherwise you end up removing the
vgacon unbind from i915.
> if (ret) {
> DRM_ERROR("failed to remove conflicting framebuffer drivers\n");
> goto err_ggtt;
> }
>
> - ret = i915_kick_out_vgacon(dev_priv);
> - if (ret) {
> - DRM_ERROR("failed to remove conflicting VGA console\n");
> - goto err_ggtt;
> - }
> -
> ret = i915_ggtt_init_hw(dev_priv);
> if (ret)
> goto err_ggtt;
> diff --git a/drivers/gpu/vga/vgaarb.c b/drivers/gpu/vga/vgaarb.c
> index dc8e039bfab5..524092a43de6 100644
> --- a/drivers/gpu/vga/vgaarb.c
> +++ b/drivers/gpu/vga/vgaarb.c
> @@ -48,6 +48,8 @@
> #include <linux/miscdevice.h>
> #include <linux/slab.h>
> #include <linux/screen_info.h>
> +#include <linux/vt.h>
> +#include <linux/console.h>
>
> #include <linux/uaccess.h>
>
> @@ -168,6 +170,42 @@ void vga_set_default_device(struct pci_dev *pdev)
> vga_default = pci_dev_get(pdev);
> }
>
kerneldoc would be nice here.
> +#if !defined(CONFIG_VGA_CONSOLE)
> +int vga_remove_vgacon(struct pci_dev *pdev)
> +{
> + return 0;
> +}
> +#elif !defined(CONFIG_DUMMY_CONSOLE)
> +int vga_remove_vgacon(struct pci_dev *pdev)
> +{
> + return -ENODEV;
> +}
> +#else
> +int vga_remove_vgacon(struct pci_dev *pdev)
> +{
> + int ret = 0;
> +
> + if (pdev != vga_default)
> + return 0;
> + vgaarb_info(&pdev->dev, "deactivate vga console\n");
> +
> + console_lock();
> + if (con_is_bound(&vga_con))
> + ret = do_take_over_console(&dummy_con, 0, MAX_NR_CONSOLES -
> 1, 1);
> + if (ret == 0) {
> + ret = do_unregister_con_driver(&vga_con);
> +
> + /* Ignore "already unregistered". */
> + if (ret == -ENODEV)
> + ret = 0;
> + }
> + console_unlock();
> +
> + return ret;
> +}
> +#endif
> +EXPORT_SYMBOL(vga_remove_vgacon);
> +
Asides from the comments, lgtm. Of course we'll need intel-gfx-ci to
approve too :-) Please cc intel-gfx on the next version for the entire
patcheset (our CI doesn't pick up incomplete patchesets).
Reviewed-by: Daniel Vetter <daniel.vet...@ffwll.ch>
> static inline void vga_irq_set_state(struct vga_device *vgadev, bool state)
> {
> if (vgadev->irq_set_state)
> --
> 2.9.3
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
