Re: [PATCH] Improved autoconfig drivers matching

2017-05-09 Thread Eric Anholt 
Adam Jackson  writes:

> On Mon, 2017-05-01 at 18:03 +0900, Michel Dänzer wrote:
>> On 28/04/17 04:11 AM, Adam Jackson wrote:
>> > Fixed up (and rebased and made meson-aware) and merged:
>> > 
>> > To ssh://git.freedesktop.org/git/xorg/xserver
>> >    1549e3037..112d0d7d0  master -> master
>> 
>> This broke things on my development box.
>> 
>> There are two GPUs. When I want Xorg to use the secondary GPU for its
>> primary screen, I use a xorg.conf with (effectively) only:
>> 
>> Section "Device"
>> Identifier "Device0"
>> BusID   "PCI:1:0:0"
>> EndSection
>> 
>> Before this change, this worked fine, automatically[0] using the amdgpu
>> driver for that GPU. With this change, Xorg fails to start up, see the
>> attached log file.
>
> Ugh. I don't know if I'm going to have time to investigate that in
> detail, so I lean towards reverting. If the original problem was that
> 20 was too small a number let's just bump it to something safely crazy
> like 256. Sound reasonable?

I just got hit by it too.  I've pushed the revert until Aaron can sort
it out.


signature.asc
Description: PGP signature
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH] Improved autoconfig drivers matching

2017-05-09 Thread Aaron Plattner 
On 05/03/2017 01:11 PM, Adam Jackson wrote:
> On Mon, 2017-05-01 at 18:03 +0900, Michel Dänzer wrote:
>> On 28/04/17 04:11 AM, Adam Jackson wrote:
>>> Fixed up (and rebased and made meson-aware) and merged:
>>>
>>> To ssh://git.freedesktop.org/git/xorg/xserver
>>>1549e3037..112d0d7d0  master -> master
>>
>> This broke things on my development box.
>>
>> There are two GPUs. When I want Xorg to use the secondary GPU for its
>> primary screen, I use a xorg.conf with (effectively) only:
>>
>> Section "Device"
>> Identifier "Device0"
>> BusID   "PCI:1:0:0"
>> EndSection
>>
>> Before this change, this worked fine, automatically[0] using the amdgpu
>> driver for that GPU. With this change, Xorg fails to start up, see the
>> attached log file.
> 
> Ugh. I don't know if I'm going to have time to investigate that in
> detail, so I lean towards reverting. If the original problem was that
> 20 was too small a number let's just bump it to something safely crazy
> like 256. Sound reasonable?

I found the problem. Karol's change lost the initialization of i here:

@@ -398,8 +412,8 @@ autoConfigDevice(GDevPtr preconf_device)

 /* for each other driver found, copy the first screen,
insert it
  * into the list of screens and set the driver */
-for (i = 1; i < num_matches; i++) {
-if (!copyScreen(slp[0].screen, ptr, i, matches[i]))
+while (i++ < md.nmatches) {
+if (!copyScreen(slp[0].screen, ptr, i, md.matches[i]))
 return NULL;
 }

Patch incoming.

-- Aaron
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH] Improved autoconfig drivers matching

2017-05-08 Thread Michel Dänzer 
On 04/05/17 05:21 AM, Aaron Plattner wrote:
> On 05/03/2017 01:11 PM, Adam Jackson wrote:
>> On Mon, 2017-05-01 at 18:03 +0900, Michel Dänzer wrote:
>>> On 28/04/17 04:11 AM, Adam Jackson wrote:
 Fixed up (and rebased and made meson-aware) and merged:

 To ssh://git.freedesktop.org/git/xorg/xserver
1549e3037..112d0d7d0  master -> master
>>>
>>> This broke things on my development box.
>>>
>>> There are two GPUs. When I want Xorg to use the secondary GPU for its
>>> primary screen, I use a xorg.conf with (effectively) only:
>>>
>>> Section "Device"
>>> Identifier "Device0"
>>> BusID   "PCI:1:0:0"
>>> EndSection
>>>
>>> Before this change, this worked fine, automatically[0] using the amdgpu
>>> driver for that GPU. With this change, Xorg fails to start up, see the
>>> attached log file.
> 
> Weird. Do you have a working log to compare against?

Sure, attached. This is with the commit in question reverted on top of
a06bb73053d9 ("xwayland: Unconditionally initialize lists in
init_tablet_manager_seat()") .


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
[ 28699.241] 
This is a pre-release version of the X server from The X.Org Foundation.
It is not supported in any way.
Bugs may be filed in the bugzilla at http://bugs.freedesktop.org/.
Select the "xorg" product for bugs you find in this release.
Before reporting bugs in pre-release versions please check the
latest version in the X.Org Foundation git repository.
See http://wiki.x.org/wiki/GitPage for git access instructions.
[ 28699.241] 
X.Org X Server 1.19.99.1
Release Date: 2016-11-18
[ 28699.241] X Protocol Version 11, Revision 0
[ 28699.241] Build Operating System: Linux 4.11.0+ x86_64 
[ 28699.241] Current Operating System: Linux kaveri 4.11.0+ #256 SMP Mon May 1 17:40:45 JST 2017 x86_64
[ 28699.241] Kernel command line: BOOT_IMAGE=/boot/vmlinuz-4.11.0+ root=/dev/mapper/VG--Debian-LV--sid ro radeon.lockup_timeout=0 radeon.bapm=1 amdgpu.bapm=1 modprobe.blacklist=amdgpu quiet
[ 28699.241] Build Date: 08 May 2017  05:02:29PM
[ 28699.241]  
[ 28699.241] Current version of pixman: 0.35.1
[ 28699.241] 	Before reporting problems, check http://wiki.x.org
	to make sure that you have the latest version.
[ 28699.241] Markers: (--) probed, (**) from config file, (==) default setting,
	(++) from command line, (!!) notice, (II) informational,
	(WW) warning, (EE) error, (NI) not implemented, (??) unknown.
[ 28699.241] (==) Log file: "/usr/local/var/log/Xorg.1.log", Time: Mon May  8 17:11:44 2017
[ 28699.241] (==) Using config file: "/etc/X11/xorg.conf"
[ 28699.241] (==) Using system config directory "/usr/local/share/X11/xorg.conf.d"
[ 28699.241] (==) No Layout section.  Using the first Screen section.
[ 28699.241] (==) No screen section available. Using defaults.
[ 28699.241] (**) |-->Screen "Default Screen Section" (0)
[ 28699.241] (**) |   |-->Monitor ""
[ 28699.242] (==) No device specified for screen "Default Screen Section".
	Using the first device section listed.
[ 28699.242] (**) |   |-->Device "Device0"
[ 28699.242] (==) No monitor specified for screen "Default Screen Section".
	Using a default monitor configuration.
[ 28699.242] (**) Option "IndirectGLX"
[ 28699.242] (==) Automatically adding devices
[ 28699.242] (==) Automatically enabling devices
[ 28699.242] (==) Automatically adding GPU devices
[ 28699.242] (==) Max clients allowed: 256, resource mask: 0x1f
[ 28699.242] (WW) The directory "/usr/share/fonts/X11/TTF/" does not exist.
[ 28699.242] 	Entry deleted from font path.
[ 28699.242] (WW) The directory "/usr/share/fonts/X11/OTF/" does not exist.
[ 28699.242] 	Entry deleted from font path.
[ 28699.242] (==) FontPath set to:
	/usr/share/fonts/X11/misc/,
	/usr/share/fonts/X11/Type1/,
	/usr/share/fonts/X11/100dpi/,
	/usr/share/fonts/X11/75dpi/
[ 28699.242] (==) ModulePath set to "/usr/local/lib/xorg/modules"
[ 28699.242] (II) The server relies on udev to provide the list of input devices.
	If no devices become available, reconfigure udev or disable AutoAddDevices.
[ 28699.242] (II) Loader magic: 0x55e9afc6ad20
[ 28699.242] (II) Module ABI versions:
[ 28699.242] 	X.Org ANSI C Emulation: 0.4
[ 28699.242] 	X.Org Video Driver: 24.0
[ 28699.242] 	X.Org XInput driver : 24.1
[ 28699.242] 	X.Org Server Extension : 10.0
[ 28699.242] (--) using VT number 2

[ 28699.242] (II) systemd-logind: logind integration requires -keeptty and -keeptty was not provided, disabling logind integration
[ 28699.243] (II) xfree86: Adding drm device (/dev/dri/card0)
[ 28699.243] (II) xfree86: Adding drm device (/dev/dri/card1)
[ 28699.251] (--) PCI:*(0:0:1:0) 1002:130f:1043:85cb rev 0, Mem @ 0xd000/268435456, 0xf080/8388608, 0xfeb0/262144, I/O @ 0xf000/256, BIOS @ 0x/131072
[ 28699.251] (--) PCI: (0:1:0:0) 1002:6939:1458:229d rev 0, Mem @ 0xe000/268435456, 0xf000/2097152, 0xfea0/262144, I/O @ 0xe000/256,

Re: [PATCH] Improved autoconfig drivers matching

2017-05-03 Thread Peter Hutterer 
On Mon, May 01, 2017 at 06:03:22PM +0900, Michel Dänzer wrote:
> On 28/04/17 04:11 AM, Adam Jackson wrote:
> > On Wed, 2017-04-12 at 23:42 +0100, Emil Velikov wrote:
> >>> On 12 April 2017 at 23:05, Aaron Plattner  wrote:
> >>> On 07/12/2016 04:31 PM, Emil Velikov wrote:
>  Since xf86platformBus.h is part of the SDK, If we do this, then the
>  new header must become one as well (should be listed in sdk_HEADERS).
>  Alternatively we can forward declare XF86MatchedDrivers and include
>  the header in EXTRA_DIST. Not sure if the latter is a good idea
>  though, since the actual ABI will be undefined/private.
> 
>  Or better yet, neither of the two exported symbols
>  (xf86PlatformDeviceCheckBusID, xf86PlatformMatchDriver) is used and
>  imho we can remove them. Seems that the header is used solely for the
>  ODEV management, which isn't platform devices specific and one can
>  just move those parts into a separate header and use _it_ in the SDK ?
> 
>  But all that (everything but the sdk_HEADERS/EXTRA_DIST fix) is added
>  bogus, which shouldn't stop the patch from landing.
> >>> Another customer ran into this recently. Adam, can this be merged? I don't
> >>> think Emil's reply was a nack.
> >>
> >> Precisely. My earlier message should have read:
> >>
> >> xf86MatchDrivers.h must be in the sdk_HEADERS or you'll need build
> >> hacks in each driver. With that the patch is
> >> Reviewed-by: Emil Velikov 
> > 
> > Fixed up (and rebased and made meson-aware) and merged:
> > 
> > To ssh://git.freedesktop.org/git/xorg/xserver
> >1549e3037..112d0d7d0  master -> master
> 
> This broke things on my development box.

broke things for me too, reverting fixed it.

May 04 11:38:22 jelly /usr/libexec/gdm-x-session[28936]: (==) Matched intel as 
autoconfigured driver 0
May 04 11:38:22 jelly /usr/libexec/gdm-x-session[28936]: (==) Matched 
modesetting as autoconfigured driver 1
May 04 11:38:22 jelly /usr/libexec/gdm-x-session[28936]: (==) Matched fbdev as 
autoconfigured driver 2
May 04 11:38:22 jelly /usr/libexec/gdm-x-session[28936]: (==) Matched vesa as 
autoconfigured driver 3
May 04 11:38:22 jelly /usr/libexec/gdm-x-session[28936]: (==) Assigned the 
driver to the xf86ConfigLayout
May 04 11:38:22 jelly /usr/libexec/gdm-x-session[28936]: (II) LoadModule: 
"intel"
May 04 11:38:22 jelly /usr/libexec/gdm-x-session[28936]: (WW) Warning, couldn't 
open module intel
May 04 11:38:22 jelly /usr/libexec/gdm-x-session[28936]: (EE) Failed to load 
module "intel" (module does not exist, 0)
May 04 11:38:22 jelly /usr/libexec/gdm-x-session[28936]: (EE) No drivers 
available.
May 04 11:38:22 jelly /usr/libexec/gdm-x-session[28936]: (EE)
May 04 11:38:22 jelly /usr/libexec/gdm-x-session[28936]: Fatal server error:
May 04 11:38:22 jelly /usr/libexec/gdm-x-session[28936]: (EE) no screens 
found(EE)

i.e. it now fails hard instead of continuing with the next driver.
 
Cheers,
   Peter
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH] Improved autoconfig drivers matching

2017-05-03 Thread Aaron Plattner 
On 05/03/2017 01:11 PM, Adam Jackson wrote:
> On Mon, 2017-05-01 at 18:03 +0900, Michel Dänzer wrote:
>> On 28/04/17 04:11 AM, Adam Jackson wrote:
>>> Fixed up (and rebased and made meson-aware) and merged:
>>>
>>> To ssh://git.freedesktop.org/git/xorg/xserver
>>>1549e3037..112d0d7d0  master -> master
>>
>> This broke things on my development box.
>>
>> There are two GPUs. When I want Xorg to use the secondary GPU for its
>> primary screen, I use a xorg.conf with (effectively) only:
>>
>> Section "Device"
>> Identifier "Device0"
>> BusID   "PCI:1:0:0"
>> EndSection
>>
>> Before this change, this worked fine, automatically[0] using the amdgpu
>> driver for that GPU. With this change, Xorg fails to start up, see the
>> attached log file.

Weird. Do you have a working log to compare against?

> Ugh. I don't know if I'm going to have time to investigate that in
> detail, so I lean towards reverting. If the original problem was that
> 20 was too small a number let's just bump it to something safely crazy
> like 256. Sound reasonable?
Sad, but sure, fine with me. 16 drivers times 16 GPUs should be rare
enough that it won't be a problem in practice.

-- Aaron
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH] Improved autoconfig drivers matching

2017-05-03 Thread Adam Jackson 
On Mon, 2017-05-01 at 18:03 +0900, Michel Dänzer wrote:
> On 28/04/17 04:11 AM, Adam Jackson wrote:
> > Fixed up (and rebased and made meson-aware) and merged:
> > 
> > To ssh://git.freedesktop.org/git/xorg/xserver
> >    1549e3037..112d0d7d0  master -> master
> 
> This broke things on my development box.
> 
> There are two GPUs. When I want Xorg to use the secondary GPU for its
> primary screen, I use a xorg.conf with (effectively) only:
> 
> Section "Device"
> Identifier "Device0"
> BusID   "PCI:1:0:0"
> EndSection
> 
> Before this change, this worked fine, automatically[0] using the amdgpu
> driver for that GPU. With this change, Xorg fails to start up, see the
> attached log file.

Ugh. I don't know if I'm going to have time to investigate that in
detail, so I lean towards reverting. If the original problem was that
20 was too small a number let's just bump it to something safely crazy
like 256. Sound reasonable?

- ajax
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH] Improved autoconfig drivers matching

2017-05-01 Thread Michel Dänzer 
On 28/04/17 04:11 AM, Adam Jackson wrote:
> On Wed, 2017-04-12 at 23:42 +0100, Emil Velikov wrote:
>>> On 12 April 2017 at 23:05, Aaron Plattner  wrote:
>>> On 07/12/2016 04:31 PM, Emil Velikov wrote:
 Since xf86platformBus.h is part of the SDK, If we do this, then the
 new header must become one as well (should be listed in sdk_HEADERS).
 Alternatively we can forward declare XF86MatchedDrivers and include
 the header in EXTRA_DIST. Not sure if the latter is a good idea
 though, since the actual ABI will be undefined/private.

 Or better yet, neither of the two exported symbols
 (xf86PlatformDeviceCheckBusID, xf86PlatformMatchDriver) is used and
 imho we can remove them. Seems that the header is used solely for the
 ODEV management, which isn't platform devices specific and one can
 just move those parts into a separate header and use _it_ in the SDK ?

 But all that (everything but the sdk_HEADERS/EXTRA_DIST fix) is added
 bogus, which shouldn't stop the patch from landing.
>>> Another customer ran into this recently. Adam, can this be merged? I don't
>>> think Emil's reply was a nack.
>>
>> Precisely. My earlier message should have read:
>>
>> xf86MatchDrivers.h must be in the sdk_HEADERS or you'll need build
>> hacks in each driver. With that the patch is
>> Reviewed-by: Emil Velikov 
> 
> Fixed up (and rebased and made meson-aware) and merged:
> 
> To ssh://git.freedesktop.org/git/xorg/xserver
>1549e3037..112d0d7d0  master -> master

This broke things on my development box.

There are two GPUs. When I want Xorg to use the secondary GPU for its
primary screen, I use a xorg.conf with (effectively) only:

Section "Device"
Identifier "Device0"
BusID   "PCI:1:0:0"
EndSection

Before this change, this worked fine, automatically[0] using the amdgpu
driver for that GPU. With this change, Xorg fails to start up, see the
attached log file.


FWIW, if I comment out the BusID stanza, Xorg still starts up fine using
the radeon driver for the primary GPU.


[0] via /usr/local/share/X11/xorg.conf.d/10-amdgpu.conf, which contains

Section "OutputClass"
Identifier "AMDgpu"
MatchDriver "amdgpu"
Driver "amdgpu"
EndSection


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
[ 22240.601] 
This is a pre-release version of the X server from The X.Org Foundation.
It is not supported in any way.
Bugs may be filed in the bugzilla at http://bugs.freedesktop.org/.
Select the "xorg" product for bugs you find in this release.
Before reporting bugs in pre-release versions please check the
latest version in the X.Org Foundation git repository.
See http://wiki.x.org/wiki/GitPage for git access instructions.
[ 22240.604] 
X.Org X Server 1.19.99.1
Release Date: 2016-11-18
[ 22240.604] X Protocol Version 11, Revision 0
[ 22240.604] Build Operating System: Linux 4.10.13+ x86_64 
[ 22240.604] Current Operating System: Linux kaveri 4.10.13+ #254 SMP Mon May 1 11:13:49 JST 2017 x86_64
[ 22240.604] Kernel command line: BOOT_IMAGE=/boot/vmlinuz-4.10.13+ root=/dev/mapper/VG--Debian-LV--sid ro radeon.lockup_timeout=0 radeon.bapm=1 amdgpu.bapm=1 modprobe.blacklist=amdgpu quiet
[ 22240.604] Build Date: 01 May 2017  05:44:28PM
[ 22240.604]  
[ 22240.604] Current version of pixman: 0.35.1
[ 22240.604] 	Before reporting problems, check http://wiki.x.org
	to make sure that you have the latest version.
[ 22240.604] Markers: (--) probed, (**) from config file, (==) default setting,
	(++) from command line, (!!) notice, (II) informational,
	(WW) warning, (EE) error, (NI) not implemented, (??) unknown.
[ 22240.604] (==) Log file: "/usr/local/var/log/Xorg.1.log", Time: Mon May  1 17:51:38 2017
[ 22240.604] (==) Using config file: "/etc/X11/xorg.conf"
[ 22240.604] (==) Using system config directory "/usr/local/share/X11/xorg.conf.d"
[ 22240.604] (==) No Layout section.  Using the first Screen section.
[ 22240.604] (==) No screen section available. Using defaults.
[ 22240.604] (**) |-->Screen "Default Screen Section" (0)
[ 22240.604] (**) |   |-->Monitor ""
[ 22240.604] (==) No device specified for screen "Default Screen Section".
	Using the first device section listed.
[ 22240.604] (**) |   |-->Device "Device0"
[ 22240.604] (==) No monitor specified for screen "Default Screen Section".
	Using a default monitor configuration.
[ 22240.604] (**) Option "IndirectGLX"
[ 22240.604] (==) Automatically adding devices
[ 22240.604] (==) Automatically enabling devices
[ 22240.604] (==) Automatically adding GPU devices
[ 22240.604] (==) Max clients allowed: 256, resource mask: 0x1f
[ 22240.604] (WW) The directory "/usr/share/fonts/X11/TTF/" does not exist.
[ 22240.604] 	Entry deleted from font path.
[ 22240.604] (WW) The directory "/usr/share/fonts/X11/OTF/" does not exist.
[ 22240.604] 	Entry deleted from font path.
[

Re: [PATCH] Improved autoconfig drivers matching

2017-04-27 Thread Adam Jackson 
On Wed, 2017-04-12 at 23:42 +0100, Emil Velikov wrote:
> > On 12 April 2017 at 23:05, Aaron Plattner  wrote:
> > On 07/12/2016 04:31 PM, Emil Velikov wrote:
> > > Since xf86platformBus.h is part of the SDK, If we do this, then the
> > > new header must become one as well (should be listed in sdk_HEADERS).
> > > Alternatively we can forward declare XF86MatchedDrivers and include
> > > the header in EXTRA_DIST. Not sure if the latter is a good idea
> > > though, since the actual ABI will be undefined/private.
> > > 
> > > Or better yet, neither of the two exported symbols
> > > (xf86PlatformDeviceCheckBusID, xf86PlatformMatchDriver) is used and
> > > imho we can remove them. Seems that the header is used solely for the
> > > ODEV management, which isn't platform devices specific and one can
> > > just move those parts into a separate header and use _it_ in the SDK ?
> > > 
> > > But all that (everything but the sdk_HEADERS/EXTRA_DIST fix) is added
> > > bogus, which shouldn't stop the patch from landing.
> > Another customer ran into this recently. Adam, can this be merged? I don't
> > think Emil's reply was a nack.
> 
> Precisely. My earlier message should have read:
> 
> xf86MatchDrivers.h must be in the sdk_HEADERS or you'll need build
> hacks in each driver. With that the patch is
> Reviewed-by: Emil Velikov 

Fixed up (and rebased and made meson-aware) and merged:

To ssh://git.freedesktop.org/git/xorg/xserver
   1549e3037..112d0d7d0  master -> master

- ajax
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH] Improved autoconfig drivers matching

2017-04-12 Thread Emil Velikov 
On 12 April 2017 at 23:05, Aaron Plattner  wrote:
> On 07/12/2016 04:31 PM, Emil Velikov wrote:

>> Since xf86platformBus.h is part of the SDK, If we do this, then the
>> new header must become one as well (should be listed in sdk_HEADERS).
>> Alternatively we can forward declare XF86MatchedDrivers and include
>> the header in EXTRA_DIST. Not sure if the latter is a good idea
>> though, since the actual ABI will be undefined/private.
>>
>> Or better yet, neither of the two exported symbols
>> (xf86PlatformDeviceCheckBusID, xf86PlatformMatchDriver) is used and
>> imho we can remove them. Seems that the header is used solely for the
>> ODEV management, which isn't platform devices specific and one can
>> just move those parts into a separate header and use _it_ in the SDK ?
>>
>> But all that (everything but the sdk_HEADERS/EXTRA_DIST fix) is added
>> bogus, which shouldn't stop the patch from landing.

> Another customer ran into this recently. Adam, can this be merged? I don't
> think Emil's reply was a nack.

Precisely. My earlier message should have read:

xf86MatchDrivers.h must be in the sdk_HEADERS or you'll need build
hacks in each driver. With that the patch is
Reviewed-by: Emil Velikov 

Regards,
Emil
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH] Improved autoconfig drivers matching

2017-04-12 Thread Aaron Plattner 
Another customer ran into this recently. Adam, can this be merged? I 
don't think Emil's reply was a nack.


On 07/12/2016 04:31 PM, Emil Velikov wrote:

On 23 July 2015 at 00:42, Karol Kosik  wrote:

Implementation of new drivers matching algorithm. New approach
doesn't add duplicate drivers and ease drivers matching phase.

Signed-off-by: Karol Kosik 
---
  hw/xfree86/common/xf86AutoConfig.c   | 124 +++
  hw/xfree86/common/xf86MatchDrivers.h |  40 +++
  hw/xfree86/common/xf86pciBus.c   |  52 ++-
  hw/xfree86/common/xf86pciBus.h   |  13 ++--
  hw/xfree86/common/xf86platformBus.c  |  31 +++--
  hw/xfree86/common/xf86platformBus.h  |   5 +-
  6 files changed, 150 insertions(+), 115 deletions(-)
  create mode 100644 hw/xfree86/common/xf86MatchDrivers.h

diff --git a/hw/xfree86/common/xf86AutoConfig.c 
b/hw/xfree86/common/xf86AutoConfig.c
index 6b8d0eb..440434c 100644
--- a/hw/xfree86/common/xf86AutoConfig.c
+++ b/hw/xfree86/common/xf86AutoConfig.c
@@ -37,6 +37,7 @@
  #include "xf86Parser.h"
  #include "xf86tokens.h"
  #include "xf86Config.h"
+#include "xf86MatchDrivers.h"
  #include "xf86Priv.h"
  #include "xf86_OSlib.h"
  #include "xf86platformBus.h"
@@ -89,7 +90,7 @@
  static const char **builtinConfig = NULL;
  static int builtinLines = 0;

-static void listPossibleVideoDrivers(char *matches[], int nmatches);
+static void listPossibleVideoDrivers(XF86MatchedDrivers *md);

  /*
   * A built-in config file is stored as an array of strings, with each string
@@ -140,33 +141,58 @@ AppendToConfig(const char *s)
  AppendToList(s, , );
  }

+void
+xf86AddMatchedDriver(XF86MatchedDrivers *md, const char *driver)
+{
+int j;
+int nmatches = md->nmatches;
+
+for (j = 0; j < nmatches; ++j) {
+if (xf86NameCmp(md->matches[j], driver) == 0) {
+// Driver already in matched drivers
+return;
+}
+}
+
+if (nmatches < MATCH_DRIVERS_LIMIT) {
+md->matches[nmatches] = xnfstrdup(driver);
+md->nmatches++;
+}
+else {
+xf86Msg(X_WARNING, "Too many drivers registered, can't add %s\n", 
driver);
+}
+}
+
  Bool
  xf86AutoConfig(void)
  {
-char *deviceList[20];
-char **p;
+XF86MatchedDrivers md;
+int i;
  const char **cp;
  char buf[1024];
  ConfigStatus ret;

-listPossibleVideoDrivers(deviceList, 20);
+listPossibleVideoDrivers();

-for (p = deviceList; *p; p++) {
-snprintf(buf, sizeof(buf), BUILTIN_DEVICE_SECTION, *p, 0, *p);
+for (i = 0; i < md.nmatches; i++) {
+snprintf(buf, sizeof(buf), BUILTIN_DEVICE_SECTION,
+md.matches[i], 0, md.matches[i]);
  AppendToConfig(buf);
-snprintf(buf, sizeof(buf), BUILTIN_SCREEN_SECTION, *p, 0, *p, 0);
+snprintf(buf, sizeof(buf), BUILTIN_SCREEN_SECTION,
+md.matches[i], 0, md.matches[i], 0);
  AppendToConfig(buf);
  }

  AppendToConfig(BUILTIN_LAYOUT_SECTION_PRE);
-for (p = deviceList; *p; p++) {
-snprintf(buf, sizeof(buf), BUILTIN_LAYOUT_SCREEN_LINE, *p, 0);
+for (i = 0; i < md.nmatches; i++) {
+snprintf(buf, sizeof(buf), BUILTIN_LAYOUT_SCREEN_LINE,
+md.matches[i], 0);
  AppendToConfig(buf);
  }
  AppendToConfig(BUILTIN_LAYOUT_SECTION_POST);

-for (p = deviceList; *p; p++) {
-free(*p);
+for (i = 0; i < md.nmatches; i++) {
+free(md.matches[i]);
  }

  xf86MsgVerb(X_DEFAULT, 0,
@@ -190,22 +216,19 @@ xf86AutoConfig(void)
  }

  static void
-listPossibleVideoDrivers(char *matches[], int nmatches)
+listPossibleVideoDrivers(XF86MatchedDrivers *md)
  {
  int i;

-for (i = 0; i < nmatches; i++) {
-matches[i] = NULL;
-}
-i = 0;
+md->nmatches = 0;

  #ifdef XSERVER_PLATFORM_BUS
-i = xf86PlatformMatchDriver(matches, nmatches);
+xf86PlatformMatchDriver(md);
  #endif
  #ifdef sun
  /* Check for driver type based on /dev/fb type and if valid, use
 it instead of PCI bus probe results */
-if (xf86Info.consoleFd >= 0 && (i < (nmatches - 1))) {
+if (xf86Info.consoleFd >= 0) {
  struct vis_identifier visid;
  const char *cp;
  int iret;
@@ -231,7 +254,7 @@ listPossibleVideoDrivers(char *matches[], int nmatches)

  /* Special case from before the general case was set */
  if (strcmp(visid.name, "NVDAnvda") == 0) {
-matches[i++] = xnfstrdup("nvidia");
+xf86AddMatchedDriver(md, "nvidia");
  }

  /* General case - split into vendor name (initial all-caps
@@ -241,55 +264,48 @@ listPossibleVideoDrivers(char *matches[], int nmatches)
  /* find end of all uppercase vendor section */
  }
  if ((cp != visid.name) && (*cp != '\0')) {
-char *driverName = xnfstrdup(cp);

Re: [PATCH] Improved autoconfig drivers matching

2016-07-12 Thread Emil Velikov 
On 23 July 2015 at 00:42, Karol Kosik  wrote:
> Implementation of new drivers matching algorithm. New approach
> doesn't add duplicate drivers and ease drivers matching phase.
>
> Signed-off-by: Karol Kosik 
> ---
>  hw/xfree86/common/xf86AutoConfig.c   | 124 
> +++
>  hw/xfree86/common/xf86MatchDrivers.h |  40 +++
>  hw/xfree86/common/xf86pciBus.c   |  52 ++-
>  hw/xfree86/common/xf86pciBus.h   |  13 ++--
>  hw/xfree86/common/xf86platformBus.c  |  31 +++--
>  hw/xfree86/common/xf86platformBus.h  |   5 +-
>  6 files changed, 150 insertions(+), 115 deletions(-)
>  create mode 100644 hw/xfree86/common/xf86MatchDrivers.h
>
> diff --git a/hw/xfree86/common/xf86AutoConfig.c 
> b/hw/xfree86/common/xf86AutoConfig.c
> index 6b8d0eb..440434c 100644
> --- a/hw/xfree86/common/xf86AutoConfig.c
> +++ b/hw/xfree86/common/xf86AutoConfig.c
> @@ -37,6 +37,7 @@
>  #include "xf86Parser.h"
>  #include "xf86tokens.h"
>  #include "xf86Config.h"
> +#include "xf86MatchDrivers.h"
>  #include "xf86Priv.h"
>  #include "xf86_OSlib.h"
>  #include "xf86platformBus.h"
> @@ -89,7 +90,7 @@
>  static const char **builtinConfig = NULL;
>  static int builtinLines = 0;
>
> -static void listPossibleVideoDrivers(char *matches[], int nmatches);
> +static void listPossibleVideoDrivers(XF86MatchedDrivers *md);
>
>  /*
>   * A built-in config file is stored as an array of strings, with each string
> @@ -140,33 +141,58 @@ AppendToConfig(const char *s)
>  AppendToList(s, , );
>  }
>
> +void
> +xf86AddMatchedDriver(XF86MatchedDrivers *md, const char *driver)
> +{
> +int j;
> +int nmatches = md->nmatches;
> +
> +for (j = 0; j < nmatches; ++j) {
> +if (xf86NameCmp(md->matches[j], driver) == 0) {
> +// Driver already in matched drivers
> +return;
> +}
> +}
> +
> +if (nmatches < MATCH_DRIVERS_LIMIT) {
> +md->matches[nmatches] = xnfstrdup(driver);
> +md->nmatches++;
> +}
> +else {
> +xf86Msg(X_WARNING, "Too many drivers registered, can't add %s\n", 
> driver);
> +}
> +}
> +
>  Bool
>  xf86AutoConfig(void)
>  {
> -char *deviceList[20];
> -char **p;
> +XF86MatchedDrivers md;
> +int i;
>  const char **cp;
>  char buf[1024];
>  ConfigStatus ret;
>
> -listPossibleVideoDrivers(deviceList, 20);
> +listPossibleVideoDrivers();
>
> -for (p = deviceList; *p; p++) {
> -snprintf(buf, sizeof(buf), BUILTIN_DEVICE_SECTION, *p, 0, *p);
> +for (i = 0; i < md.nmatches; i++) {
> +snprintf(buf, sizeof(buf), BUILTIN_DEVICE_SECTION,
> +md.matches[i], 0, md.matches[i]);
>  AppendToConfig(buf);
> -snprintf(buf, sizeof(buf), BUILTIN_SCREEN_SECTION, *p, 0, *p, 0);
> +snprintf(buf, sizeof(buf), BUILTIN_SCREEN_SECTION,
> +md.matches[i], 0, md.matches[i], 0);
>  AppendToConfig(buf);
>  }
>
>  AppendToConfig(BUILTIN_LAYOUT_SECTION_PRE);
> -for (p = deviceList; *p; p++) {
> -snprintf(buf, sizeof(buf), BUILTIN_LAYOUT_SCREEN_LINE, *p, 0);
> +for (i = 0; i < md.nmatches; i++) {
> +snprintf(buf, sizeof(buf), BUILTIN_LAYOUT_SCREEN_LINE,
> +md.matches[i], 0);
>  AppendToConfig(buf);
>  }
>  AppendToConfig(BUILTIN_LAYOUT_SECTION_POST);
>
> -for (p = deviceList; *p; p++) {
> -free(*p);
> +for (i = 0; i < md.nmatches; i++) {
> +free(md.matches[i]);
>  }
>
>  xf86MsgVerb(X_DEFAULT, 0,
> @@ -190,22 +216,19 @@ xf86AutoConfig(void)
>  }
>
>  static void
> -listPossibleVideoDrivers(char *matches[], int nmatches)
> +listPossibleVideoDrivers(XF86MatchedDrivers *md)
>  {
>  int i;
>
> -for (i = 0; i < nmatches; i++) {
> -matches[i] = NULL;
> -}
> -i = 0;
> +md->nmatches = 0;
>
>  #ifdef XSERVER_PLATFORM_BUS
> -i = xf86PlatformMatchDriver(matches, nmatches);
> +xf86PlatformMatchDriver(md);
>  #endif
>  #ifdef sun
>  /* Check for driver type based on /dev/fb type and if valid, use
> it instead of PCI bus probe results */
> -if (xf86Info.consoleFd >= 0 && (i < (nmatches - 1))) {
> +if (xf86Info.consoleFd >= 0) {
>  struct vis_identifier visid;
>  const char *cp;
>  int iret;
> @@ -231,7 +254,7 @@ listPossibleVideoDrivers(char *matches[], int nmatches)
>
>  /* Special case from before the general case was set */
>  if (strcmp(visid.name, "NVDAnvda") == 0) {
> -matches[i++] = xnfstrdup("nvidia");
> +xf86AddMatchedDriver(md, "nvidia");
>  }
>
>  /* General case - split into vendor name (initial all-caps
> @@ -241,55 +264,48 @@ listPossibleVideoDrivers(char *matches[], int nmatches)
>  /* find end of all uppercase vendor section */
>  }
>  if ((cp != visid.name) && (*cp != '\0')) {
> -

Re: [PATCH] Improved autoconfig drivers matching

2015-07-28 Thread Aaron Plattner 
On 07/22/2015 04:42 PM, Karol Kosik wrote:
 Implementation of new drivers matching algorithm. New approach
 doesn't add duplicate drivers and ease drivers matching phase.
 
 Signed-off-by: Karol Kosik kko...@nvidia.com

Reviewed-by: Aaron Plattner aplatt...@nvidia.com

though it might be good for someone else to take a look too.

In case anyone was wondering why this patch helps, the problem is that
xf86PlatformMatchDriver() loops over every platform device, which means
that if you have a whole bunch of GPUs, it keeps adding the same drivers
over and over again, overflowing the 20-element deviceList[].

 ---
  hw/xfree86/common/xf86AutoConfig.c   | 124 
 +++
  hw/xfree86/common/xf86MatchDrivers.h |  40 +++
  hw/xfree86/common/xf86pciBus.c   |  52 ++-
  hw/xfree86/common/xf86pciBus.h   |  13 ++--
  hw/xfree86/common/xf86platformBus.c  |  31 +++--
  hw/xfree86/common/xf86platformBus.h  |   5 +-
  6 files changed, 150 insertions(+), 115 deletions(-)
  create mode 100644 hw/xfree86/common/xf86MatchDrivers.h
 
 diff --git a/hw/xfree86/common/xf86AutoConfig.c 
 b/hw/xfree86/common/xf86AutoConfig.c
 index 6b8d0eb..440434c 100644
 --- a/hw/xfree86/common/xf86AutoConfig.c
 +++ b/hw/xfree86/common/xf86AutoConfig.c
 @@ -37,6 +37,7 @@
  #include xf86Parser.h
  #include xf86tokens.h
  #include xf86Config.h
 +#include xf86MatchDrivers.h
  #include xf86Priv.h
  #include xf86_OSlib.h
  #include xf86platformBus.h
 @@ -89,7 +90,7 @@
  static const char **builtinConfig = NULL;
  static int builtinLines = 0;
  
 -static void listPossibleVideoDrivers(char *matches[], int nmatches);
 +static void listPossibleVideoDrivers(XF86MatchedDrivers *md);
  
  /*
   * A built-in config file is stored as an array of strings, with each string
 @@ -140,33 +141,58 @@ AppendToConfig(const char *s)
  AppendToList(s, builtinConfig, builtinLines);
  }
  
 +void
 +xf86AddMatchedDriver(XF86MatchedDrivers *md, const char *driver)
 +{
 +int j;
 +int nmatches = md-nmatches;
 +
 +for (j = 0; j  nmatches; ++j) {
 +if (xf86NameCmp(md-matches[j], driver) == 0) {
 +// Driver already in matched drivers
 +return;
 +}
 +}
 +
 +if (nmatches  MATCH_DRIVERS_LIMIT) {
 +md-matches[nmatches] = xnfstrdup(driver);
 +md-nmatches++;
 +}
 +else {
 +xf86Msg(X_WARNING, Too many drivers registered, can't add %s\n, 
 driver);
 +}
 +}
 +
  Bool
  xf86AutoConfig(void)
  {
 -char *deviceList[20];
 -char **p;
 +XF86MatchedDrivers md;
 +int i;
  const char **cp;
  char buf[1024];
  ConfigStatus ret;
  
 -listPossibleVideoDrivers(deviceList, 20);
 +listPossibleVideoDrivers(md);
  
 -for (p = deviceList; *p; p++) {
 -snprintf(buf, sizeof(buf), BUILTIN_DEVICE_SECTION, *p, 0, *p);
 +for (i = 0; i  md.nmatches; i++) {
 +snprintf(buf, sizeof(buf), BUILTIN_DEVICE_SECTION,
 +md.matches[i], 0, md.matches[i]);
  AppendToConfig(buf);
 -snprintf(buf, sizeof(buf), BUILTIN_SCREEN_SECTION, *p, 0, *p, 0);
 +snprintf(buf, sizeof(buf), BUILTIN_SCREEN_SECTION,
 +md.matches[i], 0, md.matches[i], 0);
  AppendToConfig(buf);
  }
  
  AppendToConfig(BUILTIN_LAYOUT_SECTION_PRE);
 -for (p = deviceList; *p; p++) {
 -snprintf(buf, sizeof(buf), BUILTIN_LAYOUT_SCREEN_LINE, *p, 0);
 +for (i = 0; i  md.nmatches; i++) {
 +snprintf(buf, sizeof(buf), BUILTIN_LAYOUT_SCREEN_LINE,
 +md.matches[i], 0);
  AppendToConfig(buf);
  }
  AppendToConfig(BUILTIN_LAYOUT_SECTION_POST);
  
 -for (p = deviceList; *p; p++) {
 -free(*p);
 +for (i = 0; i  md.nmatches; i++) {
 +free(md.matches[i]);
  }
  
  xf86MsgVerb(X_DEFAULT, 0,
 @@ -190,22 +216,19 @@ xf86AutoConfig(void)
  }
  
  static void
 -listPossibleVideoDrivers(char *matches[], int nmatches)
 +listPossibleVideoDrivers(XF86MatchedDrivers *md)
  {
  int i;
  
 -for (i = 0; i  nmatches; i++) {
 -matches[i] = NULL;
 -}
 -i = 0;
 +md-nmatches = 0;
  
  #ifdef XSERVER_PLATFORM_BUS
 -i = xf86PlatformMatchDriver(matches, nmatches);
 +xf86PlatformMatchDriver(md);
  #endif
  #ifdef sun
  /* Check for driver type based on /dev/fb type and if valid, use
 it instead of PCI bus probe results */
 -if (xf86Info.consoleFd = 0  (i  (nmatches - 1))) {
 +if (xf86Info.consoleFd = 0) {
  struct vis_identifier visid;
  const char *cp;
  int iret;
 @@ -231,7 +254,7 @@ listPossibleVideoDrivers(char *matches[], int nmatches)
  
  /* Special case from before the general case was set */
  if (strcmp(visid.name, NVDAnvda) == 0) {
 -matches[i++] = xnfstrdup(nvidia);
 +xf86AddMatchedDriver(md, nvidia);
  }
  
  /* General case - split into vendor name (initial

[PATCH] Improved autoconfig drivers matching

2015-07-22 Thread Karol Kosik 
Implementation of new drivers matching algorithm. New approach
doesn't add duplicate drivers and ease drivers matching phase.

Signed-off-by: Karol Kosik kko...@nvidia.com
---
 hw/xfree86/common/xf86AutoConfig.c   | 124 +++
 hw/xfree86/common/xf86MatchDrivers.h |  40 +++
 hw/xfree86/common/xf86pciBus.c   |  52 ++-
 hw/xfree86/common/xf86pciBus.h   |  13 ++--
 hw/xfree86/common/xf86platformBus.c  |  31 +++--
 hw/xfree86/common/xf86platformBus.h  |   5 +-
 6 files changed, 150 insertions(+), 115 deletions(-)
 create mode 100644 hw/xfree86/common/xf86MatchDrivers.h

diff --git a/hw/xfree86/common/xf86AutoConfig.c 
b/hw/xfree86/common/xf86AutoConfig.c
index 6b8d0eb..440434c 100644
--- a/hw/xfree86/common/xf86AutoConfig.c
+++ b/hw/xfree86/common/xf86AutoConfig.c
@@ -37,6 +37,7 @@
 #include xf86Parser.h
 #include xf86tokens.h
 #include xf86Config.h
+#include xf86MatchDrivers.h
 #include xf86Priv.h
 #include xf86_OSlib.h
 #include xf86platformBus.h
@@ -89,7 +90,7 @@
 static const char **builtinConfig = NULL;
 static int builtinLines = 0;
 
-static void listPossibleVideoDrivers(char *matches[], int nmatches);
+static void listPossibleVideoDrivers(XF86MatchedDrivers *md);
 
 /*
  * A built-in config file is stored as an array of strings, with each string
@@ -140,33 +141,58 @@ AppendToConfig(const char *s)
 AppendToList(s, builtinConfig, builtinLines);
 }
 
+void
+xf86AddMatchedDriver(XF86MatchedDrivers *md, const char *driver)
+{
+int j;
+int nmatches = md-nmatches;
+
+for (j = 0; j  nmatches; ++j) {
+if (xf86NameCmp(md-matches[j], driver) == 0) {
+// Driver already in matched drivers
+return;
+}
+}
+
+if (nmatches  MATCH_DRIVERS_LIMIT) {
+md-matches[nmatches] = xnfstrdup(driver);
+md-nmatches++;
+}
+else {
+xf86Msg(X_WARNING, Too many drivers registered, can't add %s\n, 
driver);
+}
+}
+
 Bool
 xf86AutoConfig(void)
 {
-char *deviceList[20];
-char **p;
+XF86MatchedDrivers md;
+int i;
 const char **cp;
 char buf[1024];
 ConfigStatus ret;
 
-listPossibleVideoDrivers(deviceList, 20);
+listPossibleVideoDrivers(md);
 
-for (p = deviceList; *p; p++) {
-snprintf(buf, sizeof(buf), BUILTIN_DEVICE_SECTION, *p, 0, *p);
+for (i = 0; i  md.nmatches; i++) {
+snprintf(buf, sizeof(buf), BUILTIN_DEVICE_SECTION,
+md.matches[i], 0, md.matches[i]);
 AppendToConfig(buf);
-snprintf(buf, sizeof(buf), BUILTIN_SCREEN_SECTION, *p, 0, *p, 0);
+snprintf(buf, sizeof(buf), BUILTIN_SCREEN_SECTION,
+md.matches[i], 0, md.matches[i], 0);
 AppendToConfig(buf);
 }
 
 AppendToConfig(BUILTIN_LAYOUT_SECTION_PRE);
-for (p = deviceList; *p; p++) {
-snprintf(buf, sizeof(buf), BUILTIN_LAYOUT_SCREEN_LINE, *p, 0);
+for (i = 0; i  md.nmatches; i++) {
+snprintf(buf, sizeof(buf), BUILTIN_LAYOUT_SCREEN_LINE,
+md.matches[i], 0);
 AppendToConfig(buf);
 }
 AppendToConfig(BUILTIN_LAYOUT_SECTION_POST);
 
-for (p = deviceList; *p; p++) {
-free(*p);
+for (i = 0; i  md.nmatches; i++) {
+free(md.matches[i]);
 }
 
 xf86MsgVerb(X_DEFAULT, 0,
@@ -190,22 +216,19 @@ xf86AutoConfig(void)
 }
 
 static void
-listPossibleVideoDrivers(char *matches[], int nmatches)
+listPossibleVideoDrivers(XF86MatchedDrivers *md)
 {
 int i;
 
-for (i = 0; i  nmatches; i++) {
-matches[i] = NULL;
-}
-i = 0;
+md-nmatches = 0;
 
 #ifdef XSERVER_PLATFORM_BUS
-i = xf86PlatformMatchDriver(matches, nmatches);
+xf86PlatformMatchDriver(md);
 #endif
 #ifdef sun
 /* Check for driver type based on /dev/fb type and if valid, use
it instead of PCI bus probe results */
-if (xf86Info.consoleFd = 0  (i  (nmatches - 1))) {
+if (xf86Info.consoleFd = 0) {
 struct vis_identifier visid;
 const char *cp;
 int iret;
@@ -231,7 +254,7 @@ listPossibleVideoDrivers(char *matches[], int nmatches)
 
 /* Special case from before the general case was set */
 if (strcmp(visid.name, NVDAnvda) == 0) {
-matches[i++] = xnfstrdup(nvidia);
+xf86AddMatchedDriver(md, nvidia);
 }
 
 /* General case - split into vendor name (initial all-caps
@@ -241,55 +264,48 @@ listPossibleVideoDrivers(char *matches[], int nmatches)
 /* find end of all uppercase vendor section */
 }
 if ((cp != visid.name)  (*cp != '\0')) {
-char *driverName = xnfstrdup(cp);
 char *vendorName = xnfstrdup(visid.name);
 
 vendorName[cp - visid.name] = '\0';
 
-matches[i++] = vendorName;
-matches[i++] = driverName;
+xf86AddMatchedDriver(md, vendorName);
+