Yes, it's me again ;)
On Sun, 9 May 2010 22:58:30 +0200, Jean Delvare wrote:
> Bisection says that the faulty commit is:
> http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=b4fe945405e477cded91772b4fec854705443dd5
>
> It doesn't revert cleanly from 2.6.34-rc6+, I'll try a manual revert
> tomorrow. My device is a Radeon 9200, so r200, if it helps.
I can confirm that manually reverting the patch above helps somewhat.
The crash is gone. Now I get an essentially black image in glxgears,
which may be a different bug.
(gdb) file drivers/gpu/drm/radeon/radeon.o
Reading symbols from
/home/khali/src/linux-2.6.34-rc6/drivers/gpu/drm/radeon/radeon.o...done.
(gdb) list *(radeon_cp_cmdbuf+0x15e)
0xa58e is in radeon_cp_cmdbuf (drivers/gpu/drm/radeon/radeon_state.c:2921).
2916
2917 drm_radeon_cmd_header_t *header;
2918 header = drm_buffer_read_object(cmdbuf->buffer,
2919 sizeof(stack_header), &stack_header);
2920
2921 switch (header->header.cmd_type) {
2922 case RADEON_CMD_PACKET:
2923 DRM_DEBUG("RADEON_CMD_PACKET\n");
2924 if (radeon_emit_packets
2925 (dev_priv, file_priv, *header, cmdbuf)) {
(gdb)
I took a look at the above commit and found 3 bugs in it, one of which
is directly responsible for my crash. Candidate patch below.
Note 1: Why one would call radeon_cp_cmdbuf() with an empty buffer is
beyond me, but hey, what do I know about graphics drivers.
Note 2: Even with this patch, glxgears is all black unless I move the
window around, so there's one more bug to track. I guess I'm up for one
more bisection afternoon.
* * * * *
From: Jean Delvare <kh...@linux-fr.org>
Subject: radeon: Fix 3 regressions
Commit b4fe945405e477cded91772b4fec854705443dd5 introduced 3 bugs,
fix them:
* Use the right command dword for second packet offset in
RADEON_CNTL_PAINT/BITBLT_MULTI.
* Don't leak memory if drm_buffer_copy_from_user() fails.
* Don't call drm_buffer_unprocessed() unless drm_buffer_alloc() and
drm_buffer_copy_from_user() have been called successfully first.
Signed-off-by: Jean Delvare <kh...@linux-fr.org>
Cc: Pauli Nieminen <suok...@gmail.com>
Cc: Dave Airlie <airl...@redhat.com>
---
I can certainly provide 3 separate patches for clarity if you prefer.
drivers/gpu/drm/radeon/radeon_state.c | 19 ++++++++++---------
1 file changed, 10 insertions(+), 9 deletions(-)
--- linux-2.6.34-rc6.orig/drivers/gpu/drm/radeon/radeon_state.c 2010-05-10
09:20:07.000000000 +0200
+++ linux-2.6.34-rc6/drivers/gpu/drm/radeon/radeon_state.c 2010-05-10
15:19:19.000000000 +0200
@@ -424,7 +424,7 @@ static __inline__ int radeon_check_and_f
if ((*cmd & RADEON_GMC_SRC_PITCH_OFFSET_CNTL) &&
(*cmd & RADEON_GMC_DST_PITCH_OFFSET_CNTL)) {
u32 *cmd3 = drm_buffer_pointer_to_dword(cmdbuf->buffer,
3);
- offset = *cmd << 10;
+ offset = *cmd3 << 10;
if (radeon_check_and_fixup_offset
(dev_priv, file_priv, &offset)) {
DRM_ERROR("Invalid second packet offset\n");
@@ -2895,9 +2895,12 @@ static int radeon_cp_cmdbuf(struct drm_d
return rv;
rv = drm_buffer_copy_from_user(cmdbuf->buffer, buffer,
cmdbuf->bufsz);
- if (rv)
+ if (rv) {
+ drm_buffer_free(cmdbuf->buffer);
return rv;
- }
+ }
+ } else
+ goto done;
orig_nbox = cmdbuf->nbox;
@@ -2905,8 +2908,7 @@ static int radeon_cp_cmdbuf(struct drm_d
int temp;
temp = r300_do_cp_cmdbuf(dev, file_priv, cmdbuf);
- if (cmdbuf->bufsz != 0)
- drm_buffer_free(cmdbuf->buffer);
+ drm_buffer_free(cmdbuf->buffer);
return temp;
}
@@ -3012,16 +3014,15 @@ static int radeon_cp_cmdbuf(struct drm_d
}
}
- if (cmdbuf->bufsz != 0)
- drm_buffer_free(cmdbuf->buffer);
+ drm_buffer_free(cmdbuf->buffer);
+ done:
DRM_DEBUG("DONE\n");
COMMIT_RING();
return 0;
err:
- if (cmdbuf->bufsz != 0)
- drm_buffer_free(cmdbuf->buffer);
+ drm_buffer_free(cmdbuf->buffer);
return -EINVAL;
}
--
Jean Delvare
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
