[PATCH] drm/radeon: Add early unregister of firmware fb's

[PATCH] drm/radeon: Add early unregister of firmware fb's

[Daniel Vetter]

From: Benjamin Herrenschmidt <benh@kernel.crashing.org>

Without this, we attempt the handover too late, the firmware fb
might be accessing the chip simultaneously to us re-initializing
various parts of it, which might frighten babies or cause all sort
of nasty psychologic trauma to kitten.

Cc: stable@kernel.org
Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
[danvet: add cc: stable, forward ported and compile-fixed for X86]
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---

This patch is required to get the agp rv270 in my ppc Mac Mini working with
kms.  Also tested on my agp rv570 on amd64, doesn't seem to have ill
effects there.

Please review and apply.

Thanks, Daniel

 drivers/gpu/drm/radeon/radeon_device.c |   19 +++++++++++++++++++
 1 files changed, 19 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_device.c b/drivers/gpu/drm/radeon/radeon_device.c
index 256d204..3aea1a8 100644
--- a/drivers/gpu/drm/radeon/radeon_device.c
+++ b/drivers/gpu/drm/radeon/radeon_device.c
@@ -584,6 +584,22 @@ static bool radeon_switcheroo_can_switch(struct pci_dev *pdev)
 }
 
 
+static void radeon_kick_out_firmware_fb(struct drm_device *ddev)
+{
+	struct apertures_struct *ap;
+	bool primary = false;
+
+	ap = alloc_apertures(1);
+	ap->ranges[0].base = pci_resource_start(ddev->pdev, 0);
+	ap->ranges[0].size = pci_resource_len(ddev->pdev, 0);
+
+#ifdef CONFIG_X86
+	primary = ddev->pdev->resource[PCI_ROM_RESOURCE].flags & IORESOURCE_ROM_SHADOW;
+#endif
+	remove_conflicting_framebuffers(ap, "radeondrmfb", primary);
+	kfree(ap);
+}
+
 int radeon_device_init(struct radeon_device *rdev,
 		       struct drm_device *ddev,
 		       struct pci_dev *pdev,
@@ -623,6 +639,9 @@ int radeon_device_init(struct radeon_device *rdev,
 	init_waitqueue_head(&rdev->irq.vblank_queue);
 	init_waitqueue_head(&rdev->irq.idle_queue);
 
+	/* Get rid of things like offb */
+	radeon_kick_out_firmware_fb(ddev);
+
 	/* setup workqueue */
 	rdev->wq = create_workqueue("radeon");
 	if (rdev->wq == NULL)
-- 
1.7.1

Re: [PATCH] drm/radeon: Add early unregister of firmware fb's

[Marcin Slusarz]

On Wed, Oct 06, 2010 at 06:39:07PM +0200, Daniel Vetter wrote:
> From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> 
> Without this, we attempt the handover too late, the firmware fb
> might be accessing the chip simultaneously to us re-initializing
> various parts of it, which might frighten babies or cause all sort
> of nasty psychologic trauma to kitten.
> 
> Cc: stable@kernel.org
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> [danvet: add cc: stable, forward ported and compile-fixed for X86]
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
> 
> This patch is required to get the agp rv270 in my ppc Mac Mini working with
> kms.  Also tested on my agp rv570 on amd64, doesn't seem to have ill
> effects there.
> 
> Please review and apply.
> 
> Thanks, Daniel
> 
>  drivers/gpu/drm/radeon/radeon_device.c |   19 +++++++++++++++++++
>  1 files changed, 19 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/gpu/drm/radeon/radeon_device.c b/drivers/gpu/drm/radeon/radeon_device.c
> index 256d204..3aea1a8 100644
> --- a/drivers/gpu/drm/radeon/radeon_device.c
> +++ b/drivers/gpu/drm/radeon/radeon_device.c
> @@ -584,6 +584,22 @@ static bool radeon_switcheroo_can_switch(struct pci_dev *pdev)
>  }
>  
>  
> +static void radeon_kick_out_firmware_fb(struct drm_device *ddev)
> +{
> +	struct apertures_struct *ap;
> +	bool primary = false;
> +
> +	ap = alloc_apertures(1);
> +	ap->ranges[0].base = pci_resource_start(ddev->pdev, 0);
> +	ap->ranges[0].size = pci_resource_len(ddev->pdev, 0);

Any reason why this range differs from the one in radeonfb_create?
Maybe it needs to be fixed there too?

> +
> +#ifdef CONFIG_X86
> +	primary = ddev->pdev->resource[PCI_ROM_RESOURCE].flags & IORESOURCE_ROM_SHADOW;
> +#endif
> +	remove_conflicting_framebuffers(ap, "radeondrmfb", primary);
> +	kfree(ap);
> +}
> +
>  int radeon_device_init(struct radeon_device *rdev,
>  		       struct drm_device *ddev,
>  		       struct pci_dev *pdev,
> @@ -623,6 +639,9 @@ int radeon_device_init(struct radeon_device *rdev,
>  	init_waitqueue_head(&rdev->irq.vblank_queue);
>  	init_waitqueue_head(&rdev->irq.idle_queue);
>  
> +	/* Get rid of things like offb */
> +	radeon_kick_out_firmware_fb(ddev);
> +
>  	/* setup workqueue */
>  	rdev->wq = create_workqueue("radeon");
>  	if (rdev->wq == NULL)
> -- 
> 1.7.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/radeon: Add early unregister of firmware fb's

[Daniel Vetter]

On Wed, Oct 06, 2010 at 08:20:15PM +0200, Marcin Slusarz wrote:
> On Wed, Oct 06, 2010 at 06:39:07PM +0200, Daniel Vetter wrote:
> > +static void radeon_kick_out_firmware_fb(struct drm_device *ddev)
> > +{
> > +	struct apertures_struct *ap;
> > +	bool primary = false;
> > +
> > +	ap = alloc_apertures(1);
> > +	ap->ranges[0].base = pci_resource_start(ddev->pdev, 0);
> > +	ap->ranges[0].size = pci_resource_len(ddev->pdev, 0);
> 
> Any reason why this range differs from the one in radeonfb_create?
> Maybe it needs to be fixed there too?

I've stumbled over that, too. It won't really matter at all because the
fb subsystem does an intersection check. The values in radeonfb_create
look more like the correct ones, but I can't get at them before the card
is initialized.

-Daniel
-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48

Re: [PATCH] drm/radeon: Add early unregister of firmware fb's

[Marcin Slusarz]

On Wed, Oct 06, 2010 at 08:54:12PM +0200, Daniel Vetter wrote:
> On Wed, Oct 06, 2010 at 08:20:15PM +0200, Marcin Slusarz wrote:
> > On Wed, Oct 06, 2010 at 06:39:07PM +0200, Daniel Vetter wrote:
> > > +static void radeon_kick_out_firmware_fb(struct drm_device *ddev)
> > > +{
> > > +	struct apertures_struct *ap;
> > > +	bool primary = false;
> > > +
> > > +	ap = alloc_apertures(1);
> > > +	ap->ranges[0].base = pci_resource_start(ddev->pdev, 0);
> > > +	ap->ranges[0].size = pci_resource_len(ddev->pdev, 0);
> > 
> > Any reason why this range differs from the one in radeonfb_create?
> > Maybe it needs to be fixed there too?
> 
> I've stumbled over that, too. It won't really matter at all because the
> fb subsystem does an intersection check. The values in radeonfb_create
> look more like the correct ones, but I can't get at them before the card
> is initialized.

I think regions passed to fb layer should be "memory I want exclusive access to",
not "memory where I want to have the framebuffer", because the first one is stronger - 
you don't want other framebuffer to mess with your non-fb memory...

Marcin

Re: [PATCH] drm/radeon: Add early unregister of firmware fb's

[Daniel Vetter]

Hi Dave,

This hasn't shown up in one of your branches, yet. Do you want me to
change something or was this patch simply lost somewhere?

Thanks, Daniel

On Wed, Oct 06, 2010 at 06:39:07PM +0200, Daniel Vetter wrote:
> From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> 
> Without this, we attempt the handover too late, the firmware fb
> might be accessing the chip simultaneously to us re-initializing
> various parts of it, which might frighten babies or cause all sort
> of nasty psychologic trauma to kitten.
> 
> Cc: stable@kernel.org
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> [danvet: add cc: stable, forward ported and compile-fixed for X86]
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
> 
> This patch is required to get the agp rv270 in my ppc Mac Mini working with
> kms.  Also tested on my agp rv570 on amd64, doesn't seem to have ill
> effects there.
> 
> Please review and apply.
> 
> Thanks, Daniel
> 
>  drivers/gpu/drm/radeon/radeon_device.c |   19 +++++++++++++++++++
>  1 files changed, 19 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/gpu/drm/radeon/radeon_device.c b/drivers/gpu/drm/radeon/radeon_device.c
> index 256d204..3aea1a8 100644
> --- a/drivers/gpu/drm/radeon/radeon_device.c
> +++ b/drivers/gpu/drm/radeon/radeon_device.c
> @@ -584,6 +584,22 @@ static bool radeon_switcheroo_can_switch(struct pci_dev *pdev)
>  }
>  
>  
> +static void radeon_kick_out_firmware_fb(struct drm_device *ddev)
> +{
> +	struct apertures_struct *ap;
> +	bool primary = false;
> +
> +	ap = alloc_apertures(1);
> +	ap->ranges[0].base = pci_resource_start(ddev->pdev, 0);
> +	ap->ranges[0].size = pci_resource_len(ddev->pdev, 0);
> +
> +#ifdef CONFIG_X86
> +	primary = ddev->pdev->resource[PCI_ROM_RESOURCE].flags & IORESOURCE_ROM_SHADOW;
> +#endif
> +	remove_conflicting_framebuffers(ap, "radeondrmfb", primary);
> +	kfree(ap);
> +}
> +
>  int radeon_device_init(struct radeon_device *rdev,
>  		       struct drm_device *ddev,
>  		       struct pci_dev *pdev,
> @@ -623,6 +639,9 @@ int radeon_device_init(struct radeon_device *rdev,
>  	init_waitqueue_head(&rdev->irq.vblank_queue);
>  	init_waitqueue_head(&rdev->irq.idle_queue);
>  
> +	/* Get rid of things like offb */
> +	radeon_kick_out_firmware_fb(ddev);
> +
>  	/* setup workqueue */
>  	rdev->wq = create_workqueue("radeon");
>  	if (rdev->wq == NULL)
> -- 
> 1.7.1
> 

-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48

Re: [PATCH] drm/radeon: Add early unregister of firmware fb's

[Benjamin Herrenschmidt]

On Mon, 2010-08-16 at 09:00 +0200, Rafał Miłecki wrote:
> 2010/8/10 Benjamin Herrenschmidt <benh@kernel.crashing.org>:
> > +#ifdef CONFIG_X86
> > +       primary = dev->pdev->resource[PCI_ROM_RESOURCE].flags &
> IORESOURCE_ROM_SHADOW;
> > +#endif
> 
> It won't compile for CONFIG_X86, no "dev".

Ah right, I've done more fixes, I'll send a new series soon.

Cheers,
Ben.


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

Re: [PATCH] drm/radeon: Add early unregister of firmware fb's

[Rafał Miłecki]

2010/8/10 Benjamin Herrenschmidt <benh@kernel.crashing.org>:
> +#ifdef CONFIG_X86
> +       primary = dev->pdev->resource[PCI_ROM_RESOURCE].flags & IORESOURCE_ROM_SHADOW;
> +#endif

It won't compile for CONFIG_X86, no "dev".

-- 
Rafał
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH] drm/radeon: Add early unregister of firmware fb's

[Benjamin Herrenschmidt]

Without this, we attempt the handover too late, the firmware fb
might be accessing the chip simultaneously to us re-initializing
various parts of it, which might frighten babies or cause all sort
of nasty psychologic trauma to kitten.

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
 drivers/gpu/drm/radeon/radeon_device.c |   18 ++++++++++++++++++
 1 files changed, 18 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_device.c b/drivers/gpu/drm/radeon/radeon_device.c
index cc1464b..9a3e6ba 100644
--- a/drivers/gpu/drm/radeon/radeon_device.c
+++ b/drivers/gpu/drm/radeon/radeon_device.c
@@ -594,6 +594,21 @@ static bool radeon_switcheroo_can_switch(struct pci_dev *pdev)
 }
 
 
+static void radeon_kick_out_firmware_fb(struct drm_device *ddev)
+{
+	struct apertures_struct *ap;
+	bool primary = false;
+
+	ap = alloc_apertures(1);
+	ap->ranges[0].base = drm_get_resource_start(ddev, 0);
+	ap->ranges[0].size = drm_get_resource_len(ddev, 0);
+
+#ifdef CONFIG_X86
+	primary = dev->pdev->resource[PCI_ROM_RESOURCE].flags & IORESOURCE_ROM_SHADOW;
+#endif
+	remove_conflicting_framebuffers(ap, "radeondrmfb", primary);
+}
+
 int radeon_device_init(struct radeon_device *rdev,
 		       struct drm_device *ddev,
 		       struct pci_dev *pdev,
@@ -633,6 +648,9 @@ int radeon_device_init(struct radeon_device *rdev,
 	init_waitqueue_head(&rdev->irq.vblank_queue);
 	init_waitqueue_head(&rdev->irq.idle_queue);
 
+	/* Get rid of things like offb */
+	radeon_kick_out_firmware_fb(ddev);
+
 	/* setup workqueue */
 	rdev->wq = create_workqueue("radeon");
 	if (rdev->wq == NULL)