You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2022/06/24 11:39:50 UTC

[GitHub] [incubator-nuttx] adamkaliszan opened a new pull request, #6510: Support for 2.13 inch display v2

adamkaliszan opened a new pull request, #6510:
URL: https://github.com/apache/incubator-nuttx/pull/6510

   ## Summary
   Driver supports 2.13 inch display v2 mono
   ## Impact
   Function **putrun** (pointer in struct lcd_planeinfo_s) redraws the screen if argument **npixels** has value 0. Adding special IOCTL for framebuffes driver can call putrun function in order to refresh e-paper screen.
   We don't want to refresh the screen after single line/pixel modification. Driver is not aware if there is next **putrun** function call or not.
   ## Testing
   Tested on lilygo t5v2 with 2.9 inch display and STM Nucleo-wl55JC with 2.13 inch display.
   
   


-- 
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


[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #6510: Support for 2.13 inch display v2

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on PR #6510:
URL: https://github.com/apache/incubator-nuttx/pull/6510#issuecomment-1166186079

   @adamkaliszan  please rebase to the latest code to fix the ci build error.


-- 
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


[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #6510: Support for 2.13 inch display v2

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #6510:
URL: https://github.com/apache/incubator-nuttx/pull/6510#discussion_r906638713


##########
drivers/lcd/ssd1680.c:
##########
@@ -634,35 +679,50 @@ static int ssd1680_configuredisplay(struct ssd1680_dev_s *priv)
 
   /* Step 9: SSD1680_SET_RAMXCOUNT, 0 */
 
+  lcdinfo("Set ram X count (0x%02x): 0\n", SSD1680_SET_RAMXCOUNT);
   ssd1680_snd_cmd_with_data1(priv, SSD1680_SET_RAMXCOUNT, 0x00);
 
   /* Step 10: SSD1680_SET_RAMYCOUNT, 0, 0 */
 
+  lcdinfo("Set ram Y count (0x%02x): 0\n", SSD1680_SET_RAMYCOUNT);
   ssd1680_snd_cmd_with_data2(priv, SSD1680_SET_RAMYCOUNT, 0x00, 0x00);
 
   /* Step 11: Lookup table */
 
-  lcdinfo("Write lookup table (%d bytes)\n", sizeof (ssd1680_lut));
+  lcdinfo("Write lookup table (0x%02x): (%d bytes)\n", SSD1680_WRITE_LUT,
+      sizeof (ssd1680_lut));
   ssd1680_snd_cmd_with_data(priv, SSD1680_WRITE_LUT, ssd1680_lut,
       sizeof (ssd1680_lut));
 
   /* Step 12: Write sequence */
 
-  lcdinfo("Write controll sequence 0x%02x\n", 0xc0);
-  ssd1680_snd_cmd_with_data1(priv, SSD1680_DISP_CTRL2, 0xc0);
+  lcdinfo("Write control sequence (0x%02x): 0x%02x\n", SSD1680_DISP_CTRL2,
+      0xc0);
+  ssd1680_snd_cmd_with_data1(priv, SSD1680_DISP_CTRL2, 0xc7);
 
   /* Step 13: Master Activate and busy wait */
 
+  lcdinfo("Write master activate (0x%02x) command\n",
+      SSD1680_MASTER_ACTIVATE);
   ssd1680_snd_cmd_with_data0(priv, SSD1680_MASTER_ACTIVATE);
-  ssd1680_busy_wait(priv);
 
-  lcdinfo("Configuration ready\n");
-  priv->is_conf = true;
-  return OK;
+  ret = ssd1680_busy_wait(priv);
+exit:
+  if (ret == OK)
+    {
+      lcdinfo("Configuration ready\n");
+      priv->is_conf = true;
+    }
+  else

Review Comment:
   move exit here?



-- 
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


[GitHub] [incubator-nuttx] adamkaliszan commented on a diff in pull request #6510: Support for 2.13 inch display v2

Posted by GitBox <gi...@apache.org>.
adamkaliszan commented on code in PR #6510:
URL: https://github.com/apache/incubator-nuttx/pull/6510#discussion_r907984008


##########
drivers/lcd/ssd1680.c:
##########
@@ -878,19 +896,30 @@ static void ssd1680_select(FAR struct ssd1680_dev_s *priv, bool cs)
     }
 }
 
-static void ssd1680_busy_wait(FAR struct ssd1680_dev_s *priv)
+static int ssd1680_busy_wait(FAR struct ssd1680_dev_s *priv)
 {
+  int maks_wait_ticks = 200;
   if ((priv->board_priv != NULL) && (priv->board_priv->check_busy != NULL))
     {
-      while (priv->board_priv->check_busy())
+      while (priv->board_priv->check_busy() && maks_wait_ticks > 0)
         {
-          nxsig_usleep(1);
+          nxsig_usleep(1000);
+          maks_wait_ticks--;
         }
     }
   else
     {
-      nxsig_usleep(2000000);
+      nxsig_usleep(maks_wait_ticks);

Review Comment:
   OK. It is done.



##########
drivers/lcd/ssd1680.c:
##########
@@ -878,19 +896,30 @@ static void ssd1680_select(FAR struct ssd1680_dev_s *priv, bool cs)
     }
 }
 
-static void ssd1680_busy_wait(FAR struct ssd1680_dev_s *priv)
+static int ssd1680_busy_wait(FAR struct ssd1680_dev_s *priv)
 {
+  int maks_wait_ticks = 200;
   if ((priv->board_priv != NULL) && (priv->board_priv->check_busy != NULL))
     {
-      while (priv->board_priv->check_busy())
+      while (priv->board_priv->check_busy() && maks_wait_ticks > 0)

Review Comment:
   OK It is done



-- 
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


[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #6510: Support for 2.13 inch display v2

Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #6510:
URL: https://github.com/apache/incubator-nuttx/pull/6510#discussion_r906665867


##########
drivers/lcd/ssd1680.c:
##########
@@ -878,19 +896,30 @@ static void ssd1680_select(FAR struct ssd1680_dev_s *priv, bool cs)
     }
 }
 
-static void ssd1680_busy_wait(FAR struct ssd1680_dev_s *priv)
+static int ssd1680_busy_wait(FAR struct ssd1680_dev_s *priv)
 {
+  int maks_wait_ticks = 200;
   if ((priv->board_priv != NULL) && (priv->board_priv->check_busy != NULL))
     {
-      while (priv->board_priv->check_busy())
+      while (priv->board_priv->check_busy() && maks_wait_ticks > 0)
         {
-          nxsig_usleep(1);
+          nxsig_usleep(1000);
+          maks_wait_ticks--;
         }
     }
   else
     {
-      nxsig_usleep(2000000);
+      nxsig_usleep(maks_wait_ticks);

Review Comment:
   maybe
   ```suggestion
         nxsig_usleep(maks_wait_ticks * 1000);
   ```
   ?



##########
drivers/lcd/ssd1680.c:
##########
@@ -567,49 +567,96 @@ static int ssd1680_configuredisplay(struct ssd1680_dev_s *priv)
 
   /* Busy wait */
 
-  ssd1680_busy_wait(priv);
-  lcdinfo("SSD1680 is ready\n");
+  ret = ssd1680_busy_wait(priv);
+  if (ret != OK)
+    {
+      goto exit_err;

Review Comment:
   IMO duplicating `lcderr("Configuration is not ready\n");` here + `return ret;` is better than `goto`. Anyway if the mesage is the same, then compiler till optimize and generate the code similar to goto.



##########
drivers/lcd/ssd1680.c:
##########
@@ -878,19 +896,30 @@ static void ssd1680_select(FAR struct ssd1680_dev_s *priv, bool cs)
     }
 }
 
-static void ssd1680_busy_wait(FAR struct ssd1680_dev_s *priv)
+static int ssd1680_busy_wait(FAR struct ssd1680_dev_s *priv)
 {
+  int maks_wait_ticks = 200;
   if ((priv->board_priv != NULL) && (priv->board_priv->check_busy != NULL))
     {
-      while (priv->board_priv->check_busy())
+      while (priv->board_priv->check_busy() && maks_wait_ticks > 0)

Review Comment:
   ```suggestion
         while (priv->board_priv->check_busy() && maks_wait_ticks-- > 0)
   ```
   and remove code below.



##########
drivers/lcd/ssd1680.c:
##########
@@ -1269,11 +1299,16 @@ FAR struct lcd_dev_s *ssd1680_initialize(FAR struct spi_dev_s *dev,
   /* Initialize the framebuffer */
 
   memset(priv->shadow_fb, SSD1680_Y1_BLACK & 1 ?
-    0xff : 0x00, SSD1680_DEV_FBSIZE);
+         0xff : 0x00, SSD1680_DEV_FBSIZE);
 
   /* Power on and configure display */
 
-  ssd1680_setpower(&priv->dev, true);
+  ret = ssd1680_setpower(&priv->dev, true);
+

Review Comment:
   Optional
   ```suggestion
   ```



-- 
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


[GitHub] [incubator-nuttx] adamkaliszan commented on a diff in pull request #6510: Support for 2.13 inch display v2

Posted by GitBox <gi...@apache.org>.
adamkaliszan commented on code in PR #6510:
URL: https://github.com/apache/incubator-nuttx/pull/6510#discussion_r906647539


##########
drivers/lcd/ssd1680.c:
##########
@@ -634,35 +679,50 @@ static int ssd1680_configuredisplay(struct ssd1680_dev_s *priv)
 
   /* Step 9: SSD1680_SET_RAMXCOUNT, 0 */
 
+  lcdinfo("Set ram X count (0x%02x): 0\n", SSD1680_SET_RAMXCOUNT);
   ssd1680_snd_cmd_with_data1(priv, SSD1680_SET_RAMXCOUNT, 0x00);
 
   /* Step 10: SSD1680_SET_RAMYCOUNT, 0, 0 */
 
+  lcdinfo("Set ram Y count (0x%02x): 0\n", SSD1680_SET_RAMYCOUNT);
   ssd1680_snd_cmd_with_data2(priv, SSD1680_SET_RAMYCOUNT, 0x00, 0x00);
 
   /* Step 11: Lookup table */
 
-  lcdinfo("Write lookup table (%d bytes)\n", sizeof (ssd1680_lut));
+  lcdinfo("Write lookup table (0x%02x): (%d bytes)\n", SSD1680_WRITE_LUT,
+      sizeof (ssd1680_lut));
   ssd1680_snd_cmd_with_data(priv, SSD1680_WRITE_LUT, ssd1680_lut,
       sizeof (ssd1680_lut));
 
   /* Step 12: Write sequence */
 
-  lcdinfo("Write controll sequence 0x%02x\n", 0xc0);
-  ssd1680_snd_cmd_with_data1(priv, SSD1680_DISP_CTRL2, 0xc0);
+  lcdinfo("Write control sequence (0x%02x): 0x%02x\n", SSD1680_DISP_CTRL2,
+      0xc0);
+  ssd1680_snd_cmd_with_data1(priv, SSD1680_DISP_CTRL2, 0xc7);
 
   /* Step 13: Master Activate and busy wait */
 
+  lcdinfo("Write master activate (0x%02x) command\n",
+      SSD1680_MASTER_ACTIVATE);
   ssd1680_snd_cmd_with_data0(priv, SSD1680_MASTER_ACTIVATE);
-  ssd1680_busy_wait(priv);
 
-  lcdinfo("Configuration ready\n");
-  priv->is_conf = true;
-  return OK;
+  ret = ssd1680_busy_wait(priv);
+exit:
+  if (ret == OK)
+    {
+      lcdinfo("Configuration ready\n");
+      priv->is_conf = true;
+    }
+  else

Review Comment:
   OK. I renamed the label **exit** into **exit_err**



-- 
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


[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #6510: Support for 2.13 inch display v2

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on PR #6510:
URL: https://github.com/apache/incubator-nuttx/pull/6510#issuecomment-1168149374

   @adamkaliszan will you fix the rest comment from @pkarashchenko ?


-- 
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


[GitHub] [incubator-nuttx] acassis commented on a diff in pull request #6510: Support for 2.13 inch display v2

Posted by GitBox <gi...@apache.org>.
acassis commented on code in PR #6510:
URL: https://github.com/apache/incubator-nuttx/pull/6510#discussion_r906413463


##########
drivers/lcd/ssd1680.c:
##########
@@ -577,39 +580,83 @@ static int ssd1680_configuredisplay(struct ssd1680_dev_s *priv)
    */
 
   lcdinfo("Set the driver output controll (0x%02x): %d 0x%02x\n",
-      SSD1680_DRIVER_CONTROL, SSD1680_DEV_NATIVE_YRES - 1,
+      SSD1680_DRIVER_CONTROL,
+      SSD1680_DEV_NATIVE_YRES - 1,
       SSD1680_DEV_GATE_LAYOUT);
   ssd1680_snd_cmd_with_data3(priv, SSD1680_DRIVER_CONTROL,
      (uint8_t)((SSD1680_DEV_NATIVE_YRES - 1) & 0xff),
      (SSD1680_DEV_NATIVE_YRES - 1) >> 8,
      SSD1680_DEV_GATE_LAYOUT);
 
+#if defined(CONFIG_LCD_SSD1680_2_13_V2)
+  /* Step 1a: Set analog block controll

Review Comment:
   ```suggestion
     /* Step 1a: Set analog block control



##########
drivers/lcd/ssd1680.c:
##########
@@ -577,39 +580,83 @@ static int ssd1680_configuredisplay(struct ssd1680_dev_s *priv)
    */
 
   lcdinfo("Set the driver output controll (0x%02x): %d 0x%02x\n",
-      SSD1680_DRIVER_CONTROL, SSD1680_DEV_NATIVE_YRES - 1,
+      SSD1680_DRIVER_CONTROL,
+      SSD1680_DEV_NATIVE_YRES - 1,
       SSD1680_DEV_GATE_LAYOUT);
   ssd1680_snd_cmd_with_data3(priv, SSD1680_DRIVER_CONTROL,
      (uint8_t)((SSD1680_DEV_NATIVE_YRES - 1) & 0xff),
      (SSD1680_DEV_NATIVE_YRES - 1) >> 8,
      SSD1680_DEV_GATE_LAYOUT);
 
+#if defined(CONFIG_LCD_SSD1680_2_13_V2)
+  /* Step 1a: Set analog block controll
+   * After reset thise register should have proper value
+   */
+
+  lcdinfo("Set analog block controll (0x74): 0x54\n");

Review Comment:
   ```suggestion
     lcdinfo("Set analog block control (0x74): 0x54\n");



##########
drivers/lcd/ssd1680.c:
##########
@@ -577,39 +580,83 @@ static int ssd1680_configuredisplay(struct ssd1680_dev_s *priv)
    */
 
   lcdinfo("Set the driver output controll (0x%02x): %d 0x%02x\n",
-      SSD1680_DRIVER_CONTROL, SSD1680_DEV_NATIVE_YRES - 1,
+      SSD1680_DRIVER_CONTROL,
+      SSD1680_DEV_NATIVE_YRES - 1,
       SSD1680_DEV_GATE_LAYOUT);
   ssd1680_snd_cmd_with_data3(priv, SSD1680_DRIVER_CONTROL,
      (uint8_t)((SSD1680_DEV_NATIVE_YRES - 1) & 0xff),
      (SSD1680_DEV_NATIVE_YRES - 1) >> 8,
      SSD1680_DEV_GATE_LAYOUT);
 
+#if defined(CONFIG_LCD_SSD1680_2_13_V2)
+  /* Step 1a: Set analog block controll
+   * After reset thise register should have proper value

Review Comment:
   ```suggestion
      * After reset this register should have proper value



##########
drivers/lcd/ssd1680.c:
##########
@@ -577,39 +580,83 @@ static int ssd1680_configuredisplay(struct ssd1680_dev_s *priv)
    */
 
   lcdinfo("Set the driver output controll (0x%02x): %d 0x%02x\n",
-      SSD1680_DRIVER_CONTROL, SSD1680_DEV_NATIVE_YRES - 1,
+      SSD1680_DRIVER_CONTROL,
+      SSD1680_DEV_NATIVE_YRES - 1,
       SSD1680_DEV_GATE_LAYOUT);
   ssd1680_snd_cmd_with_data3(priv, SSD1680_DRIVER_CONTROL,
      (uint8_t)((SSD1680_DEV_NATIVE_YRES - 1) & 0xff),
      (SSD1680_DEV_NATIVE_YRES - 1) >> 8,
      SSD1680_DEV_GATE_LAYOUT);
 
+#if defined(CONFIG_LCD_SSD1680_2_13_V2)
+  /* Step 1a: Set analog block controll
+   * After reset thise register should have proper value
+   */
+
+  lcdinfo("Set analog block controll (0x74): 0x54\n");
+  ssd1680_snd_cmd_with_data1(priv, SSD1680_SET_ANALOG_BLOCK_CTRL, 0x54);
+
+  /* Step 1b: Set digital block controll
+   * After reset thise register should have proper value
+   */
+
+  lcdinfo("Set digital block controll (0x7e): 0x3b\n");

Review Comment:
   ```suggestion
     lcdinfo("Set digital block control (0x7e): 0x3b\n");



##########
drivers/lcd/ssd1680.c:
##########
@@ -577,39 +580,83 @@ static int ssd1680_configuredisplay(struct ssd1680_dev_s *priv)
    */
 
   lcdinfo("Set the driver output controll (0x%02x): %d 0x%02x\n",
-      SSD1680_DRIVER_CONTROL, SSD1680_DEV_NATIVE_YRES - 1,
+      SSD1680_DRIVER_CONTROL,
+      SSD1680_DEV_NATIVE_YRES - 1,
       SSD1680_DEV_GATE_LAYOUT);
   ssd1680_snd_cmd_with_data3(priv, SSD1680_DRIVER_CONTROL,
      (uint8_t)((SSD1680_DEV_NATIVE_YRES - 1) & 0xff),
      (SSD1680_DEV_NATIVE_YRES - 1) >> 8,
      SSD1680_DEV_GATE_LAYOUT);
 
+#if defined(CONFIG_LCD_SSD1680_2_13_V2)
+  /* Step 1a: Set analog block controll
+   * After reset thise register should have proper value
+   */
+
+  lcdinfo("Set analog block controll (0x74): 0x54\n");
+  ssd1680_snd_cmd_with_data1(priv, SSD1680_SET_ANALOG_BLOCK_CTRL, 0x54);
+
+  /* Step 1b: Set digital block controll

Review Comment:
   ```suggestion
     /* Step 1b: Set digital block control



##########
drivers/lcd/ssd1680.c:
##########
@@ -671,69 +731,28 @@ static int ssd1680_configuredisplay(struct ssd1680_dev_s *priv)
  *   priv   - Reference to private driver structure
  *
  * Assumptions:
- *   Caller has selected the OLED section.
+ *   E-ink is slow, so we don't need to optimize cod in order to send all

Review Comment:
   ```suggestion
    *   E-ink is slow, so we don't need to optimize code in order to send all



-- 
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


[GitHub] [incubator-nuttx] xiaoxiang781216 merged pull request #6510: Support for 2.13 inch display v2

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 merged PR #6510:
URL: https://github.com/apache/incubator-nuttx/pull/6510


-- 
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


[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #6510: Support for 2.13 inch display v2

Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #6510:
URL: https://github.com/apache/incubator-nuttx/pull/6510#discussion_r908395347


##########
drivers/lcd/ssd1680.c:
##########
@@ -878,19 +896,29 @@ static void ssd1680_select(FAR struct ssd1680_dev_s *priv, bool cs)
     }
 }
 
-static void ssd1680_busy_wait(FAR struct ssd1680_dev_s *priv)
+static int ssd1680_busy_wait(FAR struct ssd1680_dev_s *priv)
 {
+  int maks_wait_time = 200;

Review Comment:
   ```suggestion
     int max_wait_time = 200;
   ```



-- 
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


[GitHub] [incubator-nuttx] adamkaliszan commented on a diff in pull request #6510: Support for 2.13 inch display v2

Posted by GitBox <gi...@apache.org>.
adamkaliszan commented on code in PR #6510:
URL: https://github.com/apache/incubator-nuttx/pull/6510#discussion_r908458710


##########
drivers/lcd/ssd1680.c:
##########
@@ -878,19 +896,29 @@ static void ssd1680_select(FAR struct ssd1680_dev_s *priv, bool cs)
     }
 }
 
-static void ssd1680_busy_wait(FAR struct ssd1680_dev_s *priv)
+static int ssd1680_busy_wait(FAR struct ssd1680_dev_s *priv)
 {
+  int maks_wait_time = 200;

Review Comment:
   OK is fixed. Sorry for inconvinience.



-- 
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