Why dont my pointer works ?

[Jan Audio]

Hi, i am trying this with XC32 for PIC32MX170F256B :

Code: [Select]

unsigned char screen[ 64 ][ 16 ];

unsigned long long * lcdptr;
unsigned long long * lcdptrPlus127;

// init
lcdptr = ( unsigned long long * )screen;
lcdptrPlus127 = lcdptr + 127;

// ckear function

// clear screen
*(lcdptr++) = 0;

// this part dont works :
if( lcdptr == lcdptrPlus127 )stop clear function.


Problem : the clear function wont stop.
Ofcourse i can add a counter, i would like to use the pointer as counter.

thanks

[andersm]

Could you post your actual code? Also, alignment is important on MIPS CPUs. There is no guarantee that your char array will be to allow it to be accessed as anything but chars. As a sidenote, it's be really difficult to out-perform memset, which has the added bonus of being correct.

[nctnico]

I'd use a counter to make sure. Also a pointer to a long (typically 32 bit) will be incremented by 4 bytes. Besides that try to use types (line uint32_t, in8_t, etc) from std_int.

[Jan Audio]

Could you post your actual code? Also, alignment is important on MIPS CPUs. There is no guarantee that your char array will be to allow it to be accessed as anything but chars. As a sidenote, it's be really difficult to out-perform memset, which has the added bonus of being correct.

So the whole screen is not a solid piece of memory ?
I thought from the first byte to last is all aligned behind eachother,
i am keeping to in the memory range, to 127, instead of going over the memory range ( 128 ), something with pages or something could happen i imagined.

For the memset :
I am trying to spread out the function for in background rendering in main loop, dont stop the main loop for rendering functions since it is writing the backbuffer to screen.
Anyways : this will be faster then memset i think, i can run some tests again for this PIC32, i,m almost sure already.

btw : for clearing the screen is no problem, only the pointer.
Clearing the screen with a counter still works the same, here it only wont stop the function.

I'd use a counter to make sure. Also a pointer to a long (typically 32 bit) will be incremented by 4 bytes. Besides that try to use types (line uint32_t, in8_t, etc) from std_int.

Here its unsigned long long, increments of 8 bytes.

[andersm]

So the whole screen is not a solid piece of memory ?

That's not what I said at all. If the first byte of the array is not allocated on an address evenly divisible by 4, your program will crash.

[Jan Audio]

Ooh, thanks.
Now its getting more complicated with things i dont see in C language.

[sokoloff]

Agree with the above request to post the whole code (or at least the relevant parts, more than you've posted above).

You also have an off-by-one (or off by 8 bytes). The code above, once made to be correct, will never clear the last 8 bytes of the screen array.

You might also consider switching to just a 32 bit type. I doubt the PIC is more efficient at clearing 64 bits at a time, and it might even be slower/higher overhead to do so.

[IanB]

So the whole screen is not a solid piece of memory ?
I thought from the first byte to last is all aligned behind eachother,
i am keeping to in the memory range, to 127, instead of going over the memory range ( 128 ), something with pages or something could happen i imagined.

If you add 127 to a long long pointer, it will increment it by 127 long longs, i.e. 127*8 = 1016 bytes. If you were hoping to advance the pointer by 127 bytes it will not do that.

[sokoloff]

So the whole screen is not a solid piece of memory ?
I thought from the first byte to last is all aligned behind eachother,
i am keeping to in the memory range, to 127, instead of going over the memory range ( 128 ), something with pages or something could happen i imagined.

If you add 127 to a long long pointer, it will increment it by 127 long longs, i.e. 127*8 = 1016 bytes. If you were hoping to advance the pointer by 127 bytes it will not do that.

That part is right (though off-by-one in the last test). Screen is a 64x16 char array, which is 1024 bytes in memory.

[Nusa]

Try:

Code: [Select]

union {
   unsigned long long lcd[ 128 ]; // will be 8-byte aligned by default now
   unsigned char screen[ 64 ][ 16 ]; // by itself, might not have been aligned nicely
}

unsigned long long * lcdptr;
unsigned long long * lcdptrPlus127;

// init
lcdptr = lcd;
lcdptrPlus127 = &lcd[127]; 

...


[Jan Audio]

Here is the code :

Code: [Select]

//------------------------------------------------------------------------------
// y , x * 8 ( 128 x 64 display swapping backbuffer )
//------------------------------------------------------------------------------
struct ScreenBuffer
{
unsigned char data[ 64 ][ 16 ];
};

//------------------------------------------------------------------------------
struct ScreenBuffer*pScreen;
struct ScreenBuffer*pInternalScreen;
struct ScreenBuffer screen1,screen2;

//------------------------------------------------------------------------------
// clear graphical display backbuffer
//------------------------------------------------------------------------------
#define ClearScreen( screen ) lcdtemp = 0; \
lcdptr = ( unsigned long long * )screen; \
do{ *(lcdptr++) = 0; }while( lcdtemp++ != 127 );


void ( *HandleRender )();


// init
pScreen = &screen1;
pInternalScreen = &screen2;

// set pointer for clear
lcdptr = ( unsigned long long * )pScreen->data;
lcdptrPlus127 = lcdptr + 127;

HandleRender = Clear;

//------------------------------------------------------------------------------
// Clear
//------------------------------------------------------------------------------
void Clear()
{
*(lcdptr++) = 0;
*(lcdptr++) = 0;
*(lcdptr++) = 0;
*(lcdptr) = 0;

if( lcdptr++ == lcdptrPlus127 )
  HandleRender = PaintLine;
}


Now let me read the comments, thnx.

[Jan Audio]

Thanks Nusa i tryd that, wont work.

[andersm]

Slight rewrite:

Code: [Select]

void Clear()
{
  *lcdptr = 0;
  lcdptr += 1;
  *lcdptr = 0;
  lcdptr += 1;
  *lcdptr = 0;
  lcdptr += 1;
  *lcdptr = 0;

  if (lcdptr == lcdptrPlus127)
    HandleRender = PaintLine;
  lcdptr += 1;
}
Do you see the problem there? Or, altenatively

Code: [Select]

void Clear()
{
  lcdptr[0] = 0;
  lcdptr[1] = 0;
  lcdptr[2] = 0;
  lcdptr[3] = 0;
  lcdptr += 3;

  if (lcdptr == lcdptrPlus127)
    HandleRender = PaintLine;
  lcdptr += 1;
These two forms are functionally equivalent to the code you posted, and they all have the same problem.

[Jan Audio]

Yes all the same.
Should work right ?,
Maybe i can use a union with unsigned long longs at same memory ?

[andersm]

Should work right ?,

No. Simulate the code on paper if you can't figure it out.

[Jan Audio]

Why dont you tell me the problem please ?

[andersm]

I'm trying to be educational.

Just to prove an earlier point, here's what you get with memset, compiled with -O1. Still sure you can do better?

Code: [Select]

#include <string.h>
#include <stdint.h>

typedef void(*renderfunc)(void);
extern renderfunc pFun;
void otherfunc(void);

extern union {
    uint8_t b[1024];
    uint32_t i[256];
} a;
extern int loop_index;

void clear(void)
{
    memset(&a.i[loop_index], 0, 32);
    loop_index += 8;

    if (loop_index == 256)
        pFun = otherfunc;
}

Code: [Select]

00000000 <clear>:
   0: 8f830000 lw v1,0(gp)
   4: 00032080 sll a0,v1,0x2
   8: 3c020000 lui v0,0x0
   c: 24420000 addiu v0,v0,0
  10: 00441021 addu v0,v0,a0
  14: ac400000 sw zero,0(v0)
  18: ac400004 sw zero,4(v0)
  1c: ac400008 sw zero,8(v0)
  20: ac40000c sw zero,12(v0)
  24: ac400010 sw zero,16(v0)
  28: ac400014 sw zero,20(v0)
  2c: ac400018 sw zero,24(v0)
  30: ac40001c sw zero,28(v0)
  34: 24620008 addiu v0,v1,8
  38: 24030100 li v1,256
  3c: 14430004 bne v0,v1,50 <clear+0x50>
  40: af820000 sw v0,0(gp)
  44: 3c020000 lui v0,0x0
  48: 24420000 addiu v0,v0,0
  4c: af820000 sw v0,0(gp)
  50: 03e00008 jr ra
  54: 00000000 nop

[Jan Audio]

Did you compare it with the unsigned long long filling ?
What would be the solution to my problem without memset ?, dont you have that ?

Set timer output on display, then also for the memset,
i dont know about instructions.

[cv007]

Code: [Select]

#include "Osc.hpp"
#include "Uart.hpp"
#include "Cp0.hpp"
#include <cstdio>
#include <cstdint>
#include <cstring> //memset

uint8_t screen[ 64 ][ 16 ];

// clear screen
//screen must be multiple of 128bytes
void cls(){
    uint64_t* p = (uint64_t*)screen;
    for(uint32_t i = sizeof(screen); i; i-=128){
        *p++ = 0;*p++ = 0;*p++ = 0;*p++ = 0; //4*8=32bytes
        *p++ = 0;*p++ = 0;*p++ = 0;*p++ = 0; //4*8=32bytes
        *p++ = 0;*p++ = 0;*p++ = 0;*p++ = 0; //4*8=32bytes
        *p++ = 0;*p++ = 0;*p++ = 0;*p++ = 0; //4*8=32bytes
    }
}

//tx only
Uart info { Uart::UART2TX, Pins::C6, 230400 };
//so I can use printf with 'info' as the output
extern "C" void _mon_putc(char c){ info.putchar(c); }

int main()
{
    Osc osc;
    osc.pll_set ( osc.MUL12, osc.DIV4 );
    info.on ( true );

    printf( "\n\n&screen: 0x%04X\n", (uint32_t)screen );
    uint32_t t1 = Cp0::count();
    cls();
    uint32_t t2 = Cp0::count();;
    memset(screen, 0, sizeof(screen));
    uint32_t t3 = Cp0::count();
    printf( "cls: %d  memset: %d\n", t2-t1, t3-t2 );

    for(;;);
}
/*

&screen: 0x8000004C
cls: 154  memset: 212

*/
It also looks to me that xc32 will always align an array on 4bytes. If you create some vars as bytes, the addresses will of course only be byte aligned, but the minute you create a byte array of 1 or more,
it seems to always be 4byte aligned. I don't know the rules, but that is what seems to happen. I tried to get 'screen' unaligned, but could not.

I am trying to spread out the function for in background rendering in main loop, dont stop the main loop for rendering functions since it is writing the backbuffer to screen.

If you want screen buffer cleared while doing something else, use DMA.

[richardman]

You ClearScreen() looks reasonably correct, but I think you must have other code here. If I have to guess, you are being clever using RenderHandler to first to set to Clear, and you would call it a number of times, and then when it clears the whole buffer, you set RenderHandler to some other "real" drawing routine.

Without seeing your other code, we have no other what else you are doing. My advice is that it could be something simple, but generally, it looks a bit too "clever for clever" sake. Just have a ClearScreen() macro/function that you call to clear the whole screen, then another function to draw.

Basically, pointers work, and if you code doesn't, there is something you do not understand, and it's hard to guess what it is.

[hamster_nz]

It looks like you are trying to incrementally clean the buffer, four words at a time.

Are you sure that your pointer is aligned as you expect at the start?

Otherwise you need to test to see if you are at the end of the buffer more often.

Code: [Select]

void Clear()
{
  int i;
  for(i = 0; i < 4; i++) {
    *lcdptr = 0;
    if (lcdptr == lcdptrPlus127) {
      HandleRender = PaintLine;
      return;
    }
    lcdptr++;
  }
}

It seems to be doing a very small amount of work to do each time though....

[Jan Audio]

If you want screen buffer cleared while doing something else, use DMA.

Thanks, here i am intrested.
So i can use DMA for anything not only peripherals ?, nice.
I need to generate some DMA code in MCC then,
could i do my update routine also in another DMA ?, that would be even better.

Just have a ClearScreen() macro/function that you call to clear the whole screen, then another function to draw.

It will take to long, and it will make my MIDI in signal be handled to late, also the screen updating routine will be delayed.
Ofcourse like you say would be simpler, then i would need other things interrupt based, while MIDI data can not be handles in the interrupt, then it might be to slow to receive the next byte.

It looks like you are trying to incrementally clean the buffer, four words at a time.

Are you sure that your pointer is aligned as you expect at the start?

Otherwise you need to test to see if you are at the end of the buffer more often.

It seems to be doing a very small amount of work to do each time though....

Yes,yes, also yes small ammount to keep the main loop running,
no problems, the screen updates a bit more then 20 frames per second with total redraw,
enough time to clean and draw some lines and circles piece by piece.

The only problem is with these sucky LCDs that the update is to fast for the pixels to show : a moving line is very vague.

Anyways if it should work i will try some more, thanks.
It is not compicated to make it complicated, it is very simple only not so common.

I,m going to add some variables to the union for comparing.

[bsudbrink]

I've read through this thread twice now and, since no one explicitly told you what your problem was, I will.  Mr. andersm hinted around it, but you are hitting a problem that many novice (no offense) C programmers encounter.  In the code in message 10, in the Clear function, the post-increment instruction in the if statement:

if( lcdptr++ == lcdptrPlus127 )

does not behave as you seem to expect.  The behavior is a bit "fuzzy".  In several of the C language definitions, in the descriptions of post-increment, you will find statements like:

The increment will take place at the end of the execution unit.

That leaves you wondering, "what is an execution unit"?  I won't go any further other than to tell you the the increment takes place after the compare.  You can go read the language definitions yourself if you are having trouble sleeping or enjoy headaches.  I hope you can logically see that, due to this behavior, the if condition will never be true.

[richardman]

...In the code in message 10, in the Clear function, the post-increment instruction in the if statement:

if( lcdptr++ == lcdptrPlus127 )

does not behave as you seem to expect.  The behavior is a bit "fuzzy". ...

Not sure exactly what the OP's problem is, but this statement is "misleading"  The C language is quite clear on what this statement means:

The initial (pre-incremented) value of lcdptr is compared to lcdptrPlus127 and the result of the comparison is the (boolean) value of the test expression. lcdptr is incremented.

The only thing "fuzzy" is when does the increment happens, which has no bearing whatsoever.

[bsudbrink]

...In the code in message 10, in the Clear function, the post-increment instruction in the if statement:

if( lcdptr++ == lcdptrPlus127 )

does not behave as you seem to expect.  The behavior is a bit "fuzzy". ...

Not sure exactly what the OP's problem is, but this statement is "misleading"  The C language is quite clear on what this statement means:

The initial (pre-incremented) value of lcdptr is compared to lcdptrPlus127 and the result of the comparison is the (boolean) value of the test expression. lcdptr is incremented.

The only thing "fuzzy" is when does the increment happens, which has no bearing whatsoever.

What about this:

if ((lcdptr++) == lcdptrPlus127)