| Message ID | 1276089725-24744-2-git-send-email-tiago.vignatti@nokia.com |
|---|---|
| State | Deferred, archived |
| Headers | show |
diff --git a/hw/xfree86/common/xf86Configure.c b/hw/xfree86/common/xf86Configure.c index 2f93bb1..4663323 100644 --- a/hw/xfree86/common/xf86Configure.c +++ b/hw/xfree86/common/xf86Configure.c @@ -107,8 +107,6 @@ bus_sbus_configure(void *busData) static void bus_pci_newdev_configure(void *busData, int i, int *chipset) { - const char *VendorName; - const char *CardName; char busnum[8]; struct pci_device * pVideo = NULL; @@ -116,26 +114,6 @@ bus_pci_newdev_configure(void *busData, int i, int *chipset) DevToConfig[i].pVideo = pVideo; - VendorName = pci_device_get_vendor_name( pVideo ); - CardName = pci_device_get_device_name( pVideo ); - - if (!VendorName) { - VendorName = xnfalloc(15); - sprintf((char*)VendorName, "Unknown Vendor"); - } - - if (!CardName) { - CardName = xnfalloc(14); - sprintf((char*)CardName, "Unknown Board"); - } - - DevToConfig[i].GDev.identifier = - xnfalloc(strlen(VendorName) + strlen(CardName) + 2); - sprintf(DevToConfig[i].GDev.identifier, "%s %s", VendorName, CardName); - - DevToConfig[i].GDev.vendor = (char *)VendorName; - DevToConfig[i].GDev.board = (char *)CardName; - DevToConfig[i].GDev.busID = xnfalloc(16); xf86FormatPciBusNumber(pVideo->bus, busnum); sprintf(DevToConfig[i].GDev.busID, "PCI:%s:%d:%d", @@ -357,7 +335,6 @@ configureDeviceSection (int screennum) /* Move device info to parser structure */ sprintf(identifier, "Card%d", screennum); ptr->dev_identifier = strdup(identifier); -/* ptr->dev_identifier = DevToConfig[screennum].GDev.identifier;*/ ptr->dev_vendor = DevToConfig[screennum].GDev.vendor; ptr->dev_board = DevToConfig[screennum].GDev.board; ptr->dev_chipset = DevToConfig[screennum].GDev.chipset;
Tiago Vignatti wrote: > Vendor and board naming were never used to create the configure file a device. > This patch remove their references. I have been wanting for a while to move the autoconfiguration matching for video cards from a hardcoded list in the code to xorg.conf.d-style files like we use for input devices, and wonder if we'd want to use these in Match rules like the input devices have - though obviously we can do MatchPCIVendor "0x8086" just as well as MatchPCIVendor "Intel", and probably more reliably since pci.ids names can change without warning. Of course, wanting to do it, and having a couple weeks free to actually do it are two very different things.
On Wed, Jun 9, 2010 at 7:27 AM, Alan Coopersmith <alan.coopersmith@oracle.com> wrote: > Tiago Vignatti wrote: >> Vendor and board naming were never used to create the configure file a device. >> This patch remove their references. > > I have been wanting for a while to move the autoconfiguration matching for > video cards from a hardcoded list in the code to xorg.conf.d-style files like > we use for input devices, and wonder if we'd want to use these in Match rules > like the input devices have - though obviously we can do MatchPCIVendor "0x8086" > just as well as MatchPCIVendor "Intel", and probably more reliably since pci.ids > names can change without warning. Of course, getting the strings from pci.ids during autoconfig puts you right back to a slow initialization. I think it would be easy enough to use the IDs. I did something similar for USB IDs and InputClass: http://lists.freedesktop.org/archives/xorg-devel/2010-June/009802.html > Of course, wanting to do it, and having a couple weeks free to actually do it > are two very different things. If you want some help on this, let me know. I'm pretty familiar with how the configuration/matching part works, but less familiar with the Device initialization code. -- Dan
Dan Nicholson wrote: > On Wed, Jun 9, 2010 at 7:27 AM, Alan Coopersmith > <alan.coopersmith@oracle.com> wrote: >> Tiago Vignatti wrote: >>> Vendor and board naming were never used to create the configure file a device. >>> This patch remove their references. >> I have been wanting for a while to move the autoconfiguration matching for >> video cards from a hardcoded list in the code to xorg.conf.d-style files like >> we use for input devices, and wonder if we'd want to use these in Match rules >> like the input devices have - though obviously we can do MatchPCIVendor "0x8086" >> just as well as MatchPCIVendor "Intel", and probably more reliably since pci.ids >> names can change without warning. > > Of course, getting the strings from pci.ids during autoconfig puts you > right back to a slow initialization. I think it would be easy enough > to use the IDs. I did something similar for USB IDs and InputClass: > > http://lists.freedesktop.org/archives/xorg-devel/2010-June/009802.html Yeah, that's probably much like what we'll want. >> Of course, wanting to do it, and having a couple weeks free to actually do it >> are two very different things. > > If you want some help on this, let me know. I'm pretty familiar with > how the configuration/matching part works, but less familiar with the > Device initialization code. I don't think the device initialization code is involved. What I was thinking of was replacing the code currently in xf86AutoConfig() in common/xf86AutoConfig.c and videoPtrToDriverList() in xf86pciBus.c (and I guess matchDriverFromFiles() in there, though I don't know much about it) with a something like: xorg.conf.d/90-video-driver-defaults.conf: Section "Device" Identifier "ati" MatchPCIVendor "0x1002" Driver "ati" EndSection Section "Device" Identifier "i740" MatchPCIId "0x8086:0x00di" Driver "i740" EndSection Section "Device" Identifier "i740" MatchPCIId "0x8086:0x7800" Driver "i740" EndSection Section "Device" Identifier "poulsbo" MatchPCIId "0x8086:0x8018" Driver "vesa" EndSection Section "Device" Identifier "intel" MatchPCIVendor "0x8086" Driver "intel" EndSection Section "Device" Identifier "nv" MatchPCIVendor "0x10de" Driver "nv" EndSection xorg.conf.d/99-video-driver-fallbacks.conf: Section "Device" Identifier "vesa" Driver "vesa" EndSection Section "Device" Identifier "fbdev" Driver "fbdev" MatchOS "linux" EndSection Section "Device" Identifier "wsfb" Driver "wsfb" MatchOS "!linux" EndSection And nvidia's driver could install xorg.conf.d/80-nvidia.conf: Section "Device" Identifier "nvidia" MatchPCIVendor "0x10de" Driver "nvidia" EndSection And nouveau could install xorg.conf.d/80-nouveau.conf: Section "Device" Identifier "nouveau" MatchPCIVendor "0x10de" Driver "nouveau" EndSection and we'd never argue about or have to have distros patch the hardcoded defaults table in the code again. (I'm sure I've mangled the syntax above, since I've not yet learned how the input device matching clauses work, but you should get the idea.)
>>>>> "TV" == Tiago Vignatti <tiago.vignatti@nokia.com> writes: TV> Vendor and board naming were never used to create the configure file a device. TV> This patch remove their references. TV> Reported-by: Richard Barnette <jrbarnette@chromium.org> TV> Signed-off-by: Tiago Vignatti <tiago.vignatti@nokia.com> That first sentence is ill-formed. Did you mean «configure file for a device»? With a fixed commit message, Reviewed-by: James Cloos <cloos@jhcloos.com> -JimC
On Wed, Jun 09, 2010 at 11:15:33PM -0700, Alan Coopersmith wrote: > I don't think the device initialization code is involved. What I was thinking > of was replacing the code currently in xf86AutoConfig() in > common/xf86AutoConfig.c and videoPtrToDriverList() in xf86pciBus.c (and I guess > matchDriverFromFiles() in there, though I don't know much about it) > > with a something like: > > xorg.conf.d/90-video-driver-defaults.conf: Even better, just have the drivers themselves install these, so you won't attempt to load a driver which doesn't exist, and you can do device enables with proper autoconfig as soon as the driver's updated, rather than having to keep the server and drivers in sync as they are now. Cheers, Daniel
On Wed, Jun 9, 2010 at 11:15 PM, Alan Coopersmith <alan.coopersmith@oracle.com> wrote: > Dan Nicholson wrote: >> On Wed, Jun 9, 2010 at 7:27 AM, Alan Coopersmith >> <alan.coopersmith@oracle.com> wrote: >>> Tiago Vignatti wrote: >>>> Vendor and board naming were never used to create the configure file a device. >>>> This patch remove their references. >>> I have been wanting for a while to move the autoconfiguration matching for >>> video cards from a hardcoded list in the code to xorg.conf.d-style files like >>> we use for input devices, and wonder if we'd want to use these in Match rules >>> like the input devices have - though obviously we can do MatchPCIVendor "0x8086" >>> just as well as MatchPCIVendor "Intel", and probably more reliably since pci.ids >>> names can change without warning. >> >> Of course, getting the strings from pci.ids during autoconfig puts you >> right back to a slow initialization. I think it would be easy enough >> to use the IDs. I did something similar for USB IDs and InputClass: >> >> http://lists.freedesktop.org/archives/xorg-devel/2010-June/009802.html > > Yeah, that's probably much like what we'll want. > >>> Of course, wanting to do it, and having a couple weeks free to actually do it >>> are two very different things. >> >> If you want some help on this, let me know. I'm pretty familiar with >> how the configuration/matching part works, but less familiar with the >> Device initialization code. > > I don't think the device initialization code is involved. What I was thinking > of was replacing the code currently in xf86AutoConfig() in > common/xf86AutoConfig.c and videoPtrToDriverList() in xf86pciBus.c (and I guess > matchDriverFromFiles() in there, though I don't know much about it) Yeah, I didn't say that correctly. Not the device initialization, but the server initialization. One related thing I'd like to see is having the device configuration be smarter. Right now if you have no xorg.conf, autoconfig basically creates a whole dummy configuration for you. However, if you have parts of a xorg.conf, filling in the missing details (like the driver autoselection) is really hacky. It would be much better if this all got handled in a unified way by walking down a configuration tree so that you have a full configuration from ServerLayout through Screen. At each step down, you'd either grab an existing section or create one. E.g., the user has no Device, so you generate a stub configuration. But they do have a Screen section, so use that instead of generating one. Some of the details seem tricky, but I think we could do better. > with a something like: > > xorg.conf.d/90-video-driver-defaults.conf: > > > Section "Device" > Identifier "ati" > MatchPCIVendor "0x1002" > Driver "ati" > EndSection > > Section "Device" > Identifier "i740" > MatchPCIId "0x8086:0x00di" > Driver "i740" > EndSection > > Section "Device" > Identifier "i740" > MatchPCIId "0x8086:0x7800" > Driver "i740" > EndSection > > Section "Device" > Identifier "poulsbo" > MatchPCIId "0x8086:0x8018" > Driver "vesa" > EndSection > > Section "Device" > Identifier "intel" > MatchPCIVendor "0x8086" > Driver "intel" > EndSection > > Section "Device" > Identifier "nv" > MatchPCIVendor "0x10de" > Driver "nv" > EndSection > > > xorg.conf.d/99-video-driver-fallbacks.conf: > > Section "Device" > Identifier "vesa" > Driver "vesa" > EndSection > > Section "Device" > Identifier "fbdev" > Driver "fbdev" > MatchOS "linux" > EndSection > > Section "Device" > Identifier "wsfb" > Driver "wsfb" > MatchOS "!linux" > EndSection > > And nvidia's driver could install xorg.conf.d/80-nvidia.conf: > > Section "Device" > Identifier "nvidia" > MatchPCIVendor "0x10de" > Driver "nvidia" > EndSection > > And nouveau could install xorg.conf.d/80-nouveau.conf: > > Section "Device" > Identifier "nouveau" > MatchPCIVendor "0x10de" > Driver "nouveau" > EndSection > > and we'd never argue about or have to have distros patch the > hardcoded defaults table in the code again. > > (I'm sure I've mangled the syntax above, since I've not yet learned > how the input device matching clauses work, but you should get the > idea.) Yeah, that's basically what I had brewing in my head. To keep the same behavior as the InputClass matching, I'd suggest putting the generic vesa match first and then having the later sections override. These are just the minor details, though. As a side note, for the USB ID matching, I don't have the 0x hex prefix. They just go "0000:ffff". Do you or others feel strongly about that? If so, I'd rather change my patch now. -- Dan
On Wed, 2010-06-09 at 07:27:32 -0700, Alan Coopersmith wrote: > Tiago Vignatti wrote: > > Vendor and board naming were never used to create the configure file a device. > > This patch remove their references. > > I have been wanting for a while to move the autoconfiguration matching for > video cards from a hardcoded list in the code to xorg.conf.d-style files > like we use for input devices, and wonder if we'd want to use these in > Match rules like the input devices have - though obviously we can do > MatchPCIVendor "0x8086" just as well as MatchPCIVendor "Intel", and > probably more reliably since pci.ids names can change without warning. In case the latter was to get implemented, couldn't it just load the pci.ids only if a MatchPCIVendor (or similar) appeared in the config files, and thus only inflicting any slow down on such systems where this was explicitly requested? regards, guillem
On Thu, Jun 10, 2010 at 7:04 AM, Guillem Jover <guillem@hadrons.org> wrote: > On Wed, 2010-06-09 at 07:27:32 -0700, Alan Coopersmith wrote: >> Tiago Vignatti wrote: >> > Vendor and board naming were never used to create the configure file a device. >> > This patch remove their references. >> >> I have been wanting for a while to move the autoconfiguration matching for >> video cards from a hardcoded list in the code to xorg.conf.d-style files >> like we use for input devices, and wonder if we'd want to use these in >> Match rules like the input devices have - though obviously we can do >> MatchPCIVendor "0x8086" just as well as MatchPCIVendor "Intel", and >> probably more reliably since pci.ids names can change without warning. > > In case the latter was to get implemented, couldn't it just load the > pci.ids only if a MatchPCIVendor (or similar) appeared in the config > files, and thus only inflicting any slow down on such systems where > this was explicitly requested? In Alan's example we're matching on the ID and not the string, but I suppose you could add a slowed down variant like MatchPCIVendorString or something that grabs the strings from pci.ids. But, yeah, definitely you'd only want to reach out to pci.ids if you really needed to. -- Dan
Dan Nicholson wrote: > On Thu, Jun 10, 2010 at 7:04 AM, Guillem Jover <guillem@hadrons.org> wrote: >> On Wed, 2010-06-09 at 07:27:32 -0700, Alan Coopersmith wrote: >>> Tiago Vignatti wrote: >>>> Vendor and board naming were never used to create the configure file a device. >>>> This patch remove their references. >>> I have been wanting for a while to move the autoconfiguration matching for >>> video cards from a hardcoded list in the code to xorg.conf.d-style files >>> like we use for input devices, and wonder if we'd want to use these in >>> Match rules like the input devices have - though obviously we can do >>> MatchPCIVendor "0x8086" just as well as MatchPCIVendor "Intel", and >>> probably more reliably since pci.ids names can change without warning. >> In case the latter was to get implemented, couldn't it just load the >> pci.ids only if a MatchPCIVendor (or similar) appeared in the config >> files, and thus only inflicting any slow down on such systems where >> this was explicitly requested? > > In Alan's example we're matching on the ID and not the string, but I > suppose you could add a slowed down variant like MatchPCIVendorString > or something that grabs the strings from pci.ids. But, yeah, > definitely you'd only want to reach out to pci.ids if you really > needed to. Yeah, because after thinking about it some more, matching on vendor string coming from pci.ids is a bad idea, since configs would break when they update vendor names there (like "ATI" to "AMD" or "Sun" to "Oracle"), or correct device name strings, unlike USB vendor strings which come from the device and won't change arbitrarily. I've seen enough diffs between versions of pci.ids go by as we check in updates to know that it's not a source of stable name strings.
Vendor and board naming were never used to create the configure file a device. This patch remove their references. Reported-by: Richard Barnette <jrbarnette@chromium.org> Signed-off-by: Tiago Vignatti <tiago.vignatti@nokia.com> --- Please, test it. hw/xfree86/common/xf86Configure.c | 23 ----------------------- 1 files changed, 0 insertions(+), 23 deletions(-)