On Thursday 01 March 2007 20:11, Aapo Tahkola wrote:
> On Thu, 1 Mar 2007 17:46:12 +0200
>
> Panagiotis Papadakos <[EMAIL PROTECTED]> wrote:
> > Diving more into the code, I also found weird how the scratch cmd
> > packet is build in radeon_mm_use, in radeon_mm.c. I think it should
> > be like in the attached patch.
>
> This patch will break 64 bit systems. It only works on your system
> because drm cuts 32 bits off the pointer.
> This is what drm expects to see:
> 64_bit_pointer
> for every buffer
> 32_bit_index
>
> Thus, rmesa->rmm->u_list[id].age is at 64_bit_pointer[32_bit_index] .
> Since age and h_pending go in pairs, rmesa->rmm->u_list[id].h_pending
> is at 64_bit_pointer[32_bit_index + 1] .
>
> Hope this clears out things a bit.
Yep. I think you are right.
Two questions though:
1) Should we increment h_pending, before memcpy to the buf?
(Is there any way that the drm reads the old value?)
2) Could you also see the attached patch, for the drm r300_scratch?
I think that the logic is wrong. Before the loop, you increment the buf,
8 bytes, although you should already be reading age and h_pending,
since the outer while loop removes the head.
Somehow all the loop seems a bit weird to me.
P.S.
Before applying this patch I was getting many messages like this one:
Feb 27 15:48:53 localhost kernel: [drm:drm_ioctl] pid=3538, cmd=0x40046457,
nr=0x57, dev 0xe200, auth=1
Feb 27 15:48:53 localhost kernel: [drm:drm_ioctl] ret = fffffffc
which I have not seen after applying this patch.
Thanks for your time.
diff --git a/shared-core/r300_cmdbuf.c b/shared-core/r300_cmdbuf.c
index 0c04b5f..09b4a35 100644
--- a/shared-core/r300_cmdbuf.c
+++ b/shared-core/r300_cmdbuf.c
@@ -720,10 +720,10 @@ static int r300_scratch(drm_radeon_priva
drm_r300_cmd_header_t header)
{
u32 *ref_age_base;
- u32 i, buf_idx, h_pending;
+ u32 i, h_pending;
RING_LOCALS;
- if (cmdbuf->bufsz < sizeof(uint64_t) + header.scratch.n_bufs * sizeof(buf_idx) ) {
+ if (cmdbuf->bufsz < header.scratch.n_bufs * sizeof(uint64_t) + sizeof(u32)) {
return DRM_ERR(EINVAL);
}
@@ -732,42 +732,41 @@ static int r300_scratch(drm_radeon_priva
}
dev_priv->scratch_ages[header.scratch.reg] ++;
-
+
ref_age_base = (u32 *)(unsigned long)*((uint64_t *)cmdbuf->buf);
-
- cmdbuf->buf += sizeof(uint64_t);
- cmdbuf->bufsz -= sizeof(uint64_t);
-
+
for (i=0; i < header.scratch.n_bufs; i++) {
- buf_idx = *(u32 *)cmdbuf->buf;
- buf_idx *= 2; /* 8 bytes per buf */
-
- if (DRM_COPY_TO_USER(ref_age_base + buf_idx, &dev_priv->scratch_ages[header.scratch.reg], sizeof(u32))) {
+
+ if (DRM_COPY_TO_USER(ref_age_base + i * 2, &dev_priv->scratch_ages[header.scratch.reg], sizeof(u32))) {
return DRM_ERR(EINVAL);
}
-
- if (DRM_COPY_FROM_USER(&h_pending, ref_age_base + buf_idx + 1, sizeof(u32))) {
+
+ if (DRM_COPY_FROM_USER(&h_pending, ref_age_base + i * 2 + 1, sizeof(u32))) {
return DRM_ERR(EINVAL);
}
-
+
if (h_pending == 0) {
- return DRM_ERR(EINVAL);
+ return DRM_ERR(EINVAL);
}
-
+
h_pending--;
-
- if (DRM_COPY_TO_USER(ref_age_base + buf_idx + 1, &h_pending, sizeof(u32))) {
+
+ if (DRM_COPY_TO_USER(ref_age_base + i * 2 + 1, &h_pending, sizeof(u32))) {
return DRM_ERR(EINVAL);
}
-
- cmdbuf->buf += sizeof(buf_idx);
- cmdbuf->bufsz -= sizeof(buf_idx);
+
+ /* 8 bytes per buf */
+ cmdbuf->buf += sizeof(uint64_t);
+ cmdbuf->bufsz -= sizeof(uint64_t);
}
BEGIN_RING(2);
OUT_RING( CP_PACKET0( RADEON_SCRATCH_REG0 + header.scratch.reg * 4, 0 ) );
OUT_RING( dev_priv->scratch_ages[header.scratch.reg] );
ADVANCE_RING();
+
+ cmdbuf->buf += sizeof(u32);
+ cmdbuf->bufsz -= sizeof(u32);
return 0;
}
-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys-and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
--
_______________________________________________
Dri-devel mailing list
Dri-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/dri-devel
