[PATCH 10/10] radeontool: make card detection and mapping errors non-fatal

Jonathan Nieder jrnieder at gmail.com
Fri Feb 24 20:02:53 PST 2012


"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.
Based on a patch by Tormod Volden.

Signed-off-by: Jonathan Nieder <jrnieder at gmail.com>
---
That's the end of the series.  Thanks for reading.

 radeontool.c |   74 ++++++++++++++++++++++++++++++++++++++++------------------
 1 file changed, 51 insertions(+), 23 deletions(-)

diff --git a/radeontool.c b/radeontool.c
index 3ff691fa..024028dc 100644
--- a/radeontool.c
+++ b/radeontool.c
@@ -143,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");
@@ -161,7 +173,8 @@ void radeon_cmd_regs(void)
 {
 #define SHOW_REG(r) printf("%s\t%08x\n", #r, radeon_get(r, #r))
 #define SHOW_CLK_REG(r) printf("%s\t%08x\n", #r, radeon_get_indexed(RADEON_CLOCK_CNTL_INDEX, RADEON_CLOCK_CNTL_DATA, (r), #r, 0x3f))	
-	assert(radeon_cntl_mem);
+	if (!radeon_cntl_mem)
+		exit(-1);
 	SHOW_REG(RADEON_DAC_CNTL);
 	SHOW_REG(RADEON_DAC_EXT_CNTL);
 	SHOW_REG(RADEON_DAC_MACRO_CNTL);
@@ -725,7 +738,8 @@ void radeon_reg_match(const char *pattern)
     unsigned long address;
     unsigned int value;
 
-    assert(radeon_cntl_mem);
+    if (!radeon_cntl_mem)
+        exit(-1);
     if (pattern[0] == '0' && pattern[1] == 'x') {
         address = strtol(&(pattern[2]), NULL, 16);
         value = radeon_get(address, pattern);
@@ -800,7 +814,8 @@ void radeon_reg_set(const char *inname, unsigned int value)
     int i;
     unsigned long address;
 
-    assert(radeon_cntl_mem);
+    if (!radeon_cntl_mem)
+        exit(-1);
     if (inname[0] == '0' && inname[1] == 'x') {
         address = strtol(&(inname[2]), NULL, 16);
         set_reg(inname, "", address, value, radeon_get, radeon_set);
@@ -823,7 +838,8 @@ void radeon_cmd_bits(void)
 {
     unsigned int dac_cntl;
 
-    assert(radeon_cntl_mem);
+    if (!radeon_cntl_mem)
+        exit(-1);
     dac_cntl = radeon_get(RADEON_DAC_CNTL,"RADEON_DAC_CNTL");  
     printf("RADEON_DAC_CNTL=%08x (",dac_cntl);  
     if(dac_cntl & RADEON_DAC_RANGE_CNTL)
@@ -843,7 +859,8 @@ void radeon_cmd_dac(char *param)
 {
     unsigned long dac_cntl;
 
-    assert(radeon_cntl_mem);
+    if (!radeon_cntl_mem)
+        exit(-1);
     dac_cntl = radeon_get(RADEON_DAC_CNTL,"RADEON_DAC_CNTL");
     if(param == NULL) {
         printf("The radeon external DAC looks %s\n",(dac_cntl&(RADEON_DAC_PDWN))?"off":"on");
@@ -862,7 +879,8 @@ void radeon_cmd_light(char *param)
 {
     unsigned long lvds_gen_cntl;
 
-    assert(radeon_cntl_mem);
+    if (!radeon_cntl_mem)
+        exit(-1);
     lvds_gen_cntl = radeon_get(RADEON_LVDS_GEN_CNTL,"RADEON_LVDS_GEN_CNTL");
     if(param == NULL) {
         printf("The radeon backlight looks %s\n",(lvds_gen_cntl&(RADEON_LVDS_ON))?"on":"off");
@@ -881,7 +899,8 @@ void radeon_cmd_stretch(char *param)
 {
     unsigned long fp_vert_stretch,fp_horz_stretch;
 
-    assert(radeon_cntl_mem);
+    if (!radeon_cntl_mem)
+        exit(-1);
     fp_vert_stretch = radeon_get(RADEON_FP_VERT_STRETCH,"RADEON_FP_VERT_STRETCH");
     fp_horz_stretch = radeon_get(RADEON_FP_HORZ_STRETCH,"RADEON_FP_HORZ_STRETCH");
     if(param == NULL) {
@@ -930,8 +949,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, "error: failed to initialise libpciaccess: %s\n",
+		strerror(ret));
+        return;
+    }
 
     match.domain = PCI_MATCH_ANY;
     match.bus = PCI_MATCH_ANY;
@@ -956,17 +978,23 @@ static void map_radeon_cntl_mem(void)
         break;
     }
 
-    if (!device)
-        die("cannot find Radeon device");
+    if (!device) {
+        fprintf(stderr, "error: cannot find Radeon device\n");
+        return;
+    }
 
     ctrl_base = device->regions[2].base_addr;
     ctrl_size = device->regions[2].size;
-    if (!ctrl_size)
-        die("missing ctrl region");
+    if (!ctrl_size) {
+        fprintf(stderr, "error: missing ctrl region\n");
+        return;
+    }
     ret = pci_device_map_range(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, "cannot map ctrl region: %s\n", strerror(ret));
+        return;
+    }
 
     fb_base = device->regions[0].base_addr;
     fb_size = device->regions[0].size;
@@ -981,6 +1009,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;
 }
 
@@ -2950,10 +2980,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;
@@ -3024,6 +3050,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.2



More information about the xorg-driver-ati mailing list