[GitHub] [incubator-nuttx] gustavonihei commented on a change in pull request #4623: feat: driver: support double buffer for display

[GitBox]

gustavonihei commented on a change in pull request #4623:
URL: https://github.com/apache/incubator-nuttx/pull/4623#discussion_r718468359



##########
File path: include/nuttx/video/fb.h
##########
@@ -283,6 +283,10 @@
 #define FBIOGET_FRAMERATE     _FBIOC(0x0015)  /* Get frame rate
                                                * Argument:            int* */
 
+#define FBIOPAN_DISPLAY       _FBIOC(0x0020)  /* Pan display

Review comment:
       ```suggestion
   #define FBIOPAN_DISPLAY       _FBIOC(0x0016)  /* Pan display
   ```
   Why not follow the sequence?

##########
File path: include/nuttx/video/fb.h
##########
@@ -315,11 +319,15 @@ struct fb_videoinfo_s
 
 struct fb_planeinfo_s
 {
-  FAR void  *fbmem;       /* Start of frame buffer memory */
-  size_t     fblen;       /* Length of frame buffer memory in bytes */
-  fb_coord_t stride;      /* Length of a line in bytes */
-  uint8_t    display;     /* Display number */
-  uint8_t    bpp;         /* Bits per pixel */
+  FAR void  *fbmem;        /* Start of frame buffer memory */
+  size_t     fblen;        /* Length of frame buffer memory in bytes */
+  fb_coord_t stride;       /* Length of a line in bytes */
+  uint8_t    display;      /* Display number */
+  uint8_t    bpp;          /* Bits per pixel */
+  uint32_t   xres_virtual; /* Virtual Horizontal resolution in pixel columns */
+  uint32_t   yres_virtual; /* Virtual Vertical resolution in pixel rows */
+  uint32_t   xoffest;      /* X offset */

Review comment:
       ```suggestion
     uint32_t   xoffset;      /* X offset */
   ```
   Typo in the struct member

##########
File path: include/nuttx/video/fb.h
##########
@@ -315,11 +319,15 @@ struct fb_videoinfo_s
 
 struct fb_planeinfo_s
 {
-  FAR void  *fbmem;       /* Start of frame buffer memory */
-  size_t     fblen;       /* Length of frame buffer memory in bytes */
-  fb_coord_t stride;      /* Length of a line in bytes */
-  uint8_t    display;     /* Display number */
-  uint8_t    bpp;         /* Bits per pixel */
+  FAR void  *fbmem;        /* Start of frame buffer memory */
+  size_t     fblen;        /* Length of frame buffer memory in bytes */
+  fb_coord_t stride;       /* Length of a line in bytes */
+  uint8_t    display;      /* Display number */
+  uint8_t    bpp;          /* Bits per pixel */
+  uint32_t   xres_virtual; /* Virtual Horizontal resolution in pixel columns */
+  uint32_t   yres_virtual; /* Virtual Vertical resolution in pixel rows */
+  uint32_t   xoffest;      /* X offset */
+  uint32_t   yoffset;      /* Y offset */

Review comment:
       Please, improve the documentation for these members to make them more 
intuitive. As a reference, see the documentation for the Linux struct:
   https://elixir.bootlin.com/linux/latest/source/include/uapi/linux/fb.h#L243
   
![image](https://user-images.githubusercontent.com/38959758/135271734-04ad51ae-4e5e-4953-939d-36d578c17a13.png)
   




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscr...@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org