[PATCH 1/3] drm/radeon: fix debugfs handling
Having registered debugfs files globally causes
the files to not show up on the second, third
etc.. card in the system.
Signed-off-by: Christian König
Reviewed-by: Alex Deucher
---
drivers/gpu/drm/radeon/radeon.h|8
drivers/gpu/drm/radeon/radeon_device.c | 26 ++
2 files changed, 18 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
index e3170c7..ede1b1b 100644
--- a/drivers/gpu/drm/radeon/radeon.h
+++ b/drivers/gpu/drm/radeon/radeon.h
@@ -879,6 +879,11 @@ void radeon_test_moves(struct radeon_device *rdev);
/*
* Debugfs
*/
+struct radeon_debugfs {
+struct drm_info_list*files;
+unsignednum_files;
+};
+
int radeon_debugfs_add_files(struct radeon_device *rdev,
struct drm_info_list *files,
unsigned nfiles);
@@ -1241,6 +1246,9 @@ struct radeon_device {
struct drm_file *cmask_filp;
/* i2c buses */
struct radeon_i2c_chan *i2c_bus[RADEON_MAX_I2C_BUS];
+ /* debugfs */
+ struct radeon_debugfs debugfs[RADEON_DEBUGFS_MAX_COMPONENTS];
+ unsigneddebugfs_count;
};
int radeon_device_init(struct radeon_device *rdev,
diff --git a/drivers/gpu/drm/radeon/radeon_device.c
b/drivers/gpu/drm/radeon/radeon_device.c
index c33bc91..d50fa60 100644
--- a/drivers/gpu/drm/radeon/radeon_device.c
+++ b/drivers/gpu/drm/radeon/radeon_device.c
@@ -978,36 +978,29 @@ int radeon_gpu_reset(struct radeon_device *rdev)
/*
* Debugfs
*/
-struct radeon_debugfs {
- struct drm_info_list*files;
- unsignednum_files;
-};
-static struct radeon_debugfs _radeon_debugfs[RADEON_DEBUGFS_MAX_COMPONENTS];
-static unsigned _radeon_debugfs_count = 0;
-
int radeon_debugfs_add_files(struct radeon_device *rdev,
struct drm_info_list *files,
unsigned nfiles)
{
unsigned i;
- for (i = 0; i < _radeon_debugfs_count; i++) {
- if (_radeon_debugfs[i].files == files) {
+ for (i = 0; i < rdev->debugfs_count; i++) {
+ if (rdev->debugfs[i].files == files) {
/* Already registered */
return 0;
}
}
- i = _radeon_debugfs_count + 1;
+ i = rdev->debugfs_count + 1;
if (i > RADEON_DEBUGFS_MAX_COMPONENTS) {
DRM_ERROR("Reached maximum number of debugfs components.\n");
DRM_ERROR("Report so we increase "
"RADEON_DEBUGFS_MAX_COMPONENTS.\n");
return -EINVAL;
}
- _radeon_debugfs[_radeon_debugfs_count].files = files;
- _radeon_debugfs[_radeon_debugfs_count].num_files = nfiles;
- _radeon_debugfs_count = i;
+ rdev->debugfs[rdev->debugfs_count].files = files;
+ rdev->debugfs[rdev->debugfs_count].num_files = nfiles;
+ rdev->debugfs_count = i;
#if defined(CONFIG_DEBUG_FS)
drm_debugfs_create_files(files, nfiles,
rdev->ddev->control->debugfs_root,
@@ -1027,11 +1020,12 @@ int radeon_debugfs_init(struct drm_minor *minor)
void radeon_debugfs_cleanup(struct drm_minor *minor)
{
+struct radeon_device *rdev = minor->dev->dev_private;
unsigned i;
- for (i = 0; i < _radeon_debugfs_count; i++) {
- drm_debugfs_remove_files(_radeon_debugfs[i].files,
-_radeon_debugfs[i].num_files, minor);
+ for (i = 0; i < rdev->debugfs_count; i++) {
+ drm_debugfs_remove_files(rdev->debugfs[i].files,
+rdev->debugfs[i].num_files, minor);
}
}
#endif
--
1.7.5.4
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/3] drm/radeon: no need to check all relocs for duplicates
Only check the previously checked relocs for
duplicates. Also leaving the handle uninitialized
isn't such a good idea.
Signed-off-by: Christian König
---
drivers/gpu/drm/radeon/radeon_cs.c |5 +++--
1 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/radeon/radeon_cs.c
b/drivers/gpu/drm/radeon/radeon_cs.c
index fae00c0..7b6e98a 100644
--- a/drivers/gpu/drm/radeon/radeon_cs.c
+++ b/drivers/gpu/drm/radeon/radeon_cs.c
@@ -58,7 +58,7 @@ int radeon_cs_parser_relocs(struct radeon_cs_parser *p)
duplicate = false;
r = (struct drm_radeon_cs_reloc *)&chunk->kdata[i*4];
- for (j = 0; j < p->nrelocs; j++) {
+ for (j = 0; j < i; j++) {
if (r->handle == p->relocs[j].handle) {
p->relocs_ptr[i] = &p->relocs[j];
duplicate = true;
@@ -84,7 +84,8 @@ int radeon_cs_parser_relocs(struct radeon_cs_parser *p)
p->relocs[i].flags = r->flags;
radeon_bo_list_add_object(&p->relocs[i].lobj,
&p->validated);
- }
+ } else
+ p->relocs[i].handle = 0;
}
return radeon_bo_list_validate(&p->validated);
}
--
1.7.5.4
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
Some minor bugfixes
The following three patches are just minor bug fixes. I've send them to the list previously, but this time they are based upon drm-next instead of drm-fixes and I also fixed some spelling mistakes in the commit messages. Please commit. Thanks, Christian. ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 3/3] drm/radeon: fix a spelling mistake
Better fix it before this obvious typo spreads even more.
Signed-off-by: Christian König
Reviewed-by: Alex Deucher
---
drivers/gpu/drm/radeon/radeon.h |4 +-
drivers/gpu/drm/radeon/radeon_fence.c | 34
drivers/gpu/drm/radeon/radeon_pm.c|4 +-
drivers/gpu/drm/radeon/radeon_ring.c |2 +-
4 files changed, 22 insertions(+), 22 deletions(-)
diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
index ede1b1b..2fda4ae 100644
--- a/drivers/gpu/drm/radeon/radeon.h
+++ b/drivers/gpu/drm/radeon/radeon.h
@@ -198,7 +198,7 @@ struct radeon_fence_driver {
wait_queue_head_t queue;
rwlock_tlock;
struct list_headcreated;
- struct list_heademited;
+ struct list_heademitted;
struct list_headsignaled;
boolinitialized;
};
@@ -209,7 +209,7 @@ struct radeon_fence {
struct list_headlist;
/* protected by radeon_fence.lock */
uint32_tseq;
- boolemited;
+ boolemitted;
boolsignaled;
};
diff --git a/drivers/gpu/drm/radeon/radeon_fence.c
b/drivers/gpu/drm/radeon/radeon_fence.c
index a488b50..711bd6e 100644
--- a/drivers/gpu/drm/radeon/radeon_fence.c
+++ b/drivers/gpu/drm/radeon/radeon_fence.c
@@ -74,7 +74,7 @@ int radeon_fence_emit(struct radeon_device *rdev, struct
radeon_fence *fence)
unsigned long irq_flags;
write_lock_irqsave(&rdev->fence_drv.lock, irq_flags);
- if (fence->emited) {
+ if (fence->emitted) {
write_unlock_irqrestore(&rdev->fence_drv.lock, irq_flags);
return 0;
}
@@ -88,8 +88,8 @@ int radeon_fence_emit(struct radeon_device *rdev, struct
radeon_fence *fence)
radeon_fence_ring_emit(rdev, fence);
trace_radeon_fence_emit(rdev->ddev, fence->seq);
- fence->emited = true;
- list_move_tail(&fence->list, &rdev->fence_drv.emited);
+ fence->emitted = true;
+ list_move_tail(&fence->list, &rdev->fence_drv.emitted);
write_unlock_irqrestore(&rdev->fence_drv.lock, irq_flags);
return 0;
}
@@ -129,7 +129,7 @@ static bool radeon_fence_poll_locked(struct radeon_device
*rdev)
return false;
}
n = NULL;
- list_for_each(i, &rdev->fence_drv.emited) {
+ list_for_each(i, &rdev->fence_drv.emitted) {
fence = list_entry(i, struct radeon_fence, list);
if (fence->seq == seq) {
n = i;
@@ -145,7 +145,7 @@ static bool radeon_fence_poll_locked(struct radeon_device
*rdev)
fence = list_entry(i, struct radeon_fence, list);
fence->signaled = true;
i = n;
- } while (i != &rdev->fence_drv.emited);
+ } while (i != &rdev->fence_drv.emitted);
wake = true;
}
return wake;
@@ -159,7 +159,7 @@ static void radeon_fence_destroy(struct kref *kref)
fence = container_of(kref, struct radeon_fence, kref);
write_lock_irqsave(&fence->rdev->fence_drv.lock, irq_flags);
list_del(&fence->list);
- fence->emited = false;
+ fence->emitted = false;
write_unlock_irqrestore(&fence->rdev->fence_drv.lock, irq_flags);
kfree(fence);
}
@@ -174,7 +174,7 @@ int radeon_fence_create(struct radeon_device *rdev, struct
radeon_fence **fence)
}
kref_init(&((*fence)->kref));
(*fence)->rdev = rdev;
- (*fence)->emited = false;
+ (*fence)->emitted = false;
(*fence)->signaled = false;
(*fence)->seq = 0;
INIT_LIST_HEAD(&(*fence)->list);
@@ -203,8 +203,8 @@ bool radeon_fence_signaled(struct radeon_fence *fence)
if (fence->rdev->shutdown) {
signaled = true;
}
- if (!fence->emited) {
- WARN(1, "Querying an unemited fence : %p !\n", fence);
+ if (!fence->emitted) {
+ WARN(1, "Querying an unemitted fence : %p !\n", fence);
signaled = true;
}
if (!signaled) {
@@ -295,11 +295,11 @@ int radeon_fence_wait_next(struct radeon_device *rdev)
return 0;
}
write_lock_irqsave(&rdev->fence_drv.lock, irq_flags);
- if (list_empty(&rdev->fence_drv.emited)) {
+ if (list_empty(&rdev->fence_drv.emitted)) {
write_unlock_irqrestore(&rdev->fence_drv.lock, irq_flags);
return 0;
}
- fence = list_entry(rdev->fence_drv.emited.next,
+ fence = list_entry(rdev->fence_drv.emitted.next,
struct radeon_fence, list);
radeon_fence_ref(fence);
write_unlock_irqrestore(&rdev->fence_drv.lock, irq_f
Re: [PATCH 3/3] drm: do not sleep on vblank while holding a mutex
On Fri, Oct 28, 2011 at 08:59:04AM +0200, Michel Dänzer wrote: > On Don, 2011-10-27 at 22:19 -0500, Ilija Hadzic wrote: > > On Thu, 27 Oct 2011, Daniel Vetter wrote: > > > > > So I think the right thing to do is > > > - Kill dev->last_vblank_wait (in a prep patch). > > > > Agreed. Also drm_vblank_info function can go away > > Actually, I was rather going to submit a patch to hook it up again — > AFAICT it was unhooked without any justification. It could be useful for > debugging vblank related hangs. Any issues with it, such as > last_vblank_wait not being guaranteed to really be the last one, can > always be improved later on. I've thought a bit about the usefulness of it for debugging before proposing to kill it and I think it can die: It's only really useful for a complete hangs, if we have an issue we just missing a wakeup somewhere, that's not gonna catch things. Hence I think something that allows you to watch things while it's running is much better, i.e. either a drm debug prinkt or a tracecall. But if you're firmly attached to that debug file it should be pretty easy to shove that under the protection of one of the vblank spinlocks. -Daniel -- Daniel Vetter Mail: dan...@ffwll.ch Mobile: +41 (0)79 365 57 48 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 3/3] drm: do not sleep on vblank while holding a mutex
On Thu, Oct 27, 2011 at 10:19:39PM -0500, Ilija Hadzic wrote: > On Thu, 27 Oct 2011, Daniel Vetter wrote: > >The only really hairy thing going on is racing modeset ioctls against > >vblank waiters. But modeset is protected by the dev->mode_config.mutex > >and hence already races against wait_vblank with the current code, so I'm > >slightly inclined to ignore this for the moment. Would be great if you > >coudl check that in-depth, too. > > > > Will leave this for some other patch at some other time; the > critical path is to fix the hang/crawl that I explained in my > previous note. Agreed, one thing at a time. It's complicated enough as is. > >So I think the right thing to do is > >- Kill dev->last_vblank_wait (in a prep patch). > > Agreed. Also drm_vblank_info function can go away > > >- Imo we also have a few too many atomic ops going on, e.g. > > dev->vblank_refcount looks like it's atomic only because or the procfs > > file, so maybe kill that procfs file entirely. > > I am not sure about that. dev->vblank_refcount looks critical to me: > it is incremented through drm_vblank_get which is called from > several different contexts: page-flip functions in several drivers > (which can run in the context of multiple user processes), **_pm_** > stuff in radeon driver (which looks like it runs in the context of > kernel worker thread). Mutexes used at all these different places > are not quite consistent. If anything, as the result of a later > audit, some other mutexes may be rendered unecessary, but > definitely, I would keep vblank_refcount atomic. I've re-read the code a bit and in _get it's protected by dev->vbl_lock, but not so in _put (because we issue a timer call to actually switch it off). I think we could just shove it under the protection of dev->vbl_lock completely. Another fuzzzy area is the relation between dev->vbl_lock and dev->vblank_time_lock. The latter looks a bit rendundant. > >- Audit the vblank locking, maybe resulting in a patch with updated > > comments, if there's anything misleading or tricky going on. And it's > > always good when the locking of structe members is explained in the > > header file ... > > I'll add comments that I feel make sense for this set of patches (if > anything). Full audit, should come later at a slower pace. There are > quite a few mutexes and locks many of which are overlapping in > function and some are spurious. It will take more than just myself > to untangle this know. Yeah, agreed. Let's first drop the mutex around this and untangle the spinlock/atomic mess in a second step. This is too hairy. > >- Drop the mutex locking because it's not needed. > > > > Agreed. Will drop. > > >While locking at the code I also noticed that wait_vblank calls > >drm_vblank_get, but not always drm_vblank_put. The code is correct, the > >missing vblank_put is hidden in drm_queue_vblank_event. Can you also write > >the patch to move that around to not trip up reviewers the next time > >around? > > > > Actually, there is something fishy here. Look at the 'else' branch > under the 'if ((seq - vblwait->request.sequence) <= (1 << 23))' at > the end of the drm_queue_vblank_event. It doesn't have the > 'drm_vblank_put'. It looks wrong to me, but I need to first convince > myself that there is not some other obscure place where > drm_vblank_put is accounted for. If that branch is really missing a > drm_vblank_put, then it will be easy pull the drm_vblank_put out. > Otherwise, it will be another knot to untangle. I've re-read the code and agree with your follow-up analysis. -- Daniel Vetter Mail: dan...@ffwll.ch Mobile: +41 (0)79 365 57 48 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 3/3] drm: do not sleep on vblank while holding a mutex
On Fri, 28 Oct 2011, Daniel Vetter wrote: On Fri, Oct 28, 2011 at 08:59:04AM +0200, Michel Dänzer wrote: On Don, 2011-10-27 at 22:19 -0500, Ilija Hadzic wrote: On Thu, 27 Oct 2011, Daniel Vetter wrote: So I think the right thing to do is - Kill dev->last_vblank_wait (in a prep patch). Agreed. Also drm_vblank_info function can go away Actually, I was rather going to submit a patch to hook it up again — AFAICT it was unhooked without any justification. It could be useful for debugging vblank related hangs. Any issues with it, such as last_vblank_wait not being guaranteed to really be the last one, can always be improved later on. I've thought a bit about the usefulness of it for debugging before proposing to kill it and I think it can die: It's only really useful for a complete hangs, if we have an issue we just missing a wakeup somewhere, that's not gonna catch things. Hence I think something that allows you to watch things while it's running is much better, i.e. either a drm debug prinkt or a tracecall. But if you're firmly attached to that debug file it should be pretty easy to shove that under the protection of one of the vblank spinlocks. I'll keep it then and figure out the best mutex/spinlock to use. It can be anything that exists on one-per-CRTC basis (vblank waits on different CTCs are not contending). The critical section is from that switch in which vblwait->request.sequence is incremented until it is assigned to dev->last_vblank_wait[crtc] (and we are only protecting that section against itself, executed in contexts of different PIDs). I guess we settled now on this patch (other comments will be addressed in a different set of patches). -- Ilija___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 39202] FPS - KDE desktop effects with 3.0 rc6 kernel
https://bugs.freedesktop.org/show_bug.cgi?id=39202 --- Comment #49 from Siganderson 2011-10-28 05:39:21 PDT --- (In reply to comment #48) > > first) and apply the patch as 'patch -p1 name_of_the_patch.patch' > > Typo, it should be > > > patch -p1 < name_of_the_patch.patch > ^ > > but you probably already know that I tried the patch with a 3.0.4 kernel (latest stable). It works and gives something like 2 or 4 fps gain (before the patch the fps dropped to 24-26 while repeatedly minimizing a window, now it drops to 26-28). Do you need even the test with the logs? -- 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 39202] FPS - KDE desktop effects with 3.0 rc6 kernel
https://bugs.freedesktop.org/show_bug.cgi?id=39202 --- Comment #50 from Siganderson 2011-10-28 05:42:37 PDT --- I forgot to say that without touching anything in the window there are no changes (60 fps... obviously 60 is the maximum when vsync is on). -- 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: [Known BUG?] i915 hang on 3.0.0-12 (Ubuntu 11.10 release)
On Fri, 28 Oct 2011 20:22:35 +0800, Yong Zhang wrote: > Hi, > > Just got below error on Ubuntu-11.10 (kernel: 3.0.0-12-generic), > and after that my screen can't show normally. > No sure if it's a known issue. No, that is the first time I've seen that. It looks as if the fence was not released, or reacquired, before the the batch was executed. (There is a later batch that also uses this buffer.) The fence is programmed with: fence[15] = 04e1 valid, x-tiled, pitch: 512, start: 0x04e0, size: 1048576 And the BLT command uses: 0x0df006b0: 0x5434: XY_COLOR_BLT (rgb enabled, alpha enabled, dst tile 0) 0x0df006b4: 0x03f00100:format , pitch 256, clipping disabled 0x0df006b8: 0x:(0,0) 0x0df006bc: 0x00140037:(55,20) 0x0df006c0: 0x04e0:offset 0x04e0 0x0df006c4: 0x:color 0x0df006c8: 0x54f6: XY_SRC_COPY_BLT (rgb enabled, alpha enabled, src tile 0, dst tile 0) 0x0df006cc: 0x03cc0100:format , dst pitch 256, clipping disabled 0x0df006d0: 0x:dst (0,0) 0x0df006d4: 0x00140037:dst (55,20) 0x0df006d8: 0x04e0:dst offset 0x04e0 0x0df006dc: 0x012c0003:src (3,300) 0x0df006e0: 0x0400:src pitch 1024 0x0df006e4: 0x0850:src offset 0x0850 So we try to perform an undefined operation and the GPU hangs. I suspect this will be timing dependent, but if you can find a way to reproduce it, that would be very useful. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 39202] FPS - KDE desktop effects with 3.0 rc6 kernel
https://bugs.freedesktop.org/show_bug.cgi?id=39202 --- Comment #51 from Ilija Hadzic 2011-10-28 06:25:40 PDT --- (In reply to comment #49) > (In reply to comment #48) > > > first) and apply the patch as 'patch -p1 name_of_the_patch.patch' > > > > Typo, it should be > > > > > > patch -p1 < name_of_the_patch.patch > > ^ > > > > but you probably already know that > > I tried the patch with a 3.0.4 kernel (latest stable). > It works and gives something like 2 or 4 fps gain (before the patch the fps > dropped to 24-26 while repeatedly minimizing a window, now it drops to 26-28). > Do you need even the test with the logs? I guess then your problem is not the same as the bug I am trying to address with this patch. A few fps gain is probably just an (unrelated) incindental finding. At this time, I don't have new theories other than what I postulated back in August. Sorry. -- 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 42117] R200 driver performance, UMS, all mesa versions from 7.6
https://bugs.freedesktop.org/show_bug.cgi?id=42117 --- Comment #13 from Roland Scheidegger 2011-10-28 06:49:59 PDT --- (In reply to comment #12) > csm->gart_limit and csm->vram_limit are correct. > > With GARTSize "64", openarena works great. ETRacer does not (but no fallbacks) > In ETRacer, when I disabled show fps in options, after few seconds on the map, > framerate back to normal. I'd have thought 128MB VRAM should be enough for openarena so it wouldn't need gart for textures, especially with texture compression. So I don't know why that's really needed looks like something is wrong. You could also try FBTexPercent option though given that I think there should be plenty of vram I don't think it's going to make a difference neither. -- 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 2/3] drm/radeon: no need to check all relocs for duplicates
2011/10/28 Christian König :
> Only check the previously checked relocs for
> duplicates. Also leaving the handle uninitialized
> isn't such a good idea.
>
> Signed-off-by: Christian König
Reviewed-by: Alex Deucher
> ---
> drivers/gpu/drm/radeon/radeon_cs.c | 5 +++--
> 1 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/radeon_cs.c
> b/drivers/gpu/drm/radeon/radeon_cs.c
> index fae00c0..7b6e98a 100644
> --- a/drivers/gpu/drm/radeon/radeon_cs.c
> +++ b/drivers/gpu/drm/radeon/radeon_cs.c
> @@ -58,7 +58,7 @@ int radeon_cs_parser_relocs(struct radeon_cs_parser *p)
>
> duplicate = false;
> r = (struct drm_radeon_cs_reloc *)&chunk->kdata[i*4];
> - for (j = 0; j < p->nrelocs; j++) {
> + for (j = 0; j < i; j++) {
> if (r->handle == p->relocs[j].handle) {
> p->relocs_ptr[i] = &p->relocs[j];
> duplicate = true;
> @@ -84,7 +84,8 @@ int radeon_cs_parser_relocs(struct radeon_cs_parser *p)
> p->relocs[i].flags = r->flags;
> radeon_bo_list_add_object(&p->relocs[i].lobj,
> &p->validated);
> - }
> + } else
> + p->relocs[i].handle = 0;
> }
> return radeon_bo_list_validate(&p->validated);
> }
> --
> 1.7.5.4
>
> ___
> 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
Re: [PATCH 3/3] drm: do not sleep on vblank while holding a mutex
On Fri, Oct 28, 2011 at 07:10:51AM -0500, Ilija Hadzic wrote: > I'll keep it then and figure out the best mutex/spinlock to use. It > can be anything that exists on one-per-CRTC basis (vblank waits on > different CTCs are not contending). The critical section is from > that switch in which vblwait->request.sequence is incremented until > it is assigned to dev->last_vblank_wait[crtc] (and we are only > protecting that section against itself, executed in contexts of > different PIDs). > > I guess we settled now on this patch (other comments will be > addressed in a different set of patches). If you settle on keeping ->last_vblank_wait I think the easiest is to wrap it with the dev->vbl_lock spinlock everywhere. -Daniel -- Daniel Vetter Mail: dan...@ffwll.ch Mobile: +41 (0)79 365 57 48 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 42117] R200 driver performance, UMS, all mesa versions from 7.6
https://bugs.freedesktop.org/show_bug.cgi?id=42117 --- Comment #14 from Michal 2011-10-28 09:44:56 PDT --- Minimum GARTSize which don't return fallbacks is 16MB. Now, the problem is somewhere at the kernel side. samples| %| -- 1295082 93.6410 vmlinux 30154 2.1803 r200_dri.so 19269 1.3932 etracer 15854 1.1463 radeon samples %symbol name 1261238 97.3867 __copy_from_user_ll 1495 0.1154 delay_tsc -- 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 42117] R200 driver performance, UMS, all mesa versions from 7.6
https://bugs.freedesktop.org/show_bug.cgi?id=42117 --- Comment #15 from Michal 2011-10-28 10:13:15 PDT --- FBTexPercent to 97 bash-4.1$ cat /var/log/Xorg.0.log|grep text [ 8077.238] (II) RADEON(0): Will use 114688 kb for textures at offset 0x00c08000 etracer runs smoothly at 40 fps(with mesa 7.5.2 60fps). -- 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] drm/radeon/kms: properly set panel mode for eDP
Looks like this patch got missed. Dave can you add it and CC: stable?
Alex
On Wed, May 25, 2011 at 2:02 PM, Alex Deucher wrote:
> This should make eDP more reliable.
>
> Signed-off-by: Alex Deucher
> ---
> drivers/gpu/drm/radeon/atombios_dp.c | 11 +++
> include/drm/drm_dp_helper.h | 3 +++
> 2 files changed, 14 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/atombios_dp.c
> b/drivers/gpu/drm/radeon/atombios_dp.c
> index 8c0f9e3..892b88d 100644
> --- a/drivers/gpu/drm/radeon/atombios_dp.c
> +++ b/drivers/gpu/drm/radeon/atombios_dp.c
> @@ -543,6 +543,7 @@ static void radeon_dp_set_panel_mode(struct drm_encoder
> *encoder,
> {
> struct drm_device *dev = encoder->dev;
> struct radeon_device *rdev = dev->dev_private;
> + struct radeon_connector *radeon_connector =
> to_radeon_connector(connector);
> int panel_mode = DP_PANEL_MODE_EXTERNAL_DP_MODE;
>
> if (!ASIC_IS_DCE4(rdev))
> @@ -550,10 +551,20 @@ static void radeon_dp_set_panel_mode(struct drm_encoder
> *encoder,
>
> if (radeon_connector_encoder_is_dp_bridge(connector))
> panel_mode = DP_PANEL_MODE_INTERNAL_DP1_MODE;
> + else if (connector->connector_type == DRM_MODE_CONNECTOR_eDP) {
> + u8 tmp = radeon_read_dpcd_reg(radeon_connector,
> DP_EDP_CONFIGURATION_CAP);
> + if (tmp & 1)
> + panel_mode = DP_PANEL_MODE_INTERNAL_DP2_MODE;
> + }
>
> atombios_dig_encoder_setup(encoder,
> ATOM_ENCODER_CMD_SETUP_PANEL_MODE,
> panel_mode);
> +
> + if ((connector->connector_type == DRM_MODE_CONNECTOR_eDP) &&
> + (panel_mode == DP_PANEL_MODE_INTERNAL_DP2_MODE)) {
> + radeon_write_dpcd_reg(radeon_connector,
> DP_EDP_CONFIGURATION_SET, 1);
> + }
> }
>
> void radeon_dp_set_link_config(struct drm_connector *connector,
> diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
> index 91567bb..03eb1d6 100644
> --- a/include/drm/drm_dp_helper.h
> +++ b/include/drm/drm_dp_helper.h
> @@ -72,6 +72,7 @@
>
> #define DP_MAIN_LINK_CHANNEL_CODING 0x006
>
> +#define DP_EDP_CONFIGURATION_CAP 0x00d
> #define DP_TRAINING_AUX_RD_INTERVAL 0x00e
>
> /* link configuration */
> @@ -133,6 +134,8 @@
> #define DP_MAIN_LINK_CHANNEL_CODING_SET 0x108
> # define DP_SET_ANSI_8B10B (1 << 0)
>
> +#define DP_EDP_CONFIGURATION_SET 0x10a
> +
> #define DP_LANE0_1_STATUS 0x202
> #define DP_LANE2_3_STATUS 0x203
> # define DP_LANE_CR_DONE (1 << 0)
> --
> 1.7.4
>
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] agp: iommu_gfx_mapped only available if CONFIG_INTEL_IOMMU is set
Kernels with no iommu support cannot ever need the Ironlake
work-around, so never enable it in that case.
Might be better to completely remove the work-around from the kernel
in this case?
Signed-off-by: Keith Packard
Cc: Ben Widawsky
---
drivers/char/agp/intel-gtt.c |4
1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/drivers/char/agp/intel-gtt.c b/drivers/char/agp/intel-gtt.c
index 3a8d448..222190f 100644
--- a/drivers/char/agp/intel-gtt.c
+++ b/drivers/char/agp/intel-gtt.c
@@ -1183,6 +1183,7 @@ static void gen6_cleanup(void)
{
}
+#ifdef CONFIG_INTEL_IOMMU
/* Certain Gen5 chipsets require require idling the GPU before
* unmapping anything from the GTT when VT-d is enabled.
*/
@@ -1201,6 +1202,7 @@ static inline int needs_idle_maps(void)
return 0;
}
+#endif
static int i9xx_setup(void)
{
@@ -1236,8 +1238,10 @@ static int i9xx_setup(void)
intel_private.gtt_bus_addr = reg_addr + gtt_offset;
}
+#ifdef CONFIG_INTEL_IOMMU
if (needs_idle_maps())
intel_private.base.do_idle_maps = 1;
+#endif
intel_i9xx_setup_flush();
--
1.7.7
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] agp: iommu_gfx_mapped only available if CONFIG_INTEL_IOMMU is set
Kernels with no iommu support cannot ever need the Ironlake
work-around, so never enable it in that case.
Might be better to completely remove the work-around from the kernel
in this case?
Signed-off-by: Keith Packard
Cc: Ben Widawsky
---
Here's a shorter patch which just elides the body of the needs_idle_maps
functions
drivers/char/agp/intel-gtt.c |5 +++--
1 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/char/agp/intel-gtt.c b/drivers/char/agp/intel-gtt.c
index 3a8d448..c92424c 100644
--- a/drivers/char/agp/intel-gtt.c
+++ b/drivers/char/agp/intel-gtt.c
@@ -1186,10 +1186,11 @@ static void gen6_cleanup(void)
/* Certain Gen5 chipsets require require idling the GPU before
* unmapping anything from the GTT when VT-d is enabled.
*/
-extern int intel_iommu_gfx_mapped;
static inline int needs_idle_maps(void)
{
+#ifdef CONFIG_INTEL_IOMMU
const unsigned short gpu_devid = intel_private.pcidev->device;
+ extern int intel_iommu_gfx_mapped;
/* Query intel_iommu to see if we need the workaround. Presumably that
* was loaded first.
@@ -1198,7 +1199,7 @@ static inline int needs_idle_maps(void)
gpu_devid == PCI_DEVICE_ID_INTEL_IRONLAKE_M_IG) &&
intel_iommu_gfx_mapped)
return 1;
-
+#endif
return 0;
}
--
1.7.7
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 3/3] drm: do not sleep on vblank while holding a mutex
Message: 5 Date: Fri, 28 Oct 2011 11:30:38 +0200 From: Daniel Vetter Subject: Re: [PATCH 3/3] drm: do not sleep on vblank while holding a mutex To: Ilija Hadzic Cc: dri-devel@lists.freedesktop.org Message-ID: <20111028093038.GB2919@phenom.ffwll.local> Content-Type: text/plain; charset=us-ascii - Imo we also have a few too many atomic ops going on, e.g. dev->vblank_refcount looks like it's atomic only because or the procfs file, so maybe kill that procfs file entirely. I am not sure about that. dev->vblank_refcount looks critical to me: it is incremented through drm_vblank_get which is called from several different contexts: page-flip functions in several drivers (which can run in the context of multiple user processes), **_pm_** stuff in radeon driver (which looks like it runs in the context of kernel worker thread). Mutexes used at all these different places are not quite consistent. If anything, as the result of a later audit, some other mutexes may be rendered unecessary, but definitely, I would keep vblank_refcount atomic. Hi, be careful with vblank_refcount. I think it probably should stay atomic. The drm_vblank_put() is often used in interrupt handlers of the kms drivers where you don't want to uneccessary wait on a lock, but be as quick as possible. I've re-read the code a bit and in _get it's protected by dev- >vbl_lock, but not so in _put (because we issue a timer call to actually switch it off). I think we could just shove it under the protection of dev- >vbl_lock completely. Another fuzzzy area is the relation between dev- >vbl_lock and dev->vblank_time_lock. The latter looks a bit rendundant. The vblank_time_lock serves a different purpose from vbl_lock. It is used in the vblank irq handler drm_handle_vblank() and in the vblank irq on/off functions to protect the vblank timestamping and counting against races between the irq handler and the on/off code, and some funny races between the cpu reinitializing vblank counts and timestamps in non-irq path and the gpu firing vblank interrupts and/ or updating its hardware vblank counter at the "wrong" moment. vblank_time_lock basically blocks/delays gpu vblank irq processing during such problematic periods. Timing there is criticial for reliable vblank timestamping, kms page flipping and artifact free dynamic gpu power-management. It should be locked as seldomly and held as short as possible. I initially tried to get away only with vbl_lock, but there were uses of vbl_lock that looked as if using it in the interrupt handler as well could cause long delays in irq processing. thanks, -mario * Mario Kleiner Max Planck Institute for Biological Cybernetics Spemannstr. 38 72076 Tuebingen Germany e-mail: mario.klei...@tuebingen.mpg.de office: +49 (0)7071/601-1623 fax:+49 (0)7071/601-616 www:http://www.kyb.tuebingen.mpg.de/~kleinerm * "For a successful technology, reality must take precedence over public relations, for Nature cannot be fooled." (Richard Feynman) ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [PATCH] agp: iommu_gfx_mapped only available if CONFIG_INTEL_IOMMU is set
On Fri, 28 Oct 2011 10:56:29 -0700 Keith Packard wrote: > Kernels with no iommu support cannot ever need the Ironlake > work-around, so never enable it in that case. > > Might be better to completely remove the work-around from the kernel > in this case? > > Signed-off-by: Keith Packard > Cc: Ben Widawsky > --- wfm Reviewed-by: Ben Widawsky ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 36934] screen corruption after running a game
https://bugs.freedesktop.org/show_bug.cgi?id=36934 --- Comment #17 from aux...@gmail.com 2011-10-28 11:26:57 PDT --- I'm afraid the virtualbox modules have nothing to do with this. A hopefully helpful observation: When I first load the large image in firefox, zooming in and out several times works fine, without any corruption and Xorg uses 87 MB of RSS memory. It is after changing desktop, scrolling in okular etc that it becomes corrupted. When it becomes corrupted, Xorg use drops to 54 MB of RSS memory. Doesn't this lead to the conclusion that there is some error during memory management? If this were a hardware error, wouldn't Xorg memory use remain constant? -- 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 3/3] drm: do not sleep on vblank while holding a mutex
On Fri, Oct 28, 2011 at 08:15:11PM +0200, Mario Kleiner wrote: > be careful with vblank_refcount. I think it probably should stay > atomic. The drm_vblank_put() is often used in interrupt handlers of > the kms drivers where you don't want to uneccessary wait on a lock, > but be as quick as possible. I don't think that's a real issue - if we have lock contention anywhere with the current code, we already have a problem - and it looks like we have ;-) And the spinlocks are all already switching off irqs locally, so we should already strive for the shortest possible lock hold times. [snip] > The vblank_time_lock serves a different purpose from vbl_lock. It is > used in the vblank irq handler drm_handle_vblank() and in the vblank > irq on/off functions to protect the vblank timestamping and counting > against races between the irq handler and the on/off code, and some > funny races between the cpu reinitializing vblank counts and > timestamps in non-irq path and the gpu firing vblank interrupts and/ > or updating its hardware vblank counter at the "wrong" moment. > vblank_time_lock basically blocks/delays gpu vblank irq processing > during such problematic periods. Timing there is criticial for > reliable vblank timestamping, kms page flipping and artifact free > dynamic gpu power-management. It should be locked as seldomly and > held as short as possible. I initially tried to get away only with > vbl_lock, but there were uses of vbl_lock that looked as if using it > in the interrupt handler as well could cause long delays in irq > processing. I've gone through the code, and in almost all case where the vbl_lock is hold for a bit more than just a few load and stores we are immediately taking the vblank_time_lock and hold it almost as long. Which is why I think this is redundant. The only exeption is the irq uninstall code, which isn't really a fast path anyway. Also, all these paths with longer lock-hold times are in the initial vblank_get/final vblank_put. And because we delay the vblank disabling by a full second, we should never hit that in steady state. But I think you're one of the most timing-sensitive users of this vblank stuff, so when I get around to clean this up I'll certainly put you on cc so you can test for any adverse effects. Hopefully ripping out unecessary atomic ops makes stuff faster, not slower. -Daniel -- Daniel Vetter Mail: dan...@ffwll.ch Mobile: +41 (0)79 365 57 48 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 3/3] drm: do not sleep on vblank while holding a mutex
On Oct 28, 2011, at 9:15 PM, Daniel Vetter wrote: On Fri, Oct 28, 2011 at 08:15:11PM +0200, Mario Kleiner wrote: be careful with vblank_refcount. I think it probably should stay atomic. The drm_vblank_put() is often used in interrupt handlers of the kms drivers where you don't want to uneccessary wait on a lock, but be as quick as possible. I don't think that's a real issue - if we have lock contention anywhere with the current code, we already have a problem - and it looks like we have ;-) And the spinlocks are all already switching off irqs locally, so we should already strive for the shortest possible lock hold times. [snip] The vblank_time_lock serves a different purpose from vbl_lock. It is used in the vblank irq handler drm_handle_vblank() and in the vblank irq on/off functions to protect the vblank timestamping and counting against races between the irq handler and the on/off code, and some funny races between the cpu reinitializing vblank counts and timestamps in non-irq path and the gpu firing vblank interrupts and/ or updating its hardware vblank counter at the "wrong" moment. vblank_time_lock basically blocks/delays gpu vblank irq processing during such problematic periods. Timing there is criticial for reliable vblank timestamping, kms page flipping and artifact free dynamic gpu power-management. It should be locked as seldomly and held as short as possible. I initially tried to get away only with vbl_lock, but there were uses of vbl_lock that looked as if using it in the interrupt handler as well could cause long delays in irq processing. I've gone through the code, and in almost all case where the vbl_lock is hold for a bit more than just a few load and stores we are immediately taking the vblank_time_lock and hold it almost as long. Which is why I think this is redundant. The only exeption is the irq uninstall code, which isn't really a fast path anyway. Ok, if that is the case in the current code, fair enough. We would have to stress the importance of *very low* vbl_lock hold times in the documentation of the code if we want to get rid of the vblank_time_lock. The distinctive lock for that purpose is also meant as a marker that you should try twice as hard to be quick while you hold that lock than with other spinlocks. A few dozen usecs extra delay there could make a difference between met and missed pageflip deadlines and well working or not well working dynamic gpu reclocking, e.g., on radeon, where this stuff is triggered by vblank irq's and quite timing sensitive. Also, all these paths with longer lock-hold times are in the initial vblank_get/final vblank_put. And because we delay the vblank disabling by a full second, we should never hit that in steady state. 5 seconds by default, i think. We wanted to lower that to something like < 100 msecs at some point, but there's still some work to do to remove some potential race with the gpu in the on/off path which could cause lost/wrong vblank counts, and the nouveau kms driver doesn't implement hardware vblank counter at all atm, which makes small values on nouveau unsafe atm. With higher on/off frequencies the impact of too long lock hold times would become worse. But I think you're one of the most timing-sensitive users of this vblank stuff, so when I get around to clean this up I'll certainly put you on cc so you can test for any adverse effects. Hopefully ripping out unecessary atomic ops makes stuff faster, not slower. Yes, the current timing/timestamp precision and some of the related dri2 features of the free graphics stack is a very strong selling point to lure quite many timing sensitive users of my userspace toolkit away from other proprietary solutions. Ok, cc'ing me sounds like a plan. Thanks, -mario -Daniel -- Daniel Vetter Mail: dan...@ffwll.ch Mobile: +41 (0)79 365 57 48 * Mario Kleiner Max Planck Institute for Biological Cybernetics Spemannstr. 38 72076 Tuebingen Germany e-mail: mario.klei...@tuebingen.mpg.de office: +49 (0)7071/601-1623 fax:+49 (0)7071/601-616 www:http://www.kyb.tuebingen.mpg.de/~kleinerm * "For a successful technology, reality must take precedence over public relations, for Nature cannot be fooled." (Richard Feynman) ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 41569] [r600 KMS] Asus A53T A4-3400
https://bugs.freedesktop.org/show_bug.cgi?id=41569 Alex Deucher changed: What|Removed |Added CC||f...@tuttu.info --- Comment #4 from Alex Deucher 2011-10-21 07:43:37 PDT --- *** Bug 42004 has been marked as a duplicate of this bug. *** --- Comment #5 from Alex Deucher 2011-10-28 13:16:46 PDT --- Created attachment 52865 --> https://bugs.freedesktop.org/attachment.cgi?id=52865 possible fix patch 1/2 -- 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 41569] [r600 KMS] Asus A53T A4-3400
https://bugs.freedesktop.org/show_bug.cgi?id=41569 --- Comment #6 from Alex Deucher 2011-10-28 13:17:15 PDT --- Created attachment 52866 --> https://bugs.freedesktop.org/attachment.cgi?id=52866 possible fix patch 2/2. Do these patches help? -- 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
[PATCH v2] drm: do not sleep on vblank while holding a mutex
drm_wait_vblank must be DRM_UNLOCKED because otherwise it
will grab the drm_global_mutex and then go to sleep until the vblank
event it is waiting for. That can wreck havoc in the windowing system
because if one process issues this ioctl, it will block all other
processes for the duration of all vblanks between the current and the
one it is waiting for. In some cases it can block the entire windowing
system. So, make this ioctl DRM_UNLOCKED, wrap the remaining
(very short) critical section with dev->vbl_lock spinlock, and
add a comment to the code explaining what we are protecting with
the lock.
v2: incorporate comments received from Daniel Vetter and
Michel Daenzer.
Signed-off-by: Ilija Hadzic
---
drivers/gpu/drm/drm_drv.c |2 +-
drivers/gpu/drm/drm_irq.c | 14 +-
2 files changed, 14 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index e159f17..c990dab 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -124,7 +124,7 @@ static struct drm_ioctl_desc drm_ioctls[] = {
DRM_IOCTL_DEF(DRM_IOCTL_SG_ALLOC, drm_sg_alloc_ioctl,
DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),
DRM_IOCTL_DEF(DRM_IOCTL_SG_FREE, drm_sg_free,
DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),
- DRM_IOCTL_DEF(DRM_IOCTL_WAIT_VBLANK, drm_wait_vblank, 0),
+ DRM_IOCTL_DEF(DRM_IOCTL_WAIT_VBLANK, drm_wait_vblank, DRM_UNLOCKED),
DRM_IOCTL_DEF(DRM_IOCTL_MODESET_CTL, drm_modeset_ctl, 0),
diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
index 3830e9e..c8b4da8 100644
--- a/drivers/gpu/drm/drm_irq.c
+++ b/drivers/gpu/drm/drm_irq.c
@@ -1160,6 +1160,7 @@ int drm_wait_vblank(struct drm_device *dev, void *data,
union drm_wait_vblank *vblwait = data;
int ret = 0;
unsigned int flags, seq, crtc, high_crtc;
+ unsigned long irqflags;
if ((!drm_dev_to_irq(dev)) || (!dev->irq_enabled))
return -EINVAL;
@@ -1193,6 +1194,13 @@ int drm_wait_vblank(struct drm_device *dev, void *data,
}
seq = drm_vblank_count(dev, crtc);
+ /* the lock protects this section from itself executed in */
+ /* the context of another PID, ensuring that the process that */
+ /* wrote dev->last_vblank_wait is really the last process */
+ /* that was here; processes waiting on different CRTCs */
+ /* do not need to be interlocked, but rather than introducing */
+ /* a new per-crtc lock, we reuse vbl_lock for simplicity */
+ spin_lock_irqsave(&dev->vbl_lock, irqflags);
switch (vblwait->request.type & _DRM_VBLANK_TYPES_MASK) {
case _DRM_VBLANK_RELATIVE:
vblwait->request.sequence += seq;
@@ -1200,12 +1208,15 @@ int drm_wait_vblank(struct drm_device *dev, void *data,
case _DRM_VBLANK_ABSOLUTE:
break;
default:
+ spin_unlock_irqrestore(&dev->vbl_lock, irqflags);
ret = -EINVAL;
goto done;
}
- if (flags & _DRM_VBLANK_EVENT)
+ if (flags & _DRM_VBLANK_EVENT) {
+ spin_unlock_irqrestore(&dev->vbl_lock, irqflags);
return drm_queue_vblank_event(dev, crtc, vblwait, file_priv);
+ }
if ((flags & _DRM_VBLANK_NEXTONMISS) &&
(seq - vblwait->request.sequence) <= (1<<23)) {
@@ -1215,6 +1226,7 @@ int drm_wait_vblank(struct drm_device *dev, void *data,
DRM_DEBUG("waiting on vblank count %d, crtc %d\n",
vblwait->request.sequence, crtc);
dev->last_vblank_wait[crtc] = vblwait->request.sequence;
+ spin_unlock_irqrestore(&dev->vbl_lock, irqflags);
DRM_WAIT_ON(ret, dev->vbl_queue[crtc], 3 * DRM_HZ,
(((drm_vblank_count(dev, crtc) -
vblwait->request.sequence) <= (1 << 23)) ||
--
1.7.7
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2] drm: make DRM_UNLOCKED ioctls with their own mutex
drm_getclient, drm_getstats and drm_getmap (with a few minor
adjustments) do not need global mutex, so fix that and
make the said ioctls DRM_UNLOCKED. Details:
drm_getclient: the only thing that should be protected here
is dev->filelist and that is already protected everywhere with
dev->struct_mutex.
drm_getstats: there is no need for any mutex here because the
loop runs through quasi-static (set at load time only)
data, and the actual count access is done with atomic_read()
drm_getmap already uses dev->struct_mutex to protect
dev->maplist, which also used to protect the same structure
everywhere else except at three places:
* drm_getsarea, which doesn't grab *any* mutex before
touching dev->maplist (so no drm_global_mutex doesn't help
here either; different issue for a different patch).
However, drivers seem to call it only at
initialization time so it probably doesn't matter
* drm_master_destroy, which is called from drm_master_put,
which in turn is protected with dev->struct_mutex
everywhere else in drm module, so we are good here too.
* drm_getsareactx, which releases the dev->struct_mutex
too early, but this patch includes the fix for that.
v2: * incorporate comments received from Daniel Vetter
* include the (long) explanation above to make it clear what
we are doing (and why), also at Daniel Vetter's request
* tighten up mutex grab/release locations to only
encompass real critical sections, rather than some
random code around them
Signed-off-by: Ilija Hadzic
---
drivers/gpu/drm/drm_context.c |5 +++--
drivers/gpu/drm/drm_drv.c |6 +++---
drivers/gpu/drm/drm_ioctl.c | 15 ---
3 files changed, 10 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/drm_context.c b/drivers/gpu/drm/drm_context.c
index 6d440fb..325365f 100644
--- a/drivers/gpu/drm/drm_context.c
+++ b/drivers/gpu/drm/drm_context.c
@@ -154,8 +154,6 @@ int drm_getsareactx(struct drm_device *dev, void *data,
return -EINVAL;
}
- mutex_unlock(&dev->struct_mutex);
-
request->handle = NULL;
list_for_each_entry(_entry, &dev->maplist, head) {
if (_entry->map == map) {
@@ -164,6 +162,9 @@ int drm_getsareactx(struct drm_device *dev, void *data,
break;
}
}
+
+ mutex_unlock(&dev->struct_mutex);
+
if (request->handle == NULL)
return -EINVAL;
diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index c990dab..dc0eb0b 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -64,9 +64,9 @@ static struct drm_ioctl_desc drm_ioctls[] = {
DRM_IOCTL_DEF(DRM_IOCTL_GET_UNIQUE, drm_getunique, 0),
DRM_IOCTL_DEF(DRM_IOCTL_GET_MAGIC, drm_getmagic, 0),
DRM_IOCTL_DEF(DRM_IOCTL_IRQ_BUSID, drm_irq_by_busid,
DRM_MASTER|DRM_ROOT_ONLY),
- DRM_IOCTL_DEF(DRM_IOCTL_GET_MAP, drm_getmap, 0),
- DRM_IOCTL_DEF(DRM_IOCTL_GET_CLIENT, drm_getclient, 0),
- DRM_IOCTL_DEF(DRM_IOCTL_GET_STATS, drm_getstats, 0),
+ DRM_IOCTL_DEF(DRM_IOCTL_GET_MAP, drm_getmap, DRM_UNLOCKED),
+ DRM_IOCTL_DEF(DRM_IOCTL_GET_CLIENT, drm_getclient, DRM_UNLOCKED),
+ DRM_IOCTL_DEF(DRM_IOCTL_GET_STATS, drm_getstats, DRM_UNLOCKED),
DRM_IOCTL_DEF(DRM_IOCTL_GET_CAP, drm_getcap, DRM_UNLOCKED),
DRM_IOCTL_DEF(DRM_IOCTL_SET_VERSION, drm_setversion, DRM_MASTER),
diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index 904d7e9..956fd38 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -158,14 +158,11 @@ int drm_getmap(struct drm_device *dev, void *data,
int i;
idx = map->offset;
-
- mutex_lock(&dev->struct_mutex);
- if (idx < 0) {
- mutex_unlock(&dev->struct_mutex);
+ if (idx < 0)
return -EINVAL;
- }
i = 0;
+ mutex_lock(&dev->struct_mutex);
list_for_each(list, &dev->maplist) {
if (i == idx) {
r_list = list_entry(list, struct drm_map_list, head);
@@ -211,9 +208,9 @@ int drm_getclient(struct drm_device *dev, void *data,
int i;
idx = client->idx;
- mutex_lock(&dev->struct_mutex);
-
i = 0;
+
+ mutex_lock(&dev->struct_mutex);
list_for_each_entry(pt, &dev->filelist, lhead) {
if (i++ >= idx) {
client->auth = pt->authenticated;
@@ -249,8 +246,6 @@ int drm_getstats(struct drm_device *dev, void *data,
memset(stats, 0, sizeof(*stats));
- mutex_lock(&dev->struct_mutex);
-
for (i = 0; i < dev->counters; i++) {
if (dev->types[i] == _DRM_STAT_LOCK)
stats->data[i].value =
@@ -262,8 +257,6 @@ int drm_getstats(struct drm_device *dev, void *data,
stats->count = dev->counters;
- mutex_unlock(&dev->struct_mutex);
-
return
[PATCH] drm: add some comments to drm_wait_vblank and drm_queue_vblank_event
during the review of the fix for locks problems in drm_wait_vblank,
a couple of false concerns were raised about how the drm_vblank_get
and drm_vblank_put are used in this function; it turned out that the
code is correct and that it cannot be simplified
add a few comments to explain non-obvious flows in the code,
to prevent "false alarms" in the future
Signed-off-by: Ilija Hadzic
---
drivers/gpu/drm/drm_irq.c |8
1 files changed, 8 insertions(+), 0 deletions(-)
diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
index c8b4da8..e9dd19d 100644
--- a/drivers/gpu/drm/drm_irq.c
+++ b/drivers/gpu/drm/drm_irq.c
@@ -1065,6 +1065,10 @@ out:
return ret;
}
+/* must acquire vblank reference count (call drm_vblank_get) */
+/* before calling this function; the matching drm_vblank_put */
+/* will either be issued here or in drm_handle_vblank_events */
+/* after the vblank is signaled */
static int drm_queue_vblank_event(struct drm_device *dev, int pipe,
union drm_wait_vblank *vblwait,
struct drm_file *file_priv)
@@ -1124,6 +1128,9 @@ static int drm_queue_vblank_event(struct drm_device *dev,
int pipe,
trace_drm_vblank_event_delivered(current->pid, pipe,
vblwait->request.sequence);
} else {
+ /* can't call drm_vblank_put here because interrupt */
+ /* must remain enabled until the event occurs */
+ /* drm_handle_vblank_events will do this for us */
list_add_tail(&e->base.link, &dev->vblank_event_list);
vblwait->reply.sequence = vblwait->request.sequence;
}
@@ -1215,6 +1222,7 @@ int drm_wait_vblank(struct drm_device *dev, void *data,
if (flags & _DRM_VBLANK_EVENT) {
spin_unlock_irqrestore(&dev->vbl_lock, irqflags);
+ /* drm_queue_vblank_event() will call drm_vblank_put() */
return drm_queue_vblank_event(dev, crtc, vblwait, file_priv);
}
--
1.7.7
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/radeon: set hpd polarity at init time so hotplug detect works
From: Jerome Glisse Polarity needs to be set accordingly to connector status (connected or disconnected). Set it up at module init so first hotplug works reliably no matter what is the initial set of connector. Signed-off-by: Jerome Glisse cc: sta...@kernel.org --- drivers/gpu/drm/radeon/radeon_connectors.c |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/drivers/gpu/drm/radeon/radeon_connectors.c b/drivers/gpu/drm/radeon/radeon_connectors.c index dec6cbe..bfdd48b 100644 --- a/drivers/gpu/drm/radeon/radeon_connectors.c +++ b/drivers/gpu/drm/radeon/radeon_connectors.c @@ -1789,6 +1789,7 @@ radeon_add_atom_connector(struct drm_device *dev, connector->polled = DRM_CONNECTOR_POLL_CONNECT; } else connector->polled = DRM_CONNECTOR_POLL_HPD; + radeon_hpd_set_polarity(rdev, radeon_connector->hpd.hpd); connector->display_info.subpixel_order = subpixel_order; drm_sysfs_connector_add(connector); -- 1.7.6.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/radeon: set hpd polarity at init time so hotplug detect works
On Fri, Oct 28, 2011 at 5:52 PM, wrote: > From: Jerome Glisse > > Polarity needs to be set accordingly to connector status (connected > or disconnected). Set it up at module init so first hotplug works > reliably no matter what is the initial set of connector. > > Signed-off-by: Jerome Glisse > cc: sta...@kernel.org Reviewed-by: Alex Deucher > --- > drivers/gpu/drm/radeon/radeon_connectors.c | 1 + > 1 files changed, 1 insertions(+), 0 deletions(-) > > diff --git a/drivers/gpu/drm/radeon/radeon_connectors.c > b/drivers/gpu/drm/radeon/radeon_connectors.c > index dec6cbe..bfdd48b 100644 > --- a/drivers/gpu/drm/radeon/radeon_connectors.c > +++ b/drivers/gpu/drm/radeon/radeon_connectors.c > @@ -1789,6 +1789,7 @@ radeon_add_atom_connector(struct drm_device *dev, > connector->polled = DRM_CONNECTOR_POLL_CONNECT; > } else > connector->polled = DRM_CONNECTOR_POLL_HPD; > + radeon_hpd_set_polarity(rdev, radeon_connector->hpd.hpd); > > connector->display_info.subpixel_order = subpixel_order; > drm_sysfs_connector_add(connector); > -- > 1.7.6.4 > > ___ > 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
Re: [PATCH 09/23] drm/sis: use drm_mm instead of drm_sman
On Thu, Oct 27, 2011 at 1:07 PM, Daniel Vetter wrote: ... > @@ -139,13 +99,35 @@ static int sis_drm_alloc(struct drm_device *dev, struct > drm_file *file, ... > +#if defined(CONFIG_FB_SIS) || defined(CONFIG_FB_SIS_MODULE) > + item->req.size = mem->size; > + sis_malloc(&item->req); > + if (item->req.size == 0) > + retval = -ENOMEN; ENOMEN is omen, I guess this was not compile tested with either CONFIG? Cheers, Tormod ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/sis: Fix building with CONFIG_FB_SIS/CONFIG_FB_SIS_MODULE
From: Tormod Volden --- On top of danvet's kill-with-fire ad83cc3. Tormod drivers/gpu/drm/sis/sis_mm.c |4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/sis/sis_mm.c b/drivers/gpu/drm/sis/sis_mm.c index 112a43b..63c2f75 100644 --- a/drivers/gpu/drm/sis/sis_mm.c +++ b/drivers/gpu/drm/sis/sis_mm.c @@ -116,7 +116,7 @@ static int sis_drm_alloc(struct drm_device *dev, struct drm_file *file, item->req.size = mem->size; sis_malloc(&item->req); if (item->req.size == 0) - retval = -ENOMEN; + retval = -ENOMEM; offset = item->req.offset; #else retval = drm_mm_insert_node(&dev_priv->vram_mm, @@ -343,7 +343,7 @@ void sis_reclaim_buffers_locked(struct drm_device *dev, drm_mm_remove_node(&entry->mm_node); #if defined(CONFIG_FB_SIS) || defined(CONFIG_FB_SIS_MODULE) else - sis_free(entry->req->offset); + sis_free(entry->req.offset); #endif kfree(entry); } -- 1.7.5.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 09/23] drm/sis: use drm_mm instead of drm_sman
On Sat, Oct 29, 2011 at 12:47:18AM +0200, Tormod Volden wrote: > On Thu, Oct 27, 2011 at 1:07 PM, Daniel Vetter wrote: > ... > > @@ -139,13 +99,35 @@ static int sis_drm_alloc(struct drm_device *dev, > > struct drm_file *file, > ... > > +#if defined(CONFIG_FB_SIS) || defined(CONFIG_FB_SIS_MODULE) > > + item->req.size = mem->size; > > + sis_malloc(&item->req); > > + if (item->req.size == 0) > > + retval = -ENOMEN; > > ENOMEN is omen, I guess this was not compile tested with either CONFIG? Oh gosh, this is embarassing ;-) I've indeed fumbled this and forgotten to check that config option. Smashed your patch on top of mine. btw, do you have some actual hw to test this on, or have you just checked out of curiosity? Thanks, Daniel -- Daniel Vetter Mail: dan...@ffwll.ch Mobile: +41 (0)79 365 57 48 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] drm: do not sleep on vblank while holding a mutex
On Fri, Oct 28, 2011 at 05:42:23PM -0400, Ilija Hadzic wrote:
> drm_wait_vblank must be DRM_UNLOCKED because otherwise it
> will grab the drm_global_mutex and then go to sleep until the vblank
> event it is waiting for. That can wreck havoc in the windowing system
> because if one process issues this ioctl, it will block all other
> processes for the duration of all vblanks between the current and the
> one it is waiting for. In some cases it can block the entire windowing
> system. So, make this ioctl DRM_UNLOCKED, wrap the remaining
> (very short) critical section with dev->vbl_lock spinlock, and
> add a comment to the code explaining what we are protecting with
> the lock.
>
> v2: incorporate comments received from Daniel Vetter and
> Michel Daenzer.
>
> Signed-off-by: Ilija Hadzic
> ---
> drivers/gpu/drm/drm_drv.c |2 +-
> drivers/gpu/drm/drm_irq.c | 14 +-
> 2 files changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> index e159f17..c990dab 100644
> --- a/drivers/gpu/drm/drm_drv.c
> +++ b/drivers/gpu/drm/drm_drv.c
> @@ -124,7 +124,7 @@ static struct drm_ioctl_desc drm_ioctls[] = {
> DRM_IOCTL_DEF(DRM_IOCTL_SG_ALLOC, drm_sg_alloc_ioctl,
> DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),
> DRM_IOCTL_DEF(DRM_IOCTL_SG_FREE, drm_sg_free,
> DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),
>
> - DRM_IOCTL_DEF(DRM_IOCTL_WAIT_VBLANK, drm_wait_vblank, 0),
> + DRM_IOCTL_DEF(DRM_IOCTL_WAIT_VBLANK, drm_wait_vblank, DRM_UNLOCKED),
>
> DRM_IOCTL_DEF(DRM_IOCTL_MODESET_CTL, drm_modeset_ctl, 0),
>
> diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
> index 3830e9e..c8b4da8 100644
> --- a/drivers/gpu/drm/drm_irq.c
> +++ b/drivers/gpu/drm/drm_irq.c
> @@ -1160,6 +1160,7 @@ int drm_wait_vblank(struct drm_device *dev, void *data,
> union drm_wait_vblank *vblwait = data;
> int ret = 0;
> unsigned int flags, seq, crtc, high_crtc;
> + unsigned long irqflags;
>
> if ((!drm_dev_to_irq(dev)) || (!dev->irq_enabled))
> return -EINVAL;
> @@ -1193,6 +1194,13 @@ int drm_wait_vblank(struct drm_device *dev, void *data,
> }
> seq = drm_vblank_count(dev, crtc);
>
> + /* the lock protects this section from itself executed in */
> + /* the context of another PID, ensuring that the process that */
> + /* wrote dev->last_vblank_wait is really the last process */
> + /* that was here; processes waiting on different CRTCs */
> + /* do not need to be interlocked, but rather than introducing */
> + /* a new per-crtc lock, we reuse vbl_lock for simplicity */
Multiline comments are done with one pair of /* */, see CodingStyle
> + spin_lock_irqsave(&dev->vbl_lock, irqflags);
> switch (vblwait->request.type & _DRM_VBLANK_TYPES_MASK) {
> case _DRM_VBLANK_RELATIVE:
> vblwait->request.sequence += seq;
> @@ -1200,12 +1208,15 @@ int drm_wait_vblank(struct drm_device *dev, void
> *data,
> case _DRM_VBLANK_ABSOLUTE:
> break;
> default:
> + spin_unlock_irqrestore(&dev->vbl_lock, irqflags);
> ret = -EINVAL;
> goto done;
> }
>
> - if (flags & _DRM_VBLANK_EVENT)
> + if (flags & _DRM_VBLANK_EVENT) {
> + spin_unlock_irqrestore(&dev->vbl_lock, irqflags);
> return drm_queue_vblank_event(dev, crtc, vblwait, file_priv);
> + }
>
> if ((flags & _DRM_VBLANK_NEXTONMISS) &&
> (seq - vblwait->request.sequence) <= (1<<23)) {
> @@ -1215,6 +1226,7 @@ int drm_wait_vblank(struct drm_device *dev, void *data,
> DRM_DEBUG("waiting on vblank count %d, crtc %d\n",
> vblwait->request.sequence, crtc);
> dev->last_vblank_wait[crtc] = vblwait->request.sequence;
> + spin_unlock_irqrestore(&dev->vbl_lock, irqflags);
Imo the only thing we need to protect here is dev->last_vblank_wait
against concurrent access from drm_vblank_info in drm_info.c
But the locking there is horribly broken: It grabs dev->struct_mutex, but
that protects exactly nothing, afaics:
- dev->vblank_refcount is an atomic
- vblank_count is protected by a the handrolled seqlock in
drm_vblank_count
- dev->last_vblank_wait was protected by the global lock, but is now
protected by the dev->vbl_spinlock on the write side
- dev->vblank_inmodeset is protected by the global lock for ums setups
(locked ioctl) and by dev->mode_config.mutex for kms
And I just don't see anything else you might protect here - vblwait is
just the ioctl data copied in by the drm core ioctl helpers.
So I don't quite see what you protect here with that spinlock. Imo
- either make dev->last_vblank_wait into an atomic_t
- or just don't care about it, locking of vblank_info is already horribly
broken.
In both cases you don't need the dev->vbl_lock spinlock.
I personally vote for no additional locking at all here and just drop the
global lock. Or maybe I'm blind
[Bug 36386] Amnesia game crashes on RV570 (r300g)
https://bugs.freedesktop.org/show_bug.cgi?id=36386 Tom Stellard changed: What|Removed |Added Status|NEW |RESOLVED Resolution||NOTABUG --- Comment #4 from Tom Stellard 2011-10-28 17:59:46 PDT --- This game has been reported as working, and it's not really clear if this was even a driver bug in the first place, so I'm closing this bug. Please re-open it if you are still having problems. -- 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 v2] drm: make DRM_UNLOCKED ioctls with their own mutex
On Fri, Oct 28, 2011 at 05:43:28PM -0400, Ilija Hadzic wrote: > drm_getclient, drm_getstats and drm_getmap (with a few minor > adjustments) do not need global mutex, so fix that and > make the said ioctls DRM_UNLOCKED. Details: > > drm_getclient: the only thing that should be protected here > is dev->filelist and that is already protected everywhere with > dev->struct_mutex. > > drm_getstats: there is no need for any mutex here because the > loop runs through quasi-static (set at load time only) > data, and the actual count access is done with atomic_read() > > drm_getmap already uses dev->struct_mutex to protect > dev->maplist, which also used to protect the same structure > everywhere else except at three places: > * drm_getsarea, which doesn't grab *any* mutex before > touching dev->maplist (so no drm_global_mutex doesn't help > here either; different issue for a different patch). > However, drivers seem to call it only at > initialization time so it probably doesn't matter > * drm_master_destroy, which is called from drm_master_put, > which in turn is protected with dev->struct_mutex > everywhere else in drm module, so we are good here too. > * drm_getsareactx, which releases the dev->struct_mutex > too early, but this patch includes the fix for that. > > v2: * incorporate comments received from Daniel Vetter > * include the (long) explanation above to make it clear what > we are doing (and why), also at Daniel Vetter's request > * tighten up mutex grab/release locations to only > encompass real critical sections, rather than some > random code around them > > Signed-off-by: Ilija Hadzic Quite a load of stuff to check for this, i.e. next time around maybe split it up into one patch for each of the three functions changed (and move the locking bugfix in drm_getsareactx into its own patch). Reviewed-by: Daniel Vetter -- Daniel Vetter Mail: dan...@ffwll.ch Mobile: +41 (0)79 365 57 48 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 36609] 45920d2ecb38b14fdda5253fecce996570c22863 breaks sauerbraten on r300g
https://bugs.freedesktop.org/show_bug.cgi?id=36609 Tom Stellard changed: What|Removed |Added Status|REOPENED|RESOLVED Resolution||FIXED --- Comment #6 from Tom Stellard 2011-10-28 18:23:44 UTC --- The issue with sauerbraten has been fixed. If you are still experiencing problems with ut2004, please file a new bug and attach a screenshot demonstrating the bug. -- 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 29951] [r300g] xscreensaver hack "antspotlight" reveals something other than desktop
https://bugs.freedesktop.org/show_bug.cgi?id=29951 Tom Stellard changed: What|Removed |Added Status|NEW |RESOLVED Resolution||NOTOURBUG --- Comment #19 from Tom Stellard 2011-10-28 18:29:56 PDT --- This appears to be an issue with xscreensaver and not a driver bug. I recommend notifying the xscreensaver developers. -- 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 v2] drm: do not sleep on vblank while holding a mutex
On Sat, 29 Oct 2011, Daniel Vetter wrote: + /* the lock protects this section from itself executed in */ + /* the context of another PID, ensuring that the process that */ + /* wrote dev->last_vblank_wait is really the last process */ + /* that was here; processes waiting on different CRTCs */ + /* do not need to be interlocked, but rather than introducing */ + /* a new per-crtc lock, we reuse vbl_lock for simplicity */ Multiline comments are done with one pair of /* */, see CodingStyle Will fix the multiline comments (though scripts/checkpatch.pl didn't complain so I figured it was OK) I personally vote for no additional locking at all here and just drop the global lock. Or maybe I'm blind and we need this. In that case, please clue me up. What I am doing with the lock, is makeing sure that the last vblank wait reported is really last. You said in your previous comment (which I agree with) that the value of having last_vblank_wait in debugfs is in case of hangs. In that case you want to see who really was the last one to issue the vblank. Now suppose that the drm_wait_vblank is enteret in the context of two different PIDs and suppose that there are no locks. Let's say that the first process wants to wait on vblank N and the second wants to wait on vblank N+1. First process can reach the point just before it wants to write to last_vblank_wait and lose the processor there (so it has N in vblank.request (on its stack) calculated, but it has not written it into last_vblank_wait yet (because it lost the CPU right there). Then the second process gets the CPU, and executes and let's say that it goes all the way through so it writes N+1 into last_vblank_wait and then schedules away (it called DRM_WAIT_ON). Now the first process gets the CPU where it left off and overwrites N+1 in last_vblank_wait with N, which is now stale. I explained in the comment (and in my note this morning), that the only race condition is against itself in the context of different processes. The above is the scenario. Race against drm_vblank_info exists in theory, but in practice doesn't matter because the reader of debugfs (or proc file system) runs asynchronously (and typically at slower pace) from processes issuing vblank waits (if it's a human looking at the filesystem then definitely much slower pace). Since processes issuing vblanks are overwriting the same last_vblank_value, drm_vblank_info is already doing a form of downsampling and debugfs is losing information, anyway, so who cares about the lock. In case of a hang, the value of last_vblank_wait doesn't change, so again the lock in drm_vblank_info doesn't change anything. So adding a lock there (which would have to be vbl_lock to make it usable) is only a matter of theoretical correctness, but no practical value. So I decided not to touch drm_vblank_info. drm_wait_vblank, however, needs a protection from itself as I explained above. DRM_WAIT_ON(ret, dev->vbl_queue[crtc], 3 * DRM_HZ, (((drm_vblank_count(dev, crtc) - vblwait->request.sequence) <= (1 << 23)) || One line below there's the curios case of us rechecking dev->irq_enabled. Without any locking in place. But afaics this is only changed by the driver at setup and teardown when we are guaranteed (hopefully) that there's no open fd and hence no way to call an ioctl. So checking once at the beginning should be good enough. Whatever. It's probably redundant statement because it won't be reached if irq_enabled is false and there is nothing to change it to true at runtime, but that's a topic for a different patch. -- Ilija ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 41569] [r600 KMS] Asus A53T A4-3400
https://bugs.freedesktop.org/show_bug.cgi?id=41569 Robert Nelson changed: What|Removed |Added Status|NEW |RESOLVED Resolution||FIXED --- Comment #7 from Robert Nelson 2011-10-28 22:41:01 PDT --- Awesome! Thanks Alex. KMS is now works perfectly on this laptop.. Using v3.1.0 + airlied's drm-next pull for (3.2) Patch 2 was missing another change you had posted, so it didn't apply: http://people.freedesktop.org/~agd5f/0009-drm-radeon-kms-rework-DP-bridge-checks.patch and that patch missed one of the function conversions, so i'm posting a git-format patch dump of the 4 patches I used here: http://rcn-ee.homeip.net:81/testing/asus-a53t/a4-kms/ Thanks, -- 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 09/23] drm/sis: use drm_mm instead of drm_sman
On Sat, Oct 29, 2011 at 2:25 AM, Daniel Vetter wrote: > On Sat, Oct 29, 2011 at 12:47:18AM +0200, Tormod Volden wrote: >> On Thu, Oct 27, 2011 at 1:07 PM, Daniel Vetter >> wrote: >> ... >> > @@ -139,13 +99,35 @@ static int sis_drm_alloc(struct drm_device *dev, >> > struct drm_file *file, >> ... >> > +#if defined(CONFIG_FB_SIS) || defined(CONFIG_FB_SIS_MODULE) >> > + item->req.size = mem->size; >> > + sis_malloc(&item->req); >> > + if (item->req.size == 0) >> > + retval = -ENOMEN; >> >> ENOMEN is omen, I guess this was not compile tested with either CONFIG? > > Oh gosh, this is embarassing ;-) I've indeed fumbled this and forgotten to > check that config option. Smashed your patch on top of mine. btw, do you > have some actual hw to test this on, or have you just checked out of > curiosity? I do not have sis hardware, but I am testing this on savage and my config happened to have this option set. By the way, is there anything in special I can try to (stress-)test this? I have tested Quake III demos and multiple parallel GL screensaver hacks. On the other hand, is there a possibility this might fix some random DMA crashes or lock-ups (I mean the savage locking fix here)? Thanks, Tormod > Thanks, Daniel > -- > Daniel Vetter > Mail: dan...@ffwll.ch > Mobile: +41 (0)79 365 57 48 > ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 42090] [r300/compiler] [bisected] sauerbraten texture corruption
https://bugs.freedesktop.org/show_bug.cgi?id=42090 Tom Stellard changed: What|Removed |Added Status|NEW |RESOLVED Resolution||FIXED --- Comment #3 from Tom Stellard 2011-10-27 17:59:51 PDT --- Fix in git master commit 17a1c0cb0d9e04607c1726d04ef23485979dfc98 -- Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug.
[PATCH 3/3] drm: do not sleep on vblank while holding a mutex
On Don, 2011-10-27 at 22:19 -0500, Ilija Hadzic wrote: > On Thu, 27 Oct 2011, Daniel Vetter wrote: > > > So I think the right thing to do is > > - Kill dev->last_vblank_wait (in a prep patch). > > Agreed. Also drm_vblank_info function can go away Actually, I was rather going to submit a patch to hook it up again ? AFAICT it was unhooked without any justification. It could be useful for debugging vblank related hangs. Any issues with it, such as last_vblank_wait not being guaranteed to really be the last one, can always be improved later on. -- Earthling Michel D?nzer | http://www.amd.com Libre software enthusiast | Debian, X and DRI developer
[PATCH 1/3] drm/radeon: fix debugfs handling
Having registered debugfs files globally causes
the files to not show up on the second, third
etc.. card in the system.
Signed-off-by: Christian K?nig
Reviewed-by: Alex Deucher
---
drivers/gpu/drm/radeon/radeon.h|8
drivers/gpu/drm/radeon/radeon_device.c | 26 ++
2 files changed, 18 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
index e3170c7..ede1b1b 100644
--- a/drivers/gpu/drm/radeon/radeon.h
+++ b/drivers/gpu/drm/radeon/radeon.h
@@ -879,6 +879,11 @@ void radeon_test_moves(struct radeon_device *rdev);
/*
* Debugfs
*/
+struct radeon_debugfs {
+struct drm_info_list*files;
+unsignednum_files;
+};
+
int radeon_debugfs_add_files(struct radeon_device *rdev,
struct drm_info_list *files,
unsigned nfiles);
@@ -1241,6 +1246,9 @@ struct radeon_device {
struct drm_file *cmask_filp;
/* i2c buses */
struct radeon_i2c_chan *i2c_bus[RADEON_MAX_I2C_BUS];
+ /* debugfs */
+ struct radeon_debugfs debugfs[RADEON_DEBUGFS_MAX_COMPONENTS];
+ unsigneddebugfs_count;
};
int radeon_device_init(struct radeon_device *rdev,
diff --git a/drivers/gpu/drm/radeon/radeon_device.c
b/drivers/gpu/drm/radeon/radeon_device.c
index c33bc91..d50fa60 100644
--- a/drivers/gpu/drm/radeon/radeon_device.c
+++ b/drivers/gpu/drm/radeon/radeon_device.c
@@ -978,36 +978,29 @@ int radeon_gpu_reset(struct radeon_device *rdev)
/*
* Debugfs
*/
-struct radeon_debugfs {
- struct drm_info_list*files;
- unsignednum_files;
-};
-static struct radeon_debugfs _radeon_debugfs[RADEON_DEBUGFS_MAX_COMPONENTS];
-static unsigned _radeon_debugfs_count = 0;
-
int radeon_debugfs_add_files(struct radeon_device *rdev,
struct drm_info_list *files,
unsigned nfiles)
{
unsigned i;
- for (i = 0; i < _radeon_debugfs_count; i++) {
- if (_radeon_debugfs[i].files == files) {
+ for (i = 0; i < rdev->debugfs_count; i++) {
+ if (rdev->debugfs[i].files == files) {
/* Already registered */
return 0;
}
}
- i = _radeon_debugfs_count + 1;
+ i = rdev->debugfs_count + 1;
if (i > RADEON_DEBUGFS_MAX_COMPONENTS) {
DRM_ERROR("Reached maximum number of debugfs components.\n");
DRM_ERROR("Report so we increase "
"RADEON_DEBUGFS_MAX_COMPONENTS.\n");
return -EINVAL;
}
- _radeon_debugfs[_radeon_debugfs_count].files = files;
- _radeon_debugfs[_radeon_debugfs_count].num_files = nfiles;
- _radeon_debugfs_count = i;
+ rdev->debugfs[rdev->debugfs_count].files = files;
+ rdev->debugfs[rdev->debugfs_count].num_files = nfiles;
+ rdev->debugfs_count = i;
#if defined(CONFIG_DEBUG_FS)
drm_debugfs_create_files(files, nfiles,
rdev->ddev->control->debugfs_root,
@@ -1027,11 +1020,12 @@ int radeon_debugfs_init(struct drm_minor *minor)
void radeon_debugfs_cleanup(struct drm_minor *minor)
{
+struct radeon_device *rdev = minor->dev->dev_private;
unsigned i;
- for (i = 0; i < _radeon_debugfs_count; i++) {
- drm_debugfs_remove_files(_radeon_debugfs[i].files,
-_radeon_debugfs[i].num_files, minor);
+ for (i = 0; i < rdev->debugfs_count; i++) {
+ drm_debugfs_remove_files(rdev->debugfs[i].files,
+rdev->debugfs[i].num_files, minor);
}
}
#endif
--
1.7.5.4
[PATCH 2/3] drm/radeon: no need to check all relocs for duplicates
Only check the previously checked relocs for
duplicates. Also leaving the handle uninitialized
isn't such a good idea.
Signed-off-by: Christian K?nig
---
drivers/gpu/drm/radeon/radeon_cs.c |5 +++--
1 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/radeon/radeon_cs.c
b/drivers/gpu/drm/radeon/radeon_cs.c
index fae00c0..7b6e98a 100644
--- a/drivers/gpu/drm/radeon/radeon_cs.c
+++ b/drivers/gpu/drm/radeon/radeon_cs.c
@@ -58,7 +58,7 @@ int radeon_cs_parser_relocs(struct radeon_cs_parser *p)
duplicate = false;
r = (struct drm_radeon_cs_reloc *)&chunk->kdata[i*4];
- for (j = 0; j < p->nrelocs; j++) {
+ for (j = 0; j < i; j++) {
if (r->handle == p->relocs[j].handle) {
p->relocs_ptr[i] = &p->relocs[j];
duplicate = true;
@@ -84,7 +84,8 @@ int radeon_cs_parser_relocs(struct radeon_cs_parser *p)
p->relocs[i].flags = r->flags;
radeon_bo_list_add_object(&p->relocs[i].lobj,
&p->validated);
- }
+ } else
+ p->relocs[i].handle = 0;
}
return radeon_bo_list_validate(&p->validated);
}
--
1.7.5.4
Some minor bugfixes
The following three patches are just minor bug fixes. I've send them to the list previously, but this time they are based upon drm-next instead of drm-fixes and I also fixed some spelling mistakes in the commit messages. Please commit. Thanks, Christian.
[PATCH 3/3] drm/radeon: fix a spelling mistake
Better fix it before this obvious typo spreads even more.
Signed-off-by: Christian K?nig
Reviewed-by: Alex Deucher
---
drivers/gpu/drm/radeon/radeon.h |4 +-
drivers/gpu/drm/radeon/radeon_fence.c | 34
drivers/gpu/drm/radeon/radeon_pm.c|4 +-
drivers/gpu/drm/radeon/radeon_ring.c |2 +-
4 files changed, 22 insertions(+), 22 deletions(-)
diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
index ede1b1b..2fda4ae 100644
--- a/drivers/gpu/drm/radeon/radeon.h
+++ b/drivers/gpu/drm/radeon/radeon.h
@@ -198,7 +198,7 @@ struct radeon_fence_driver {
wait_queue_head_t queue;
rwlock_tlock;
struct list_headcreated;
- struct list_heademited;
+ struct list_heademitted;
struct list_headsignaled;
boolinitialized;
};
@@ -209,7 +209,7 @@ struct radeon_fence {
struct list_headlist;
/* protected by radeon_fence.lock */
uint32_tseq;
- boolemited;
+ boolemitted;
boolsignaled;
};
diff --git a/drivers/gpu/drm/radeon/radeon_fence.c
b/drivers/gpu/drm/radeon/radeon_fence.c
index a488b50..711bd6e 100644
--- a/drivers/gpu/drm/radeon/radeon_fence.c
+++ b/drivers/gpu/drm/radeon/radeon_fence.c
@@ -74,7 +74,7 @@ int radeon_fence_emit(struct radeon_device *rdev, struct
radeon_fence *fence)
unsigned long irq_flags;
write_lock_irqsave(&rdev->fence_drv.lock, irq_flags);
- if (fence->emited) {
+ if (fence->emitted) {
write_unlock_irqrestore(&rdev->fence_drv.lock, irq_flags);
return 0;
}
@@ -88,8 +88,8 @@ int radeon_fence_emit(struct radeon_device *rdev, struct
radeon_fence *fence)
radeon_fence_ring_emit(rdev, fence);
trace_radeon_fence_emit(rdev->ddev, fence->seq);
- fence->emited = true;
- list_move_tail(&fence->list, &rdev->fence_drv.emited);
+ fence->emitted = true;
+ list_move_tail(&fence->list, &rdev->fence_drv.emitted);
write_unlock_irqrestore(&rdev->fence_drv.lock, irq_flags);
return 0;
}
@@ -129,7 +129,7 @@ static bool radeon_fence_poll_locked(struct radeon_device
*rdev)
return false;
}
n = NULL;
- list_for_each(i, &rdev->fence_drv.emited) {
+ list_for_each(i, &rdev->fence_drv.emitted) {
fence = list_entry(i, struct radeon_fence, list);
if (fence->seq == seq) {
n = i;
@@ -145,7 +145,7 @@ static bool radeon_fence_poll_locked(struct radeon_device
*rdev)
fence = list_entry(i, struct radeon_fence, list);
fence->signaled = true;
i = n;
- } while (i != &rdev->fence_drv.emited);
+ } while (i != &rdev->fence_drv.emitted);
wake = true;
}
return wake;
@@ -159,7 +159,7 @@ static void radeon_fence_destroy(struct kref *kref)
fence = container_of(kref, struct radeon_fence, kref);
write_lock_irqsave(&fence->rdev->fence_drv.lock, irq_flags);
list_del(&fence->list);
- fence->emited = false;
+ fence->emitted = false;
write_unlock_irqrestore(&fence->rdev->fence_drv.lock, irq_flags);
kfree(fence);
}
@@ -174,7 +174,7 @@ int radeon_fence_create(struct radeon_device *rdev, struct
radeon_fence **fence)
}
kref_init(&((*fence)->kref));
(*fence)->rdev = rdev;
- (*fence)->emited = false;
+ (*fence)->emitted = false;
(*fence)->signaled = false;
(*fence)->seq = 0;
INIT_LIST_HEAD(&(*fence)->list);
@@ -203,8 +203,8 @@ bool radeon_fence_signaled(struct radeon_fence *fence)
if (fence->rdev->shutdown) {
signaled = true;
}
- if (!fence->emited) {
- WARN(1, "Querying an unemited fence : %p !\n", fence);
+ if (!fence->emitted) {
+ WARN(1, "Querying an unemitted fence : %p !\n", fence);
signaled = true;
}
if (!signaled) {
@@ -295,11 +295,11 @@ int radeon_fence_wait_next(struct radeon_device *rdev)
return 0;
}
write_lock_irqsave(&rdev->fence_drv.lock, irq_flags);
- if (list_empty(&rdev->fence_drv.emited)) {
+ if (list_empty(&rdev->fence_drv.emitted)) {
write_unlock_irqrestore(&rdev->fence_drv.lock, irq_flags);
return 0;
}
- fence = list_entry(rdev->fence_drv.emited.next,
+ fence = list_entry(rdev->fence_drv.emitted.next,
struct radeon_fence, list);
radeon_fence_ref(fence);
write_unlock_irqrestore(&rdev->fence_drv.lock, irq_flag
[PATCH 3/3] drm: do not sleep on vblank while holding a mutex
On Fri, Oct 28, 2011 at 08:59:04AM +0200, Michel D?nzer wrote: > On Don, 2011-10-27 at 22:19 -0500, Ilija Hadzic wrote: > > On Thu, 27 Oct 2011, Daniel Vetter wrote: > > > > > So I think the right thing to do is > > > - Kill dev->last_vblank_wait (in a prep patch). > > > > Agreed. Also drm_vblank_info function can go away > > Actually, I was rather going to submit a patch to hook it up again ? > AFAICT it was unhooked without any justification. It could be useful for > debugging vblank related hangs. Any issues with it, such as > last_vblank_wait not being guaranteed to really be the last one, can > always be improved later on. I've thought a bit about the usefulness of it for debugging before proposing to kill it and I think it can die: It's only really useful for a complete hangs, if we have an issue we just missing a wakeup somewhere, that's not gonna catch things. Hence I think something that allows you to watch things while it's running is much better, i.e. either a drm debug prinkt or a tracecall. But if you're firmly attached to that debug file it should be pretty easy to shove that under the protection of one of the vblank spinlocks. -Daniel -- Daniel Vetter Mail: daniel at ffwll.ch Mobile: +41 (0)79 365 57 48
[PATCH 3/3] drm: do not sleep on vblank while holding a mutex
On Thu, Oct 27, 2011 at 10:19:39PM -0500, Ilija Hadzic wrote: > On Thu, 27 Oct 2011, Daniel Vetter wrote: > >The only really hairy thing going on is racing modeset ioctls against > >vblank waiters. But modeset is protected by the dev->mode_config.mutex > >and hence already races against wait_vblank with the current code, so I'm > >slightly inclined to ignore this for the moment. Would be great if you > >coudl check that in-depth, too. > > > > Will leave this for some other patch at some other time; the > critical path is to fix the hang/crawl that I explained in my > previous note. Agreed, one thing at a time. It's complicated enough as is. > >So I think the right thing to do is > >- Kill dev->last_vblank_wait (in a prep patch). > > Agreed. Also drm_vblank_info function can go away > > >- Imo we also have a few too many atomic ops going on, e.g. > > dev->vblank_refcount looks like it's atomic only because or the procfs > > file, so maybe kill that procfs file entirely. > > I am not sure about that. dev->vblank_refcount looks critical to me: > it is incremented through drm_vblank_get which is called from > several different contexts: page-flip functions in several drivers > (which can run in the context of multiple user processes), **_pm_** > stuff in radeon driver (which looks like it runs in the context of > kernel worker thread). Mutexes used at all these different places > are not quite consistent. If anything, as the result of a later > audit, some other mutexes may be rendered unecessary, but > definitely, I would keep vblank_refcount atomic. I've re-read the code a bit and in _get it's protected by dev->vbl_lock, but not so in _put (because we issue a timer call to actually switch it off). I think we could just shove it under the protection of dev->vbl_lock completely. Another fuzzzy area is the relation between dev->vbl_lock and dev->vblank_time_lock. The latter looks a bit rendundant. > >- Audit the vblank locking, maybe resulting in a patch with updated > > comments, if there's anything misleading or tricky going on. And it's > > always good when the locking of structe members is explained in the > > header file ... > > I'll add comments that I feel make sense for this set of patches (if > anything). Full audit, should come later at a slower pace. There are > quite a few mutexes and locks many of which are overlapping in > function and some are spurious. It will take more than just myself > to untangle this know. Yeah, agreed. Let's first drop the mutex around this and untangle the spinlock/atomic mess in a second step. This is too hairy. > >- Drop the mutex locking because it's not needed. > > > > Agreed. Will drop. > > >While locking at the code I also noticed that wait_vblank calls > >drm_vblank_get, but not always drm_vblank_put. The code is correct, the > >missing vblank_put is hidden in drm_queue_vblank_event. Can you also write > >the patch to move that around to not trip up reviewers the next time > >around? > > > > Actually, there is something fishy here. Look at the 'else' branch > under the 'if ((seq - vblwait->request.sequence) <= (1 << 23))' at > the end of the drm_queue_vblank_event. It doesn't have the > 'drm_vblank_put'. It looks wrong to me, but I need to first convince > myself that there is not some other obscure place where > drm_vblank_put is accounted for. If that branch is really missing a > drm_vblank_put, then it will be easy pull the drm_vblank_put out. > Otherwise, it will be another knot to untangle. I've re-read the code and agree with your follow-up analysis. -- Daniel Vetter Mail: daniel at ffwll.ch Mobile: +41 (0)79 365 57 48
[PATCH 3/3] drm: do not sleep on vblank while holding a mutex
On Fri, 28 Oct 2011, Daniel Vetter wrote: > On Fri, Oct 28, 2011 at 08:59:04AM +0200, Michel D?nzer wrote: >> On Don, 2011-10-27 at 22:19 -0500, Ilija Hadzic wrote: >>> On Thu, 27 Oct 2011, Daniel Vetter wrote: >>> So I think the right thing to do is - Kill dev->last_vblank_wait (in a prep patch). >>> >>> Agreed. Also drm_vblank_info function can go away >> >> Actually, I was rather going to submit a patch to hook it up again ? >> AFAICT it was unhooked without any justification. It could be useful for >> debugging vblank related hangs. Any issues with it, such as >> last_vblank_wait not being guaranteed to really be the last one, can >> always be improved later on. > > I've thought a bit about the usefulness of it for debugging before > proposing to kill it and I think it can die: It's only really useful for a > complete hangs, if we have an issue we just missing a wakeup somewhere, > that's not gonna catch things. Hence I think something that allows you to > watch things while it's running is much better, i.e. either a drm debug > prinkt or a tracecall. > > But if you're firmly attached to that debug file it should be pretty easy > to shove that under the protection of one of the vblank spinlocks. I'll keep it then and figure out the best mutex/spinlock to use. It can be anything that exists on one-per-CRTC basis (vblank waits on different CTCs are not contending). The critical section is from that switch in which vblwait->request.sequence is incremented until it is assigned to dev->last_vblank_wait[crtc] (and we are only protecting that section against itself, executed in contexts of different PIDs). I guess we settled now on this patch (other comments will be addressed in a different set of patches). -- Ilija
[Bug 39202] FPS - KDE desktop effects with 3.0 rc6 kernel
https://bugs.freedesktop.org/show_bug.cgi?id=39202 --- Comment #49 from Siganderson 2011-10-28 05:39:21 PDT --- (In reply to comment #48) > > first) and apply the patch as 'patch -p1 name_of_the_patch.patch' > > Typo, it should be > > > patch -p1 < name_of_the_patch.patch > ^ > > but you probably already know that I tried the patch with a 3.0.4 kernel (latest stable). It works and gives something like 2 or 4 fps gain (before the patch the fps dropped to 24-26 while repeatedly minimizing a window, now it drops to 26-28). Do you need even the test with the logs? -- Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug.
[Bug 39202] FPS - KDE desktop effects with 3.0 rc6 kernel
https://bugs.freedesktop.org/show_bug.cgi?id=39202 --- Comment #50 from Siganderson 2011-10-28 05:42:37 PDT --- I forgot to say that without touching anything in the window there are no changes (60 fps... obviously 60 is the maximum when vsync is on). -- Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug.
[Known BUG?] i915 hang on 3.0.0-12 (Ubuntu 11.10 release)
On Fri, 28 Oct 2011 20:22:35 +0800, Yong Zhang wrote: > Hi, > > Just got below error on Ubuntu-11.10 (kernel: 3.0.0-12-generic), > and after that my screen can't show normally. > No sure if it's a known issue. No, that is the first time I've seen that. It looks as if the fence was not released, or reacquired, before the the batch was executed. (There is a later batch that also uses this buffer.) The fence is programmed with: fence[15] = 04e1 valid, x-tiled, pitch: 512, start: 0x04e0, size: 1048576 And the BLT command uses: 0x0df006b0: 0x5434: XY_COLOR_BLT (rgb enabled, alpha enabled, dst tile 0) 0x0df006b4: 0x03f00100:format , pitch 256, clipping disabled 0x0df006b8: 0x:(0,0) 0x0df006bc: 0x00140037:(55,20) 0x0df006c0: 0x04e0:offset 0x04e0 0x0df006c4: 0x:color 0x0df006c8: 0x54f6: XY_SRC_COPY_BLT (rgb enabled, alpha enabled, src tile 0, dst tile 0) 0x0df006cc: 0x03cc0100:format , dst pitch 256, clipping disabled 0x0df006d0: 0x:dst (0,0) 0x0df006d4: 0x00140037:dst (55,20) 0x0df006d8: 0x04e0:dst offset 0x04e0 0x0df006dc: 0x012c0003:src (3,300) 0x0df006e0: 0x0400:src pitch 1024 0x0df006e4: 0x0850:src offset 0x0850 So we try to perform an undefined operation and the GPU hangs. I suspect this will be timing dependent, but if you can find a way to reproduce it, that would be very useful. -Chris -- Chris Wilson, Intel Open Source Technology Centre
[Bug 39202] FPS - KDE desktop effects with 3.0 rc6 kernel
https://bugs.freedesktop.org/show_bug.cgi?id=39202 --- Comment #51 from Ilija Hadzic 2011-10-28 06:25:40 PDT --- (In reply to comment #49) > (In reply to comment #48) > > > first) and apply the patch as 'patch -p1 name_of_the_patch.patch' > > > > Typo, it should be > > > > > > patch -p1 < name_of_the_patch.patch > > ^ > > > > but you probably already know that > > I tried the patch with a 3.0.4 kernel (latest stable). > It works and gives something like 2 or 4 fps gain (before the patch the fps > dropped to 24-26 while repeatedly minimizing a window, now it drops to 26-28). > Do you need even the test with the logs? I guess then your problem is not the same as the bug I am trying to address with this patch. A few fps gain is probably just an (unrelated) incindental finding. At this time, I don't have new theories other than what I postulated back in August. Sorry. -- Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug.
[Bug 42117] R200 driver performance, UMS, all mesa versions from 7.6
https://bugs.freedesktop.org/show_bug.cgi?id=42117 --- Comment #13 from Roland Scheidegger 2011-10-28 06:49:59 PDT --- (In reply to comment #12) > csm->gart_limit and csm->vram_limit are correct. > > With GARTSize "64", openarena works great. ETRacer does not (but no fallbacks) > In ETRacer, when I disabled show fps in options, after few seconds on the map, > framerate back to normal. I'd have thought 128MB VRAM should be enough for openarena so it wouldn't need gart for textures, especially with texture compression. So I don't know why that's really needed looks like something is wrong. You could also try FBTexPercent option though given that I think there should be plenty of vram I don't think it's going to make a difference neither. -- Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug.
[PATCH 2/3] drm/radeon: no need to check all relocs for duplicates
2011/10/28 Christian K?nig :
> Only check the previously checked relocs for
> duplicates. Also leaving the handle uninitialized
> isn't such a good idea.
>
> Signed-off-by: Christian K?nig
Reviewed-by: Alex Deucher
> ---
> ?drivers/gpu/drm/radeon/radeon_cs.c | ? ?5 +++--
> ?1 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/radeon_cs.c
> b/drivers/gpu/drm/radeon/radeon_cs.c
> index fae00c0..7b6e98a 100644
> --- a/drivers/gpu/drm/radeon/radeon_cs.c
> +++ b/drivers/gpu/drm/radeon/radeon_cs.c
> @@ -58,7 +58,7 @@ int radeon_cs_parser_relocs(struct radeon_cs_parser *p)
>
> ? ? ? ? ? ? ? ?duplicate = false;
> ? ? ? ? ? ? ? ?r = (struct drm_radeon_cs_reloc *)&chunk->kdata[i*4];
> - ? ? ? ? ? ? ? for (j = 0; j < p->nrelocs; j++) {
> + ? ? ? ? ? ? ? for (j = 0; j < i; j++) {
> ? ? ? ? ? ? ? ? ? ? ? ?if (r->handle == p->relocs[j].handle) {
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?p->relocs_ptr[i] = &p->relocs[j];
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?duplicate = true;
> @@ -84,7 +84,8 @@ int radeon_cs_parser_relocs(struct radeon_cs_parser *p)
> ? ? ? ? ? ? ? ? ? ? ? ?p->relocs[i].flags = r->flags;
> ? ? ? ? ? ? ? ? ? ? ? ?radeon_bo_list_add_object(&p->relocs[i].lobj,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?&p->validated);
> - ? ? ? ? ? ? ? }
> + ? ? ? ? ? ? ? } else
> + ? ? ? ? ? ? ? ? ? ? ? p->relocs[i].handle = 0;
> ? ? ? ?}
> ? ? ? ?return radeon_bo_list_validate(&p->validated);
> ?}
> --
> 1.7.5.4
>
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>
[PATCH 3/3] drm: do not sleep on vblank while holding a mutex
On Fri, Oct 28, 2011 at 07:10:51AM -0500, Ilija Hadzic wrote: > I'll keep it then and figure out the best mutex/spinlock to use. It > can be anything that exists on one-per-CRTC basis (vblank waits on > different CTCs are not contending). The critical section is from > that switch in which vblwait->request.sequence is incremented until > it is assigned to dev->last_vblank_wait[crtc] (and we are only > protecting that section against itself, executed in contexts of > different PIDs). > > I guess we settled now on this patch (other comments will be > addressed in a different set of patches). If you settle on keeping ->last_vblank_wait I think the easiest is to wrap it with the dev->vbl_lock spinlock everywhere. -Daniel -- Daniel Vetter Mail: daniel at ffwll.ch Mobile: +41 (0)79 365 57 48
[Bug 42117] R200 driver performance, UMS, all mesa versions from 7.6
https://bugs.freedesktop.org/show_bug.cgi?id=42117 --- Comment #14 from Michal 2011-10-28 09:44:56 PDT --- Minimum GARTSize which don't return fallbacks is 16MB. Now, the problem is somewhere at the kernel side. samples| %| -- 1295082 93.6410 vmlinux 30154 2.1803 r200_dri.so 19269 1.3932 etracer 15854 1.1463 radeon samples %symbol name 1261238 97.3867 __copy_from_user_ll 1495 0.1154 delay_tsc -- Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug.
[Bug 42117] R200 driver performance, UMS, all mesa versions from 7.6
https://bugs.freedesktop.org/show_bug.cgi?id=42117 --- Comment #15 from Michal 2011-10-28 10:13:15 PDT --- FBTexPercent to 97 bash-4.1$ cat /var/log/Xorg.0.log|grep text [ 8077.238] (II) RADEON(0): Will use 114688 kb for textures at offset 0x00c08000 etracer runs smoothly at 40 fps(with mesa 7.5.2 60fps). -- 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/kms: properly set panel mode for eDP
Looks like this patch got missed. Dave can you add it and CC: stable?
Alex
On Wed, May 25, 2011 at 2:02 PM, Alex Deucher wrote:
> This should make eDP more reliable.
>
> Signed-off-by: Alex Deucher
> ---
> ?drivers/gpu/drm/radeon/atombios_dp.c | ? 11 +++
> ?include/drm/drm_dp_helper.h ? ? ? ? ?| ? ?3 +++
> ?2 files changed, 14 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/atombios_dp.c
> b/drivers/gpu/drm/radeon/atombios_dp.c
> index 8c0f9e3..892b88d 100644
> --- a/drivers/gpu/drm/radeon/atombios_dp.c
> +++ b/drivers/gpu/drm/radeon/atombios_dp.c
> @@ -543,6 +543,7 @@ static void radeon_dp_set_panel_mode(struct drm_encoder
> *encoder,
> ?{
> ? ? ? ?struct drm_device *dev = encoder->dev;
> ? ? ? ?struct radeon_device *rdev = dev->dev_private;
> + ? ? ? struct radeon_connector *radeon_connector =
> to_radeon_connector(connector);
> ? ? ? ?int panel_mode = DP_PANEL_MODE_EXTERNAL_DP_MODE;
>
> ? ? ? ?if (!ASIC_IS_DCE4(rdev))
> @@ -550,10 +551,20 @@ static void radeon_dp_set_panel_mode(struct drm_encoder
> *encoder,
>
> ? ? ? ?if (radeon_connector_encoder_is_dp_bridge(connector))
> ? ? ? ? ? ? ? ?panel_mode = DP_PANEL_MODE_INTERNAL_DP1_MODE;
> + ? ? ? else if (connector->connector_type == DRM_MODE_CONNECTOR_eDP) {
> + ? ? ? ? ? ? ? u8 tmp = radeon_read_dpcd_reg(radeon_connector,
> DP_EDP_CONFIGURATION_CAP);
> + ? ? ? ? ? ? ? if (tmp & 1)
> + ? ? ? ? ? ? ? ? ? ? ? panel_mode = DP_PANEL_MODE_INTERNAL_DP2_MODE;
> + ? ? ? }
>
> ? ? ? ?atombios_dig_encoder_setup(encoder,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ATOM_ENCODER_CMD_SETUP_PANEL_MODE,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? panel_mode);
> +
> + ? ? ? if ((connector->connector_type == DRM_MODE_CONNECTOR_eDP) &&
> + ? ? ? ? ? (panel_mode == DP_PANEL_MODE_INTERNAL_DP2_MODE)) {
> + ? ? ? ? ? ? ? radeon_write_dpcd_reg(radeon_connector,
> DP_EDP_CONFIGURATION_SET, 1);
> + ? ? ? }
> ?}
>
> ?void radeon_dp_set_link_config(struct drm_connector *connector,
> diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
> index 91567bb..03eb1d6 100644
> --- a/include/drm/drm_dp_helper.h
> +++ b/include/drm/drm_dp_helper.h
> @@ -72,6 +72,7 @@
>
> ?#define DP_MAIN_LINK_CHANNEL_CODING ? ? ? ? 0x006
>
> +#define DP_EDP_CONFIGURATION_CAP ? ? ? ? ? ?0x00d
> ?#define DP_TRAINING_AUX_RD_INTERVAL ? ? ? ? 0x00e
>
> ?/* link configuration */
> @@ -133,6 +134,8 @@
> ?#define DP_MAIN_LINK_CHANNEL_CODING_SET ? ? ? ? ? ?0x108
> ?# define DP_SET_ANSI_8B10B ? ? ? ? ? ? ? ? (1 << 0)
>
> +#define DP_EDP_CONFIGURATION_SET ? ? ? ? ? ?0x10a
> +
> ?#define DP_LANE0_1_STATUS ? ? ? ? ? ? ? ? ?0x202
> ?#define DP_LANE2_3_STATUS ? ? ? ? ? ? ? ? ?0x203
> ?# define DP_LANE_CR_DONE ? ? ? ? ? ? ? ? ? (1 << 0)
> --
> 1.7.4
>
>
[PATCH] agp: iommu_gfx_mapped only available if CONFIG_INTEL_IOMMU is set
Kernels with no iommu support cannot ever need the Ironlake
work-around, so never enable it in that case.
Might be better to completely remove the work-around from the kernel
in this case?
Signed-off-by: Keith Packard
Cc: Ben Widawsky
---
drivers/char/agp/intel-gtt.c |4
1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/drivers/char/agp/intel-gtt.c b/drivers/char/agp/intel-gtt.c
index 3a8d448..222190f 100644
--- a/drivers/char/agp/intel-gtt.c
+++ b/drivers/char/agp/intel-gtt.c
@@ -1183,6 +1183,7 @@ static void gen6_cleanup(void)
{
}
+#ifdef CONFIG_INTEL_IOMMU
/* Certain Gen5 chipsets require require idling the GPU before
* unmapping anything from the GTT when VT-d is enabled.
*/
@@ -1201,6 +1202,7 @@ static inline int needs_idle_maps(void)
return 0;
}
+#endif
static int i9xx_setup(void)
{
@@ -1236,8 +1238,10 @@ static int i9xx_setup(void)
intel_private.gtt_bus_addr = reg_addr + gtt_offset;
}
+#ifdef CONFIG_INTEL_IOMMU
if (needs_idle_maps())
intel_private.base.do_idle_maps = 1;
+#endif
intel_i9xx_setup_flush();
--
1.7.7
[PATCH] agp: iommu_gfx_mapped only available if CONFIG_INTEL_IOMMU is set
Kernels with no iommu support cannot ever need the Ironlake
work-around, so never enable it in that case.
Might be better to completely remove the work-around from the kernel
in this case?
Signed-off-by: Keith Packard
Cc: Ben Widawsky
---
Here's a shorter patch which just elides the body of the needs_idle_maps
functions
drivers/char/agp/intel-gtt.c |5 +++--
1 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/char/agp/intel-gtt.c b/drivers/char/agp/intel-gtt.c
index 3a8d448..c92424c 100644
--- a/drivers/char/agp/intel-gtt.c
+++ b/drivers/char/agp/intel-gtt.c
@@ -1186,10 +1186,11 @@ static void gen6_cleanup(void)
/* Certain Gen5 chipsets require require idling the GPU before
* unmapping anything from the GTT when VT-d is enabled.
*/
-extern int intel_iommu_gfx_mapped;
static inline int needs_idle_maps(void)
{
+#ifdef CONFIG_INTEL_IOMMU
const unsigned short gpu_devid = intel_private.pcidev->device;
+ extern int intel_iommu_gfx_mapped;
/* Query intel_iommu to see if we need the workaround. Presumably that
* was loaded first.
@@ -1198,7 +1199,7 @@ static inline int needs_idle_maps(void)
gpu_devid == PCI_DEVICE_ID_INTEL_IRONLAKE_M_IG) &&
intel_iommu_gfx_mapped)
return 1;
-
+#endif
return 0;
}
--
1.7.7
[PATCH 3/3] drm: do not sleep on vblank while holding a mutex
> Message: 5 > Date: Fri, 28 Oct 2011 11:30:38 +0200 > From: Daniel Vetter > Subject: Re: [PATCH 3/3] drm: do not sleep on vblank while holding a > mutex > To: Ilija Hadzic > Cc: dri-devel at lists.freedesktop.org > Message-ID: <20111028093038.GB2919 at phenom.ffwll.local> > Content-Type: text/plain; charset=us-ascii >> >>> - Imo we also have a few too many atomic ops going on, e.g. >>> dev->vblank_refcount looks like it's atomic only because or the >>> procfs >>> file, so maybe kill that procfs file entirely. >> >> I am not sure about that. dev->vblank_refcount looks critical to me: >> it is incremented through drm_vblank_get which is called from >> several different contexts: page-flip functions in several drivers >> (which can run in the context of multiple user processes), **_pm_** >> stuff in radeon driver (which looks like it runs in the context of >> kernel worker thread). Mutexes used at all these different places >> are not quite consistent. If anything, as the result of a later >> audit, some other mutexes may be rendered unecessary, but >> definitely, I would keep vblank_refcount atomic. > Hi, be careful with vblank_refcount. I think it probably should stay atomic. The drm_vblank_put() is often used in interrupt handlers of the kms drivers where you don't want to uneccessary wait on a lock, but be as quick as possible. > I've re-read the code a bit and in _get it's protected by dev- > >vbl_lock, > but not so in _put (because we issue a timer call to actually > switch it > off). I think we could just shove it under the protection of dev- > >vbl_lock > completely. Another fuzzzy area is the relation between dev- > >vbl_lock and > dev->vblank_time_lock. The latter looks a bit rendundant. > The vblank_time_lock serves a different purpose from vbl_lock. It is used in the vblank irq handler drm_handle_vblank() and in the vblank irq on/off functions to protect the vblank timestamping and counting against races between the irq handler and the on/off code, and some funny races between the cpu reinitializing vblank counts and timestamps in non-irq path and the gpu firing vblank interrupts and/ or updating its hardware vblank counter at the "wrong" moment. vblank_time_lock basically blocks/delays gpu vblank irq processing during such problematic periods. Timing there is criticial for reliable vblank timestamping, kms page flipping and artifact free dynamic gpu power-management. It should be locked as seldomly and held as short as possible. I initially tried to get away only with vbl_lock, but there were uses of vbl_lock that looked as if using it in the interrupt handler as well could cause long delays in irq processing. thanks, -mario * Mario Kleiner Max Planck Institute for Biological Cybernetics Spemannstr. 38 72076 Tuebingen Germany e-mail: mario.kleiner at tuebingen.mpg.de office: +49 (0)7071/601-1623 fax:+49 (0)7071/601-616 www:http://www.kyb.tuebingen.mpg.de/~kleinerm * "For a successful technology, reality must take precedence over public relations, for Nature cannot be fooled." (Richard Feynman)
[Intel-gfx] [PATCH] agp: iommu_gfx_mapped only available if CONFIG_INTEL_IOMMU is set
On Fri, 28 Oct 2011 10:56:29 -0700 Keith Packard wrote: > Kernels with no iommu support cannot ever need the Ironlake > work-around, so never enable it in that case. > > Might be better to completely remove the work-around from the kernel > in this case? > > Signed-off-by: Keith Packard > Cc: Ben Widawsky > --- wfm Reviewed-by: Ben Widawsky
[Bug 36934] screen corruption after running a game
https://bugs.freedesktop.org/show_bug.cgi?id=36934 --- Comment #17 from auxsvr at gmail.com 2011-10-28 11:26:57 PDT --- I'm afraid the virtualbox modules have nothing to do with this. A hopefully helpful observation: When I first load the large image in firefox, zooming in and out several times works fine, without any corruption and Xorg uses 87 MB of RSS memory. It is after changing desktop, scrolling in okular etc that it becomes corrupted. When it becomes corrupted, Xorg use drops to 54 MB of RSS memory. Doesn't this lead to the conclusion that there is some error during memory management? If this were a hardware error, wouldn't Xorg memory use remain constant? -- Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug.
[PATCH 3/3] drm: do not sleep on vblank while holding a mutex
On Fri, Oct 28, 2011 at 08:15:11PM +0200, Mario Kleiner wrote: > be careful with vblank_refcount. I think it probably should stay > atomic. The drm_vblank_put() is often used in interrupt handlers of > the kms drivers where you don't want to uneccessary wait on a lock, > but be as quick as possible. I don't think that's a real issue - if we have lock contention anywhere with the current code, we already have a problem - and it looks like we have ;-) And the spinlocks are all already switching off irqs locally, so we should already strive for the shortest possible lock hold times. [snip] > The vblank_time_lock serves a different purpose from vbl_lock. It is > used in the vblank irq handler drm_handle_vblank() and in the vblank > irq on/off functions to protect the vblank timestamping and counting > against races between the irq handler and the on/off code, and some > funny races between the cpu reinitializing vblank counts and > timestamps in non-irq path and the gpu firing vblank interrupts and/ > or updating its hardware vblank counter at the "wrong" moment. > vblank_time_lock basically blocks/delays gpu vblank irq processing > during such problematic periods. Timing there is criticial for > reliable vblank timestamping, kms page flipping and artifact free > dynamic gpu power-management. It should be locked as seldomly and > held as short as possible. I initially tried to get away only with > vbl_lock, but there were uses of vbl_lock that looked as if using it > in the interrupt handler as well could cause long delays in irq > processing. I've gone through the code, and in almost all case where the vbl_lock is hold for a bit more than just a few load and stores we are immediately taking the vblank_time_lock and hold it almost as long. Which is why I think this is redundant. The only exeption is the irq uninstall code, which isn't really a fast path anyway. Also, all these paths with longer lock-hold times are in the initial vblank_get/final vblank_put. And because we delay the vblank disabling by a full second, we should never hit that in steady state. But I think you're one of the most timing-sensitive users of this vblank stuff, so when I get around to clean this up I'll certainly put you on cc so you can test for any adverse effects. Hopefully ripping out unecessary atomic ops makes stuff faster, not slower. -Daniel -- Daniel Vetter Mail: daniel at ffwll.ch Mobile: +41 (0)79 365 57 48
[PATCH 3/3] drm: do not sleep on vblank while holding a mutex
On Oct 28, 2011, at 9:15 PM, Daniel Vetter wrote: > On Fri, Oct 28, 2011 at 08:15:11PM +0200, Mario Kleiner wrote: >> be careful with vblank_refcount. I think it probably should stay >> atomic. The drm_vblank_put() is often used in interrupt handlers of >> the kms drivers where you don't want to uneccessary wait on a lock, >> but be as quick as possible. > > I don't think that's a real issue - if we have lock contention > anywhere > with the current code, we already have a problem - and it looks > like we > have ;-) And the spinlocks are all already switching off irqs > locally, so > we should already strive for the shortest possible lock hold times. > > [snip] > >> The vblank_time_lock serves a different purpose from vbl_lock. It is >> used in the vblank irq handler drm_handle_vblank() and in the vblank >> irq on/off functions to protect the vblank timestamping and counting >> against races between the irq handler and the on/off code, and some >> funny races between the cpu reinitializing vblank counts and >> timestamps in non-irq path and the gpu firing vblank interrupts and/ >> or updating its hardware vblank counter at the "wrong" moment. >> vblank_time_lock basically blocks/delays gpu vblank irq processing >> during such problematic periods. Timing there is criticial for >> reliable vblank timestamping, kms page flipping and artifact free >> dynamic gpu power-management. It should be locked as seldomly and >> held as short as possible. I initially tried to get away only with >> vbl_lock, but there were uses of vbl_lock that looked as if using it >> in the interrupt handler as well could cause long delays in irq >> processing. > > I've gone through the code, and in almost all case where the > vbl_lock is > hold for a bit more than just a few load and stores we are immediately > taking the vblank_time_lock and hold it almost as long. Which is why I > think this is redundant. The only exeption is the irq uninstall code, > which isn't really a fast path anyway. Ok, if that is the case in the current code, fair enough. We would have to stress the importance of *very low* vbl_lock hold times in the documentation of the code if we want to get rid of the vblank_time_lock. The distinctive lock for that purpose is also meant as a marker that you should try twice as hard to be quick while you hold that lock than with other spinlocks. A few dozen usecs extra delay there could make a difference between met and missed pageflip deadlines and well working or not well working dynamic gpu reclocking, e.g., on radeon, where this stuff is triggered by vblank irq's and quite timing sensitive. > > Also, all these paths with longer lock-hold times are in the initial > vblank_get/final vblank_put. And because we delay the vblank > disabling by > a full second, we should never hit that in steady state. 5 seconds by default, i think. We wanted to lower that to something like < 100 msecs at some point, but there's still some work to do to remove some potential race with the gpu in the on/off path which could cause lost/wrong vblank counts, and the nouveau kms driver doesn't implement hardware vblank counter at all atm, which makes small values on nouveau unsafe atm. With higher on/off frequencies the impact of too long lock hold times would become worse. > But I think you're one of the most timing-sensitive users of this > vblank > stuff, so when I get around to clean this up I'll certainly put you > on cc > so you can test for any adverse effects. Hopefully ripping out > unecessary > atomic ops makes stuff faster, not slower. Yes, the current timing/timestamp precision and some of the related dri2 features of the free graphics stack is a very strong selling point to lure quite many timing sensitive users of my userspace toolkit away from other proprietary solutions. Ok, cc'ing me sounds like a plan. Thanks, -mario > -Daniel > -- > Daniel Vetter > Mail: daniel at ffwll.ch > Mobile: +41 (0)79 365 57 48 * Mario Kleiner Max Planck Institute for Biological Cybernetics Spemannstr. 38 72076 Tuebingen Germany e-mail: mario.kleiner at tuebingen.mpg.de office: +49 (0)7071/601-1623 fax:+49 (0)7071/601-616 www:http://www.kyb.tuebingen.mpg.de/~kleinerm * "For a successful technology, reality must take precedence over public relations, for Nature cannot be fooled." (Richard Feynman)
[Bug 41569] [r600 KMS] Asus A53T A4-3400
https://bugs.freedesktop.org/show_bug.cgi?id=41569 Alex Deucher changed: What|Removed |Added CC||feth at tuttu.info --- Comment #4 from Alex Deucher 2011-10-21 07:43:37 PDT --- *** Bug 42004 has been marked as a duplicate of this bug. *** --- Comment #5 from Alex Deucher 2011-10-28 13:16:46 PDT --- Created attachment 52865 --> https://bugs.freedesktop.org/attachment.cgi?id=52865 possible fix patch 1/2 -- Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug.
[Bug 41569] [r600 KMS] Asus A53T A4-3400
https://bugs.freedesktop.org/show_bug.cgi?id=41569 --- Comment #6 from Alex Deucher 2011-10-28 13:17:15 PDT --- Created attachment 52866 --> https://bugs.freedesktop.org/attachment.cgi?id=52866 possible fix patch 2/2. Do these patches help? -- Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug.
[PATCH v2] drm: do not sleep on vblank while holding a mutex
drm_wait_vblank must be DRM_UNLOCKED because otherwise it
will grab the drm_global_mutex and then go to sleep until the vblank
event it is waiting for. That can wreck havoc in the windowing system
because if one process issues this ioctl, it will block all other
processes for the duration of all vblanks between the current and the
one it is waiting for. In some cases it can block the entire windowing
system. So, make this ioctl DRM_UNLOCKED, wrap the remaining
(very short) critical section with dev->vbl_lock spinlock, and
add a comment to the code explaining what we are protecting with
the lock.
v2: incorporate comments received from Daniel Vetter and
Michel Daenzer.
Signed-off-by: Ilija Hadzic
---
drivers/gpu/drm/drm_drv.c |2 +-
drivers/gpu/drm/drm_irq.c | 14 +-
2 files changed, 14 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index e159f17..c990dab 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -124,7 +124,7 @@ static struct drm_ioctl_desc drm_ioctls[] = {
DRM_IOCTL_DEF(DRM_IOCTL_SG_ALLOC, drm_sg_alloc_ioctl,
DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),
DRM_IOCTL_DEF(DRM_IOCTL_SG_FREE, drm_sg_free,
DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),
- DRM_IOCTL_DEF(DRM_IOCTL_WAIT_VBLANK, drm_wait_vblank, 0),
+ DRM_IOCTL_DEF(DRM_IOCTL_WAIT_VBLANK, drm_wait_vblank, DRM_UNLOCKED),
DRM_IOCTL_DEF(DRM_IOCTL_MODESET_CTL, drm_modeset_ctl, 0),
diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
index 3830e9e..c8b4da8 100644
--- a/drivers/gpu/drm/drm_irq.c
+++ b/drivers/gpu/drm/drm_irq.c
@@ -1160,6 +1160,7 @@ int drm_wait_vblank(struct drm_device *dev, void *data,
union drm_wait_vblank *vblwait = data;
int ret = 0;
unsigned int flags, seq, crtc, high_crtc;
+ unsigned long irqflags;
if ((!drm_dev_to_irq(dev)) || (!dev->irq_enabled))
return -EINVAL;
@@ -1193,6 +1194,13 @@ int drm_wait_vblank(struct drm_device *dev, void *data,
}
seq = drm_vblank_count(dev, crtc);
+ /* the lock protects this section from itself executed in */
+ /* the context of another PID, ensuring that the process that */
+ /* wrote dev->last_vblank_wait is really the last process */
+ /* that was here; processes waiting on different CRTCs */
+ /* do not need to be interlocked, but rather than introducing */
+ /* a new per-crtc lock, we reuse vbl_lock for simplicity */
+ spin_lock_irqsave(&dev->vbl_lock, irqflags);
switch (vblwait->request.type & _DRM_VBLANK_TYPES_MASK) {
case _DRM_VBLANK_RELATIVE:
vblwait->request.sequence += seq;
@@ -1200,12 +1208,15 @@ int drm_wait_vblank(struct drm_device *dev, void *data,
case _DRM_VBLANK_ABSOLUTE:
break;
default:
+ spin_unlock_irqrestore(&dev->vbl_lock, irqflags);
ret = -EINVAL;
goto done;
}
- if (flags & _DRM_VBLANK_EVENT)
+ if (flags & _DRM_VBLANK_EVENT) {
+ spin_unlock_irqrestore(&dev->vbl_lock, irqflags);
return drm_queue_vblank_event(dev, crtc, vblwait, file_priv);
+ }
if ((flags & _DRM_VBLANK_NEXTONMISS) &&
(seq - vblwait->request.sequence) <= (1<<23)) {
@@ -1215,6 +1226,7 @@ int drm_wait_vblank(struct drm_device *dev, void *data,
DRM_DEBUG("waiting on vblank count %d, crtc %d\n",
vblwait->request.sequence, crtc);
dev->last_vblank_wait[crtc] = vblwait->request.sequence;
+ spin_unlock_irqrestore(&dev->vbl_lock, irqflags);
DRM_WAIT_ON(ret, dev->vbl_queue[crtc], 3 * DRM_HZ,
(((drm_vblank_count(dev, crtc) -
vblwait->request.sequence) <= (1 << 23)) ||
--
1.7.7
[PATCH v2] drm: make DRM_UNLOCKED ioctls with their own mutex
drm_getclient, drm_getstats and drm_getmap (with a few minor
adjustments) do not need global mutex, so fix that and
make the said ioctls DRM_UNLOCKED. Details:
drm_getclient: the only thing that should be protected here
is dev->filelist and that is already protected everywhere with
dev->struct_mutex.
drm_getstats: there is no need for any mutex here because the
loop runs through quasi-static (set at load time only)
data, and the actual count access is done with atomic_read()
drm_getmap already uses dev->struct_mutex to protect
dev->maplist, which also used to protect the same structure
everywhere else except at three places:
* drm_getsarea, which doesn't grab *any* mutex before
touching dev->maplist (so no drm_global_mutex doesn't help
here either; different issue for a different patch).
However, drivers seem to call it only at
initialization time so it probably doesn't matter
* drm_master_destroy, which is called from drm_master_put,
which in turn is protected with dev->struct_mutex
everywhere else in drm module, so we are good here too.
* drm_getsareactx, which releases the dev->struct_mutex
too early, but this patch includes the fix for that.
v2: * incorporate comments received from Daniel Vetter
* include the (long) explanation above to make it clear what
we are doing (and why), also at Daniel Vetter's request
* tighten up mutex grab/release locations to only
encompass real critical sections, rather than some
random code around them
Signed-off-by: Ilija Hadzic
---
drivers/gpu/drm/drm_context.c |5 +++--
drivers/gpu/drm/drm_drv.c |6 +++---
drivers/gpu/drm/drm_ioctl.c | 15 ---
3 files changed, 10 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/drm_context.c b/drivers/gpu/drm/drm_context.c
index 6d440fb..325365f 100644
--- a/drivers/gpu/drm/drm_context.c
+++ b/drivers/gpu/drm/drm_context.c
@@ -154,8 +154,6 @@ int drm_getsareactx(struct drm_device *dev, void *data,
return -EINVAL;
}
- mutex_unlock(&dev->struct_mutex);
-
request->handle = NULL;
list_for_each_entry(_entry, &dev->maplist, head) {
if (_entry->map == map) {
@@ -164,6 +162,9 @@ int drm_getsareactx(struct drm_device *dev, void *data,
break;
}
}
+
+ mutex_unlock(&dev->struct_mutex);
+
if (request->handle == NULL)
return -EINVAL;
diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index c990dab..dc0eb0b 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -64,9 +64,9 @@ static struct drm_ioctl_desc drm_ioctls[] = {
DRM_IOCTL_DEF(DRM_IOCTL_GET_UNIQUE, drm_getunique, 0),
DRM_IOCTL_DEF(DRM_IOCTL_GET_MAGIC, drm_getmagic, 0),
DRM_IOCTL_DEF(DRM_IOCTL_IRQ_BUSID, drm_irq_by_busid,
DRM_MASTER|DRM_ROOT_ONLY),
- DRM_IOCTL_DEF(DRM_IOCTL_GET_MAP, drm_getmap, 0),
- DRM_IOCTL_DEF(DRM_IOCTL_GET_CLIENT, drm_getclient, 0),
- DRM_IOCTL_DEF(DRM_IOCTL_GET_STATS, drm_getstats, 0),
+ DRM_IOCTL_DEF(DRM_IOCTL_GET_MAP, drm_getmap, DRM_UNLOCKED),
+ DRM_IOCTL_DEF(DRM_IOCTL_GET_CLIENT, drm_getclient, DRM_UNLOCKED),
+ DRM_IOCTL_DEF(DRM_IOCTL_GET_STATS, drm_getstats, DRM_UNLOCKED),
DRM_IOCTL_DEF(DRM_IOCTL_GET_CAP, drm_getcap, DRM_UNLOCKED),
DRM_IOCTL_DEF(DRM_IOCTL_SET_VERSION, drm_setversion, DRM_MASTER),
diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index 904d7e9..956fd38 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -158,14 +158,11 @@ int drm_getmap(struct drm_device *dev, void *data,
int i;
idx = map->offset;
-
- mutex_lock(&dev->struct_mutex);
- if (idx < 0) {
- mutex_unlock(&dev->struct_mutex);
+ if (idx < 0)
return -EINVAL;
- }
i = 0;
+ mutex_lock(&dev->struct_mutex);
list_for_each(list, &dev->maplist) {
if (i == idx) {
r_list = list_entry(list, struct drm_map_list, head);
@@ -211,9 +208,9 @@ int drm_getclient(struct drm_device *dev, void *data,
int i;
idx = client->idx;
- mutex_lock(&dev->struct_mutex);
-
i = 0;
+
+ mutex_lock(&dev->struct_mutex);
list_for_each_entry(pt, &dev->filelist, lhead) {
if (i++ >= idx) {
client->auth = pt->authenticated;
@@ -249,8 +246,6 @@ int drm_getstats(struct drm_device *dev, void *data,
memset(stats, 0, sizeof(*stats));
- mutex_lock(&dev->struct_mutex);
-
for (i = 0; i < dev->counters; i++) {
if (dev->types[i] == _DRM_STAT_LOCK)
stats->data[i].value =
@@ -262,8 +257,6 @@ int drm_getstats(struct drm_device *dev, void *data,
stats->count = dev->counters;
- mutex_unlock(&dev->struct_mutex);
-
return 0;
}
--
[PATCH] drm: add some comments to drm_wait_vblank and drm_queue_vblank_event
during the review of the fix for locks problems in drm_wait_vblank,
a couple of false concerns were raised about how the drm_vblank_get
and drm_vblank_put are used in this function; it turned out that the
code is correct and that it cannot be simplified
add a few comments to explain non-obvious flows in the code,
to prevent "false alarms" in the future
Signed-off-by: Ilija Hadzic
---
drivers/gpu/drm/drm_irq.c |8
1 files changed, 8 insertions(+), 0 deletions(-)
diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
index c8b4da8..e9dd19d 100644
--- a/drivers/gpu/drm/drm_irq.c
+++ b/drivers/gpu/drm/drm_irq.c
@@ -1065,6 +1065,10 @@ out:
return ret;
}
+/* must acquire vblank reference count (call drm_vblank_get) */
+/* before calling this function; the matching drm_vblank_put */
+/* will either be issued here or in drm_handle_vblank_events */
+/* after the vblank is signaled */
static int drm_queue_vblank_event(struct drm_device *dev, int pipe,
union drm_wait_vblank *vblwait,
struct drm_file *file_priv)
@@ -1124,6 +1128,9 @@ static int drm_queue_vblank_event(struct drm_device *dev,
int pipe,
trace_drm_vblank_event_delivered(current->pid, pipe,
vblwait->request.sequence);
} else {
+ /* can't call drm_vblank_put here because interrupt */
+ /* must remain enabled until the event occurs */
+ /* drm_handle_vblank_events will do this for us */
list_add_tail(&e->base.link, &dev->vblank_event_list);
vblwait->reply.sequence = vblwait->request.sequence;
}
@@ -1215,6 +1222,7 @@ int drm_wait_vblank(struct drm_device *dev, void *data,
if (flags & _DRM_VBLANK_EVENT) {
spin_unlock_irqrestore(&dev->vbl_lock, irqflags);
+ /* drm_queue_vblank_event() will call drm_vblank_put() */
return drm_queue_vblank_event(dev, crtc, vblwait, file_priv);
}
--
1.7.7
[PATCH] drm/radeon: set hpd polarity at init time so hotplug detect works
From: Jerome Glisse Polarity needs to be set accordingly to connector status (connected or disconnected). Set it up at module init so first hotplug works reliably no matter what is the initial set of connector. Signed-off-by: Jerome Glisse cc: stable at kernel.org --- drivers/gpu/drm/radeon/radeon_connectors.c |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/drivers/gpu/drm/radeon/radeon_connectors.c b/drivers/gpu/drm/radeon/radeon_connectors.c index dec6cbe..bfdd48b 100644 --- a/drivers/gpu/drm/radeon/radeon_connectors.c +++ b/drivers/gpu/drm/radeon/radeon_connectors.c @@ -1789,6 +1789,7 @@ radeon_add_atom_connector(struct drm_device *dev, connector->polled = DRM_CONNECTOR_POLL_CONNECT; } else connector->polled = DRM_CONNECTOR_POLL_HPD; + radeon_hpd_set_polarity(rdev, radeon_connector->hpd.hpd); connector->display_info.subpixel_order = subpixel_order; drm_sysfs_connector_add(connector); -- 1.7.6.4
[PATCH] drm/radeon: set hpd polarity at init time so hotplug detect works
On Fri, Oct 28, 2011 at 5:52 PM, wrote: > From: Jerome Glisse > > Polarity needs to be set accordingly to connector status (connected > or disconnected). Set it up at module init so first hotplug works > reliably no matter what is the initial set of connector. > > Signed-off-by: Jerome Glisse > cc: stable at kernel.org Reviewed-by: Alex Deucher > --- > ?drivers/gpu/drm/radeon/radeon_connectors.c | ? ?1 + > ?1 files changed, 1 insertions(+), 0 deletions(-) > > diff --git a/drivers/gpu/drm/radeon/radeon_connectors.c > b/drivers/gpu/drm/radeon/radeon_connectors.c > index dec6cbe..bfdd48b 100644 > --- a/drivers/gpu/drm/radeon/radeon_connectors.c > +++ b/drivers/gpu/drm/radeon/radeon_connectors.c > @@ -1789,6 +1789,7 @@ radeon_add_atom_connector(struct drm_device *dev, > ? ? ? ? ? ? ? ? ? ? ? ?connector->polled = DRM_CONNECTOR_POLL_CONNECT; > ? ? ? ?} else > ? ? ? ? ? ? ? ?connector->polled = DRM_CONNECTOR_POLL_HPD; > + ? ? ? radeon_hpd_set_polarity(rdev, radeon_connector->hpd.hpd); > > ? ? ? ?connector->display_info.subpixel_order = subpixel_order; > ? ? ? ?drm_sysfs_connector_add(connector); > -- > 1.7.6.4 > > ___ > dri-devel mailing list > dri-devel at lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel >
[Bug 36609] 45920d2ecb38b14fdda5253fecce996570c22863 breaks sauerbraten on r300g
https://bugs.freedesktop.org/show_bug.cgi?id=36609 Tom Stellard changed: What|Removed |Added Status|REOPENED|RESOLVED Resolution||FIXED --- Comment #6 from Tom Stellard 2011-10-28 18:23:44 UTC --- The issue with sauerbraten has been fixed. If you are still experiencing problems with ut2004, please file a new bug and attach a screenshot demonstrating the bug. -- Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug.
[PATCH v2] drm: do not sleep on vblank while holding a mutex
On Sat, 29 Oct 2011, Daniel Vetter wrote: >> +/* the lock protects this section from itself executed in */ >> +/* the context of another PID, ensuring that the process that */ >> +/* wrote dev->last_vblank_wait is really the last process */ >> +/* that was here; processes waiting on different CRTCs */ >> +/* do not need to be interlocked, but rather than introducing */ >> +/* a new per-crtc lock, we reuse vbl_lock for simplicity */ > > Multiline comments are done with one pair of /* */, see CodingStyle > Will fix the multiline comments (though scripts/checkpatch.pl didn't complain so I figured it was OK) > > I personally vote for no additional locking at all here and just drop the > global lock. Or maybe I'm blind and we need this. In that case, please > clue me up. > What I am doing with the lock, is makeing sure that the last vblank wait reported is really last. You said in your previous comment (which I agree with) that the value of having last_vblank_wait in debugfs is in case of hangs. In that case you want to see who really was the last one to issue the vblank. Now suppose that the drm_wait_vblank is enteret in the context of two different PIDs and suppose that there are no locks. Let's say that the first process wants to wait on vblank N and the second wants to wait on vblank N+1. First process can reach the point just before it wants to write to last_vblank_wait and lose the processor there (so it has N in vblank.request (on its stack) calculated, but it has not written it into last_vblank_wait yet (because it lost the CPU right there). Then the second process gets the CPU, and executes and let's say that it goes all the way through so it writes N+1 into last_vblank_wait and then schedules away (it called DRM_WAIT_ON). Now the first process gets the CPU where it left off and overwrites N+1 in last_vblank_wait with N, which is now stale. I explained in the comment (and in my note this morning), that the only race condition is against itself in the context of different processes. The above is the scenario. Race against drm_vblank_info exists in theory, but in practice doesn't matter because the reader of debugfs (or proc file system) runs asynchronously (and typically at slower pace) from processes issuing vblank waits (if it's a human looking at the filesystem then definitely much slower pace). Since processes issuing vblanks are overwriting the same last_vblank_value, drm_vblank_info is already doing a form of downsampling and debugfs is losing information, anyway, so who cares about the lock. In case of a hang, the value of last_vblank_wait doesn't change, so again the lock in drm_vblank_info doesn't change anything. So adding a lock there (which would have to be vbl_lock to make it usable) is only a matter of theoretical correctness, but no practical value. So I decided not to touch drm_vblank_info. drm_wait_vblank, however, needs a protection from itself as I explained above. >> DRM_WAIT_ON(ret, dev->vbl_queue[crtc], 3 * DRM_HZ, >> (((drm_vblank_count(dev, crtc) - >> vblwait->request.sequence) <= (1 << 23)) || > > One line below there's the curios case of us rechecking dev->irq_enabled. > Without any locking in place. But afaics this is only changed by the > driver at setup and teardown when we are guaranteed (hopefully) that > there's no open fd and hence no way to call an ioctl. So checking once at > the beginning should be good enough. Whatever. It's probably redundant statement because it won't be reached if irq_enabled is false and there is nothing to change it to true at runtime, but that's a topic for a different patch. -- Ilija
[Known BUG?] i915 hang on 3.0.0-12 (Ubuntu 11.10 release)
Hi, Just got below error on Ubuntu-11.10 (kernel: 3.0.0-12-generic), and after that my screen can't show normally. No sure if it's a known issue. [169509.080020] [drm:i915_hangcheck_elapsed] *ERROR* Hangcheck timer elapsed... GPU hung [169509.080026] [drm] capturing error event; look for more information in /debug/dri/0/i915_error_state [169509.080604] [drm:i915_wait_request] *ERROR* i915_wait_request returns -11 (awaiting 1640573 at 1640569, next 1640574) [169509.080688] [drm:i915_reset] *ERROR* Failed to reset chip. BTW, dmesg and i915_error_state and lspci is attached. Thanks, Yong -- next part -- [0.00] Initializing cgroup subsys cpuset [0.00] Initializing cgroup subsys cpu [0.00] Linux version 3.0.0-12-generic (buildd at vernadsky) (gcc version 4.6.1 (Ubuntu/Linaro 4.6.1-9ubuntu3) ) #20-Ubuntu SMP Fri Oct 7 14:50:42 UTC 2011 (Ubuntu 3.0.0-12.20-generic 3.0.4) [0.00] KERNEL supported cpus: [0.00] Intel GenuineIntel [0.00] AMD AuthenticAMD [0.00] NSC Geode by NSC [0.00] Cyrix CyrixInstead [0.00] Centaur CentaurHauls [0.00] Transmeta GenuineTMx86 [0.00] Transmeta TransmetaCPU [0.00] UMC UMC UMC UMC [0.00] BIOS-provided physical RAM map: [0.00] BIOS-e820: - 0009ec00 (usable) [0.00] BIOS-e820: 000f - 0010 (reserved) [0.00] BIOS-e820: 0010 - 7d3ff800 (usable) [0.00] BIOS-e820: 7d3ff800 - 7d453c00 (ACPI NVS) [0.00] BIOS-e820: 7d453c00 - 7d455c00 (ACPI data) [0.00] BIOS-e820: 7d455c00 - 7e00 (reserved) [0.00] BIOS-e820: e000 - f000 (reserved) [0.00] BIOS-e820: fec0 - fed00400 (reserved) [0.00] BIOS-e820: fed2 - feda (reserved) [0.00] BIOS-e820: fee0 - fef0 (reserved) [0.00] BIOS-e820: ffb0 - 0001 (reserved) [0.00] Notice: NX (Execute Disable) protection cannot be enabled in hardware: non-PAE kernel! [0.00] NX (Execute Disable) protection: approximated by x86 segment limits [0.00] DMI 2.5 present. [0.00] DMI: Dell Inc. OptiPlex 755 /0Y255C, BIOS A11 08/04/2008 [0.00] e820 update range: - 0001 (usable) ==> (reserved) [0.00] e820 remove range: 000a - 0010 (usable) [0.00] last_pfn = 0x7d3ff max_arch_pfn = 0x10 [0.00] MTRR default type: uncachable [0.00] MTRR fixed ranges enabled: [0.00] 0-9 write-back [0.00] A-B uncachable [0.00] C-C write-protect [0.00] D-E uncachable [0.00] F-F write-protect [0.00] MTRR variable ranges enabled: [0.00] 0 base 0 mask 0 write-back [0.00] 1 base 07D60 mask FFFE0 uncachable [0.00] 2 base 07D80 mask FFF80 uncachable [0.00] 3 base 07E00 mask FFE00 uncachable [0.00] 4 base 07D50 mask 0 uncachable [0.00] 5 base 08000 mask F8000 uncachable [0.00] 6 disabled [0.00] 7 disabled [0.00] x86 PAT enabled: cpu 0, old 0x7040600070406, new 0x7010600070106 [0.00] original variable MTRRs [0.00] reg 0, base: 0GB, range: 64GB, type WB [0.00] reg 1, base: 2006MB, range: 2MB, type UC [0.00] reg 2, base: 2008MB, range: 8MB, type UC [0.00] reg 3, base: 2016MB, range: 32MB, type UC [0.00] reg 4, base: 2005MB, range: 1MB, type UC [0.00] reg 5, base: 2GB, range: 2GB, type UC [0.00] total RAM covered: 63445M [0.00] gran_size: 64K chunk_size: 64K num_reg: 8 lose cover RAM: 60G [0.00] gran_size: 64K chunk_size: 128Knum_reg: 8 lose cover RAM: 60G [0.00] gran_size: 64K chunk_size: 256Knum_reg: 8 lose cover RAM: 60G [0.00] gran_size: 64K chunk_size: 512Knum_reg: 8 lose cover RAM: 60G [0.00] gran_size: 64K chunk_size: 1M num_reg: 8 lose cover RAM: 60G [0.00] gran_size: 64K chunk_size: 2M num_reg: 8 lose cover RAM: 61439M [0.00] gran_size: 64K chunk_size: 4M num_reg: 8 lose cover RAM: 61438M [0.00] gran_size: 64K chunk_size: 8M num_reg: 8 lose cover RAM: 61438M [0.00] gran_size: 64K chunk_size: 16M num_reg: 8 lose cover RAM: 61432M [0.00] gran_size: 64K chunk_size: 32M num_reg: 8 lose cover RAM: 61432M [0.00] gran_size: 64K chunk_size: 64M num_reg: 8 lose cover RAM: 32G [0.00] gran_size: 64K chunk_size: 128Mnum_reg: 8 lose cover R
[Known BUG?] i915 hang on 3.0.0-12 (Ubuntu 11.10 release)
On Fri, Oct 28, 2011 at 01:56:39PM +0100, Chris Wilson wrote: > On Fri, 28 Oct 2011 20:22:35 +0800, Yong Zhang > wrote: > > Hi, > > > > Just got below error on Ubuntu-11.10 (kernel: 3.0.0-12-generic), > > and after that my screen can't show normally. > > No sure if it's a known issue. > > No, that is the first time I've seen that. It looks as if the fence was > not released, or reacquired, before the the batch was executed. (There > is a later batch that also uses this buffer.) > > The fence is programmed with: > fence[15] = 04e1 > valid, x-tiled, pitch: 512, start: 0x04e0, size: 1048576 > > And the BLT command uses: > 0x0df006b0: 0x5434: XY_COLOR_BLT (rgb enabled, alpha enabled, dst > tile > 0) > 0x0df006b4: 0x03f00100:format , pitch 256, clipping disabled > 0x0df006b8: 0x:(0,0) > 0x0df006bc: 0x00140037:(55,20) > 0x0df006c0: 0x04e0:offset 0x04e0 > 0x0df006c4: 0x:color > 0x0df006c8: 0x54f6: XY_SRC_COPY_BLT (rgb enabled, alpha enabled, > src tile 0, dst tile 0) > 0x0df006cc: 0x03cc0100:format , dst pitch 256, clipping > disabled > 0x0df006d0: 0x:dst (0,0) > 0x0df006d4: 0x00140037:dst (55,20) > 0x0df006d8: 0x04e0:dst offset 0x04e0 > 0x0df006dc: 0x012c0003:src (3,300) > 0x0df006e0: 0x0400:src pitch 1024 > 0x0df006e4: 0x0850:src offset 0x0850 > > So we try to perform an undefined operation and the GPU hangs. I suspect > this will be timing dependent, but if you can find a way to reproduce > it, that would be very useful. Just got it by accident when I'm browsing web with firefox & chromium, and the machine has been running for a long time. No reliable way to reproduce it for now :( Thanks, Yong
