"radeontool --help" tries to map the control region in order to print
dac and light state indicators embedded in the usage message.
Unfortunately that means that running "radeontool --help" without
sufficient privileges errors out without a usage message that would
have hinted at what the tool is for and why it needs root.

        fatal error: cannot map ctrl region: Permission denied

It would be more helpful to write

        error: cannot map ctrl region: Permission denied
        usage: radeontool [options] [command]

followed by a usage message without the dac and light indicators.  Do
so.

Based on a patch by Tormod Volden.

Signed-off-by: Jonathan Nieder <[email protected]>
---
That's the end of the series.  Sorry about the off-by-one error in
patch numbering.  Hopefully the patches were entertaining anyway.

I'd also like to address the -Wformat warnings, but it's getting late,
so probably best to put it off to another day.

Good night,
Jonathan

 radeontool.c |   83 +++++++++++++++++++++++++++++++++++++---------------------
 1 files changed, 53 insertions(+), 30 deletions(-)

diff --git a/radeontool.c b/radeontool.c
index 2c215e5a..c9bafb4c 100644
--- a/radeontool.c
+++ b/radeontool.c
@@ -55,10 +55,8 @@ static unsigned int radeon_get(unsigned long offset, const 
char *name)
     unsigned int value;
     if(debug) 
         printf("reading %s (%lx) is ",name,offset);
-    if(radeon_cntl_mem == NULL) {
-        printf("internal error\n");
-       exit(-2);
-    };
+    if (!radeon_cntl_mem)
+        exit(-1);
 #ifdef __powerpc__
     __asm__ __volatile__ ("lwbrx %0,%1,%2\n\t"
                          "eieio"
@@ -76,10 +74,8 @@ static void radeon_set(unsigned long offset, const char 
*name, unsigned int valu
 {
     if(debug) 
         printf("writing %s (%lx) -> %08x\n",name,offset,value);
-    if(radeon_cntl_mem == NULL) {
-        printf("internal error\n");
-       exit(-2);
-    };
+    if (!radeon_cntl_mem)
+        exit(-1);
 #ifdef __powerpc__
     __asm__ __volatile__ ("stwbrx %1,%2,%3\n\t"
                          "eieio"
@@ -147,13 +143,25 @@ static void radeon_set_mcind(unsigned long offset, const 
char *name,
 
 static void usage(void)
 {
+    const char *dac_status, *light_status;
+
+    if (!radeon_cntl_mem) {
+        dac_status = "";
+        light_status = "";
+    } else {
+        dac_status = (radeon_get(RADEON_DAC_CNTL, "RADEON_DAC_CNTL") & 
RADEON_DAC_PDWN)
+            ? " (off)" : " (on)";
+        light_status = 
(radeon_get(RADEON_LVDS_GEN_CNTL,"RADEON_LVDS_GEN_CNTL") & RADEON_LVDS_ON)
+            ? " (on)" : " (off)";
+    }
+
     fprintf(stderr,"usage: radeontool [options] [command]\n");
     fprintf(stderr,"         --debug            - show a little debug info\n");
     fprintf(stderr,"         --skip=1           - use the second radeon 
card\n");
-    fprintf(stderr,"         dac [on|off]       - power down the external 
video outputs (%s)\n",
-          
(radeon_get(RADEON_DAC_CNTL,"RADEON_DAC_CNTL")&RADEON_DAC_PDWN)?"off":"on");
-    fprintf(stderr,"         light [on|off]     - power down the backlight 
(%s)\n",
-          
(radeon_get(RADEON_LVDS_GEN_CNTL,"RADEON_LVDS_GEN_CNTL")&RADEON_LVDS_ON)?"on":"off");
+    fprintf(stderr,"         dac [on|off]       - power down the external 
video outputs%s\n",
+          dac_status);
+    fprintf(stderr,"         light [on|off]     - power down the 
backlight%s\n",
+          light_status);
     fprintf(stderr,"         stretch [on|off|vert|horiz|auto|manual]   - 
stretching for resolution mismatch \n");
     fprintf(stderr,"         regs               - show a listing of some 
random registers\n");
     fprintf(stderr,"         regmatch <pattern> - show registers matching 
wildcard pattern\n");
@@ -929,8 +937,11 @@ static void map_radeon_cntl_mem(void)
     int i = 0, ret;
 
     ret = pci_system_init();
-    if (ret)
-        die_error(ret, "failed to initialise libpciaccess");
+    if (ret) {
+        fprintf(stderr, "failed to initialise libpciaccess: %s\n",
+               strerror(ret));
+        return;
+    }
 
     match.domain = PCI_MATCH_ANY;
     match.bus = PCI_MATCH_ANY;
@@ -955,33 +966,45 @@ static void map_radeon_cntl_mem(void)
             for (i = 0; i < 6; i++) {
                 if (device->regions[i].size >= 16 * 1024 &&
                     device->regions[i].size <= 64 * 1024) {
-                    if (ctrl_size)
-                        die("cannot distinguish ctrl region");
+                    if (ctrl_size) {
+                        fprintf(stderr, "error: cannot distinguish ctrl 
region\n");
+                        return;
+                    }
                     ctrl_base = device->regions[i].base_addr;
                     ctrl_size = device->regions[i].size;
                 } else if (device->regions[i].size >= 64 * 1024 * 1024) {
-                    if (fb_size)
-                        die("cannot distinguish fb region");
+                    if (fb_size) {
+                        fprintf(stderr, "error: cannot distinguish fb 
region\n");
+                        return;
+                    }
                     fb_base = device->regions[i].base_addr;
                     fb_size = device->regions[i].size;
                 }
             }
-            if (!ctrl_size)
-                die("cannot find ctrl region");
-            if (!fb_size)
-                die("cannot find fb region");
+            if (!ctrl_size) {
+                fprintf(stderr, "error: cannot find ctrl region\n");
+                return;
+            }
+            if (!fb_size) {
+                fprintf(stderr, "error: cannot find fb region\n");
+                return;
+            }
             avivo_device = device;
             break;
         }
     }
 
-    if (!avivo_device)
-        die("cannot find Radeon device");
+    if (!avivo_device) {
+        fprintf(stderr, "error: cannot find Radeon device\n");
+        return;
+    }
 
     ret = pci_device_map_range(avivo_device, ctrl_base, ctrl_size,
                             PCI_DEV_MAP_FLAG_WRITABLE, (void **) &ctrl_mem);
-    if (ret)
-        die_error(ret, "cannot map ctrl region");
+    if (ret) {
+        fprintf(stderr, "error: cannot map ctrl region: %s\n", strerror(ret));
+        return;
+    }
 
     if (pci_device_map_range(avivo_device, fb_base, fb_size,
                             PCI_DEV_MAP_FLAG_WRITABLE, (void **) &fb_mem))
@@ -994,6 +1017,8 @@ static void map_radeon_cntl_mem(void)
                "base framebuffer address is %lx.\n",
                (unsigned long) ctrl_mem, (unsigned long) fb_mem);
 
+    if (!ctrl_mem)
+        die("internal error");
     radeon_cntl_mem = ctrl_mem;
 }
 
@@ -2963,10 +2988,6 @@ void radeon_rom_tables(const char * file)
 
 int main(int argc,char *argv[]) 
 {
-    if(argc == 1) {
-        map_radeon_cntl_mem();
-       usage();
-    }
     while (argc >= 2) {
         if(strcmp(argv[1],"--debug") == 0) {
             debug=1;
@@ -3037,6 +3058,8 @@ int main(int argc,char *argv[])
            radeon_reg_set(argv[2], strtoul(argv[3], NULL, 0));
            return 0;
        }
+    } else {
+        map_radeon_cntl_mem();
     }
 
     usage();
-- 
1.7.9.1

_______________________________________________
xorg-driver-ati mailing list
[email protected]
http://lists.x.org/mailman/listinfo/xorg-driver-ati

Reply via email to