Re: [Intel-gfx] Fwd: State of 10 bits/channel?

2010-07-23 Thread Chris Wilson
On Thu, 22 Jul 2010 17:34:09 -0400, Andrew Lutomirski l...@mit.edu wrote:
 [resend b/c I used the wrong from address last time]
 
 I have a 10 bit/channel monitor (DisplayPort) which works quite nicely
 in 8 bit mode.  I saw some patches from Peter Clifton related to 10
 bit support go in awhile ago.

[snip]
 
 What is the hardware capable of, and what is the state of affairs
 right now?

AIUI, all the intel chips are capable of driving 30-bit displays. The
later generations are more flexible and faster with greater bit depth for
LUTs etc. The output driver support should (in theory) be complete, but it
hasn't been well tested due to scarcity of suitable displays.

  I'm running 2.6.35-rc4+ with a hacked up xf86-video-intel
 with this patch:

Please submit this as a format-patch to the list for inclusion.

 With that patch and DefaultDepth 30, I get a mostly working system,
 but there's no direct rendering (seems to be disabled because DRI is
 disabled because it runs only at depths 16 and 24) title bars on
 gnome-terminal draw incorrectly.

Hmm, the render paths should work on 10bpc surfaces. Please do file bugs
for the failures.

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] Fwd: State of 10 bits/channel?

2010-07-23 Thread Andrew Lutomirski
On Fri, Jul 23, 2010 at 8:26 AM, Chris Wilson ch...@chris-wilson.co.uk wrote:
 On Thu, 22 Jul 2010 17:34:09 -0400, Andrew Lutomirski l...@mit.edu wrote:
 [resend b/c I used the wrong from address last time]

 I have a 10 bit/channel monitor (DisplayPort) which works quite nicely
 in 8 bit mode.  I saw some patches from Peter Clifton related to 10
 bit support go in awhile ago.

 [snip]

 What is the hardware capable of, and what is the state of affairs
 right now?

 AIUI, all the intel chips are capable of driving 30-bit displays. The
 later generations are more flexible and faster with greater bit depth for
 LUTs etc. The output driver support should (in theory) be complete, but it
 hasn't been well tested due to scarcity of suitable displays.

OK.  I'll try to figure out how to program the output driver.  Do we
want to drive outputs at 30 bits even when the primary surface is 24
bits?  (Benefit: 10-bit LUTs.  Disadvantage: Could break existing
setups.)

Expect lots of questions from me when I run into things that are
unclear or wrong in the docs.


  I'm running 2.6.35-rc4+ with a hacked up xf86-video-intel
 with this patch:

 Please submit this as a format-patch to the list for inclusion.

You only say that because there's one line too few of context to see
how dumb the patch is.  I'll fix and submit.


 With that patch and DefaultDepth 30, I get a mostly working system,
 but there's no direct rendering (seems to be disabled because DRI is
 disabled because it runs only at depths 16 and 24) title bars on
 gnome-terminal draw incorrectly.

 Hmm, the render paths should work on 10bpc surfaces. Please do file bugs
 for the failures.

Will do.  I'll also see how close to working I can get DRI in 30-bit
mode.  (Enabling it right now exposes a silly bug in the compiz
startup scripts which results in having no window manager at all on
F13.  I'll figure out whose bug it is and file that one as well.)


 --
 Chris Wilson, Intel Open Source Technology Centre

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH] drm/i915: Repeat unbinding during free if interrupted

2010-07-23 Thread Chris Wilson
If during the freeing of an object the unbind is interrupted a system
call, which is quite possible if we have outstanding GPU writes that
must be flused, the unbind is silently aborted. This still leaves the
AGP region and backing pages allocated, and perhaps more importantly,
the object remains upon the various lists exposing us to memory
corruption.

I think this is the cause behind the use-after-free, such as

  Bug 15664 - Graphics hang and kernel backtrace when starting Azureus
  with Compiz enabled
  https://bugzilla.kernel.org/show_bug.cgi?id=15664

Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
Cc: sta...@kernel.org
---
 drivers/gpu/drm/i915/i915_gem.c |5 -
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 3c7c0f7..a4c0b44 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4292,6 +4292,7 @@ void i915_gem_free_object(struct drm_gem_object *obj)
 {
struct drm_device *dev = obj-dev;
struct drm_i915_gem_object *obj_priv = to_intel_bo(obj);
+   int ret;
 
trace_i915_gem_object_destroy(obj);
 
@@ -4301,7 +4302,9 @@ void i915_gem_free_object(struct drm_gem_object *obj)
if (obj_priv-phys_obj)
i915_gem_detach_phys_object(dev, obj);
 
-   i915_gem_object_unbind(obj);
+   do {
+   ret = i915_gem_object_unbind(obj);
+   } while (ret == -ERESTARTSYS);
 
if (obj_priv-mmap_offset)
i915_gem_free_mmap_offset(obj);
-- 
1.7.1

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH] drm/i915: Repeat unbinding during free if interrupted (v2)

2010-07-23 Thread Chris Wilson
If during the freeing of an object the unbind is interrupted by a system
call, which is quite possible if we have outstanding GPU writes that
must be flushed, the unbind is silently aborted. This still leaves the
AGP region and backing pages allocated, and perhaps more importantly,
the object remains upon the various lists exposing us to memory
corruption.

I think this is the cause behind the use-after-free, such as

  Bug 15664 - Graphics hang and kernel backtrace when starting Azureus
  with Compiz enabled
  https://bugzilla.kernel.org/show_bug.cgi?id=15664

v2: Daniel Vetter reminded me that kernel space programming is never easy.
We cannot simply spin to clear the pending signal and so must deferred
the freeing of the object until later.

Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
Cc: sta...@kernel.org
---
 drivers/gpu/drm/i915/i915_drv.h |8 +
 drivers/gpu/drm/i915/i915_gem.c |   56 --
 2 files changed, 49 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 0e7bf85..a66503c 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -551,6 +551,14 @@ typedef struct drm_i915_private {
struct list_head fence_list;
 
/**
+* List of objects currently pending being freed.
+*
+* These objects are no longer in use, but due to a signal
+* we were prevented from freeing them at the appointed time.
+*/
+   struct list_head deferred_free_list;
+
+   /**
 * We leave the user IRQ off as much as possible,
 * but this means that requests will finish and never
 * be retired once the system goes idle. Set a timer to
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 3c7c0f7..28a71e4 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -52,6 +52,7 @@ static void i915_gem_clear_fence_reg(struct drm_gem_object 
*obj);
 static int i915_gem_phys_pwrite(struct drm_device *dev, struct drm_gem_object 
*obj,
struct drm_i915_gem_pwrite *args,
struct drm_file *file_priv);
+static void i915_gem_free_object_tail(struct drm_gem_object *obj);
 
 static LIST_HEAD(shrink_list);
 static DEFINE_SPINLOCK(shrink_list_lock);
@@ -1707,6 +1708,15 @@ i915_gem_retire_requests_ring(struct drm_device *dev,
drm_i915_private_t *dev_priv = dev-dev_private;
uint32_t seqno;
 
+   if (!list_empty(dev_priv-mm.deferred_free_list)) {
+   struct drm_i915_gem_object *obj_priv, *tmp;
+
+   list_for_each_entry_safe(obj_priv, tmp,
+dev_priv-mm.deferred_free_list,
+list)
+   i915_gem_free_object_tail(obj_priv-base);
+   }
+
if (!ring-status_page.page_addr
|| list_empty(ring-request_list))
return;
@@ -1929,11 +1939,12 @@ i915_gem_object_unbind(struct drm_gem_object *obj)
 * before we unbind.
 */
ret = i915_gem_object_set_to_cpu_domain(obj, 1);
-   if (ret) {
-   if (ret != -ERESTARTSYS)
-   DRM_ERROR(set_domain failed: %d\n, ret);
+   if (ret == -ERESTARTSYS)
return ret;
-   }
+   /* Continue on if we fail due to EIO, the GPU is hung so we
+* should be safe and we need to cleanup or else we might
+* cause memory corruption through use-after-free.
+*/
 
BUG_ON(obj_priv-active);
 
@@ -1967,7 +1978,7 @@ i915_gem_object_unbind(struct drm_gem_object *obj)
 
trace_i915_gem_object_unbind(obj);
 
-   return 0;
+   return ret;
 }
 
 int
@@ -4288,20 +4299,19 @@ int i915_gem_init_object(struct drm_gem_object *obj)
return 0;
 }
 
-void i915_gem_free_object(struct drm_gem_object *obj)
+static void i915_gem_free_object_tail(struct drm_gem_object *obj)
 {
struct drm_device *dev = obj-dev;
+   drm_i915_private_t *dev_priv = dev-dev_private;
struct drm_i915_gem_object *obj_priv = to_intel_bo(obj);
+   int ret;
 
-   trace_i915_gem_object_destroy(obj);
-
-   while (obj_priv-pin_count  0)
-   i915_gem_object_unpin(obj);
-
-   if (obj_priv-phys_obj)
-   i915_gem_detach_phys_object(dev, obj);
-
-   i915_gem_object_unbind(obj);
+   ret = i915_gem_object_unbind(obj);
+   if (ret == -ERESTARTSYS) {
+   list_move_tail(obj_priv-list,
+  dev_priv-mm.deferred_free_list);
+   return;
+   }
 
if (obj_priv-mmap_offset)
i915_gem_free_mmap_offset(obj);
@@ -4313,6 +4323,22 @@ void i915_gem_free_object(struct drm_gem_object *obj)
kfree(obj_priv);
 }
 
+void 

[Intel-gfx] [PATCH] drm/i915: Repeat unbinding during free if interrupted (v3)

2010-07-23 Thread Chris Wilson
If during the freeing of an object the unbind is interrupted by a system
call, which is quite possible if we have outstanding GPU writes that
must be flushed, the unbind is silently aborted. This still leaves the
AGP region and backing pages allocated, and perhaps more importantly,
the object remains upon the various lists exposing us to memory
corruption.

I think this is the cause behind the use-after-free, such as

  Bug 15664 - Graphics hang and kernel backtrace when starting Azureus
  with Compiz enabled
  https://bugzilla.kernel.org/show_bug.cgi?id=15664

v2: Daniel Vetter reminded me that kernel space programming is never easy.
We cannot simply spin to clear the pending signal and so must deferred
the freeing of the object until later.
v3: Run from the top level retire requests.

Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
Cc: sta...@kernel.org
---
 drivers/gpu/drm/i915/i915_drv.h |8 +
 drivers/gpu/drm/i915/i915_gem.c |   56 --
 2 files changed, 49 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 0e7bf85..a66503c 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -551,6 +551,14 @@ typedef struct drm_i915_private {
struct list_head fence_list;
 
/**
+* List of objects currently pending being freed.
+*
+* These objects are no longer in use, but due to a signal
+* we were prevented from freeing them at the appointed time.
+*/
+   struct list_head deferred_free_list;
+
+   /**
 * We leave the user IRQ off as much as possible,
 * but this means that requests will finish and never
 * be retired once the system goes idle. Set a timer to
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 3c7c0f7..1f2c5f5 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -52,6 +52,7 @@ static void i915_gem_clear_fence_reg(struct drm_gem_object 
*obj);
 static int i915_gem_phys_pwrite(struct drm_device *dev, struct drm_gem_object 
*obj,
struct drm_i915_gem_pwrite *args,
struct drm_file *file_priv);
+static void i915_gem_free_object_tail(struct drm_gem_object *obj);
 
 static LIST_HEAD(shrink_list);
 static DEFINE_SPINLOCK(shrink_list_lock);
@@ -1746,6 +1747,15 @@ i915_gem_retire_requests(struct drm_device *dev)
 {
drm_i915_private_t *dev_priv = dev-dev_private;
 
+   if (!list_empty(dev_priv-mm.deferred_free_list)) {
+   struct drm_i915_gem_object *obj_priv, *tmp;
+
+   list_for_each_entry_safe(obj_priv, tmp,
+dev_priv-mm.deferred_free_list,
+list)
+   i915_gem_free_object_tail(obj_priv-base);
+   }
+
i915_gem_retire_requests_ring(dev, dev_priv-render_ring);
if (HAS_BSD(dev))
i915_gem_retire_requests_ring(dev, dev_priv-bsd_ring);
@@ -1929,11 +1939,12 @@ i915_gem_object_unbind(struct drm_gem_object *obj)
 * before we unbind.
 */
ret = i915_gem_object_set_to_cpu_domain(obj, 1);
-   if (ret) {
-   if (ret != -ERESTARTSYS)
-   DRM_ERROR(set_domain failed: %d\n, ret);
+   if (ret == -ERESTARTSYS)
return ret;
-   }
+   /* Continue on if we fail due to EIO, the GPU is hung so we
+* should be safe and we need to cleanup or else we might
+* cause memory corruption through use-after-free.
+*/
 
BUG_ON(obj_priv-active);
 
@@ -1967,7 +1978,7 @@ i915_gem_object_unbind(struct drm_gem_object *obj)
 
trace_i915_gem_object_unbind(obj);
 
-   return 0;
+   return ret;
 }
 
 int
@@ -4288,20 +4299,19 @@ int i915_gem_init_object(struct drm_gem_object *obj)
return 0;
 }
 
-void i915_gem_free_object(struct drm_gem_object *obj)
+static void i915_gem_free_object_tail(struct drm_gem_object *obj)
 {
struct drm_device *dev = obj-dev;
+   drm_i915_private_t *dev_priv = dev-dev_private;
struct drm_i915_gem_object *obj_priv = to_intel_bo(obj);
+   int ret;
 
-   trace_i915_gem_object_destroy(obj);
-
-   while (obj_priv-pin_count  0)
-   i915_gem_object_unpin(obj);
-
-   if (obj_priv-phys_obj)
-   i915_gem_detach_phys_object(dev, obj);
-
-   i915_gem_object_unbind(obj);
+   ret = i915_gem_object_unbind(obj);
+   if (ret == -ERESTARTSYS) {
+   list_move_tail(obj_priv-list,
+  dev_priv-mm.deferred_free_list);
+   return;
+   }
 
if (obj_priv-mmap_offset)
i915_gem_free_mmap_offset(obj);
@@ -4313,6 +4323,22 @@ void i915_gem_free_object(struct 

Re: [Intel-gfx] [PATCH] drm/i915: Repeat unbinding during free if interrupted (v3)

2010-07-23 Thread Daniel Vetter
On Fri, Jul 23, 2010 at 03:54:44PM +0100, Chris Wilson wrote:
 If during the freeing of an object the unbind is interrupted by a system
 call, which is quite possible if we have outstanding GPU writes that
 must be flushed, the unbind is silently aborted. This still leaves the
 AGP region and backing pages allocated, and perhaps more importantly,
 the object remains upon the various lists exposing us to memory
 corruption.
 
 I think this is the cause behind the use-after-free, such as
 
   Bug 15664 - Graphics hang and kernel backtrace when starting Azureus
   with Compiz enabled
   https://bugzilla.kernel.org/show_bug.cgi?id=15664
 
 v2: Daniel Vetter reminded me that kernel space programming is never easy.
 We cannot simply spin to clear the pending signal and so must deferred
 the freeing of the object until later.
 v3: Run from the top level retire requests.
 
 Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
 Cc: sta...@kernel.org

Cleaning up the deferred free list in retire_request looks much saner than
what I've had in mind when discussing this on irc.

Reviewed-By: Daniel Vetter dan...@ffwll.ch
-- 
Daniel Vetter
Mail: dan...@ffwll.ch
Mobile: +41 (0)79 365 57 48
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH] drm/i915: Repeat unbinding during free if interrupted (v4)

2010-07-23 Thread Chris Wilson
If during the freeing of an object the unbind is interrupted by a system
call, which is quite possible if we have outstanding GPU writes that
must be flushed, the unbind is silently aborted. This still leaves the
AGP region and backing pages allocated, and perhaps more importantly,
the object remains upon the various lists exposing us to memory
corruption.

I think this is the cause behind the use-after-free, such as

  Bug 15664 - Graphics hang and kernel backtrace when starting Azureus
  with Compiz enabled
  https://bugzilla.kernel.org/show_bug.cgi?id=15664

v2: Daniel Vetter reminded me that kernel space programming is never easy.
We cannot simply spin to clear the pending signal and so must deferred
the freeing of the object until later.
v3: Run from the top level retire requests.
v4: Tested with P(return -ERESTARTSYS)=.5 from i915_gem_do_wait_request()

Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
Cc: sta...@kernel.org
Reviewed-by: Daniel Vetter dan...@ffwll.ch
---
 drivers/gpu/drm/i915/i915_drv.h |8 +
 drivers/gpu/drm/i915/i915_gem.c |   57 --
 2 files changed, 50 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 0e7bf85..a66503c 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -551,6 +551,14 @@ typedef struct drm_i915_private {
struct list_head fence_list;
 
/**
+* List of objects currently pending being freed.
+*
+* These objects are no longer in use, but due to a signal
+* we were prevented from freeing them at the appointed time.
+*/
+   struct list_head deferred_free_list;
+
+   /**
 * We leave the user IRQ off as much as possible,
 * but this means that requests will finish and never
 * be retired once the system goes idle. Set a timer to
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 78835f8..9424d9b 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -52,6 +52,7 @@ static void i915_gem_clear_fence_reg(struct drm_gem_object 
*obj);
 static int i915_gem_phys_pwrite(struct drm_device *dev, struct drm_gem_object 
*obj,
struct drm_i915_gem_pwrite *args,
struct drm_file *file_priv);
+static void i915_gem_free_object_tail(struct drm_gem_object *obj);
 
 static LIST_HEAD(shrink_list);
 static DEFINE_SPINLOCK(shrink_list_lock);
@@ -1746,6 +1747,15 @@ i915_gem_retire_requests(struct drm_device *dev)
 {
drm_i915_private_t *dev_priv = dev-dev_private;
 
+   if (!list_empty(dev_priv-mm.deferred_free_list)) {
+   struct drm_i915_gem_object *obj_priv, *tmp;
+
+   list_for_each_entry_safe(obj_priv, tmp,
+dev_priv-mm.deferred_free_list,
+list)
+   i915_gem_free_object_tail(obj_priv-base);
+   }
+
i915_gem_retire_requests_ring(dev, dev_priv-render_ring);
if (HAS_BSD(dev))
i915_gem_retire_requests_ring(dev, dev_priv-bsd_ring);
@@ -1929,11 +1939,12 @@ i915_gem_object_unbind(struct drm_gem_object *obj)
 * before we unbind.
 */
ret = i915_gem_object_set_to_cpu_domain(obj, 1);
-   if (ret) {
-   if (ret != -ERESTARTSYS)
-   DRM_ERROR(set_domain failed: %d\n, ret);
+   if (ret == -ERESTARTSYS)
return ret;
-   }
+   /* Continue on if we fail due to EIO, the GPU is hung so we
+* should be safe and we need to cleanup or else we might
+* cause memory corruption through use-after-free.
+*/
 
BUG_ON(obj_priv-active);
 
@@ -1967,7 +1978,7 @@ i915_gem_object_unbind(struct drm_gem_object *obj)
 
trace_i915_gem_object_unbind(obj);
 
-   return 0;
+   return ret;
 }
 
 int
@@ -4292,20 +4303,19 @@ int i915_gem_init_object(struct drm_gem_object *obj)
return 0;
 }
 
-void i915_gem_free_object(struct drm_gem_object *obj)
+static void i915_gem_free_object_tail(struct drm_gem_object *obj)
 {
struct drm_device *dev = obj-dev;
+   drm_i915_private_t *dev_priv = dev-dev_private;
struct drm_i915_gem_object *obj_priv = to_intel_bo(obj);
+   int ret;
 
-   trace_i915_gem_object_destroy(obj);
-
-   while (obj_priv-pin_count  0)
-   i915_gem_object_unpin(obj);
-
-   if (obj_priv-phys_obj)
-   i915_gem_detach_phys_object(dev, obj);
-
-   i915_gem_object_unbind(obj);
+   ret = i915_gem_object_unbind(obj);
+   if (ret == -ERESTARTSYS) {
+   list_move_tail(obj_priv-list,
+  dev_priv-mm.deferred_free_list);
+   return;
+   }
 
if 

[Intel-gfx] rfc: breaking old userspace gamma for 10-bit support

2010-07-23 Thread Andrew Lutomirski
AFAICT intel hardware wants a 129-entry LUT when using high precision
gamma ramps.  Rather than hacking some kind of decimation into the
kernel driver (and thus silently breaking DirectColor), I'd like to
teach userspace how to deal with variable gamma sizes.

gnome-color-manager already more-or-less supports arbitrary gamma ramp
sizes (supposedly), dispwin ought to do it, and there might not be any
other software that really cares.  gnome-screensaver saves and
restores the gamma ramp, and I haven't checked if it works right for
funny sizes.

The worst problem we'll have is that current xf86-drv-intel can't
handle non-256 gamma sizes at all.  So if we change the kernel we'll
break it completely.

One option is to have the kernel report gamma_size = 129 but still
accept 256 and decimate itself.  That might cause current userspace to
keep working (except for DirectColor).

Any thoughts?
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH] drm/i915: Repeat unbinding during free if interrupted (v5)

2010-07-23 Thread Chris Wilson
If during the freeing of an object the unbind is interrupted by a system
call, which is quite possible if we have outstanding GPU writes that
must be flushed, the unbind is silently aborted. This still leaves the
AGP region and backing pages allocated, and perhaps more importantly,
the object remains upon the various lists exposing us to memory
corruption.

I think this is the cause behind the use-after-free, such as

  Bug 15664 - Graphics hang and kernel backtrace when starting Azureus
  with Compiz enabled
  https://bugzilla.kernel.org/show_bug.cgi?id=15664

v2: Daniel Vetter reminded me that kernel space programming is never easy.
We cannot simply spin to clear the pending signal and so must deferred
the freeing of the object until later.
v3: Run from the top level retire requests.
v4: Tested with P(return -ERESTARTSYS)=.5 from i915_gem_do_wait_request()
v5: Rebase against Eric's for-linus tree.

Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
Cc: sta...@kernel.org
Cc: Daniel Vetter dan...@ffwll.ch
---
 drivers/gpu/drm/i915/i915_drv.c |2 +-
 drivers/gpu/drm/i915/i915_drv.h |   11 -
 drivers/gpu/drm/i915/i915_gem.c |   96 ---
 3 files changed, 70 insertions(+), 39 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 119692f..5044f65 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -340,7 +340,7 @@ int i965_reset(struct drm_device *dev, u8 flags)
/*
 * Clear request list
 */
-   i915_gem_retire_requests(dev, dev_priv-render_ring);
+   i915_gem_retire_requests(dev);
 
if (need_display)
i915_save_display(dev);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 5a0100e..e3ff35e 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -547,6 +547,14 @@ typedef struct drm_i915_private {
struct list_head fence_list;
 
/**
+* List of objects currently pending being freed.
+*
+* These objects are no longer in use, but due to a signal
+* we were prevented from freeing them at the appointed time.
+*/
+   struct list_head deferred_free_list;
+
+   /**
 * We leave the user IRQ off as much as possible,
 * but this means that requests will finish and never
 * be retired once the system goes idle. Set a timer to
@@ -955,8 +963,7 @@ uint32_t i915_get_gem_seqno(struct drm_device *dev,
 bool i915_seqno_passed(uint32_t seq1, uint32_t seq2);
 int i915_gem_object_get_fence_reg(struct drm_gem_object *obj);
 int i915_gem_object_put_fence_reg(struct drm_gem_object *obj);
-void i915_gem_retire_requests(struct drm_device *dev,
-struct intel_ring_buffer *ring);
+void i915_gem_retire_requests(struct drm_device *dev);
 void i915_gem_retire_work_handler(struct work_struct *work);
 void i915_gem_clflush_object(struct drm_gem_object *obj);
 int i915_gem_object_set_domain(struct drm_gem_object *obj,
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 2d0e109..6a371e3 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -53,6 +53,7 @@ static int i915_gem_evict_from_inactive_list(struct 
drm_device *dev);
 static int i915_gem_phys_pwrite(struct drm_device *dev, struct drm_gem_object 
*obj,
struct drm_i915_gem_pwrite *args,
struct drm_file *file_priv);
+static void i915_gem_free_object_tail(struct drm_gem_object *obj);
 
 static LIST_HEAD(shrink_list);
 static DEFINE_SPINLOCK(shrink_list_lock);
@@ -1709,9 +1710,9 @@ i915_get_gem_seqno(struct drm_device *dev,
 /**
  * This function clears the request list as sequence numbers are passed.
  */
-void
-i915_gem_retire_requests(struct drm_device *dev,
-   struct intel_ring_buffer *ring)
+static void
+i915_gem_retire_requests_ring(struct drm_device *dev,
+ struct intel_ring_buffer *ring)
 {
drm_i915_private_t *dev_priv = dev-dev_private;
uint32_t seqno;
@@ -1751,6 +1752,25 @@ i915_gem_retire_requests(struct drm_device *dev,
 }
 
 void
+i915_gem_retire_requests(struct drm_device *dev)
+{
+   drm_i915_private_t *dev_priv = dev-dev_private;
+
+   if (!list_empty(dev_priv-mm.deferred_free_list)) {
+   struct drm_i915_gem_object *obj_priv, *tmp;
+
+   list_for_each_entry_safe(obj_priv, tmp,
+dev_priv-mm.deferred_free_list,
+list)
+   i915_gem_free_object_tail(obj_priv-base);
+   }
+
+   i915_gem_retire_requests_ring(dev, dev_priv-render_ring);
+   if (HAS_BSD(dev))
+   i915_gem_retire_requests_ring(dev, dev_priv-bsd_ring);
+}
+
+void
 

[Intel-gfx] [PATCH] drm/i915: handle shared framebuffers when flipping

2010-07-23 Thread Jesse Barnes
If a framebuffer is shared across CRTCs, the x,y position of one of them
is likely to be something other than the origin (e.g. for extended
desktop configs).  So calculate the offset at flip time so such
configurations can work.

Fixes https://bugs.freedesktop.org/show_bug.cgi?id=28518.

Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org
---
 drivers/gpu/drm/i915/intel_display.c |   10 +++---
 1 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 8359c50..cf5a91d 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4863,7 +4863,7 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
struct drm_gem_object *obj;
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
struct intel_unpin_work *work;
-   unsigned long flags;
+   unsigned long flags, offset;
int pipesrc_reg = (intel_crtc-pipe == 0) ? PIPEASRC : PIPEBSRC;
int ret, pipesrc;
u32 flip_mask;
@@ -4925,19 +4925,23 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
while (I915_READ(ISR)  flip_mask)
;
 
+   /* Offset into the new buffer for cases of shared fbs between CRTCs */
+   offset = obj_priv-gtt_offset;
+   offset += (crtc-y * fb-pitch) + (crtc-x * (fb-bits_per_pixel) / 8);
+
BEGIN_LP_RING(4);
if (IS_I965G(dev)) {
OUT_RING(MI_DISPLAY_FLIP |
 MI_DISPLAY_FLIP_PLANE(intel_crtc-plane));
OUT_RING(fb-pitch);
-   OUT_RING(obj_priv-gtt_offset | obj_priv-tiling_mode);
+   OUT_RING(offset | obj_priv-tiling_mode);
pipesrc = I915_READ(pipesrc_reg); 
OUT_RING(pipesrc  0x0fff0fff);
} else {
OUT_RING(MI_DISPLAY_FLIP_I915 |
 MI_DISPLAY_FLIP_PLANE(intel_crtc-plane));
OUT_RING(fb-pitch);
-   OUT_RING(obj_priv-gtt_offset);
+   OUT_RING(offset);
OUT_RING(MI_NOOP);
}
ADVANCE_LP_RING();
-- 
1.7.0.4

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH 3/3] drm/i915: Attempt to uncouple object after catastrophic failure in unbind

2010-07-23 Thread Chris Wilson
If we fail to flush outstanding GPU writes but return the memory to the
system, we risk corrupting memory should the GPU recovery and complete
those writes. On the other hand, if we bail early and free the object
then we have a definite use-after-free and real memory corruption.
Choose the lesser of two evils, since in order to recover from the hung
GPU we need to completely reset it, those pending writes should
never happen.

Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
---
 drivers/gpu/drm/i915/i915_gem.c |   11 ++-
 1 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 4a21053..56fd9e2 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1967,11 +1967,12 @@ i915_gem_object_unbind(struct drm_gem_object *obj)
 * before we unbind.
 */
ret = i915_gem_object_set_to_cpu_domain(obj, 1);
-   if (ret) {
-   if (ret != -ERESTARTSYS)
-   DRM_ERROR(set_domain failed: %d\n, ret);
+   if (ret == -ERESTARTSYS)
return ret;
-   }
+   /* Continue on if we fail due to EIO, the GPU is hung so we
+* should be safe and we need to cleanup or else we might
+* cause memory corruption through use-after-free.
+*/
 
BUG_ON(obj_priv-active);
 
@@ -2007,7 +2008,7 @@ i915_gem_object_unbind(struct drm_gem_object *obj)
 
trace_i915_gem_object_unbind(obj);
 
-   return 0;
+   return ret;
 }
 
 static struct drm_gem_object *
-- 
1.7.1

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] KMS and kdb

2010-07-23 Thread Pedro Ribeiro
Hi,

I've been trying to use kdb in 2.6.35-rc6 together with kms.

I have kgdb/kdb compiled in with keyboard support and i915 compiled as
a module. When I try to do
echo kms,kbd  /sys/module/kgdboc/parameters/kgdboc
it says write error.

Echoing kbd only works. However, if I invoke kdb inside X, it freezes
until I type go followed by enter. Of couse I can't see what I'm
typing, because X is frozen and it did not switch to the VT.

Am I doing something wrong? I have a GM45 chipset.

Regards,
Pedro
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx