[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #4504: add new interface for fb driver and lcd dev driver

[GitBox <gi...@apache.org>]

xiaoxiang781216 commented on a change in pull request #4504:
URL: https://github.com/apache/incubator-nuttx/pull/4504#discussion_r705904932



##########
File path: include/nuttx/video/fb.h
##########
@@ -569,6 +583,24 @@ struct fb_vtable_s
                FAR const struct fb_overlayblend_s *blend);
 # endif
 #endif
+
+  /* Specific Controls ******************************************************/
+
+  /* Set the frequency of the framebuffer update panel (0: disable refresh) */
+
+  int (*setframerate)(FAR struct fb_vtable_s *vtable, int rate);
+
+  /* Get the frequency of the framebuffer update panel (0: disable refresh) */
+
+  int (*getframerate)(FAR struct fb_vtable_s *vtable);
+
+  /* Get the panel power status (0: full off). */
+
+  int (*getpower)(FAR struct fb_vtable_s *vtable);
+
+  /* Enable/disable panel power (0: full off). */
+
+  int (*setpower)(FAR struct fb_vtable_s *vtable, int power);

Review comment:
       > It does not seem correct for the Framebuffer API to handle these operations on the display device.
   > Why not make these go through the LCD APIs?
   
   Here is several reasons:
   
   1. FB and LCD driver interface is already separated in the current design(e.g. FBIOPUT_CURSOR v.s. LCDDEVIO_SETCURSOR). So, the new IOCTL(e.g. FBIOSET_POWER) reduce the difference between two interfaces.
   2. If we expose the display related operation into two device nodes, it make the userspace GUI library integration harder, because:
      - The library need to manage two handles instead one
      - It's hard to identify fb/lcd pair if the device support multiple display
   
   > I believe the expected method for powering off the display device according to the Framebuffer API is through `fb_uninitialize`, but nobody is currently calling it.
   
   Two problems here:
   1. LCD driver already expose LCDDEVIO_GETPOWER/LCDDEVIO_SETPOWER
   2. FBIOSET_POWER accept an integer argument to specify the power level, but is strange to pass the power level to FBIO_INIT
   2. More important, up_fbuninitialize will destroy device node, and then is impossible to reenable FB again




-- 
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-unsubscribe@nuttx.apache.org

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