Design flaw in KosGL

If you have any questions on programming, this is the place to ask them, whether you're a newbie or an experienced programmer. Discussion on programming in general is also welcome. We will help you with programming homework, but we will not do your work for you! Any porting requests must be made in Developmental Ideas.
Post Reply
Chilly Willy
DC Developer
DC Developer
Posts: 414
https://www.artistsworkshop.eu/meble-kuchenne-na-wymiar-warszawa-gdzie-zamowic/
Joined: Thu Aug 20, 2009 11:00 am
Has thanked: 0
Been thanked: 2 times

Design flaw in KosGL

Post by Chilly Willy »

I knew I'd find it eventually. While hunting through the PVR code for any reason why the game would simply stop flipping buffers, I noticed this:

Code: Select all

            if(flags & PVR_TXRLOAD_DMA) {
                mutex_lock((mutex_t *)&pvr_state.dma_lock);
                pvr_txr_load_dma(img->data, dst, img->byte_count,
                                 (flags & PVR_TXRLOAD_NONBLOCK) ? 1 : 0, NULL, 0);
                mutex_unlock((mutex_t *)&pvr_state.dma_lock);
            }
            else if(flags & PVR_TXRLOAD_SQ) {
                sq_cpy(dst, img->data, img->byte_count);
            }
            else {
                /* Store Queue usage here can screw things up if you're
                   loading textures while sending display lists. Enable the
                   PVR_TXRLOAD_SQ flag if you know you want it. */
                memcpy4(dst, img->data, img->byte_count);
                /* pvr_txr_load(img->data, dst, img->byte_count); */
            }
Looking through KosGL, I found this:

Code: Select all

                switch(internalFormat) {
                    case GL_RGB:
                        _glKosPixelConvertRGB(type, width, height, (void *)data, tex);
                        GL_KOS_TEXTURE_UNIT[GL_KOS_ACTIVE_TEXTURE]->color = (PVR_TXRFMT_RGB565 | PVR_TXRFMT_NONTWIDDLED);
                        sq_cpy(GL_KOS_TEXTURE_UNIT[GL_KOS_ACTIVE_TEXTURE]->data, tex, bytes);
                        break;

                    case GL_RGBA:
                        _glKosPixelConvertRGBA(type, width, height, (void *)data, tex);
                        GL_KOS_TEXTURE_UNIT[GL_KOS_ACTIVE_TEXTURE]->color = (PVR_TXRFMT_ARGB4444 | PVR_TXRFMT_NONTWIDDLED);
                        sq_cpy(GL_KOS_TEXTURE_UNIT[GL_KOS_ACTIVE_TEXTURE]->data, tex, bytes);
                        break;
                }

                free(tex);
            }
            break;

            case GL_UNSIGNED_SHORT_5_6_5:   /* Texture Formats that do not need conversion  */
            case GL_UNSIGNED_SHORT_5_6_5_TWID:
            case GL_UNSIGNED_SHORT_1_5_5_5:
            case GL_UNSIGNED_SHORT_1_5_5_5_TWID:
            case GL_UNSIGNED_SHORT_4_4_4_4:
            case GL_UNSIGNED_SHORT_4_4_4_4_TWID:
                sq_cpy(GL_KOS_TEXTURE_UNIT[GL_KOS_ACTIVE_TEXTURE]->data, data, bytes);
                break;
There's no provision for moving textures while moving display lists with the store queue. I quickly swapped all the sq_cpy over to memcpy4 and PING! Everything's peachy. KosGL needs a flag to tell it whether or not to transfer texture data with store queues, memcpy4, or eventually the DMA, like the PVR code.

But that's why the demos all work while the game didn't - different designs. The game can upload textures at any time, while the demos carefully upload apart from any other transfers. It's not a bug, which is why the thread title says 'design flaw'. Maybe pass the flag in the glKosInit()? Or maybe add them to the internal format passed to glTexImage2D()? The latter would be more flexible.
User avatar
BlueCrab
The Crabby Overlord
The Crabby Overlord
Posts: 5652
Joined: Mon May 27, 2002 11:31 am
Location: Sailing the Skies of Arcadia
Has thanked: 9 times
Been thanked: 69 times
Contact:

Re: Design flaw in KosGL

Post by BlueCrab »

I'm guessing you're doing things with multiple threads here? The sq_cpy() function blocks, so that'd be the only way that you'd run into problems unless PH3NOM's libgl sets certain registers ahead of time for transferring display lists and doesn't make sure they haven't changed underneath of it (I can't imagine he does this, but I haven't looked, so I'm only mentioning it as a possibility).

That said, I'm somewhat glad it isn't a problem with KOS proper. I was worried that it was going to be some really ugly problem that would be a pain to fix. :lol:
Chilly Willy
DC Developer
DC Developer
Posts: 414
Joined: Thu Aug 20, 2009 11:00 am
Has thanked: 0
Been thanked: 2 times

Re: Design flaw in KosGL

Post by Chilly Willy »

Actually, as far as I know it's a single thread.

I haven't looked at the display list routines enough to see what the issue could be, but your guess was mine as well. All I know right now is that using memcpy4 works every time while sq_cpy fails within random amount of time, rarely allowing you to play a single level (usually failing in less than one second). Now I can play multiple levels every time.

I proposed the flag because that's what the PVR lib does. It's not always going to be possible to structure a program so that conflicts don't exist (not without a lot of work for certain ports), so you have a flag that allows using a mem copy that doesn't interfere.
User avatar
BlueCrab
The Crabby Overlord
The Crabby Overlord
Posts: 5652
Joined: Mon May 27, 2002 11:31 am
Location: Sailing the Skies of Arcadia
Has thanked: 9 times
Been thanked: 69 times
Contact:

Re: Design flaw in KosGL

Post by BlueCrab »

Well, if there's actually a problem that's causing conflicts in a single-threaded application, that's a bug that needs to be fixed. There shouldn't be any conflicts in that case. In a multithreaded application, appropriate locking could be added to fix it, but once again, that's an issue for libgl. You shouldn't have to resort to using memcpy4 in any case...

I suppose it's possible that he's hooking interrupts and doing things in an IRQ context, but I don't actually think that is the case... Either way, it seems like enough of an issue that it really should be fixed, since most non-trivial tasks will potentially trigger that bug (i.e, anywhere where you don't upload all textures at once before doing any rendering).
Chilly Willy
DC Developer
DC Developer
Posts: 414
Joined: Thu Aug 20, 2009 11:00 am
Has thanked: 0
Been thanked: 2 times

Re: Design flaw in KosGL

Post by Chilly Willy »

Hmm - looking at the kosgl code, I THINK this might be the problem:

Code: Select all

/* Custom version of sq_cpy from KOS for copying vertex data to the PVR */
static inline void pvr_list_submit(void *src, int n) {
    GLuint *d = TA_SQ_ADDR;
    GLuint *s = src;

    /* fill/write queues as many times necessary */
    while(n--) {
        asm("pref @%0" : : "r"(s + 8));  /* prefetch 32 bytes for next loop */
        d[0] = *(s++);
        d[1] = *(s++);
        d[2] = *(s++);
        d[3] = *(s++);
        d[4] = *(s++);
        d[5] = *(s++);
        d[6] = *(s++);
        d[7] = *(s++);
        asm("pref @%0" : : "r"(d));
        d += 8;
    }

    /* Wait for both store queues to complete */
    d = (GLuint *)0xe0000000;
    d[0] = d[8] = 0;
}

/* Custom version of sq_cpy from KOS for copying 32bytes of vertex data to the PVR */
static inline void pvr_hdr_submit(const GLuint *src) {
    GLuint *d = TA_SQ_ADDR;

    d[0] = *(src++);
    d[1] = *(src++);
    d[2] = *(src++);
    d[3] = *(src++);
    d[4] = *(src++);
    d[5] = *(src++);
    d[6] = *(src++);
    d[7] = *(src++);

    asm("pref @%0" : : "r"(d));
}
Notice that it doesn't set QACR0 or 1 while using the store queue. It must be counting on a pvr function leaving the proper value in place. But any use of the texture upload would use sq_cpy which changes both registers.
User avatar
BlueCrab
The Crabby Overlord
The Crabby Overlord
Posts: 5652
Joined: Mon May 27, 2002 11:31 am
Location: Sailing the Skies of Arcadia
Has thanked: 9 times
Been thanked: 69 times
Contact:

Re: Design flaw in KosGL

Post by BlueCrab »

Yep. That indeed looks like it's definitely problematic... Well, at least the problem has been found. :wink:
User avatar
BlueCrab
The Crabby Overlord
The Crabby Overlord
Posts: 5652
Joined: Mon May 27, 2002 11:31 am
Location: Sailing the Skies of Arcadia
Has thanked: 9 times
Been thanked: 69 times
Contact:

Re: Design flaw in KosGL

Post by BlueCrab »

Ok... I'm thinking it should be easy to fix this... Add an "#include <dc/sq.h>" at the top of the file, then the following few lines to the beginning of the glutSwapBuffer() function (before the pvr_list_begin() call at the top):

Code: Select all

#ifndef GL_KOS_USE_DMA
    QACR0 = QACRTA;
    QACR1 = QACRTA;
#endif
Try building that, and see if it fixes the issue without having to resort to memcpy4(). If that works for you guys, I'll go ahead and commit that patch. :wink:

Hint: Do a "make fetch unpack" on libgl in kos-ports, patch the gl-pvr.c file in the dist subdirectory, then do a "make force-install".
Chilly Willy
DC Developer
DC Developer
Posts: 414
Joined: Thu Aug 20, 2009 11:00 am
Has thanked: 0
Been thanked: 2 times

Re: Design flaw in KosGL

Post by Chilly Willy »

I'd say that works. Ran three for three, going a few levels each time.
User avatar
BlueCrab
The Crabby Overlord
The Crabby Overlord
Posts: 5652
Joined: Mon May 27, 2002 11:31 am
Location: Sailing the Skies of Arcadia
Has thanked: 9 times
Been thanked: 69 times
Contact:

Re: Design flaw in KosGL

Post by BlueCrab »

And... the change has been committed. Glad we've wrapped up this annoying bug. Thanks for figuring out what was going on!
Chilly Willy
DC Developer
DC Developer
Posts: 414
Joined: Thu Aug 20, 2009 11:00 am
Has thanked: 0
Been thanked: 2 times

Re: Design flaw in KosGL

Post by Chilly Willy »

Glad to help. That was a tough nut to crack. :)
User avatar
PH3NOM
DC Developer
DC Developer
Posts: 576
Joined: Fri Jun 18, 2010 9:29 pm
Has thanked: 0
Been thanked: 5 times

Re: Design flaw in KosGL

Post by PH3NOM »

Hehe nice thanks guys! And sorry for that problem :oops:

When I saw BlueCrabs first reply
unless PH3NOM's libgl sets certain registers ahead of time for transferring display lists and doesn't make sure they haven't changed underneath of it (I can't imagine he does this, but I haven't looked, so I'm only mentioning it as a possibility).
I knew that the problem was setting the QACR registers, but I have been on vacation in Italy for the last few weeks and have not been keeping up here 8-)

Thanks for finding that bug Chilly and finding a nice fix BlueCrab! :o
User avatar
BlueCrab
The Crabby Overlord
The Crabby Overlord
Posts: 5652
Joined: Mon May 27, 2002 11:31 am
Location: Sailing the Skies of Arcadia
Has thanked: 9 times
Been thanked: 69 times
Contact:

Re: Design flaw in KosGL

Post by BlueCrab »

PH3NOM wrote:Hehe nice thanks guys! And sorry for that problem :oops:

When I saw BlueCrabs first reply
unless PH3NOM's libgl sets certain registers ahead of time for transferring display lists and doesn't make sure they haven't changed underneath of it (I can't imagine he does this, but I haven't looked, so I'm only mentioning it as a possibility).
I knew that the problem was setting the QACR registers, but I have been on vacation in Italy for the last few weeks and have not been keeping up here 8-)

Thanks for finding that bug Chilly and finding a nice fix BlueCrab! :o
It's all good. This is what makes open source so great -- all of us can participate in fixing things when they go wrong. :grin:

I'm just glad that it was easy to fix once we found the issue in the first place. :mrgreen:
Chilly Willy
DC Developer
DC Developer
Posts: 414
Joined: Thu Aug 20, 2009 11:00 am
Has thanked: 0
Been thanked: 2 times

Re: Design flaw in KosGL

Post by Chilly Willy »

Yeah, it was a relief to see it was just a minor issue. Good to see you back!
Post Reply