2009-03-19 04:11:32

by Greg KH

[permalink] [raw]
Subject: [patch 0/5] Intel Poulsbo/Morrestown DRM driver and DRM core changes

Hi,

Here's 5 patches that add the Intel Poulsbo/Morrestown DRM driver to the
kernel tree.

There are 4 patches that make changes to the DRM core, and one patch
that adds the DRM driver itself. The driver is added to the
drivers/staging/ directory because it is not the "final" driver that
Intel wishes to support over time. The userspace api is going to
change, and work is currently underway to hook up properly with the
memory management system.

However this work is going to take a while, and in the meantime, users
want to run Linux on this kind of hardware. I'd really like to add the
driver to the staging tree, but it needs these core DRM changes in order
to get it to work properly.

Originally I had a patch that basically duplicated the existing DRM
core, and embedded it with these changes and the PSB driver together
into one big mess of a kernel module. But Richard convinced me that
this wasn't the "nicest" thing to do, and he did work on the PSB code
and dug out these older DRM patches.

The only thing that looks a bit "odd" to me is the unlocked ioctl patch,
Thomas, is that thing really correct?

David, I'd be glad to take the DRM changes through the staging tree, but
only if you ack them.

thanks,

greg k-h


2009-03-19 04:11:51

by Greg KH

[permalink] [raw]
Subject: [patch 1/5] drm: Split out the mm declarations in a separate header. Add atomic operations.

[coding style issues fixed by gregkh]

Signed-off-by: Thomas Hellstrom <[email protected]>
Signed-off-by: Richard Purdie <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>

---
drivers/gpu/drm/drm_mm.c | 165 +++++++++++++++++++++++++++++++++++++++--------
include/drm/drmP.h | 37 ----------
include/drm/drm_mm.h | 90 +++++++++++++++++++++++++
3 files changed, 228 insertions(+), 64 deletions(-)

--- a/drivers/gpu/drm/drm_mm.c
+++ b/drivers/gpu/drm/drm_mm.c
@@ -42,8 +42,11 @@
*/

#include "drmP.h"
+#include "drm_mm.h"
#include <linux/slab.h>

+#define MM_UNUSED_TARGET 4
+
unsigned long drm_mm_tail_space(struct drm_mm *mm)
{
struct list_head *tail_node;
@@ -74,16 +77,62 @@ int drm_mm_remove_space_from_tail(struct
return 0;
}

+static struct drm_mm_node *drm_mm_kmalloc(struct drm_mm *mm, int atomic)
+{
+ struct drm_mm_node *child;
+
+ if (atomic)
+ child = kmalloc(sizeof(*child), GFP_ATOMIC);
+ else
+ child = kmalloc(sizeof(*child), GFP_KERNEL);
+
+ if (unlikely(child == NULL)) {
+ spin_lock(&mm->unused_lock);
+ if (list_empty(&mm->unused_nodes))
+ child = NULL;
+ else {
+ child =
+ list_entry(mm->unused_nodes.next,
+ struct drm_mm_node, fl_entry);
+ list_del(&child->fl_entry);
+ --mm->num_unused;
+ }
+ spin_unlock(&mm->unused_lock);
+ }
+ return child;
+}
+
+int drm_mm_pre_get(struct drm_mm *mm)
+{
+ struct drm_mm_node *node;
+
+ spin_lock(&mm->unused_lock);
+ while (mm->num_unused < MM_UNUSED_TARGET) {
+ spin_unlock(&mm->unused_lock);
+ node = kmalloc(sizeof(*node), GFP_KERNEL);
+ spin_lock(&mm->unused_lock);
+
+ if (unlikely(node == NULL)) {
+ int ret = (mm->num_unused < 2) ? -ENOMEM : 0;
+ spin_unlock(&mm->unused_lock);
+ return ret;
+ }
+ ++mm->num_unused;
+ list_add_tail(&node->fl_entry, &mm->unused_nodes);
+ }
+ spin_unlock(&mm->unused_lock);
+ return 0;
+}
+EXPORT_SYMBOL(drm_mm_pre_get);

static int drm_mm_create_tail_node(struct drm_mm *mm,
- unsigned long start,
- unsigned long size)
+ unsigned long start,
+ unsigned long size, int atomic)
{
struct drm_mm_node *child;

- child = (struct drm_mm_node *)
- drm_alloc(sizeof(*child), DRM_MEM_MM);
- if (!child)
+ child = drm_mm_kmalloc(mm, atomic);
+ if (unlikely(child == NULL))
return -ENOMEM;

child->free = 1;
@@ -97,8 +146,7 @@ static int drm_mm_create_tail_node(struc
return 0;
}

-
-int drm_mm_add_space_to_tail(struct drm_mm *mm, unsigned long size)
+int drm_mm_add_space_to_tail(struct drm_mm *mm, unsigned long size, int atomic)
{
struct list_head *tail_node;
struct drm_mm_node *entry;
@@ -106,20 +154,21 @@ int drm_mm_add_space_to_tail(struct drm_
tail_node = mm->ml_entry.prev;
entry = list_entry(tail_node, struct drm_mm_node, ml_entry);
if (!entry->free) {
- return drm_mm_create_tail_node(mm, entry->start + entry->size, size);
+ return drm_mm_create_tail_node(mm, entry->start + entry->size,
+ size, atomic);
}
entry->size += size;
return 0;
}

static struct drm_mm_node *drm_mm_split_at_start(struct drm_mm_node *parent,
- unsigned long size)
+ unsigned long size,
+ int atomic)
{
struct drm_mm_node *child;

- child = (struct drm_mm_node *)
- drm_alloc(sizeof(*child), DRM_MEM_MM);
- if (!child)
+ child = drm_mm_kmalloc(parent->mm, atomic);
+ if (unlikely(child == NULL))
return NULL;

INIT_LIST_HEAD(&child->fl_entry);
@@ -151,8 +200,9 @@ struct drm_mm_node *drm_mm_get_block(str
tmp = parent->start % alignment;

if (tmp) {
- align_splitoff = drm_mm_split_at_start(parent, alignment - tmp);
- if (!align_splitoff)
+ align_splitoff =
+ drm_mm_split_at_start(parent, alignment - tmp, 0);
+ if (unlikely(align_splitoff == NULL))
return NULL;
}

@@ -161,7 +211,7 @@ struct drm_mm_node *drm_mm_get_block(str
parent->free = 0;
return parent;
} else {
- child = drm_mm_split_at_start(parent, size);
+ child = drm_mm_split_at_start(parent, size, 0);
}

if (align_splitoff)
@@ -169,14 +219,49 @@ struct drm_mm_node *drm_mm_get_block(str

return child;
}
+
EXPORT_SYMBOL(drm_mm_get_block);

+struct drm_mm_node *drm_mm_get_block_atomic(struct drm_mm_node *parent,
+ unsigned long size,
+ unsigned alignment)
+{
+
+ struct drm_mm_node *align_splitoff = NULL;
+ struct drm_mm_node *child;
+ unsigned tmp = 0;
+
+ if (alignment)
+ tmp = parent->start % alignment;
+
+ if (tmp) {
+ align_splitoff =
+ drm_mm_split_at_start(parent, alignment - tmp, 1);
+ if (unlikely(align_splitoff == NULL))
+ return NULL;
+ }
+
+ if (parent->size == size) {
+ list_del_init(&parent->fl_entry);
+ parent->free = 0;
+ return parent;
+ } else {
+ child = drm_mm_split_at_start(parent, size, 1);
+ }
+
+ if (align_splitoff)
+ drm_mm_put_block(align_splitoff);
+
+ return child;
+}
+EXPORT_SYMBOL(drm_mm_get_block_atomic);
+
/*
* Put a block. Merge with the previous and / or next block if they are free.
* Otherwise add to the free stack.
*/

-void drm_mm_put_block(struct drm_mm_node * cur)
+void drm_mm_put_block(struct drm_mm_node *cur)
{

struct drm_mm *mm = cur->mm;
@@ -188,21 +273,27 @@ void drm_mm_put_block(struct drm_mm_node
int merged = 0;

if (cur_head->prev != root_head) {
- prev_node = list_entry(cur_head->prev, struct drm_mm_node, ml_entry);
+ prev_node =
+ list_entry(cur_head->prev, struct drm_mm_node, ml_entry);
if (prev_node->free) {
prev_node->size += cur->size;
merged = 1;
}
}
if (cur_head->next != root_head) {
- next_node = list_entry(cur_head->next, struct drm_mm_node, ml_entry);
+ next_node =
+ list_entry(cur_head->next, struct drm_mm_node, ml_entry);
if (next_node->free) {
if (merged) {
prev_node->size += next_node->size;
list_del(&next_node->ml_entry);
list_del(&next_node->fl_entry);
- drm_free(next_node, sizeof(*next_node),
- DRM_MEM_MM);
+ if (mm->num_unused < MM_UNUSED_TARGET) {
+ list_add(&next_node->fl_entry,
+ &mm->unused_nodes);
+ ++mm->num_unused;
+ } else
+ kfree(next_node);
} else {
next_node->size += cur->size;
next_node->start = cur->start;
@@ -215,14 +306,19 @@ void drm_mm_put_block(struct drm_mm_node
list_add(&cur->fl_entry, &mm->fl_entry);
} else {
list_del(&cur->ml_entry);
- drm_free(cur, sizeof(*cur), DRM_MEM_MM);
+ if (mm->num_unused < MM_UNUSED_TARGET) {
+ list_add(&cur->fl_entry, &mm->unused_nodes);
+ ++mm->num_unused;
+ } else
+ kfree(cur);
}
}
+
EXPORT_SYMBOL(drm_mm_put_block);

-struct drm_mm_node *drm_mm_search_free(const struct drm_mm * mm,
- unsigned long size,
- unsigned alignment, int best_match)
+struct drm_mm_node *drm_mm_search_free(const struct drm_mm *mm,
+ unsigned long size,
+ unsigned alignment, int best_match)
{
struct list_head *list;
const struct list_head *free_stack = &mm->fl_entry;
@@ -247,7 +343,6 @@ struct drm_mm_node *drm_mm_search_free(c
wasted += alignment - tmp;
}

-
if (entry->size >= size + wasted) {
if (!best_match)
return entry;
@@ -260,6 +355,7 @@ struct drm_mm_node *drm_mm_search_free(c

return best;
}
+EXPORT_SYMBOL(drm_mm_search_free);

int drm_mm_clean(struct drm_mm * mm)
{
@@ -267,14 +363,17 @@ int drm_mm_clean(struct drm_mm * mm)

return (head->next->next == head);
}
-EXPORT_SYMBOL(drm_mm_search_free);
+EXPORT_SYMBOL(drm_mm_clean);

int drm_mm_init(struct drm_mm * mm, unsigned long start, unsigned long size)
{
INIT_LIST_HEAD(&mm->ml_entry);
INIT_LIST_HEAD(&mm->fl_entry);
+ INIT_LIST_HEAD(&mm->unused_nodes);
+ mm->num_unused = 0;
+ spin_lock_init(&mm->unused_lock);

- return drm_mm_create_tail_node(mm, start, size);
+ return drm_mm_create_tail_node(mm, start, size, 0);
}
EXPORT_SYMBOL(drm_mm_init);

@@ -282,6 +381,7 @@ void drm_mm_takedown(struct drm_mm * mm)
{
struct list_head *bnode = mm->fl_entry.next;
struct drm_mm_node *entry;
+ struct drm_mm_node *next;

entry = list_entry(bnode, struct drm_mm_node, fl_entry);

@@ -293,7 +393,16 @@ void drm_mm_takedown(struct drm_mm * mm)

list_del(&entry->fl_entry);
list_del(&entry->ml_entry);
+ kfree(entry);
+
+ spin_lock(&mm->unused_lock);
+ list_for_each_entry_safe(entry, next, &mm->unused_nodes, fl_entry) {
+ list_del(&entry->fl_entry);
+ kfree(entry);
+ --mm->num_unused;
+ }
+ spin_unlock(&mm->unused_lock);

- drm_free(entry, sizeof(*entry), DRM_MEM_MM);
+ BUG_ON(mm->num_unused != 0);
}
EXPORT_SYMBOL(drm_mm_takedown);
--- /dev/null
+++ b/include/drm/drm_mm.h
@@ -0,0 +1,90 @@
+/**************************************************************************
+ *
+ * Copyright 2006-2008 Tungsten Graphics, Inc., Cedar Park, TX. USA.
+ * All Rights Reserved.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the
+ * "Software"), to deal in the Software without restriction, including
+ * without limitation the rights to use, copy, modify, merge, publish,
+ * distribute, sub license, and/or sell copies of the Software, and to
+ * permit persons to whom the Software is furnished to do so, subject to
+ * the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the
+ * next paragraph) shall be included in all copies or substantial portions
+ * of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT SHALL
+ * THE COPYRIGHT HOLDERS, AUTHORS AND/OR ITS SUPPLIERS BE LIABLE FOR ANY CLAIM,
+ * DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
+ * OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
+ * USE OR OTHER DEALINGS IN THE SOFTWARE.
+ *
+ *
+ **************************************************************************/
+/*
+ * Authors:
+ * Thomas Hellstrom <thomas-at-tungstengraphics-dot-com>
+ */
+
+#ifndef _DRM_MM_H_
+#define _DRM_MM_H_
+
+/*
+ * Generic range manager structs
+ */
+#include <linux/list.h>
+
+struct drm_mm_node {
+ struct list_head fl_entry;
+ struct list_head ml_entry;
+ int free;
+ unsigned long start;
+ unsigned long size;
+ struct drm_mm *mm;
+ void *private;
+};
+
+struct drm_mm {
+ struct list_head fl_entry;
+ struct list_head ml_entry;
+ struct list_head unused_nodes;
+ int num_unused;
+ spinlock_t unused_lock;
+};
+
+/*
+ * Basic range manager support (drm_mm.c)
+ */
+
+extern struct drm_mm_node *drm_mm_get_block(struct drm_mm_node *parent,
+ unsigned long size,
+ unsigned alignment);
+extern struct drm_mm_node *drm_mm_get_block_atomic(struct drm_mm_node *parent,
+ unsigned long size,
+ unsigned alignment);
+extern void drm_mm_put_block(struct drm_mm_node *cur);
+extern struct drm_mm_node *drm_mm_search_free(const struct drm_mm *mm,
+ unsigned long size,
+ unsigned alignment,
+ int best_match);
+extern int drm_mm_init(struct drm_mm *mm, unsigned long start,
+ unsigned long size);
+extern void drm_mm_takedown(struct drm_mm *mm);
+extern int drm_mm_clean(struct drm_mm *mm);
+extern unsigned long drm_mm_tail_space(struct drm_mm *mm);
+extern int drm_mm_remove_space_from_tail(struct drm_mm *mm,
+ unsigned long size);
+extern int drm_mm_add_space_to_tail(struct drm_mm *mm,
+ unsigned long size, int atomic);
+extern int drm_mm_pre_get(struct drm_mm *mm);
+
+static inline struct drm_mm *drm_get_mm(struct drm_mm_node *block)
+{
+ return block->mm;
+}
+
+#endif
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -86,6 +86,7 @@ struct drm_device;

#include "drm_os_linux.h"
#include "drm_hashtab.h"
+#include "drm_mm.h"

/***********************************************************************/
/** \name DRM template customization defaults */
@@ -502,26 +503,6 @@ struct drm_sigdata {
};


-/*
- * Generic memory manager structs
- */
-
-struct drm_mm_node {
- struct list_head fl_entry;
- struct list_head ml_entry;
- int free;
- unsigned long start;
- unsigned long size;
- struct drm_mm *mm;
- void *private;
-};
-
-struct drm_mm {
- struct list_head fl_entry;
- struct list_head ml_entry;
-};
-
-
/**
* Mappings list
*/
@@ -1298,22 +1279,6 @@ extern char *drm_get_connector_status_na
extern int drm_sysfs_connector_add(struct drm_connector *connector);
extern void drm_sysfs_connector_remove(struct drm_connector *connector);

-/*
- * Basic memory manager support (drm_mm.c)
- */
-extern struct drm_mm_node *drm_mm_get_block(struct drm_mm_node * parent,
- unsigned long size,
- unsigned alignment);
-extern void drm_mm_put_block(struct drm_mm_node * cur);
-extern struct drm_mm_node *drm_mm_search_free(const struct drm_mm *mm, unsigned long size,
- unsigned alignment, int best_match);
-extern int drm_mm_init(struct drm_mm *mm, unsigned long start, unsigned long size);
-extern void drm_mm_takedown(struct drm_mm *mm);
-extern int drm_mm_clean(struct drm_mm *mm);
-extern unsigned long drm_mm_tail_space(struct drm_mm *mm);
-extern int drm_mm_remove_space_from_tail(struct drm_mm *mm, unsigned long size);
-extern int drm_mm_add_space_to_tail(struct drm_mm *mm, unsigned long size);
-
/* Graphics Execution Manager library functions (drm_gem.c) */
int drm_gem_init(struct drm_device *dev);
void drm_gem_destroy(struct drm_device *dev);

2009-03-19 04:12:48

by Greg KH

[permalink] [raw]
Subject: [patch 2/5] drm: Add a tracker for global objects.

[coding style issues fixed by gregkh]

Signed-off-by: Thomas Hellstrom <[email protected]>
Signed-off-by: Richard Purdie <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>

---
drivers/gpu/drm/Makefile | 3 -
drivers/gpu/drm/drm_drv.c | 3 +
drivers/gpu/drm/drm_global.c | 105 +++++++++++++++++++++++++++++++++++++++++++
include/drm/drmP.h | 20 ++++++++
4 files changed, 130 insertions(+), 1 deletion(-)

--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -386,6 +386,8 @@ static int __init drm_core_init(void)

DRM_INFO("Initialized %s %d.%d.%d %s\n",
CORE_NAME, CORE_MAJOR, CORE_MINOR, CORE_PATCHLEVEL, CORE_DATE);
+ drm_global_init();
+
return 0;
err_p3:
drm_sysfs_destroy();
@@ -399,6 +401,7 @@ err_p1:

static void __exit drm_core_exit(void)
{
+ drm_global_release();
remove_proc_entry("dri", NULL);
drm_sysfs_destroy();

--- /dev/null
+++ b/drivers/gpu/drm/drm_global.c
@@ -0,0 +1,105 @@
+/**************************************************************************
+ *
+ * Copyright 2008-2009 VMware, Inc., Palo Alto, CA., USA
+ * All Rights Reserved.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the
+ * "Software"), to deal in the Software without restriction, including
+ * without limitation the rights to use, copy, modify, merge, publish,
+ * distribute, sub license, and/or sell copies of the Software, and to
+ * permit persons to whom the Software is furnished to do so, subject to
+ * the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the
+ * next paragraph) shall be included in all copies or substantial portions
+ * of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT SHALL
+ * THE COPYRIGHT HOLDERS, AUTHORS AND/OR ITS SUPPLIERS BE LIABLE FOR ANY CLAIM,
+ * DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
+ * OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
+ * USE OR OTHER DEALINGS IN THE SOFTWARE.
+ *
+ **************************************************************************/
+#include <drmP.h>
+struct drm_global_item {
+ struct mutex mutex;
+ void *object;
+ int refcount;
+};
+
+static struct drm_global_item glob[DRM_GLOBAL_NUM];
+
+void drm_global_init(void)
+{
+ int i;
+
+ for (i = 0; i < DRM_GLOBAL_NUM; ++i) {
+ struct drm_global_item *item = &glob[i];
+ mutex_init(&item->mutex);
+ item->object = NULL;
+ item->refcount = 0;
+ }
+}
+
+void drm_global_release(void)
+{
+ int i;
+ for (i = 0; i < DRM_GLOBAL_NUM; ++i) {
+ struct drm_global_item *item = &glob[i];
+ BUG_ON(item->object != NULL);
+ BUG_ON(item->refcount != 0);
+ }
+}
+
+int drm_global_item_ref(struct drm_global_reference *ref)
+{
+ int ret;
+ struct drm_global_item *item = &glob[ref->global_type];
+ void *object;
+
+ mutex_lock(&item->mutex);
+ if (item->refcount == 0) {
+ item->object = kmalloc(ref->size, GFP_KERNEL);
+ if (unlikely(item->object == NULL)) {
+ ret = -ENOMEM;
+ goto out_err;
+ }
+
+ ref->object = item->object;
+ ret = ref->init(ref);
+ if (unlikely(ret != 0))
+ goto out_err;
+
+ ++item->refcount;
+ }
+ ref->object = item->object;
+ object = item->object;
+ mutex_unlock(&item->mutex);
+ return 0;
+out_err:
+ kfree(item->object);
+ mutex_unlock(&item->mutex);
+ item->object = NULL;
+ return ret;
+}
+EXPORT_SYMBOL(drm_global_item_ref);
+
+void drm_global_item_unref(struct drm_global_reference *ref)
+{
+ struct drm_global_item *item = &glob[ref->global_type];
+
+ mutex_lock(&item->mutex);
+ BUG_ON(item->refcount == 0);
+ BUG_ON(ref->object != item->object);
+ if (--item->refcount == 0) {
+ ref->release(ref);
+ kfree(item->object);
+ item->object = NULL;
+ }
+ mutex_unlock(&item->mutex);
+}
+EXPORT_SYMBOL(drm_global_item_unref);
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -10,7 +10,8 @@ drm-y := drm_auth.o drm_bufs.o drm
drm_lock.o drm_memory.o drm_proc.o drm_stub.o drm_vm.o \
drm_agpsupport.o drm_scatter.o ati_pcigart.o drm_pci.o \
drm_sysfs.o drm_hashtab.o drm_sman.o drm_mm.o \
- drm_crtc.o drm_crtc_helper.o drm_modes.o drm_edid.o
+ drm_crtc.o drm_crtc_helper.o drm_modes.o drm_edid.o \
+ drm_global.o

drm-$(CONFIG_COMPAT) += drm_ioc32.o

--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -1405,5 +1405,25 @@ extern void *drm_calloc(size_t nmemb, si

/*@}*/

+enum drm_global_types {
+ DRM_GLOBAL_TTM_MEM = 0,
+ DRM_GLOBAL_TTM_BO,
+ DRM_GLOBAL_TTM_OBJECT,
+ DRM_GLOBAL_NUM
+};
+
+struct drm_global_reference {
+ enum drm_global_types global_type;
+ size_t size;
+ void *object;
+ int (*init) (struct drm_global_reference *);
+ void (*release) (struct drm_global_reference *);
+};
+
+extern void drm_global_init(void);
+extern void drm_global_release(void);
+extern int drm_global_item_ref(struct drm_global_reference *ref);
+extern void drm_global_item_unref(struct drm_global_reference *ref);
+
#endif /* __KERNEL__ */
#endif

2009-03-19 04:13:05

by Greg KH

[permalink] [raw]
Subject: [patch 4/5] drm: Add unlocked IOCTL functionality from the drm repo.

Signed-off-by: Thomas Hellstrom <[email protected]>
Signed-off-by: Richard Purdie <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>

---
drivers/gpu/drm/drm_drv.c | 9 +++++++--
include/drm/drmP.h | 2 ++
2 files changed, 9 insertions(+), 2 deletions(-)

--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -455,6 +455,12 @@ static int drm_version(struct drm_device
int drm_ioctl(struct inode *inode, struct file *filp,
unsigned int cmd, unsigned long arg)
{
+ return drm_unlocked_ioctl(filp, cmd, arg);
+}
+EXPORT_SYMBOL(drm_ioctl);
+
+long drm_unlocked_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
+{
struct drm_file *file_priv = filp->private_data;
struct drm_device *dev = file_priv->minor->dev;
struct drm_ioctl_desc *ioctl;
@@ -530,8 +536,7 @@ int drm_ioctl(struct inode *inode, struc
DRM_DEBUG("ret = %x\n", retcode);
return retcode;
}
-
-EXPORT_SYMBOL(drm_ioctl);
+EXPORT_SYMBOL(drm_unlocked_ioctl);

drm_local_map_t *drm_getsarea(struct drm_device *dev)
{
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -1016,6 +1016,8 @@ extern int drm_init(struct drm_driver *d
extern void drm_exit(struct drm_driver *driver);
extern int drm_ioctl(struct inode *inode, struct file *filp,
unsigned int cmd, unsigned long arg);
+extern long drm_unlocked_ioctl(struct file *filp,
+ unsigned int cmd, unsigned long arg);
extern long drm_compat_ioctl(struct file *filp,
unsigned int cmd, unsigned long arg);
extern int drm_lastclose(struct drm_device *dev);

2009-03-19 04:13:25

by Greg KH

[permalink] [raw]
Subject: [patch 3/5] drm: Export hash table functionality.

These drm functions are needed for the psb module.

Signed-off-by: Thomas Hellstrom <[email protected]>
Signed-off-by: Richard Purdie <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>

---
drivers/gpu/drm/drm_hashtab.c | 4 ++++
1 file changed, 4 insertions(+)

--- a/drivers/gpu/drm/drm_hashtab.c
+++ b/drivers/gpu/drm/drm_hashtab.c
@@ -62,6 +62,7 @@ int drm_ht_create(struct drm_open_hash *
}
return 0;
}
+EXPORT_SYMBOL(drm_ht_create);

void drm_ht_verbose_list(struct drm_open_hash *ht, unsigned long key)
{
@@ -156,6 +157,7 @@ int drm_ht_just_insert_please(struct drm
}
return 0;
}
+EXPORT_SYMBOL(drm_ht_just_insert_please);

int drm_ht_find_item(struct drm_open_hash *ht, unsigned long key,
struct drm_hash_item **item)
@@ -169,6 +171,7 @@ int drm_ht_find_item(struct drm_open_has
*item = hlist_entry(list, struct drm_hash_item, head);
return 0;
}
+EXPORT_SYMBOL(drm_ht_find_item);

int drm_ht_remove_key(struct drm_open_hash *ht, unsigned long key)
{
@@ -202,3 +205,4 @@ void drm_ht_remove(struct drm_open_hash
ht->table = NULL;
}
}
+EXPORT_SYMBOL(drm_ht_remove);

2009-03-19 06:49:14

by Dave Airlie

[permalink] [raw]
Subject: Re: [patch 0/5] Intel Poulsbo/Morrestown DRM driver and DRM core changes

On Thu, Mar 19, 2009 at 2:08 PM, Greg KH <[email protected]> wrote:
> Hi,
>
> Here's 5 patches that add the Intel Poulsbo/Morrestown DRM driver to the
> kernel tree.
>
> There are 4 patches that make changes to the DRM core, and one patch
> that adds the DRM driver itself. ?The driver is added to the
> drivers/staging/ directory because it is not the "final" driver that
> Intel wishes to support over time. ?The userspace api is going to
> change, and work is currently underway to hook up properly with the
> memory management system.

>
> However this work is going to take a while, and in the meantime, users
> want to run Linux on this kind of hardware. ?I'd really like to add the
> driver to the staging tree, but it needs these core DRM changes in order
> to get it to work properly.
>
> Originally I had a patch that basically duplicated the existing DRM
> core, and embedded it with these changes and the PSB driver together
> into one big mess of a kernel module. ?But Richard convinced me that
> this wasn't the "nicest" thing to do, and he did work on the PSB code
> and dug out these older DRM patches.
>
> The only thing that looks a bit "odd" to me is the unlocked ioctl patch,
> Thomas, is that thing really correct?
>
> David, I'd be glad to take the DRM changes through the staging tree, but
> only if you ack them.

First off, the non-staging patches need more complete changelog entries,
a bit of meaning goes a long way. I'll ack them if they are documented and
make sense. The unlocked ioctl hook makes sense to me at least!

Now the non-core DRM driver comes with some caveats no one mentioned,
only the userspace 2D is open, no userspace 3D is available and I've no idea if
one is forthcoming. Now I don't know enough about the Poulsbo to say this
drm implementation is secure and can't DMA over my password file.

There is no upstream X.org driver available for this yet, Ubuntu
shipped something
but really we should be at least seeing X.org and Mesa commitments from Intel
to supporting this code in the future before we go shipping it all in
the kernel.

Staging something with a very broken userspace API is going to be an nightmare,
its fine if my audio doesn't work, but when X fails to start people
are left in a lot worse
place, personally I would never stage any driver that can break the
users userspace
this badly when we finally do get a version we want to upstream
properly. GPU drivers
are not just a kernel bit, they are not like a network card or sound
driver, where the
userspace API is mostly defined or a filesystem where we have POSIX.

So really I'm NAKing this from ever entering the mainline in its
current form, without
a supporting roadmap and plans for the userspace bits.

Dave.

>
> thanks,
>
> greg k-h
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at ?http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at ?http://www.tux.org/lkml/
>

2009-03-19 10:15:56

by Thomas Hellstrom

[permalink] [raw]
Subject: Re: [patch 0/5] Intel Poulsbo/Morrestown DRM driver and DRM core changes

Dave and Greg,

Some quick comments below.

Dave Airlie wrote:
> On Thu, Mar 19, 2009 at 2:08 PM, Greg KH <[email protected]> wrote:
>
>> Hi,
>>
>> Here's 5 patches that add the Intel Poulsbo/Morrestown DRM driver to the
>> kernel tree.
>>
>> There are 4 patches that make changes to the DRM core, and one patch
>> that adds the DRM driver itself. The driver is added to the
>> drivers/staging/ directory because it is not the "final" driver that
>> Intel wishes to support over time. The userspace api is going to
>> change, and work is currently underway to hook up properly with the
>> memory management system.
>>
>
>
>> However this work is going to take a while, and in the meantime, users
>> want to run Linux on this kind of hardware. I'd really like to add the
>> driver to the staging tree, but it needs these core DRM changes in order
>> to get it to work properly.
>>
>> Originally I had a patch that basically duplicated the existing DRM
>> core, and embedded it with these changes and the PSB driver together
>> into one big mess of a kernel module. But Richard convinced me that
>> this wasn't the "nicest" thing to do, and he did work on the PSB code
>> and dug out these older DRM patches.
>>
>> The only thing that looks a bit "odd" to me is the unlocked ioctl patch,
>> Thomas, is that thing really correct?
>>
>> David, I'd be glad to take the DRM changes through the staging tree, but
>> only if you ack them.
>>
>
> First off, the non-staging patches need more complete changelog entries,
> a bit of meaning goes a long way. I'll ack them if they are documented and
> make sense. The unlocked ioctl hook makes sense to me at least!
>

For the non-staging patches, (which also sit in the modesetting-newttm
tree),
I can add some more elaborate comments. However I think all of them are
targeted to support functionality for TTM, so unless the TTM code goes
into staging or mainstream, there is little point in merging them to
core drm before other drivers find them useful.

Although I see the patch adding TTM is including some backwards
compatibility defines
(In particular the PAT compat stuff) that needs to be stripped.


> Now the non-core DRM driver comes with some caveats no one mentioned,
> only the userspace 2D is open, no userspace 3D is available and I've no idea if
> one is forthcoming. Now I don't know enough about the Poulsbo to say this
> drm implementation is secure and can't DMA over my password file.
>
>

The GPU in Poulsbo / Morestown can only access memory through a
multi-context 4GB two-level page-table, and the implementation of that
is open in psb_mmu.c.
The VDC is accessing memory through a traditional Intel GTT: psb_gtt.c
The registers used to set up the page table are blocked from user-space
access. Any registers used to fire GPU assembly code with privileges to
change those registers are also blocked.

So security-wise there is nothing worse with this device than with other
devices without full open documentation. And AFAICT it's more secure
than some drivers _with_ full open documentation.
The non-existence of an open-source 3D implementation doesn't really
alter that situation.

However, if there's a policy issue about adding Linux kernel support for
closed-source user-space drivers, I think it helps to be explicit about
that.


> There is no upstream X.org driver available for this yet, Ubuntu
> shipped something
> but really we should be at least seeing X.org and Mesa commitments from Intel
> to supporting this code in the future before we go shipping it all in
> the kernel.
>
>

...

> Staging something with a very broken userspace API is going to be an nightmare,
> its fine if my audio doesn't work, but when X fails to start people
> are left in a lot worse
> place, personally I would never stage any driver that can break the
> users userspace
> this badly when we finally do get a version we want to upstream
> properly. GPU drivers
> are not just a kernel bit, they are not like a network card or sound
> driver, where the
> userspace API is mostly defined or a filesystem where we have POSIX.
>
>

I can't really comment on this since I have no knowledge about the
upcoming interface changes.

Thanks,
Thomas

> So really I'm NAKing this from ever entering the mainline in its
> current form, without
> a supporting roadmap and plans for the userspace bits.
>
> Dave.
>
>
>> thanks,
>>
>> greg k-h
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>> the body of a message to [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>> Please read the FAQ at http://www.tux.org/lkml/
>>
>>
>
> ------------------------------------------------------------------------------
> Apps built with the Adobe(R) Flex(R) framework and Flex Builder(TM) are
> powering Web 2.0 with engaging, cross-platform capabilities. Quickly and
> easily build your RIAs with Flex Builder, the Eclipse(TM)based development
> software that enables intelligent coding and step-through debugging.
> Download the free 60 day trial. http://p.sf.net/sfu/www-adobe-com
> --
> _______________________________________________
> Dri-devel mailing list
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/dri-devel
>


2009-03-19 10:38:46

by Alan

[permalink] [raw]
Subject: Re: [patch 0/5] Intel Poulsbo/Morrestown DRM driver and DRM core changes

> The non-existence of an open-source 3D implementation doesn't really
> alter that situation.

I think it does to an extent
>
> However, if there's a policy issue about adding Linux kernel support for
> closed-source user-space drivers, I think it helps to be explicit about
> that.

Actually its a lawyer question not just policy. If the two parts being put
together are tightly dependant on each other and in fact form a single
work which it seems could reasonably be the case in this situation then
if one half is GPL the other must also be so.

There is also policy on this, I refer you back to the Intel wireless and
binary management daemon discussions. I likewise refer you to some of the
input device discussions related to USB HID and devices where vendors
wanted us to exclude their device in favour of proprietary libusb drivers.

Alan

2009-03-19 10:43:58

by Richard Purdie

[permalink] [raw]
Subject: Re: [patch 0/5] Intel Poulsbo/Morrestown DRM driver and DRM core changes

Hi,

On Thu, 2009-03-19 at 16:48 +1000, Dave Airlie wrote:
> First off, the non-staging patches need more complete changelog entries,
> a bit of meaning goes a long way. I'll ack them if they are documented and
> make sense. The unlocked ioctl hook makes sense to me at least!
>
> Now the non-core DRM driver comes with some caveats no one mentioned,
> only the userspace 2D is open, no userspace 3D is available and I've no idea if
> one is forthcoming. Now I don't know enough about the Poulsbo to say this
> drm implementation is secure and can't DMA over my password file.

I think Thomas has covered these things and between us, we can improve
the patch series.

> There is no upstream X.org driver available for this yet, Ubuntu
> shipped something
> but really we should be at least seeing X.org and Mesa commitments from Intel
> to supporting this code in the future before we go shipping it all in
> the kernel.

An older version of the X.org driver has been available (with various
urls) at http://git.moblin.org/cgit.cgi/deprecated/xf86-video-psb/ for a
while. I'm working on getting this updated and that should happen soon.

I'm also working on getting the binary bits for 3D support publicly
available somewhere but these are not open source and unlikely to be any
time soon. We know this sucks, we're working on it but thats all I can
really say.

As Greg said, we don't envisage this driver being the final solution.
There is hardware out there, running Linux which needs any driver rather
than no driver now. Having a standard implementation that everyone uses
has to be preferable to everyone rolling their own modified version of
it. For 2D, the driver is open and people can chose to use the 3D
binaries or not. This would seem to fit the staging tree's mandate where
this driver could live until things get sorted properly.

Does this ease some of your concerns? I don't claim this is an ideal
situation but we're all trying to make the best of what we've got...

Cheers,

Richard


2009-03-19 16:03:25

by Sindhudweep Sarkar

[permalink] [raw]
Subject: Re: [patch 0/5] Intel Poulsbo/Morrestown DRM driver and DRM core changes

This might be the opinion of a completely non educated end user but it
seems that an intel specific drm and other bits (xorg, mesa) would be
somewhat of a maintenance waste.

TI-OMAP 3xxx and a couple of other arm processors use similar SGX-5xx
graphics cores. IIRC arm is often little endian so perhaps a unified
driver would be easier in the long term.

On Thu, Mar 19, 2009 at 12:08 AM, Greg KH <[email protected]> wrote:
> Hi,
>
> Here's 5 patches that add the Intel Poulsbo/Morrestown DRM driver to the
> kernel tree.
>
> There are 4 patches that make changes to the DRM core, and one patch
> that adds the DRM driver itself. ?The driver is added to the
> drivers/staging/ directory because it is not the "final" driver that
> Intel wishes to support over time. ?The userspace api is going to
> change, and work is currently underway to hook up properly with the
> memory management system.
>
> However this work is going to take a while, and in the meantime, users
> want to run Linux on this kind of hardware. ?I'd really like to add the
> driver to the staging tree, but it needs these core DRM changes in order
> to get it to work properly.
>
> Originally I had a patch that basically duplicated the existing DRM
> core, and embedded it with these changes and the PSB driver together
> into one big mess of a kernel module. ?But Richard convinced me that
> this wasn't the "nicest" thing to do, and he did work on the PSB code
> and dug out these older DRM patches.
>
> The only thing that looks a bit "odd" to me is the unlocked ioctl patch,
> Thomas, is that thing really correct?
>
> David, I'd be glad to take the DRM changes through the staging tree, but
> only if you ack them.
>
> thanks,
>
> greg k-h
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at ?http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at ?http://www.tux.org/lkml/
>

2009-03-19 16:28:27

by Greg KH

[permalink] [raw]
Subject: Re: [patch 0/5] Intel Poulsbo/Morrestown DRM driver and DRM core changes

On Thu, Mar 19, 2009 at 12:03:09PM -0400, Sindhudweep Sarkar wrote:
> This might be the opinion of a completely non educated end user but it
> seems that an intel specific drm and other bits (xorg, mesa) would be
> somewhat of a maintenance waste.

What do you mean by this?

> TI-OMAP 3xxx and a couple of other arm processors use similar SGX-5xx
> graphics cores. IIRC arm is often little endian so perhaps a unified
> driver would be easier in the long term.

Long term lots of things are good.

But how do I get my laptop that I currently have right now up and
running properly with linux in a better-than-800x600-framebuffer mode?

That's why I need/want this driver now, there are hundreds of thousands
of these types of laptops in the pipeline to users and I want them to
run Linux, not be forced to run some other operating system...

thanks,

greg k-h

2009-03-19 17:12:59

by Richard Purdie

[permalink] [raw]
Subject: Re: [patch 0/5] Intel Poulsbo/Morrestown DRM driver and DRM core changes


On Thu, 2009-03-19 at 12:03 -0400, Sindhudweep Sarkar wrote:
> This might be the opinion of a completely non educated end user but it
> seems that an intel specific drm and other bits (xorg, mesa) would be
> somewhat of a maintenance waste.
>
> TI-OMAP 3xxx and a couple of other arm processors use similar SGX-5xx
> graphics cores. IIRC arm is often little endian so perhaps a unified
> driver would be easier in the long term.

Long term a unified driver would be very nice to have and nobody
disagrees with that. Things don't happen overnight and you have to take
smaller steps to get there. This proposal is one step on a road that may
lead to a driver for the TI part too. It will need someone in the ARM
community to step up and write the ARM specific bits.

Cheers,

Richard

2009-03-19 18:12:36

by Sindhudweep Sarkar

[permalink] [raw]
Subject: Re: [patch 0/5] Intel Poulsbo/Morrestown DRM driver and DRM core changes

Of course real support for poulsbo is really important and desirable
since netbooks make such a huge percentage of computer sales these
days. I'm guessing, however, that the number of
embedded/mobile/appliance devices that use or will use the SGX5xx is
much larger than even the plethora of netbooks being pumped out. If
more hardware support can be gained for a little extra burden why not
go for it? I would argue that a unified driver stack would actually
have less maintenance cost since common bugs could be killed. Even
ignoring all the arm devices that will use that graphics core, what
about intel's own use of the SGX 535 elsewhere? Does this "poulsbo"
driver support the intel CE3100 processors?


I think i'm really apprehensive about device specific one-offs. Of
course a mainline driver upstream is an important step to prevent that
but without a roadmap it only seems marginally better.

Best,
Sindhudweep

On Thu, Mar 19, 2009 at 12:27 PM, Greg KH <[email protected]> wrote:
> On Thu, Mar 19, 2009 at 12:03:09PM -0400, Sindhudweep Sarkar wrote:
>> This might be the opinion of a completely non educated end user but it
>> seems that an intel specific drm and other bits (xorg, mesa) would be
>> somewhat of a maintenance waste.
>
> What do you mean by this?
>
>> TI-OMAP 3xxx and a couple of other arm processors use similar SGX-5xx
>> graphics cores. IIRC arm is often little endian so perhaps a unified
>> driver would be easier in the long term.
>
> Long term lots of things are good.
>
> But how do I get my laptop that I currently have right now up and
> running properly with linux in a better-than-800x600-framebuffer mode?
>
> That's why I need/want this driver now, there are hundreds of thousands
> of these types of laptops in the pipeline to users and I want them to
> run Linux, not be forced to run some other operating system...
>
> thanks,
>
> greg k-h
>

2009-03-19 18:54:01

by Matthew Garrett

[permalink] [raw]
Subject: Re: [patch 0/5] Intel Poulsbo/Morrestown DRM driver and DRM core changes

On Thu, Mar 19, 2009 at 09:27:19AM -0700, Greg KH wrote:

> But how do I get my laptop that I currently have right now up and
> running properly with linux in a better-than-800x600-framebuffer mode?

You don't, because there's no working X driver.

--
Matthew Garrett | [email protected]

2009-03-19 19:03:18

by Greg KH

[permalink] [raw]
Subject: Re: [patch 0/5] Intel Poulsbo/Morrestown DRM driver and DRM core changes

On Thu, Mar 19, 2009 at 06:53:38PM +0000, Matthew Garrett wrote:
> On Thu, Mar 19, 2009 at 09:27:19AM -0700, Greg KH wrote:
>
> > But how do I get my laptop that I currently have right now up and
> > running properly with linux in a better-than-800x600-framebuffer mode?
>
> You don't, because there's no working X driver.

Um, no, I have one right here, seems to work ok. Richard pointed out
the public git tree for it.

Yes, there's no excellerated 3d without the binary xorg "blob", but I
really don't need that right now.

thanks,

greg k-h

2009-03-19 19:05:49

by Matthew Garrett

[permalink] [raw]
Subject: Re: [patch 0/5] Intel Poulsbo/Morrestown DRM driver and DRM core changes

On Thu, Mar 19, 2009 at 12:02:54PM -0700, Greg KH wrote:
> On Thu, Mar 19, 2009 at 06:53:38PM +0000, Matthew Garrett wrote:
> > On Thu, Mar 19, 2009 at 09:27:19AM -0700, Greg KH wrote:
> >
> > > But how do I get my laptop that I currently have right now up and
> > > running properly with linux in a better-than-800x600-framebuffer mode?
> >
> > You don't, because there's no working X driver.
>
> Um, no, I have one right here, seems to work ok. Richard pointed out
> the public git tree for it.

With current X? If that's been fixed up recently then it's a definite
improvement, but last I checked it only built against old X servers.

--
Matthew Garrett | [email protected]

2009-03-19 19:20:48

by Greg KH

[permalink] [raw]
Subject: Re: [patch 0/5] Intel Poulsbo/Morrestown DRM driver and DRM core changes

On Thu, Mar 19, 2009 at 07:05:30PM +0000, Matthew Garrett wrote:
> On Thu, Mar 19, 2009 at 12:02:54PM -0700, Greg KH wrote:
> > On Thu, Mar 19, 2009 at 06:53:38PM +0000, Matthew Garrett wrote:
> > > On Thu, Mar 19, 2009 at 09:27:19AM -0700, Greg KH wrote:
> > >
> > > > But how do I get my laptop that I currently have right now up and
> > > > running properly with linux in a better-than-800x600-framebuffer mode?
> > >
> > > You don't, because there's no working X driver.
> >
> > Um, no, I have one right here, seems to work ok. Richard pointed out
> > the public git tree for it.
>
> With current X? If that's been fixed up recently then it's a definite
> improvement, but last I checked it only built against old X servers.

It currently only builds with new X servers, which turned out to be a
problem trying to get it to work on an openSUSE 11.1 machine, which uses
an older xserver, but that's a distro issue, not an upstream issue...

thanks,

greg k-h

2009-03-19 20:14:31

by Greg KH

[permalink] [raw]
Subject: Re: [patch 0/5] Intel Poulsbo/Morrestown DRM driver and DRM core changes

On Thu, Mar 19, 2009 at 04:48:54PM +1000, Dave Airlie wrote:
> On Thu, Mar 19, 2009 at 2:08 PM, Greg KH <[email protected]> wrote:
> > Hi,
> >
> > Here's 5 patches that add the Intel Poulsbo/Morrestown DRM driver to the
> > kernel tree.
> >
> > There are 4 patches that make changes to the DRM core, and one patch
> > that adds the DRM driver itself. ?The driver is added to the
> > drivers/staging/ directory because it is not the "final" driver that
> > Intel wishes to support over time. ?The userspace api is going to
> > change, and work is currently underway to hook up properly with the
> > memory management system.
>
> >
> > However this work is going to take a while, and in the meantime, users
> > want to run Linux on this kind of hardware. ?I'd really like to add the
> > driver to the staging tree, but it needs these core DRM changes in order
> > to get it to work properly.
> >
> > Originally I had a patch that basically duplicated the existing DRM
> > core, and embedded it with these changes and the PSB driver together
> > into one big mess of a kernel module. ?But Richard convinced me that
> > this wasn't the "nicest" thing to do, and he did work on the PSB code
> > and dug out these older DRM patches.
> >
> > The only thing that looks a bit "odd" to me is the unlocked ioctl patch,
> > Thomas, is that thing really correct?
> >
> > David, I'd be glad to take the DRM changes through the staging tree, but
> > only if you ack them.
>
> First off, the non-staging patches need more complete changelog entries,
> a bit of meaning goes a long way. I'll ack them if they are documented and
> make sense. The unlocked ioctl hook makes sense to me at least!

Heh, that one doesn't make sense to me as it doesn't seem to change any
locking issues :)

I'll write up better changelog comments based on other responses in this
thread.

> Now the non-core DRM driver comes with some caveats no one mentioned,
> only the userspace 2D is open, no userspace 3D is available and I've no idea if
> one is forthcoming. Now I don't know enough about the Poulsbo to say this
> drm implementation is secure and can't DMA over my password file.
>
> There is no upstream X.org driver available for this yet, Ubuntu
> shipped something but really we should be at least seeing X.org and
> Mesa commitments from Intel to supporting this code in the future
> before we go shipping it all in the kernel.

The xorg driver is public, as has been pointed out.

As for the legality of the 3d binary blob, I really have no idea, and
for now, I really don't care, I just want basic 2d to work properly on
my hardware.

> Staging something with a very broken userspace API is going to be an nightmare,

That's exactly what staging is for :)

Seriously, stuff in drivers/staging has horrible userspace api issues,
and has no guarantee to work properly at all in future kernel versions.
That's why we are using the staging tree, it lets users have a common
place to get the latest code to get their hardware working, and lets
developers work together in one place.

See my lkml post yesterday about staging if you have other questions
about it.

> So really I'm NAKing this from ever entering the mainline in its
> current form, without a supporting roadmap and plans for the userspace
> bits.

staging isn't really "mainline" so much as it is a place to get things
working.

If you want, I'll respin the drm changes and resubmit them.

If you aren't going to want to accept those drm changes, that's fine,
and I have no problem with that.

In that case, I'll just "embed" a private copy of the drm core in the
psb kernel driver in staging, much like we have done many times already
with wireless drivers. Yes, it's not the nicest or prettiest thing at
all, but it lets people use their hardware and is a hell of a lot better
than burrying this stuff in random git trees around the world in ways
that no one else can figure out how to get working (like Ubuntu has
already done...)

thanks,

greg k-h

2009-03-19 20:14:48

by Greg KH

[permalink] [raw]
Subject: Re: [patch 0/5] Intel Poulsbo/Morrestown DRM driver and DRM core changes

On Thu, Mar 19, 2009 at 11:14:35AM +0100, Thomas Hellstr?m wrote:
>> First off, the non-staging patches need more complete changelog entries,
>> a bit of meaning goes a long way. I'll ack them if they are documented and
>> make sense. The unlocked ioctl hook makes sense to me at least!
>>
>
> For the non-staging patches, (which also sit in the modesetting-newttm
> tree), I can add some more elaborate comments. However I think all of
> them are targeted to support functionality for TTM, so unless the TTM
> code goes into staging or mainstream, there is little point in merging
> them to core drm before other drivers find them useful.
>
> Although I see the patch adding TTM is including some backwards
> compatibility defines (In particular the PAT compat stuff) that needs
> to be stripped.

Great, care to respin it and send it to me?

thanks,

greg k-h

2009-03-19 20:15:04

by Greg KH

[permalink] [raw]
Subject: Re: [patch 0/5] Intel Poulsbo/Morrestown DRM driver and DRM core changes

On Thu, Mar 19, 2009 at 10:39:03AM +0000, Alan Cox wrote:
> > The non-existence of an open-source 3D implementation doesn't really
> > alter that situation.
>
> I think it does to an extent

With an opensource 2d solution it does?

> > However, if there's a policy issue about adding Linux kernel support for
> > closed-source user-space drivers, I think it helps to be explicit about
> > that.
>
> Actually its a lawyer question not just policy. If the two parts being put
> together are tightly dependant on each other and in fact form a single
> work which it seems could reasonably be the case in this situation then
> if one half is GPL the other must also be so.

I'm not going to disagree with that, which would force the 3d portions
open. At the moment I'm more worried about just getting 2d to work at
all.

I was going to wait for the "rewrite" to worry about 3d stuff, as it
will be done properly that way.

thanks,

greg k-h

2009-03-19 20:15:32

by Greg KH

[permalink] [raw]
Subject: Re: [patch 0/5] Intel Poulsbo/Morrestown DRM driver and DRM core changes

On Thu, Mar 19, 2009 at 02:11:18PM -0400, Sindhudweep Sarkar wrote:
> Of course real support for poulsbo is really important and desirable
> since netbooks make such a huge percentage of computer sales these
> days. I'm guessing, however, that the number of
> embedded/mobile/appliance devices that use or will use the SGX5xx is
> much larger than even the plethora of netbooks being pumped out. If
> more hardware support can be gained for a little extra burden why not
> go for it? I would argue that a unified driver stack would actually
> have less maintenance cost since common bugs could be killed. Even
> ignoring all the arm devices that will use that graphics core, what
> about intel's own use of the SGX 535 elsewhere? Does this "poulsbo"
> driver support the intel CE3100 processors?
>
>
> I think i'm really apprehensive about device specific one-offs. Of
> course a mainline driver upstream is an important step to prevent that
> but without a roadmap it only seems marginally better.

Staging is all aobut "device specific one-offs". the code is then in a
common area where everyone can work to fix it up properly.

I don't want to burry this work again in random git trees that no one
has any clue how to put together, like has already been done in the
past.

thanks,

greg k-h

2009-03-19 20:15:51

by Corbin Simpson

[permalink] [raw]
Subject: Re: [patch 0/5] Intel Poulsbo/Morrestown DRM driver and DRM core changes

Richard Purdie wrote:
> On Thu, 2009-03-19 at 12:03 -0400, Sindhudweep Sarkar wrote:
>> This might be the opinion of a completely non educated end user but it
>> seems that an intel specific drm and other bits (xorg, mesa) would be
>> somewhat of a maintenance waste.
>>
>> TI-OMAP 3xxx and a couple of other arm processors use similar SGX-5xx
>> graphics cores. IIRC arm is often little endian so perhaps a unified
>> driver would be easier in the long term.
>
> Long term a unified driver would be very nice to have and nobody
> disagrees with that. Things don't happen overnight and you have to take
> smaller steps to get there. This proposal is one step on a road that may
> lead to a driver for the TI part too. It will need someone in the ARM
> community to step up and write the ARM specific bits.

Nokia's written and open-sourced some related code for DRM.

My university is paying me to work on an ARM-based SoC handheld with an
SGX core, and I might be more into this later. (Y'know, once the device
is actually out the door.)

~ C.

2009-03-19 20:40:39

by Thomas Hellstrom

[permalink] [raw]
Subject: Re: [patch 0/5] Intel Poulsbo/Morrestown DRM driver and DRM core changes

Greg KH wrote:
> On Thu, Mar 19, 2009 at 11:14:35AM +0100, Thomas Hellstr?m wrote:
>
>>> First off, the non-staging patches need more complete changelog entries,
>>> a bit of meaning goes a long way. I'll ack them if they are documented and
>>> make sense. The unlocked ioctl hook makes sense to me at least!
>>>
>>>
>> For the non-staging patches, (which also sit in the modesetting-newttm
>> tree), I can add some more elaborate comments. However I think all of
>> them are targeted to support functionality for TTM, so unless the TTM
>> code goes into staging or mainstream, there is little point in merging
>> them to core drm before other drivers find them useful.
>>
>> Although I see the patch adding TTM is including some backwards
>> compatibility defines (In particular the PAT compat stuff) that needs
>> to be stripped.
>>
>
> Great, care to respin it and send it to me?
>
> thanks,
>
> greg k-h
>
Greg,
A clean TTM patch was sent to Intel with the other patches.
I'm not sure why it got lost along the way?

/Thomas



2009-03-19 21:15:19

by Sindhudweep Sarkar

[permalink] [raw]
Subject: Re: [patch 0/5] Intel Poulsbo/Morrestown DRM driver and DRM core changes

> Nokia's written and open-sourced some related code for DRM.
>
> My university is paying me to work on an ARM-based SoC handheld with an
> SGX core, and I might be more into this later. (Y'know, once the device
> is actually out the door.)
>
> ~ C.
>

Could you point me to the aforementioned open-sourced drm code?

2009-03-20 00:46:19

by Greg KH

[permalink] [raw]
Subject: Re: [patch 0/5] Intel Poulsbo/Morrestown DRM driver and DRM core changes

On Thu, Mar 19, 2009 at 09:40:08PM +0100, Thomas Hellstr?m wrote:
> Greg KH wrote:
>> On Thu, Mar 19, 2009 at 11:14:35AM +0100, Thomas Hellstr?m wrote:
>>
>>>> First off, the non-staging patches need more complete changelog entries,
>>>> a bit of meaning goes a long way. I'll ack them if they are documented
>>>> and
>>>> make sense. The unlocked ioctl hook makes sense to me at least!
>>>>
>>> For the non-staging patches, (which also sit in the modesetting-newttm
>>> tree), I can add some more elaborate comments. However I think all of
>>> them are targeted to support functionality for TTM, so unless the TTM
>>> code goes into staging or mainstream, there is little point in merging
>>> them to core drm before other drivers find them useful.
>>>
>>> Although I see the patch adding TTM is including some backwards
>>> compatibility defines (In particular the PAT compat stuff) that needs
>>> to be stripped.
>>>
>>
>> Great, care to respin it and send it to me?
>>
> Greg,
> A clean TTM patch was sent to Intel with the other patches.
> I'm not sure why it got lost along the way?

As there seems to be an unknown number of people in the "intel chain"
that resulted in me finally getting these patches, I don't hold out much
hope that I'm going to be able to get more than what I already got from
them :(

So, any chance you could just resend it? I'll be glad to trade it for a
beer at the next conference :)

thanks,

greg k-h

2009-03-20 06:45:28

by Daniel Stone

[permalink] [raw]
Subject: Re: [patch 0/5] Intel Poulsbo/Morrestown DRM driver and DRM core changes

Greg,

On Thu, Mar 19, 2009 at 09:27:19AM -0700, Greg KH wrote:
> On Thu, Mar 19, 2009 at 12:03:09PM -0400, Sindhudweep Sarkar wrote:
> > TI-OMAP 3xxx and a couple of other arm processors use similar SGX-5xx
> > graphics cores. IIRC arm is often little endian so perhaps a unified
> > driver would be easier in the long term.
>
> Long term lots of things are good.
>
> But how do I get my laptop that I currently have right now up and
> running properly with linux in a better-than-800x600-framebuffer mode?
>
> That's why I need/want this driver now, there are hundreds of thousands
> of these types of laptops in the pipeline to users and I want them to
> run Linux, not be forced to run some other operating system...

By the same logic, would you support including the proprietary NVIDIA
driver while we wait for Nouveau to catch up?

Cheers,
Daniel


Attachments:
(No filename) (857.00 B)
signature.asc (197.00 B)
Digital signature
Download all attachments

2009-03-20 14:18:41

by Thomas Hellstrom

[permalink] [raw]
Subject: Re: [patch 0/5] Intel Poulsbo/Morrestown DRM driver and DRM core changes

Greg KH wrote:
> On Thu, Mar 19, 2009 at 09:40:08PM +0100, Thomas Hellstr?m wrote:
>
>> Greg KH wrote:
>>
>>> On Thu, Mar 19, 2009 at 11:14:35AM +0100, Thomas Hellstr?m wrote:
>>>
>>>
>>>>> First off, the non-staging patches need more complete changelog entries,
>>>>> a bit of meaning goes a long way. I'll ack them if they are documented
>>>>> and
>>>>> make sense. The unlocked ioctl hook makes sense to me at least!
>>>>>
>>>>>
>>>> For the non-staging patches, (which also sit in the modesetting-newttm
>>>> tree), I can add some more elaborate comments. However I think all of
>>>> them are targeted to support functionality for TTM, so unless the TTM
>>>> code goes into staging or mainstream, there is little point in merging
>>>> them to core drm before other drivers find them useful.
>>>>
>>>> Although I see the patch adding TTM is including some backwards
>>>> compatibility defines (In particular the PAT compat stuff) that needs
>>>> to be stripped.
>>>>
>>>>
>>> Great, care to respin it and send it to me?
>>>
>>>
>> Greg,
>> A clean TTM patch was sent to Intel with the other patches.
>> I'm not sure why it got lost along the way?
>>
>
> As there seems to be an unknown number of people in the "intel chain"
> that resulted in me finally getting these patches, I don't hold out much
> hope that I'm going to be able to get more than what I already got from
> them :(
>
> So, any chance you could just resend it? I'll be glad to trade it for a
> beer at the next conference :)
>
> thanks,
>
> greg k-h
>
So, a beer it is :)

I'm attaching it with the latest TTM bugfixes and passing checkpatch.pl.
Note that this is against the DRM tree, not the staging tree.

Actually, there are two patches. One exporting the X86 PAT
pgprot_writecombine functionality, as the
psb / mrst architecture is dependant on it. There's no IO region we can
write-combine like for a traditional GPU, and not even the VDC GTT
supports traditional write-combining.

The psb / mrst part is not included, though but it should be sufficient
just to replace the ttm files
and remove those files not in this patch.

Now while we're at it, we could perhaps discuss the placement of TTM.
It will probably see some continous evolvement; Dave has suggested it
being a submodule on its own,
but nothing that should stop upstream merging or have a dramatic impact
on the in-kernel interfaces.
And there is another user of TTM as well, the openChrome drm module.
It's user mode interface is quite fixed save for the video decoder
functionality, and that can be stripped until there is user-space
support, so this module is probably mature enough to go at least into
the staging tree if not mainstream.

That would suggest placing TTM not as a subdirectory of the psb module
but as a freestanding subdirectory either of drm in mainstream or in
staging. Suggestions?

I attach the patches to the mail, rather than sending them standalone as
they will likely need some massaging by Greg.

/Thomas






Attachments:
0001-x86-pat-Export-pgprot_writecombine.patch (933.00 B)
0002-Add-the-TTM-gpu-memory-subsystem.patch (267.03 kB)
Download all attachments

2009-03-20 14:55:23

by Greg KH

[permalink] [raw]
Subject: Re: [patch 0/5] Intel Poulsbo/Morrestown DRM driver and DRM core changes

On Fri, Mar 20, 2009 at 05:08:23PM +1100, Daniel Stone wrote:
> Greg,
>
> On Thu, Mar 19, 2009 at 09:27:19AM -0700, Greg KH wrote:
> > On Thu, Mar 19, 2009 at 12:03:09PM -0400, Sindhudweep Sarkar wrote:
> > > TI-OMAP 3xxx and a couple of other arm processors use similar SGX-5xx
> > > graphics cores. IIRC arm is often little endian so perhaps a unified
> > > driver would be easier in the long term.
> >
> > Long term lots of things are good.
> >
> > But how do I get my laptop that I currently have right now up and
> > running properly with linux in a better-than-800x600-framebuffer mode?
> >
> > That's why I need/want this driver now, there are hundreds of thousands
> > of these types of laptops in the pipeline to users and I want them to
> > run Linux, not be forced to run some other operating system...
>
> By the same logic, would you support including the proprietary NVIDIA
> driver while we wait for Nouveau to catch up?

The license of the NVIDIA driver does not allow that to even be a
possibility.

thanks,

greg k-h

2009-03-20 15:01:29

by Alan

[permalink] [raw]
Subject: Re: [patch 0/5] Intel Poulsbo/Morrestown DRM driver and DRM core changes

> > By the same logic, would you support including the proprietary NVIDIA
> > driver while we wait for Nouveau to catch up?
>
> The license of the NVIDIA driver does not allow that to even be a
> possibility.

I'm not convinced this is any different. If you accept the 3D changes you
step into a dangerous world of estoppel and since it has many
rightsholders also the wonderful world of contributory infringement. It
really really needs lawyers to look into it.

Now the other way to do it that might be more productive and simpler
would be to rip all the 3D crap out of that driver and just include the
minimum needed 2D bits for the open source X driver. Makes the code
smaller and cleaner, avoids an legal questions and lets people get on
with real work.

2009-03-20 15:50:46

by Greg KH

[permalink] [raw]
Subject: Re: [patch 0/5] Intel Poulsbo/Morrestown DRM driver and DRM core changes

On Fri, Mar 20, 2009 at 03:00:32PM +0000, Alan Cox wrote:
> > > By the same logic, would you support including the proprietary NVIDIA
> > > driver while we wait for Nouveau to catch up?
> >
> > The license of the NVIDIA driver does not allow that to even be a
> > possibility.
>
> I'm not convinced this is any different. If you accept the 3D changes you
> step into a dangerous world of estoppel and since it has many
> rightsholders also the wonderful world of contributory infringement. It
> really really needs lawyers to look into it.

I would hope that Intel's lawyers would have done such a thing before
releasing the kernel and xorg code under the licenses that they did :)

> Now the other way to do it that might be more productive and simpler
> would be to rip all the 3D crap out of that driver and just include the
> minimum needed 2D bits for the open source X driver. Makes the code
> smaller and cleaner, avoids an legal questions and lets people get on
> with real work.

Hm, that sounds fine to me.

Does that mean that all of these drm patches that I posted are _only_
needed for the 3d portions of the driver? If I rip out the portions of
the psb kernel driver that need these changes, do I end up with an
working 2d driver as well? Richard and Thomas, any thoughts here?

thanks,

greg k-h

2009-03-20 17:01:54

by Thomas Hellstrom

[permalink] [raw]
Subject: Re: [patch 0/5] Intel Poulsbo/Morrestown DRM driver and DRM core changes

Greg KH wrote:
> On Fri, Mar 20, 2009 at 03:00:32PM +0000, Alan Cox wrote:
>
>>>> By the same logic, would you support including the proprietary NVIDIA
>>>> driver while we wait for Nouveau to catch up?
>>>>
>>> The license of the NVIDIA driver does not allow that to even be a
>>> possibility.
>>>
>> I'm not convinced this is any different. If you accept the 3D changes you
>> step into a dangerous world of estoppel and since it has many
>> rightsholders also the wonderful world of contributory infringement. It
>> really really needs lawyers to look into it.
>>
>
> I would hope that Intel's lawyers would have done such a thing before
> releasing the kernel and xorg code under the licenses that they did :)
>
>
>> Now the other way to do it that might be more productive and simpler
>> would be to rip all the 3D crap out of that driver and just include the
>> minimum needed 2D bits for the open source X driver. Makes the code
>> smaller and cleaner, avoids an legal questions and lets people get on
>> with real work.
>>
>
> Hm, that sounds fine to me.
>
> Does that mean that all of these drm patches that I posted are _only_
> needed for the 3d portions of the driver? If I rip out the portions of
> the psb kernel driver that need these changes, do I end up with an
> working 2d driver as well? Richard and Thomas, any thoughts here?
>
> thanks,
>
> greg k-h
>
>
Greg,
Let's not forget the video stuff. I'm not sure about the status of that.

But all the other patches are needed for 2D functionality and kernel
modesetting.
If you want to strip 3D you can strip.

psb_schedule.[ch] - The 3D scheduler.
psb_xhw.c - The interface to the binary X server module.
psb_scene.c - The 3D scene tracker.

and anything that appears unused after that.
This means you lose Render acceleration and textured video and
downscaling in the X server as well,
although accelerated rotation should still work, since it uses the 2D
engine.

I think the video acceleration sits in

psb_msvdx.c
psb_msvdxinit.c
lnc_topaz.c
lnc_topazinit.c

/Thomas


2009-03-21 03:00:30

by Greg KH

[permalink] [raw]
Subject: Re: [patch 0/5] Intel Poulsbo/Morrestown DRM driver and DRM core changes

On Wed, Mar 18, 2009 at 09:08:09PM -0700, Greg KH wrote:
> Hi,
>
> Here's 5 patches that add the Intel Poulsbo/Morrestown DRM driver to the
> kernel tree.

Ok, in reviewing the issues that Alan rightfully brought up, I'm going
to "retract" these patches right now and go back and try to only get the
2d stuff up and working properly.

Due to a vacation on my side in the next few weeks, this might take a
while, I'll come back when I have something that is working and doesn't
support the binary blob mess.

thanks,

greg k-h