[MMTests] IO metadata on XFS

2012-07-02 Thread Mel Gorman
Adding dri-devel and a few others because an i915 patch contributed to
the regression.

On Mon, Jul 02, 2012 at 03:32:15PM +0100, Mel Gorman wrote:
> On Mon, Jul 02, 2012 at 02:32:26AM -0400, Christoph Hellwig wrote:
> > > It increases the CPU overhead (dirty_inode can be called up to 4
> > > times per write(2) call, IIRC), so with limited numbers of
> > > threads/limited CPU power it will result in lower performance. Where
> > > you have lots of CPU power, there will be little difference in
> > > performance...
> > 
> > When I checked it it could only be called twice, and we'd already
> > optimize away the second call.  I'd defintively like to track down where
> > the performance changes happend, at least to a major version but even
> > better to a -rc or git commit.
> > 
> 
> By all means feel free to run the test yourself and run the bisection :)
> 
> It's rare but on this occasion the test machine is idle so I started an
> automated git bisection. As you know the milage with an automated bisect
> varies so it may or may not find the right commit. Test machine is sandy so
> http://www.csn.ul.ie/~mel/postings/mmtests-20120424/global-dhp__io-metadata-xfs/sandy/comparison.html
> is the report of interest. The script is doing a full search between v3.3 and
> v3.4 for a point where average files/sec for fsmark-single drops below 25000.
> I did not limit the search to fs/xfs on the off-chance that it is an
> apparently unrelated patch that caused the problem.
> 

It was obvious very quickly that there were two distinct regression so I
ran two bisections. One led to a XFS and the other led to an i915 patch
that enables RC6 to reduce power usage.

[c999a223: xfs: introduce an allocation workqueue]
[aa464191: drm/i915: enable plain RC6 on Sandy Bridge by default]

gdm was running on the machine so i915 would have been in use.  In case it
is of interest this is the log of the bisection. Lines beginning with #
are notes I made and all other lines are from the bisection script. The
second-last column is the files/sec recorded by fsmark.

# MARK v3.3..v3.4 Search for BAD files/sec -lt 28000
# BAD 16536
# GOOD 34757
Mon Jul 2 15:46:13 IST 2012 sandy xfsbisect 
141124c02059eee9dbc5c86ea797b1ca888e77f7 37454 good
Mon Jul 2 15:56:06 IST 2012 sandy xfsbisect 
55a320308902f7a0746569ee57eeb3f254e6ed16 25192 bad
Mon Jul 2 16:08:34 IST 2012 sandy xfsbisect 
281b05392fc2cb26209b4d85abaf4889ab1991f3 38807 good
Mon Jul 2 16:18:02 IST 2012 sandy xfsbisect 
a8364db2030d093cde0f07951628e55454e1 37553 good
Mon Jul 2 16:27:22 IST 2012 sandy xfsbisect 
d2a2fc18d98d8ee2dec1542efc7f47beec256144 36676 good
Mon Jul 2 16:36:48 IST 2012 sandy xfsbisect 
2e7580b0e75d771d93e24e681031a165b1d31071 37756 good
Mon Jul 2 16:46:36 IST 2012 sandy xfsbisect 
532bfc851a7475fb6a36c1e953aa395798a7cca7 25416 bad
Mon Jul 2 16:56:10 IST 2012 sandy xfsbisect 
0c9aac08261512d70d7d4817bd222abca8b6bdd6 38486 good
Mon Jul 2 17:05:40 IST 2012 sandy xfsbisect 
0fc9d1040313047edf6a39fd4d7c7defdca97c62 37970 good
Mon Jul 2 17:16:01 IST 2012 sandy xfsbisect 
5a5881cdeec2c019b5c9a307800218ee029f7f61 24493 bad
Mon Jul 2 17:21:15 IST 2012 sandy xfsbisect 
f616137519feb17b849894fcbe634a021d3fa7db 24405 bad
Mon Jul 2 17:26:16 IST 2012 sandy xfsbisect 
5575acc7807595687288b3bbac15103f2a5462e1 37336 good
Mon Jul 2 17:31:25 IST 2012 sandy xfsbisect 
c999a223c2f0d31c64ef7379814cea1378b2b800 24552 bad
Mon Jul 2 17:36:34 IST 2012 sandy xfsbisect 
1a1d772433d42aaff7315b3468fef5951604f5c6 36872 good
# c999a223c2f0d31c64ef7379814cea1378b2b800 is the first bad commit
# [c999a223: xfs: introduce an allocation workqueue]
#
# MARK c999a223c2f0d31c64ef7379814cea1378b2b800..v3.4 Search for BAD files/sec 
-lt 2
# BAD  16536
# GOOD 24552
Mon Jul 2 17:48:39 IST 2012 sandy xfsbisect 
b2094ef840697bc8ca5d17a83b7e30fad5f1e9fa 37435 good
Mon Jul 2 17:58:12 IST 2012 sandy xfsbisect 
d2a2fc18d98d8ee2dec1542efc7f47beec256144 38303 good
Mon Jul 2 18:08:18 IST 2012 sandy xfsbisect 
5d32c88f0b94061b3af2e3ade92422407282eb12 16718 bad
Mon Jul 2 18:18:02 IST 2012 sandy xfsbisect 
2f7fa1be66dce77608330c5eb918d6360b5525f2 24964 good
Mon Jul 2 18:24:14 IST 2012 sandy xfsbisect 
923f79743c76583ed4684e2c80c8da51a7268af3 24963 good
Mon Jul 2 18:33:49 IST 2012 sandy xfsbisect 
b61c37f57988567c84359645f8202a7c84bc798a 24824 good
Mon Jul 2 18:40:20 IST 2012 sandy xfsbisect 
20a2a811602b16c42ce88bada3d52712cdfb988b 17155 bad
Mon Jul 2 18:50:12 IST 2012 sandy xfsbisect 
78fb72f7936c01d5b426c03a691eca082b03f2b9 38494 good
Mon Jul 2 19:00:24 IST 2012 sandy xfsbisect 
e1a7eb08ee097e97e928062a242b0de5b2599a11 25033 good
Mon Jul 2 19:10:24 IST 2012 sandy xfsbisect 
97effadb65ed08809e1720c8d3ee80b73a93665c 16520 bad
Mon Jul 2 19:16:16 IST 2012 sandy xfsbisect 
25e341cfc33d94435472983825163e97fe370a6c 16748 bad
Mon Jul 2 19:21:52 IST 2012 sandy xfsbisect 
7dd4906586274f3945f2aeaaa5a33b451c3b4bba 24957 good
Mon Jul 2 19:27:35 IST 2012 sandy xfsbisect 
aa46419186992e6b8b8010319f0ca7f40a0d13f5 17088 bad
Mon Jul 

[Bug 33309] [855GM] GPU freeze due to overlay hang

2012-07-02 Thread bugzilla-dae...@freedesktop.org
https://bugs.freedesktop.org/show_bug.cgi?id=33309

--- Comment #29 from Daniel Vetter  2012-07-02 13:29:18 PDT 
---
(In reply to comment #28)
> are there any news on this issue?
> The 3.4 and 3.5-rc series seem stable wrt this issue,
> but unfortunately something broke resume from s2ram badly,
> the backlight stays off and the machine does not respond
> even to SysRq and I need to do a hard power-off.

That's good news. Can you try to bisect the backlight regression that has
been introduce in 3.4 and open a new bug report? That usually helps in fixing
it ...

-- 
Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are the assignee for the bug.


[Bug 28622] radeon video lockup

2012-07-02 Thread bugzilla-dae...@bugzilla.kernel.org
https://bugzilla.kernel.org/show_bug.cgi?id=28622





--- Comment #11 from Alex Deucher   2012-07-02 
20:21:02 ---
Is this still an issue with a more recent kernel (3.x)?

-- 
Configure bugmail: https://bugzilla.kernel.org/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are watching the assignee of the bug.


[Bug 33309] [855GM] GPU freeze due to overlay hang

2012-07-02 Thread bugzilla-dae...@freedesktop.org
https://bugs.freedesktop.org/show_bug.cgi?id=33309

--- Comment #28 from stefan  2012-07-02 13:13:59 PDT 
---
Hi,
are there any news on this issue?
The 3.4 and 3.5-rc series seem stable wrt this issue,
but unfortunately something broke resume from s2ram badly,
the backlight stays off and the machine does not respond
even to SysRq and I need to do a hard power-off.

Cheers,
Stefan.

-- 
Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are the assignee for the bug.


[PATCH] drm/radeon: add an exclusive lock for GPU reset

2012-07-02 Thread Christian König
On 02.07.2012 18:41, Jerome Glisse wrote:
> On Mon, Jul 2, 2012 at 12:26 PM, Christian K?nig
>  wrote:
>> On 02.07.2012 17:39, j.glisse at gmail.com wrote:
>>> From: Jerome Glisse 
>>>
>>> GPU reset need to be exclusive, one happening at a time. For this
>>> add a rw semaphore so that any path that trigger GPU activities
>>> have to take the semaphore as a reader thus allowing concurency.
>>>
>>> The GPU reset path take the semaphore as a writer ensuring that
>>> no concurrent reset take place.
>>>
>>> Signed-off-by: Jerome Glisse 
>> NAK, that isn't as bad as the cs mutex was but still to complicated. It's
>> just too far up in the call stack, e.g. it tries to catch ioctl operations,
>> instead of catching the underlying hardware operation which is caused by the
>> ioctl/ttm/etc...
>>
>> Why not just take the ring look as I suggested?
>>
>>
> No we can't use ring lock, we need to protect suspend/resume path and
> we need an exclusive lock for that so we need a reset mutex at the
> very least. But instead of having a reset mutex i prefer using a rw
> lock so that we can stop ioctl until a reset goes through an let
> pending ioctl take proper action. Think about multiple context trying
> to reset GPU ...
>
> Really this is the best option, the rw locking wont induce any lock
> contention execept in gpu reset case which is anyway bad news.
Why? That makes no sense to me. Well I don't want to prevent lock 
contention, but understand why we should add locking at the ioctl level. 
That violates locking rule number one "lock data instead of code" (or in 
our case "lock hardware access instead of code path") and it really is 
the reason why we ended up with the cs_mutex protecting practically 
everything.

Multiple context trying to reset the GPU should be pretty fine, current 
it would just reset the GPU twice, but in the future asic_reset should 
be much more fine grained and only reset the parts of the GPU which 
really needs an reset.

Cheers,
Christian.


[PATCH] drm/radeon: add an exclusive lock for GPU reset

2012-07-02 Thread Christian König
On 02.07.2012 17:39, j.glisse at gmail.com wrote:
> From: Jerome Glisse 
>
> GPU reset need to be exclusive, one happening at a time. For this
> add a rw semaphore so that any path that trigger GPU activities
> have to take the semaphore as a reader thus allowing concurency.
>
> The GPU reset path take the semaphore as a writer ensuring that
> no concurrent reset take place.
>
> Signed-off-by: Jerome Glisse 
NAK, that isn't as bad as the cs mutex was but still to complicated. 
It's just too far up in the call stack, e.g. it tries to catch ioctl 
operations, instead of catching the underlying hardware operation which 
is caused by the ioctl/ttm/etc...

Why not just take the ring look as I suggested?

Christian.
> ---
>   drivers/gpu/drm/radeon/radeon.h|1 +
>   drivers/gpu/drm/radeon/radeon_cs.c |5 +
>   drivers/gpu/drm/radeon/radeon_device.c |2 ++
>   drivers/gpu/drm/radeon/radeon_gem.c|7 +++
>   4 files changed, 15 insertions(+)
>
> diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
> index 77b4519b..29d6986 100644
> --- a/drivers/gpu/drm/radeon/radeon.h
> +++ b/drivers/gpu/drm/radeon/radeon.h
> @@ -1446,6 +1446,7 @@ struct radeon_device {
>   struct device   *dev;
>   struct drm_device   *ddev;
>   struct pci_dev  *pdev;
> + struct rw_semaphore exclusive_lock;
>   /* ASIC */
>   union radeon_asic_configconfig;
>   enum radeon_family  family;
> diff --git a/drivers/gpu/drm/radeon/radeon_cs.c 
> b/drivers/gpu/drm/radeon/radeon_cs.c
> index f1b7527..7ee6491 100644
> --- a/drivers/gpu/drm/radeon/radeon_cs.c
> +++ b/drivers/gpu/drm/radeon/radeon_cs.c
> @@ -499,7 +499,9 @@ int radeon_cs_ioctl(struct drm_device *dev, void *data, 
> struct drm_file *filp)
>   struct radeon_cs_parser parser;
>   int r;
>   
> + down_read(>exclusive_lock);
>   if (!rdev->accel_working) {
> + up_read(>exclusive_lock);
>   return -EBUSY;
>   }
>   /* initialize parser */
> @@ -512,6 +514,7 @@ int radeon_cs_ioctl(struct drm_device *dev, void *data, 
> struct drm_file *filp)
>   if (r) {
>   DRM_ERROR("Failed to initialize parser !\n");
>   radeon_cs_parser_fini(, r);
> + up_read(>exclusive_lock);
>   r = radeon_cs_handle_lockup(rdev, r);
>   return r;
>   }
> @@ -520,6 +523,7 @@ int radeon_cs_ioctl(struct drm_device *dev, void *data, 
> struct drm_file *filp)
>   if (r != -ERESTARTSYS)
>   DRM_ERROR("Failed to parse relocation %d!\n", r);
>   radeon_cs_parser_fini(, r);
> + up_read(>exclusive_lock);
>   r = radeon_cs_handle_lockup(rdev, r);
>   return r;
>   }
> @@ -533,6 +537,7 @@ int radeon_cs_ioctl(struct drm_device *dev, void *data, 
> struct drm_file *filp)
>   }
>   out:
>   radeon_cs_parser_fini(, r);
> + up_read(>exclusive_lock);
>   r = radeon_cs_handle_lockup(rdev, r);
>   return r;
>   }
> diff --git a/drivers/gpu/drm/radeon/radeon_device.c 
> b/drivers/gpu/drm/radeon/radeon_device.c
> index f654ba8..c8fdb40 100644
> --- a/drivers/gpu/drm/radeon/radeon_device.c
> +++ b/drivers/gpu/drm/radeon/radeon_device.c
> @@ -988,6 +988,7 @@ int radeon_gpu_reset(struct radeon_device *rdev)
>   int r;
>   int resched;
>   
> + down_write(>exclusive_lock);
>   radeon_save_bios_scratch_regs(rdev);
>   /* block TTM */
>   resched = ttm_bo_lock_delayed_workqueue(>mman.bdev);
> @@ -1007,6 +1008,7 @@ int radeon_gpu_reset(struct radeon_device *rdev)
>   dev_info(rdev->dev, "GPU reset failed\n");
>   }
>   
> + up_write(>exclusive_lock);
>   return r;
>   }
>   
> diff --git a/drivers/gpu/drm/radeon/radeon_gem.c 
> b/drivers/gpu/drm/radeon/radeon_gem.c
> index d9b0809..f99db63 100644
> --- a/drivers/gpu/drm/radeon/radeon_gem.c
> +++ b/drivers/gpu/drm/radeon/radeon_gem.c
> @@ -215,12 +215,14 @@ int radeon_gem_create_ioctl(struct drm_device *dev, 
> void *data,
>   uint32_t handle;
>   int r;
>   
> + down_read(>exclusive_lock);
>   /* create a gem object to contain this object in */
>   args->size = roundup(args->size, PAGE_SIZE);
>   r = radeon_gem_object_create(rdev, args->size, args->alignment,
>   args->initial_domain, false,
>   false, );
>   if (r) {
> + up_read(>exclusive_lock);
>   r = radeon_gem_handle_lockup(rdev, r);
>   return r;
>   }
> @@ -228,10 +230,12 @@ int radeon_gem_create_ioctl(struct drm_device *dev, 
> void *data,
>   /* drop reference from allocate - handle holds it now */
>   drm_gem_object_unreference_unlocked(gobj);
>   if (r) {
> + up_read(>exclusive_lock);
>   r = radeon_gem_handle_lockup(rdev, r);
>  

[RFC v2] implicit drm synchronization wip with dma-buf

2012-07-02 Thread Maarten Lankhorst
Well,

V2 time! I was hinted to look at ttm_execbuf_util, and it does indeed contain 
some nice code.
My goal was to extend dma-buf in a generic way now, some elements from v1 are 
remaining,
most notably a dma-buf used for syncing. However it is expected now that 
dma-buf syncing will
work in a very specific way now, slightly more strict than in v1.

Instead of each buffer having their own dma-buf I put in 1 per command 
submission.

This submission hasn't been run-time tested yet, but I expect the api to go 
something like this.

Intended to be used like this:

list_init();
add_list_tail(>entry, );
add_list_tail(>entry, );
add_list_tail(>entry, );

r = dmabufmgr_eu_reserve_buffers();
if (r) return r;

// add waits on cpu or gpu
list_for_each_entry(validate, ...) {

if (!validate->sync_buf)
continue;

// Check attachments to see if we already imported sync_buf
// somewhere, if not attach to it.
// waiting until cur_seq - dmabuf->sync_val >= 0 on either cpu
// or as command submitted to gpu
// sync_buf itself is a dma-buf, so it should be trivial
// TODO: sync_buf should NEVER be validated, add is_sync_buf to dma_buf?

// If this step fails: dmabufmgr_eu_backoff_reservation
// else:
// dmabufmgr_eu_fence_buffer_objects(our_own_sync_buf,
// hwchannel * max(minhwalign, 4), ++counter[hwchannel]);

// XXX: Do we still require a minimum alignment? I set up 16 for 
nouveau,
// but this is no longer needed in this design since it only matters
// for writes for which nouveau would already control the offset.
}

// Some time after execbuffer was executed, doesn't have to be right away but 
before
// getting in the danger of our own counter wrapping around:
// grab dmabufmgr.lru_lock, and cleanup by unreffing sync_buf when
// sync_buf == ownbuf, sync_ofs == ownofs, and sync_val == saved_counter
// In the meantime someone else or even us might have reserved this 
dma_buf
// again, which is why all those checks are needed before unreffing.

diff --git a/drivers/base/Makefile b/drivers/base/Makefile
index 5aa2d70..86e7598 100644
--- a/drivers/base/Makefile
+++ b/drivers/base/Makefile
@@ -10,7 +10,7 @@ obj-$(CONFIG_CMA) += dma-contiguous.o
 obj-y  += power/
 obj-$(CONFIG_HAS_DMA)  += dma-mapping.o
 obj-$(CONFIG_HAVE_GENERIC_DMA_COHERENT) += dma-coherent.o
-obj-$(CONFIG_DMA_SHARED_BUFFER) += dma-buf.o
+obj-$(CONFIG_DMA_SHARED_BUFFER) += dma-buf.o dma-buf-mgr.o dma-buf-mgr-eu.o
 obj-$(CONFIG_ISA)  += isa.o
 obj-$(CONFIG_FW_LOADER)+= firmware_class.o
 obj-$(CONFIG_NUMA) += node.o
diff --git a/drivers/base/dma-buf-mgr-eu.c b/drivers/base/dma-buf-mgr-eu.c
new file mode 100644
index 000..27ebc68
--- /dev/null
+++ b/drivers/base/dma-buf-mgr-eu.c
@@ -0,0 +1,170 @@
+/*
+ * Copyright (C) 2012 Canonical Ltd
+ *
+ * Based on ttm_bo.c which bears the following copyright notice,
+ * but is dual licensed:
+ *
+ * Copyright (c) 2006-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 
+#include 
+#include 
+
+static void dmabufmgr_eu_backoff_reservation_locked(struct list_head *list)
+{
+   struct dmabufmgr_validate *entry;
+
+   list_for_each_entry(entry, list, head) {
+   struct dma_buf *bo = entry->bo;
+   if (!entry->reserved)
+   continue;
+
+   entry->reserved = false;
+   atomic_set(>reserved, 0);
+   wake_up_all(>event_queue);
+   if (entry->sync_buf)
+   dma_buf_put(entry->sync_buf);
+   entry->sync_buf = NULL;
+   }
+}
+
+static int

[Bug 51658] r200 (& possibly radeon) DRI fixes for gnome shell on Mesa 8.0.3

2012-07-02 Thread bugzilla-dae...@freedesktop.org
https://bugs.freedesktop.org/show_bug.cgi?id=51658

--- Comment #8 from Eugene St Leger  2012-07-02 11:04:54 
PDT ---
A summary of all of the patches follows.

r200 essential patches (1st 3 patches) for gnome shell:
"Essential patch to disable texture formats that are reported unrenderable
elsewhere in driver."
"Essential patch to allow 2048 pixel blits."
"Essential patch to reapply dirtied texenv registers."

suggested minor fixes patch (4th patch):
"Unessential fixes/enhancements patch."

proposed r200 blit patch (5th patch) - unknown importance:
"Proposed blit register dirtying patch."

untested but probably essential r100/radeon patch (7th patch) for gnome shell:
"Untested but probably essential patch to allow 2048 pixel blits on r100."

optional unrecommended patches to supplement blit patches already mentioned
above (6th & 8th patches):
"Optional patch to warn (once) when a blit with 2048 pixel dimension occurs on
r200."
"Optional untested patch to warn (once) when a blit with 2048 pixel dimension
occurs on r100."

-- 
Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are the assignee for the bug.


[PATCH 1/3] drm/radeon: move ring locking out of reset path

2012-07-02 Thread Christian König
On 02.07.2012 17:41, Jerome Glisse wrote:
> On Fri, Jun 29, 2012 at 12:15 PM, Michel D?nzer  wrote:
>> On Fre, 2012-06-29 at 17:18 +0200, Christian K?nig wrote:
>>> On 29.06.2012 17:09, Michel D?nzer wrote:
 On Fre, 2012-06-29 at 16:45 +0200, Christian K?nig wrote:
> Hold the ring lock the whole time the reset is in progress,
> otherwise another process can submit new jobs.
 Sounds good, but doesn't this create other paths (e.g. initialization,
 resume) where the ring is being accessed without holding the lock? Isn't
 that a problem?
>>> Thought about that also.
>>>
>>> For init I'm pretty sure that no application can submit commands before
>>> we are done, otherwise we are doomed anyway.
>>>
>>> For resume I'm not really sure, but I think that applications are
>>> resumed after the hardware driver had a chance of doing so.
>> I hope you're right... but if it's not too much trouble, it might be
>> better to be safe than sorry and take the lock for those paths as well.
>>
>>
> NAK this is the wrong way to solve the issue, we need a global lock on
> all path that can trigger gpu activities. Previously it was the cs
> mutex, but i haven't thought about it too much when it got removed. So
> to fix the situation i am sending a patch with rw semaphore.
So what I'm missing? What else can trigger GPU activity when not the rings?

I'm currently working on ring-partial resets and also resets where you 
only skip over a single faulty IB instead of flushing the whole ring. 
And my current idea for that to work is that we hold the ring lock while 
we do suspend, ring_save, asic_reset, resume and ring_restore.

Christian.

>
> Cheers,
> Jerome
>




[Bug 51658] r200 (& possibly radeon) DRI fixes for gnome shell on Mesa 8.0.3

2012-07-02 Thread bugzilla-dae...@freedesktop.org
https://bugs.freedesktop.org/show_bug.cgi?id=51658

--- Comment #7 from Eugene St Leger  2012-07-02 10:51:10 
PDT ---
Created attachment 63718
  --> https://bugs.freedesktop.org/attachment.cgi?id=63718
Optional untested patch to warn (once) when a blit with 2048 pixel dimension
occurs on r100.

If 2048 pixel blits cause graphical glitches/problems on r100, this patch can
be used to provide a single warning. This patch is untested.

-- 
Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are the assignee for the bug.


[Bug 51658] r200 (& possibly radeon) DRI fixes for gnome shell on Mesa 8.0.3

2012-07-02 Thread bugzilla-dae...@freedesktop.org
https://bugs.freedesktop.org/show_bug.cgi?id=51658

--- Comment #6 from Eugene St Leger  2012-07-02 10:49:07 
PDT ---
Created attachment 63717
  --> https://bugs.freedesktop.org/attachment.cgi?id=63717
Untested but probably essential patch to allow 2048 pixel blits on r100.

Without this patch, gnome shell is expected to crash. This patch is untested.

-- 
Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are the assignee for the bug.


[Bug 51658] r200 (& possibly radeon) DRI fixes for gnome shell on Mesa 8.0.3

2012-07-02 Thread bugzilla-dae...@freedesktop.org
https://bugs.freedesktop.org/show_bug.cgi?id=51658

--- Comment #4 from Eugene St Leger  2012-07-02 10:40:51 
PDT ---
Created attachment 63715
  --> https://bugs.freedesktop.org/attachment.cgi?id=63715
Proposed blit register dirtying patch.

It appears bliting dirties some registers without notifying. This proposed
patch notifies of register dirtying.

-- 
Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are the assignee for the bug.


[Bug 51658] r200 (& possibly radeon) DRI fixes for gnome shell on Mesa 8.0.3

2012-07-02 Thread bugzilla-dae...@freedesktop.org
https://bugs.freedesktop.org/show_bug.cgi?id=51658

--- Comment #3 from Eugene St Leger  2012-07-02 10:37:47 
PDT ---
Created attachment 63714
  --> https://bugs.freedesktop.org/attachment.cgi?id=63714
Unessential fixes/enhancements patch.

Minor spelling fixes & enhancements.

-- 
Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are the assignee for the bug.


[Bug 51658] r200 (& possibly radeon) DRI fixes for gnome shell on Mesa 8.0.3

2012-07-02 Thread bugzilla-dae...@freedesktop.org
https://bugs.freedesktop.org/show_bug.cgi?id=51658

--- Comment #2 from Eugene St Leger  2012-07-02 10:34:41 
PDT ---
Created attachment 63713
  --> https://bugs.freedesktop.org/attachment.cgi?id=63713
Essential patch to reapply dirtied texenv registers.

Without this patch, colour corruption happens with xv.  xf86-video-ati textured
xv dirties texenv registers and r200 DRI does not reapply them.

-- 
Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are the assignee for the bug.


[Bug 51658] r200 (& possibly radeon) DRI fixes for gnome shell on Mesa 8.0.3

2012-07-02 Thread bugzilla-dae...@freedesktop.org
https://bugs.freedesktop.org/show_bug.cgi?id=51658

--- Comment #1 from Eugene St Leger  2012-07-02 10:29:44 
PDT ---
Created attachment 63712
  --> https://bugs.freedesktop.org/attachment.cgi?id=63712
Essential patch to allow 2048 pixel blits.

Without this patch, gnome shell crashes. 2048 pixel blits are used.

-- 
Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are the assignee for the bug.


[Bug 51658] New: r200 (& possibly radeon) DRI fixes for gnome shell on Mesa 8.0.3

2012-07-02 Thread bugzilla-dae...@freedesktop.org
https://bugs.freedesktop.org/show_bug.cgi?id=51658

 Bug #: 51658
   Summary: r200 (& possibly radeon) DRI fixes for gnome shell on
Mesa 8.0.3
Classification: Unclassified
   Product: Mesa
   Version: 8.0
  Platform: All
OS/Version: All
Status: NEW
  Severity: major
  Priority: medium
 Component: Drivers/DRI/r200
AssignedTo: dri-devel at lists.freedesktop.org
ReportedBy: grimrc at yahoo.com


Created attachment 63711
  --> https://bugs.freedesktop.org/attachment.cgi?id=63711
Essential patch to disable texture formats that are reported unrenderable
elsewhere in driver.

Without r200-tex-format-fix.patch, gnome shell crashes. Texture formats are
selected by radeonChooseTexFormat but reported as unrenderable by
radeonIsFormatRenderable.

-- 
Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are the assignee for the bug.


[PATCH] staging: drm/omap: add rotation properties

2012-07-02 Thread Ville Syrjälä
On Fri, Jun 29, 2012 at 07:17:23AM -0500, Rob Clark wrote:
> On Fri, Jun 29, 2012 at 5:46 AM, Tomi Valkeinen  
> wrote:
> > On Wed, 2012-06-27 at 09:06 -0500, Rob Clark wrote:
> >> From: Rob Clark 
> >>
> >> Use tiled buffers for rotated/reflected scanout, with CRTC and plane
> >> properties as the interface for userspace to configure rotation.
> >>
> >> Signed-off-by: Rob Clark 
> >
> >> +/* this should probably be in drm-core to standardize amongst drivers */
> >> +#define DRM_ROTATE_0 0
> >> +#define DRM_ROTATE_90 ? ? ? ?1
> >> +#define DRM_ROTATE_180 ? ? ? 2
> >> +#define DRM_ROTATE_270 ? ? ? 3
> >> +#define DRM_REFLECT_X ? ? ? ?4
> >> +#define DRM_REFLECT_Y ? ? ? ?5
> >
> > Are both reflect X and Y needed? You can get all the possible
> > orientations with just one of the reflects.
> >
> > And I think the word "mirror" represents nicely what the reflect does,
> > i.e. if you look at the mirror, the image you see is flipped
> > horizontally.
> 
> fwiw these values are aligned with what is used in userspace xrandr..
> an earlier version of this patch used just 3 bits, which where aligned
> with what the omap hw uses and can describe all possible combinations
> of mirroring and isomorphic rotation (x-invert, y-invert, and
> xy-flip).  But the advantage of the xrandr approach is you can more
> easily leave out bits for rotation/mirroring modes that your hw does
> not support.. for example if some hw supports only certain rotations
> or does not support mirror/reflect.

When I hear "mirror" I always think of it as the happening after
rotation, but that's not how xrandr does things. I suppose "reflection"
has more or less the same connotation for me, but since it's already
used by xrandr, I know I have to do the necessary mental gymnastics.

If you think about it in terms of matrix multiplication, xrandr does
things like this: x' = Rot * Ref * x, whereas for me the more natural
order (for whatever reason) would be x' = Ref * Rot * x, where x is the
source coordinates, and x' the destination coordinates.

IIRC in dss mirroring was specified in the post-rotation way, and
the direction of the rotation was also opposite to xrandr (cw in dss,
ccw in xrandr).

So FWIW, my vote goes to "reflect" if we want to match xrandr
semantics. Also I agree on the number of bits, six is better than
three since it allows you to describe the hw capabilities.

-- 
Ville Syrj?l?
Intel OTC


[PATCH v3] DRM: Add DRM kms/fb cma helper

2012-07-02 Thread Lars-Peter Clausen
This patchset introduces a set of helper function for implementing the KMS
framebuffer layer for drivers which use the drm gem CMA helper function.

Signed-off-by: Lars-Peter Clausen 

---
Note: This patch depends on Sascha's "DRM: add drm gem CMA helper" patch

Changes since v2:
* Adapt to changes in the GEM CMA helper
* Add basic buffer size checking in drm_fb_cma_create
Changes since v1:
* Some spelling fixes
* Add missing kfree in drm_fb_cma_alloc error path
* Add multi-plane support
---
 drivers/gpu/drm/Kconfig |   10 +
 drivers/gpu/drm/Makefile|1 +
 drivers/gpu/drm/drm_fb_cma_helper.c |  393 +++
 include/drm/drm_fb_cma_helper.h |   27 +++
 4 files changed, 431 insertions(+)
 create mode 100644 drivers/gpu/drm/drm_fb_cma_helper.c
 create mode 100644 include/drm/drm_fb_cma_helper.h

diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index 41bbd95..e511c9a 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -41,6 +41,16 @@ config DRM_GEM_CMA_HELPER
help
  Choose this if you need the GEM CMA helper functions

+config DRM_KMS_CMA_HELPER
+   tristate
+   select DRM_GEM_CMA_HELPER
+   select DRM_KMS_HELPER
+   select FB_SYS_FILLRECT
+   select FB_SYS_COPYAREA
+   select FB_SYS_IMAGEBLIT
+   help
+ Choose this if you need the KMS cma helper functions
+
 config DRM_TDFX
tristate "3dfx Banshee/Voodoo3+"
depends on DRM && PCI
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index 6e9e948..5dcb1a5 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -18,6 +18,7 @@ drm-$(CONFIG_COMPAT) += drm_ioc32.o
 drm-$(CONFIG_DRM_GEM_CMA_HELPER) += drm_gem_cma_helper.o

 drm_kms_helper-y := drm_fb_helper.o drm_crtc_helper.o drm_dp_i2c_helper.o
+drm_kms_helper-$(CONFIG_DRM_KMS_CMA_HELPER) += drm_fb_cma_helper.o

 obj-$(CONFIG_DRM_KMS_HELPER) += drm_kms_helper.o

diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c 
b/drivers/gpu/drm/drm_fb_cma_helper.c
new file mode 100644
index 000..9042233
--- /dev/null
+++ b/drivers/gpu/drm/drm_fb_cma_helper.c
@@ -0,0 +1,393 @@
+/*
+ * drm kms/fb cma (contiguous memory allocator) helper functions
+ *
+ * Copyright (C) 2012 Analog Device Inc.
+ *   Author: Lars-Peter Clausen 
+ *
+ * Based on udl_fbdev.c
+ *  Copyright (C) 2012 Red Hat
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+struct drm_fb_cma {
+   struct drm_framebuffer  fb;
+   struct drm_gem_cma_object   *obj[4];
+};
+
+struct drm_fbdev_cma {
+   struct drm_fb_helperfb_helper;
+   struct drm_fb_cma   *fb;
+};
+
+static inline struct drm_fbdev_cma *to_fbdev_cma(struct drm_fb_helper *helper)
+{
+   return container_of(helper, struct drm_fbdev_cma, fb_helper);
+}
+
+static inline struct drm_fb_cma *to_fb_cma(struct drm_framebuffer *fb)
+{
+   return container_of(fb, struct drm_fb_cma, fb);
+}
+
+static void drm_fb_cma_destroy(struct drm_framebuffer *fb)
+{
+   struct drm_fb_cma *fb_cma = to_fb_cma(fb);
+   int i;
+
+   for (i = 0; i < 4; i++) {
+   if (fb_cma->obj[i])
+   
drm_gem_object_unreference_unlocked(_cma->obj[i]->base);
+   }
+
+   drm_framebuffer_cleanup(fb);
+   kfree(fb_cma);
+}
+
+static int drm_fb_cma_create_handle(struct drm_framebuffer *fb,
+   struct drm_file *file_priv, unsigned int *handle)
+{
+   struct drm_fb_cma *fb_cma = to_fb_cma(fb);
+
+   return drm_gem_handle_create(file_priv,
+   _cma->obj[0]->base, handle);
+}
+
+static struct drm_framebuffer_funcs drm_fb_cma_funcs = {
+   .destroy= drm_fb_cma_destroy,
+   .create_handle  = drm_fb_cma_create_handle,
+};
+
+static struct drm_fb_cma *drm_fb_cma_alloc(struct drm_device *dev,
+   struct drm_mode_fb_cmd2 *mode_cmd, struct drm_gem_cma_object **obj,
+   unsigned int num_planes)
+{
+   struct drm_fb_cma *fb_cma;
+   int ret;
+   int i;
+
+   fb_cma = kzalloc(sizeof(*fb_cma), GFP_KERNEL);
+   if (!fb_cma)
+   return ERR_PTR(-ENOMEM);
+
+   ret = drm_framebuffer_init(dev, _cma->fb, _fb_cma_funcs);
+   if (ret) {
+   dev_err(dev->dev, "Failed to initalize framebuffer: %d\n", ret);
+   kfree(fb_cma);
+   return ERR_PTR(ret);
+   }
+
+   

[PATCH] drm/radeon: fix rare segfault

2012-07-02 Thread Alex Deucher
On Mon, Jul 2, 2012 at 12:40 PM,   wrote:
> From: Jerome Glisse 
>
> In gem idle/busy ioctl the radeon object was derefenced after
> drm_gem_object_unreference_unlocked which in case the object
> have been destroyed lead to use of a possibly free pointer with
> possibly wrong data.
>
> Signed-off-by: Jerome Glisse 

Reviewed-by: Alex Deucher 

> ---
>  drivers/gpu/drm/radeon/radeon_gem.c |   10 ++
>  1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/radeon_gem.c 
> b/drivers/gpu/drm/radeon/radeon_gem.c
> index 74176c5..c8838fc 100644
> --- a/drivers/gpu/drm/radeon/radeon_gem.c
> +++ b/drivers/gpu/drm/radeon/radeon_gem.c
> @@ -325,6 +325,7 @@ int radeon_gem_mmap_ioctl(struct drm_device *dev, void 
> *data,
>  int radeon_gem_busy_ioctl(struct drm_device *dev, void *data,
>   struct drm_file *filp)
>  {
> +   struct radeon_device *rdev = dev->dev_private;
> struct drm_radeon_gem_busy *args = data;
> struct drm_gem_object *gobj;
> struct radeon_bo *robj;
> @@ -350,13 +351,14 @@ int radeon_gem_busy_ioctl(struct drm_device *dev, void 
> *data,
> break;
> }
> drm_gem_object_unreference_unlocked(gobj);
> -   r = radeon_gem_handle_lockup(robj->rdev, r);
> +   r = radeon_gem_handle_lockup(rdev, r);
> return r;
>  }
>
>  int radeon_gem_wait_idle_ioctl(struct drm_device *dev, void *data,
>   struct drm_file *filp)
>  {
> +   struct radeon_device *rdev = dev->dev_private;
> struct drm_radeon_gem_wait_idle *args = data;
> struct drm_gem_object *gobj;
> struct radeon_bo *robj;
> @@ -369,10 +371,10 @@ int radeon_gem_wait_idle_ioctl(struct drm_device *dev, 
> void *data,
> robj = gem_to_radeon_bo(gobj);
> r = radeon_bo_wait(robj, NULL, false);
> /* callback hw specific functions if any */
> -   if (robj->rdev->asic->ioctl_wait_idle)
> -   robj->rdev->asic->ioctl_wait_idle(robj->rdev, robj);
> +   if (rdev->asic->ioctl_wait_idle)
> +   robj->rdev->asic->ioctl_wait_idle(rdev, robj);
> drm_gem_object_unreference_unlocked(gobj);
> -   r = radeon_gem_handle_lockup(robj->rdev, r);
> +   r = radeon_gem_handle_lockup(rdev, r);
> return r;
>  }
>
> --
> 1.7.10.2
>
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel


Bogus video resolution in Linux 3.5-rc4

2012-07-02 Thread Adam Jackson
On 6/26/12 3:21 AM, Takashi Iwai wrote:

> From: Takashi Iwai 
> Subject: [PATCH] drm: edid: Don't add inferred modes with higher resolution
>
> When a monitor EDID doesn't give the preferred bit, driver assumes
> that the mode with the higest resolution and rate is the preferred
> mode.  Meanwhile the recent changes for allowing more modes in the
> GFT/CVT ranges give actually more modes, and some modes may be over
> the native size.  Thus such a mode would be picked up as the preferred
> mode although it's no native resolution.
>
> For avoiding such a problem, this patch limits the addition of
> inferred modes by checking not to be greater than other modes.
> Also, it checks the duplicated mode entry at the same time.

This is a little aggressive on CRTs, but whatever, better than what's there.

Reviewed-by: Adam Jackson 

- ajax


[PATCH libdrm] intel: Fix build failure in test_decode.c

2012-07-02 Thread Ben Widawsky
On Sat, 30 Jun 2012 13:12:45 +0300
Lauri Kasanen  wrote:

> Hi list
> 
> The recently released libdrm 2.4.37 does not compile the Intel part:
> 
> test_decode.c: In function 'compare_batch':
> test_decode.c:107: error: implicit declaration of function 'open_memstream'
> 
> PS: Please CC me.
> 
> Signed-off-by: Lauri Kasanen 
> ---
>  intel/test_decode.c |2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
> 
> diff --git a/intel/test_decode.c b/intel/test_decode.c
> index c9ab7ad..0fcdf3b 100644
> --- a/intel/test_decode.c
> +++ b/intel/test_decode.c
> @@ -21,6 +21,8 @@
>   * IN THE SOFTWARE.
>   */
>  
> +#define _GNU_SOURCE
> +
>  #include 
>  #include 
>  #include 

I can't reproduce this. Can anyone else confirm this is broken, and if
so that the above patch fixes it?


-- 
Ben Widawsky, Intel Open Source Technology Center


[PATCH 1/3] pci_regs: define LNKSTA2 pcie cap + bits.

2012-07-02 Thread Bjorn Helgaas
On Thu, Jun 28, 2012 at 3:45 AM, Dave Airlie  wrote:
> On Wed, Jun 27, 2012 at 8:35 AM, Dave Airlie  wrote:
>> From: Dave Airlie 
>>
>> We need these for detecting the max link speed for drm drivers.
>
> Hi Bjorn,
>
> Can you ack this patch so I can carry it in the drm-next tree? we need
> these regs for getting the PCIE v2/v3 supported link speeds.

Sure.  Note that I already have a patch in my "next" tree that will
conflict with this because it contains this hunk:

 #define  PCI_EXP_OBFF_WAKE_EN  0x6000  /* OBFF using WAKE# signaling */
+#define PCI_CAP_EXP_ENDPOINT_SIZEOF_V2 44  /* v2 endpoints end here */
 #define PCI_EXP_LNKCTL248  /* Link Control 2 */

at this commit:
http://git.kernel.org/?p=linux/kernel/git/helgaas/pci.git;a=commitdiff;h=a0dee2ed0cdc666b5622f1fc74979355a6b36850

I think the comments would make more sense as "2.5GT/s supported",
etc., since these bits don't tell you anything about the "Current Link
Speed".

Maybe it would make sense for me to incorporate this patch into my
"next" branch, and you could carry both commits in your drm-next tree?
 I don't know what results in the easiest merge later.  But if you
need it:

Acked-by: Bjorn Helgaas 

Bjorn

>> Signed-off-by: Dave Airlie 
>> ---
>> ?include/linux/pci_regs.h | ? ?5 +
>> ?1 files changed, 5 insertions(+), 0 deletions(-)
>>
>> diff --git a/include/linux/pci_regs.h b/include/linux/pci_regs.h
>> index 4b608f5..7f04132 100644
>> --- a/include/linux/pci_regs.h
>> +++ b/include/linux/pci_regs.h
>> @@ -521,6 +521,11 @@
>> ?#define ?PCI_EXP_OBFF_MSGA_EN ?0x2000 ?/* OBFF enable with Message type A */
>> ?#define ?PCI_EXP_OBFF_MSGB_EN ?0x4000 ?/* OBFF enable with Message type B */
>> ?#define ?PCI_EXP_OBFF_WAKE_EN ?0x6000 ?/* OBFF using WAKE# signaling */
>> +#define PCI_EXP_LNKCAP2 ? ? ? ? ? ? ? ?44 ? ? ?/* Link Capability 2 */
>> +#define ?PCI_EXP_LNKCAP2_SLS_2_5GB 0x01 ? ? ? ?/* Current Link Speed 
>> 2.5GT/s */
>> +#define ?PCI_EXP_LNKCAP2_SLS_5_0GB 0x02 ? ? ? ?/* Current Link Speed 
>> 5.0GT/s */
>> +#define ?PCI_EXP_LNKCAP2_SLS_8_0GB 0x04 ? ? ? ?/* Current Link Speed 
>> 8.0GT/s */
>> +#define ?PCI_EXP_LNKCAP2_CROSSLINK 0x100 /* Crosslink supported */
>> ?#define PCI_EXP_LNKCTL2 ? ? ? ? ? ? ? ?48 ? ? ?/* Link Control 2 */
>> ?#define PCI_EXP_SLTCTL2 ? ? ? ? ? ? ? ?56 ? ? ?/* Slot Control 2 */
>>
>> --
>> 1.7.7.6
>>
>> ___
>> dri-devel mailing list
>> dri-devel at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/dri-devel


[Bug 28622] radeon video lockup

2012-07-02 Thread bugzilla-dae...@bugzilla.kernel.org
https://bugzilla.kernel.org/show_bug.cgi?id=28622


Alan  changed:

   What|Removed |Added

 CC||alan at lxorguk.ukuu.org.uk
 Kernel Version|2.6.37  |2.6.39




-- 
Configure bugmail: https://bugzilla.kernel.org/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are watching the assignee of the bug.


[PATCH] drm/radeon: add an exclusive lock for GPU reset

2012-07-02 Thread Jerome Glisse
On Mon, Jul 2, 2012 at 1:05 PM, Christian K?nig  
wrote:
> On 02.07.2012 18:41, Jerome Glisse wrote:
>>
>> On Mon, Jul 2, 2012 at 12:26 PM, Christian K?nig
>>  wrote:
>>>
>>> On 02.07.2012 17:39, j.glisse at gmail.com wrote:

 From: Jerome Glisse 

 GPU reset need to be exclusive, one happening at a time. For this
 add a rw semaphore so that any path that trigger GPU activities
 have to take the semaphore as a reader thus allowing concurency.

 The GPU reset path take the semaphore as a writer ensuring that
 no concurrent reset take place.

 Signed-off-by: Jerome Glisse 
>>>
>>> NAK, that isn't as bad as the cs mutex was but still to complicated. It's
>>> just too far up in the call stack, e.g. it tries to catch ioctl
>>> operations,
>>> instead of catching the underlying hardware operation which is caused by
>>> the
>>> ioctl/ttm/etc...
>>>
>>> Why not just take the ring look as I suggested?
>>>
>>>
>> No we can't use ring lock, we need to protect suspend/resume path and
>> we need an exclusive lock for that so we need a reset mutex at the
>> very least. But instead of having a reset mutex i prefer using a rw
>> lock so that we can stop ioctl until a reset goes through an let
>> pending ioctl take proper action. Think about multiple context trying
>> to reset GPU ...
>>
>> Really this is the best option, the rw locking wont induce any lock
>> contention execept in gpu reset case which is anyway bad news.
>
> Why? That makes no sense to me. Well I don't want to prevent lock
> contention, but understand why we should add locking at the ioctl level.
> That violates locking rule number one "lock data instead of code" (or in our
> case "lock hardware access instead of code path") and it really is the
> reason why we ended up with the cs_mutex protecting practically everything.
>
> Multiple context trying to reset the GPU should be pretty fine, current it
> would just reset the GPU twice, but in the future asic_reset should be much
> more fine grained and only reset the parts of the GPU which really needs an
> reset.
>
> Cheers,
> Christian.

No multiple reset is not fine, try it your self and you will see all
kind of oops (strongly advise you to sync you hd before stress testing
that). Yes we need to protect code path because suspend/resume code
path is special one it touch many data in the device structure. GPU
reset is a rare event or should be a rare event.

I stress it we need at very least a mutex to protect gpu reset and i
will stand on that position because there is no other way around.
Using rw lock have a bonus of allowing proper handling of gpu reset
failure and that what the patch i sent to linus fix tree is about, so
to make drm next merge properly while preserving proper behavior in
gpu reset failure the rw semaphore is the best option.

Cheers,
Jerome


[PATCH, RFC] i.MX DRM support

2012-07-02 Thread Dirk Behme
On 02.07.2012 12:05, Sascha Hauer wrote:
> On Thu, Jun 14, 2012 at 03:43:22PM +0200, Sascha Hauer wrote:
>> Hi All,
>>
>> The following is the state-of-the-art i.MX IPU (Image Processing Unit)
>> DRM support.
>>
>> This code is around for quite some time now and has been posted several
>> times with different APIs, first with plain old framebuffer support, now
>> DRM, first platform device binding, now devicetree. Unfortunately there's
>> quite much code needed to get something useful out of the IPU, so these
>> patches haven't received a lot of attention from people not involved in
>> i.MX. I think we have now come to a point where this code needs more public
>> exposure and where it's easier to talk in incremental changes instead of
>> blobs. Therefore I request this to go to staging for some cycles.
> 
> Dave, Greg,
> 
> Comments to this one? I addressed the comments I received so far and am
> about to respin this series. Is it ok to put this to staging? If yes,
> should I move the whole stuff into drivers/staging/ or should it stay
> in drivers/gpu/drm with just a Kconfig dependency on STAGING?

We are very interested in this, so +1 from my side for this.

Many thanks and best regards

Dirk

Btw.: Based on

http://git.pengutronix.de/?p=imx/linux-2.6.git;a=shortlog;h=refs/heads/work/gpu/imx-drm-ipu-complete

with [1] below I fixed some compiler warnings.

I have some additional warnings

drivers/gpu/drm/drm_edid.c: In function 'drm_find_cea_extension':
drivers/gpu/drm/drm_edid.c:1466: warning: array subscript is above array 
bounds
drivers/gpu/drm/drm_edid.c: In function 'drm_detect_hdmi_monitor':
drivers/gpu/drm/drm_edid.c:1466: warning: array subscript is above array 
bounds
drivers/gpu/drm/drm_edid.c: In function 'drm_detect_monitor_audio':
drivers/gpu/drm/drm_edid.c:1466: warning: array subscript is above array 
bounds
drivers/gpu/drm/drm_edid.c: In function 'drm_edid_to_eld':
drivers/gpu/drm/drm_edid.c:1466: warning: array subscript is above array 
bounds
drivers/gpu/drm/drm_edid.c: In function 'drm_add_edid_modes':
drivers/gpu/drm/drm_edid.c:1466: warning: array subscript is above array 
bounds
drivers/gpu/drm/drm_edid.c:1466: warning: array subscript is above array 
bounds

but these seems to be compiler specific and not related to Sacha's patches.

[1]

From: Dirk Behme 
Subject: [PATCH 1/2] DRM i.MX: ldb: Fix compiler warnings

Fix the compiler warnings

drivers/gpu/drm/imx/imx-ldb.c: In function 'imx_ldb_encoder_mode_fixup':
drivers/gpu/drm/imx/imx-ldb.c:118: warning: unused variable 'imx_ldb_ch'
drivers/gpu/drm/imx/imx-ldb.c: In function 'imx_ldb_encoder_prepare':
drivers/gpu/drm/imx/imx-ldb.c:134: warning: format '%ld' expects type 
'long int', but argument 6 has type 'int'

Signed-off-by: Dirk Behme 

---
  drivers/gpu/drm/imx/imx-ldb.c |4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

Index: freescale-linux-2.6-imx.git/drivers/gpu/drm/imx/imx-ldb.c
===
--- freescale-linux-2.6-imx.git.orig/drivers/gpu/drm/imx/imx-ldb.c
+++ freescale-linux-2.6-imx.git/drivers/gpu/drm/imx/imx-ldb.c
@@ -115,9 +115,9 @@ static bool imx_ldb_encoder_mode_fixup(s
struct drm_display_mode *mode,
struct drm_display_mode *adjusted_mode)
  {
+/*
 struct imx_ldb_channel *imx_ldb_ch = enc_to_imx_ldb_ch(encoder);

-/*
 adjusted_mode->clock = clk_round_rate(imx_ldb_ch->clk_pll,
 adjusted_mode->clock * 1000) / 1000;
  */
@@ -131,7 +131,7 @@ static void imx_ldb_encoder_prepare(stru
 int ret;
 struct drm_display_mode *mode = >crtc->mode;

-   dev_dbg(ldb->dev, "%s: now: %ld want: %ld\n", __func__,
+   dev_dbg(ldb->dev, "%s: now: %ld want: %d\n", __func__,
 clk_get_rate(imx_ldb_ch->clk_pll),
 mode->clock * 1000 * 7);
 clk_set_rate(imx_ldb_ch->clk_pll, mode->clock * 1000 * 7);




[Bug 51652] New: [6550D SUMO] problems with secondar monitor on VGA, causing GPU lockups

2012-07-02 Thread bugzilla-dae...@freedesktop.org
https://bugs.freedesktop.org/show_bug.cgi?id=51652

 Bug #: 51652
   Summary: [6550D SUMO] problems with secondar monitor on VGA,
causing GPU lockups
Classification: Unclassified
   Product: DRI
   Version: XOrg CVS
  Platform: x86-64 (AMD64)
OS/Version: Linux (All)
Status: NEW
  Severity: critical
  Priority: medium
 Component: DRM/Radeon
AssignedTo: dri-devel at lists.freedesktop.org
ReportedBy: d.okias at gmail.com


Created attachment 63701
  --> https://bugs.freedesktop.org/attachment.cgi?id=63701
dmesg_VGAonBootConnected.txt

1) when is computer booted with VGA connected, ends with repeatly restarting
GPU

2) when is monitor connected on-fly in running Xorg, it seems fine (except
small artifacts, i guess EXA, on screen mostly on top of DVI and bottom VGA
trigerred by switching KDE virtual desktops). No errors in dmesg

3) when is DPMS activated and monitors suspends, back to online is returned
only DVI

4) randomly I found:
[drm:radeon_dp_link_train_cr] *ERROR* clock recovery reached max voltage
[drm:radeon_dp_link_train_cr] *ERROR* clock recovery failed

5) out-topic, exist even only on DVI: when is enabled OGL KWIN and OpenGL video
render, color of rendered context goes to white (every frame is whitier, until
it's #ff)

No HDMI or DP conneted to system.

DVI-D: LG IPS235 1920x1080
VGA: ASUS VW224 1680x1050
Gentoo ~amd64
Kernel,libdrm,mesa,ddx: git, KMS

I'm able to test patches against anything.

-- 
Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are the assignee for the bug.


[PATCH] drm/radeon: add an exclusive lock for GPU reset v2

2012-07-02 Thread j.gli...@gmail.com
From: Jerome Glisse 

GPU reset need to be exclusive, one happening at a time. For this
add a rw semaphore so that any path that trigger GPU activities
have to take the semaphore as a reader thus allowing concurency.

The GPU reset path take the semaphore as a writer ensuring that
no concurrent reset take place.

v2: init rw semaphore

Signed-off-by: Jerome Glisse 
---
 drivers/gpu/drm/radeon/radeon.h|1 +
 drivers/gpu/drm/radeon/radeon_cs.c |5 +
 drivers/gpu/drm/radeon/radeon_device.c |3 +++
 drivers/gpu/drm/radeon/radeon_gem.c|8 
 4 files changed, 17 insertions(+)

diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
index 77b4519b..29d6986 100644
--- a/drivers/gpu/drm/radeon/radeon.h
+++ b/drivers/gpu/drm/radeon/radeon.h
@@ -1446,6 +1446,7 @@ struct radeon_device {
struct device   *dev;
struct drm_device   *ddev;
struct pci_dev  *pdev;
+   struct rw_semaphore exclusive_lock;
/* ASIC */
union radeon_asic_configconfig;
enum radeon_family  family;
diff --git a/drivers/gpu/drm/radeon/radeon_cs.c 
b/drivers/gpu/drm/radeon/radeon_cs.c
index f1b7527..7ee6491 100644
--- a/drivers/gpu/drm/radeon/radeon_cs.c
+++ b/drivers/gpu/drm/radeon/radeon_cs.c
@@ -499,7 +499,9 @@ int radeon_cs_ioctl(struct drm_device *dev, void *data, 
struct drm_file *filp)
struct radeon_cs_parser parser;
int r;

+   down_read(>exclusive_lock);
if (!rdev->accel_working) {
+   up_read(>exclusive_lock);
return -EBUSY;
}
/* initialize parser */
@@ -512,6 +514,7 @@ int radeon_cs_ioctl(struct drm_device *dev, void *data, 
struct drm_file *filp)
if (r) {
DRM_ERROR("Failed to initialize parser !\n");
radeon_cs_parser_fini(, r);
+   up_read(>exclusive_lock);
r = radeon_cs_handle_lockup(rdev, r);
return r;
}
@@ -520,6 +523,7 @@ int radeon_cs_ioctl(struct drm_device *dev, void *data, 
struct drm_file *filp)
if (r != -ERESTARTSYS)
DRM_ERROR("Failed to parse relocation %d!\n", r);
radeon_cs_parser_fini(, r);
+   up_read(>exclusive_lock);
r = radeon_cs_handle_lockup(rdev, r);
return r;
}
@@ -533,6 +537,7 @@ int radeon_cs_ioctl(struct drm_device *dev, void *data, 
struct drm_file *filp)
}
 out:
radeon_cs_parser_fini(, r);
+   up_read(>exclusive_lock);
r = radeon_cs_handle_lockup(rdev, r);
return r;
 }
diff --git a/drivers/gpu/drm/radeon/radeon_device.c 
b/drivers/gpu/drm/radeon/radeon_device.c
index f654ba8..254fdb4 100644
--- a/drivers/gpu/drm/radeon/radeon_device.c
+++ b/drivers/gpu/drm/radeon/radeon_device.c
@@ -734,6 +734,7 @@ int radeon_device_init(struct radeon_device *rdev,
mutex_init(>gem.mutex);
mutex_init(>pm.mutex);
init_rwsem(>pm.mclk_lock);
+   init_rwsem(>exclusive_lock);
init_waitqueue_head(>irq.vblank_queue);
init_waitqueue_head(>irq.idle_queue);
r = radeon_gem_init(rdev);
@@ -988,6 +989,7 @@ int radeon_gpu_reset(struct radeon_device *rdev)
int r;
int resched;

+   down_write(>exclusive_lock);
radeon_save_bios_scratch_regs(rdev);
/* block TTM */
resched = ttm_bo_lock_delayed_workqueue(>mman.bdev);
@@ -1007,6 +1009,7 @@ int radeon_gpu_reset(struct radeon_device *rdev)
dev_info(rdev->dev, "GPU reset failed\n");
}

+   up_write(>exclusive_lock);
return r;
 }

diff --git a/drivers/gpu/drm/radeon/radeon_gem.c 
b/drivers/gpu/drm/radeon/radeon_gem.c
index d9b0809..b0be9c4 100644
--- a/drivers/gpu/drm/radeon/radeon_gem.c
+++ b/drivers/gpu/drm/radeon/radeon_gem.c
@@ -215,12 +215,14 @@ int radeon_gem_create_ioctl(struct drm_device *dev, void 
*data,
uint32_t handle;
int r;

+   down_read(>exclusive_lock);
/* create a gem object to contain this object in */
args->size = roundup(args->size, PAGE_SIZE);
r = radeon_gem_object_create(rdev, args->size, args->alignment,
args->initial_domain, false,
false, );
if (r) {
+   up_read(>exclusive_lock);
r = radeon_gem_handle_lockup(rdev, r);
return r;
}
@@ -228,10 +230,12 @@ int radeon_gem_create_ioctl(struct drm_device *dev, void 
*data,
/* drop reference from allocate - handle holds it now */
drm_gem_object_unreference_unlocked(gobj);
if (r) {
+   up_read(>exclusive_lock);
r = radeon_gem_handle_lockup(rdev, r);
return r;
}
args->handle = handle;
+   up_read(>exclusive_lock);
return 0;
 }


[PATCH 01/10] drm/radeon: document radeon_device.c (v2)

2012-07-02 Thread Christian König
On 29.06.2012 18:50, alexdeucher at gmail.com wrote:
> From: Alex Deucher 
>
> Adds documentation to most of the functions in
> radeon_device.c
>
> v2: split out general descriptions as per Christian's
> comments.
>
> Signed-off-by: Alex Deucher 
For the whole series:

Reviewed-by: Christian K?nig 


> ---
>   drivers/gpu/drm/radeon/radeon_device.c |  313 
> +++-
>   1 files changed, 310 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/radeon_device.c 
> b/drivers/gpu/drm/radeon/radeon_device.c
> index f654ba8..926d7e8 100644
> --- a/drivers/gpu/drm/radeon/radeon_device.c
> +++ b/drivers/gpu/drm/radeon/radeon_device.c
> @@ -96,8 +96,12 @@ static const char radeon_family_name[][16] = {
>   "LAST",
>   };
>   
> -/*
> - * Clear GPU surface registers.
> +/**
> + * radeon_surface_init - Clear GPU surface registers.
> + *
> + * @rdev: radeon_device pointer
> + *
> + * Clear GPU surface registers (r1xx-r5xx).
>*/
>   void radeon_surface_init(struct radeon_device *rdev)
>   {
> @@ -119,6 +123,13 @@ void radeon_surface_init(struct radeon_device *rdev)
>   /*
>* GPU scratch registers helpers function.
>*/
> +/**
> + * radeon_scratch_init - Init scratch register driver information.
> + *
> + * @rdev: radeon_device pointer
> + *
> + * Init CP scratch register driver information (r1xx-r5xx)
> + */
>   void radeon_scratch_init(struct radeon_device *rdev)
>   {
>   int i;
> @@ -136,6 +147,15 @@ void radeon_scratch_init(struct radeon_device *rdev)
>   }
>   }
>   
> +/**
> + * radeon_scratch_get - Allocate a scratch register
> + *
> + * @rdev: radeon_device pointer
> + * @reg: scratch register mmio offset
> + *
> + * Allocate a CP scratch register for use by the driver (all asics).
> + * Returns 0 on success or -EINVAL on failure.
> + */
>   int radeon_scratch_get(struct radeon_device *rdev, uint32_t *reg)
>   {
>   int i;
> @@ -150,6 +170,14 @@ int radeon_scratch_get(struct radeon_device *rdev, 
> uint32_t *reg)
>   return -EINVAL;
>   }
>   
> +/**
> + * radeon_scratch_free - Free a scratch register
> + *
> + * @rdev: radeon_device pointer
> + * @reg: scratch register mmio offset
> + *
> + * Free a CP scratch register allocated for use by the driver (all asics)
> + */
>   void radeon_scratch_free(struct radeon_device *rdev, uint32_t reg)
>   {
>   int i;
> @@ -162,6 +190,20 @@ void radeon_scratch_free(struct radeon_device *rdev, 
> uint32_t reg)
>   }
>   }
>   
> +/*
> + * radeon_wb_*()
> + * Writeback is the the method by which the the GPU updates special pages
> + * in memory with the status of certain GPU events (fences, ring pointers,
> + * etc.).
> + */
> +
> +/**
> + * radeon_wb_disable - Disable Writeback
> + *
> + * @rdev: radeon_device pointer
> + *
> + * Disables Writeback (all asics).  Used for suspend.
> + */
>   void radeon_wb_disable(struct radeon_device *rdev)
>   {
>   int r;
> @@ -177,6 +219,14 @@ void radeon_wb_disable(struct radeon_device *rdev)
>   rdev->wb.enabled = false;
>   }
>   
> +/**
> + * radeon_wb_fini - Disable Writeback and free memory
> + *
> + * @rdev: radeon_device pointer
> + *
> + * Disables Writeback and frees the Writeback memory (all asics).
> + * Used at driver shutdown.
> + */
>   void radeon_wb_fini(struct radeon_device *rdev)
>   {
>   radeon_wb_disable(rdev);
> @@ -187,6 +237,15 @@ void radeon_wb_fini(struct radeon_device *rdev)
>   }
>   }
>   
> +/**
> + * radeon_wb_init- Init Writeback driver info and allocate memory
> + *
> + * @rdev: radeon_device pointer
> + *
> + * Disables Writeback and frees the Writeback memory (all asics).
> + * Used at driver startup.
> + * Returns 0 on success or an -error on failure.
> + */
>   int radeon_wb_init(struct radeon_device *rdev)
>   {
>   int r;
> @@ -355,6 +414,15 @@ void radeon_gtt_location(struct radeon_device *rdev, 
> struct radeon_mc *mc)
>   /*
>* GPU helpers function.
>*/
> +/**
> + * radeon_card_posted - check if the hw has already been initialized
> + *
> + * @rdev: radeon_device pointer
> + *
> + * Check if the asic has been initialized (all asics).
> + * Used at driver startup.
> + * Returns true if initialized or false if not.
> + */
>   bool radeon_card_posted(struct radeon_device *rdev)
>   {
>   uint32_t reg;
> @@ -404,6 +472,14 @@ bool radeon_card_posted(struct radeon_device *rdev)
>   
>   }
>   
> +/**
> + * radeon_update_bandwidth_info - update display bandwidth params
> + *
> + * @rdev: radeon_device pointer
> + *
> + * Used when sclk/mclk are switched or display modes are set.
> + * params are used to calculate display watermarks (all asics)
> + */
>   void radeon_update_bandwidth_info(struct radeon_device *rdev)
>   {
>   fixed20_12 a;
> @@ -424,6 +500,15 @@ void radeon_update_bandwidth_info(struct radeon_device 
> *rdev)
>   }
>   }
>   
> +/**
> + * radeon_boot_test_post_card - check and possibly initialize the hw
> + *
> + * @rdev: radeon_device pointer
> + *
> + * Check if the 

[PATCH] drm/radeon: add an exclusive lock for GPU reset

2012-07-02 Thread Jerome Glisse
On Mon, Jul 2, 2012 at 12:26 PM, Christian K?nig
 wrote:
> On 02.07.2012 17:39, j.glisse at gmail.com wrote:
>>
>> From: Jerome Glisse 
>>
>> GPU reset need to be exclusive, one happening at a time. For this
>> add a rw semaphore so that any path that trigger GPU activities
>> have to take the semaphore as a reader thus allowing concurency.
>>
>> The GPU reset path take the semaphore as a writer ensuring that
>> no concurrent reset take place.
>>
>> Signed-off-by: Jerome Glisse 
>
> NAK, that isn't as bad as the cs mutex was but still to complicated. It's
> just too far up in the call stack, e.g. it tries to catch ioctl operations,
> instead of catching the underlying hardware operation which is caused by the
> ioctl/ttm/etc...
>
> Why not just take the ring look as I suggested?
>
>

No we can't use ring lock, we need to protect suspend/resume path and
we need an exclusive lock for that so we need a reset mutex at the
very least. But instead of having a reset mutex i prefer using a rw
lock so that we can stop ioctl until a reset goes through an let
pending ioctl take proper action. Think about multiple context trying
to reset GPU ...

Really this is the best option, the rw locking wont induce any lock
contention execept in gpu reset case which is anyway bad news.

Jerome


[PATCH] drm/radeon: fix rare segfault

2012-07-02 Thread j.gli...@gmail.com
From: Jerome Glisse 

In gem idle/busy ioctl the radeon object was derefenced after
drm_gem_object_unreference_unlocked which in case the object
have been destroyed lead to use of a possibly free pointer with
possibly wrong data.

Signed-off-by: Jerome Glisse 
---
 drivers/gpu/drm/radeon/radeon_gem.c |   10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_gem.c 
b/drivers/gpu/drm/radeon/radeon_gem.c
index 74176c5..c8838fc 100644
--- a/drivers/gpu/drm/radeon/radeon_gem.c
+++ b/drivers/gpu/drm/radeon/radeon_gem.c
@@ -325,6 +325,7 @@ int radeon_gem_mmap_ioctl(struct drm_device *dev, void 
*data,
 int radeon_gem_busy_ioctl(struct drm_device *dev, void *data,
  struct drm_file *filp)
 {
+   struct radeon_device *rdev = dev->dev_private;
struct drm_radeon_gem_busy *args = data;
struct drm_gem_object *gobj;
struct radeon_bo *robj;
@@ -350,13 +351,14 @@ int radeon_gem_busy_ioctl(struct drm_device *dev, void 
*data,
break;
}
drm_gem_object_unreference_unlocked(gobj);
-   r = radeon_gem_handle_lockup(robj->rdev, r);
+   r = radeon_gem_handle_lockup(rdev, r);
return r;
 }

 int radeon_gem_wait_idle_ioctl(struct drm_device *dev, void *data,
  struct drm_file *filp)
 {
+   struct radeon_device *rdev = dev->dev_private;
struct drm_radeon_gem_wait_idle *args = data;
struct drm_gem_object *gobj;
struct radeon_bo *robj;
@@ -369,10 +371,10 @@ int radeon_gem_wait_idle_ioctl(struct drm_device *dev, 
void *data,
robj = gem_to_radeon_bo(gobj);
r = radeon_bo_wait(robj, NULL, false);
/* callback hw specific functions if any */
-   if (robj->rdev->asic->ioctl_wait_idle)
-   robj->rdev->asic->ioctl_wait_idle(robj->rdev, robj);
+   if (rdev->asic->ioctl_wait_idle)
+   robj->rdev->asic->ioctl_wait_idle(rdev, robj);
drm_gem_object_unreference_unlocked(gobj);
-   r = radeon_gem_handle_lockup(robj->rdev, r);
+   r = radeon_gem_handle_lockup(rdev, r);
return r;
 }

-- 
1.7.10.2



[PATCH] drm/radeon: add an exclusive lock for GPU reset

2012-07-02 Thread Jerome Glisse
On Mon, Jul 2, 2012 at 11:39 AM,   wrote:
> From: Jerome Glisse 
>
> GPU reset need to be exclusive, one happening at a time. For this
> add a rw semaphore so that any path that trigger GPU activities
> have to take the semaphore as a reader thus allowing concurency.
>
> The GPU reset path take the semaphore as a writer ensuring that
> no concurrent reset take place.
>
> Signed-off-by: Jerome Glisse 

Wrong patch sorry resending

> ---
> ?drivers/gpu/drm/radeon/radeon.h ? ? ? ?| ? ?1 +
> ?drivers/gpu/drm/radeon/radeon_cs.c ? ? | ? ?5 +
> ?drivers/gpu/drm/radeon/radeon_device.c | ? ?2 ++
> ?drivers/gpu/drm/radeon/radeon_gem.c ? ?| ? ?7 +++
> ?4 files changed, 15 insertions(+)
>
> diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
> index 77b4519b..29d6986 100644
> --- a/drivers/gpu/drm/radeon/radeon.h
> +++ b/drivers/gpu/drm/radeon/radeon.h
> @@ -1446,6 +1446,7 @@ struct radeon_device {
> ? ? ? ?struct device ? ? ? ? ? ? ? ? ? *dev;
> ? ? ? ?struct drm_device ? ? ? ? ? ? ? *ddev;
> ? ? ? ?struct pci_dev ? ? ? ? ? ? ? ? ?*pdev;
> + ? ? ? struct rw_semaphore ? ? ? ? ? ? exclusive_lock;
> ? ? ? ?/* ASIC */
> ? ? ? ?union radeon_asic_config ? ? ? ?config;
> ? ? ? ?enum radeon_family ? ? ? ? ? ? ?family;
> diff --git a/drivers/gpu/drm/radeon/radeon_cs.c 
> b/drivers/gpu/drm/radeon/radeon_cs.c
> index f1b7527..7ee6491 100644
> --- a/drivers/gpu/drm/radeon/radeon_cs.c
> +++ b/drivers/gpu/drm/radeon/radeon_cs.c
> @@ -499,7 +499,9 @@ int radeon_cs_ioctl(struct drm_device *dev, void *data, 
> struct drm_file *filp)
> ? ? ? ?struct radeon_cs_parser parser;
> ? ? ? ?int r;
>
> + ? ? ? down_read(>exclusive_lock);
> ? ? ? ?if (!rdev->accel_working) {
> + ? ? ? ? ? ? ? up_read(>exclusive_lock);
> ? ? ? ? ? ? ? ?return -EBUSY;
> ? ? ? ?}
> ? ? ? ?/* initialize parser */
> @@ -512,6 +514,7 @@ int radeon_cs_ioctl(struct drm_device *dev, void *data, 
> struct drm_file *filp)
> ? ? ? ?if (r) {
> ? ? ? ? ? ? ? ?DRM_ERROR("Failed to initialize parser !\n");
> ? ? ? ? ? ? ? ?radeon_cs_parser_fini(, r);
> + ? ? ? ? ? ? ? up_read(>exclusive_lock);
> ? ? ? ? ? ? ? ?r = radeon_cs_handle_lockup(rdev, r);
> ? ? ? ? ? ? ? ?return r;
> ? ? ? ?}
> @@ -520,6 +523,7 @@ int radeon_cs_ioctl(struct drm_device *dev, void *data, 
> struct drm_file *filp)
> ? ? ? ? ? ? ? ?if (r != -ERESTARTSYS)
> ? ? ? ? ? ? ? ? ? ? ? ?DRM_ERROR("Failed to parse relocation %d!\n", r);
> ? ? ? ? ? ? ? ?radeon_cs_parser_fini(, r);
> + ? ? ? ? ? ? ? up_read(>exclusive_lock);
> ? ? ? ? ? ? ? ?r = radeon_cs_handle_lockup(rdev, r);
> ? ? ? ? ? ? ? ?return r;
> ? ? ? ?}
> @@ -533,6 +537,7 @@ int radeon_cs_ioctl(struct drm_device *dev, void *data, 
> struct drm_file *filp)
> ? ? ? ?}
> ?out:
> ? ? ? ?radeon_cs_parser_fini(, r);
> + ? ? ? up_read(>exclusive_lock);
> ? ? ? ?r = radeon_cs_handle_lockup(rdev, r);
> ? ? ? ?return r;
> ?}
> diff --git a/drivers/gpu/drm/radeon/radeon_device.c 
> b/drivers/gpu/drm/radeon/radeon_device.c
> index f654ba8..c8fdb40 100644
> --- a/drivers/gpu/drm/radeon/radeon_device.c
> +++ b/drivers/gpu/drm/radeon/radeon_device.c
> @@ -988,6 +988,7 @@ int radeon_gpu_reset(struct radeon_device *rdev)
> ? ? ? ?int r;
> ? ? ? ?int resched;
>
> + ? ? ? down_write(>exclusive_lock);
> ? ? ? ?radeon_save_bios_scratch_regs(rdev);
> ? ? ? ?/* block TTM */
> ? ? ? ?resched = ttm_bo_lock_delayed_workqueue(>mman.bdev);
> @@ -1007,6 +1008,7 @@ int radeon_gpu_reset(struct radeon_device *rdev)
> ? ? ? ? ? ? ? ?dev_info(rdev->dev, "GPU reset failed\n");
> ? ? ? ?}
>
> + ? ? ? up_write(>exclusive_lock);
> ? ? ? ?return r;
> ?}
>
> diff --git a/drivers/gpu/drm/radeon/radeon_gem.c 
> b/drivers/gpu/drm/radeon/radeon_gem.c
> index d9b0809..f99db63 100644
> --- a/drivers/gpu/drm/radeon/radeon_gem.c
> +++ b/drivers/gpu/drm/radeon/radeon_gem.c
> @@ -215,12 +215,14 @@ int radeon_gem_create_ioctl(struct drm_device *dev, 
> void *data,
> ? ? ? ?uint32_t handle;
> ? ? ? ?int r;
>
> + ? ? ? down_read(>exclusive_lock);
> ? ? ? ?/* create a gem object to contain this object in */
> ? ? ? ?args->size = roundup(args->size, PAGE_SIZE);
> ? ? ? ?r = radeon_gem_object_create(rdev, args->size, args->alignment,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?args->initial_domain, false,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?false, );
> ? ? ? ?if (r) {
> + ? ? ? ? ? ? ? up_read(>exclusive_lock);
> ? ? ? ? ? ? ? ?r = radeon_gem_handle_lockup(rdev, r);
> ? ? ? ? ? ? ? ?return r;
> ? ? ? ?}
> @@ -228,10 +230,12 @@ int radeon_gem_create_ioctl(struct drm_device *dev, 
> void *data,
> ? ? ? ?/* drop reference from allocate - handle holds it now */
> ? ? ? ?drm_gem_object_unreference_unlocked(gobj);
> ? ? ? ?if (r) {
> + ? ? ? ? ? ? ? up_read(>exclusive_lock);
> ? ? ? ? ? ? ? ?r = radeon_gem_handle_lockup(rdev, r);
> ? ? ? ? ? ? ? ?return r;
> ? ? ? ?}
> ? ? ? ?args->handle = handle;
> + ? ? ? up_read(>exclusive_lock);
> ? ? ? ?return 0;
> ?}
>
> @@ -240,6 +244,7 @@ int radeon_gem_set_domain_ioctl(struct drm_device *dev, 
> void *data,
> ?{
> ? ? ? ?/* 

[PATCH 1/3] drm/radeon: move ring locking out of reset path

2012-07-02 Thread Jerome Glisse
On Mon, Jul 2, 2012 at 11:58 AM, Christian K?nig
 wrote:
> On 02.07.2012 17:41, Jerome Glisse wrote:
>>
>> On Fri, Jun 29, 2012 at 12:15 PM, Michel D?nzer 
>> wrote:
>>>
>>> On Fre, 2012-06-29 at 17:18 +0200, Christian K?nig wrote:

 On 29.06.2012 17:09, Michel D?nzer wrote:
>
> On Fre, 2012-06-29 at 16:45 +0200, Christian K?nig wrote:
>>
>> Hold the ring lock the whole time the reset is in progress,
>> otherwise another process can submit new jobs.
>
> Sounds good, but doesn't this create other paths (e.g. initialization,
> resume) where the ring is being accessed without holding the lock?
> Isn't
> that a problem?

 Thought about that also.

 For init I'm pretty sure that no application can submit commands before
 we are done, otherwise we are doomed anyway.

 For resume I'm not really sure, but I think that applications are
 resumed after the hardware driver had a chance of doing so.
>>>
>>> I hope you're right... but if it's not too much trouble, it might be
>>> better to be safe than sorry and take the lock for those paths as well.
>>>
>>>
>> NAK this is the wrong way to solve the issue, we need a global lock on
>> all path that can trigger gpu activities. Previously it was the cs
>> mutex, but i haven't thought about it too much when it got removed. So
>> to fix the situation i am sending a patch with rw semaphore.
>
> So what I'm missing? What else can trigger GPU activity when not the rings?
>
> I'm currently working on ring-partial resets and also resets where you only
> skip over a single faulty IB instead of flushing the whole ring. And my
> current idea for that to work is that we hold the ring lock while we do
> suspend, ring_save, asic_reset, resume and ring_restore.
>
> Christian.
>

I should add that gpu_reset should be an heavy reset, and if you want
to only reset one ring and see if gpu can continue without heavy reset
then you should do it as a special ring reset that doesn't reset mc
and some other block but only the affected ring (and i am assuming
that hw behave properly here). If that light weight ring reset doesn't
work than let the heavy reset kicks in. So yes your light weight per
ring reset would only need to take the ring lock but now need to
change the ring lock usage we have now.

Cheers,
Jerome


[PATCH, RFC] i.MX DRM support

2012-07-02 Thread Sascha Hauer
On Thu, Jun 14, 2012 at 03:43:22PM +0200, Sascha Hauer wrote:
> Hi All,
> 
> The following is the state-of-the-art i.MX IPU (Image Processing Unit)
> DRM support.
> 
> This code is around for quite some time now and has been posted several
> times with different APIs, first with plain old framebuffer support, now
> DRM, first platform device binding, now devicetree. Unfortunately there's
> quite much code needed to get something useful out of the IPU, so these
> patches haven't received a lot of attention from people not involved in
> i.MX. I think we have now come to a point where this code needs more public
> exposure and where it's easier to talk in incremental changes instead of
> blobs. Therefore I request this to go to staging for some cycles.

Dave, Greg,

Comments to this one? I addressed the comments I received so far and am
about to respin this series. Is it ok to put this to staging? If yes,
should I move the whole stuff into drivers/staging/ or should it stay
in drivers/gpu/drm with just a Kconfig dependency on STAGING?

Thanks
 Sascha

-- 
Pengutronix e.K.   | |
Industrial Linux Solutions | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |


[PATCH 1/3] drm/radeon: move ring locking out of reset path

2012-07-02 Thread Jerome Glisse
On Mon, Jul 2, 2012 at 11:58 AM, Christian K?nig
 wrote:
> On 02.07.2012 17:41, Jerome Glisse wrote:
>>
>> On Fri, Jun 29, 2012 at 12:15 PM, Michel D?nzer 
>> wrote:
>>>
>>> On Fre, 2012-06-29 at 17:18 +0200, Christian K?nig wrote:

 On 29.06.2012 17:09, Michel D?nzer wrote:
>
> On Fre, 2012-06-29 at 16:45 +0200, Christian K?nig wrote:
>>
>> Hold the ring lock the whole time the reset is in progress,
>> otherwise another process can submit new jobs.
>
> Sounds good, but doesn't this create other paths (e.g. initialization,
> resume) where the ring is being accessed without holding the lock?
> Isn't
> that a problem?

 Thought about that also.

 For init I'm pretty sure that no application can submit commands before
 we are done, otherwise we are doomed anyway.

 For resume I'm not really sure, but I think that applications are
 resumed after the hardware driver had a chance of doing so.
>>>
>>> I hope you're right... but if it's not too much trouble, it might be
>>> better to be safe than sorry and take the lock for those paths as well.
>>>
>>>
>> NAK this is the wrong way to solve the issue, we need a global lock on
>> all path that can trigger gpu activities. Previously it was the cs
>> mutex, but i haven't thought about it too much when it got removed. So
>> to fix the situation i am sending a patch with rw semaphore.
>
> So what I'm missing? What else can trigger GPU activity when not the rings?
>
> I'm currently working on ring-partial resets and also resets where you only
> skip over a single faulty IB instead of flushing the whole ring. And my
> current idea for that to work is that we hold the ring lock while we do
> suspend, ring_save, asic_reset, resume and ring_restore.
>
> Christian.
>

I just sent a patch, the idea is that you want gpu reset to be an
exclusive operation like gpu init, or gpu resume. So by taking rw
semaphore you allow the gpu reset to be exclusive and so you know
nobody can trigger gpu activies while still allowing concurrency in
case no gpu reset is on going.

Cheers,
Jerome


gma500 suspend to ram fails (3.4)

2012-07-02 Thread Alan Cox
On Mon, 2 Jul 2012 07:06:59 +0900
Mattia Dongili  wrote:

> On Thu, Jun 21, 2012 at 06:19:25AM +0900, Mattia Dongili wrote:
> > On Tue, Jun 19, 2012 at 07:56:52PM +0900, Mattia Dongili wrote:
> > > On Mon, Jun 18, 2012 at 10:04:00PM +0200, Patrik Jakobsson wrote:
> > > > On Sun, Jun 17, 2012 at 12:03 PM, Mattia Dongili
> > > >  wrote:
> > ...
> > > > If possible, add drm.debug=7 to your boot line and send a dmesg
> > > > of 3.5-rc3.
> > > 
> > > attached the dmesg on rc3 with drm.debug=7.
> > > In addition I have a black screen and 3 modprobe processes stuck
> > > that udev tried to kill for a bit... then the console went blank
> > > and I can't tell if it's still trying:
> > > 
> > >   359 ?R 11:08 /sbin/modprobe -b
> > > pci:v8086d8108sv104Dsd905Fbc03sc00i00
> > > 361 ?D  0:00 /sbin/modprobe -b
> > > pci:v8086d811Bsv104Dsd905Fbc04sc03i00
> > > 422 ?D  0:00 /sbin/modprobe -b
> > > pci:v8086d8119sv104Dsd905Fbc06sc01i00
> > 
> > for the record, here below is where the screen flickers a bit and
> > booting gets stuck until udev starts trying to kill the modprobe
> > calls.
> 
> I gave 3.5-rc4 a try, no significant change but by comparing 3.4 and
> 3.5 logs I have this difference that may mean something:
> 
> [0.00] Linux version 3.5.0-rc4...
> [5.712020] gma500 :00:02.0: Backlight lvds set brightness
> 74367436
> 
> while with 3.4
> 
> [5.525296] gma500 :00:02.0: Backlight lvds set brightness
> 74407440
> 
> a did put some printks here and there and psb_driver_init seems to
> never return from gma_backlight_init.

Is it ok if you just comment out gma_backlight_init (at which point it
should just skip backlight handling and leave the firmware configured
one)

Alan



[PATCH 1/3] drm/radeon: move ring locking out of reset path

2012-07-02 Thread Jerome Glisse
On Fri, Jun 29, 2012 at 12:15 PM, Michel D?nzer  wrote:
> On Fre, 2012-06-29 at 17:18 +0200, Christian K?nig wrote:
>> On 29.06.2012 17:09, Michel D?nzer wrote:
>> > On Fre, 2012-06-29 at 16:45 +0200, Christian K?nig wrote:
>> >> Hold the ring lock the whole time the reset is in progress,
>> >> otherwise another process can submit new jobs.
>> > Sounds good, but doesn't this create other paths (e.g. initialization,
>> > resume) where the ring is being accessed without holding the lock? Isn't
>> > that a problem?
>>
>> Thought about that also.
>>
>> For init I'm pretty sure that no application can submit commands before
>> we are done, otherwise we are doomed anyway.
>>
>> For resume I'm not really sure, but I think that applications are
>> resumed after the hardware driver had a chance of doing so.
>
> I hope you're right... but if it's not too much trouble, it might be
> better to be safe than sorry and take the lock for those paths as well.
>
>

NAK this is the wrong way to solve the issue, we need a global lock on
all path that can trigger gpu activities. Previously it was the cs
mutex, but i haven't thought about it too much when it got removed. So
to fix the situation i am sending a patch with rw semaphore.

Cheers,
Jerome


[PATCH] drm/radeon: add an exclusive lock for GPU reset

2012-07-02 Thread j.gli...@gmail.com
From: Jerome Glisse 

GPU reset need to be exclusive, one happening at a time. For this
add a rw semaphore so that any path that trigger GPU activities
have to take the semaphore as a reader thus allowing concurency.

The GPU reset path take the semaphore as a writer ensuring that
no concurrent reset take place.

Signed-off-by: Jerome Glisse 
---
 drivers/gpu/drm/radeon/radeon.h|1 +
 drivers/gpu/drm/radeon/radeon_cs.c |5 +
 drivers/gpu/drm/radeon/radeon_device.c |2 ++
 drivers/gpu/drm/radeon/radeon_gem.c|7 +++
 4 files changed, 15 insertions(+)

diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
index 77b4519b..29d6986 100644
--- a/drivers/gpu/drm/radeon/radeon.h
+++ b/drivers/gpu/drm/radeon/radeon.h
@@ -1446,6 +1446,7 @@ struct radeon_device {
struct device   *dev;
struct drm_device   *ddev;
struct pci_dev  *pdev;
+   struct rw_semaphore exclusive_lock;
/* ASIC */
union radeon_asic_configconfig;
enum radeon_family  family;
diff --git a/drivers/gpu/drm/radeon/radeon_cs.c 
b/drivers/gpu/drm/radeon/radeon_cs.c
index f1b7527..7ee6491 100644
--- a/drivers/gpu/drm/radeon/radeon_cs.c
+++ b/drivers/gpu/drm/radeon/radeon_cs.c
@@ -499,7 +499,9 @@ int radeon_cs_ioctl(struct drm_device *dev, void *data, 
struct drm_file *filp)
struct radeon_cs_parser parser;
int r;

+   down_read(>exclusive_lock);
if (!rdev->accel_working) {
+   up_read(>exclusive_lock);
return -EBUSY;
}
/* initialize parser */
@@ -512,6 +514,7 @@ int radeon_cs_ioctl(struct drm_device *dev, void *data, 
struct drm_file *filp)
if (r) {
DRM_ERROR("Failed to initialize parser !\n");
radeon_cs_parser_fini(, r);
+   up_read(>exclusive_lock);
r = radeon_cs_handle_lockup(rdev, r);
return r;
}
@@ -520,6 +523,7 @@ int radeon_cs_ioctl(struct drm_device *dev, void *data, 
struct drm_file *filp)
if (r != -ERESTARTSYS)
DRM_ERROR("Failed to parse relocation %d!\n", r);
radeon_cs_parser_fini(, r);
+   up_read(>exclusive_lock);
r = radeon_cs_handle_lockup(rdev, r);
return r;
}
@@ -533,6 +537,7 @@ int radeon_cs_ioctl(struct drm_device *dev, void *data, 
struct drm_file *filp)
}
 out:
radeon_cs_parser_fini(, r);
+   up_read(>exclusive_lock);
r = radeon_cs_handle_lockup(rdev, r);
return r;
 }
diff --git a/drivers/gpu/drm/radeon/radeon_device.c 
b/drivers/gpu/drm/radeon/radeon_device.c
index f654ba8..c8fdb40 100644
--- a/drivers/gpu/drm/radeon/radeon_device.c
+++ b/drivers/gpu/drm/radeon/radeon_device.c
@@ -988,6 +988,7 @@ int radeon_gpu_reset(struct radeon_device *rdev)
int r;
int resched;

+   down_write(>exclusive_lock);
radeon_save_bios_scratch_regs(rdev);
/* block TTM */
resched = ttm_bo_lock_delayed_workqueue(>mman.bdev);
@@ -1007,6 +1008,7 @@ int radeon_gpu_reset(struct radeon_device *rdev)
dev_info(rdev->dev, "GPU reset failed\n");
}

+   up_write(>exclusive_lock);
return r;
 }

diff --git a/drivers/gpu/drm/radeon/radeon_gem.c 
b/drivers/gpu/drm/radeon/radeon_gem.c
index d9b0809..f99db63 100644
--- a/drivers/gpu/drm/radeon/radeon_gem.c
+++ b/drivers/gpu/drm/radeon/radeon_gem.c
@@ -215,12 +215,14 @@ int radeon_gem_create_ioctl(struct drm_device *dev, void 
*data,
uint32_t handle;
int r;

+   down_read(>exclusive_lock);
/* create a gem object to contain this object in */
args->size = roundup(args->size, PAGE_SIZE);
r = radeon_gem_object_create(rdev, args->size, args->alignment,
args->initial_domain, false,
false, );
if (r) {
+   up_read(>exclusive_lock);
r = radeon_gem_handle_lockup(rdev, r);
return r;
}
@@ -228,10 +230,12 @@ int radeon_gem_create_ioctl(struct drm_device *dev, void 
*data,
/* drop reference from allocate - handle holds it now */
drm_gem_object_unreference_unlocked(gobj);
if (r) {
+   up_read(>exclusive_lock);
r = radeon_gem_handle_lockup(rdev, r);
return r;
}
args->handle = handle;
+   up_read(>exclusive_lock);
return 0;
 }

@@ -240,6 +244,7 @@ int radeon_gem_set_domain_ioctl(struct drm_device *dev, 
void *data,
 {
/* transition the BO to a domain -
 * just validate the BO into a certain domain */
+   struct radeon_device *rdev = dev->dev_private;
struct drm_radeon_gem_set_domain *args = data;
struct drm_gem_object *gobj;
struct 

[Bug 51658] r200 (& possibly radeon) DRI fixes for gnome shell on Mesa 8.0.3

2012-07-02 Thread bugzilla-dae...@freedesktop.org
https://bugs.freedesktop.org/show_bug.cgi?id=51658

--- Comment #5 from Eugene St Leger  2012-07-02 10:45:01 
UTC ---
Created attachment 63716
  --> https://bugs.freedesktop.org/attachment.cgi?id=63716
Optional patch to warn (once) when a blit with 2048 pixel dimension occurs on
r200.

If 2048 pixel blits cause graphical glitches/problems on r200, this patch can
be used to provide a single warning.

-- 
Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are the assignee for the bug.


gma500 suspend to ram fails (3.4)

2012-07-02 Thread Mattia Dongili
On Thu, Jun 21, 2012 at 06:19:25AM +0900, Mattia Dongili wrote:
> On Tue, Jun 19, 2012 at 07:56:52PM +0900, Mattia Dongili wrote:
> > On Mon, Jun 18, 2012 at 10:04:00PM +0200, Patrik Jakobsson wrote:
> > > On Sun, Jun 17, 2012 at 12:03 PM, Mattia Dongili  
> > > wrote:
> ...
> > > If possible, add drm.debug=7 to your boot line and send a dmesg of 
> > > 3.5-rc3.
> > 
> > attached the dmesg on rc3 with drm.debug=7.
> > In addition I have a black screen and 3 modprobe processes stuck that
> > udev tried to kill for a bit... then the console went blank and I can't
> > tell if it's still trying:
> > 
> >   359 ?R 11:08 /sbin/modprobe -b 
> > pci:v8086d8108sv104Dsd905Fbc03sc00i00
> >   361 ?D  0:00 /sbin/modprobe -b 
> > pci:v8086d811Bsv104Dsd905Fbc04sc03i00
> >   422 ?D  0:00 /sbin/modprobe -b 
> > pci:v8086d8119sv104Dsd905Fbc06sc01i00
> 
> for the record, here below is where the screen flickers a bit and
> booting gets stuck until udev starts trying to kill the modprobe calls.

I gave 3.5-rc4 a try, no significant change but by comparing 3.4 and 3.5
logs I have this difference that may mean something:

[0.00] Linux version 3.5.0-rc4...
[5.712020] gma500 :00:02.0: Backlight lvds set brightness 74367436

while with 3.4

[5.525296] gma500 :00:02.0: Backlight lvds set brightness 74407440

a did put some printks here and there and psb_driver_init seems to never
return from gma_backlight_init.

-- 
mattia
:wq!


Re: [PATCH, RFC] i.MX DRM support

2012-07-02 Thread Sascha Hauer
On Thu, Jun 14, 2012 at 03:43:22PM +0200, Sascha Hauer wrote:
 Hi All,
 
 The following is the state-of-the-art i.MX IPU (Image Processing Unit)
 DRM support.
 
 This code is around for quite some time now and has been posted several
 times with different APIs, first with plain old framebuffer support, now
 DRM, first platform device binding, now devicetree. Unfortunately there's
 quite much code needed to get something useful out of the IPU, so these
 patches haven't received a lot of attention from people not involved in
 i.MX. I think we have now come to a point where this code needs more public
 exposure and where it's easier to talk in incremental changes instead of
 blobs. Therefore I request this to go to staging for some cycles.

Dave, Greg,

Comments to this one? I addressed the comments I received so far and am
about to respin this series. Is it ok to put this to staging? If yes,
should I move the whole stuff into drivers/staging/ or should it stay
in drivers/gpu/drm with just a Kconfig dependency on STAGING?

Thanks
 Sascha

-- 
Pengutronix e.K.   | |
Industrial Linux Solutions | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: gma500 suspend to ram fails (3.4)

2012-07-02 Thread Alan Cox
On Mon, 2 Jul 2012 07:06:59 +0900
Mattia Dongili malat...@linux.it wrote:

 On Thu, Jun 21, 2012 at 06:19:25AM +0900, Mattia Dongili wrote:
  On Tue, Jun 19, 2012 at 07:56:52PM +0900, Mattia Dongili wrote:
   On Mon, Jun 18, 2012 at 10:04:00PM +0200, Patrik Jakobsson wrote:
On Sun, Jun 17, 2012 at 12:03 PM, Mattia Dongili
malat...@linux.it wrote:
  ...
If possible, add drm.debug=7 to your boot line and send a dmesg
of 3.5-rc3.
   
   attached the dmesg on rc3 with drm.debug=7.
   In addition I have a black screen and 3 modprobe processes stuck
   that udev tried to kill for a bit... then the console went blank
   and I can't tell if it's still trying:
   
 359 ?R 11:08 /sbin/modprobe -b
   pci:v8086d8108sv104Dsd905Fbc03sc00i00
   361 ?D  0:00 /sbin/modprobe -b
   pci:v8086d811Bsv104Dsd905Fbc04sc03i00
   422 ?D  0:00 /sbin/modprobe -b
   pci:v8086d8119sv104Dsd905Fbc06sc01i00
  
  for the record, here below is where the screen flickers a bit and
  booting gets stuck until udev starts trying to kill the modprobe
  calls.
 
 I gave 3.5-rc4 a try, no significant change but by comparing 3.4 and
 3.5 logs I have this difference that may mean something:
 
 [0.00] Linux version 3.5.0-rc4...
 [5.712020] gma500 :00:02.0: Backlight lvds set brightness
 74367436
 
 while with 3.4
 
 [5.525296] gma500 :00:02.0: Backlight lvds set brightness
 74407440
 
 a did put some printks here and there and psb_driver_init seems to
 never return from gma_backlight_init.

Is it ok if you just comment out gma_backlight_init (at which point it
should just skip backlight handling and leave the firmware configured
one)

Alan

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH 01/10] drm/radeon: document radeon_device.c (v2)

2012-07-02 Thread Christian König

On 29.06.2012 18:50, alexdeuc...@gmail.com wrote:

From: Alex Deucher alexander.deuc...@amd.com

Adds documentation to most of the functions in
radeon_device.c

v2: split out general descriptions as per Christian's
comments.

Signed-off-by: Alex Deucher alexander.deuc...@amd.com

For the whole series:

Reviewed-by: Christian König christian.koe...@amd.com



---
  drivers/gpu/drm/radeon/radeon_device.c |  313 +++-
  1 files changed, 310 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_device.c 
b/drivers/gpu/drm/radeon/radeon_device.c
index f654ba8..926d7e8 100644
--- a/drivers/gpu/drm/radeon/radeon_device.c
+++ b/drivers/gpu/drm/radeon/radeon_device.c
@@ -96,8 +96,12 @@ static const char radeon_family_name[][16] = {
LAST,
  };
  
-/*

- * Clear GPU surface registers.
+/**
+ * radeon_surface_init - Clear GPU surface registers.
+ *
+ * @rdev: radeon_device pointer
+ *
+ * Clear GPU surface registers (r1xx-r5xx).
   */
  void radeon_surface_init(struct radeon_device *rdev)
  {
@@ -119,6 +123,13 @@ void radeon_surface_init(struct radeon_device *rdev)
  /*
   * GPU scratch registers helpers function.
   */
+/**
+ * radeon_scratch_init - Init scratch register driver information.
+ *
+ * @rdev: radeon_device pointer
+ *
+ * Init CP scratch register driver information (r1xx-r5xx)
+ */
  void radeon_scratch_init(struct radeon_device *rdev)
  {
int i;
@@ -136,6 +147,15 @@ void radeon_scratch_init(struct radeon_device *rdev)
}
  }
  
+/**

+ * radeon_scratch_get - Allocate a scratch register
+ *
+ * @rdev: radeon_device pointer
+ * @reg: scratch register mmio offset
+ *
+ * Allocate a CP scratch register for use by the driver (all asics).
+ * Returns 0 on success or -EINVAL on failure.
+ */
  int radeon_scratch_get(struct radeon_device *rdev, uint32_t *reg)
  {
int i;
@@ -150,6 +170,14 @@ int radeon_scratch_get(struct radeon_device *rdev, 
uint32_t *reg)
return -EINVAL;
  }
  
+/**

+ * radeon_scratch_free - Free a scratch register
+ *
+ * @rdev: radeon_device pointer
+ * @reg: scratch register mmio offset
+ *
+ * Free a CP scratch register allocated for use by the driver (all asics)
+ */
  void radeon_scratch_free(struct radeon_device *rdev, uint32_t reg)
  {
int i;
@@ -162,6 +190,20 @@ void radeon_scratch_free(struct radeon_device *rdev, 
uint32_t reg)
}
  }
  
+/*

+ * radeon_wb_*()
+ * Writeback is the the method by which the the GPU updates special pages
+ * in memory with the status of certain GPU events (fences, ring pointers,
+ * etc.).
+ */
+
+/**
+ * radeon_wb_disable - Disable Writeback
+ *
+ * @rdev: radeon_device pointer
+ *
+ * Disables Writeback (all asics).  Used for suspend.
+ */
  void radeon_wb_disable(struct radeon_device *rdev)
  {
int r;
@@ -177,6 +219,14 @@ void radeon_wb_disable(struct radeon_device *rdev)
rdev-wb.enabled = false;
  }
  
+/**

+ * radeon_wb_fini - Disable Writeback and free memory
+ *
+ * @rdev: radeon_device pointer
+ *
+ * Disables Writeback and frees the Writeback memory (all asics).
+ * Used at driver shutdown.
+ */
  void radeon_wb_fini(struct radeon_device *rdev)
  {
radeon_wb_disable(rdev);
@@ -187,6 +237,15 @@ void radeon_wb_fini(struct radeon_device *rdev)
}
  }
  
+/**

+ * radeon_wb_init- Init Writeback driver info and allocate memory
+ *
+ * @rdev: radeon_device pointer
+ *
+ * Disables Writeback and frees the Writeback memory (all asics).
+ * Used at driver startup.
+ * Returns 0 on success or an -error on failure.
+ */
  int radeon_wb_init(struct radeon_device *rdev)
  {
int r;
@@ -355,6 +414,15 @@ void radeon_gtt_location(struct radeon_device *rdev, 
struct radeon_mc *mc)
  /*
   * GPU helpers function.
   */
+/**
+ * radeon_card_posted - check if the hw has already been initialized
+ *
+ * @rdev: radeon_device pointer
+ *
+ * Check if the asic has been initialized (all asics).
+ * Used at driver startup.
+ * Returns true if initialized or false if not.
+ */
  bool radeon_card_posted(struct radeon_device *rdev)
  {
uint32_t reg;
@@ -404,6 +472,14 @@ bool radeon_card_posted(struct radeon_device *rdev)
  
  }
  
+/**

+ * radeon_update_bandwidth_info - update display bandwidth params
+ *
+ * @rdev: radeon_device pointer
+ *
+ * Used when sclk/mclk are switched or display modes are set.
+ * params are used to calculate display watermarks (all asics)
+ */
  void radeon_update_bandwidth_info(struct radeon_device *rdev)
  {
fixed20_12 a;
@@ -424,6 +500,15 @@ void radeon_update_bandwidth_info(struct radeon_device 
*rdev)
}
  }
  
+/**

+ * radeon_boot_test_post_card - check and possibly initialize the hw
+ *
+ * @rdev: radeon_device pointer
+ *
+ * Check if the asic is initialized and if not, attempt to initialize
+ * it (all asics).
+ * Returns true if initialized or false if not.
+ */
  bool radeon_boot_test_post_card(struct radeon_device *rdev)
  {
if (radeon_card_posted(rdev))
@@ 

[Bug 51652] New: [6550D SUMO] problems with secondar monitor on VGA, causing GPU lockups

2012-07-02 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=51652

 Bug #: 51652
   Summary: [6550D SUMO] problems with secondar monitor on VGA,
causing GPU lockups
Classification: Unclassified
   Product: DRI
   Version: XOrg CVS
  Platform: x86-64 (AMD64)
OS/Version: Linux (All)
Status: NEW
  Severity: critical
  Priority: medium
 Component: DRM/Radeon
AssignedTo: dri-devel@lists.freedesktop.org
ReportedBy: d.ok...@gmail.com


Created attachment 63701
  -- https://bugs.freedesktop.org/attachment.cgi?id=63701
dmesg_VGAonBootConnected.txt

1) when is computer booted with VGA connected, ends with repeatly restarting
GPU

2) when is monitor connected on-fly in running Xorg, it seems fine (except
small artifacts, i guess EXA, on screen mostly on top of DVI and bottom VGA
trigerred by switching KDE virtual desktops). No errors in dmesg

3) when is DPMS activated and monitors suspends, back to online is returned
only DVI

4) randomly I found:
[drm:radeon_dp_link_train_cr] *ERROR* clock recovery reached max voltage
[drm:radeon_dp_link_train_cr] *ERROR* clock recovery failed

5) out-topic, exist even only on DVI: when is enabled OGL KWIN and OpenGL video
render, color of rendered context goes to white (every frame is whitier, until
it's #ff)

No HDMI or DP conneted to system.

DVI-D: LG IPS235 1920x1080
VGA: ASUS VW224 1680x1050
Gentoo ~amd64
Kernel,libdrm,mesa,ddx: git, KMS

I'm able to test patches against anything.

-- 
Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are the assignee for the bug.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel


[Bug 28622] radeon video lockup

2012-07-02 Thread bugzilla-daemon
https://bugzilla.kernel.org/show_bug.cgi?id=28622


Alan a...@lxorguk.ukuu.org.uk changed:

   What|Removed |Added

 CC||a...@lxorguk.ukuu.org.uk
 Kernel Version|2.6.37  |2.6.39




-- 
Configure bugmail: https://bugzilla.kernel.org/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are watching the assignee of the bug.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH] staging: drm/omap: add rotation properties

2012-07-02 Thread Ville Syrjälä
On Fri, Jun 29, 2012 at 07:17:23AM -0500, Rob Clark wrote:
 On Fri, Jun 29, 2012 at 5:46 AM, Tomi Valkeinen tomi.valkei...@ti.com wrote:
  On Wed, 2012-06-27 at 09:06 -0500, Rob Clark wrote:
  From: Rob Clark r...@ti.com
 
  Use tiled buffers for rotated/reflected scanout, with CRTC and plane
  properties as the interface for userspace to configure rotation.
 
  Signed-off-by: Rob Clark r...@ti.com
 
  +/* this should probably be in drm-core to standardize amongst drivers */
  +#define DRM_ROTATE_0 0
  +#define DRM_ROTATE_90        1
  +#define DRM_ROTATE_180       2
  +#define DRM_ROTATE_270       3
  +#define DRM_REFLECT_X        4
  +#define DRM_REFLECT_Y        5
 
  Are both reflect X and Y needed? You can get all the possible
  orientations with just one of the reflects.
 
  And I think the word mirror represents nicely what the reflect does,
  i.e. if you look at the mirror, the image you see is flipped
  horizontally.
 
 fwiw these values are aligned with what is used in userspace xrandr..
 an earlier version of this patch used just 3 bits, which where aligned
 with what the omap hw uses and can describe all possible combinations
 of mirroring and isomorphic rotation (x-invert, y-invert, and
 xy-flip).  But the advantage of the xrandr approach is you can more
 easily leave out bits for rotation/mirroring modes that your hw does
 not support.. for example if some hw supports only certain rotations
 or does not support mirror/reflect.

When I hear mirror I always think of it as the happening after
rotation, but that's not how xrandr does things. I suppose reflection
has more or less the same connotation for me, but since it's already
used by xrandr, I know I have to do the necessary mental gymnastics.

If you think about it in terms of matrix multiplication, xrandr does
things like this: x' = Rot * Ref * x, whereas for me the more natural
order (for whatever reason) would be x' = Ref * Rot * x, where x is the
source coordinates, and x' the destination coordinates.

IIRC in dss mirroring was specified in the post-rotation way, and
the direction of the rotation was also opposite to xrandr (cw in dss,
ccw in xrandr).

So FWIW, my vote goes to reflect if we want to match xrandr
semantics. Also I agree on the number of bits, six is better than
three since it allows you to describe the hw capabilities.

-- 
Ville Syrjälä
Intel OTC
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH v3] DRM: Add DRM kms/fb cma helper

2012-07-02 Thread Lars-Peter Clausen
This patchset introduces a set of helper function for implementing the KMS
framebuffer layer for drivers which use the drm gem CMA helper function.

Signed-off-by: Lars-Peter Clausen l...@metafoo.de

---
Note: This patch depends on Sascha's DRM: add drm gem CMA helper patch

Changes since v2:
* Adapt to changes in the GEM CMA helper
* Add basic buffer size checking in drm_fb_cma_create
Changes since v1:
* Some spelling fixes
* Add missing kfree in drm_fb_cma_alloc error path
* Add multi-plane support
---
 drivers/gpu/drm/Kconfig |   10 +
 drivers/gpu/drm/Makefile|1 +
 drivers/gpu/drm/drm_fb_cma_helper.c |  393 +++
 include/drm/drm_fb_cma_helper.h |   27 +++
 4 files changed, 431 insertions(+)
 create mode 100644 drivers/gpu/drm/drm_fb_cma_helper.c
 create mode 100644 include/drm/drm_fb_cma_helper.h

diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index 41bbd95..e511c9a 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -41,6 +41,16 @@ config DRM_GEM_CMA_HELPER
help
  Choose this if you need the GEM CMA helper functions
 
+config DRM_KMS_CMA_HELPER
+   tristate
+   select DRM_GEM_CMA_HELPER
+   select DRM_KMS_HELPER
+   select FB_SYS_FILLRECT
+   select FB_SYS_COPYAREA
+   select FB_SYS_IMAGEBLIT
+   help
+ Choose this if you need the KMS cma helper functions
+
 config DRM_TDFX
tristate 3dfx Banshee/Voodoo3+
depends on DRM  PCI
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index 6e9e948..5dcb1a5 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -18,6 +18,7 @@ drm-$(CONFIG_COMPAT) += drm_ioc32.o
 drm-$(CONFIG_DRM_GEM_CMA_HELPER) += drm_gem_cma_helper.o
 
 drm_kms_helper-y := drm_fb_helper.o drm_crtc_helper.o drm_dp_i2c_helper.o
+drm_kms_helper-$(CONFIG_DRM_KMS_CMA_HELPER) += drm_fb_cma_helper.o
 
 obj-$(CONFIG_DRM_KMS_HELPER) += drm_kms_helper.o
 
diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c 
b/drivers/gpu/drm/drm_fb_cma_helper.c
new file mode 100644
index 000..9042233
--- /dev/null
+++ b/drivers/gpu/drm/drm_fb_cma_helper.c
@@ -0,0 +1,393 @@
+/*
+ * drm kms/fb cma (contiguous memory allocator) helper functions
+ *
+ * Copyright (C) 2012 Analog Device Inc.
+ *   Author: Lars-Peter Clausen l...@metafoo.de
+ *
+ * Based on udl_fbdev.c
+ *  Copyright (C) 2012 Red Hat
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include drm/drmP.h
+#include drm/drm_crtc.h
+#include drm/drm_fb_helper.h
+#include drm/drm_crtc_helper.h
+#include drm/drm_gem_cma_helper.h
+#include drm/drm_fb_cma_helper.h
+#include linux/module.h
+
+struct drm_fb_cma {
+   struct drm_framebuffer  fb;
+   struct drm_gem_cma_object   *obj[4];
+};
+
+struct drm_fbdev_cma {
+   struct drm_fb_helperfb_helper;
+   struct drm_fb_cma   *fb;
+};
+
+static inline struct drm_fbdev_cma *to_fbdev_cma(struct drm_fb_helper *helper)
+{
+   return container_of(helper, struct drm_fbdev_cma, fb_helper);
+}
+
+static inline struct drm_fb_cma *to_fb_cma(struct drm_framebuffer *fb)
+{
+   return container_of(fb, struct drm_fb_cma, fb);
+}
+
+static void drm_fb_cma_destroy(struct drm_framebuffer *fb)
+{
+   struct drm_fb_cma *fb_cma = to_fb_cma(fb);
+   int i;
+
+   for (i = 0; i  4; i++) {
+   if (fb_cma-obj[i])
+   
drm_gem_object_unreference_unlocked(fb_cma-obj[i]-base);
+   }
+
+   drm_framebuffer_cleanup(fb);
+   kfree(fb_cma);
+}
+
+static int drm_fb_cma_create_handle(struct drm_framebuffer *fb,
+   struct drm_file *file_priv, unsigned int *handle)
+{
+   struct drm_fb_cma *fb_cma = to_fb_cma(fb);
+
+   return drm_gem_handle_create(file_priv,
+   fb_cma-obj[0]-base, handle);
+}
+
+static struct drm_framebuffer_funcs drm_fb_cma_funcs = {
+   .destroy= drm_fb_cma_destroy,
+   .create_handle  = drm_fb_cma_create_handle,
+};
+
+static struct drm_fb_cma *drm_fb_cma_alloc(struct drm_device *dev,
+   struct drm_mode_fb_cmd2 *mode_cmd, struct drm_gem_cma_object **obj,
+   unsigned int num_planes)
+{
+   struct drm_fb_cma *fb_cma;
+   int ret;
+   int i;
+
+   fb_cma = kzalloc(sizeof(*fb_cma), GFP_KERNEL);
+   if (!fb_cma)
+   return ERR_PTR(-ENOMEM);
+
+   ret = drm_framebuffer_init(dev, fb_cma-fb, drm_fb_cma_funcs);
+   if (ret) {
+   dev_err(dev-dev, 

Re: [PATCH 1/3] drm/radeon: move ring locking out of reset path

2012-07-02 Thread Jerome Glisse
On Fri, Jun 29, 2012 at 12:15 PM, Michel Dänzer mic...@daenzer.net wrote:
 On Fre, 2012-06-29 at 17:18 +0200, Christian König wrote:
 On 29.06.2012 17:09, Michel Dänzer wrote:
  On Fre, 2012-06-29 at 16:45 +0200, Christian König wrote:
  Hold the ring lock the whole time the reset is in progress,
  otherwise another process can submit new jobs.
  Sounds good, but doesn't this create other paths (e.g. initialization,
  resume) where the ring is being accessed without holding the lock? Isn't
  that a problem?

 Thought about that also.

 For init I'm pretty sure that no application can submit commands before
 we are done, otherwise we are doomed anyway.

 For resume I'm not really sure, but I think that applications are
 resumed after the hardware driver had a chance of doing so.

 I hope you're right... but if it's not too much trouble, it might be
 better to be safe than sorry and take the lock for those paths as well.



NAK this is the wrong way to solve the issue, we need a global lock on
all path that can trigger gpu activities. Previously it was the cs
mutex, but i haven't thought about it too much when it got removed. So
to fix the situation i am sending a patch with rw semaphore.

Cheers,
Jerome
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH] drm/radeon: add an exclusive lock for GPU reset

2012-07-02 Thread j . glisse
From: Jerome Glisse jgli...@redhat.com

GPU reset need to be exclusive, one happening at a time. For this
add a rw semaphore so that any path that trigger GPU activities
have to take the semaphore as a reader thus allowing concurency.

The GPU reset path take the semaphore as a writer ensuring that
no concurrent reset take place.

Signed-off-by: Jerome Glisse jgli...@redhat.com
---
 drivers/gpu/drm/radeon/radeon.h|1 +
 drivers/gpu/drm/radeon/radeon_cs.c |5 +
 drivers/gpu/drm/radeon/radeon_device.c |2 ++
 drivers/gpu/drm/radeon/radeon_gem.c|7 +++
 4 files changed, 15 insertions(+)

diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
index 77b4519b..29d6986 100644
--- a/drivers/gpu/drm/radeon/radeon.h
+++ b/drivers/gpu/drm/radeon/radeon.h
@@ -1446,6 +1446,7 @@ struct radeon_device {
struct device   *dev;
struct drm_device   *ddev;
struct pci_dev  *pdev;
+   struct rw_semaphore exclusive_lock;
/* ASIC */
union radeon_asic_configconfig;
enum radeon_family  family;
diff --git a/drivers/gpu/drm/radeon/radeon_cs.c 
b/drivers/gpu/drm/radeon/radeon_cs.c
index f1b7527..7ee6491 100644
--- a/drivers/gpu/drm/radeon/radeon_cs.c
+++ b/drivers/gpu/drm/radeon/radeon_cs.c
@@ -499,7 +499,9 @@ int radeon_cs_ioctl(struct drm_device *dev, void *data, 
struct drm_file *filp)
struct radeon_cs_parser parser;
int r;
 
+   down_read(rdev-exclusive_lock);
if (!rdev-accel_working) {
+   up_read(rdev-exclusive_lock);
return -EBUSY;
}
/* initialize parser */
@@ -512,6 +514,7 @@ int radeon_cs_ioctl(struct drm_device *dev, void *data, 
struct drm_file *filp)
if (r) {
DRM_ERROR(Failed to initialize parser !\n);
radeon_cs_parser_fini(parser, r);
+   up_read(rdev-exclusive_lock);
r = radeon_cs_handle_lockup(rdev, r);
return r;
}
@@ -520,6 +523,7 @@ int radeon_cs_ioctl(struct drm_device *dev, void *data, 
struct drm_file *filp)
if (r != -ERESTARTSYS)
DRM_ERROR(Failed to parse relocation %d!\n, r);
radeon_cs_parser_fini(parser, r);
+   up_read(rdev-exclusive_lock);
r = radeon_cs_handle_lockup(rdev, r);
return r;
}
@@ -533,6 +537,7 @@ int radeon_cs_ioctl(struct drm_device *dev, void *data, 
struct drm_file *filp)
}
 out:
radeon_cs_parser_fini(parser, r);
+   up_read(rdev-exclusive_lock);
r = radeon_cs_handle_lockup(rdev, r);
return r;
 }
diff --git a/drivers/gpu/drm/radeon/radeon_device.c 
b/drivers/gpu/drm/radeon/radeon_device.c
index f654ba8..c8fdb40 100644
--- a/drivers/gpu/drm/radeon/radeon_device.c
+++ b/drivers/gpu/drm/radeon/radeon_device.c
@@ -988,6 +988,7 @@ int radeon_gpu_reset(struct radeon_device *rdev)
int r;
int resched;
 
+   down_write(rdev-exclusive_lock);
radeon_save_bios_scratch_regs(rdev);
/* block TTM */
resched = ttm_bo_lock_delayed_workqueue(rdev-mman.bdev);
@@ -1007,6 +1008,7 @@ int radeon_gpu_reset(struct radeon_device *rdev)
dev_info(rdev-dev, GPU reset failed\n);
}
 
+   up_write(rdev-exclusive_lock);
return r;
 }
 
diff --git a/drivers/gpu/drm/radeon/radeon_gem.c 
b/drivers/gpu/drm/radeon/radeon_gem.c
index d9b0809..f99db63 100644
--- a/drivers/gpu/drm/radeon/radeon_gem.c
+++ b/drivers/gpu/drm/radeon/radeon_gem.c
@@ -215,12 +215,14 @@ int radeon_gem_create_ioctl(struct drm_device *dev, void 
*data,
uint32_t handle;
int r;
 
+   down_read(rdev-exclusive_lock);
/* create a gem object to contain this object in */
args-size = roundup(args-size, PAGE_SIZE);
r = radeon_gem_object_create(rdev, args-size, args-alignment,
args-initial_domain, false,
false, gobj);
if (r) {
+   up_read(rdev-exclusive_lock);
r = radeon_gem_handle_lockup(rdev, r);
return r;
}
@@ -228,10 +230,12 @@ int radeon_gem_create_ioctl(struct drm_device *dev, void 
*data,
/* drop reference from allocate - handle holds it now */
drm_gem_object_unreference_unlocked(gobj);
if (r) {
+   up_read(rdev-exclusive_lock);
r = radeon_gem_handle_lockup(rdev, r);
return r;
}
args-handle = handle;
+   up_read(rdev-exclusive_lock);
return 0;
 }
 
@@ -240,6 +244,7 @@ int radeon_gem_set_domain_ioctl(struct drm_device *dev, 
void *data,
 {
/* transition the BO to a domain -
 * just validate the BO into a certain domain */
+   struct radeon_device *rdev = dev-dev_private;
struct 

Re: gma500 suspend to ram fails (3.4)

2012-07-02 Thread Mattia Dongili
On Thu, Jun 21, 2012 at 06:19:25AM +0900, Mattia Dongili wrote:
 On Tue, Jun 19, 2012 at 07:56:52PM +0900, Mattia Dongili wrote:
  On Mon, Jun 18, 2012 at 10:04:00PM +0200, Patrik Jakobsson wrote:
   On Sun, Jun 17, 2012 at 12:03 PM, Mattia Dongili malat...@linux.it 
   wrote:
 ...
   If possible, add drm.debug=7 to your boot line and send a dmesg of 
   3.5-rc3.
  
  attached the dmesg on rc3 with drm.debug=7.
  In addition I have a black screen and 3 modprobe processes stuck that
  udev tried to kill for a bit... then the console went blank and I can't
  tell if it's still trying:
  
359 ?R 11:08 /sbin/modprobe -b 
  pci:v8086d8108sv104Dsd905Fbc03sc00i00
361 ?D  0:00 /sbin/modprobe -b 
  pci:v8086d811Bsv104Dsd905Fbc04sc03i00
422 ?D  0:00 /sbin/modprobe -b 
  pci:v8086d8119sv104Dsd905Fbc06sc01i00
 
 for the record, here below is where the screen flickers a bit and
 booting gets stuck until udev starts trying to kill the modprobe calls.

I gave 3.5-rc4 a try, no significant change but by comparing 3.4 and 3.5
logs I have this difference that may mean something:

[0.00] Linux version 3.5.0-rc4...
[5.712020] gma500 :00:02.0: Backlight lvds set brightness 74367436

while with 3.4

[5.525296] gma500 :00:02.0: Backlight lvds set brightness 74407440

a did put some printks here and there and psb_driver_init seems to never
return from gma_backlight_init.

-- 
mattia
:wq!
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH, RFC] i.MX DRM support

2012-07-02 Thread Dirk Behme

On 02.07.2012 12:05, Sascha Hauer wrote:

On Thu, Jun 14, 2012 at 03:43:22PM +0200, Sascha Hauer wrote:

Hi All,

The following is the state-of-the-art i.MX IPU (Image Processing Unit)
DRM support.

This code is around for quite some time now and has been posted several
times with different APIs, first with plain old framebuffer support, now
DRM, first platform device binding, now devicetree. Unfortunately there's
quite much code needed to get something useful out of the IPU, so these
patches haven't received a lot of attention from people not involved in
i.MX. I think we have now come to a point where this code needs more public
exposure and where it's easier to talk in incremental changes instead of
blobs. Therefore I request this to go to staging for some cycles.


Dave, Greg,

Comments to this one? I addressed the comments I received so far and am
about to respin this series. Is it ok to put this to staging? If yes,
should I move the whole stuff into drivers/staging/ or should it stay
in drivers/gpu/drm with just a Kconfig dependency on STAGING?


We are very interested in this, so +1 from my side for this.

Many thanks and best regards

Dirk

Btw.: Based on

http://git.pengutronix.de/?p=imx/linux-2.6.git;a=shortlog;h=refs/heads/work/gpu/imx-drm-ipu-complete

with [1] below I fixed some compiler warnings.

I have some additional warnings

drivers/gpu/drm/drm_edid.c: In function 'drm_find_cea_extension':
drivers/gpu/drm/drm_edid.c:1466: warning: array subscript is above array 
bounds

drivers/gpu/drm/drm_edid.c: In function 'drm_detect_hdmi_monitor':
drivers/gpu/drm/drm_edid.c:1466: warning: array subscript is above array 
bounds

drivers/gpu/drm/drm_edid.c: In function 'drm_detect_monitor_audio':
drivers/gpu/drm/drm_edid.c:1466: warning: array subscript is above array 
bounds

drivers/gpu/drm/drm_edid.c: In function 'drm_edid_to_eld':
drivers/gpu/drm/drm_edid.c:1466: warning: array subscript is above array 
bounds

drivers/gpu/drm/drm_edid.c: In function 'drm_add_edid_modes':
drivers/gpu/drm/drm_edid.c:1466: warning: array subscript is above array 
bounds
drivers/gpu/drm/drm_edid.c:1466: warning: array subscript is above array 
bounds


but these seems to be compiler specific and not related to Sacha's patches.

[1]

From: Dirk Behme dirk.be...@de.bosch.com
Subject: [PATCH 1/2] DRM i.MX: ldb: Fix compiler warnings

Fix the compiler warnings

drivers/gpu/drm/imx/imx-ldb.c: In function 'imx_ldb_encoder_mode_fixup':
drivers/gpu/drm/imx/imx-ldb.c:118: warning: unused variable 'imx_ldb_ch'
drivers/gpu/drm/imx/imx-ldb.c: In function 'imx_ldb_encoder_prepare':
drivers/gpu/drm/imx/imx-ldb.c:134: warning: format '%ld' expects type 
'long int', but argument 6 has type 'int'


Signed-off-by: Dirk Behme dirk.be...@de.bosch.com

---
 drivers/gpu/drm/imx/imx-ldb.c |4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Index: freescale-linux-2.6-imx.git/drivers/gpu/drm/imx/imx-ldb.c
===
--- freescale-linux-2.6-imx.git.orig/drivers/gpu/drm/imx/imx-ldb.c
+++ freescale-linux-2.6-imx.git/drivers/gpu/drm/imx/imx-ldb.c
@@ -115,9 +115,9 @@ static bool imx_ldb_encoder_mode_fixup(s
   struct drm_display_mode *mode,
   struct drm_display_mode *adjusted_mode)
 {
+/*
struct imx_ldb_channel *imx_ldb_ch = enc_to_imx_ldb_ch(encoder);

-/*
adjusted_mode-clock = clk_round_rate(imx_ldb_ch-clk_pll,
adjusted_mode-clock * 1000) / 1000;
 */
@@ -131,7 +131,7 @@ static void imx_ldb_encoder_prepare(stru
int ret;
struct drm_display_mode *mode = encoder-crtc-mode;

-   dev_dbg(ldb-dev, %s: now: %ld want: %ld\n, __func__,
+   dev_dbg(ldb-dev, %s: now: %ld want: %d\n, __func__,
clk_get_rate(imx_ldb_ch-clk_pll),
mode-clock * 1000 * 7);
clk_set_rate(imx_ldb_ch-clk_pll, mode-clock * 1000 * 7);


___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH 1/3] drm/radeon: move ring locking out of reset path

2012-07-02 Thread Christian König

On 02.07.2012 17:41, Jerome Glisse wrote:

On Fri, Jun 29, 2012 at 12:15 PM, Michel Dänzer mic...@daenzer.net wrote:

On Fre, 2012-06-29 at 17:18 +0200, Christian König wrote:

On 29.06.2012 17:09, Michel Dänzer wrote:

On Fre, 2012-06-29 at 16:45 +0200, Christian König wrote:

Hold the ring lock the whole time the reset is in progress,
otherwise another process can submit new jobs.

Sounds good, but doesn't this create other paths (e.g. initialization,
resume) where the ring is being accessed without holding the lock? Isn't
that a problem?

Thought about that also.

For init I'm pretty sure that no application can submit commands before
we are done, otherwise we are doomed anyway.

For resume I'm not really sure, but I think that applications are
resumed after the hardware driver had a chance of doing so.

I hope you're right... but if it's not too much trouble, it might be
better to be safe than sorry and take the lock for those paths as well.



NAK this is the wrong way to solve the issue, we need a global lock on
all path that can trigger gpu activities. Previously it was the cs
mutex, but i haven't thought about it too much when it got removed. So
to fix the situation i am sending a patch with rw semaphore.

So what I'm missing? What else can trigger GPU activity when not the rings?

I'm currently working on ring-partial resets and also resets where you 
only skip over a single faulty IB instead of flushing the whole ring. 
And my current idea for that to work is that we hold the ring lock while 
we do suspend, ring_save, asic_reset, resume and ring_restore.


Christian.



Cheers,
Jerome




___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH 1/3] drm/radeon: move ring locking out of reset path

2012-07-02 Thread Jerome Glisse
On Mon, Jul 2, 2012 at 11:58 AM, Christian König
deathsim...@vodafone.de wrote:
 On 02.07.2012 17:41, Jerome Glisse wrote:

 On Fri, Jun 29, 2012 at 12:15 PM, Michel Dänzer mic...@daenzer.net
 wrote:

 On Fre, 2012-06-29 at 17:18 +0200, Christian König wrote:

 On 29.06.2012 17:09, Michel Dänzer wrote:

 On Fre, 2012-06-29 at 16:45 +0200, Christian König wrote:

 Hold the ring lock the whole time the reset is in progress,
 otherwise another process can submit new jobs.

 Sounds good, but doesn't this create other paths (e.g. initialization,
 resume) where the ring is being accessed without holding the lock?
 Isn't
 that a problem?

 Thought about that also.

 For init I'm pretty sure that no application can submit commands before
 we are done, otherwise we are doomed anyway.

 For resume I'm not really sure, but I think that applications are
 resumed after the hardware driver had a chance of doing so.

 I hope you're right... but if it's not too much trouble, it might be
 better to be safe than sorry and take the lock for those paths as well.


 NAK this is the wrong way to solve the issue, we need a global lock on
 all path that can trigger gpu activities. Previously it was the cs
 mutex, but i haven't thought about it too much when it got removed. So
 to fix the situation i am sending a patch with rw semaphore.

 So what I'm missing? What else can trigger GPU activity when not the rings?

 I'm currently working on ring-partial resets and also resets where you only
 skip over a single faulty IB instead of flushing the whole ring. And my
 current idea for that to work is that we hold the ring lock while we do
 suspend, ring_save, asic_reset, resume and ring_restore.

 Christian.


I just sent a patch, the idea is that you want gpu reset to be an
exclusive operation like gpu init, or gpu resume. So by taking rw
semaphore you allow the gpu reset to be exclusive and so you know
nobody can trigger gpu activies while still allowing concurrency in
case no gpu reset is on going.

Cheers,
Jerome
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel


[RFC v2] implicit drm synchronization wip with dma-buf

2012-07-02 Thread Maarten Lankhorst
Well,

V2 time! I was hinted to look at ttm_execbuf_util, and it does indeed contain 
some nice code.
My goal was to extend dma-buf in a generic way now, some elements from v1 are 
remaining,
most notably a dma-buf used for syncing. However it is expected now that 
dma-buf syncing will
work in a very specific way now, slightly more strict than in v1.

Instead of each buffer having their own dma-buf I put in 1 per command 
submission.

This submission hasn't been run-time tested yet, but I expect the api to go 
something like this.

Intended to be used like this:

list_init(head);
add_list_tail(validate1-entry, head);
add_list_tail(validate2-entry, head);
add_list_tail(validate3-entry, head);

r = dmabufmgr_eu_reserve_buffers(head);
if (r) return r;

// add waits on cpu or gpu
list_for_each_entry(validate, ...) {

if (!validate-sync_buf)
continue;

// Check attachments to see if we already imported sync_buf
// somewhere, if not attach to it.
// waiting until cur_seq - dmabuf-sync_val = 0 on either cpu
// or as command submitted to gpu
// sync_buf itself is a dma-buf, so it should be trivial
// TODO: sync_buf should NEVER be validated, add is_sync_buf to dma_buf?

// If this step fails: dmabufmgr_eu_backoff_reservation
// else:
// dmabufmgr_eu_fence_buffer_objects(our_own_sync_buf,
// hwchannel * max(minhwalign, 4), ++counter[hwchannel]);

// XXX: Do we still require a minimum alignment? I set up 16 for 
nouveau,
// but this is no longer needed in this design since it only matters
// for writes for which nouveau would already control the offset.
}

// Some time after execbuffer was executed, doesn't have to be right away but 
before
// getting in the danger of our own counter wrapping around:
// grab dmabufmgr.lru_lock, and cleanup by unreffing sync_buf when
// sync_buf == ownbuf, sync_ofs == ownofs, and sync_val == saved_counter
// In the meantime someone else or even us might have reserved this 
dma_buf
// again, which is why all those checks are needed before unreffing.

diff --git a/drivers/base/Makefile b/drivers/base/Makefile
index 5aa2d70..86e7598 100644
--- a/drivers/base/Makefile
+++ b/drivers/base/Makefile
@@ -10,7 +10,7 @@ obj-$(CONFIG_CMA) += dma-contiguous.o
 obj-y  += power/
 obj-$(CONFIG_HAS_DMA)  += dma-mapping.o
 obj-$(CONFIG_HAVE_GENERIC_DMA_COHERENT) += dma-coherent.o
-obj-$(CONFIG_DMA_SHARED_BUFFER) += dma-buf.o
+obj-$(CONFIG_DMA_SHARED_BUFFER) += dma-buf.o dma-buf-mgr.o dma-buf-mgr-eu.o
 obj-$(CONFIG_ISA)  += isa.o
 obj-$(CONFIG_FW_LOADER)+= firmware_class.o
 obj-$(CONFIG_NUMA) += node.o
diff --git a/drivers/base/dma-buf-mgr-eu.c b/drivers/base/dma-buf-mgr-eu.c
new file mode 100644
index 000..27ebc68
--- /dev/null
+++ b/drivers/base/dma-buf-mgr-eu.c
@@ -0,0 +1,170 @@
+/*
+ * Copyright (C) 2012 Canonical Ltd
+ *
+ * Based on ttm_bo.c which bears the following copyright notice,
+ * but is dual licensed:
+ *
+ * Copyright (c) 2006-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 linux/dma-buf-mgr.h
+#include linux/sched.h
+#include linux/export.h
+
+static void dmabufmgr_eu_backoff_reservation_locked(struct list_head *list)
+{
+   struct dmabufmgr_validate *entry;
+
+   list_for_each_entry(entry, list, head) {
+   struct dma_buf *bo = entry-bo;
+   if (!entry-reserved)
+   continue;
+
+   entry-reserved = false;
+   atomic_set(bo-reserved, 0);
+   wake_up_all(bo-event_queue);
+   if (entry-sync_buf)
+   dma_buf_put(entry-sync_buf);
+  

Re: [PATCH 1/3] drm/radeon: move ring locking out of reset path

2012-07-02 Thread Jerome Glisse
On Mon, Jul 2, 2012 at 11:58 AM, Christian König
deathsim...@vodafone.de wrote:
 On 02.07.2012 17:41, Jerome Glisse wrote:

 On Fri, Jun 29, 2012 at 12:15 PM, Michel Dänzer mic...@daenzer.net
 wrote:

 On Fre, 2012-06-29 at 17:18 +0200, Christian König wrote:

 On 29.06.2012 17:09, Michel Dänzer wrote:

 On Fre, 2012-06-29 at 16:45 +0200, Christian König wrote:

 Hold the ring lock the whole time the reset is in progress,
 otherwise another process can submit new jobs.

 Sounds good, but doesn't this create other paths (e.g. initialization,
 resume) where the ring is being accessed without holding the lock?
 Isn't
 that a problem?

 Thought about that also.

 For init I'm pretty sure that no application can submit commands before
 we are done, otherwise we are doomed anyway.

 For resume I'm not really sure, but I think that applications are
 resumed after the hardware driver had a chance of doing so.

 I hope you're right... but if it's not too much trouble, it might be
 better to be safe than sorry and take the lock for those paths as well.


 NAK this is the wrong way to solve the issue, we need a global lock on
 all path that can trigger gpu activities. Previously it was the cs
 mutex, but i haven't thought about it too much when it got removed. So
 to fix the situation i am sending a patch with rw semaphore.

 So what I'm missing? What else can trigger GPU activity when not the rings?

 I'm currently working on ring-partial resets and also resets where you only
 skip over a single faulty IB instead of flushing the whole ring. And my
 current idea for that to work is that we hold the ring lock while we do
 suspend, ring_save, asic_reset, resume and ring_restore.

 Christian.


I should add that gpu_reset should be an heavy reset, and if you want
to only reset one ring and see if gpu can continue without heavy reset
then you should do it as a special ring reset that doesn't reset mc
and some other block but only the affected ring (and i am assuming
that hw behave properly here). If that light weight ring reset doesn't
work than let the heavy reset kicks in. So yes your light weight per
ring reset would only need to take the ring lock but now need to
change the ring lock usage we have now.

Cheers,
Jerome
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH] drm/radeon: add an exclusive lock for GPU reset

2012-07-02 Thread Jerome Glisse
On Mon, Jul 2, 2012 at 11:39 AM,  j.gli...@gmail.com wrote:
 From: Jerome Glisse jgli...@redhat.com

 GPU reset need to be exclusive, one happening at a time. For this
 add a rw semaphore so that any path that trigger GPU activities
 have to take the semaphore as a reader thus allowing concurency.

 The GPU reset path take the semaphore as a writer ensuring that
 no concurrent reset take place.

 Signed-off-by: Jerome Glisse jgli...@redhat.com

Wrong patch sorry resending

 ---
  drivers/gpu/drm/radeon/radeon.h        |    1 +
  drivers/gpu/drm/radeon/radeon_cs.c     |    5 +
  drivers/gpu/drm/radeon/radeon_device.c |    2 ++
  drivers/gpu/drm/radeon/radeon_gem.c    |    7 +++
  4 files changed, 15 insertions(+)

 diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
 index 77b4519b..29d6986 100644
 --- a/drivers/gpu/drm/radeon/radeon.h
 +++ b/drivers/gpu/drm/radeon/radeon.h
 @@ -1446,6 +1446,7 @@ struct radeon_device {
        struct device                   *dev;
        struct drm_device               *ddev;
        struct pci_dev                  *pdev;
 +       struct rw_semaphore             exclusive_lock;
        /* ASIC */
        union radeon_asic_config        config;
        enum radeon_family              family;
 diff --git a/drivers/gpu/drm/radeon/radeon_cs.c 
 b/drivers/gpu/drm/radeon/radeon_cs.c
 index f1b7527..7ee6491 100644
 --- a/drivers/gpu/drm/radeon/radeon_cs.c
 +++ b/drivers/gpu/drm/radeon/radeon_cs.c
 @@ -499,7 +499,9 @@ int radeon_cs_ioctl(struct drm_device *dev, void *data, 
 struct drm_file *filp)
        struct radeon_cs_parser parser;
        int r;

 +       down_read(rdev-exclusive_lock);
        if (!rdev-accel_working) {
 +               up_read(rdev-exclusive_lock);
                return -EBUSY;
        }
        /* initialize parser */
 @@ -512,6 +514,7 @@ int radeon_cs_ioctl(struct drm_device *dev, void *data, 
 struct drm_file *filp)
        if (r) {
                DRM_ERROR(Failed to initialize parser !\n);
                radeon_cs_parser_fini(parser, r);
 +               up_read(rdev-exclusive_lock);
                r = radeon_cs_handle_lockup(rdev, r);
                return r;
        }
 @@ -520,6 +523,7 @@ int radeon_cs_ioctl(struct drm_device *dev, void *data, 
 struct drm_file *filp)
                if (r != -ERESTARTSYS)
                        DRM_ERROR(Failed to parse relocation %d!\n, r);
                radeon_cs_parser_fini(parser, r);
 +               up_read(rdev-exclusive_lock);
                r = radeon_cs_handle_lockup(rdev, r);
                return r;
        }
 @@ -533,6 +537,7 @@ int radeon_cs_ioctl(struct drm_device *dev, void *data, 
 struct drm_file *filp)
        }
  out:
        radeon_cs_parser_fini(parser, r);
 +       up_read(rdev-exclusive_lock);
        r = radeon_cs_handle_lockup(rdev, r);
        return r;
  }
 diff --git a/drivers/gpu/drm/radeon/radeon_device.c 
 b/drivers/gpu/drm/radeon/radeon_device.c
 index f654ba8..c8fdb40 100644
 --- a/drivers/gpu/drm/radeon/radeon_device.c
 +++ b/drivers/gpu/drm/radeon/radeon_device.c
 @@ -988,6 +988,7 @@ int radeon_gpu_reset(struct radeon_device *rdev)
        int r;
        int resched;

 +       down_write(rdev-exclusive_lock);
        radeon_save_bios_scratch_regs(rdev);
        /* block TTM */
        resched = ttm_bo_lock_delayed_workqueue(rdev-mman.bdev);
 @@ -1007,6 +1008,7 @@ int radeon_gpu_reset(struct radeon_device *rdev)
                dev_info(rdev-dev, GPU reset failed\n);
        }

 +       up_write(rdev-exclusive_lock);
        return r;
  }

 diff --git a/drivers/gpu/drm/radeon/radeon_gem.c 
 b/drivers/gpu/drm/radeon/radeon_gem.c
 index d9b0809..f99db63 100644
 --- a/drivers/gpu/drm/radeon/radeon_gem.c
 +++ b/drivers/gpu/drm/radeon/radeon_gem.c
 @@ -215,12 +215,14 @@ int radeon_gem_create_ioctl(struct drm_device *dev, 
 void *data,
        uint32_t handle;
        int r;

 +       down_read(rdev-exclusive_lock);
        /* create a gem object to contain this object in */
        args-size = roundup(args-size, PAGE_SIZE);
        r = radeon_gem_object_create(rdev, args-size, args-alignment,
                                        args-initial_domain, false,
                                        false, gobj);
        if (r) {
 +               up_read(rdev-exclusive_lock);
                r = radeon_gem_handle_lockup(rdev, r);
                return r;
        }
 @@ -228,10 +230,12 @@ int radeon_gem_create_ioctl(struct drm_device *dev, 
 void *data,
        /* drop reference from allocate - handle holds it now */
        drm_gem_object_unreference_unlocked(gobj);
        if (r) {
 +               up_read(rdev-exclusive_lock);
                r = radeon_gem_handle_lockup(rdev, r);
                return r;
        }
        args-handle = handle;
 +       up_read(rdev-exclusive_lock);
        return 0;
  }

 @@ -240,6 +244,7 @@ int radeon_gem_set_domain_ioctl(struct drm_device *dev, 
 void *data,
  {
        /* transition the BO to a domain 

Re: [PATCH] drm/radeon: add an exclusive lock for GPU reset

2012-07-02 Thread Christian König

On 02.07.2012 17:39, j.gli...@gmail.com wrote:

From: Jerome Glisse jgli...@redhat.com

GPU reset need to be exclusive, one happening at a time. For this
add a rw semaphore so that any path that trigger GPU activities
have to take the semaphore as a reader thus allowing concurency.

The GPU reset path take the semaphore as a writer ensuring that
no concurrent reset take place.

Signed-off-by: Jerome Glisse jgli...@redhat.com
NAK, that isn't as bad as the cs mutex was but still to complicated. 
It's just too far up in the call stack, e.g. it tries to catch ioctl 
operations, instead of catching the underlying hardware operation which 
is caused by the ioctl/ttm/etc...


Why not just take the ring look as I suggested?

Christian.

---
  drivers/gpu/drm/radeon/radeon.h|1 +
  drivers/gpu/drm/radeon/radeon_cs.c |5 +
  drivers/gpu/drm/radeon/radeon_device.c |2 ++
  drivers/gpu/drm/radeon/radeon_gem.c|7 +++
  4 files changed, 15 insertions(+)

diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
index 77b4519b..29d6986 100644
--- a/drivers/gpu/drm/radeon/radeon.h
+++ b/drivers/gpu/drm/radeon/radeon.h
@@ -1446,6 +1446,7 @@ struct radeon_device {
struct device   *dev;
struct drm_device   *ddev;
struct pci_dev  *pdev;
+   struct rw_semaphore exclusive_lock;
/* ASIC */
union radeon_asic_configconfig;
enum radeon_family  family;
diff --git a/drivers/gpu/drm/radeon/radeon_cs.c 
b/drivers/gpu/drm/radeon/radeon_cs.c
index f1b7527..7ee6491 100644
--- a/drivers/gpu/drm/radeon/radeon_cs.c
+++ b/drivers/gpu/drm/radeon/radeon_cs.c
@@ -499,7 +499,9 @@ int radeon_cs_ioctl(struct drm_device *dev, void *data, 
struct drm_file *filp)
struct radeon_cs_parser parser;
int r;
  
+	down_read(rdev-exclusive_lock);

if (!rdev-accel_working) {
+   up_read(rdev-exclusive_lock);
return -EBUSY;
}
/* initialize parser */
@@ -512,6 +514,7 @@ int radeon_cs_ioctl(struct drm_device *dev, void *data, 
struct drm_file *filp)
if (r) {
DRM_ERROR(Failed to initialize parser !\n);
radeon_cs_parser_fini(parser, r);
+   up_read(rdev-exclusive_lock);
r = radeon_cs_handle_lockup(rdev, r);
return r;
}
@@ -520,6 +523,7 @@ int radeon_cs_ioctl(struct drm_device *dev, void *data, 
struct drm_file *filp)
if (r != -ERESTARTSYS)
DRM_ERROR(Failed to parse relocation %d!\n, r);
radeon_cs_parser_fini(parser, r);
+   up_read(rdev-exclusive_lock);
r = radeon_cs_handle_lockup(rdev, r);
return r;
}
@@ -533,6 +537,7 @@ int radeon_cs_ioctl(struct drm_device *dev, void *data, 
struct drm_file *filp)
}
  out:
radeon_cs_parser_fini(parser, r);
+   up_read(rdev-exclusive_lock);
r = radeon_cs_handle_lockup(rdev, r);
return r;
  }
diff --git a/drivers/gpu/drm/radeon/radeon_device.c 
b/drivers/gpu/drm/radeon/radeon_device.c
index f654ba8..c8fdb40 100644
--- a/drivers/gpu/drm/radeon/radeon_device.c
+++ b/drivers/gpu/drm/radeon/radeon_device.c
@@ -988,6 +988,7 @@ int radeon_gpu_reset(struct radeon_device *rdev)
int r;
int resched;
  
+	down_write(rdev-exclusive_lock);

radeon_save_bios_scratch_regs(rdev);
/* block TTM */
resched = ttm_bo_lock_delayed_workqueue(rdev-mman.bdev);
@@ -1007,6 +1008,7 @@ int radeon_gpu_reset(struct radeon_device *rdev)
dev_info(rdev-dev, GPU reset failed\n);
}
  
+	up_write(rdev-exclusive_lock);

return r;
  }
  
diff --git a/drivers/gpu/drm/radeon/radeon_gem.c b/drivers/gpu/drm/radeon/radeon_gem.c

index d9b0809..f99db63 100644
--- a/drivers/gpu/drm/radeon/radeon_gem.c
+++ b/drivers/gpu/drm/radeon/radeon_gem.c
@@ -215,12 +215,14 @@ int radeon_gem_create_ioctl(struct drm_device *dev, void 
*data,
uint32_t handle;
int r;
  
+	down_read(rdev-exclusive_lock);

/* create a gem object to contain this object in */
args-size = roundup(args-size, PAGE_SIZE);
r = radeon_gem_object_create(rdev, args-size, args-alignment,
args-initial_domain, false,
false, gobj);
if (r) {
+   up_read(rdev-exclusive_lock);
r = radeon_gem_handle_lockup(rdev, r);
return r;
}
@@ -228,10 +230,12 @@ int radeon_gem_create_ioctl(struct drm_device *dev, void 
*data,
/* drop reference from allocate - handle holds it now */
drm_gem_object_unreference_unlocked(gobj);
if (r) {
+   up_read(rdev-exclusive_lock);
r = radeon_gem_handle_lockup(rdev, r);
return r;
}
args-handle = 

Re: [PATCH] drm/radeon: add an exclusive lock for GPU reset

2012-07-02 Thread Jerome Glisse
On Mon, Jul 2, 2012 at 12:26 PM, Christian König
deathsim...@vodafone.de wrote:
 On 02.07.2012 17:39, j.gli...@gmail.com wrote:

 From: Jerome Glisse jgli...@redhat.com

 GPU reset need to be exclusive, one happening at a time. For this
 add a rw semaphore so that any path that trigger GPU activities
 have to take the semaphore as a reader thus allowing concurency.

 The GPU reset path take the semaphore as a writer ensuring that
 no concurrent reset take place.

 Signed-off-by: Jerome Glisse jgli...@redhat.com

 NAK, that isn't as bad as the cs mutex was but still to complicated. It's
 just too far up in the call stack, e.g. it tries to catch ioctl operations,
 instead of catching the underlying hardware operation which is caused by the
 ioctl/ttm/etc...

 Why not just take the ring look as I suggested?



No we can't use ring lock, we need to protect suspend/resume path and
we need an exclusive lock for that so we need a reset mutex at the
very least. But instead of having a reset mutex i prefer using a rw
lock so that we can stop ioctl until a reset goes through an let
pending ioctl take proper action. Think about multiple context trying
to reset GPU ...

Really this is the best option, the rw locking wont induce any lock
contention execept in gpu reset case which is anyway bad news.

Jerome
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH] drm/radeon: fix rare segfault

2012-07-02 Thread j . glisse
From: Jerome Glisse jgli...@redhat.com

In gem idle/busy ioctl the radeon object was derefenced after
drm_gem_object_unreference_unlocked which in case the object
have been destroyed lead to use of a possibly free pointer with
possibly wrong data.

Signed-off-by: Jerome Glisse jgli...@redhat.com
---
 drivers/gpu/drm/radeon/radeon_gem.c |   10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_gem.c 
b/drivers/gpu/drm/radeon/radeon_gem.c
index 74176c5..c8838fc 100644
--- a/drivers/gpu/drm/radeon/radeon_gem.c
+++ b/drivers/gpu/drm/radeon/radeon_gem.c
@@ -325,6 +325,7 @@ int radeon_gem_mmap_ioctl(struct drm_device *dev, void 
*data,
 int radeon_gem_busy_ioctl(struct drm_device *dev, void *data,
  struct drm_file *filp)
 {
+   struct radeon_device *rdev = dev-dev_private;
struct drm_radeon_gem_busy *args = data;
struct drm_gem_object *gobj;
struct radeon_bo *robj;
@@ -350,13 +351,14 @@ int radeon_gem_busy_ioctl(struct drm_device *dev, void 
*data,
break;
}
drm_gem_object_unreference_unlocked(gobj);
-   r = radeon_gem_handle_lockup(robj-rdev, r);
+   r = radeon_gem_handle_lockup(rdev, r);
return r;
 }
 
 int radeon_gem_wait_idle_ioctl(struct drm_device *dev, void *data,
  struct drm_file *filp)
 {
+   struct radeon_device *rdev = dev-dev_private;
struct drm_radeon_gem_wait_idle *args = data;
struct drm_gem_object *gobj;
struct radeon_bo *robj;
@@ -369,10 +371,10 @@ int radeon_gem_wait_idle_ioctl(struct drm_device *dev, 
void *data,
robj = gem_to_radeon_bo(gobj);
r = radeon_bo_wait(robj, NULL, false);
/* callback hw specific functions if any */
-   if (robj-rdev-asic-ioctl_wait_idle)
-   robj-rdev-asic-ioctl_wait_idle(robj-rdev, robj);
+   if (rdev-asic-ioctl_wait_idle)
+   robj-rdev-asic-ioctl_wait_idle(rdev, robj);
drm_gem_object_unreference_unlocked(gobj);
-   r = radeon_gem_handle_lockup(robj-rdev, r);
+   r = radeon_gem_handle_lockup(rdev, r);
return r;
 }
 
-- 
1.7.10.2

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH] drm/radeon: add an exclusive lock for GPU reset v2

2012-07-02 Thread j . glisse
From: Jerome Glisse jgli...@redhat.com

GPU reset need to be exclusive, one happening at a time. For this
add a rw semaphore so that any path that trigger GPU activities
have to take the semaphore as a reader thus allowing concurency.

The GPU reset path take the semaphore as a writer ensuring that
no concurrent reset take place.

v2: init rw semaphore

Signed-off-by: Jerome Glisse jgli...@redhat.com
---
 drivers/gpu/drm/radeon/radeon.h|1 +
 drivers/gpu/drm/radeon/radeon_cs.c |5 +
 drivers/gpu/drm/radeon/radeon_device.c |3 +++
 drivers/gpu/drm/radeon/radeon_gem.c|8 
 4 files changed, 17 insertions(+)

diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
index 77b4519b..29d6986 100644
--- a/drivers/gpu/drm/radeon/radeon.h
+++ b/drivers/gpu/drm/radeon/radeon.h
@@ -1446,6 +1446,7 @@ struct radeon_device {
struct device   *dev;
struct drm_device   *ddev;
struct pci_dev  *pdev;
+   struct rw_semaphore exclusive_lock;
/* ASIC */
union radeon_asic_configconfig;
enum radeon_family  family;
diff --git a/drivers/gpu/drm/radeon/radeon_cs.c 
b/drivers/gpu/drm/radeon/radeon_cs.c
index f1b7527..7ee6491 100644
--- a/drivers/gpu/drm/radeon/radeon_cs.c
+++ b/drivers/gpu/drm/radeon/radeon_cs.c
@@ -499,7 +499,9 @@ int radeon_cs_ioctl(struct drm_device *dev, void *data, 
struct drm_file *filp)
struct radeon_cs_parser parser;
int r;
 
+   down_read(rdev-exclusive_lock);
if (!rdev-accel_working) {
+   up_read(rdev-exclusive_lock);
return -EBUSY;
}
/* initialize parser */
@@ -512,6 +514,7 @@ int radeon_cs_ioctl(struct drm_device *dev, void *data, 
struct drm_file *filp)
if (r) {
DRM_ERROR(Failed to initialize parser !\n);
radeon_cs_parser_fini(parser, r);
+   up_read(rdev-exclusive_lock);
r = radeon_cs_handle_lockup(rdev, r);
return r;
}
@@ -520,6 +523,7 @@ int radeon_cs_ioctl(struct drm_device *dev, void *data, 
struct drm_file *filp)
if (r != -ERESTARTSYS)
DRM_ERROR(Failed to parse relocation %d!\n, r);
radeon_cs_parser_fini(parser, r);
+   up_read(rdev-exclusive_lock);
r = radeon_cs_handle_lockup(rdev, r);
return r;
}
@@ -533,6 +537,7 @@ int radeon_cs_ioctl(struct drm_device *dev, void *data, 
struct drm_file *filp)
}
 out:
radeon_cs_parser_fini(parser, r);
+   up_read(rdev-exclusive_lock);
r = radeon_cs_handle_lockup(rdev, r);
return r;
 }
diff --git a/drivers/gpu/drm/radeon/radeon_device.c 
b/drivers/gpu/drm/radeon/radeon_device.c
index f654ba8..254fdb4 100644
--- a/drivers/gpu/drm/radeon/radeon_device.c
+++ b/drivers/gpu/drm/radeon/radeon_device.c
@@ -734,6 +734,7 @@ int radeon_device_init(struct radeon_device *rdev,
mutex_init(rdev-gem.mutex);
mutex_init(rdev-pm.mutex);
init_rwsem(rdev-pm.mclk_lock);
+   init_rwsem(rdev-exclusive_lock);
init_waitqueue_head(rdev-irq.vblank_queue);
init_waitqueue_head(rdev-irq.idle_queue);
r = radeon_gem_init(rdev);
@@ -988,6 +989,7 @@ int radeon_gpu_reset(struct radeon_device *rdev)
int r;
int resched;
 
+   down_write(rdev-exclusive_lock);
radeon_save_bios_scratch_regs(rdev);
/* block TTM */
resched = ttm_bo_lock_delayed_workqueue(rdev-mman.bdev);
@@ -1007,6 +1009,7 @@ int radeon_gpu_reset(struct radeon_device *rdev)
dev_info(rdev-dev, GPU reset failed\n);
}
 
+   up_write(rdev-exclusive_lock);
return r;
 }
 
diff --git a/drivers/gpu/drm/radeon/radeon_gem.c 
b/drivers/gpu/drm/radeon/radeon_gem.c
index d9b0809..b0be9c4 100644
--- a/drivers/gpu/drm/radeon/radeon_gem.c
+++ b/drivers/gpu/drm/radeon/radeon_gem.c
@@ -215,12 +215,14 @@ int radeon_gem_create_ioctl(struct drm_device *dev, void 
*data,
uint32_t handle;
int r;
 
+   down_read(rdev-exclusive_lock);
/* create a gem object to contain this object in */
args-size = roundup(args-size, PAGE_SIZE);
r = radeon_gem_object_create(rdev, args-size, args-alignment,
args-initial_domain, false,
false, gobj);
if (r) {
+   up_read(rdev-exclusive_lock);
r = radeon_gem_handle_lockup(rdev, r);
return r;
}
@@ -228,10 +230,12 @@ int radeon_gem_create_ioctl(struct drm_device *dev, void 
*data,
/* drop reference from allocate - handle holds it now */
drm_gem_object_unreference_unlocked(gobj);
if (r) {
+   up_read(rdev-exclusive_lock);
r = radeon_gem_handle_lockup(rdev, r);
return r;
 

Re: [PATCH] drm/radeon: add an exclusive lock for GPU reset

2012-07-02 Thread Christian König

On 02.07.2012 18:41, Jerome Glisse wrote:

On Mon, Jul 2, 2012 at 12:26 PM, Christian König
deathsim...@vodafone.de wrote:

On 02.07.2012 17:39, j.gli...@gmail.com wrote:

From: Jerome Glisse jgli...@redhat.com

GPU reset need to be exclusive, one happening at a time. For this
add a rw semaphore so that any path that trigger GPU activities
have to take the semaphore as a reader thus allowing concurency.

The GPU reset path take the semaphore as a writer ensuring that
no concurrent reset take place.

Signed-off-by: Jerome Glisse jgli...@redhat.com

NAK, that isn't as bad as the cs mutex was but still to complicated. It's
just too far up in the call stack, e.g. it tries to catch ioctl operations,
instead of catching the underlying hardware operation which is caused by the
ioctl/ttm/etc...

Why not just take the ring look as I suggested?



No we can't use ring lock, we need to protect suspend/resume path and
we need an exclusive lock for that so we need a reset mutex at the
very least. But instead of having a reset mutex i prefer using a rw
lock so that we can stop ioctl until a reset goes through an let
pending ioctl take proper action. Think about multiple context trying
to reset GPU ...

Really this is the best option, the rw locking wont induce any lock
contention execept in gpu reset case which is anyway bad news.
Why? That makes no sense to me. Well I don't want to prevent lock 
contention, but understand why we should add locking at the ioctl level. 
That violates locking rule number one lock data instead of code (or in 
our case lock hardware access instead of code path) and it really is 
the reason why we ended up with the cs_mutex protecting practically 
everything.


Multiple context trying to reset the GPU should be pretty fine, current 
it would just reset the GPU twice, but in the future asic_reset should 
be much more fine grained and only reset the parts of the GPU which 
really needs an reset.


Cheers,
Christian.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH] drm/radeon: add an exclusive lock for GPU reset

2012-07-02 Thread Jerome Glisse
On Mon, Jul 2, 2012 at 1:05 PM, Christian König deathsim...@vodafone.de wrote:
 On 02.07.2012 18:41, Jerome Glisse wrote:

 On Mon, Jul 2, 2012 at 12:26 PM, Christian König
 deathsim...@vodafone.de wrote:

 On 02.07.2012 17:39, j.gli...@gmail.com wrote:

 From: Jerome Glisse jgli...@redhat.com

 GPU reset need to be exclusive, one happening at a time. For this
 add a rw semaphore so that any path that trigger GPU activities
 have to take the semaphore as a reader thus allowing concurency.

 The GPU reset path take the semaphore as a writer ensuring that
 no concurrent reset take place.

 Signed-off-by: Jerome Glisse jgli...@redhat.com

 NAK, that isn't as bad as the cs mutex was but still to complicated. It's
 just too far up in the call stack, e.g. it tries to catch ioctl
 operations,
 instead of catching the underlying hardware operation which is caused by
 the
 ioctl/ttm/etc...

 Why not just take the ring look as I suggested?


 No we can't use ring lock, we need to protect suspend/resume path and
 we need an exclusive lock for that so we need a reset mutex at the
 very least. But instead of having a reset mutex i prefer using a rw
 lock so that we can stop ioctl until a reset goes through an let
 pending ioctl take proper action. Think about multiple context trying
 to reset GPU ...

 Really this is the best option, the rw locking wont induce any lock
 contention execept in gpu reset case which is anyway bad news.

 Why? That makes no sense to me. Well I don't want to prevent lock
 contention, but understand why we should add locking at the ioctl level.
 That violates locking rule number one lock data instead of code (or in our
 case lock hardware access instead of code path) and it really is the
 reason why we ended up with the cs_mutex protecting practically everything.

 Multiple context trying to reset the GPU should be pretty fine, current it
 would just reset the GPU twice, but in the future asic_reset should be much
 more fine grained and only reset the parts of the GPU which really needs an
 reset.

 Cheers,
 Christian.

No multiple reset is not fine, try it your self and you will see all
kind of oops (strongly advise you to sync you hd before stress testing
that). Yes we need to protect code path because suspend/resume code
path is special one it touch many data in the device structure. GPU
reset is a rare event or should be a rare event.

I stress it we need at very least a mutex to protect gpu reset and i
will stand on that position because there is no other way around.
Using rw lock have a bonus of allowing proper handling of gpu reset
failure and that what the patch i sent to linus fix tree is about, so
to make drm next merge properly while preserving proper behavior in
gpu reset failure the rw semaphore is the best option.

Cheers,
Jerome
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel


[Bug 51658] r200 ( possibly radeon) DRI fixes for gnome shell on Mesa 8.0.3

2012-07-02 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=51658

--- Comment #1 from Eugene St Leger gri...@yahoo.com 2012-07-02 10:29:44 PDT 
---
Created attachment 63712
  -- https://bugs.freedesktop.org/attachment.cgi?id=63712
Essential patch to allow 2048 pixel blits.

Without this patch, gnome shell crashes. 2048 pixel blits are used.

-- 
Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are the assignee for the bug.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel


[Bug 51658] r200 ( possibly radeon) DRI fixes for gnome shell on Mesa 8.0.3

2012-07-02 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=51658

--- Comment #2 from Eugene St Leger gri...@yahoo.com 2012-07-02 10:34:41 PDT 
---
Created attachment 63713
  -- https://bugs.freedesktop.org/attachment.cgi?id=63713
Essential patch to reapply dirtied texenv registers.

Without this patch, colour corruption happens with xv.  xf86-video-ati textured
xv dirties texenv registers and r200 DRI does not reapply them.

-- 
Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are the assignee for the bug.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel


[Bug 51658] r200 ( possibly radeon) DRI fixes for gnome shell on Mesa 8.0.3

2012-07-02 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=51658

--- Comment #3 from Eugene St Leger gri...@yahoo.com 2012-07-02 10:37:47 PDT 
---
Created attachment 63714
  -- https://bugs.freedesktop.org/attachment.cgi?id=63714
Unessential fixes/enhancements patch.

Minor spelling fixes  enhancements.

-- 
Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are the assignee for the bug.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel


[Bug 51658] r200 ( possibly radeon) DRI fixes for gnome shell on Mesa 8.0.3

2012-07-02 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=51658

--- Comment #4 from Eugene St Leger gri...@yahoo.com 2012-07-02 10:40:51 PDT 
---
Created attachment 63715
  -- https://bugs.freedesktop.org/attachment.cgi?id=63715
Proposed blit register dirtying patch.

It appears bliting dirties some registers without notifying. This proposed
patch notifies of register dirtying.

-- 
Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are the assignee for the bug.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel


[Bug 51658] r200 ( possibly radeon) DRI fixes for gnome shell on Mesa 8.0.3

2012-07-02 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=51658

--- Comment #5 from Eugene St Leger gri...@yahoo.com 2012-07-02 10:45:01 UTC 
---
Created attachment 63716
  -- https://bugs.freedesktop.org/attachment.cgi?id=63716
Optional patch to warn (once) when a blit with 2048 pixel dimension occurs on
r200.

If 2048 pixel blits cause graphical glitches/problems on r200, this patch can
be used to provide a single warning.

-- 
Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are the assignee for the bug.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel


[Bug 51658] r200 ( possibly radeon) DRI fixes for gnome shell on Mesa 8.0.3

2012-07-02 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=51658

--- Comment #6 from Eugene St Leger gri...@yahoo.com 2012-07-02 10:49:07 PDT 
---
Created attachment 63717
  -- https://bugs.freedesktop.org/attachment.cgi?id=63717
Untested but probably essential patch to allow 2048 pixel blits on r100.

Without this patch, gnome shell is expected to crash. This patch is untested.

-- 
Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are the assignee for the bug.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel


[Bug 51658] r200 ( possibly radeon) DRI fixes for gnome shell on Mesa 8.0.3

2012-07-02 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=51658

--- Comment #7 from Eugene St Leger gri...@yahoo.com 2012-07-02 10:51:10 PDT 
---
Created attachment 63718
  -- https://bugs.freedesktop.org/attachment.cgi?id=63718
Optional untested patch to warn (once) when a blit with 2048 pixel dimension
occurs on r100.

If 2048 pixel blits cause graphical glitches/problems on r100, this patch can
be used to provide a single warning. This patch is untested.

-- 
Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are the assignee for the bug.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: Bogus video resolution in Linux 3.5-rc4

2012-07-02 Thread Adam Jackson

On 6/26/12 3:21 AM, Takashi Iwai wrote:


From: Takashi Iwai ti...@suse.de
Subject: [PATCH] drm: edid: Don't add inferred modes with higher resolution

When a monitor EDID doesn't give the preferred bit, driver assumes
that the mode with the higest resolution and rate is the preferred
mode.  Meanwhile the recent changes for allowing more modes in the
GFT/CVT ranges give actually more modes, and some modes may be over
the native size.  Thus such a mode would be picked up as the preferred
mode although it's no native resolution.

For avoiding such a problem, this patch limits the addition of
inferred modes by checking not to be greater than other modes.
Also, it checks the duplicated mode entry at the same time.


This is a little aggressive on CRTs, but whatever, better than what's there.

Reviewed-by: Adam Jackson a...@redhat.com

- ajax
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel


[Bug 33309] [855GM] GPU freeze due to overlay hang

2012-07-02 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=33309

--- Comment #28 from stefan fdo.12.ben...@xoxy.net 2012-07-02 13:13:59 PDT ---
Hi,
are there any news on this issue?
The 3.4 and 3.5-rc series seem stable wrt this issue,
but unfortunately something broke resume from s2ram badly,
the backlight stays off and the machine does not respond
even to SysRq and I need to do a hard power-off.

Cheers,
Stefan.

-- 
Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are the assignee for the bug.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel


[Bug 28622] radeon video lockup

2012-07-02 Thread bugzilla-daemon
https://bugzilla.kernel.org/show_bug.cgi?id=28622





--- Comment #11 from Alex Deucher alexdeuc...@gmail.com  2012-07-02 20:21:02 
---
Is this still an issue with a more recent kernel (3.x)?

-- 
Configure bugmail: https://bugzilla.kernel.org/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are watching the assignee of the bug.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH] drm/radeon: fix rare segfault

2012-07-02 Thread Alex Deucher
On Mon, Jul 2, 2012 at 12:40 PM,  j.gli...@gmail.com wrote:
 From: Jerome Glisse jgli...@redhat.com

 In gem idle/busy ioctl the radeon object was derefenced after
 drm_gem_object_unreference_unlocked which in case the object
 have been destroyed lead to use of a possibly free pointer with
 possibly wrong data.

 Signed-off-by: Jerome Glisse jgli...@redhat.com

Reviewed-by: Alex Deucher alexander.deuc...@amd.com

 ---
  drivers/gpu/drm/radeon/radeon_gem.c |   10 ++
  1 file changed, 6 insertions(+), 4 deletions(-)

 diff --git a/drivers/gpu/drm/radeon/radeon_gem.c 
 b/drivers/gpu/drm/radeon/radeon_gem.c
 index 74176c5..c8838fc 100644
 --- a/drivers/gpu/drm/radeon/radeon_gem.c
 +++ b/drivers/gpu/drm/radeon/radeon_gem.c
 @@ -325,6 +325,7 @@ int radeon_gem_mmap_ioctl(struct drm_device *dev, void 
 *data,
  int radeon_gem_busy_ioctl(struct drm_device *dev, void *data,
   struct drm_file *filp)
  {
 +   struct radeon_device *rdev = dev-dev_private;
 struct drm_radeon_gem_busy *args = data;
 struct drm_gem_object *gobj;
 struct radeon_bo *robj;
 @@ -350,13 +351,14 @@ int radeon_gem_busy_ioctl(struct drm_device *dev, void 
 *data,
 break;
 }
 drm_gem_object_unreference_unlocked(gobj);
 -   r = radeon_gem_handle_lockup(robj-rdev, r);
 +   r = radeon_gem_handle_lockup(rdev, r);
 return r;
  }

  int radeon_gem_wait_idle_ioctl(struct drm_device *dev, void *data,
   struct drm_file *filp)
  {
 +   struct radeon_device *rdev = dev-dev_private;
 struct drm_radeon_gem_wait_idle *args = data;
 struct drm_gem_object *gobj;
 struct radeon_bo *robj;
 @@ -369,10 +371,10 @@ int radeon_gem_wait_idle_ioctl(struct drm_device *dev, 
 void *data,
 robj = gem_to_radeon_bo(gobj);
 r = radeon_bo_wait(robj, NULL, false);
 /* callback hw specific functions if any */
 -   if (robj-rdev-asic-ioctl_wait_idle)
 -   robj-rdev-asic-ioctl_wait_idle(robj-rdev, robj);
 +   if (rdev-asic-ioctl_wait_idle)
 +   robj-rdev-asic-ioctl_wait_idle(rdev, robj);
 drm_gem_object_unreference_unlocked(gobj);
 -   r = radeon_gem_handle_lockup(robj-rdev, r);
 +   r = radeon_gem_handle_lockup(rdev, r);
 return r;
  }

 --
 1.7.10.2

 ___
 dri-devel mailing list
 dri-devel@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/dri-devel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel


[Bug 33309] [855GM] GPU freeze due to overlay hang

2012-07-02 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=33309

--- Comment #29 from Daniel Vetter dan...@ffwll.ch 2012-07-02 13:29:18 PDT ---
(In reply to comment #28)
 are there any news on this issue?
 The 3.4 and 3.5-rc series seem stable wrt this issue,
 but unfortunately something broke resume from s2ram badly,
 the backlight stays off and the machine does not respond
 even to SysRq and I need to do a hard power-off.

That's goodbad news. Can you try to bisect the backlight regression that has
been introduce in 3.4 and open a new bug report? That usually helps in fixing
it ...

-- 
Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are the assignee for the bug.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH libdrm] intel: Fix build failure in test_decode.c

2012-07-02 Thread Ben Widawsky
On Sat, 30 Jun 2012 13:12:45 +0300
Lauri Kasanen c...@gmx.com wrote:

 Hi list
 
 The recently released libdrm 2.4.37 does not compile the Intel part:
 
 test_decode.c: In function 'compare_batch':
 test_decode.c:107: error: implicit declaration of function 'open_memstream'
 
 PS: Please CC me.
 
 Signed-off-by: Lauri Kasanen c...@gmx.com
 ---
  intel/test_decode.c |2 ++
  1 files changed, 2 insertions(+), 0 deletions(-)
 
 diff --git a/intel/test_decode.c b/intel/test_decode.c
 index c9ab7ad..0fcdf3b 100644
 --- a/intel/test_decode.c
 +++ b/intel/test_decode.c
 @@ -21,6 +21,8 @@
   * IN THE SOFTWARE.
   */
  
 +#define _GNU_SOURCE
 +
  #include string.h
  #include stdlib.h
  #include stdio.h

I can't reproduce this. Can anyone else confirm this is broken, and if
so that the above patch fixes it?


-- 
Ben Widawsky, Intel Open Source Technology Center
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel