[PATCH v4 0/2] drm & vgaarb: handle vgacon removal in vgaarb.

[PATCH v4 0/2] drm & vgaarb: handle vgacon removal in vgaarb.

[Gerd Hoffmann]

Gerd Hoffmann (2):
  drm: move i915_kick_out_vgacon to vgaarb
  drm/qxl: remove conflicting framebuffers earlier

 include/drm/drm_fb_helper.h     | 14 +++++++++++---
 include/linux/vgaarb.h          |  2 ++
 drivers/gpu/drm/i915/i915_drv.c | 43 -----------------------------------------
 drivers/gpu/drm/qxl/qxl_drv.c   |  5 ++++-
 drivers/gpu/vga/vgaarb.c        | 38 ++++++++++++++++++++++++++++++++++++
 5 files changed, 55 insertions(+), 47 deletions(-)

-- 
2.9.3

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v4 1/2] drm: move i915_kick_out_vgacon to vgaarb

[Gerd Hoffmann]

Also rename it and call it automatically from
drm_fb_helper_remove_conflicting_pci_framebuffers()

Signed-off-by: Gerd Hoffmann <kraxel@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);
 	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);
 }
 
+#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);
+
 static inline void vga_irq_set_state(struct vga_device *vgadev, bool state)
 {
 	if (vgadev->irq_set_state)
-- 
2.9.3

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v4 2/2] drm/qxl: remove conflicting framebuffers earlier

[Gerd Hoffmann]

Add error checking while being at it.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 drivers/gpu/drm/qxl/qxl_drv.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/qxl/qxl_drv.c b/drivers/gpu/drm/qxl/qxl_drv.c
index bb81e310eb6d..578d867a81d5 100644
--- a/drivers/gpu/drm/qxl/qxl_drv.c
+++ b/drivers/gpu/drm/qxl/qxl_drv.c
@@ -79,6 +79,10 @@ qxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	if (ret)
 		goto free_dev;
 
+	ret = drm_fb_helper_remove_conflicting_pci_framebuffers(pdev, 0, "qxl");
+	if (ret)
+		goto disable_pci;
+
 	ret = qxl_device_init(qdev, &qxl_driver, pdev);
 	if (ret)
 		goto disable_pci;
@@ -94,7 +98,6 @@ qxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	if (ret)
 		goto modeset_cleanup;
 
-	drm_fb_helper_remove_conflicting_pci_framebuffers(pdev, 0, "qxl");
 	drm_fbdev_generic_setup(&qdev->ddev, 32);
 	return 0;
 
-- 
2.9.3

Re: [PATCH v4 1/2] drm: move i915_kick_out_vgacon to vgaarb

[Daniel Vetter]

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 <kraxel@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.vetter@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

Re: [PATCH v4 2/2] drm/qxl: remove conflicting framebuffers earlier

[Daniel Vetter]

On Fri, Feb 22, 2019 at 08:16:04AM +0100, Gerd Hoffmann wrote:
> Add error checking while being at it.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> ---
>  drivers/gpu/drm/qxl/qxl_drv.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/qxl/qxl_drv.c b/drivers/gpu/drm/qxl/qxl_drv.c
> index bb81e310eb6d..578d867a81d5 100644
> --- a/drivers/gpu/drm/qxl/qxl_drv.c
> +++ b/drivers/gpu/drm/qxl/qxl_drv.c
> @@ -79,6 +79,10 @@ qxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>  	if (ret)
>  		goto free_dev;
>  
> +	ret = drm_fb_helper_remove_conflicting_pci_framebuffers(pdev, 0, "qxl");
> +	if (ret)
> +		goto disable_pci;
> +
>  	ret = qxl_device_init(qdev, &qxl_driver, pdev);
>  	if (ret)
>  		goto disable_pci;
> @@ -94,7 +98,6 @@ qxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>  	if (ret)
>  		goto modeset_cleanup;
>  
> -	drm_fb_helper_remove_conflicting_pci_framebuffers(pdev, 0, "qxl");
>  	drm_fbdev_generic_setup(&qdev->ddev, 32);
>  	return 0;
>  
> -- 
> 2.9.3
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [PATCH v4 1/2] drm: move i915_kick_out_vgacon to vgaarb

[Gerd Hoffmann]

  Hi,

> > -	/*
> > -	 * 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.

Ah, little but important difference I didn't notice on the first look.
That wrapper calls the non-pci version.  But seems it isn't that easy
to switch over because the framebuffer is in stolen memory instead of a
pci bar ...

> >  	if (ret) {
> >  		DRM_ERROR("failed to remove conflicting framebuffer drivers\n");
> >  		goto err_ggtt;
> >  	}
> >  
> > -	ret = i915_kick_out_vgacon(dev_priv);

... but we can continue to just call vga_remove_vgacon() here.

> 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).

Ok, will do.

cheers,
  Gerd

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v4 1/2] drm: move i915_kick_out_vgacon to vgaarb

[Daniel Vetter]

On Fri, Feb 22, 2019 at 12:03 PM Gerd Hoffmann <kraxel@redhat.com> wrote:
>
>   Hi,
>
> > > -   /*
> > > -    * 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.
>
> Ah, little but important difference I didn't notice on the first look.
> That wrapper calls the non-pci version.  But seems it isn't that easy
> to switch over because the framebuffer is in stolen memory instead of a
> pci bar ...

stolen memory is where the fb physically resides. the pci bar is how
you access it (as long as you take all the pci bars). From a quick
look i915 and pci version of remove_conflicting_fb matched.
-Daniel

> > >     if (ret) {
> > >             DRM_ERROR("failed to remove conflicting framebuffer drivers\n");
> > >             goto err_ggtt;
> > >     }
> > >
> > > -   ret = i915_kick_out_vgacon(dev_priv);
>
> ... but we can continue to just call vga_remove_vgacon() here.
>
> > 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).
>
> Ok, will do.
>
> cheers,
>   Gerd
>


-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v4 1/2] drm: move i915_kick_out_vgacon to vgaarb

[Gerd Hoffmann]

On Fri, Feb 22, 2019 at 06:20:11PM +0100, Daniel Vetter wrote:
> On Fri, Feb 22, 2019 at 12:03 PM Gerd Hoffmann <kraxel@redhat.com> wrote:
> >
> >   Hi,
> >
> > > > -   /*
> > > > -    * 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.
> >
> > Ah, little but important difference I didn't notice on the first look.
> > That wrapper calls the non-pci version.  But seems it isn't that easy
> > to switch over because the framebuffer is in stolen memory instead of a
> > pci bar ...
> 
> stolen memory is where the fb physically resides. the pci bar is how
> you access it (as long as you take all the pci bars). From a quick
> look i915 and pci version of remove_conflicting_fb matched.

Well, it is

	ap->ranges[0].base = ggtt->gmadr.start;
	ap->ranges[0].size = ggtt->mappable_end;

vs.

	ap->ranges[0].base = pci_resource_start(pdev, res_id);
	ap->ranges[0].size = pci_resource_len(pdev, res_id);

So not obvious that they are the same.  At least to me, maybe it is a
different story for someone knowing the i915 driver better.

thanks,
  Gerd

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v4 1/2] drm: move i915_kick_out_vgacon to vgaarb

[Daniel Vetter]

On Mon, Feb 25, 2019 at 09:34:09AM +0100, Gerd Hoffmann wrote:
> On Fri, Feb 22, 2019 at 06:20:11PM +0100, Daniel Vetter wrote:
> > On Fri, Feb 22, 2019 at 12:03 PM Gerd Hoffmann <kraxel@redhat.com> wrote:
> > >
> > >   Hi,
> > >
> > > > > -   /*
> > > > > -    * 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.
> > >
> > > Ah, little but important difference I didn't notice on the first look.
> > > That wrapper calls the non-pci version.  But seems it isn't that easy
> > > to switch over because the framebuffer is in stolen memory instead of a
> > > pci bar ...
> > 
> > stolen memory is where the fb physically resides. the pci bar is how
> > you access it (as long as you take all the pci bars). From a quick
> > look i915 and pci version of remove_conflicting_fb matched.
> 
> Well, it is
> 
> 	ap->ranges[0].base = ggtt->gmadr.start;
> 	ap->ranges[0].size = ggtt->mappable_end;
> 
> vs.
> 
> 	ap->ranges[0].base = pci_resource_start(pdev, res_id);
> 	ap->ranges[0].size = pci_resource_len(pdev, res_id);
> 
> So not obvious that they are the same.  At least to me, maybe it is a
> different story for someone knowing the i915 driver better.

It's one of the pci bars, so amounts to the same. But yeah not obvious if
you don't know that.
-Daniel
-- 
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

Re: [PATCH v4 1/2] drm: move i915_kick_out_vgacon to vgaarb

[Gerd Hoffmann]

> > > stolen memory is where the fb physically resides. the pci bar is how
> > > you access it (as long as you take all the pci bars). From a quick
> > > look i915 and pci version of remove_conflicting_fb matched.
> > 
> > Well, it is
> > 
> > 	ap->ranges[0].base = ggtt->gmadr.start;
> > 	ap->ranges[0].size = ggtt->mappable_end;
> > 
> > vs.
> > 
> > 	ap->ranges[0].base = pci_resource_start(pdev, res_id);
> > 	ap->ranges[0].size = pci_resource_len(pdev, res_id);
> > 
> > So not obvious that they are the same.  At least to me, maybe it is a
> > different story for someone knowing the i915 driver better.
> 
> It's one of the pci bars, so amounts to the same. But yeah not obvious if
> you don't know that.

On my system (kaby lake) it happens to be pci bar 2, is that true for
all igd devices?

cheers,
  Gerd

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel