2004-09-13 23:28:42

by Dave Airlie

[permalink] [raw]
Subject: Re: [PATCH] DRM: add missing pci_enable_device()


This causes problems when DRI and fb are loaded and you unload dri.. guess
what happens your fb??, or it does in theory I might have time to practice
later,

now the quick fix is to take the stealth/non-stealth code from CVS which
we know works or we wait for Alan to finish his vga device code and we fix
up the DRM and fb to use it ... this patch won't help anyways...

Dave.

On Mon, 13 Sep 2004, Bjorn Helgaas wrote:

> Add pci_enable_device()/pci_disable_device. In the past, drivers often worked
> without this, but it is now required in order to route PCI interrupts
> correctly.
>
> Evan Paul Fletcher found this problem with 2.6.9-rc1-mm4 and X.org 6.8.0
> and verified that this patch fixes it.
>
> Signed-off-by: Bjorn Helgaas <[email protected]>
>
> ===== drivers/char/drm/drm_drv.h 1.47 vs edited =====
> --- 1.47/drivers/char/drm/drm_drv.h 2004-09-08 03:41:23 -06:00
> +++ edited/drivers/char/drm/drm_drv.h 2004-09-13 12:27:16 -06:00
> @@ -443,6 +443,8 @@
> }
> up( &dev->struct_sem );
>
> + pci_disable_device( dev->pdev );
> +
> return 0;
> }
>
> @@ -492,6 +494,9 @@
> return -EPERM;
> dev->device = MKDEV(DRM_MAJOR, dev->minor );
> dev->name = DRIVER_NAME;
> +
> + if ((retcode = pci_enable_device(pdev)))
> + return retcode;
>
> dev->pdev = pdev;
> #ifdef __alpha__
>

--
David Airlie, Software Engineer
http://www.skynet.ie/~airlied / airlied at skynet.ie
pam_smb / Linux DECstation / Linux VAX / ILUG person


2004-09-14 14:47:03

by Bjorn Helgaas

[permalink] [raw]
Subject: Re: [PATCH] DRM: add missing pci_enable_device()

On Monday 13 September 2004 5:28 pm, Dave Airlie wrote:
> This causes problems when DRI and fb are loaded and you unload dri.. guess
> what happens your fb??, or it does in theory I might have time to practice
> later,
>
> now the quick fix is to take the stealth/non-stealth code from CVS which
> we know works or we wait for Alan to finish his vga device code and we fix
> up the DRM and fb to use it ... this patch won't help anyways...

OK, I'll assume you understand the issue and will resolve it. In the
meantime, users of DRM will have to supply "pci=routeirq".

> On Mon, 13 Sep 2004, Bjorn Helgaas wrote:
>
> > Add pci_enable_device()/pci_disable_device. In the past, drivers often worked
> > without this, but it is now required in order to route PCI interrupts
> > correctly.
> >
> > Evan Paul Fletcher found this problem with 2.6.9-rc1-mm4 and X.org 6.8.0
> > and verified that this patch fixes it.
> >
> > Signed-off-by: Bjorn Helgaas <[email protected]>
> >
> > ===== drivers/char/drm/drm_drv.h 1.47 vs edited =====
> > --- 1.47/drivers/char/drm/drm_drv.h 2004-09-08 03:41:23 -06:00
> > +++ edited/drivers/char/drm/drm_drv.h 2004-09-13 12:27:16 -06:00
> > @@ -443,6 +443,8 @@
> > }
> > up( &dev->struct_sem );
> >
> > + pci_disable_device( dev->pdev );
> > +
> > return 0;
> > }
> >
> > @@ -492,6 +494,9 @@
> > return -EPERM;
> > dev->device = MKDEV(DRM_MAJOR, dev->minor );
> > dev->name = DRIVER_NAME;
> > +
> > + if ((retcode = pci_enable_device(pdev)))
> > + return retcode;
> >
> > dev->pdev = pdev;
> > #ifdef __alpha__
> >
>
> --
> David Airlie, Software Engineer
> http://www.skynet.ie/~airlied / airlied at skynet.ie
> pam_smb / Linux DECstation / Linux VAX / ILUG person
>
>

2004-09-14 23:19:02

by Dave Airlie

[permalink] [raw]
Subject: Re: [PATCH] DRM: add missing pci_enable_device()

>
> OK, I'll assume you understand the issue and will resolve it. In the
> meantime, users of DRM will have to supply "pci=routeirq".
>

is this -mm only or is it mainline kernel stuff now?

I'll throw an enable in to the bk tree later on....

Dave.

--
David Airlie, Software Engineer
http://www.skynet.ie/~airlied / airlied at skynet.ie
pam_smb / Linux DECstation / Linux VAX / ILUG person

2004-09-14 23:28:29

by Bjorn Helgaas

[permalink] [raw]
Subject: Re: [PATCH] DRM: add missing pci_enable_device()

On Tuesday 14 September 2004 5:12 pm, Dave Airlie wrote:
> > OK, I'll assume you understand the issue and will resolve it. In the
> > meantime, users of DRM will have to supply "pci=routeirq".
>
> is this -mm only or is it mainline kernel stuff now?

It's been in -mm for about a month so far, and it still
needs some cooking before it's ready for mainline. The
remaining issues are:

- nvidia: Nvidia posted a patch for the open-source part
of their driver, but we'll likely have to keep
the "pci=routeirq" option longer than I originally
hoped.
- swsusp: Some devices like prism54, USB don't
work after suspend/resume. Prototype patch
being tested. I suspect we'll trip over
more issues here, because the resume hooks
are poorly documented and inconsistently
implemented.
- DRI: Sounds like you can do the trivial "enable-only"
change that will make things work.

I'm a little surprised that I've only heard one report about
DRI, actually.

> I'll throw an enable in to the bk tree later on....

Great, thanks!

2004-09-14 23:41:32

by [email protected]

[permalink] [raw]
Subject: Re: [PATCH] DRM: add missing pci_enable_device()

It appears that the kernel bk tree is lagging behind the DRM CVS
source. Allow more DRM updates into the kernel and these things will
be fixed. If you want more up to date drivers get them directly from
DRM CVS.

pci_enable/disable_device are correct in the dyn-minor patch. They
also appear to correct in the currently checked in DRM cvs. If fbdev
is loaded DRM does not do pci_enable/disable_device. It is assumed
that these calls are handled by the fbdev device.

You must follow the order rules if using them together.
modprobe fbdev
modprobe DRM
rmmod DRM
rmmod fbdev

You cannot remove these modules out of order or things will break.


On Wed, 15 Sep 2004 00:12:01 +0100 (IST), Dave Airlie <[email protected]> wrote:
> >
> > OK, I'll assume you understand the issue and will resolve it. In the
> > meantime, users of DRM will have to supply "pci=routeirq".
> >
>
> is this -mm only or is it mainline kernel stuff now?
>
> I'll throw an enable in to the bk tree later on....
>
> Dave.
>
> --
> David Airlie, Software Engineer
> http://www.skynet.ie/~airlied / airlied at skynet.ie
> pam_smb / Linux DECstation / Linux VAX / ILUG person
>
>
> -------------------------------------------------------
> This SF.Net email is sponsored by: thawte's Crypto Challenge Vl
> Crack the code and win a Sony DCRHC40 MiniDV Digital Handycam
> Camcorder. More prizes in the weekly Lunch Hour Challenge.
> Sign up NOW http://ad.doubleclick.net/clk;10740251;10262165;m
>
>
> --
> _______________________________________________
> Dri-devel mailing list
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/dri-devel
>



--
Jon Smirl
[email protected]

2004-09-15 13:27:47

by Alan

[permalink] [raw]
Subject: Re: [PATCH] DRM: add missing pci_enable_device()

On Mer, 2004-09-15 at 00:41, Jon Smirl wrote:
> pci_enable/disable_device are correct in the dyn-minor patch. They
> also appear to correct in the currently checked in DRM cvs. If fbdev
> is loaded DRM does not do pci_enable/disable_device. It is assumed
> that these calls are handled by the fbdev device.

If you are calling pci_disable_device at all in the fb driver or DRI
driver it is wrong, always wrong, always will be wrong for the main
head. The video device is almost unique in that when you unload all
the video drivers vgacon still owns and is using it. On some devices
that needs PCI master enabled because of internal magic (like
rendering text modes from the bios via SMM traps)

Alan

2004-09-15 15:35:25

by [email protected]

[permalink] [raw]
Subject: Re: [PATCH] DRM: add missing pci_enable_device()

On Wed, 15 Sep 2004 13:22:48 +0100, Alan Cox <[email protected]> wrote:
> On Mer, 2004-09-15 at 00:41, Jon Smirl wrote:
> > pci_enable/disable_device are correct in the dyn-minor patch. They
> > also appear to correct in the currently checked in DRM cvs. If fbdev
> > is loaded DRM does not do pci_enable/disable_device. It is assumed
> > that these calls are handled by the fbdev device.
>
> If you are calling pci_disable_device at all in the fb driver or DRI
> driver it is wrong, always wrong, always will be wrong for the main
> head. The video device is almost unique in that when you unload all
> the video drivers vgacon still owns and is using it. On some devices
> that needs PCI master enabled because of internal magic (like
> rendering text modes from the bios via SMM traps)
>

How do I trigger this mode on a card supported by DRM so that we can test it?

--
Jon Smirl
[email protected]

2004-09-15 18:14:11

by Alan

[permalink] [raw]
Subject: Re: [PATCH] DRM: add missing pci_enable_device()

On Mer, 2004-09-15 at 16:35, Jon Smirl wrote:
> > the video drivers vgacon still owns and is using it. On some devices
> > that needs PCI master enabled because of internal magic (like
> > rendering text modes from the bios via SMM traps)
> >
> How do I trigger this mode on a card supported by DRM so that we can test it?

I don't know which DRM supporting cards are affected and which platforms
will turn off enough for disable_device to break. I guess
vgacon will need a place in the vga class driver stuff that way the
"ISA" console space would always be owned ?