-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Add new property/value mailbox channel #47
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
I'd suggest including the length of each property and the size of the request/response buffer, so that it's not limited to predefined 32 bit values. The buffer could also hold multiple properties in a TLV format. For command line, serial number and board revision I think these would arrive too late in the boot process if the mbox driver had to be used to get them. The command line and memory size/reserved memory must be passed through the device tree file. As the MAC address is static it can go in the device tree file too. For power control the easiest option is to define a unique 32-bit identifier for each device that can be turned on or off, as this seems to work quite well in the device tree file (currently I use the value to define the bit of the power state array). I'd also prefer to always get a response, it's easier to return success/failure that way... For the clocks there should be properties for getting the current state (enabled/disabled), rate, min rate, max rate. Instead of having to define multiple tags for each of these there could be CLOCK_ENABLE, CLOCK_GET_MIN_RATE etc. that had a unique clock identifier as part of the value. I also need to know which clocks are parents of the other clocks (so that any power saving disabling of clocks doesn't have unintended side effects). See include/linux/clk-provider.h for what the clock interface expects. The enable/disable functions can't sleep... I can implement polled mode mbox calls but it'd need to have a response in the mailbox as soon as I've written the request. How much of the mbox/bell/property interface is generic to all BCM2835 and how much of it is Raspberry Pi specific? |
Definitely the right sort of thing. Could you add SetProperty for framebuffer_ignore_alpha=1 then there would be no need to know about what is in kernel .img beforehand? |
I'd prefer not to have any access to config.txt at all because this information could be out of date. For example, we could have changed the clock rates since then (and forgotten that we did so because this might be a new kernel executed with kexec). You'd end up having to define two ways to access the same data. |
Yes, you will still get the device tree options. It's useful to have an alternative mechanism for non-linux/devtree users.
Well you'll have to poll for GPU to respond. I'd imagine that was typically in the tens of microseconds range, but could be worse when we are busy (I'd have thought well under 1ms)
|
Example get request: Example get response: Some sort of tag for "error, buffer is too small" would also be appropriate. |
Yes, there's less reason for linux to use this, with devtree/ATAGs. However it could be useful for non-linux (e.g. RISCOS) so it seems reasonable to support it in firmware, but not make use of it in linux. |
With this scheme you don't actually need two mbox channels, as the tag id indicates get/set. The response message should probably be the same value (memory location) as the request message so that it can be used asynchronously with multiple buffers. |
My point is that it gets out of date quickly as config.txt configures the VC. For example, if the OS queries for a config.txt property and it's not there it would need to know what the default is, which could vary based on firmware version. If it always retrieved the current data instead it wouldn't need to do this. |
Can you confirm that the same buffer is shared for the request and response? Feel free to specify the data format for any commands you care about. |
Yes, the same buffer would be shared for the request and response. The mbox values could a status code in the lower bits if we increased the alignment size of the buffer. Could you enable the wiki for this repository so that a format could be documented? |
Should be done. |
lp0: access to command line and memory sizes etc within RISCOS via this mechanism would be brilliant.. we do have sufficient control of the boot process that we can request this information in good time to make use of it. |
Here's a first attempt at defining a format: https://github.com/raspberrypi/firmware/wiki/Mailbox-property-interface I've left out the extra frame buffer tags for rosery to define. Depth should probably be replaced with a tag defining the whole fb_info format. I don't know if this is going to be too verbose or not, but there's plenty of memory and a 4K page could let you get 200+ properties at once. Update: I've specified that the value is padded to make the next tag 32-bit aligned. |
@lp0 I have added tags for the 2 most critical RISCOS things.(I hope I've done it correctly). Were you proposing that the current framebuffer request channel be redundant? if so, we probably would benefit from a tag that gave and received all the frame buffer elements in 1, including the bytes per line, which can be significantly greater than the pixels per line. (this is currently in the standard fb channel request. Secondly, checking my understanding, in channel 8, the buffer used is presumably always within arm space, provided by the arm.. |
Yes, it already can't be extended without breaking compatibility with existing kernels.
The request can include multiple tags that would get processed together.
Yes |
On 17/06/2012 16:02, Simon Arlott wrote:
OK.. there are a couple more tags we need to add, then, to make sure we John John Ballance C.Eng MIET - [email protected] - 07976 295923
|
get timing has a response length of 8, but only u32 is described |
I've added all the other values that are currently used and fixed the sizes. I've also added test versions of the setter properties so that new display settings can be proposed before setting them, and removed the set buffer option as it should be unnecessary if the settings are tested first. |
Added a tag to release the buffer and included the alignment in the get buffer request. |
I'm not overly keen on the framebuffer tag interface. |
I agree that the other tags are easier to work with but the frame buffer is more complex. A single tag becomes limiting when it needs to be extended. Yes, any order of response is ok - it'll just scan through them to get the ones it wants. The same tag shouldn't occur twice in the request so the behaviour in that scenario would be undefined. The same tag twice in the response also makes the result undefined. Using the last value in each case would be the likely outcome. Yes, in Set mode it would refuse to change anything. In Test mode it should try to correct the values to the nearest limit or not return them if there's nothing it can do to fix it. |
@popcornmix @lp0 Doing the whole frame buffer in 1 tag would fit my needs well What I would do, probably, is set/specify framealignment, PixelOrder and Alphamode individually, as these would need doing just once (in a while), whereas all the elements of framebuffer would need altering in unison for a screen mode change. Read/Modify/Write is fine.. |
I've modified the clock tags to specify that it always returns a tag even if the clock doesn't exist so that it's easier to process the response if someone does request the same clock rate is set multiple times... |
But now you've got some parts of it that get set once and some that get set every time you make a change and the result if you change both of them at the same time isn't defined (can one of those changes fail but the other succed?). |
Can I assume you won't mix test/get/set calls in a single message? Suppose you have a framebuffer in use that you have previously called get buffer on. |
Yes, mixing test and set is invalid. Test and get would be ok in theory but I can see that being difficult to process.
Yes, I'd expect get to return after all the set operation is finished.
If you don't include a get buffer tag then the buffer is not allowed to change base or size so the request would fail. If a get buffer tag is included then the new buffer is returned when the change is successful. I've renamed "get buffer" to "allocate buffer". |
Okay, so a sequence of sets and a get buffer implicitly frees the old buffer. |
OK.. Address : 3 2 1 0 7 6 5 4 B A 9 8 F E D C 3 2 1 0 7 6 5 4 B A 9 8 F E D C Address : 3 2 1 0 7 6 5 4 B A 9 8 F E D C 3 2 1 0 7 6 5 4 B A 9 8 F E D C tag 40001 sent alignment of 0x100000 in first 4 bytes. this is all it sends so the count in the preceeding word ought (as I read it) to be 4 (actually it seems that either 4 or 8 works, though surely 4 is the required value?) tag 40001 received a byte count of 8 - correct- with first parameter 7f8000 -which is correct for the size of 780x438x20 (hex)- BUT this should be thr frame buffer start address. Second parameter (word) received is 0, whereas it should receive the contents of the first parameter, 7f8000 Nearly there, but fixing this will enable the use of the message channel to actually get frame buffers!! Many thanks.. John |
Whilst I note you don't request the pitch/stride, 780*4 = 1E00, which doesn't look like it needs rounding up. If the stride is 1E00 then 438 lines would be 7E9000, not 7F8000. I'd be more tempted to assume that the reply is following the standard for "If the requested alignment is unsupported..." Or have I made a mistake in my maths? |
I didn't request the pitch as at this stage I'm far more interested in gleaning the buffer address .. then I can use it. also in getting the correct response to the resolutions .. that way I would expect the test parameters to work. As for the math, 7E9000 is mathematically correct .. result of (decimal) 1920 * 1080 * 4. I checked what was returned in the mailbox channel 1 - frame buffer, and the same error exists. For this it reports a pitch of 0x1e00, as expected, and a frame buffer length of 0x7f8000 I'll put the pitch request in and see what happens .. ... ... it isnt yet implemented .. flagged as not responded to.. I put in 0x00040008 0x4 0x0 0x0. the response buffer has these unchanged .. i.e. no hi bit set in the 3rd word. @popcornmix this math seems odd.. can you confirm what you expect? Many thanks to you both |
Fixed the framebuffer base/size being written to same word. Not sure why you fill in request length for allocate as 8. The spec says 4. |
Hi John Ballance C.Eng MIET - [email protected] - 07976 295923
thanks
I take it this is a test image.. will these changes make it out on the
no set_pitch or test_pitch.. agreed, but yes get_pitch .. it was
thanks for the explanation .. does that mean we'll have to specify
which is what I expected, though some confusion is around. Again, thanks Thanks John
|
@popcornmix Thanks for the dropbox bit above. Yes it returns consistent values for all I've now tried except the get pitch call. Do you propose to implement that? .. needed I believe, to deal with some of the more awkward modes .. or at least permit clients to sort themselves out without specific knowledge of how you allocate in the vc side |
I've updated dropbox file. It might handle get pitch now. |
That is brilliant.. pitch info reported correctly. Do you have an ETA for this to hit github? I'm off to use this method in Many thanks John Broadband from £2.99/month, no tie in Intrigued? Call me John Ballance C.Eng MIET - [email protected] - 07976 295923On 20/07/2012 20:24, popcornmix wrote:
|
It's in the main source tree, so will be in next firmware update. Probably this weekend. |
Thanks for all your work on this. Really is appreciated. If anyone could address my questions above I'd be happy to update the wiki to make it more informative/consistent. |
@popcornmix . I have now implemented frame buffer fetch using message system. Compared to requesting an identical buffer through the framebuffer channel it looks as tough the VC side is being set up with a different cacheing mode .. I presume the through channel 1 it sets up using 0x40000000 mode ..L2 non aligned(?). Through the messaging channel is the VC side set up identically? . It seems not. The effect seen here is that in a buffer set up through the messaging channel, identical parameters, pixels don't always appear instantly.. there can be visible delay to pixels or partial lines.. . they seem to get there in the end. Other than that, (and presumably alpha channel 0x1: Alpha channel reversed (0 = fully transparent) not yet implemented) the frame buffer and general information calls are looking good. I haven't done anything with clocks or power yet. Thanks, John |
It's been pushed to github. There's possibly a fix for cache coherency in framebuffer. |
Hi.. Regrettably not fixed. Would it help to send you a kernel (riscos) showing this? no extra disc load is required, just the kernel.img |
To clarify: what doesn't behave .. call via the messaging channel (using the tag sequence posted earlier) to get a frame buffer. As time progresses the screen gradually paints. A flashing cursor never shows and a moving mouse pointer just leaves odd specs. |
Yes, send me a kernel image that shows the issue and I'll have a look. |
For the framebuffer (not) painting issue, I'd guess an ARM-side caching issue; it needs to be mapped uncached or flush operations performed. I assume that VC-side caching is less of an issue for a dumb FB since it'd be a HW block scanning out the FB content, which would never cache, and not the VC CPU which might cache |
kernel with fb set up in message channel followed by fb set up using the fb channel. same parameters. same buffer returned Use this first. Once booted, typing 'desktop' will put it to the desktop. may need to click cancel if offered. look at the mouse pointer for example. solid movement https://www.dropbox.com/s/x4nmi3v7kp945qm/KwithFBchan.zip This second one uses only the message channel. One line of code is commented out.. the line calling the subroutine that sends a message to a channel (subroutine is used for both the message channel and the framebuffer channel). https://www.dropbox.com/s/o2l34d0waevvqc3/KnoFBchan.zip Let me know if you cannot see any blatantly obvious difference. |
@swarren . Thanks for comment. see above. I'm referring to different behaviour in a framebuffer returned by the fb channel to that returned by the messaging channel. Behaviour should be identical. thanks |
@popcornmix BTW screen display assumes 1920 x 1080 x 32bpp at the moment. it ignores alpha .. but then I guess you'd spot that from your VC side diagnostics... |
@rosery Are you accessing the buffer through the frist mapping in the ARM MMU in both cases (i.e. setting the top bits to zero in the framebuffer channel case)? |
Hi Nathan The frame buffer supplied by the VC side is accessed identically however I wonder if anyone else is yet using the messaging channel as the only -- John Ballance C.Eng MIET - [email protected] - 07976 295923On 23/07/2012 09:59, Nathan Phillips wrote:
|
Sorry, potential cache fix didn't make it into official image. Your kenrel does work with my local image, which I've updated to earlier dropbox link. |
Ah that explains it. The version in the dropbox now does indeed behave as needed. Many thanks.. When do you think it'll reach github? ..mainly so I can easily point others at it Many thanks John |
I tried yesterday this Mailbox Function: My Results: (controlbits) (result1) x (result2) I get a 80000000 for sucessfull, but no Adress? Here my code:
Am i missing something? Another thing: What are the options for the pixeltype? They are missing in the wiki? |
my fault. i found my bug. allocated the memory wrong. sry |
This should all be implemented now. |
Tweak for hdmi mode selection when no detailed timing. Add hdmi_ignore_hotplug, disable_l2cache_witealloc and arm_control configs. Add mailbox property channel, see github issue raspberrypi#47
Suggestion for property/value message passing interface from ARM to GPU.
We have a generic message passing structure. The first word is always the property of interest (could be small integers or fourcc codes).
The remainder of the structure is property specific data whose length depends on the property. This data is used for both input and output data.
define MBOX_CHAN_GETPROP 8
define MBOX_CHAN_SETPROP 9
// generic property structure
struct property_s {
uint32_t property;
uint32_t data[];
}
// get property
void *property = <>
uint32_t success;
mbox_write(MBOX_CHAN_GETPROP, property);
mbox_read(MBOX_CHAN_GETPROP, &success);
// set property
void *property = <>
uint32_t success;
mbox_write(MBOX_CHAN_SETPROP, property);
mbox_read(MBOX_CHAN_SETPROP, &success);
Example:
define CLOCK_EMMC 1
struct property_one_word_s {
uint32_t property;
uint32_t value;
} *emmc_clock;
uint32_t success;
emmc_clock =
emmc_clock->property = CLOCK_EMMC;
emmc_clock->value = 50000000;
_wmb();
mbox_write(MBOX_CHAN_SETPROP, emmc_clock);
mbox_read(MBOX_CHAN_SETPROP, &success);
_rmb();
We could also use this to handle the existing framebuffer allocation and power control in a more consistent way.
Any config.txt properties can be retrieved with this mechanism. E.g.
struct property_one_word_one_string_s {
uint32_t property;
uint32_t value;
uint8_t string[MAX_STRING];
} *config_txt;
Similarly cmdline.txt and other ATAGs (e.g. memory size) can be got.
Properties that can be got or set:
CLOCK_EMMC
CLOCK_UART
CLOCK_ARM
CLOCK_CORE
CLOCK_V3D
CLOCK_H264
CLOCK_ISP
POWER_CONTROL
FRAMEBUFFER
Properties that can be got:
CMDLINE
CONFIG_STRING
SERIAL_NUM
BOARD_REV
MAC_ADDRESS
DISPLAY_WIDTH
DISPLAY_HEIGHT
DISPLAY_DEPTH
DMA_CHANS
MEMORY_SIZE
Any additional suggestions?
The text was updated successfully, but these errors were encountered: