[parisc-linux] J5000 LCD patch

* [parisc-linux] J5000 LCD patch
@ 2005-03-23 18:55 David Pye
  2005-03-23 20:18 ` Grant Grundler
  0 siblings, 1 reply; 3+ messages in thread
From: David Pye @ 2005-03-23 18:55 UTC (permalink / raw)
  To: parisc-linux


[-- Attachment #1.1.1: Type: text/plain, Size: 1072 bytes --]

Hi,

I've been working on my own LCD patch for the J5000 and other LCD front 
panel-equipped machines. 

My idea is slightly different to some of the other solutions that have been 
submitted recently.  Rather than come up with a complex method of writing a 
different register each time the tasklet is triggered (which still does not 
guarantee that you are waiting the correct min_cmd_delay), I decided to 
remove the tasklet, and use a dedicated work queue to drive this.  The 
primary advantage is that work queue tasks can sleep, whereas tasklets 
cannot. So, it now sleeps the correct amount of time per LCD writes.   I do 
not see this affecting LED based machines, but for LCD based machines, it 
should now give you working heartbeats, and more responsive HDD/network 
symbols also.

It can DEFINITELY be improved further, primarily by increasing the delay 
between rescheduling the work queue item, so reducing CPU util.  I'll submit 
a later patch for that pending feedback on this one, which is of course very 
welcome!

Thanks,

David

[-- Attachment #1.1.2: lcdpatch --]
[-- Type: text/x-diff, Size: 9242 bytes --]

diff -urN vanillabuilt/arch/parisc/kernel/time.c lcdtree/arch/parisc/kernel/time.c

--- vanillabuilt/arch/parisc/kernel/time.c	2005-03-22 20:07:26.000000000 +0000
+++ lcdtree/arch/parisc/kernel/time.c	2005-03-23 18:29:49.000000000 +0000
@@ -88,14 +88,6 @@
 			write_sequnlock(&xtime_lock);
 		}
 	}
-    
-#ifdef CONFIG_CHASSIS_LCD_LED
-	/* Only schedule the led tasklet on cpu 0, and only if it
-	 * is enabled.
-	 */
-	if (cpu == 0 && !atomic_read(&led_tasklet.count))
-		tasklet_schedule(&led_tasklet);
-#endif
 
 	/* check soft power switch status */
 	if (cpu == 0 && !atomic_read(&power_tasklet.count))
diff -urN vanillabuilt/drivers/parisc/led.c lcdtree/drivers/parisc/led.c
--- vanillabuilt/drivers/parisc/led.c	2005-03-22 20:08:25.000000000 +0000
+++ lcdtree/drivers/parisc/led.c	2005-03-23 18:27:40.000000000 +0000
@@ -37,6 +37,7 @@
 #include <linux/proc_fs.h>
 #include <linux/ctype.h>
 #include <linux/blkdev.h>
+#include <linux/workqueue.h>
 #include <asm/io.h>
 #include <asm/processor.h>
 #include <asm/hardware.h>
@@ -46,10 +47,10 @@
 #include <asm/uaccess.h>
 
 /* The control of the LEDs and LCDs on PARISC-machines have to be done 
-   completely in software. The necessary calculations are done in a tasklet
-   which is scheduled at every timer interrupt and since the calculations 
-   may consume relatively much CPU-time some of the calculations can be 
-   turned off with the following variables (controlled via procfs) */
+   completely in software. The necessary calculations are done in a work queue
+   item which is scheduled at every timer interrupt and since the calculations 
+   may consume a relatively large amount of CPU-time, some of the calculations 
+   can be turned off with the following variables (controlled via procfs) */
 
 static int led_type = -1;
 static int led_heartbeat = 1;
@@ -58,6 +59,14 @@
 static char lcd_text[32];
 static char lcd_text_default[32];
 
+
+static int led_wq_created = 0;
+static struct workqueue_struct *led_wq;
+static struct work_struct led_task;
+static void led_work_func(void *);
+static DECLARE_WORK(led_task, led_work_func, NULL);
+
+
 #if 0
 #define DPRINTK(x)	printk x
 #else
@@ -109,13 +118,20 @@
 	.reset_cmd2 =		0xc0,
 };
 
+/* The workqueue must be created at init-time */
+static void start_task(void) {
+	led_wq = create_singlethread_workqueue("led_wq");	
+	queue_work(led_wq, &led_task);
+	led_wq_created = 1;
+}
+
+__initcall(start_task);
 
 /* direct access to some of the lcd_info variables */
 #define LCD_CMD_REG	lcd_info.lcd_cmd_reg_addr	 
 #define LCD_DATA_REG	lcd_info.lcd_data_reg_addr	 
 #define LED_DATA_REG	lcd_info.lcd_cmd_reg_addr	/* LASI & ASP only */
 
-
 /* ptr to LCD/LED-specific function */
 static void (*led_func_ptr) (unsigned char);
 
@@ -285,56 +301,40 @@
 /*
    ** 
    ** led_LCD_driver()
-   ** 
-   ** The logic of the LCD driver is, that we write at every scheduled call
-   ** only to one of LCD_CMD_REG _or_ LCD_DATA_REG - registers.
-   ** That way we don't need to let this tasklet busywait for min_cmd_delay
-   ** milliseconds.
-   **
-   ** TODO: check the value of "min_cmd_delay" against the value of HZ.
    **   
  */
 static void led_LCD_driver(unsigned char leds)
 {
-	static int last_index;	/* 0:heartbeat, 1:disk, 2:lan_in, 3:lan_out */
-	static int last_was_cmd;/* 0: CMD was written last, 1: DATA was last */
-	struct lcd_block *block_ptr;
-	int value;
-
-	switch (last_index) {
-	    case 0:	block_ptr = &lcd_info.heartbeat;
-			value = leds & LED_HEARTBEAT;
-			break;
-	    case 1:	block_ptr = &lcd_info.disk_io;
-			value = leds & LED_DISK_IO;
-			break;					
-	    case 2:	block_ptr = &lcd_info.lan_rcv;
-			value = leds & LED_LAN_RCV;
-			break;					
-	    case 3:	block_ptr = &lcd_info.lan_tx;
-			value = leds & LED_LAN_TX;
-			break;
-	    default:	/* should never happen: */
-			return;
-	}
-
-	if (last_was_cmd) {
-	    /* write the value to the LCD data port */
-    	    gsc_writeb( value ? block_ptr->on : block_ptr->off, LCD_DATA_REG );
-	} else {
-	    /* write the command-byte to the LCD command register */
-    	    gsc_writeb( block_ptr->command, LCD_CMD_REG );
-	}    
-	
-	/* now update the vars for the next interrupt iteration */ 
-	if (++last_was_cmd == 2) { /* switch between cmd & data */
-	    last_was_cmd = 0;
-	    if (++last_index == 4) 
-		last_index = 0;	 /* switch back to heartbeat index */
+	static int lastleds;
+	static unsigned char mask[4] = { LED_HEARTBEAT, LED_DISK_IO,
+		LED_LAN_RCV, LED_LAN_TX };
+	
+	static struct lcd_block * blockp[4] = {
+		&lcd_info.heartbeat,
+		&lcd_info.disk_io,
+		&lcd_info.lan_rcv,
+		&lcd_info.lan_tx
+	};
+	
+	int i;
+	unsigned long wait_jiffies = (lcd_info.min_cmd_delay / 1000000) * HZ;
+	for (i=0; i<4; ++i) {
+		if ((leds & mask[i]) != (lastleds & mask[i])) {
+			gsc_writeb( blockp[i]->command, LCD_CMD_REG );
+			
+			set_current_state(TASK_UNINTERRUPTIBLE);		
+			schedule_timeout(wait_jiffies);	
+
+			gsc_writeb( leds & mask[i] ? blockp[i]->on : 
+					blockp[i]->off, LCD_DATA_REG );
+			
+			set_current_state(TASK_UNINTERRUPTIBLE);		
+			schedule_timeout(wait_jiffies);	
+		}
 	}
+	lastleds = leds;
 }
 
-
 /*
    ** 
    ** led_get_net_activity()
@@ -355,7 +355,7 @@
 
 	rx_total = tx_total = 0;
 	
-	/* we are running as tasklet, so locking dev_base 
+	/* we are running as a work queue item, so locking dev_base 
 	 * for reading should be OK */
 	read_lock(&dev_base_lock);
 	for (dev = dev_base; dev; dev = dev->next) {
@@ -417,7 +417,7 @@
 
 
 /*
-   ** led_tasklet_func()
+   ** led_work_func()
    ** 
    ** is scheduled at every timer interrupt from time.c and
    ** updates the chassis LCD/LED 
@@ -433,10 +433,11 @@
 
 #define NORMALIZED_COUNT(count) (count/(HZ/100))
 
-static void led_tasklet_func(unsigned long unused)
+static void led_work_func(void *unused)
 {
 	static unsigned char lastleds;
 	unsigned char currentleds; /* stores current value of the LEDs */
+	
 	static unsigned long count; /* static incremented value, not wrapped */
 	static unsigned long count_HZ; /* counter in range 0..HZ */
 
@@ -485,11 +486,11 @@
 	    led_func_ptr(currentleds);
 	    lastleds = currentleds;
 	}
+	
+	/* Requeue ourself to run next jiffy */	
+	queue_delayed_work(led_wq, &led_task, 1);
 }
 
-/* main led tasklet struct (scheduled from time.c) */
-DECLARE_TASKLET_DISABLED(led_tasklet, led_tasklet_func, 0);
-
 
 /*
    ** led_halt()
@@ -519,8 +520,14 @@
 	default:		return NOTIFY_DONE;
 	}
 	
-	/* completely stop the LED/LCD tasklet */
-	tasklet_disable(&led_tasklet);
+	/* completely stop the LED/LCD task */
+
+	if (led_wq_created) {
+		if (cancel_delayed_work(&led_task) == 0) 
+			flush_workqueue(led_wq);
+	
+		destroy_workqueue(led_wq);
+	}
 
 	if (lcd_info.model == DISPLAY_MODEL_LCD)
 		lcd_print(txt);
@@ -587,9 +594,6 @@
 	initialized++;
 	register_reboot_notifier(&led_notifier);
 
-	/* start the led tasklet for the first time */
-	tasklet_enable(&led_tasklet);
-	
 	return 0;
 }
 
@@ -624,7 +628,7 @@
    ** lcd_print()
    ** 
    ** Displays the given string on the LCD-Display of newer machines.
-   ** lcd_print() disables the timer-based led tasklet during its 
+   ** lcd_print() disables the workqueue-based led task during its 
    ** execution and enables it afterwards again.
    **
  */
@@ -635,9 +639,11 @@
 	if (!led_func_ptr || lcd_info.model != DISPLAY_MODEL_LCD)
 	    return 0;
 	
-	/* temporarily disable the led tasklet */
-	tasklet_disable(&led_tasklet);
-
+	/* temporarily disable the led task */
+	if (led_wq_created) {
+		if (cancel_delayed_work(&led_task) == 0) 
+			flush_workqueue(led_wq);
+	}
 	/* copy display string to buffer for procfs */
 	strlcpy(lcd_text, str, sizeof(lcd_text));
 	
@@ -654,8 +660,10 @@
 	    udelay(lcd_info.min_cmd_delay);
 	}
 	
-	/* re-enable the led tasklet */
-	tasklet_enable(&led_tasklet);
+	if (led_wq_created) {
+		/* re-enable the led task */
+		queue_work(led_wq, &led_task);
+	}
 
 	return lcd_info.lcd_width;
 }
diff -urN vanillabuilt/include/asm/led.h lcdtree/include/asm/led.h
--- vanillabuilt/include/asm/led.h	2005-03-22 20:09:08.000000000 +0000
+++ lcdtree/include/asm/led.h	2005-03-23 18:27:47.000000000 +0000
@@ -23,9 +23,6 @@
 
 #define LED_CMD_REG_NONE 0		/* NULL == no addr for the cmd register */
 
-/* led tasklet struct */
-extern struct tasklet_struct led_tasklet;
-
 /* register_led_driver() */
 int __init register_led_driver(int model, unsigned long cmd_reg, unsigned long data_reg);
 
diff -urN vanillabuilt/include/asm-parisc/led.h lcdtree/include/asm-parisc/led.h
--- vanillabuilt/include/asm-parisc/led.h	2005-03-22 20:09:08.000000000 +0000
+++ lcdtree/include/asm-parisc/led.h	2005-03-23 18:27:47.000000000 +0000
@@ -23,9 +23,6 @@
 
 #define LED_CMD_REG_NONE 0		/* NULL == no addr for the cmd register */
 
-/* led tasklet struct */
-extern struct tasklet_struct led_tasklet;
-
 /* register_led_driver() */
 int __init register_led_driver(int model, unsigned long cmd_reg, unsigned long data_reg);
 

[-- Attachment #1.2: Type: application/pgp-signature, Size: 189 bytes --]

[-- Attachment #2: Type: text/plain, Size: 169 bytes --]

_______________________________________________
parisc-linux mailing list
parisc-linux@lists.parisc-linux.org
http://lists.parisc-linux.org/mailman/listinfo/parisc-linux

^ permalink raw reply	[flat|nested] 3+ messages in thread