Provide a cleaner Mailbox interface

src/platform/mailbox.rs

@@ -61,6 +61,20 @@ pub enum MboxError {
     Timeout,
 }
 
+impl core::fmt::Display for MboxError {
+    fn fmt(&self, f: &mut core::fmt::Formatter) -> core::fmt::Result {
+        write!(
+            f,
+            "{}",
+            match self {
+                MboxError::ResponseError => "ResponseError",
+                MboxError::UnknownError => "UnknownError",
+                MboxError::Timeout => "Timeout",
+            }
+        )
+    }
+}
+
 pub type Result<T> = ::core::result::Result<T, MboxError>;
 
 /*
@@ -97,13 +111,15 @@ pub struct GpuFb {
     pub size: u32,
 }
 
+// Single code indicating request
 pub const REQUEST: u32 = 0;
 
-// Responses
+// Possible responses
 pub mod response {
     pub const SUCCESS: u32 = 0x8000_0000;
     pub const ERROR: u32 = 0x8000_0001; // error parsing request buffer (partial response)
-    /** When responding, the VC sets this bit in val_len to indicate a response */
+    /** When responding, the VC sets this bit in val_len to indicate a response. */
+    /** Each tag with this bit set will contain VC response data. */
     pub const VAL_LEN_FLAG: u32 = 0x8000_0000;
 }
 
@@ -333,6 +349,60 @@ impl Mailbox {
         self.write(channel)?;
         self.read(channel)
     }
+
+    // Specific mailbox functions
+
+    #[inline]
+    pub fn request(&mut self) -> usize {
+        self.buffer[1] = REQUEST;
+        2
+    }
+
+    #[inline]
+    pub fn end(&mut self, index: usize) -> () {
+        // @todo return Result
+        self.buffer[index] = tag::End;
+        self.buffer[0] = (index as u32 + 1) * 4;
+    }
+
+    #[inline]
+    pub fn set_physical_wh(&mut self, index: usize, width: u32, height: u32) -> usize {
+        self.buffer[index] = tag::SetPhysicalWH;
+        self.buffer[index + 1] = 8; // Buffer size   // val buf size
+        self.buffer[index + 2] = 8; // Request size  // val size
+        self.buffer[index + 3] = width; // Space for horizontal resolution
+        self.buffer[index + 4] = height; // Space for vertical resolution
+        index + 5
+    }
+
+    #[inline]
+    pub fn set_virtual_wh(&mut self, index: usize, width: u32, height: u32) -> usize {
+        self.buffer[index] = tag::SetVirtualWH;
+        self.buffer[index + 1] = 8; // Buffer size   // val buf size
+        self.buffer[index + 2] = 8; // Request size  // val size
+        self.buffer[index + 3] = width; // Space for horizontal resolution
+        self.buffer[index + 4] = height; // Space for vertical resolution
+        index + 5
+    }
+
+    #[inline]
+    pub fn set_depth(&mut self, index: usize, depth: u32) -> usize {
+        self.buffer[index] = tag::SetDepth;
+        self.buffer[index + 1] = 4; // Buffer size   // val buf size
+        self.buffer[index + 2] = 4; // Request size  // val size
+        self.buffer[index + 3] = depth; // bpp
+        index + 4
+    }
+
+    #[inline]
+    pub fn allocate_buffer_aligned(&mut self, index: usize, alignment: u32) -> usize {
+        self.buffer[index] = tag::AllocateBuffer;
+        self.buffer[index + 1] = 8; // Buffer size   // val buf size
+        self.buffer[index + 2] = 4; // Request size  // val size
+        self.buffer[index + 3] = alignment; // Alignment = 16 -- fb_ptr will be here
+        self.buffer[index + 4] = 0; // Space for response -- fb_size will be here
+        index + 5
+    }
 }
 
 /// Deref to RegisterBlock

src/platform/vc.rs

@@ -17,37 +17,30 @@ impl VC {
         // Use property channel
         let mut mbox = Mailbox::new();
 
-        mbox.buffer[0] = 22 * 4;
-        mbox.buffer[1] = mailbox::REQUEST;
+        /*
+         *  * All tags in the request are processed in one operation.
+         *  * It is not valid to mix Test tags with Get/Set tags
+         *    in the same operation and no tags will be returned.
+         *  * Get tags will be processed after all Set tags.
+         *  * If an allocate buffer tag is omitted when setting parameters,
+         *    then no change occurs unless it can be accommodated without changing
+         *    the buffer base or size.
+         *  * When an allocate buffer response is returned, the old buffer area
+         *    (if the base or size has changed) is implicitly freed.
+         */
 
-        mbox.buffer[2] = tag::SetPhysicalWH;
-        mbox.buffer[3] = 8; // Buffer size   // val buf size
-        mbox.buffer[4] = 8; // Request size  // val size
-        mbox.buffer[5] = size.x; // Space for horizontal resolution
-        mbox.buffer[6] = size.y; // Space for vertical resolution
+        let index = mbox.request();
+        let index = mbox.set_physical_wh(index, size.x, size.y);
+        let index = mbox.set_virtual_wh(index, size.x, size.y);
+        let index = mbox.set_depth(index, depth);
+        let index = mbox.allocate_buffer_aligned(index, 16);
+        mbox.end(index);
 
-        mbox.buffer[7] = tag::SetVirtualWH as u32;
-        mbox.buffer[8] = 8; // Buffer size   // val buf size
-        mbox.buffer[9] = 8; // Request size  // val size
-        mbox.buffer[10] = size.x; // Space for horizontal resolution
-        mbox.buffer[11] = size.y; // Space for vertical resolution
-
-        mbox.buffer[12] = tag::SetDepth as u32;
-        mbox.buffer[13] = 4; // Buffer size   // val buf size
-        mbox.buffer[14] = 4; // Request size  // val size
-        mbox.buffer[15] = depth; // bpp
-
-        mbox.buffer[16] = tag::AllocateBuffer as u32;
-        mbox.buffer[17] = 8; // Buffer size   // val buf size
-        mbox.buffer[18] = 4; // Request size  // val size
-        mbox.buffer[19] = 16; // Alignment = 16 -- fb_ptr will be here
-        mbox.buffer[20] = 0; // Space for response -- fb_size will be here
-
-        mbox.buffer[21] = tag::End as u32;
-
-        mbox.call(channel::PropertyTagsArmToVc).map_err(|_| ());
-
-        jtag_dbg_wait();
+        mbox.call(channel::PropertyTagsArmToVc).map_err(|e| {
+            println!("Mailbox call returned error {}", e);
+            println!("Mailbox contents: {}", mbox);
+            ()
+        });
 
         if (mbox.buffer[18] & VAL_LEN_FLAG) == 0 {
             return None;