Andrea Arcangeli (andrea@e-mind.com)
Tue, 15 Dec 1998 22:28:09 +0100 (CET)
On Tue, 15 Dec 1998, Tim Waugh wrote:
>Hi Andrea,
>
>> I don' t need to say that the CPU get very lighter efficiently used
>> and being far to lose printing performance...
>
>For me, the load is 0.00 while printing.
Hmm how long is your printing in time? ;). /proc/loadavg needs 5 minutes
of printing to show up a reliable value.
>> +#if 0 /* this sucks every kind of irq handshake */
>> /* 500usecs of fast polling.
>> *
>> * This should be adjustable.
>> @@ -62,6 +63,7 @@
>> break;
>> udelay(25);
>> }
>> +#endif
>>
>> /* 40ms of slow polling. */
>> init_timer (&timer);
>
>Doesn't that kill polling? And interrupt-driven operation, for that matter?
Hmm yes probably polling would not be very performant...
>I'm _sure_ that polling fast to start off with will get good results, and
>isn't a noticeable performance loss. (Remember, 0.00 load.)
Tim, now I don' t trust your 0.00 load because your stock patch was
causing my X to stall during the printing and the machine was really
overloaded. Probably you have a different hardware and the 0 load can' t
be generalized. But I still suggest you to monitor the load of the machine
using top, procinfo -d -n1 (to know the irq/sec), and procmeter CPU field.
>This looks dubious. All it means, if the semaphore is signalled, is that
>fast polling may have worked before resorting to interrupts. If that were
>_never_ true for anyone, perhaps I'd agree with your patch that takes out
>fast polling.
I can' t understand very well here, but I agree that my brute patch could
have killed polling (that I never use btw). But I am pretty sure that irq
printing need the code commented out to be rasonable efficient at least on
Stylus Color (and I guess on many other new printers). Can somebody out
there confirm this on some other hardware?
So the right incremental fix should be this:
Index: linux/drivers/misc/parport_ieee1284.c
diff -u linux/drivers/misc/parport_ieee1284.c:1.1.1.1.2.3 linux/drivers/misc/parport_ieee1284.c:1.1.1.1.2.5
--- linux/drivers/misc/parport_ieee1284.c:1.1.1.1.2.3 Tue Dec 15 16:42:25 1998
+++ linux/drivers/misc/parport_ieee1284.c Tue Dec 15 22:36:36 1998
@@ -46,24 +46,25 @@
struct timer_list timer;
typedef void (*tm_func) (unsigned long);
-#if 0 /* this sucks every kind of irq handshake */
- /* 500usecs of fast polling.
- *
- * This should be adjustable.
- * How about making a note (in the device structure) of how long
- * it takes, so we know for next time?
- */
- for (counter = 0; counter < 20; counter++) {
- status = parport_read_status(port);
- if ((status & mask) == result)
- return 0;
- if (signal_pending (current))
- return -EINTR;
- if (current->need_resched)
- break;
- udelay(25);
+ if (!(port->cad->flags & PARPORT_DEV_IEEE1284IRQ))
+ {
+ /* if polled do 500usecs of fast polling.
+ *
+ * This should be adjustable.
+ * How about making a note (in the device structure)
+ * of how long it takes, so we know for next time?
+ */
+ for (counter = 0; counter < 20; counter++) {
+ status = parport_read_status(port);
+ if ((status & mask) == result)
+ return 0;
+ if (signal_pending (current))
+ return -EINTR;
+ if (current->need_resched)
+ break;
+ udelay(25);
+ }
}
-#endif
/* 40ms of slow polling. */
init_timer (&timer);
>What's up with that? The 'count' is there so that it doesn't do it the
>first time, incidentally -- that means we can see if the driver's
>giving us interrupts.
-- > +#if 0 > /* Yield the port for a while. */ > if (count && polling (dev)) > parport_release (dev); > +#endif > [...] > +#if 0 > if (count && polling (dev)) > parport_claim_or_block (dev); > +#endif--Hmm, yes, probably I have not interpreded well the point of the timeout handling code there. If I understand well such code should be emergency code. Right? My point was that we must release the port only according to parport_yield_blocking to be efficient and assure a good timeslice for every pardevice...
> >> /* Wait longer next time. */ >> - if (wait < (port->ieee1284.timeout / 2)) >> - wait *= 2; >> + if (wait < (port->ieee1284.timeout >> 1)) >> + wait <<= 1; > >Can't the compiler figure that out? Anyway, I think the 2s make the code >easier to read.
Yes, the patch is pure estethic.
Andrea Arcangeli
-- To unsubscribe, send mail to: linux-parport-request@torque.net -- -- with the single word "unsubscribe" in the body of the message. --
This archive was generated by hypermail 2.0b3 on Wed 30 Dec 1998 - 10:18:55 EST