Framebuffer's callback buffer starts from the area being drawn.

The commit 664d45dcba updated the
behavior of the framebuffer's putarea callback enabling it to be
used to draw a particular area of the display. Previously, putarea
was only used to draw the entire area of the display. Any different
area was drawn, row by row, by putrun. Also, before checking for
which callback to call, the framebuffer driver adjusted the buffer
reference that was going to be used for calling the driver's callback
to point to the init of the relevant data. After that commit, the
framebuffer's buffer reference passed to the driver's putarea now
contains the data to draw the entire display. Unlike the previous
version of that implementation, only the putrun's callback buffer
was being referenced from the address that contains the data that
actually is being drawn.

This commit fixes it by adjusting the reference for the run buffer
passed to the putrun/putarea callback. It always starts from the
beginning of the relevant data that is actually being drawn. That is
necessary because lcddev (which uses the same LCD display driver
callbacks) actually don't allocate a buffer containing the data to
draw the whole display, so the same putarea implementation of the
LCD drivers would'n be able to work for both lcddev and framebuffer.
Also it's necessary to pass the stride argument to the LCD drivers
in order to enable them to do partial writes by calculating the
buffer offset while sending row-by-row. The stride is equal the
width multiplied by the bytes per pixel (may add some padding)
for framebuffer and is equal to the lenght of the row being drawn
(multiplied by the same BPP) for lcddev.

Why this approach?
Other possible approaches would be:
1) modify lcddev driver to translate received buffer data to a
buffer similar to the framebuffer. That wouldn't be efficient
considering memory allocation.
2) Create a new callback function. While possible, it'd be confusing
to create a different callback to draw the whole screen and another
to draw only an area of the screen. Also, these callbacks would
differ themselves only from the way the buffer is filled.
3) Simply reverting 664d45dcba would
break the usage of the putarea callback to draw an area of the
display, which would also be inefficient.

This approach is based on the Zephyr's implementation of the ST7789
driver: the buffer starts from the beginiing of the region that would
be drawn. The display device driver's putarea implementation should
check if the operation refers to a full screen/full row and implement
(if possible) a single operation to send the data to be drawn more
efficiently.

Finally, this approach requires that the drivers which implement
the putarea callback and expects the entire framebuffer buffer
to be modified. They don't need to calculate where the data begins
as the new buffer represents the data from the address that is
actually being drawn. This includes adjusting the LCD drivers
GC9A01 and ST7789 and the driver for APA102-based LED matrix display.

arch/sim/src/sim/up_lcd.c

@@ -104,9 +104,10 @@ struct sim_dev_s
 
 static int sim_putrun(struct lcd_dev_s *dev, fb_coord_t row, fb_coord_t col,
                       const uint8_t *buffer, size_t npixels);
-static int sim_putarea(struct lcd_dev_s *dev, fb_coord_t row_start,
+static int sim_putarea(struct lcd_dev_s *dev,
-                       fb_coord_t row_end, fb_coord_t col_start,
+                       fb_coord_t row_start, fb_coord_t row_end,
-                       fb_coord_t col_end, const uint8_t *buffer);
+                       fb_coord_t col_start, fb_coord_t col_end,
+                       const uint8_t *buffer, fb_coord_t stride);
 static int sim_getrun(struct lcd_dev_s *dev, fb_coord_t row, fb_coord_t col,
                       uint8_t *buffer, size_t npixels);
 
@@ -252,12 +253,17 @@ static int sim_putrun(struct lcd_dev_s *dev, fb_coord_t row, fb_coord_t col,
  *   col_end   - Ending column to write to
  *               (range: col_start <= col_end < xres)
  *   buffer    - The buffer containing the area to be written to the LCD
+ *   stride    - Length of a line in bytes. This parameter may be necessary
+ *               to allow the LCD driver to calculate the offset for partial
+ *               writes when the buffer needs to be splited for row-by-row
+ *               writing.
  *
  ****************************************************************************/
 
 static int sim_putarea(struct lcd_dev_s *dev, fb_coord_t row_start,
                        fb_coord_t row_end, fb_coord_t col_start,
-                       fb_coord_t col_end, const uint8_t *buffer)
+                       fb_coord_t col_end, const uint8_t *buffer,
+                       fb_coord_t stride)
 {
   fb_coord_t row;
   size_t rows;

drivers/lcd/apa102.c

@@ -162,7 +162,7 @@ static int apa102_putrun(FAR struct lcd_dev_s *dev, fb_coord_t row,
 static int apa102_putarea(FAR struct lcd_dev_s *dev,
                           fb_coord_t row_start, fb_coord_t row_end,
                           fb_coord_t col_start, fb_coord_t col_end,
-                          FAR const uint8_t *buffer);
+                          FAR const uint8_t *buffer, fb_coord_t stride);
 static int apa102_getrun(FAR struct lcd_dev_s *dev, fb_coord_t row,
                          fb_coord_t col, FAR uint8_t *buffer,
                          size_t npixels);
@@ -474,13 +474,17 @@ static int apa102_putrun(FAR struct lcd_dev_s *dev, fb_coord_t row,
  *   col_end   - Ending column to write to
  *               (range: col_start <= col_end < xres)
  *   buffer    - The buffer containing the area to be written to the LCD
+ *   stride    - Length of a line in bytes. This parameter may be necessary
+ *               to allow the LCD driver to calculate the offset for partial
+ *               writes when the buffer needs to be splited for row-by-row
+ *               writing.
  *
  ****************************************************************************/
 
 static int apa102_putarea(FAR struct lcd_dev_s *dev,
                           fb_coord_t row_start, fb_coord_t row_end,
                           fb_coord_t col_start, fb_coord_t col_end,
-                          FAR const uint8_t *buffer)
+                          FAR const uint8_t *buffer, fb_coord_t stride)
 {
   FAR struct apa102_dev_s *priv = (FAR struct apa102_dev_s *)dev;
   FAR uint16_t *src = (FAR uint16_t *)buffer;

drivers/lcd/gc9a01.c

@@ -189,7 +189,8 @@ static void gc9a01_setarea(FAR struct gc9a01_dev_s *dev,
                            uint16_t x1, uint16_t y1);
 static void gc9a01_bpp(FAR struct gc9a01_dev_s *dev, int bpp);
 static void gc9a01_wrram(FAR struct gc9a01_dev_s *dev,
-                         FAR const uint16_t *buff, size_t size);
+                         FAR const uint8_t *buff, size_t size , size_t skip,
+                         size_t count);
 #ifndef CONFIG_LCD_NOGETRUN
 static void gc9a01_rdram(FAR struct gc9a01_dev_s *dev,
                          FAR uint16_t *buff, size_t size);
@@ -204,7 +205,7 @@ static int gc9a01_putrun(FAR struct lcd_dev_s *dev,
 static int gc9a01_putarea(FAR struct lcd_dev_s *dev,
                           fb_coord_t row_start, fb_coord_t row_end,
                           fb_coord_t col_start, fb_coord_t col_end,
-                          FAR const uint8_t *buffer);
+                          FAR const uint8_t *buffer, fb_coord_t stride);
 #ifndef CONFIG_LCD_NOGETRUN
 static int gc9a01_getrun(FAR struct lcd_dev_s *dev,
                          fb_coord_t row, fb_coord_t col,
@@ -535,17 +536,26 @@ static void gc9a01_bpp(FAR struct gc9a01_dev_s *dev, int bpp)
  * Name: gc9a01_wrram
  *
  * Description:
- *   Write to the driver's RAM.
+ *   Write to the driver's RAM. It is possible to write multiples of size
+ *   while skipping some values.
  *
  ****************************************************************************/
 
 static void gc9a01_wrram(FAR struct gc9a01_dev_s *dev,
-                         FAR const uint16_t *buff, size_t size)
+                         FAR const uint8_t *buff, size_t size, size_t skip,
+                         size_t count)
 {
+  size_t i;
+
   gc9a01_sendcmd(dev, GC9A01_RAMWR);
 
-  gc9a01_select(dev->spi, GC9A01_BYTESPP * 8);
+  gc9a01_select(dev->spi, 8);
-  SPI_SNDBLOCK(dev->spi, buff, size);
+
+  for (i = 0; i < count; i++)
+    {
+      SPI_SNDBLOCK(dev->spi, buff + (i * (size + skip)), size);
+    }
+
   gc9a01_deselect(dev->spi);
 }
 
@@ -614,13 +624,12 @@ static int gc9a01_putrun(FAR struct lcd_dev_s *dev,
                          FAR const uint8_t *buffer, size_t npixels)
 {
   FAR struct gc9a01_dev_s *priv = (FAR struct gc9a01_dev_s *)dev;
-  FAR const uint16_t *src = (FAR const uint16_t *)buffer;
 
   ginfo("row: %d col: %d npixels: %d\n", row, col, npixels);
   DEBUGASSERT(buffer && ((uintptr_t)buffer & 1) == 0);
 
   gc9a01_setarea(priv, col, row, col + npixels - 1, row);
-  gc9a01_wrram(priv, src, npixels);
+  gc9a01_wrram(priv, buffer, npixels, 0, 1);
 
   return OK;
 }
@@ -638,16 +647,22 @@ static int gc9a01_putrun(FAR struct lcd_dev_s *dev,
  *   col_end   - Ending column to write to
  *               (range: col_start <= col_end < xres)
  *   buffer    - The buffer containing the area to be written to the LCD
+ *   stride    - Length of a line in bytes. This parameter may be necessary
+ *               to allow the LCD driver to calculate the offset for partial
+ *               writes when the buffer needs to be splited for row-by-row
+ *               writing.
  *
  ****************************************************************************/
 
 static int gc9a01_putarea(FAR struct lcd_dev_s *dev,
                           fb_coord_t row_start, fb_coord_t row_end,
                           fb_coord_t col_start, fb_coord_t col_end,
-                          FAR const uint8_t *buffer)
+                          FAR const uint8_t *buffer, fb_coord_t stride)
 {
   FAR struct gc9a01_dev_s *priv = (FAR struct gc9a01_dev_s *)dev;
-  FAR const uint16_t *src = (FAR const uint16_t *)buffer;
+  size_t cols = col_end - col_start + 1;
+  size_t rows = row_end - row_start + 1;
+  size_t row_size = cols * (priv->bpp >> 3);
 
   ginfo("row_start: %d row_end: %d col_start: %d col_end: %d\n",
          row_start, row_end, col_start, col_end);
@@ -655,8 +670,26 @@ static int gc9a01_putarea(FAR struct lcd_dev_s *dev,
   DEBUGASSERT(buffer && ((uintptr_t)buffer & 1) == 0);
 
   gc9a01_setarea(priv, col_start, row_start, col_end, row_end);
-  gc9a01_wrram(priv, src,
+
-               (row_end - row_start + 1) * (col_end - col_start + 1));
+  /* If the stride is the same of the row, a single SPI transfer is enough.
+   * That is always true for lcddev. For framebuffer, that indicates a full
+   * screen or full row update.
+   */
+
+  if (stride == row_size)
+    {
+      /* simpler case, we can just send the whole buffer */
+
+      ginfo("Using full screen/full row mode\n");
+      gc9a01_wrram(priv, buffer, rows * row_size, 0, 1);
+    }
+  else
+    {
+      /* We have to go row by row */
+
+      ginfo("Falling-back to row by row mode\n");
+      gc9a01_wrram(priv, buffer, row_size, stride - row_size, rows);
+    }
 
   return OK;
 }

drivers/lcd/lcd_dev.c

@@ -121,6 +121,8 @@ static int lcddev_ioctl(FAR struct file *filep, int cmd, unsigned long arg)
       {
         FAR struct lcddev_area_s *lcd_area =
             (FAR struct lcddev_area_s *)arg;
+        size_t cols = lcd_area->col_end - lcd_area->col_start + 1;
+        size_t row_size = cols * (priv->planeinfo.bpp >> 3);
 
         if (priv->planeinfo.getarea)
           {
@@ -129,27 +131,27 @@ static int lcddev_ioctl(FAR struct file *filep, int cmd, unsigned long arg)
                                           lcd_area->row_end,
                                           lcd_area->col_start,
                                           lcd_area->col_end,
-                                          lcd_area->data);
+                                          lcd_area->data,
+                                          row_size);
           }
         else
           {
             /* Emulate getarea() using getrun() */
 
             uint8_t *buf = lcd_area->data;
-            size_t npixels = (lcd_area->col_end - lcd_area->col_start + 1);
             int row;
 
             for (row = lcd_area->row_start; row <= lcd_area->row_end; row++)
               {
                 ret = priv->planeinfo.getrun(priv->lcd_ptr, row,
                                              lcd_area->col_start, buf,
-                                             npixels);
+                                             row_size);
                 if (ret < 0)
                   {
                     break;
                   }
 
-                buf += npixels * (priv->planeinfo.bpp >> 3);
+                buf += row_size;
               }
           }
       }
@@ -158,6 +160,8 @@ static int lcddev_ioctl(FAR struct file *filep, int cmd, unsigned long arg)
       {
         FAR const struct lcddev_area_s *lcd_area =
             (FAR const struct lcddev_area_s *)arg;
+        size_t cols = lcd_area->col_end - lcd_area->col_start + 1;
+        size_t row_size = cols * (priv->planeinfo.bpp >> 3);
 
         if (priv->planeinfo.putarea)
           {
@@ -166,27 +170,27 @@ static int lcddev_ioctl(FAR struct file *filep, int cmd, unsigned long arg)
                                           lcd_area->row_end,
                                           lcd_area->col_start,
                                           lcd_area->col_end,
-                                          lcd_area->data);
+                                          lcd_area->data,
+                                          row_size);
           }
         else
           {
             /* Emulate putarea() using putrun() */
 
             uint8_t *buf = lcd_area->data;
-            size_t npixels = (lcd_area->col_end - lcd_area->col_start + 1);
             int row;
 
             for (row = lcd_area->row_start; row <= lcd_area->row_end; row++)
               {
                 ret = priv->planeinfo.putrun(priv->lcd_ptr, row,
                                              lcd_area->col_start, buf,
-                                             npixels);
+                                             row_size);
                 if (ret < 0)
                   {
                     break;
                   }
 
-                buf += npixels * (priv->planeinfo.bpp >> 3);
+                buf += row_size;
               }
           }
       }

drivers/lcd/lcd_framebuffer.c

@@ -211,6 +211,11 @@ static int lcdfb_updateearea(FAR struct fb_vtable_s *vtable,
           unsigned int pixperbyte = 8 / pinfo->bpp;
           startx &= ~(pixperbyte - 1);
         }
+
+      /* Get the starting position in the framebuffer */
+
+      run  = priv->fbmem + starty * priv->stride;
+      run += (startx * pinfo->bpp + 7) >> 3;
     }
 
   if (pinfo->putarea != NULL)
@@ -225,7 +230,8 @@ static int lcdfb_updateearea(FAR struct fb_vtable_s *vtable,
        * - apply DMA channel to transfer data to driver memory.
        */
 
-      ret = pinfo->putarea(pinfo->dev, starty, endy, startx, endx, run);
+      ret = pinfo->putarea(pinfo->dev, starty, endy, startx, endx,
+                           run, priv->stride);
       if (ret < 0)
         {
           lcderr("Failed to update area");
@@ -236,11 +242,6 @@ static int lcdfb_updateearea(FAR struct fb_vtable_s *vtable,
     {
       width = endx - startx + 1;
 
-      /* Get the starting position in the framebuffer */
-
-      run  = priv->fbmem + starty * priv->stride;
-      run += (startx * pinfo->bpp + 7) >> 3;
-
       for (row = starty; row <= endy; row++)
         {
           ret = pinfo->putrun(pinfo->dev, row, startx, run, width);

drivers/lcd/st7789.c

@@ -186,7 +186,7 @@ static void st7789_setarea(FAR struct st7789_dev_s *dev,
                            uint16_t x1, uint16_t y1);
 static void st7789_bpp(FAR struct st7789_dev_s *dev, int bpp);
 static void st7789_wrram(FAR struct st7789_dev_s *dev,
-                         FAR const uint16_t *buff, size_t size, size_t skip,
+                         FAR const uint8_t *buff, size_t size, size_t skip,
                          size_t count);
 #ifndef CONFIG_LCD_NOGETRUN
 static void st7789_rdram(FAR struct st7789_dev_s *dev,
@@ -202,7 +202,7 @@ static int st7789_putrun(FAR struct lcd_dev_s *dev,
 static int st7789_putarea(FAR struct lcd_dev_s *dev,
                           fb_coord_t row_start, fb_coord_t row_end,
                           fb_coord_t col_start, fb_coord_t col_end,
-                          FAR const uint8_t *buffer);
+                          FAR const uint8_t *buffer, fb_coord_t stride);
 #ifndef CONFIG_LCD_NOGETRUN
 static int st7789_getrun(FAR struct lcd_dev_s *dev,
                          fb_coord_t row, fb_coord_t col,
@@ -448,14 +448,14 @@ static void st7789_bpp(FAR struct st7789_dev_s *dev, int bpp)
  ****************************************************************************/
 
 static void st7789_wrram(FAR struct st7789_dev_s *dev,
-                         FAR const uint16_t *buff, size_t size, size_t skip,
+                         FAR const uint8_t *buff, size_t size, size_t skip,
                          size_t count)
 {
   size_t i;
 
   st7789_sendcmd(dev, ST7789_RAMWR);
 
-  st7789_select(dev->spi, ST7789_BYTESPP * 8);
+  st7789_select(dev->spi, 8);
 
   for (i = 0; i < count; i++)
     {
@@ -530,13 +530,12 @@ static int st7789_putrun(FAR struct lcd_dev_s *dev,
                          FAR const uint8_t *buffer, size_t npixels)
 {
   FAR struct st7789_dev_s *priv = (FAR struct st7789_dev_s *)dev;
-  FAR const uint16_t *src = (FAR const uint16_t *)buffer;
 
   ginfo("row: %d col: %d npixels: %d\n", row, col, npixels);
   DEBUGASSERT(buffer && ((uintptr_t)buffer & 1) == 0);
 
   st7789_setarea(priv, col, row, col + npixels - 1, row);
-  st7789_wrram(priv, src, npixels, 0, 1);
+  st7789_wrram(priv, buffer, npixels, 0, 1);
 
   return OK;
 }
@@ -554,17 +553,22 @@ static int st7789_putrun(FAR struct lcd_dev_s *dev,
  *   col_end   - Ending column to write to
  *               (range: col_start <= col_end < xres)
  *   buffer    - The buffer containing the area to be written to the LCD
+ *   stride    - Length of a line in bytes. This parameter may be necessary
+ *               to allow the LCD driver to calculate the offset for partial
+ *               writes when the buffer needs to be splited for row-by-row
+ *               writing.
  *
  ****************************************************************************/
 
 static int st7789_putarea(FAR struct lcd_dev_s *dev,
                           fb_coord_t row_start, fb_coord_t row_end,
                           fb_coord_t col_start, fb_coord_t col_end,
-                          FAR const uint8_t *buffer)
+                          FAR const uint8_t *buffer, fb_coord_t stride)
 {
   FAR struct st7789_dev_s *priv = (FAR struct st7789_dev_s *)dev;
-  FAR const uint16_t *src = (FAR const uint16_t *)buffer;
+  size_t cols = col_end - col_start + 1;
-  fb_coord_t bsiz = col_end - col_start + 1;
+  size_t rows = row_end - row_start + 1;
+  size_t row_size = cols * (priv->bpp >> 3);
 
   ginfo("row_start: %d row_end: %d col_start: %d col_end: %d\n",
          row_start, row_end, col_start, col_end);
@@ -572,8 +576,26 @@ static int st7789_putarea(FAR struct lcd_dev_s *dev,
   DEBUGASSERT(buffer && ((uintptr_t)buffer & 1) == 0);
 
   st7789_setarea(priv, col_start, row_start, col_end, row_end);
-  st7789_wrram(priv, src + (ST7789_XRES * row_start) + col_start,
+
-               bsiz, ST7789_XRES - bsiz, row_end - row_start + 1);
+  /* If the stride is the same of the row, a single SPI transfer is enough.
+   * That is always true for lcddev. For framebuffer, that indicates a full
+   * screen or full row update.
+   */
+
+  if (stride == row_size)
+    {
+      /* simpler case, we can just send the whole buffer */
+
+      ginfo("Using full screen/full row mode\n");
+      st7789_wrram(priv, buffer, rows * row_size, 0, 1);
+    }
+  else
+    {
+      /* We have to go row by row */
+
+      ginfo("Falling-back to row by row mode\n");
+      st7789_wrram(priv, buffer, row_size, stride - row_size, rows);
+    }
 
   return OK;
 }

include/nuttx/lcd/lcd.h

@@ -79,9 +79,11 @@ struct lcd_planeinfo_s
    *  col_start - Starting column to write to (range: 0 <= col <= xres)
    *  col_end   - Ending column to write to
    *              (range: col_start <= col_end < xres)
-   *  buffer    - The buffer containing the complete frame to be written to
+   *  buffer    - The buffer containing the area to be written to the LCD
-   *              the display (the correct rows and columns have to be
+   *  stride    - Length of a line in bytes. This parameter may be necessary
-   *              selected from it)
+   *              to allow the LCD driver to calculate the offset for partial
+   *              writes when the buffer needs to be splited for row-by-row
+   *              writing.
    *
    * NOTE: this operation may not be supported by the device, in which case
    * the callback pointer will be NULL. In that case, putrun() should be
@@ -90,7 +92,8 @@ struct lcd_planeinfo_s
 
   int (*putarea)(FAR struct lcd_dev_s *dev, fb_coord_t row_start,
                  fb_coord_t row_end, fb_coord_t col_start,
-                 fb_coord_t col_end, FAR const uint8_t *buffer);
+                 fb_coord_t col_end, FAR const uint8_t *buffer,
+                 fb_coord_t stride);
 
   /* This method can be used to read a partial raster line from the LCD:
    *
@@ -115,6 +118,7 @@ struct lcd_planeinfo_s
    *  col_end   - Ending column to read from
    *              (range: col_start <= col_end < xres)
    *  buffer    - The buffer where the data will be written
+   *  stride    - Length of a line in bytes.
    *
    * NOTE: this operation may not be supported by the device, in which case
    * the callback pointer will be NULL. In that case, getrun() should be
@@ -123,7 +127,8 @@ struct lcd_planeinfo_s
 
   int (*getarea)(FAR struct lcd_dev_s *dev, fb_coord_t row_start,
                  fb_coord_t row_end, fb_coord_t col_start,
-                 fb_coord_t col_end, FAR uint8_t *buffer);
+                 fb_coord_t col_end, FAR uint8_t *buffer,
+                 fb_coord_t stride);
 
   /* This method can be used to redraw display's content.
    *