Re: [Dri-devel] Re: [Linux-fbdev-devel] DRM and pci_driver conversion
[ Jeff: is that PCI ROM enable _really_ that complicated? Ouch. Or is
there some helper function I missed? ]
On Thu, 23 Oct 2003, Jon Smirl wrote:
>
> I don't think DRM drivers are doing things correctly yet. DRM is missing the
> code for marking PCI resources as being in use while DRM is using them. This
> could lead to problems with hotplug. XFree is also mapping PCI ROMs in without
> informing the kernel and that can definitely cause problems.
Absolutely. Changing PCI configurations without telling the kernel _will_
cause problems. Especially for hotplug systems, but it's also very iffy to
do if the card is behind a PCI bridge, since you have to take bridge
resources into account (and know which bridges are transparent, which are
not, etc etc).
The kernel spends a lot of effort on getting this right, and even so it
fails every once in a while (devices that use IO or memory regions without
announcing them in the standard BAR's are quite common, and the kernel has
to have special quirk entries for things like that).
Few enough drivers actually want to enable the roms, but the code should
look something like
/* Assign space for ROM resource if not already assigned. Ugly. */
if (!pci_resource_start(dev, PCI_ROM_RESOURCE))
if (pci_assign_resource(dev, PCI_ROM_RESOURCE) < 0)
return -ENOMEM;
/* Enable it. This is too ugly */
if (!(pci_resource_flags(dev, PCI_ROM_RESOURCE) & PCI_ROM_ADDRESS_ENABLE)) {
u32 val;
pci_read_config_dword(dev, PCI_ROM_ADDRESS, &val);
val |= PCI_ROM_ADDRESS_ENABLE;
pci_write_config_dword(dev, PCI_ROM_ADDRESS, val);
pci_resource_flags(dev, PCI_ROM_RESOURCE) |= PCI_ROM_ADDRESS_ENABLE;
}
/* Enable the device, and regular resources */
if (pci_enable_device(dev))
return -ENODEV;
pci_set_master(dev);/* If we want to */
pci_set_mwi(dev); /* If we want to */
(Yeah, that is more complex than it really should need to be. That's just
a sign of exactly how few device drivers tend to want to do this: the
usual helper stuff is all geared for the normal case).
> new style probe
> if (new probe has device)
>mark resources in use
You shouldn't need to mark the resources in use. Just registering the
driver should do everything for you, including making sure that no other
driver will register that device.
Of course, if you are worried about mixing with drivers that use the old
"find device and just use it", then yes, you'll need to mark the resources
in use. That can be as trivial as just doing a
if (pci_request_regions(dev, "drivername") < 0)
return -EAII!;
in the probe function (but then you need to remember to release them in
the drop function).
Linus
---
This SF.net email is sponsored by: The SF.net Donation Program.
Do you like what SourceForge.net is doing for the Open
Source Community? Make a contribution, and help us add new
features and functionality. Click here: http://sourceforge.net/donate/
___
Dri-devel mailing list
[EMAIL PROTECTED]
https://lists.sourceforge.net/lists/listinfo/dri-devel
Re: [Dri-devel] Radeon DRM memory layout transition
Jens Owen wrote:
We can definitely remove the xf86drmCompat layer for XFree86 5.0. I
believe it's well understood that major version changes will break
existing binary interfaces.
Oh goodie! There's a whole ton of crap that will get ripped out of
lib/GL/glx/glx{cmds,ext}.c then! All of the stuff for determining if
the client-side driver supports glcontextmodes and bindContext2 /
unbindContext2 will go bye-bye. :) This is the best news I've heard all
day...
---
This SF.net email is sponsored by: The SF.net Donation Program.
Do you like what SourceForge.net is doing for the Open
Source Community? Make a contribution, and help us add new
features and functionality. Click here: http://sourceforge.net/donate/
___
Dri-devel mailing list
[EMAIL PROTECTED]
https://lists.sourceforge.net/lists/listinfo/dri-devel
Re: [Dri-devel] Re: [Linux-fbdev-devel] DRM and pci_driver conversion
On Thu, 2003-10-23 at 16:23, Linus Torvalds wrote:
> [ Jeff: is that PCI ROM enable _really_ that complicated? Ouch. Or is
> there some helper function I missed? ]
>
> On Thu, 23 Oct 2003, Jon Smirl wrote:
> >
> > I don't think DRM drivers are doing things correctly yet. DRM is missing the
> > code for marking PCI resources as being in use while DRM is using them. This
> > could lead to problems with hotplug. XFree is also mapping PCI ROMs in without
> > informing the kernel and that can definitely cause problems.
>
> Absolutely. Changing PCI configurations without telling the kernel _will_
> cause problems. Especially for hotplug systems, but it's also very iffy to
> do if the card is behind a PCI bridge, since you have to take bridge
> resources into account (and know which bridges are transparent, which are
> not, etc etc).
>
> The kernel spends a lot of effort on getting this right, and even so it
> fails every once in a while (devices that use IO or memory regions without
> announcing them in the standard BAR's are quite common, and the kernel has
> to have special quirk entries for things like that).
>
> Few enough drivers actually want to enable the roms, but the code should
> look something like
>
> /* Assign space for ROM resource if not already assigned. Ugly. */
> if (!pci_resource_start(dev, PCI_ROM_RESOURCE))
> if (pci_assign_resource(dev, PCI_ROM_RESOURCE) < 0)
> return -ENOMEM;
>
> /* Enable it. This is too ugly */
> if (!(pci_resource_flags(dev, PCI_ROM_RESOURCE) & PCI_ROM_ADDRESS_ENABLE)) {
> u32 val;
> pci_read_config_dword(dev, PCI_ROM_ADDRESS, &val);
> val |= PCI_ROM_ADDRESS_ENABLE;
> pci_write_config_dword(dev, PCI_ROM_ADDRESS, val);
> pci_resource_flags(dev, PCI_ROM_RESOURCE) |= PCI_ROM_ADDRESS_ENABLE;
> }
>
>
> /* Enable the device, and regular resources */
> if (pci_enable_device(dev))
> return -ENODEV;
>
> pci_set_master(dev);/* If we want to */
> pci_set_mwi(dev); /* If we want to */
>
> (Yeah, that is more complex than it really should need to be. That's just
> a sign of exactly how few device drivers tend to want to do this: the
> usual helper stuff is all geared for the normal case).
>
> > new style probe
> > if (new probe has device)
> >mark resources in use
>
> You shouldn't need to mark the resources in use. Just registering the
> driver should do everything for you, including making sure that no other
> driver will register that device.
>
> Of course, if you are worried about mixing with drivers that use the old
> "find device and just use it", then yes, you'll need to mark the resources
> in use. That can be as trivial as just doing a
>
> if (pci_request_regions(dev, "drivername") < 0)
> return -EAII!;
>
> in the probe function (but then you need to remember to release them in
> the drop function).
However, the DRM is now using the old-style probing instead, because of
the radeonfb conflict. In the case of the DRM, we want drivers to
coexist, at least when loaded in the FB then DRM order. What should the
DRM be doing exactly in this case? Is it exactly what Jon Smirl said,
or something else?
--
Eric Anholt[EMAIL PROTECTED]
http://people.freebsd.org/~anholt/ [EMAIL PROTECTED]
---
This SF.net email is sponsored by: The SF.net Donation Program.
Do you like what SourceForge.net is doing for the Open
Source Community? Make a contribution, and help us add new
features and functionality. Click here: http://sourceforge.net/donate/
___
Dri-devel mailing list
[EMAIL PROTECTED]
https://lists.sourceforge.net/lists/listinfo/dri-devel
Re: [Dri-devel] Re: [Linux-fbdev-devel] DRM and pci_driver conversion
On Thu, 23 Oct 2003, Jeff Garzik wrote:
>
> The mechanics aren't complicated, but I seem to recall there being a
> Real Good Reason why you want to leave it disabled 99% of the time. No,
> I don't recall that reason :( But my fuzzy memory seems to think that
> "enable, grab a slice o 'rom, disable" was viable.
That's not what I'm worried about: yes, it's doable, and it's only ten
lines of code, but my point is that having drivers do something like this:
> /* Assign space for ROM resource if not already assigned. Ugly. */
> if (!pci_resource_start(dev, PCI_ROM_RESOURCE))
> if (pci_assign_resource(dev, PCI_ROM_RESOURCE) < 0)
> return -ENOMEM;
>
> /* Enable it. This is too ugly */
> if (!(pci_resource_flags(dev, PCI_ROM_RESOURCE) & PCI_ROM_ADDRESS_ENABLE)) {
> u32 val;
> pci_read_config_dword(dev, PCI_ROM_ADDRESS, &val);
> val |= PCI_ROM_ADDRESS_ENABLE;
> pci_write_config_dword(dev, PCI_ROM_ADDRESS, val);
> pci_resource_flags(dev, PCI_ROM_RESOURCE) |= PCI_ROM_ADDRESS_ENABLE;
> }
over and over again _is_ going to cause us problems later. Either we'll
change some interface slightly (and have to fix up the drivers), or a
couple of the drivers are going to do things _slightly_ wrong (forget to
assign the resource, or forget to mark it enabled, or whatever), and then
we'll have a subtle bug that only shows up on certain hardware and
firmware combinations, depending on how the BIOS sets up regions etc.
So wouldn't it be nice if we just had those ten lines as a generic
function like
int pci_enable_rom(struct pci_device *dev)
{
...
int pci_disable_rom(..);
that does all of this properly for the (few) drivers that need this in
order to fetch resource information from the ROM's?
That way people could just do
if (pci_enable_device(dev))
return -ENODEV;
if (pci_enable_rom(dev))
return -Exxx;
and we wouldn't have to have drivers that have _waay_ too much knowledge
about things like assigning bus resources and the PCI_ROM_ADDRESS_ENABLE
bit.
Drivers really shouldn't need to know or care. A driver writer should just
know that his card has a ROM attached to it, and that he will need to
enable it explicitly because for most chips we just don't want to waste IO
mapping space by enabling it by default.
(Yeah, the driver still needs to know about doing the ioremap() etc and
only accessing it with read/write[bwl(), but those are common to any
memory-mapped thing, so that's not "new" knowledge per se).
Case in point: I had to dig around a bit to write those ten lines of code.
And I consider myself above-average-knowledgeable about PCI device issues.
So I don't think driver writers should have to write those ten lines, the
same way I think they should just do "pci_enable_device()" to get the
interrupt line and regular BAR things up, and the chip powered on..
And no, most drivers really don't care about their roms. Most devices
don't even have any roms. But grepping for "PCI_ROM_" shows that there
clearly are more than one or two that do (and I'm afraid to think about
the ones that use hardcoded values rather than the symbolic ones, just
because the driver writer didn't find a helper function and just did it by
hand by reading the PCI spec).
Linus
---
This SF.net email is sponsored by: The SF.net Donation Program.
Do you like what SourceForge.net is doing for the Open
Source Community? Make a contribution, and help us add new
features and functionality. Click here: http://sourceforge.net/donate/
___
Dri-devel mailing list
[EMAIL PROTECTED]
https://lists.sourceforge.net/lists/listinfo/dri-devel
Re: [Dri-devel] Re: [Linux-fbdev-devel] DRM and pci_driver conversion
Video drivers only enable the ROM long enough to get some values out it and then disable it. You don't want to leave ROMs enabled because there is some hardware that uses the same address decoder for ROM and RAM and you can't use them both at the same time. --- Jeff Garzik <[EMAIL PROTECTED]> wrote: > Linus Torvalds wrote: > > [ Jeff: is that PCI ROM enable _really_ that complicated? Ouch. Or is > > there some helper function I missed? ] > > > The mechanics aren't complicated, but I seem to recall there being a > Real Good Reason why you want to leave it disabled 99% of the time. No, > I don't recall that reason :( But my fuzzy memory seems to think that > "enable, grab a slice o 'rom, disable" was viable. > > Jeff > > > = Jon Smirl [EMAIL PROTECTED] __ Do you Yahoo!? The New Yahoo! Shopping - with improved product search http://shopping.yahoo.com --- This SF.net email is sponsored by: The SF.net Donation Program. Do you like what SourceForge.net is doing for the Open Source Community? Make a contribution, and help us add new features and functionality. Click here: http://sourceforge.net/donate/ ___ Dri-devel mailing list [EMAIL PROTECTED] https://lists.sourceforge.net/lists/listinfo/dri-devel
Re: [Dri-devel] Re: [Linux-fbdev-devel] DRM and pci_driver conversion
On Fri, Oct 24, 2003 at 09:44:38AM -0700, Linus Torvalds wrote:
> So wouldn't it be nice if we just had those ten lines as a generic
> function like
>
> int pci_enable_rom(struct pci_device *dev)
> {
> ...
>
> int pci_disable_rom(..);
Yes. Agreed,
Jeff
---
This SF.net email is sponsored by: The SF.net Donation Program.
Do you like what SourceForge.net is doing for the Open
Source Community? Make a contribution, and help us add new
features and functionality. Click here: http://sourceforge.net/donate/
___
Dri-devel mailing list
[EMAIL PROTECTED]
https://lists.sourceforge.net/lists/listinfo/dri-devel
Re: [Dri-devel] Re: [Linux-fbdev-devel] DRM and pci_driver conversion
On Fri, 24 Oct 2003, Petr Vandrovec wrote: > > It would be nice if it works... For matrox hardware I have to map ROM > over framebuffer (it is solution recommended by datasheet), as there is > no way to get memory range allocated for ROM unless ROM was left enabled > all the time. That's fine - it sounds like Matrox hardware is just buggy, and then you will never be able to use the generic "enable ROM" routines. That shouldn't detract from other drivers doing it, though. On the other hand, we might well be able to work around the matrox behaviour if we really want to: writing all-ones to the register should work, and that is the way we figure out the size of the allocation anyway. So this is one of those things where having a generic routine and knowing a bit about some implementation oddities migth well work out. Maybe some other cards have the same odd behaviour. But since Matrox has a separate recommended solution in their datasheets, I suspect the right thing is just to ignore Matrox when talking about the generic thing. Linus --- This SF.net email is sponsored by: The SF.net Donation Program. Do you like what SourceForge.net is doing for the Open Source Community? Make a contribution, and help us add new features and functionality. Click here: http://sourceforge.net/donate/ ___ Dri-devel mailing list [EMAIL PROTECTED] https://lists.sourceforge.net/lists/listinfo/dri-devel
Re: [Dri-devel] Re: [Linux-fbdev-devel] DRM and pci_driver conversion
PCI ROM enabale/disable has come up before on LKML. Russell made this comment about making the code more portable. --- Russell King <[EMAIL PROTECTED]> wrote: > You should use pcibios_resource_to_bus() to convert a resource to a > representation suitable for a BAR. http://lkml.org/lkml/2003/8/19/279 = Jon Smirl [EMAIL PROTECTED] __ Do you Yahoo!? The New Yahoo! Shopping - with improved product search http://shopping.yahoo.com --- This SF.net email is sponsored by: The SF.net Donation Program. Do you like what SourceForge.net is doing for the Open Source Community? Make a contribution, and help us add new features and functionality. Click here: http://sourceforge.net/donate/ ___ Dri-devel mailing list [EMAIL PROTECTED] https://lists.sourceforge.net/lists/listinfo/dri-devel
Re: [Dri-devel] new 2048 limit code...
Unless anyone says otherwise, I'm going to remove this code. All it has done is generate complaints from MergedFB users. Apparently it doesn't hurt anything (ie. cause a crash) to leave direct rendering enabled if the virtual desktop is larger than 2048. Since MergedFB users seem to be the only ones running into it, I don't see this affecting regular DRI users. I will put a warning in the code if the virtual res is larger than 2048 to let them know that they may run into issues. Alex --- Michel Dänzer <[EMAIL PROTECTED]> wrote: > On Sat, 2003-10-18 at 12:27, Keith Whitwell wrote: > > Eric Anholt wrote: > > > On Fri, 2003-10-17 at 17:27, Alex Deucher wrote: > > > > > >>perhaps we can not disable the DRI if mergedfb is active and the > viral > > >>is larger than 2048? > > It was only MergedFB which made this necessary... > > > > Maybe in the 3d driver you could fall back to software on > grabbing the > > > lock if the width is too large? > > > > Better still would be to have the 3d driver subdivide cliprects > larger than > > width 2048, and adjust the origin cooridnates to make it work. > > Interesting idea. > > I think we should keep the status quo until the 3D drivers can deal > with > this though, to avoid confusion with users who don't know the > details. > > __ Do you Yahoo!? The New Yahoo! Shopping - with improved product search http://shopping.yahoo.com --- This SF.net email is sponsored by: The SF.net Donation Program. Do you like what SourceForge.net is doing for the Open Source Community? Make a contribution, and help us add new features and functionality. Click here: http://sourceforge.net/donate/ ___ Dri-devel mailing list [EMAIL PROTECTED] https://lists.sourceforge.net/lists/listinfo/dri-devel
[Dri-devel] Re: new 2048 limit code...
On Fri, 24 Oct 2003, Alex Deucher wrote: >Unless anyone says otherwise, I'm going to remove this code. All it >has done is generate complaints from MergedFB users. Apparently it >doesn't hurt anything (ie. cause a crash) to leave direct rendering >enabled if the virtual desktop is larger than 2048. Since MergedFB >users seem to be the only ones running into it, I don't see this >affecting regular DRI users. I will put a warning in the code if the >virtual res is larger than 2048 to let them know that they may run into >issues. My suggestion would be to make it a config file option defaulting to the current behaviour. That way users who don't know any better get 3D working full desktop, but get an error/warning to the log file if their resolution goes beyond what is supported. Users who know what they're doing and want 3D on part of the desktop anyway can force enable it. That seems to be less problematic for most people IMHO, no? Anyhow, just a bit of feedback/suggestion, I don't have any hard opinion any way about it... And of course nothing is written in stone so whatever you go with can always be changed again later depending on user feedback, etc. TTYL -- Mike A. Harris --- This SF.net email is sponsored by: The SF.net Donation Program. Do you like what SourceForge.net is doing for the Open Source Community? Make a contribution, and help us add new features and functionality. Click here: http://sourceforge.net/donate/ ___ Dri-devel mailing list [EMAIL PROTECTED] https://lists.sourceforge.net/lists/listinfo/dri-devel
