Remote LCD Display Code Suggestion

[Eriol]

In the recent (today's) update, there was some preliminary writer code committed.  Awesome.  Hope to test some of it myself one of these days.  But there was one thing that struck me as a bit... inefficient: every time you write one byte, there's a function call.  Now for the commands, I can live with it, as those are generally short sequences anyways.  But it's the array one that I believe could be improved so that you don't have to jump in/out of functions for every single byte.  Though at the same time, I think that inlining EVERY call to updating the display could get rather large in the code.  The command and single-writes do NOT need to be inlined.  And I don't want two copies of the functional part of the remote_write() function, one inline, one not in the code.

So I propose a non-inline "wrapper" to an inline functional part.  What I mean is below:

Code: [Select]

// Taken from: * $Id: lcd-remote-x5.c,v 1.1 2006/07/26 13:38:31 linus Exp $
// ... Code above omitted

static void remote_write(unsigned char byte, bool is_command)
{
    remote_write_inlined(byte, is_command);
}

inline static void remote_write_inlined(unsigned char byte, bool is_command)
{
    int i;
  
    CS_LO;
    if (is_command)
  	RS_LO;
    else
        RS_HI;
  
    for (i = 0x80; i; i >>= 1)
    {
        CLK_LO;
        if (i & byte) 
            DATA_HI;
        else
            DATA_LO;
        CLK_HI;
    }

    CS_HI;
}

// Other functions here omitted

void lcd_remote_write_data(const unsigned char* p_bytes, int count)
{
    while(count--)
        remote_write_inlined(*p_bytes++, false);
}
// ... Rest of code after this.

Basically, this is all that would change.  A non-inline "wrapper" that would compile out to EXACTLY what we have now, and inlining the function where it is called continually.  We keep only one block of text in the source that does actual writing, but preserve the efficiency of an inline function, while also not polluting ALL the calls with inlines.  And the rest of the code "should" work exactly the same.

I don't have an environment set up (I really do have to get around to that... keep bugging me.  ), but if somebody else could check this out, and/or give an opinion on it, please speak up.  I just see making a function call on every write in a continual write operation as grossly inefficient, and offending my coding sensibilities, whereas a macro clearly could create other problems.  I think this is a better solution.

Edit: bah.  Missed a bracket on original post, and 2nd edit is for another typo.

[LinusN]

Thanks for the heads up.

The X5 remote LCD code is just a proof-of-concept to get the ball rolling. We will soon change it to the asm optimized version seen in firmware/drivers/lcd-h100-remote.c, which is far better.

[Eriol]

The X5 remote LCD code is just a proof-of-concept to get the ball rolling. We will soon change it to the asm optimized version seen in firmware/drivers/lcd-h100-remote.c, which is far better.

Ah, I see that.  They're both 68k processors, so shouldn't be radically different (hopefully).  Been a while since I did 68k asm myself, so I'll leave that for others until I re-aquaint myself.

Thanks for looking at it.

[jhMikeS]

This was something I kind of looked forward to working on but I don't want to duplicate efforts (got other stuff to wrap up too). There's no button interface yet either (right)?

We're going to use all four gray shades I hope. I didn't actually look yet if it does. 

[LinusN]

This was something I kind of looked forward to working on but I don't want to duplicate efforts (got other stuff to wrap up too). There's no button interface yet either (right)?

The button driver is there and it works. The only thing that is missing is mapping the buttons to actions. That will be done as soon as we commit the new action-based button handling.

We're going to use all four gray shades I hope. I didn't actually look yet if it does. 

Yes it does.