User avatar
pmoore
Posts: 5
Joined: Thu Jun 28, 2018 9:53 am
Location: Neuss, Germany

RGB/BGR pixel order not respected

Tue Jan 21, 2020 1:42 pm

Hi guys, has anyone else stumbled on https://github.com/raspberrypi/firmware/issues/1320 ? I'm not sure at which point it changed, but it looks like sometimes a request for BGR/RGB encoding is not accepted when allocating a framebuffer. Previously, RGB was always accepted, so I'm really not sure if this is a bug or just a nuance. But I'd like to get to the bottom of it, in case I am using a malformed framebuffer allocation mailbox property request...

LdB
Posts: 1429
Joined: Wed Dec 07, 2016 2:29 pm

Re: RGB/BGR pixel order not respected

Tue Jan 21, 2020 2:23 pm

The message detail is wrong ... the response size should be 4 .. your own comments even tells you it responds.
I am guessing it dislikes that you told it that it has a response buffer size of zero.
https://github.com/raspberrypi/firmware ... -interface

Try

Code: Select all

 .word 0x48006  // Tag 4 - Set Pixel Order (really is "Colour Order", not "Pixel Order")
  .word 4   //   request size
  .word 4   //  >>>>  response size ... becomes request result <<<<
  .word 1  //   request: 0 => BGR, 1 => RGB   response: 0 => BGR, 1 => RGB

User avatar
pmoore
Posts: 5
Joined: Thu Jun 28, 2018 9:53 am
Location: Neuss, Germany

Re: RGB/BGR pixel order not respected

Wed Jan 22, 2020 1:28 pm

Thanks Leon. I'm not sure I understand.

The wiki page says:

Tag format:

u32: tag identifier
u32: value buffer size in bytes
u32:
Request codes:
b31 clear: request
b30-b0: reserved
Response codes:
b31 set: response
b30-b0: value length in bytes
u8...: value buffer
u8...: padding to align the tag to 32 bits.

The tag is comprised of 16 bytes (4 words):

Code: Select all

  .word 0x00048006        // tag identifier
  .word 0x00000004        // value buffer size = 4 bytes (1 word)
  .word 0x00000000        // request code: "b31 clear, request / b30-b0: reserved" - so setting 0 (all 32 bits clear)
  .word 0x00000001        // value buffer, 4 bytes, 1 => RGB encoding
In other words, I believe the third word is not the response buffer size, instead the second word is the maximum buffer size of the request and the response since they share the same buffer. At least, if I understand the wiki page correctly and it is accurate, and the third word is required to be 0.

I also tried setting the third word to 4 instead of 0, but it did not fix the issue.

Many thanks in advance!

LdB
Posts: 1429
Joined: Wed Dec 07, 2016 2:29 pm

Re: RGB/BGR pixel order not respected

Wed Jan 22, 2020 2:09 pm

Then it's dorked, unsurprising the setpalette/getpalette has some issues as well.

Look at your pasted Wiki text specifically here
>>> b30-b0: value length in bytes <<
Bit31 is the response the other bits have meaning

Alternatively just look at that value on every other entry in your code block ..
Explain the marked entry?

Code: Select all

mbox[2] = 0x48003; 
mbox[3] = 8;
mbox[4] = 8;  <===  EXPLAIN THIS VALUE .. IT ISN'T ZERO SO WHY SET 8
mbox[5] = 1024;
mbox[6] = 768;

There is only one tag in your entire block you set that tag value to zero :-)

I would also point out the place you reference also sets that value to 4
https://github.com/bztsrc/raspi3-tutori ... ffer/lfb.c

Code: Select all

mbox[21] = 0x48006; //set pixel order
mbox[22] = 4;
mbox[23] = 4;
mbox[24] = 1;           //RGB, not BGR preferably 
he also deals with switching the channel if the call fails

Code: Select all

// the image is in RGB. So if we have an RGB framebuffer, we can copy the pixels
// directly, but for BGR we must swap R (pixel[0]) and B (pixel[2]) channels.
*((unsigned int*)ptr)=isrgb ? *((unsigned int *)&pixel) : (unsigned int)(pixel[0]<<16 | pixel[1]<<8 | pixel[2]);

User avatar
pmoore
Posts: 5
Joined: Thu Jun 28, 2018 9:53 am
Location: Neuss, Germany

Re: RGB/BGR pixel order not respected

Wed Jan 22, 2020 5:13 pm

Hi Leon,

Many thanks again for your reply.
LdB wrote: Then it's dorked, unsurprising the setpalette/getpalette has some issues as well.

Look at your pasted Wiki text specifically here
>>> b30-b0: value length in bytes <<
Bit31 is the response the other bits have meaning
I interpret the wiki page differently, to say that in the *request*, bits 0-30 are reserved, and bit 31 needs to be clear, to indicate it is a request. When the call has completed, bit 31 will be set, to indicate that GPU has processed the tag, and bits 0-30 do indicate the buffer size, but they are set by the GPU in the response, and are not to be set in the request. This tells the caller how many bytes to read for the response, in case the response has a variable length that the caller cannot determine. Note, the length of the response cannot be longer than the length of the request, since it is written to the same buffer that the request was written to.
LdB wrote: Alternatively just look at that value on every other entry in your code block ..
Explain the marked entry?

Code: Select all

mbox[2] = 0x48003; 
mbox[3] = 8;
mbox[4] = 8;  <===  EXPLAIN THIS VALUE .. IT ISN'T ZERO SO WHY SET 8
mbox[5] = 1024;
mbox[6] = 768;
That isn't my code! Maybe this is why you got confused. :-)

I think you got that from a link in the issue, to a different project (written by Zoltan), that (I believe) incorrectly sets the value, since the mailbox property interface wiki page says the in the request, this word should have bit 31 clear, and all the other bits are reserved.
LdB wrote: There is only one tag in your entire block you set that tag value to zero :-)
No, in my code all of the tag request codes are set to 0. See https://github.com/petemoore/redscreen/ ... .s#L45-L83 :-)

I think the point of confusion is you were looking at bzt's code, where he does set the value to the buffer size, which I believe is a relatively harmless mistake (harmless since those bits are reserved in the request, and currently have no meaning, so setting them does no real harm, they are just ignored).
LdB wrote: I would also point out the place you reference also sets that value to 4
https://github.com/bztsrc/raspi3-tutori ... ffer/lfb.c

Code: Select all

mbox[21] = 0x48006; //set pixel order
mbox[22] = 4;
mbox[23] = 4;
mbox[24] = 1;           //RGB, not BGR preferably 
he also deals with switching the channel if the call fails

Code: Select all

// the image is in RGB. So if we have an RGB framebuffer, we can copy the pixels
// directly, but for BGR we must swap R (pixel[0]) and B (pixel[2]) channels.
*((unsigned int*)ptr)=isrgb ? *((unsigned int *)&pixel) : (unsigned int)(pixel[0]<<16 | pixel[1]<<8 | pixel[2]);
The reason Zoltan's code handles both RGB and BGR is precisely because I created an issue in his repository to highlight that his code needs to handle both BGR and RGB encodings, due to the firmware bug we're discussing in this thread. He has implemented that code as a workaround for this very issue. ;-)

See https://github.com/bztsrc/raspi3-tutori ... -550165780

Zoltan confirms he has experienced the same firmware bug too: https://github.com/bztsrc/raspi3-tutori ... -575682919

Also please note that I did try setting all the tag request codes to the buffer length (like Zoltan does) but that it had no effect. It did not fix the issue. So even if I am interpreting the wiki page incorrectly, this cannot be the reason for the failure, otherwise setting the values as you suggest would have fixed it.

It is clear that if the request to use RGB fails, and a framebuffer with BGR is allocated, that calling code will need to handle both RGB and BGR cases, but the issue I'm raising is not that the response says it is BGR/RGB when it isn't, my issue is that despite requesting RGB, we get BGR, whereas in the past, you could request RGB and get RGB.

I hope this has cleared up some of the misunderstanding, but let me know if anything still isn't clear.

Thanks in advance!

Return to “Bare metal, Assembly language”