Hi again,
Sorry for the silence.
Tormod Volden wrote:
> On Fri, Mar 5, 2010 at 8:38 PM, Jonathan Nieder <jrnie...@gmail.com> wrote:
>> Side note: The wildly ill-advised ‘radeontool regmatch '*'’ command
>> locks up the system (it didn’t in version 1.5).
>
> Can you find out which register read locks up? As a wild guess I would
> think more registers have been added and some are not compatible with
> r128.
Apparently version 1.5 didn’t have a ‘regmatch’ command. I don’t know
what I was thinking.
Here are some some patches to answer your questions.
Jonathan Nieder (3):
radeontool regmatch: print name before reading each register
radeontool regmatch: sync() before reading a bunch of registers
Avoid lockup on ‘radeontool regmatch '*'’ on r128
radeontool.c | 25 +++++++++++++++++++++++--
1 files changed, 23 insertions(+), 2 deletions(-)
From: Jonathan Nieder <jrnie...@gmail.com>
Date: Sun, 14 Mar 2010 14:32:37 -0500
Subject: radeontool regmatch: print name before reading each register
Register reads sometimes take a long time or lock up the system.
Let the user know which register is being read before such an event.
Signed-off-by: Jonathan Nieder <jrnie...@gmail.com>
---
radeontool.c | 10 ++++++++--
1 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/radeontool.c b/radeontool.c
index 870c872..426c36e 100644
--- a/radeontool.c
+++ b/radeontool.c
@@ -712,9 +712,15 @@ void radeon_reg_match(const char *pattern)
else {
for (i = 0; i < sizeof(reg_list) / sizeof(reg_list[0]); i++) {
if (fnmatch(pattern, reg_list[i].name, 0) == 0) {
+ printf("%s (%s%04x)\t", reg_list[i].name,
+ reg_list[i].type, reg_list[i].address);
+
+ /* Force output in case the register read locks up. */
+ fflush(stdout);
+ fsync(STDOUT_FILENO);
+
value = reg_list[i].get(reg_list[i].address, reg_list[i].name);
- printf("%s (%s%04x)\t0x%08x (%d)\n", reg_list[i].name,
- reg_list[i].type, reg_list[i].address, value, value);
+ printf("0x%08x (%d)\n", value, value);
}
}
for (i = 0; i < 0x2f; i++) {
--
1.7.0
From: Jonathan Nieder <jrnie...@gmail.com>
Date: Sun, 14 Mar 2010 14:38:02 -0500
Subject: radeontool regmatch: sync() before reading a bunch of registers
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
A hard lock-up is always an unpleasant event, but it can be less
unpleasant if any unrelated pending I/O is finished first. Make
‘radeontool regmatch '*'’ a little safer by calling sync().
Signed-off-by: Jonathan Nieder <jrnie...@gmail.com>
---
radeontool.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/radeontool.c b/radeontool.c
index 426c36e..313ac8f 100644
--- a/radeontool.c
+++ b/radeontool.c
@@ -710,6 +710,9 @@ void radeon_reg_match(const char *pattern)
printf("%s\t0x%08x (%d)\n", pattern, value, value);
}
else {
+ /* Prepare for a crash (just in case). */
+ sync();
+
for (i = 0; i < sizeof(reg_list) / sizeof(reg_list[0]); i++) {
if (fnmatch(pattern, reg_list[i].name, 0) == 0) {
printf("%s (%s%04x)\t", reg_list[i].name,
--
1.7.0
From: Jonathan Nieder <jrnie...@gmail.com>
Date: Sun, 14 Mar 2010 14:52:11 -0500
Subject: Avoid lockup on ‘radeontool regmatch '*'’ on r128
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Running ‘radeontool regmatch '*'’ with an ATI Rage 128 Mobility M3 LF
(AGP) locks up the system.
The register reads that lock up are CRTC2_CRNT_FRAME and
CRTC2_VLINE_CRNT_VLINE; it is not clear to me why. From the
xf86-video-r128 source I can see that this is one of the handful of
r128 chips that supports dual-head operation, but at the time of the
lockup, the CRTC2_GEN_CNTL register had value 0x0 (i.e., CRTC2
disabled); maybe that’s why.
Presumably these two registers would only be relevant if CRTC2 is
enabled, so as a safety measure, check for that before displaying them
with ‘radeontool regmatch [pattern]’.
A similar crash was reported on 2009-12-02 on #radeon for an R300, so
presumably this problem is not restricted to pre-Radeon cards.
Signed-off-by: Jonathan Nieder <jrnie...@gmail.com>
---
radeontool.c | 12 ++++++++++++
1 files changed, 12 insertions(+), 0 deletions(-)
diff --git a/radeontool.c b/radeontool.c
index 313ac8f..8d477e4 100644
--- a/radeontool.c
+++ b/radeontool.c
@@ -710,11 +710,23 @@ void radeon_reg_match(const char *pattern)
printf("%s\t0x%08x (%d)\n", pattern, value, value);
}
else {
+ int crtc2_enabled;
+
/* Prepare for a crash (just in case). */
sync();
+ /* dual-head setup? */
+ value = radeon_get(RADEON_CRTC2_GEN_CNTL, "RADEON_CRTC2_GEN_CTRL");
+ crtc2_enabled = (value & RADEON_CRTC2_EN) ? 1 : 0;
+
for (i = 0; i < sizeof(reg_list) / sizeof(reg_list[0]); i++) {
if (fnmatch(pattern, reg_list[i].name, 0) == 0) {
+ if (!crtc2_enabled &&
+ (reg_list[i].address == RADEON_CRTC2_CRNT_FRAME ||
+ reg_list[i].address == RADEON_CRTC2_VLINE_CRNT_VLINE))
+ /* might freeze, so skip it */
+ continue;
+
printf("%s (%s%04x)\t", reg_list[i].name,
reg_list[i].type, reg_list[i].address);
--
1.7.0
