Bugfix + dri1 cleanup patches

Bugfix + dri1 cleanup patches

[Emil Velikov]

Had those patches living in my local tree for a while now.
Here is a respin on top of latest master

Patch 1: regression fix,  preventing the ddx from loading if kernel
module is not loaded

Patches 2-5: Completely nuke dri1, make dri2 hard dependency

Patches 6-7: Assist people with first-time build of nouveau

Git complains about whitespace errors in patch 7, which for the sake
of me I cannot see

--

[PATCH 1/7] nouveau: Check if the device supports modesetting, after
opening it

[PATCH 2/7] nouveau: stop using dri1 function DRICreatePCIBusID
[PATCH 3/7] nouveau: Do not load dri {sub,}module
[PATCH 4/7] dri1: purge the final references
[PATCH 5/7] nouveau: mandate dri2 build

[PATCH 6/7] configure: require xorg-macros 1.8
[PATCH 7/7] configure: printout the configuration info

--

 configure.ac       | 33 ++++++++++++++++++++++++++++++++-
 src/nouveau_dri2.c | 16 ++--------------
 src/nv_driver.c    | 46 +++++++++++++++++-----------------------------
 src/nv_type.h      |  8 --------
 4 files changed, 51 insertions(+), 52 deletions(-)

[PATCH 1/7] nouveau: Check if the device supports modesetting, after opening it

[Emil Velikov]

"Regression" caused by
commit e34cfbd5bd23f7f15372af52d8a39a5715ce7310
Author: Emil Velikov <emil.l.velikov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Date:   Fri Nov 2 03:57:41 2012 +0000

    nouveau: Factor out common code to NVHasKMS()

    As the name suggests checks if it has kernel mode setting,
    prints out the interface version and checkes if the chipset
    is supported

    Function is used in NVPciProbe and NVPlatformProbe

Without this change X will fail with '[drm] KMS not enabled' if
the kernel module is not loaded

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=60772
Signed-off-by: Emil Velikov <emil.l.velikov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 src/nv_driver.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/src/nv_driver.c b/src/nv_driver.c
index 9f62fe2..b1410f5 100644
--- a/src/nv_driver.c
+++ b/src/nv_driver.c
@@ -260,17 +260,10 @@ NVHasKMS(struct pci_device *pci_dev)
 	}
 	busid = DRICreatePCIBusID(pci_dev);
 
-	ret = drmCheckModesettingSupported(busid);
-	if (ret) {
-		xf86DrvMsg(-1, X_ERROR, "[drm] KMS not enabled\n");
-		free(busid);
-		return FALSE;
-	}
-
 	ret = nouveau_device_open(busid, &dev);
-	free(busid);
 	if (ret) {
 		xf86DrvMsg(-1, X_ERROR, "[drm] failed to open device\n");
+		free(busid);
 		return FALSE;
 	}
 
@@ -288,6 +281,12 @@ NVHasKMS(struct pci_device *pci_dev)
 	chipset = dev->chipset;
 	nouveau_device_del(&dev);
 
+	ret = drmCheckModesettingSupported(busid);
+	free(busid);
+	if (ret) {
+		xf86DrvMsg(-1, X_ERROR, "[drm] KMS not enabled\n");
+		return FALSE;
+	}
 
 	switch (chipset & 0xf0) {
 	case 0x00:
-- 
1.8.1.3

Re: [PATCH 1/7] nouveau: Check if the device supports modesetting, after opening it

[Dave Airlie]

On Sun, Feb 17, 2013 at 6:48 AM, Emil Velikov <emil.l.velikov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> "Regression" caused by
> commit e34cfbd5bd23f7f15372af52d8a39a5715ce7310
> Author: Emil Velikov <emil.l.velikov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Date:   Fri Nov 2 03:57:41 2012 +0000
>
>     nouveau: Factor out common code to NVHasKMS()
>
>     As the name suggests checks if it has kernel mode setting,
>     prints out the interface version and checkes if the chipset
>     is supported
>
>     Function is used in NVPciProbe and NVPlatformProbe
>
> Without this change X will fail with '[drm] KMS not enabled' if
> the kernel module is not loaded

Thats correct, the X server is not responsible for loading the kernel module.

If the kernel module isn't loaded then there is a bug elsewhere.

Dave.

Re: [PATCH 1/7] nouveau: Check if the device supports modesetting, after opening it

[Emil Velikov]

On 18/02/13 00:47, Dave Airlie wrote:
...

> Thats correct, the X server is not responsible for loading the kernel module.
> 
> If the kernel module isn't loaded then there is a bug elsewhere.
> 
> Dave.
> 

Same feeling in here as well. Although that does not stop people's
workflow habits [1]


Emil
[1] http://xkcd.com/1172/

Re: [PATCH 1/7] nouveau: Check if the device supports modesetting, after opening it

[Dave Airlie]

On Mon, Feb 18, 2013 at 6:44 PM, Emil Velikov <emil.l.velikov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On 18/02/13 00:47, Dave Airlie wrote:
> ...
>
>> Thats correct, the X server is not responsible for loading the kernel module.
>>
>> If the kernel module isn't loaded then there is a bug elsewhere.
>>
>> Dave.
>>
>
> Same feeling in here as well. Although that does not stop people's
> workflow habits [1]

Not supported, means not supported, won't fix.

The OS loads the kernel modules not the X server, pre-KMS things were
different, but with KMS this is how it works. So yeah totally nak on this.

(xkcd never said you should care about people's workflows).

Dave.

[PATCHv2] nouveau: call drmCheckModesettingSupported after opening the device

[Emil Velikov]

If the kernel module is not yet loaded X will fail to load with
"[drm] KMS not enabled". Some of the cases which may lead to this are

* using hotplug
* blacklisted nouveau

The issue was caused by commit

    commit e34cfbd5bd23f7f15372af52d8a39a5715ce7310
    Author: Emil Velikov <emil.l.velikov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
    Date:   Fri Nov 2 03:57:41 2012 +0000

        nouveau: Factor out common code to NVHasKMS()

        As the name suggests checks if it has kernel mode setting,
        prints out the interface version and checkes if the chipset
        is supported

        Function is used in NVPciProbe and NVPlatformProbe

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=60772
Signed-off-by: Emil Velikov <emil.l.velikov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
v2: More sensible commit message

 src/nv_driver.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/src/nv_driver.c b/src/nv_driver.c
index 9f62fe2..b1410f5 100644
--- a/src/nv_driver.c
+++ b/src/nv_driver.c
@@ -260,17 +260,10 @@ NVHasKMS(struct pci_device *pci_dev)
 	}
 	busid = DRICreatePCIBusID(pci_dev);
 
-	ret = drmCheckModesettingSupported(busid);
-	if (ret) {
-		xf86DrvMsg(-1, X_ERROR, "[drm] KMS not enabled\n");
-		free(busid);
-		return FALSE;
-	}
-
 	ret = nouveau_device_open(busid, &dev);
-	free(busid);
 	if (ret) {
 		xf86DrvMsg(-1, X_ERROR, "[drm] failed to open device\n");
+		free(busid);
 		return FALSE;
 	}
 
@@ -288,6 +281,12 @@ NVHasKMS(struct pci_device *pci_dev)
 	chipset = dev->chipset;
 	nouveau_device_del(&dev);
 
+	ret = drmCheckModesettingSupported(busid);
+	free(busid);
+	if (ret) {
+		xf86DrvMsg(-1, X_ERROR, "[drm] KMS not enabled\n");
+		return FALSE;
+	}
 
 	switch (chipset & 0xf0) {
 	case 0x00:
-- 
1.8.1.3

Re: [PATCHv2] nouveau: call drmCheckModesettingSupported after opening the device

[Dave Airlie]

On Tue, Feb 19, 2013 at 8:14 AM, Emil Velikov <emil.l.velikov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> If the kernel module is not yet loaded X will fail to load with
> "[drm] KMS not enabled". Some of the cases which may lead to this are
>
> * using hotplug
> * blacklisted nouveau

If you blacklist it, its not up to the server to load it, the whole
point of the blacklist to not load the modules.

I've no idea what hotplug does wrong, but it should load the drivers
at bootup using pci ids like say udev does.

NAK.

Dave.

[PATCH 2/7] nouveau: stop using dri1 function DRICreatePCIBusID

[Emil Velikov]

Step 1 to completely rip out dri1 out of nouveau

Signed-off-by: Emil Velikov <emil.l.velikov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 src/nv_driver.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/src/nv_driver.c b/src/nv_driver.c
index b1410f5..2b2f698 100644
--- a/src/nv_driver.c
+++ b/src/nv_driver.c
@@ -254,11 +254,13 @@ NVHasKMS(struct pci_device *pci_dev)
 	char *busid;
 	int chipset, ret;
 
-	if (!xf86LoaderCheckSymbol("DRICreatePCIBusID")) {
-		xf86DrvMsg(-1, X_ERROR, "[drm] No DRICreatePCIBusID symbol\n");
-		return FALSE;
-	}
-	busid = DRICreatePCIBusID(pci_dev);
+#if XORG_VERSION_CURRENT >= XORG_VERSION_NUMERIC(1,9,99,901,0)
+	XNFasprintf(&busid, "pci:%04x:%02x:%02x.%d",
+		    pci_dev->domain, pci_dev->bus, pci_dev->dev, pci_dev->func);
+#else
+	busid = XNFprintf("pci:%04x:%02x:%02x.%d",
+			  pci_dev->domain, pci_dev->bus, pci_dev->dev, pci_dev->func);
+#endif
 
 	ret = nouveau_device_open(busid, &dev);
 	if (ret) {
-- 
1.8.1.3

[PATCH 3/7] nouveau: Do not load dri {sub,}module

[Emil Velikov]

Nouveau does not use dri1, thus loading XFree86-DRI is not needed
On the other hand, we do use dri2

As a side effect, purge the 'set-but-unused' variable pLibDRMVersion

Signed-off-by: Emil Velikov <emil.l.velikov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 src/nv_driver.c | 19 +++----------------
 src/nv_type.h   |  2 --
 2 files changed, 3 insertions(+), 18 deletions(-)

diff --git a/src/nv_driver.c b/src/nv_driver.c
index 2b2f698..f10da00 100644
--- a/src/nv_driver.c
+++ b/src/nv_driver.c
@@ -633,26 +633,13 @@ static Bool
 NVDRIGetVersion(ScrnInfoPtr pScrn)
 {
 	NVPtr pNv = NVPTR(pScrn);
-	int errmaj, errmin;
-	pointer ret;
 
-	ret = LoadSubModule(pScrn->module, "dri", NULL, NULL, NULL,
-			    NULL, &errmaj, &errmin);
-	if (!ret) {
-		xf86DrvMsg(pScrn->scrnIndex, X_INFO,
-				"error %d\n", errmaj);
-		LoaderErrorMsg(pScrn->name, "dri", errmaj, errmin);
-	}
-
-	if (!ret && errmaj != LDR_ONCEONLY)
+	if (!xf86LoadSubModule(pScrn, "dri2"))
 		return FALSE;
 
-	xf86DrvMsg(pScrn->scrnIndex, X_INFO, "Loaded DRI module\n");
-
 	/* Check the lib version */
-	if (xf86LoaderCheckSymbol("drmGetLibVersion"))
-		pNv->pLibDRMVersion = drmGetLibVersion(0);
-	if (pNv->pLibDRMVersion == NULL) {
+	if ((xf86LoaderCheckSymbol("drmGetLibVersion")) &&
+	    (drmGetLibVersion(pNv->dev->fd) == NULL)) {
 		xf86DrvMsg(pScrn->scrnIndex, X_ERROR,
 		"NVDRIGetVersion failed because libDRM is really "
 		"way to old to even get a version number out of it.\n"
diff --git a/src/nv_type.h b/src/nv_type.h
index ed21c6d..5963cb6 100644
--- a/src/nv_type.h
+++ b/src/nv_type.h
@@ -69,8 +69,6 @@ typedef struct _NVRec {
 
     CARD32              currentRop;
 
-    drmVersionPtr       pLibDRMVersion;
-
 	void *drmmode; /* for KMS */
 
 	/* DRM interface */
-- 
1.8.1.3

[PATCH 4/7] dri1: purge the final references

[Emil Velikov]

Signed-off-by: Emil Velikov <emil.l.velikov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 configure.ac  | 2 +-
 src/nv_type.h | 6 ------
 2 files changed, 1 insertion(+), 7 deletions(-)

diff --git a/configure.ac b/configure.ac
index 7535bfe..302bd2b 100644
--- a/configure.ac
+++ b/configure.ac
@@ -71,7 +71,7 @@ PKG_CHECK_MODULES(LIBDRM_NOUVEAU, [libdrm_nouveau >= 2.4.25])
 AC_SUBST(LIBDRM_NOUVEAU_CFLAGS)
 AC_SUBST(LIBDRM_NOUVEAU_LIBS)
 
-PKG_CHECK_MODULES(XORG, [xorg-server >= 1.8] xproto fontsproto libdrm xf86driproto $REQUIRED_MODULES)
+PKG_CHECK_MODULES(XORG, [xorg-server >= 1.8] xproto fontsproto libdrm $REQUIRED_MODULES)
 PKG_CHECK_MODULES(XEXT, [xextproto >= 7.0.99.1],
 		  HAVE_XEXTPROTO_71="yes"; AC_DEFINE(HAVE_XEXTPROTO_71, 1, [xextproto 7.1 available]),
 		  HAVE_XEXTPROTO_71="no")
diff --git a/src/nv_type.h b/src/nv_type.h
index 5963cb6..fc67832 100644
--- a/src/nv_type.h
+++ b/src/nv_type.h
@@ -4,16 +4,10 @@
 #include "colormapst.h"
 #include "xf86Cursor.h"
 #include "exa.h"
-#ifdef XF86DRI
-#define _XF86DRI_SERVER_
 #include "xf86drm.h"
-#include "dri.h"
 #include <stdbool.h>
 #include <stdint.h>
 #include "xf86Crtc.h"
-#else
-#error "This driver requires a DRI-enabled X server"
-#endif
 
 #if XF86_CRTC_VERSION >= 5
 #define NOUVEAU_PIXMAP_SHARING 1
-- 
1.8.1.3

[PATCH 5/7] nouveau: mandate dri2 build

[Emil Velikov]

Building nouveau without dri2 is just silly

Signed-off-by: Emil Velikov <emil.l.velikov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 configure.ac       |  1 +
 src/nouveau_dri2.c | 16 ++--------------
 2 files changed, 3 insertions(+), 14 deletions(-)

diff --git a/configure.ac b/configure.ac
index 302bd2b..ad5a2ea 100644
--- a/configure.ac
+++ b/configure.ac
@@ -72,6 +72,7 @@ AC_SUBST(LIBDRM_NOUVEAU_CFLAGS)
 AC_SUBST(LIBDRM_NOUVEAU_LIBS)
 
 PKG_CHECK_MODULES(XORG, [xorg-server >= 1.8] xproto fontsproto libdrm $REQUIRED_MODULES)
+PKG_CHECK_MODULES(DRI2, [dri2proto >= 2.6])
 PKG_CHECK_MODULES(XEXT, [xextproto >= 7.0.99.1],
 		  HAVE_XEXTPROTO_71="yes"; AC_DEFINE(HAVE_XEXTPROTO_71, 1, [xextproto 7.1 available]),
 		  HAVE_XEXTPROTO_71="no")
diff --git a/src/nouveau_dri2.c b/src/nouveau_dri2.c
index e462a86..9e09038 100644
--- a/src/nouveau_dri2.c
+++ b/src/nouveau_dri2.c
@@ -7,9 +7,10 @@
 #include "nv_include.h"
 #ifdef DRI2
 #include "dri2.h"
+#else
+#error "This driver requires a DRI2-enabled X server"
 #endif
 
-#if defined(DRI2) && DRI2INFOREC_VERSION >= 3
 struct nouveau_dri2_buffer {
 	DRI2BufferRec base;
 	PixmapPtr ppix;
@@ -814,16 +815,3 @@ nouveau_dri2_fini(ScreenPtr pScreen)
 {
 	DRI2CloseScreen(pScreen);
 }
-#else
-Bool
-nouveau_dri2_init(ScreenPtr pScreen)
-{
-	return TRUE;
-}
-
-void
-nouveau_dri2_fini(ScreenPtr pScreen)
-{
-}
-#endif
-
-- 
1.8.1.3

[PATCH 6/7] configure: require xorg-macros 1.8

[Emil Velikov]

Explicitly check for xorg-macros 1.8, as a subtle hint
to update your build deps if building your own ddx

Signed-off-by: Emil Velikov <emil.l.velikov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 configure.ac | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/configure.ac b/configure.ac
index ad5a2ea..433131d 100644
--- a/configure.ac
+++ b/configure.ac
@@ -47,6 +47,18 @@ AM_INIT_AUTOMAKE([dist-bzip2])
 
 m4_ifdef([AM_SILENT_RULES], [AM_SILENT_RULES([yes])])
 
+# Require X.Org macros 1.8 or later for MAN_SUBSTS set by XORG_MANPAGE_SECTIONS
+m4_ifndef([XORG_MACROS_VERSION],
+          [m4_fatal([must install xorg-macros 1.8 or later before running autoconf/autogen])])
+
+XORG_MACROS_VERSION(1.8)
+XORG_DEFAULT_OPTIONS
+
+# Require X.Org server macros (i.e. XORG_DRIVER_CHECK_EXT) to check for required modules 
+m4_ifndef([XORG_DRIVER_CHECK_EXT],
+          [m4_fatal([must install xorg-server macros before running autoconf/autogen])])
+
+
 # Checks for programs.
 AC_DISABLE_STATIC
 LT_INIT
-- 
1.8.1.3

[PATCH 7/7] configure: printout the configuration info

[Emil Velikov]

Let the bikeshedding begin

Signed-off-by: Emil Velikov <emil.l.velikov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 configure.ac | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/configure.ac b/configure.ac
index 433131d..50219e9 100644
--- a/configure.ac
+++ b/configure.ac
@@ -132,3 +132,21 @@ AC_CONFIG_FILES([
 	man/Makefile
 ])
 AC_OUTPUT
+
+dnl
+dnl Output some configuration info for the user
+dnl
+echo ""
+echo "        prefix:              $prefix"
+echo "        exec_prefix:         $exec_prefix"
+echo "        libdir:              $libdir"
+echo "        includedir:          $includedir"
+
+echo ""
+echo "        CFLAGS:              $CFLAGS"
+echo "        CXXFLAGS:            $CXXFLAGS"
+echo "        Macros:              $DEFINES"
+
+echo ""
+echo "        Run '${MAKE-make}' to build xf86-video-nouveau"
+echo ""
-- 
1.8.1.3