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 2020/09/14 06:09:02 UTC

[GitHub] [incubator-nuttx] btashton opened a new pull request #1778: WIP board: Keyboard FeatherWing on nrf52-feather

btashton opened a new pull request #1778:
URL: https://github.com/apache/incubator-nuttx/pull/1778


   ## Summary
   This adds support for using the Solder Party FeatherWing to the NRF52 Feather Board
   https://www.solder.party/docs/keyboard-featherwing/
   
   Supported Hardware Interfaces:
   - [x] LCD
   - [ ] Touch Screen
   - [ ] 5-way Button
   - [ ] 4 Soft Buttons
   - [ ] Keyboard
   - [ ] NeoPixel
   - [ ] Qwiic / STEMMA QT I2C port
   
   
   ## Testing
   
   ### LCD running NX demo application:
   ![image](https://user-images.githubusercontent.com/173245/93049286-9010fd00-f615-11ea-822c-c04797195d65.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.

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



[GitHub] [incubator-nuttx] patacongo commented on a change in pull request #1778: WIP board: Keyboard FeatherWing on nrf52-feather

Posted by GitBox <gi...@apache.org>.
patacongo commented on a change in pull request #1778:
URL: https://github.com/apache/incubator-nuttx/pull/1778#discussion_r487983115



##########
File path: boards/arm/nrf52/nrf52-feather/src/nrf52_ili9341_lcd.c
##########
@@ -0,0 +1,410 @@
+/****************************************************************************
+ * boards/arm/nrf52/nrf52-feather/src/nrf52_ili9341_lcd.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <stdint.h>
+#include <stdbool.h>
+#include <endian.h>
+#include <debug.h>
+
+#include <nuttx/spi/spi.h>
+#include <nuttx/lcd/lcd.h>
+#include <nuttx/lcd/ili9341.h>
+
+#include "chip.h"
+#include "arm_arch.h"
+#include "arm_internal.h"
+#include "nrf52_gpio.h"
+#include "nrf52_spi.h"
+
+#include "nrf52-feather.h"
+#include <arch/board/board.h>
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+#define GRAM_ENDIAN_SWAP_BUFFER_LEN 1    /* Gram swap space */
+
+#if (GRAM_ENDIAN_SWAP_BUFFER_LEN > 127)
+#  error "GRAM_ENDIAN_SWAP_BUFFER_LEN must fit in a SPI transfer 127 words!"
+#endif
+
+/****************************************************************************
+ * Private Type Definition
+ ****************************************************************************/
+
+struct ili93414ws_lcd_s
+{
+  struct ili9341_lcd_s dev;
+  FAR struct spi_dev_s *spi;
+};
+
+/****************************************************************************
+ * Private Function Protototypes
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+static struct ili93414ws_lcd_s g_lcddev;
+static struct lcd_dev_s *g_lcd = NULL;
+static uint8_t swapbuf[GRAM_ENDIAN_SWAP_BUFFER_LEN * 2];
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: nrf52_ili93414ws_backlight
+ *
+ * Description:
+ *   Set the backlight level of the connected display.
+ *
+ * Input Parameters:
+ *   spi   - Reference to the public driver structure
+ *   level - backlight level
+ *
+ * Returned Value:
+ *   OK - On Success
+ *
+ ****************************************************************************/
+
+static int nrf52_ili93414ws_backlight(FAR struct ili9341_lcd_s *lcd,
+                                      int level)
+{
+  return OK;
+}
+
+/****************************************************************************
+ * Name: nrf52_ili93414ws_select
+ *
+ * Description:
+ *   Select the SPI, locking and re-configuring if necessary
+ *
+ * Input Parameters:
+ *   spi  - Reference to the public driver structure
+ *
+ * Returned Value:
+ *
+ ****************************************************************************/
+
+static void nrf52_ili93414ws_select(FAR struct ili9341_lcd_s *lcd)
+{
+  FAR struct ili93414ws_lcd_s *priv = (FAR struct ili93414ws_lcd_s *)lcd;
+
+  SPI_LOCK(priv->spi, true);
+  SPI_SELECT(priv->spi, SPIDEV_DISPLAY(0), true);
+}
+
+/****************************************************************************
+ * Name: nrf52_ili93414ws_deselect
+ *
+ * Description:
+ *   De-select the SPI
+ *
+ * Input Parameters:
+ *   spi  - Reference to the public driver structure
+ *
+ * Returned Value:
+ *
+ ****************************************************************************/
+
+static void nrf52_ili93414ws_deselect(FAR struct ili9341_lcd_s *lcd)
+{
+  FAR struct ili93414ws_lcd_s *priv = (FAR struct ili93414ws_lcd_s *)lcd;
+
+  SPI_LOCK(priv->spi, false);
+  SPI_SELECT(priv->spi, SPIDEV_DISPLAY(0), false);
+}
+
+/****************************************************************************
+ * Name: nrf52_ili93414ws_sndcmd
+ *
+ * Description:
+ *   Send a command to the lcd driver.
+ *
+ * Input Parameters:
+ *   lcd  - Reference to the ili9341_lcd_s driver structure
+ *   cmd  - command to send
+ *
+ * Returned Value:
+ *   On success - OK
+ *
+ ****************************************************************************/
+
+static int nrf52_ili93414ws_sendcmd(
+    FAR struct ili9341_lcd_s *lcd, const uint8_t cmd)
+{
+  FAR struct ili93414ws_lcd_s *priv = (FAR struct ili93414ws_lcd_s *)lcd;
+
+  lcdinfo("%02x\n", cmd);
+
+  nrf52_gpio_write(ILI9341_DISPLAY_DC, false); /* Indicate CMD */
+  SPI_SEND(priv->spi, cmd);
+
+  return OK;
+}
+
+/****************************************************************************
+ * Name: nrf52_ili93414ws_sendparam
+ *
+ * Description:
+ *   Send a parameter to the lcd driver.
+ *
+ * Input Parameters:
+ *   lcd    - Reference to the ili9341_lcd_s driver structure
+ *   param  - parameter to send
+ *
+ * Returned Value:
+ *   OK - On Success
+ *
+ ****************************************************************************/
+
+static int nrf52_ili93414ws_sendparam(FAR struct ili9341_lcd_s *lcd,
+                                      const uint8_t param)
+{
+  FAR struct ili93414ws_lcd_s *priv = (FAR struct ili93414ws_lcd_s *)lcd;
+
+  nrf52_gpio_write(ILI9341_DISPLAY_DC, true);  /* Indicate DATA */
+  SPI_SEND(priv->spi, param);
+
+  return OK;
+}
+
+/****************************************************************************
+ * Name: nrf52_ili93414ws_sendgram
+ *
+ * Description:
+ *   Send a number of pixel words to the lcd driver gram.
+ *
+ * Input Parameters:
+ *   lcd    - Reference to the ili9341_lcd_s driver structure
+ *   wd     - Reference to the words to send
+ *   nwords - number of words to send
+ *
+ * Returned Value:
+ *   OK - On Success
+ *
+ ****************************************************************************/
+
+static int nrf52_ili93414ws_sendgram(FAR struct ili9341_lcd_s *lcd,

Review comment:
       > The location where the blitting is done is in graphics/nxglib/lcd. But that uses macros defined in graphics/nxglib/nxglib_bitblit.h.
   
   Sorry, that is bad advice.  Those macros are only used by the framebuffer bit-blitters (graphics/nxglib/fb).  The blitting is hardcoded for the LCD in the files under graphics/nxglib/lcd.
   
   So you can't change the blitting in one place but rather in all of the 6 files at that location.  Sorry.
   
   Hmm.. there is common logic used for the LCD blitters in graphics/nxglib/nxglib_copyrun.h and graphics/nxglib/nxglib_fillrun.h.  Those will cover most, but not all, of the blit operations in graphics/nxglib/lcd.
   
   




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

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



[GitHub] [incubator-nuttx] patacongo commented on a change in pull request #1778: WIP board: Keyboard FeatherWing on nrf52-feather

Posted by GitBox <gi...@apache.org>.
patacongo commented on a change in pull request #1778:
URL: https://github.com/apache/incubator-nuttx/pull/1778#discussion_r488026470



##########
File path: boards/arm/nrf52/nrf52-feather/Kconfig
##########
@@ -5,4 +5,54 @@
 
 if ARCH_BOARD_NRF52_FEATHER
 
-endif
+menuconfig NRF52_FEATHER_KB_FEATHERWING
+	bool "Keyboard FeatherWing"
+    default n
+    ---help---
+        Enable support for the SolderParty FeatherWing
+        https://www.solder.party/docs/keyboard-featherwing/
+
+if NRF52_FEATHER_KB_FEATHERWING
+config KB_FEATHERWING_LCD

Review comment:
       > 
   > 
   > > One question:  The use of the FeatherWing does not depend on NRF really.  It could be selected for any Feather board potentially.  So I wonder if this does not below in drivers/lcd and if it should not be named LCD_FEATHER_KB_FEATHERWING.
   > 
   > If we do that what would the difference be between LCD_FEATHER_KB_FEATHERWING and LCD_ILI9341? Seems like this would belong in `boards/` maybe in an expansion boards folder where there would be Kconfigs for different addon hardware for devkits?
   
   Yeah, drivers/lcd is not the perfect place.  But the point is that the featherwing is usable across all feather architectures.




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

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



[GitHub] [incubator-nuttx] patacongo commented on a change in pull request #1778: WIP board: Keyboard FeatherWing on nrf52-feather

Posted by GitBox <gi...@apache.org>.
patacongo commented on a change in pull request #1778:
URL: https://github.com/apache/incubator-nuttx/pull/1778#discussion_r487963450



##########
File path: boards/arm/nrf52/nrf52-feather/src/nrf52_ili9341_lcd.c
##########
@@ -0,0 +1,410 @@
+/****************************************************************************
+ * boards/arm/nrf52/nrf52-feather/src/nrf52_ili9341_lcd.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <stdint.h>
+#include <stdbool.h>
+#include <endian.h>
+#include <debug.h>
+
+#include <nuttx/spi/spi.h>
+#include <nuttx/lcd/lcd.h>
+#include <nuttx/lcd/ili9341.h>
+
+#include "chip.h"
+#include "arm_arch.h"
+#include "arm_internal.h"
+#include "nrf52_gpio.h"
+#include "nrf52_spi.h"
+
+#include "nrf52-feather.h"
+#include <arch/board/board.h>
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+#define GRAM_ENDIAN_SWAP_BUFFER_LEN 1    /* Gram swap space */
+
+#if (GRAM_ENDIAN_SWAP_BUFFER_LEN > 127)
+#  error "GRAM_ENDIAN_SWAP_BUFFER_LEN must fit in a SPI transfer 127 words!"
+#endif
+
+/****************************************************************************
+ * Private Type Definition
+ ****************************************************************************/
+
+struct ili93414ws_lcd_s
+{
+  struct ili9341_lcd_s dev;
+  FAR struct spi_dev_s *spi;
+};
+
+/****************************************************************************
+ * Private Function Protototypes
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+static struct ili93414ws_lcd_s g_lcddev;
+static struct lcd_dev_s *g_lcd = NULL;
+static uint8_t swapbuf[GRAM_ENDIAN_SWAP_BUFFER_LEN * 2];
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: nrf52_ili93414ws_backlight
+ *
+ * Description:
+ *   Set the backlight level of the connected display.
+ *
+ * Input Parameters:
+ *   spi   - Reference to the public driver structure
+ *   level - backlight level
+ *
+ * Returned Value:
+ *   OK - On Success
+ *
+ ****************************************************************************/
+
+static int nrf52_ili93414ws_backlight(FAR struct ili9341_lcd_s *lcd,
+                                      int level)
+{
+  return OK;
+}
+
+/****************************************************************************
+ * Name: nrf52_ili93414ws_select
+ *
+ * Description:
+ *   Select the SPI, locking and re-configuring if necessary
+ *
+ * Input Parameters:
+ *   spi  - Reference to the public driver structure
+ *
+ * Returned Value:
+ *
+ ****************************************************************************/
+
+static void nrf52_ili93414ws_select(FAR struct ili9341_lcd_s *lcd)
+{
+  FAR struct ili93414ws_lcd_s *priv = (FAR struct ili93414ws_lcd_s *)lcd;
+
+  SPI_LOCK(priv->spi, true);
+  SPI_SELECT(priv->spi, SPIDEV_DISPLAY(0), true);
+}
+
+/****************************************************************************
+ * Name: nrf52_ili93414ws_deselect
+ *
+ * Description:
+ *   De-select the SPI
+ *
+ * Input Parameters:
+ *   spi  - Reference to the public driver structure
+ *
+ * Returned Value:
+ *
+ ****************************************************************************/
+
+static void nrf52_ili93414ws_deselect(FAR struct ili9341_lcd_s *lcd)
+{
+  FAR struct ili93414ws_lcd_s *priv = (FAR struct ili93414ws_lcd_s *)lcd;
+
+  SPI_LOCK(priv->spi, false);
+  SPI_SELECT(priv->spi, SPIDEV_DISPLAY(0), false);
+}
+
+/****************************************************************************
+ * Name: nrf52_ili93414ws_sndcmd
+ *
+ * Description:
+ *   Send a command to the lcd driver.
+ *
+ * Input Parameters:
+ *   lcd  - Reference to the ili9341_lcd_s driver structure
+ *   cmd  - command to send
+ *
+ * Returned Value:
+ *   On success - OK
+ *
+ ****************************************************************************/
+
+static int nrf52_ili93414ws_sendcmd(
+    FAR struct ili9341_lcd_s *lcd, const uint8_t cmd)
+{
+  FAR struct ili93414ws_lcd_s *priv = (FAR struct ili93414ws_lcd_s *)lcd;
+
+  lcdinfo("%02x\n", cmd);
+
+  nrf52_gpio_write(ILI9341_DISPLAY_DC, false); /* Indicate CMD */
+  SPI_SEND(priv->spi, cmd);
+
+  return OK;
+}
+
+/****************************************************************************
+ * Name: nrf52_ili93414ws_sendparam
+ *
+ * Description:
+ *   Send a parameter to the lcd driver.
+ *
+ * Input Parameters:
+ *   lcd    - Reference to the ili9341_lcd_s driver structure
+ *   param  - parameter to send
+ *
+ * Returned Value:
+ *   OK - On Success
+ *
+ ****************************************************************************/
+
+static int nrf52_ili93414ws_sendparam(FAR struct ili9341_lcd_s *lcd,
+                                      const uint8_t param)
+{
+  FAR struct ili93414ws_lcd_s *priv = (FAR struct ili93414ws_lcd_s *)lcd;
+
+  nrf52_gpio_write(ILI9341_DISPLAY_DC, true);  /* Indicate DATA */
+  SPI_SEND(priv->spi, param);
+
+  return OK;
+}
+
+/****************************************************************************
+ * Name: nrf52_ili93414ws_sendgram
+ *
+ * Description:
+ *   Send a number of pixel words to the lcd driver gram.
+ *
+ * Input Parameters:
+ *   lcd    - Reference to the ili9341_lcd_s driver structure
+ *   wd     - Reference to the words to send
+ *   nwords - number of words to send
+ *
+ * Returned Value:
+ *   OK - On Success
+ *
+ ****************************************************************************/
+
+static int nrf52_ili93414ws_sendgram(FAR struct ili9341_lcd_s *lcd,

Review comment:
       > 
   > 
   > @patacongo for the gram send recv interfaces I had to add a byte swapping buffer because nrf52 SPI hardware does not support 16bit SPI. If there was a way to tell the whole graphics stack to use BE instead of LE for color this could be avoided. The buffer can be configured in size, but right now it is just set to the minimal of a single gram this also means that there is no real DMA advantage since the transfers are limited to the size of this buffer.
   
   The location where the blitting is done is in graphics/nxglib/lcd.
   
   _[Bad advice follows]_
   But that uses macros defined in graphics/nxglib/nxglib_bitblit.h.  There you can see macros for:
   
       116 #if NXGLIB_BITSPERPIXEL < 8
       144 #elif NXGLIB_BITSPERPIXEL == 24
       188 #else /* NXGLIB_BITSPERPIXEL == 16 || NXGLIB_BITSPERPIXEL == 32 */
       221 #endif /* NXGLIB_BITSPERPIXEL */
   
   You can see that for the 8, 16, and 32 bit version its just uses the native machine byte order (notice that the comment is wrong at line 188).  I think you would need to add and new configuration setting specifying the endianness of the RGB data.
   
   I wouldn't just swap bytes, I would specifically ask for endian-ness because if this were used a big-endian machine that you would not want to swap.  The setting CONFIG_BIG_ENDIAN provides the native endian-ness.  You would swap only if !defined(CONFIG_BIG_ENDIAN) && defined(big-endian-LCD).




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

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



[GitHub] [incubator-nuttx] patacongo commented on a change in pull request #1778: WIP board: Keyboard FeatherWing on nrf52-feather

Posted by GitBox <gi...@apache.org>.
patacongo commented on a change in pull request #1778:
URL: https://github.com/apache/incubator-nuttx/pull/1778#discussion_r487948071



##########
File path: boards/arm/nrf52/nrf52-feather/Kconfig
##########
@@ -5,4 +5,54 @@
 
 if ARCH_BOARD_NRF52_FEATHER
 
-endif
+menuconfig NRF52_FEATHER_KB_FEATHERWING
+	bool "Keyboard FeatherWing"
+    default n
+    ---help---
+        Enable support for the SolderParty FeatherWing
+        https://www.solder.party/docs/keyboard-featherwing/
+
+if NRF52_FEATHER_KB_FEATHERWING
+config KB_FEATHERWING_LCD

Review comment:
       > 
   > 
   > @patacongo what are your thoughts here on how this calls select? Originally I had the LCD dependent on LCD_ILI9341 and no selects, but here it seems like you would want as a user to enable the top level hardware you have and have it turn on the required hardware.
   > 
   > I am fine changing this back if you think that makes sense. I just was thinking this would make is easy for a user.
   
   I agree with you.  FEATHERWING is like a "macro" that just automatically supports everything you need for the board.
   
   One question:  The use of the FeatherWing does not depend on NRF really.  It could be selected for any Feather board potentially.  So I wonder if this does not below in drivers/lcd and if it should not be named LCD_FEATHER_KB_FEATHERWING.
   




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

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



[GitHub] [incubator-nuttx] patacongo commented on pull request #1778: WIP board: Keyboard FeatherWing on nrf52-feather

Posted by GitBox <gi...@apache.org>.
patacongo commented on pull request #1778:
URL: https://github.com/apache/incubator-nuttx/pull/1778#issuecomment-692070606


   Cool!  what kind of performance to you get with the LCD?  I imagine it needs a really high SPI rate and maybe DMA to be fully satisfactory.
   
   NX is a windowing system.  NxWidgets is the companion library for buttons and the like.  LVGL is a non-windowed option.  LVGL should have lower resource usage and probably better performance since less clipping involved.
   
   Another option option is pdcurses.  Ken Pettit did some great displays (and even added logic to remote the curses display.  You would have to use a NxTerm for pdcurses.


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

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



[GitHub] [incubator-nuttx] btashton commented on a change in pull request #1778: WIP board: Keyboard FeatherWing on nrf52-feather

Posted by GitBox <gi...@apache.org>.
btashton commented on a change in pull request #1778:
URL: https://github.com/apache/incubator-nuttx/pull/1778#discussion_r487999097



##########
File path: boards/arm/nrf52/nrf52-feather/Kconfig
##########
@@ -5,4 +5,54 @@
 
 if ARCH_BOARD_NRF52_FEATHER
 
-endif
+menuconfig NRF52_FEATHER_KB_FEATHERWING
+	bool "Keyboard FeatherWing"
+    default n
+    ---help---
+        Enable support for the SolderParty FeatherWing
+        https://www.solder.party/docs/keyboard-featherwing/
+
+if NRF52_FEATHER_KB_FEATHERWING
+config KB_FEATHERWING_LCD

Review comment:
       > One question:  The use of the FeatherWing does not depend on NRF really.  It could be selected for any Feather board potentially.  So I wonder if this does not below in drivers/lcd and if it should not be named LCD_FEATHER_KB_FEATHERWING.
   > 
   
   If we do that what would the difference be between LCD_FEATHER_KB_FEATHERWING and LCD_ILI9341?  Seems like this would belong in `boards/` maybe in an expansion boards folder where there would be Kconfigs for different addon hardware for devkits?




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

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



[GitHub] [incubator-nuttx] btashton commented on pull request #1778: WIP board: Keyboard FeatherWing on nrf52-feather

Posted by GitBox <gi...@apache.org>.
btashton commented on pull request #1778:
URL: https://github.com/apache/incubator-nuttx/pull/1778#issuecomment-693552570


   Looks like the stmpe811 driver currently does not support SPI, so I will have to add that in.   Also unfortunately the interrupt pin for this chip is not connected to the feather interface, but there is a very large test point that could be jumpered to support this.  I'll try to get a PR up against this driver sometime today.


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

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



[GitHub] [incubator-nuttx] btashton edited a comment on pull request #1778: WIP board: Keyboard FeatherWing on nrf52-feather

Posted by GitBox <gi...@apache.org>.
btashton edited a comment on pull request #1778:
URL: https://github.com/apache/incubator-nuttx/pull/1778#issuecomment-692107744


   > Cool!  what kind of performance to you get with the LCD?  I imagine it needs a really high SPI rate and maybe DMA to be fully satisfactory.
   
   So unfortunately this chip nRF52832 maxes out at 8MHz but the LCD can go much higher. There is another version the nRF52840 which is capable of 32MHz.
   
   When I allocated a couple rows and wrote them via DMA it was ok for slow tasks like the background. 
   
   > 
   > NX is a windowing system.  NxWidgets is the companion library for buttons and the like.  LVGL is a non-windowed option.  LVGL should have lower resource usage and probably better performance since less clipping involved.
   > 
   
   I agree, this was just easier to get going and I had never used NXGLIB before os I thought I would give it a try.  One thing I noticed is it seems that each pixel was being written individually which is super slow compared to writing a "rectangle" which could be DMA.  I did not dig into this yet.
   
   > Another option option is pdcurses.  Ken Pettit did some great displays (and even added logic to remote the curses display.  You would have to use a NxTerm for pdcurses.
   
   Yeah that would be a cool little demo with the keyboard.
   
   


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

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



[GitHub] [incubator-nuttx] btashton commented on pull request #1778: WIP board: Keyboard FeatherWing on nrf52-feather

Posted by GitBox <gi...@apache.org>.
btashton commented on pull request #1778:
URL: https://github.com/apache/incubator-nuttx/pull/1778#issuecomment-692107744


   > Cool!  what kind of performance to you get with the LCD?  I imagine it needs a really high SPI rate and maybe DMA to be fully satisfactory.
   
   So unfortunately this chip nRF52838 maxes out at 8MHz but the LCD can go much higher. There is another version the nRF52840 which is capable of 32MHz.
   
   When I allocated a couple rows and wrote them via DMA it was ok for slow tasks like the background. 
   
   > 
   > NX is a windowing system.  NxWidgets is the companion library for buttons and the like.  LVGL is a non-windowed option.  LVGL should have lower resource usage and probably better performance since less clipping involved.
   > 
   
   I agree, this was just easier to get going and I had never used NXGLIB before os I thought I would give it a try.  One thing I noticed is it seems that each pixel was being written individually which is super slow compared to writing a "rectangle" which could be DMA.  I did not dig into this yet.
   
   > Another option option is pdcurses.  Ken Pettit did some great displays (and even added logic to remote the curses display.  You would have to use a NxTerm for pdcurses.
   
   Yeah that would be a cool little demo with the keyboard.
   
   


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

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



[GitHub] [incubator-nuttx] patacongo commented on pull request #1778: WIP board: Keyboard FeatherWing on nrf52-feather

Posted by GitBox <gi...@apache.org>.
patacongo commented on pull request #1778:
URL: https://github.com/apache/incubator-nuttx/pull/1778#issuecomment-692187632


   Some of this is a little confusing to me.
   
   First, let me recommend https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=139629398 which is my most recent attempt to describe the graphics sub-system.
   
   > > > One thing I noticed is it seems that each pixel was being written individually which is super slow compared to writing a "rectangle" which could be DMA. I did not dig into this yet.
   > > 
   > > 
   > > No, the NX LCD interface outputs runs: A horizontal extent of pixels. This has been discussion that this limits performance for painting large regions that are not bounded by the line length. But this is not a performance limitation for drawing rectangles that are inherently bounded in the window.
   > 
   > I understand you are using nxgraphics with LCD interface right? As I mentioned in the past this really limited performance when wanting to transfer various lines in one go. I have in my TODO list to add support for putrun() to accept npixels > rowsize for such displays but I never got to do it. Anyway, as Greg says, this will not improve NX since putrun() is used once per line obviously.
   > I used LVGL via a character driver which exposed the LCD interface and this allowed me to send putrun() for many rows.
   > Contributing this to mainline is also in my backlog since I have to go back to test with another board to do so. If you're interested in trying it, it is here: https://gitlab.com/nuttx_projects/lcd_dev
   > Note that it uses a non-standard putrows() since this was my solution to the aforementioned problem, but as we discussed previously, extending putrun() would be easier.
   
   NOTE that LVGL does not use the NxGraphics sub-system.  It uses a framebuffer driver that bypasses the entire graphics sub-system and interfaces directly to the LCD driver.  No discussion regarding putrun in that context would apply in the NxGraphics context.
   
   The framebuffer driver is a small bag hanging off the side of the graphics sub-system and not really integrated into the design.
   
   I think it would be worthwhile to consider the overall graphics design and flow of data before focusing on some minute details that are not part of the graphics subsystem.  I think we need to take a fully integrated approach to anything we do.
   
   


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

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



[GitHub] [incubator-nuttx] patacongo commented on a change in pull request #1778: WIP board: Keyboard FeatherWing on nrf52-feather

Posted by GitBox <gi...@apache.org>.
patacongo commented on a change in pull request #1778:
URL: https://github.com/apache/incubator-nuttx/pull/1778#discussion_r487963450



##########
File path: boards/arm/nrf52/nrf52-feather/src/nrf52_ili9341_lcd.c
##########
@@ -0,0 +1,410 @@
+/****************************************************************************
+ * boards/arm/nrf52/nrf52-feather/src/nrf52_ili9341_lcd.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <stdint.h>
+#include <stdbool.h>
+#include <endian.h>
+#include <debug.h>
+
+#include <nuttx/spi/spi.h>
+#include <nuttx/lcd/lcd.h>
+#include <nuttx/lcd/ili9341.h>
+
+#include "chip.h"
+#include "arm_arch.h"
+#include "arm_internal.h"
+#include "nrf52_gpio.h"
+#include "nrf52_spi.h"
+
+#include "nrf52-feather.h"
+#include <arch/board/board.h>
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+#define GRAM_ENDIAN_SWAP_BUFFER_LEN 1    /* Gram swap space */
+
+#if (GRAM_ENDIAN_SWAP_BUFFER_LEN > 127)
+#  error "GRAM_ENDIAN_SWAP_BUFFER_LEN must fit in a SPI transfer 127 words!"
+#endif
+
+/****************************************************************************
+ * Private Type Definition
+ ****************************************************************************/
+
+struct ili93414ws_lcd_s
+{
+  struct ili9341_lcd_s dev;
+  FAR struct spi_dev_s *spi;
+};
+
+/****************************************************************************
+ * Private Function Protototypes
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+static struct ili93414ws_lcd_s g_lcddev;
+static struct lcd_dev_s *g_lcd = NULL;
+static uint8_t swapbuf[GRAM_ENDIAN_SWAP_BUFFER_LEN * 2];
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: nrf52_ili93414ws_backlight
+ *
+ * Description:
+ *   Set the backlight level of the connected display.
+ *
+ * Input Parameters:
+ *   spi   - Reference to the public driver structure
+ *   level - backlight level
+ *
+ * Returned Value:
+ *   OK - On Success
+ *
+ ****************************************************************************/
+
+static int nrf52_ili93414ws_backlight(FAR struct ili9341_lcd_s *lcd,
+                                      int level)
+{
+  return OK;
+}
+
+/****************************************************************************
+ * Name: nrf52_ili93414ws_select
+ *
+ * Description:
+ *   Select the SPI, locking and re-configuring if necessary
+ *
+ * Input Parameters:
+ *   spi  - Reference to the public driver structure
+ *
+ * Returned Value:
+ *
+ ****************************************************************************/
+
+static void nrf52_ili93414ws_select(FAR struct ili9341_lcd_s *lcd)
+{
+  FAR struct ili93414ws_lcd_s *priv = (FAR struct ili93414ws_lcd_s *)lcd;
+
+  SPI_LOCK(priv->spi, true);
+  SPI_SELECT(priv->spi, SPIDEV_DISPLAY(0), true);
+}
+
+/****************************************************************************
+ * Name: nrf52_ili93414ws_deselect
+ *
+ * Description:
+ *   De-select the SPI
+ *
+ * Input Parameters:
+ *   spi  - Reference to the public driver structure
+ *
+ * Returned Value:
+ *
+ ****************************************************************************/
+
+static void nrf52_ili93414ws_deselect(FAR struct ili9341_lcd_s *lcd)
+{
+  FAR struct ili93414ws_lcd_s *priv = (FAR struct ili93414ws_lcd_s *)lcd;
+
+  SPI_LOCK(priv->spi, false);
+  SPI_SELECT(priv->spi, SPIDEV_DISPLAY(0), false);
+}
+
+/****************************************************************************
+ * Name: nrf52_ili93414ws_sndcmd
+ *
+ * Description:
+ *   Send a command to the lcd driver.
+ *
+ * Input Parameters:
+ *   lcd  - Reference to the ili9341_lcd_s driver structure
+ *   cmd  - command to send
+ *
+ * Returned Value:
+ *   On success - OK
+ *
+ ****************************************************************************/
+
+static int nrf52_ili93414ws_sendcmd(
+    FAR struct ili9341_lcd_s *lcd, const uint8_t cmd)
+{
+  FAR struct ili93414ws_lcd_s *priv = (FAR struct ili93414ws_lcd_s *)lcd;
+
+  lcdinfo("%02x\n", cmd);
+
+  nrf52_gpio_write(ILI9341_DISPLAY_DC, false); /* Indicate CMD */
+  SPI_SEND(priv->spi, cmd);
+
+  return OK;
+}
+
+/****************************************************************************
+ * Name: nrf52_ili93414ws_sendparam
+ *
+ * Description:
+ *   Send a parameter to the lcd driver.
+ *
+ * Input Parameters:
+ *   lcd    - Reference to the ili9341_lcd_s driver structure
+ *   param  - parameter to send
+ *
+ * Returned Value:
+ *   OK - On Success
+ *
+ ****************************************************************************/
+
+static int nrf52_ili93414ws_sendparam(FAR struct ili9341_lcd_s *lcd,
+                                      const uint8_t param)
+{
+  FAR struct ili93414ws_lcd_s *priv = (FAR struct ili93414ws_lcd_s *)lcd;
+
+  nrf52_gpio_write(ILI9341_DISPLAY_DC, true);  /* Indicate DATA */
+  SPI_SEND(priv->spi, param);
+
+  return OK;
+}
+
+/****************************************************************************
+ * Name: nrf52_ili93414ws_sendgram
+ *
+ * Description:
+ *   Send a number of pixel words to the lcd driver gram.
+ *
+ * Input Parameters:
+ *   lcd    - Reference to the ili9341_lcd_s driver structure
+ *   wd     - Reference to the words to send
+ *   nwords - number of words to send
+ *
+ * Returned Value:
+ *   OK - On Success
+ *
+ ****************************************************************************/
+
+static int nrf52_ili93414ws_sendgram(FAR struct ili9341_lcd_s *lcd,

Review comment:
       > 
   > 
   > @patacongo for the gram send recv interfaces I had to add a byte swapping buffer because nrf52 SPI hardware does not support 16bit SPI. If there was a way to tell the whole graphics stack to use BE instead of LE for color this could be avoided. The buffer can be configured in size, but right now it is just set to the minimal of a single gram this also means that there is no real DMA advantage since the transfers are limited to the size of this buffer.
   
   The location where the blitting is done is in graphics/nxglib/lcd.  But that uses macros defined in graphics/nxglib/nxglib_bitblit.h.  There you can see macros for:
   
       116 #if NXGLIB_BITSPERPIXEL < 8
       144 #elif NXGLIB_BITSPERPIXEL == 24
       188 #else /* NXGLIB_BITSPERPIXEL == 16 || NXGLIB_BITSPERPIXEL == 32 */
       221 #endif /* NXGLIB_BITSPERPIXEL */
   
   You can see that for the 8, 16, and 32 bit version its just uses the native machine byte order (notice that the comment is wrong at line 188).  I think you would need to add and new configuration setting specifying the endianness of the RGB data.
   
   I wouldn't just swap bytes, I would specifically ask for endian-ness because if this were used a big-endian machine that you would not want to swap.  The setting CONFIG_BIG_ENDIAN provides the native endian-ness.  You would swap only if !defined(CONFIG_BIG_ENDIAN) && defined(big-endian-LCD).




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

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



[GitHub] [incubator-nuttx] btashton commented on pull request #1778: WIP board: Keyboard FeatherWing on nrf52-feather

Posted by GitBox <gi...@apache.org>.
btashton commented on pull request #1778:
URL: https://github.com/apache/incubator-nuttx/pull/1778#issuecomment-692166575


   Yeah that is where I was saying having the ability for NX to render "rectangles" that are short in the X axis would be nice because right now a vertical line is expensive.
   
   I did set my gram byte swap buffer to the max of 127 grams for DMA and it improved the performance quite a bit.  You can see it here:
   https://media.giphy.com/media/j736EdTwUBwQHiPKhS/giphy.gif
   
   I am going to leave the performance bits aside for now and work on adding the other hardware support.


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

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



[GitHub] [incubator-nuttx] patacongo commented on pull request #1778: WIP board: Keyboard FeatherWing on nrf52-feather

Posted by GitBox <gi...@apache.org>.
patacongo commented on pull request #1778:
URL: https://github.com/apache/incubator-nuttx/pull/1778#issuecomment-692133625


   > One thing I noticed is it seems that each pixel was being written individually which is super slow compared to writing a "rectangle" which could be DMA. I did not dig into this yet.
   
   No, the NX LCD interface outputs runs:  A horizontal extent of pixels.  This has been discussion that this limits performance for painting large regions that are not bounded by the line length.  But this is not a performance limitation for drawing rectangles that are inherently bounded in the window.
   
   The ILI9341 lower-half drivers probably does output SPI, one pixel at a time.  That implementation is not part of the ILI9341 upper-half driver at drivers/lcd/ili9341.c.  To output a line it does:
   
        655   lcd->select(lcd);
        659   ili9341_selectarea(lcd, col, row, col + npixels - 1, row);
        663   lcd->sendcmd(lcd, ILI9341_MEMORY_WRITE);
        667   lcd->sendgram(lcd, src, npixels);
        671   lcd->deselect(lcd);
   
   So from the standpoint of the ILI9341 driver it outputs the entire run of pixels in one command (sendgram).  Whether or not the lower half driver supports DMA or not is up to the board implementation of the sendgram() method of the struct ili8431_lcd_s interface.
   
   


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

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



[GitHub] [incubator-nuttx] btashton commented on pull request #1778: board: Keyboard FeatherWing on nrf52-feather

Posted by GitBox <gi...@apache.org>.
btashton commented on pull request #1778:
URL: https://github.com/apache/incubator-nuttx/pull/1778#issuecomment-779642643


   Ok most of this work ended up being spread over a few different PRs to add the driver support to NuttX.  At this point I would like to get these board configurations merged in.  I am still interested in trying to break this out a little bit later to make it easier to support this featherwing breakout board in other boards, but I would like to do that in another PR.  I am somewhat motivated to do that because the nrf5232 was actually not a great choice for this board, the SPI hardware is very limited at 255 byte transfers and *only 8MHz* SPI clock.  I know these SPI displays are generally not great, but this makes it a lot worse.
   
   You can see an example of this running here:
   https://github.com/apache/incubator-nuttx-apps/pull/591


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

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



[GitHub] [incubator-nuttx] btashton commented on a change in pull request #1778: WIP board: Keyboard FeatherWing on nrf52-feather

Posted by GitBox <gi...@apache.org>.
btashton commented on a change in pull request #1778:
URL: https://github.com/apache/incubator-nuttx/pull/1778#discussion_r487673440



##########
File path: boards/arm/nrf52/nrf52-feather/src/nrf52_ili9341_lcd.c
##########
@@ -0,0 +1,410 @@
+/****************************************************************************
+ * boards/arm/nrf52/nrf52-feather/src/nrf52_ili9341_lcd.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <stdint.h>
+#include <stdbool.h>
+#include <endian.h>
+#include <debug.h>
+
+#include <nuttx/spi/spi.h>
+#include <nuttx/lcd/lcd.h>
+#include <nuttx/lcd/ili9341.h>
+
+#include "chip.h"
+#include "arm_arch.h"
+#include "arm_internal.h"
+#include "nrf52_gpio.h"
+#include "nrf52_spi.h"
+
+#include "nrf52-feather.h"
+#include <arch/board/board.h>
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+#define GRAM_ENDIAN_SWAP_BUFFER_LEN 1    /* Gram swap space */
+
+#if (GRAM_ENDIAN_SWAP_BUFFER_LEN > 127)
+#  error "GRAM_ENDIAN_SWAP_BUFFER_LEN must fit in a SPI transfer 127 words!"
+#endif
+
+/****************************************************************************
+ * Private Type Definition
+ ****************************************************************************/
+
+struct ili93414ws_lcd_s
+{
+  struct ili9341_lcd_s dev;
+  FAR struct spi_dev_s *spi;
+};
+
+/****************************************************************************
+ * Private Function Protototypes
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+static struct ili93414ws_lcd_s g_lcddev;
+static struct lcd_dev_s *g_lcd = NULL;
+static uint8_t swapbuf[GRAM_ENDIAN_SWAP_BUFFER_LEN * 2];
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: nrf52_ili93414ws_backlight
+ *
+ * Description:
+ *   Set the backlight level of the connected display.
+ *
+ * Input Parameters:
+ *   spi   - Reference to the public driver structure
+ *   level - backlight level
+ *
+ * Returned Value:
+ *   OK - On Success
+ *
+ ****************************************************************************/
+
+static int nrf52_ili93414ws_backlight(FAR struct ili9341_lcd_s *lcd,
+                                      int level)
+{
+  return OK;
+}
+
+/****************************************************************************
+ * Name: nrf52_ili93414ws_select
+ *
+ * Description:
+ *   Select the SPI, locking and re-configuring if necessary
+ *
+ * Input Parameters:
+ *   spi  - Reference to the public driver structure
+ *
+ * Returned Value:
+ *
+ ****************************************************************************/
+
+static void nrf52_ili93414ws_select(FAR struct ili9341_lcd_s *lcd)
+{
+  FAR struct ili93414ws_lcd_s *priv = (FAR struct ili93414ws_lcd_s *)lcd;
+
+  SPI_LOCK(priv->spi, true);
+  SPI_SELECT(priv->spi, SPIDEV_DISPLAY(0), true);
+}
+
+/****************************************************************************
+ * Name: nrf52_ili93414ws_deselect
+ *
+ * Description:
+ *   De-select the SPI
+ *
+ * Input Parameters:
+ *   spi  - Reference to the public driver structure
+ *
+ * Returned Value:
+ *
+ ****************************************************************************/
+
+static void nrf52_ili93414ws_deselect(FAR struct ili9341_lcd_s *lcd)
+{
+  FAR struct ili93414ws_lcd_s *priv = (FAR struct ili93414ws_lcd_s *)lcd;
+
+  SPI_LOCK(priv->spi, false);
+  SPI_SELECT(priv->spi, SPIDEV_DISPLAY(0), false);
+}
+
+/****************************************************************************
+ * Name: nrf52_ili93414ws_sndcmd
+ *
+ * Description:
+ *   Send a command to the lcd driver.
+ *
+ * Input Parameters:
+ *   lcd  - Reference to the ili9341_lcd_s driver structure
+ *   cmd  - command to send
+ *
+ * Returned Value:
+ *   On success - OK
+ *
+ ****************************************************************************/
+
+static int nrf52_ili93414ws_sendcmd(
+    FAR struct ili9341_lcd_s *lcd, const uint8_t cmd)
+{
+  FAR struct ili93414ws_lcd_s *priv = (FAR struct ili93414ws_lcd_s *)lcd;
+
+  lcdinfo("%02x\n", cmd);
+
+  nrf52_gpio_write(ILI9341_DISPLAY_DC, false); /* Indicate CMD */
+  SPI_SEND(priv->spi, cmd);
+
+  return OK;
+}
+
+/****************************************************************************
+ * Name: nrf52_ili93414ws_sendparam
+ *
+ * Description:
+ *   Send a parameter to the lcd driver.
+ *
+ * Input Parameters:
+ *   lcd    - Reference to the ili9341_lcd_s driver structure
+ *   param  - parameter to send
+ *
+ * Returned Value:
+ *   OK - On Success
+ *
+ ****************************************************************************/
+
+static int nrf52_ili93414ws_sendparam(FAR struct ili9341_lcd_s *lcd,
+                                      const uint8_t param)
+{
+  FAR struct ili93414ws_lcd_s *priv = (FAR struct ili93414ws_lcd_s *)lcd;
+
+  nrf52_gpio_write(ILI9341_DISPLAY_DC, true);  /* Indicate DATA */
+  SPI_SEND(priv->spi, param);
+
+  return OK;
+}
+
+/****************************************************************************
+ * Name: nrf52_ili93414ws_sendgram
+ *
+ * Description:
+ *   Send a number of pixel words to the lcd driver gram.
+ *
+ * Input Parameters:
+ *   lcd    - Reference to the ili9341_lcd_s driver structure
+ *   wd     - Reference to the words to send
+ *   nwords - number of words to send
+ *
+ * Returned Value:
+ *   OK - On Success
+ *
+ ****************************************************************************/
+
+static int nrf52_ili93414ws_sendgram(FAR struct ili9341_lcd_s *lcd,

Review comment:
       @patacongo I for the gram send recv interfaces I had to add a byte swapping buffer because nrf52 SPI hardware does not support 16bit SPI.  If there was a way to tell the whole graphics stack to use BE instead of LE this could be avoided.  The buffer can be configured in size, but right now it is just set to the minimal of a single gram.  This also means that there is no real DMA advantage since the transfers are only 2 bytes at a time.




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

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



[GitHub] [incubator-nuttx] btashton commented on a change in pull request #1778: WIP board: Keyboard FeatherWing on nrf52-feather

Posted by GitBox <gi...@apache.org>.
btashton commented on a change in pull request #1778:
URL: https://github.com/apache/incubator-nuttx/pull/1778#discussion_r488001338



##########
File path: boards/arm/nrf52/nrf52-feather/src/nrf52_ili9341_lcd.c
##########
@@ -0,0 +1,410 @@
+/****************************************************************************
+ * boards/arm/nrf52/nrf52-feather/src/nrf52_ili9341_lcd.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <stdint.h>
+#include <stdbool.h>
+#include <endian.h>
+#include <debug.h>
+
+#include <nuttx/spi/spi.h>
+#include <nuttx/lcd/lcd.h>
+#include <nuttx/lcd/ili9341.h>
+
+#include "chip.h"
+#include "arm_arch.h"
+#include "arm_internal.h"
+#include "nrf52_gpio.h"
+#include "nrf52_spi.h"
+
+#include "nrf52-feather.h"
+#include <arch/board/board.h>
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+#define GRAM_ENDIAN_SWAP_BUFFER_LEN 1    /* Gram swap space */
+
+#if (GRAM_ENDIAN_SWAP_BUFFER_LEN > 127)
+#  error "GRAM_ENDIAN_SWAP_BUFFER_LEN must fit in a SPI transfer 127 words!"
+#endif
+
+/****************************************************************************
+ * Private Type Definition
+ ****************************************************************************/
+
+struct ili93414ws_lcd_s
+{
+  struct ili9341_lcd_s dev;
+  FAR struct spi_dev_s *spi;
+};
+
+/****************************************************************************
+ * Private Function Protototypes
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+static struct ili93414ws_lcd_s g_lcddev;
+static struct lcd_dev_s *g_lcd = NULL;
+static uint8_t swapbuf[GRAM_ENDIAN_SWAP_BUFFER_LEN * 2];
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: nrf52_ili93414ws_backlight
+ *
+ * Description:
+ *   Set the backlight level of the connected display.
+ *
+ * Input Parameters:
+ *   spi   - Reference to the public driver structure
+ *   level - backlight level
+ *
+ * Returned Value:
+ *   OK - On Success
+ *
+ ****************************************************************************/
+
+static int nrf52_ili93414ws_backlight(FAR struct ili9341_lcd_s *lcd,
+                                      int level)
+{
+  return OK;
+}
+
+/****************************************************************************
+ * Name: nrf52_ili93414ws_select
+ *
+ * Description:
+ *   Select the SPI, locking and re-configuring if necessary
+ *
+ * Input Parameters:
+ *   spi  - Reference to the public driver structure
+ *
+ * Returned Value:
+ *
+ ****************************************************************************/
+
+static void nrf52_ili93414ws_select(FAR struct ili9341_lcd_s *lcd)
+{
+  FAR struct ili93414ws_lcd_s *priv = (FAR struct ili93414ws_lcd_s *)lcd;
+
+  SPI_LOCK(priv->spi, true);
+  SPI_SELECT(priv->spi, SPIDEV_DISPLAY(0), true);
+}
+
+/****************************************************************************
+ * Name: nrf52_ili93414ws_deselect
+ *
+ * Description:
+ *   De-select the SPI
+ *
+ * Input Parameters:
+ *   spi  - Reference to the public driver structure
+ *
+ * Returned Value:
+ *
+ ****************************************************************************/
+
+static void nrf52_ili93414ws_deselect(FAR struct ili9341_lcd_s *lcd)
+{
+  FAR struct ili93414ws_lcd_s *priv = (FAR struct ili93414ws_lcd_s *)lcd;
+
+  SPI_LOCK(priv->spi, false);
+  SPI_SELECT(priv->spi, SPIDEV_DISPLAY(0), false);
+}
+
+/****************************************************************************
+ * Name: nrf52_ili93414ws_sndcmd
+ *
+ * Description:
+ *   Send a command to the lcd driver.
+ *
+ * Input Parameters:
+ *   lcd  - Reference to the ili9341_lcd_s driver structure
+ *   cmd  - command to send
+ *
+ * Returned Value:
+ *   On success - OK
+ *
+ ****************************************************************************/
+
+static int nrf52_ili93414ws_sendcmd(
+    FAR struct ili9341_lcd_s *lcd, const uint8_t cmd)
+{
+  FAR struct ili93414ws_lcd_s *priv = (FAR struct ili93414ws_lcd_s *)lcd;
+
+  lcdinfo("%02x\n", cmd);
+
+  nrf52_gpio_write(ILI9341_DISPLAY_DC, false); /* Indicate CMD */
+  SPI_SEND(priv->spi, cmd);
+
+  return OK;
+}
+
+/****************************************************************************
+ * Name: nrf52_ili93414ws_sendparam
+ *
+ * Description:
+ *   Send a parameter to the lcd driver.
+ *
+ * Input Parameters:
+ *   lcd    - Reference to the ili9341_lcd_s driver structure
+ *   param  - parameter to send
+ *
+ * Returned Value:
+ *   OK - On Success
+ *
+ ****************************************************************************/
+
+static int nrf52_ili93414ws_sendparam(FAR struct ili9341_lcd_s *lcd,
+                                      const uint8_t param)
+{
+  FAR struct ili93414ws_lcd_s *priv = (FAR struct ili93414ws_lcd_s *)lcd;
+
+  nrf52_gpio_write(ILI9341_DISPLAY_DC, true);  /* Indicate DATA */
+  SPI_SEND(priv->spi, param);
+
+  return OK;
+}
+
+/****************************************************************************
+ * Name: nrf52_ili93414ws_sendgram
+ *
+ * Description:
+ *   Send a number of pixel words to the lcd driver gram.
+ *
+ * Input Parameters:
+ *   lcd    - Reference to the ili9341_lcd_s driver structure
+ *   wd     - Reference to the words to send
+ *   nwords - number of words to send
+ *
+ * Returned Value:
+ *   OK - On Success
+ *
+ ****************************************************************************/
+
+static int nrf52_ili93414ws_sendgram(FAR struct ili9341_lcd_s *lcd,

Review comment:
       Yeah it also will cause issues with other stacks like LVGL as well so it might be best to just leave the byteswap where it is for now. Most hardware supports 16bit mode, this just has a somewhat limited SPI controller.




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

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



[GitHub] [incubator-nuttx] v01d commented on pull request #1778: WIP board: Keyboard FeatherWing on nrf52-feather

Posted by GitBox <gi...@apache.org>.
v01d commented on pull request #1778:
URL: https://github.com/apache/incubator-nuttx/pull/1778#issuecomment-692150618


   > > One thing I noticed is it seems that each pixel was being written individually which is super slow compared to writing a "rectangle" which could be DMA. I did not dig into this yet.
   > 
   > No, the NX LCD interface outputs runs: A horizontal extent of pixels. This has been discussion that this limits performance for painting large regions that are not bounded by the line length. But this is not a performance limitation for drawing rectangles that are inherently bounded in the window.
   
   I understand you are using nxgraphics with LCD interface right? As I mentioned in the past this really limited performance when wanting to transfer various lines in one go. I have in my TODO list to add support for putrun() to accept npixels > rowsize for such displays but I never got to do it. Anyway, as Greg says, this will not improve NX since putrun() is used once per line obviously.
   I used LVGL via a character driver which exposed the LCD interface and this allowed me to send putrun() for many rows.
   Contributing this to mainline is also in my backlog since I have to go back to test with another board to do so. If you're interested in trying it, it is here: https://gitlab.com/nuttx_projects/lcd_dev 
   Note that it uses a non-standard putrows() since this was my solution to the aforementioned problem, but as we discussed previously, extending putrun() would be easier.


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

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



[GitHub] [incubator-nuttx] btashton commented on a change in pull request #1778: WIP board: Keyboard FeatherWing on nrf52-feather

Posted by GitBox <gi...@apache.org>.
btashton commented on a change in pull request #1778:
URL: https://github.com/apache/incubator-nuttx/pull/1778#discussion_r487673440



##########
File path: boards/arm/nrf52/nrf52-feather/src/nrf52_ili9341_lcd.c
##########
@@ -0,0 +1,410 @@
+/****************************************************************************
+ * boards/arm/nrf52/nrf52-feather/src/nrf52_ili9341_lcd.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <stdint.h>
+#include <stdbool.h>
+#include <endian.h>
+#include <debug.h>
+
+#include <nuttx/spi/spi.h>
+#include <nuttx/lcd/lcd.h>
+#include <nuttx/lcd/ili9341.h>
+
+#include "chip.h"
+#include "arm_arch.h"
+#include "arm_internal.h"
+#include "nrf52_gpio.h"
+#include "nrf52_spi.h"
+
+#include "nrf52-feather.h"
+#include <arch/board/board.h>
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+#define GRAM_ENDIAN_SWAP_BUFFER_LEN 1    /* Gram swap space */
+
+#if (GRAM_ENDIAN_SWAP_BUFFER_LEN > 127)
+#  error "GRAM_ENDIAN_SWAP_BUFFER_LEN must fit in a SPI transfer 127 words!"
+#endif
+
+/****************************************************************************
+ * Private Type Definition
+ ****************************************************************************/
+
+struct ili93414ws_lcd_s
+{
+  struct ili9341_lcd_s dev;
+  FAR struct spi_dev_s *spi;
+};
+
+/****************************************************************************
+ * Private Function Protototypes
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+static struct ili93414ws_lcd_s g_lcddev;
+static struct lcd_dev_s *g_lcd = NULL;
+static uint8_t swapbuf[GRAM_ENDIAN_SWAP_BUFFER_LEN * 2];
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: nrf52_ili93414ws_backlight
+ *
+ * Description:
+ *   Set the backlight level of the connected display.
+ *
+ * Input Parameters:
+ *   spi   - Reference to the public driver structure
+ *   level - backlight level
+ *
+ * Returned Value:
+ *   OK - On Success
+ *
+ ****************************************************************************/
+
+static int nrf52_ili93414ws_backlight(FAR struct ili9341_lcd_s *lcd,
+                                      int level)
+{
+  return OK;
+}
+
+/****************************************************************************
+ * Name: nrf52_ili93414ws_select
+ *
+ * Description:
+ *   Select the SPI, locking and re-configuring if necessary
+ *
+ * Input Parameters:
+ *   spi  - Reference to the public driver structure
+ *
+ * Returned Value:
+ *
+ ****************************************************************************/
+
+static void nrf52_ili93414ws_select(FAR struct ili9341_lcd_s *lcd)
+{
+  FAR struct ili93414ws_lcd_s *priv = (FAR struct ili93414ws_lcd_s *)lcd;
+
+  SPI_LOCK(priv->spi, true);
+  SPI_SELECT(priv->spi, SPIDEV_DISPLAY(0), true);
+}
+
+/****************************************************************************
+ * Name: nrf52_ili93414ws_deselect
+ *
+ * Description:
+ *   De-select the SPI
+ *
+ * Input Parameters:
+ *   spi  - Reference to the public driver structure
+ *
+ * Returned Value:
+ *
+ ****************************************************************************/
+
+static void nrf52_ili93414ws_deselect(FAR struct ili9341_lcd_s *lcd)
+{
+  FAR struct ili93414ws_lcd_s *priv = (FAR struct ili93414ws_lcd_s *)lcd;
+
+  SPI_LOCK(priv->spi, false);
+  SPI_SELECT(priv->spi, SPIDEV_DISPLAY(0), false);
+}
+
+/****************************************************************************
+ * Name: nrf52_ili93414ws_sndcmd
+ *
+ * Description:
+ *   Send a command to the lcd driver.
+ *
+ * Input Parameters:
+ *   lcd  - Reference to the ili9341_lcd_s driver structure
+ *   cmd  - command to send
+ *
+ * Returned Value:
+ *   On success - OK
+ *
+ ****************************************************************************/
+
+static int nrf52_ili93414ws_sendcmd(
+    FAR struct ili9341_lcd_s *lcd, const uint8_t cmd)
+{
+  FAR struct ili93414ws_lcd_s *priv = (FAR struct ili93414ws_lcd_s *)lcd;
+
+  lcdinfo("%02x\n", cmd);
+
+  nrf52_gpio_write(ILI9341_DISPLAY_DC, false); /* Indicate CMD */
+  SPI_SEND(priv->spi, cmd);
+
+  return OK;
+}
+
+/****************************************************************************
+ * Name: nrf52_ili93414ws_sendparam
+ *
+ * Description:
+ *   Send a parameter to the lcd driver.
+ *
+ * Input Parameters:
+ *   lcd    - Reference to the ili9341_lcd_s driver structure
+ *   param  - parameter to send
+ *
+ * Returned Value:
+ *   OK - On Success
+ *
+ ****************************************************************************/
+
+static int nrf52_ili93414ws_sendparam(FAR struct ili9341_lcd_s *lcd,
+                                      const uint8_t param)
+{
+  FAR struct ili93414ws_lcd_s *priv = (FAR struct ili93414ws_lcd_s *)lcd;
+
+  nrf52_gpio_write(ILI9341_DISPLAY_DC, true);  /* Indicate DATA */
+  SPI_SEND(priv->spi, param);
+
+  return OK;
+}
+
+/****************************************************************************
+ * Name: nrf52_ili93414ws_sendgram
+ *
+ * Description:
+ *   Send a number of pixel words to the lcd driver gram.
+ *
+ * Input Parameters:
+ *   lcd    - Reference to the ili9341_lcd_s driver structure
+ *   wd     - Reference to the words to send
+ *   nwords - number of words to send
+ *
+ * Returned Value:
+ *   OK - On Success
+ *
+ ****************************************************************************/
+
+static int nrf52_ili93414ws_sendgram(FAR struct ili9341_lcd_s *lcd,

Review comment:
       @patacongo for the gram send recv interfaces I had to add a byte swapping buffer because nrf52 SPI hardware does not support 16bit SPI.  If there was a way to tell the whole graphics stack to use BE instead of LE for color this could be avoided.  The buffer can be configured in size, but right now it is just set to the minimal of a single gram.  This also means that there is no real DMA advantage since the transfers are only 2 bytes at a time.




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

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



[GitHub] [incubator-nuttx] patacongo commented on a change in pull request #1778: WIP board: Keyboard FeatherWing on nrf52-feather

Posted by GitBox <gi...@apache.org>.
patacongo commented on a change in pull request #1778:
URL: https://github.com/apache/incubator-nuttx/pull/1778#discussion_r487983115



##########
File path: boards/arm/nrf52/nrf52-feather/src/nrf52_ili9341_lcd.c
##########
@@ -0,0 +1,410 @@
+/****************************************************************************
+ * boards/arm/nrf52/nrf52-feather/src/nrf52_ili9341_lcd.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <stdint.h>
+#include <stdbool.h>
+#include <endian.h>
+#include <debug.h>
+
+#include <nuttx/spi/spi.h>
+#include <nuttx/lcd/lcd.h>
+#include <nuttx/lcd/ili9341.h>
+
+#include "chip.h"
+#include "arm_arch.h"
+#include "arm_internal.h"
+#include "nrf52_gpio.h"
+#include "nrf52_spi.h"
+
+#include "nrf52-feather.h"
+#include <arch/board/board.h>
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+#define GRAM_ENDIAN_SWAP_BUFFER_LEN 1    /* Gram swap space */
+
+#if (GRAM_ENDIAN_SWAP_BUFFER_LEN > 127)
+#  error "GRAM_ENDIAN_SWAP_BUFFER_LEN must fit in a SPI transfer 127 words!"
+#endif
+
+/****************************************************************************
+ * Private Type Definition
+ ****************************************************************************/
+
+struct ili93414ws_lcd_s
+{
+  struct ili9341_lcd_s dev;
+  FAR struct spi_dev_s *spi;
+};
+
+/****************************************************************************
+ * Private Function Protototypes
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+static struct ili93414ws_lcd_s g_lcddev;
+static struct lcd_dev_s *g_lcd = NULL;
+static uint8_t swapbuf[GRAM_ENDIAN_SWAP_BUFFER_LEN * 2];
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: nrf52_ili93414ws_backlight
+ *
+ * Description:
+ *   Set the backlight level of the connected display.
+ *
+ * Input Parameters:
+ *   spi   - Reference to the public driver structure
+ *   level - backlight level
+ *
+ * Returned Value:
+ *   OK - On Success
+ *
+ ****************************************************************************/
+
+static int nrf52_ili93414ws_backlight(FAR struct ili9341_lcd_s *lcd,
+                                      int level)
+{
+  return OK;
+}
+
+/****************************************************************************
+ * Name: nrf52_ili93414ws_select
+ *
+ * Description:
+ *   Select the SPI, locking and re-configuring if necessary
+ *
+ * Input Parameters:
+ *   spi  - Reference to the public driver structure
+ *
+ * Returned Value:
+ *
+ ****************************************************************************/
+
+static void nrf52_ili93414ws_select(FAR struct ili9341_lcd_s *lcd)
+{
+  FAR struct ili93414ws_lcd_s *priv = (FAR struct ili93414ws_lcd_s *)lcd;
+
+  SPI_LOCK(priv->spi, true);
+  SPI_SELECT(priv->spi, SPIDEV_DISPLAY(0), true);
+}
+
+/****************************************************************************
+ * Name: nrf52_ili93414ws_deselect
+ *
+ * Description:
+ *   De-select the SPI
+ *
+ * Input Parameters:
+ *   spi  - Reference to the public driver structure
+ *
+ * Returned Value:
+ *
+ ****************************************************************************/
+
+static void nrf52_ili93414ws_deselect(FAR struct ili9341_lcd_s *lcd)
+{
+  FAR struct ili93414ws_lcd_s *priv = (FAR struct ili93414ws_lcd_s *)lcd;
+
+  SPI_LOCK(priv->spi, false);
+  SPI_SELECT(priv->spi, SPIDEV_DISPLAY(0), false);
+}
+
+/****************************************************************************
+ * Name: nrf52_ili93414ws_sndcmd
+ *
+ * Description:
+ *   Send a command to the lcd driver.
+ *
+ * Input Parameters:
+ *   lcd  - Reference to the ili9341_lcd_s driver structure
+ *   cmd  - command to send
+ *
+ * Returned Value:
+ *   On success - OK
+ *
+ ****************************************************************************/
+
+static int nrf52_ili93414ws_sendcmd(
+    FAR struct ili9341_lcd_s *lcd, const uint8_t cmd)
+{
+  FAR struct ili93414ws_lcd_s *priv = (FAR struct ili93414ws_lcd_s *)lcd;
+
+  lcdinfo("%02x\n", cmd);
+
+  nrf52_gpio_write(ILI9341_DISPLAY_DC, false); /* Indicate CMD */
+  SPI_SEND(priv->spi, cmd);
+
+  return OK;
+}
+
+/****************************************************************************
+ * Name: nrf52_ili93414ws_sendparam
+ *
+ * Description:
+ *   Send a parameter to the lcd driver.
+ *
+ * Input Parameters:
+ *   lcd    - Reference to the ili9341_lcd_s driver structure
+ *   param  - parameter to send
+ *
+ * Returned Value:
+ *   OK - On Success
+ *
+ ****************************************************************************/
+
+static int nrf52_ili93414ws_sendparam(FAR struct ili9341_lcd_s *lcd,
+                                      const uint8_t param)
+{
+  FAR struct ili93414ws_lcd_s *priv = (FAR struct ili93414ws_lcd_s *)lcd;
+
+  nrf52_gpio_write(ILI9341_DISPLAY_DC, true);  /* Indicate DATA */
+  SPI_SEND(priv->spi, param);
+
+  return OK;
+}
+
+/****************************************************************************
+ * Name: nrf52_ili93414ws_sendgram
+ *
+ * Description:
+ *   Send a number of pixel words to the lcd driver gram.
+ *
+ * Input Parameters:
+ *   lcd    - Reference to the ili9341_lcd_s driver structure
+ *   wd     - Reference to the words to send
+ *   nwords - number of words to send
+ *
+ * Returned Value:
+ *   OK - On Success
+ *
+ ****************************************************************************/
+
+static int nrf52_ili93414ws_sendgram(FAR struct ili9341_lcd_s *lcd,

Review comment:
       > The location where the blitting is done is in graphics/nxglib/lcd. But that uses macros defined in graphics/nxglib/nxglib_bitblit.h.
   
   Sorry, that is bad advice.  Those macros are only used by the framebuffer bit-blitters (graphics/nxglib/fb).  The blitting is hardcoded for the LCD in the files unser graphics/nxglib/lcd.
   
   So you can't change the blitting in one place but rather in all of the 6 files at that location.  Sorry.
   
   Hmm.. there is common logic used for the LCD blitters in graphics/nxglib/nxglib_copyrun.h and graphics/nxglib/nxglib_fillrun.h.  Those will cover most, but not all, of the blit operations in graphics/nxglib/lcd.
   
   




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

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



[GitHub] [incubator-nuttx] v01d commented on pull request #1778: WIP board: Keyboard FeatherWing on nrf52-feather

Posted by GitBox <gi...@apache.org>.
v01d commented on pull request #1778:
URL: https://github.com/apache/incubator-nuttx/pull/1778#issuecomment-692192618


   LVGL as it is now on NuttX indeed uses this "frambuffer character driver to LCD interface" but I really dislike this, it is an unnecessary confusion of interfaces. The character driver I wrote is a very simple mapping of LCD interface to userspace without performance penalty and clear semantics. LVGL can use whatever you want. See how I implemented the interface myself: 
   https://gitlab.com/bicycle-companion/firmware/-/blob/master/extra_apps/bicycle_companion/display.cxx#L93
   
   Yes, what I'm talking about does not impact NX in anyway. I was actually suggesting Brennan that he could try LVGL for this if he wanted, and how it could be made faster with this interface than with the framebuffer -> LCD interface. As per improving NX, I'm personally more vested in LVGL for my use cases so I cannot comment.


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

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



[GitHub] [incubator-nuttx] btashton commented on a change in pull request #1778: WIP board: Keyboard FeatherWing on nrf52-feather

Posted by GitBox <gi...@apache.org>.
btashton commented on a change in pull request #1778:
URL: https://github.com/apache/incubator-nuttx/pull/1778#discussion_r487673440



##########
File path: boards/arm/nrf52/nrf52-feather/src/nrf52_ili9341_lcd.c
##########
@@ -0,0 +1,410 @@
+/****************************************************************************
+ * boards/arm/nrf52/nrf52-feather/src/nrf52_ili9341_lcd.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <stdint.h>
+#include <stdbool.h>
+#include <endian.h>
+#include <debug.h>
+
+#include <nuttx/spi/spi.h>
+#include <nuttx/lcd/lcd.h>
+#include <nuttx/lcd/ili9341.h>
+
+#include "chip.h"
+#include "arm_arch.h"
+#include "arm_internal.h"
+#include "nrf52_gpio.h"
+#include "nrf52_spi.h"
+
+#include "nrf52-feather.h"
+#include <arch/board/board.h>
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+#define GRAM_ENDIAN_SWAP_BUFFER_LEN 1    /* Gram swap space */
+
+#if (GRAM_ENDIAN_SWAP_BUFFER_LEN > 127)
+#  error "GRAM_ENDIAN_SWAP_BUFFER_LEN must fit in a SPI transfer 127 words!"
+#endif
+
+/****************************************************************************
+ * Private Type Definition
+ ****************************************************************************/
+
+struct ili93414ws_lcd_s
+{
+  struct ili9341_lcd_s dev;
+  FAR struct spi_dev_s *spi;
+};
+
+/****************************************************************************
+ * Private Function Protototypes
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+static struct ili93414ws_lcd_s g_lcddev;
+static struct lcd_dev_s *g_lcd = NULL;
+static uint8_t swapbuf[GRAM_ENDIAN_SWAP_BUFFER_LEN * 2];
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: nrf52_ili93414ws_backlight
+ *
+ * Description:
+ *   Set the backlight level of the connected display.
+ *
+ * Input Parameters:
+ *   spi   - Reference to the public driver structure
+ *   level - backlight level
+ *
+ * Returned Value:
+ *   OK - On Success
+ *
+ ****************************************************************************/
+
+static int nrf52_ili93414ws_backlight(FAR struct ili9341_lcd_s *lcd,
+                                      int level)
+{
+  return OK;
+}
+
+/****************************************************************************
+ * Name: nrf52_ili93414ws_select
+ *
+ * Description:
+ *   Select the SPI, locking and re-configuring if necessary
+ *
+ * Input Parameters:
+ *   spi  - Reference to the public driver structure
+ *
+ * Returned Value:
+ *
+ ****************************************************************************/
+
+static void nrf52_ili93414ws_select(FAR struct ili9341_lcd_s *lcd)
+{
+  FAR struct ili93414ws_lcd_s *priv = (FAR struct ili93414ws_lcd_s *)lcd;
+
+  SPI_LOCK(priv->spi, true);
+  SPI_SELECT(priv->spi, SPIDEV_DISPLAY(0), true);
+}
+
+/****************************************************************************
+ * Name: nrf52_ili93414ws_deselect
+ *
+ * Description:
+ *   De-select the SPI
+ *
+ * Input Parameters:
+ *   spi  - Reference to the public driver structure
+ *
+ * Returned Value:
+ *
+ ****************************************************************************/
+
+static void nrf52_ili93414ws_deselect(FAR struct ili9341_lcd_s *lcd)
+{
+  FAR struct ili93414ws_lcd_s *priv = (FAR struct ili93414ws_lcd_s *)lcd;
+
+  SPI_LOCK(priv->spi, false);
+  SPI_SELECT(priv->spi, SPIDEV_DISPLAY(0), false);
+}
+
+/****************************************************************************
+ * Name: nrf52_ili93414ws_sndcmd
+ *
+ * Description:
+ *   Send a command to the lcd driver.
+ *
+ * Input Parameters:
+ *   lcd  - Reference to the ili9341_lcd_s driver structure
+ *   cmd  - command to send
+ *
+ * Returned Value:
+ *   On success - OK
+ *
+ ****************************************************************************/
+
+static int nrf52_ili93414ws_sendcmd(
+    FAR struct ili9341_lcd_s *lcd, const uint8_t cmd)
+{
+  FAR struct ili93414ws_lcd_s *priv = (FAR struct ili93414ws_lcd_s *)lcd;
+
+  lcdinfo("%02x\n", cmd);
+
+  nrf52_gpio_write(ILI9341_DISPLAY_DC, false); /* Indicate CMD */
+  SPI_SEND(priv->spi, cmd);
+
+  return OK;
+}
+
+/****************************************************************************
+ * Name: nrf52_ili93414ws_sendparam
+ *
+ * Description:
+ *   Send a parameter to the lcd driver.
+ *
+ * Input Parameters:
+ *   lcd    - Reference to the ili9341_lcd_s driver structure
+ *   param  - parameter to send
+ *
+ * Returned Value:
+ *   OK - On Success
+ *
+ ****************************************************************************/
+
+static int nrf52_ili93414ws_sendparam(FAR struct ili9341_lcd_s *lcd,
+                                      const uint8_t param)
+{
+  FAR struct ili93414ws_lcd_s *priv = (FAR struct ili93414ws_lcd_s *)lcd;
+
+  nrf52_gpio_write(ILI9341_DISPLAY_DC, true);  /* Indicate DATA */
+  SPI_SEND(priv->spi, param);
+
+  return OK;
+}
+
+/****************************************************************************
+ * Name: nrf52_ili93414ws_sendgram
+ *
+ * Description:
+ *   Send a number of pixel words to the lcd driver gram.
+ *
+ * Input Parameters:
+ *   lcd    - Reference to the ili9341_lcd_s driver structure
+ *   wd     - Reference to the words to send
+ *   nwords - number of words to send
+ *
+ * Returned Value:
+ *   OK - On Success
+ *
+ ****************************************************************************/
+
+static int nrf52_ili93414ws_sendgram(FAR struct ili9341_lcd_s *lcd,

Review comment:
       @patacongo for the gram send recv interfaces I had to add a byte swapping buffer because nrf52 SPI hardware does not support 16bit SPI.  If there was a way to tell the whole graphics stack to use BE instead of LE for color this could be avoided.  The buffer can be configured in size, but right now it is just set to the minimal of a single gram this also means that there is no real DMA advantage since the transfers are limited to the size of this buffer.




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

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



[GitHub] [incubator-nuttx] patacongo commented on pull request #1778: WIP board: Keyboard FeatherWing on nrf52-feather

Posted by GitBox <gi...@apache.org>.
patacongo commented on pull request #1778:
URL: https://github.com/apache/incubator-nuttx/pull/1778#issuecomment-692231967


   > I did set my gram byte swap buffer to the max of 127 grams for DMA and it improved the performance quite a bit. You can see it here:
   > https://media.giphy.com/media/j736EdTwUBwQHiPKhS/giphy.gif
   
   Yes, the performance looks great to me.
   
   I cannot tell for sure from the video, but there should be text in the two little windows.  Is it there?  Or is there a text problem.
   
   It would be cool to see something like the NxWM window manager running on that platform.  It looks like you have the band with to do that.
   


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

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



[GitHub] [incubator-nuttx] btashton commented on pull request #1778: WIP board: Keyboard FeatherWing on nrf52-feather

Posted by GitBox <gi...@apache.org>.
btashton commented on pull request #1778:
URL: https://github.com/apache/incubator-nuttx/pull/1778#issuecomment-692200063


   @patacongo Here is what the it looks like when NXGraphics writes
   ![image](https://user-images.githubusercontent.com/173245/93116730-87024900-f672-11ea-95fc-c6760b2825e2.png)
   
   What we see on the left is the setup of the region it is going to write which is a single row at 0x00c5.  The first bit is spaced out because it sends a command byte and then has to toggle the DC line and send parameter bytes.  I could improve the ili9341 driver to send the parameters in one transfer which would improve things quite a bit in that section.
   
   Then there is a gap where I'm guessing it is byte swapping the buffer.
   
   Then we see two DMA transfers of 0xdcdc grams.  This is actually one transfer but it has to be split because of the nRF SPI driver which only allows 0xff bytes to be sent at a time unless you use the DMA list feature which I dont quite understand.  We are still limited by the size of the swap buffer anyway. 
   
   Then we see the start of the next row where we send the same selection region for writting just one row down. This row did not need a new setup to happen if we had just extended the region in the previous setup.
   
   Now I understand there are complexities here.  If we are going to clear the background we are not going to allocate a whole buffer for the entire display, but if I was going to draw a thin rectangle from the top to the bottom of the screen this would 240 full row transactions just to send 480 bytes of data.
   
   Here is an example of drawing a skinny rectangle:
   ![image](https://user-images.githubusercontent.com/173245/93118087-98e4eb80-f674-11ea-8ee2-4e3273cfc642.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.

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



[GitHub] [incubator-nuttx] btashton commented on a change in pull request #1778: WIP board: Keyboard FeatherWing on nrf52-feather

Posted by GitBox <gi...@apache.org>.
btashton commented on a change in pull request #1778:
URL: https://github.com/apache/incubator-nuttx/pull/1778#discussion_r487672068



##########
File path: boards/arm/nrf52/nrf52-feather/Kconfig
##########
@@ -5,4 +5,54 @@
 
 if ARCH_BOARD_NRF52_FEATHER
 
-endif
+menuconfig NRF52_FEATHER_KB_FEATHERWING
+	bool "Keyboard FeatherWing"
+    default n
+    ---help---
+        Enable support for the SolderParty FeatherWing
+        https://www.solder.party/docs/keyboard-featherwing/
+
+if NRF52_FEATHER_KB_FEATHERWING
+config KB_FEATHERWING_LCD

Review comment:
       @patacongo what are your thoughts here on how this calls select? Originally I had the LCD dependent on LCD_ILI9341 and no selects, but here it seems like you would want as a user to enable the top level hardware you have and have it turn on the required hardware.
   
   I am fine changing this back if you think that makes sense.  I just was thinking this would make is easy for a user.




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

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



[GitHub] [incubator-nuttx] patacongo commented on a change in pull request #1778: WIP board: Keyboard FeatherWing on nrf52-feather

Posted by GitBox <gi...@apache.org>.
patacongo commented on a change in pull request #1778:
URL: https://github.com/apache/incubator-nuttx/pull/1778#discussion_r487983115



##########
File path: boards/arm/nrf52/nrf52-feather/src/nrf52_ili9341_lcd.c
##########
@@ -0,0 +1,410 @@
+/****************************************************************************
+ * boards/arm/nrf52/nrf52-feather/src/nrf52_ili9341_lcd.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <stdint.h>
+#include <stdbool.h>
+#include <endian.h>
+#include <debug.h>
+
+#include <nuttx/spi/spi.h>
+#include <nuttx/lcd/lcd.h>
+#include <nuttx/lcd/ili9341.h>
+
+#include "chip.h"
+#include "arm_arch.h"
+#include "arm_internal.h"
+#include "nrf52_gpio.h"
+#include "nrf52_spi.h"
+
+#include "nrf52-feather.h"
+#include <arch/board/board.h>
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+#define GRAM_ENDIAN_SWAP_BUFFER_LEN 1    /* Gram swap space */
+
+#if (GRAM_ENDIAN_SWAP_BUFFER_LEN > 127)
+#  error "GRAM_ENDIAN_SWAP_BUFFER_LEN must fit in a SPI transfer 127 words!"
+#endif
+
+/****************************************************************************
+ * Private Type Definition
+ ****************************************************************************/
+
+struct ili93414ws_lcd_s
+{
+  struct ili9341_lcd_s dev;
+  FAR struct spi_dev_s *spi;
+};
+
+/****************************************************************************
+ * Private Function Protototypes
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+static struct ili93414ws_lcd_s g_lcddev;
+static struct lcd_dev_s *g_lcd = NULL;
+static uint8_t swapbuf[GRAM_ENDIAN_SWAP_BUFFER_LEN * 2];
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: nrf52_ili93414ws_backlight
+ *
+ * Description:
+ *   Set the backlight level of the connected display.
+ *
+ * Input Parameters:
+ *   spi   - Reference to the public driver structure
+ *   level - backlight level
+ *
+ * Returned Value:
+ *   OK - On Success
+ *
+ ****************************************************************************/
+
+static int nrf52_ili93414ws_backlight(FAR struct ili9341_lcd_s *lcd,
+                                      int level)
+{
+  return OK;
+}
+
+/****************************************************************************
+ * Name: nrf52_ili93414ws_select
+ *
+ * Description:
+ *   Select the SPI, locking and re-configuring if necessary
+ *
+ * Input Parameters:
+ *   spi  - Reference to the public driver structure
+ *
+ * Returned Value:
+ *
+ ****************************************************************************/
+
+static void nrf52_ili93414ws_select(FAR struct ili9341_lcd_s *lcd)
+{
+  FAR struct ili93414ws_lcd_s *priv = (FAR struct ili93414ws_lcd_s *)lcd;
+
+  SPI_LOCK(priv->spi, true);
+  SPI_SELECT(priv->spi, SPIDEV_DISPLAY(0), true);
+}
+
+/****************************************************************************
+ * Name: nrf52_ili93414ws_deselect
+ *
+ * Description:
+ *   De-select the SPI
+ *
+ * Input Parameters:
+ *   spi  - Reference to the public driver structure
+ *
+ * Returned Value:
+ *
+ ****************************************************************************/
+
+static void nrf52_ili93414ws_deselect(FAR struct ili9341_lcd_s *lcd)
+{
+  FAR struct ili93414ws_lcd_s *priv = (FAR struct ili93414ws_lcd_s *)lcd;
+
+  SPI_LOCK(priv->spi, false);
+  SPI_SELECT(priv->spi, SPIDEV_DISPLAY(0), false);
+}
+
+/****************************************************************************
+ * Name: nrf52_ili93414ws_sndcmd
+ *
+ * Description:
+ *   Send a command to the lcd driver.
+ *
+ * Input Parameters:
+ *   lcd  - Reference to the ili9341_lcd_s driver structure
+ *   cmd  - command to send
+ *
+ * Returned Value:
+ *   On success - OK
+ *
+ ****************************************************************************/
+
+static int nrf52_ili93414ws_sendcmd(
+    FAR struct ili9341_lcd_s *lcd, const uint8_t cmd)
+{
+  FAR struct ili93414ws_lcd_s *priv = (FAR struct ili93414ws_lcd_s *)lcd;
+
+  lcdinfo("%02x\n", cmd);
+
+  nrf52_gpio_write(ILI9341_DISPLAY_DC, false); /* Indicate CMD */
+  SPI_SEND(priv->spi, cmd);
+
+  return OK;
+}
+
+/****************************************************************************
+ * Name: nrf52_ili93414ws_sendparam
+ *
+ * Description:
+ *   Send a parameter to the lcd driver.
+ *
+ * Input Parameters:
+ *   lcd    - Reference to the ili9341_lcd_s driver structure
+ *   param  - parameter to send
+ *
+ * Returned Value:
+ *   OK - On Success
+ *
+ ****************************************************************************/
+
+static int nrf52_ili93414ws_sendparam(FAR struct ili9341_lcd_s *lcd,
+                                      const uint8_t param)
+{
+  FAR struct ili93414ws_lcd_s *priv = (FAR struct ili93414ws_lcd_s *)lcd;
+
+  nrf52_gpio_write(ILI9341_DISPLAY_DC, true);  /* Indicate DATA */
+  SPI_SEND(priv->spi, param);
+
+  return OK;
+}
+
+/****************************************************************************
+ * Name: nrf52_ili93414ws_sendgram
+ *
+ * Description:
+ *   Send a number of pixel words to the lcd driver gram.
+ *
+ * Input Parameters:
+ *   lcd    - Reference to the ili9341_lcd_s driver structure
+ *   wd     - Reference to the words to send
+ *   nwords - number of words to send
+ *
+ * Returned Value:
+ *   OK - On Success
+ *
+ ****************************************************************************/
+
+static int nrf52_ili93414ws_sendgram(FAR struct ili9341_lcd_s *lcd,

Review comment:
       > The location where the blitting is done is in graphics/nxglib/lcd. But that uses macros defined in graphics/nxglib/nxglib_bitblit.h.
   
   Sorry, that is bad advice.  Those macros are only used by the framebuffer bit-blitters (graphics/nxglib/fb).  The blitting is hardcoded for the LCD in the files unser graphics/nxglib/lcd.
   
   So you can't change the blitting in one place but rather in all of the 6 files at that location.  Sorry.
   
   




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

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



[GitHub] [incubator-nuttx] btashton edited a comment on pull request #1778: WIP board: Keyboard FeatherWing on nrf52-feather

Posted by GitBox <gi...@apache.org>.
btashton edited a comment on pull request #1778:
URL: https://github.com/apache/incubator-nuttx/pull/1778#issuecomment-692166575


   Yeah that is where I was saying having the ability for NX to render "rectangles" that are short in the X axis in one transfer would be nice because right now a vertical line is expensive.
   
   I did set my gram byte swap buffer to the max of 127 grams for DMA and it improved the performance quite a bit.  You can see it here:
   https://media.giphy.com/media/j736EdTwUBwQHiPKhS/giphy.gif
   
   I am going to leave the performance bits aside for now and work on adding the other hardware support.


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

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



[GitHub] [incubator-nuttx] btashton commented on a change in pull request #1778: WIP board: Keyboard FeatherWing on nrf52-feather

Posted by GitBox <gi...@apache.org>.
btashton commented on a change in pull request #1778:
URL: https://github.com/apache/incubator-nuttx/pull/1778#discussion_r487673440



##########
File path: boards/arm/nrf52/nrf52-feather/src/nrf52_ili9341_lcd.c
##########
@@ -0,0 +1,410 @@
+/****************************************************************************
+ * boards/arm/nrf52/nrf52-feather/src/nrf52_ili9341_lcd.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <stdint.h>
+#include <stdbool.h>
+#include <endian.h>
+#include <debug.h>
+
+#include <nuttx/spi/spi.h>
+#include <nuttx/lcd/lcd.h>
+#include <nuttx/lcd/ili9341.h>
+
+#include "chip.h"
+#include "arm_arch.h"
+#include "arm_internal.h"
+#include "nrf52_gpio.h"
+#include "nrf52_spi.h"
+
+#include "nrf52-feather.h"
+#include <arch/board/board.h>
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+#define GRAM_ENDIAN_SWAP_BUFFER_LEN 1    /* Gram swap space */
+
+#if (GRAM_ENDIAN_SWAP_BUFFER_LEN > 127)
+#  error "GRAM_ENDIAN_SWAP_BUFFER_LEN must fit in a SPI transfer 127 words!"
+#endif
+
+/****************************************************************************
+ * Private Type Definition
+ ****************************************************************************/
+
+struct ili93414ws_lcd_s
+{
+  struct ili9341_lcd_s dev;
+  FAR struct spi_dev_s *spi;
+};
+
+/****************************************************************************
+ * Private Function Protototypes
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+static struct ili93414ws_lcd_s g_lcddev;
+static struct lcd_dev_s *g_lcd = NULL;
+static uint8_t swapbuf[GRAM_ENDIAN_SWAP_BUFFER_LEN * 2];
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: nrf52_ili93414ws_backlight
+ *
+ * Description:
+ *   Set the backlight level of the connected display.
+ *
+ * Input Parameters:
+ *   spi   - Reference to the public driver structure
+ *   level - backlight level
+ *
+ * Returned Value:
+ *   OK - On Success
+ *
+ ****************************************************************************/
+
+static int nrf52_ili93414ws_backlight(FAR struct ili9341_lcd_s *lcd,
+                                      int level)
+{
+  return OK;
+}
+
+/****************************************************************************
+ * Name: nrf52_ili93414ws_select
+ *
+ * Description:
+ *   Select the SPI, locking and re-configuring if necessary
+ *
+ * Input Parameters:
+ *   spi  - Reference to the public driver structure
+ *
+ * Returned Value:
+ *
+ ****************************************************************************/
+
+static void nrf52_ili93414ws_select(FAR struct ili9341_lcd_s *lcd)
+{
+  FAR struct ili93414ws_lcd_s *priv = (FAR struct ili93414ws_lcd_s *)lcd;
+
+  SPI_LOCK(priv->spi, true);
+  SPI_SELECT(priv->spi, SPIDEV_DISPLAY(0), true);
+}
+
+/****************************************************************************
+ * Name: nrf52_ili93414ws_deselect
+ *
+ * Description:
+ *   De-select the SPI
+ *
+ * Input Parameters:
+ *   spi  - Reference to the public driver structure
+ *
+ * Returned Value:
+ *
+ ****************************************************************************/
+
+static void nrf52_ili93414ws_deselect(FAR struct ili9341_lcd_s *lcd)
+{
+  FAR struct ili93414ws_lcd_s *priv = (FAR struct ili93414ws_lcd_s *)lcd;
+
+  SPI_LOCK(priv->spi, false);
+  SPI_SELECT(priv->spi, SPIDEV_DISPLAY(0), false);
+}
+
+/****************************************************************************
+ * Name: nrf52_ili93414ws_sndcmd
+ *
+ * Description:
+ *   Send a command to the lcd driver.
+ *
+ * Input Parameters:
+ *   lcd  - Reference to the ili9341_lcd_s driver structure
+ *   cmd  - command to send
+ *
+ * Returned Value:
+ *   On success - OK
+ *
+ ****************************************************************************/
+
+static int nrf52_ili93414ws_sendcmd(
+    FAR struct ili9341_lcd_s *lcd, const uint8_t cmd)
+{
+  FAR struct ili93414ws_lcd_s *priv = (FAR struct ili93414ws_lcd_s *)lcd;
+
+  lcdinfo("%02x\n", cmd);
+
+  nrf52_gpio_write(ILI9341_DISPLAY_DC, false); /* Indicate CMD */
+  SPI_SEND(priv->spi, cmd);
+
+  return OK;
+}
+
+/****************************************************************************
+ * Name: nrf52_ili93414ws_sendparam
+ *
+ * Description:
+ *   Send a parameter to the lcd driver.
+ *
+ * Input Parameters:
+ *   lcd    - Reference to the ili9341_lcd_s driver structure
+ *   param  - parameter to send
+ *
+ * Returned Value:
+ *   OK - On Success
+ *
+ ****************************************************************************/
+
+static int nrf52_ili93414ws_sendparam(FAR struct ili9341_lcd_s *lcd,
+                                      const uint8_t param)
+{
+  FAR struct ili93414ws_lcd_s *priv = (FAR struct ili93414ws_lcd_s *)lcd;
+
+  nrf52_gpio_write(ILI9341_DISPLAY_DC, true);  /* Indicate DATA */
+  SPI_SEND(priv->spi, param);
+
+  return OK;
+}
+
+/****************************************************************************
+ * Name: nrf52_ili93414ws_sendgram
+ *
+ * Description:
+ *   Send a number of pixel words to the lcd driver gram.
+ *
+ * Input Parameters:
+ *   lcd    - Reference to the ili9341_lcd_s driver structure
+ *   wd     - Reference to the words to send
+ *   nwords - number of words to send
+ *
+ * Returned Value:
+ *   OK - On Success
+ *
+ ****************************************************************************/
+
+static int nrf52_ili93414ws_sendgram(FAR struct ili9341_lcd_s *lcd,

Review comment:
       @patacongo I for the gram send recv interfaces I had to add a byte swapping buffer because nrf52 SPI hardware does not support 16bit SPI.  If there was a way to tell the whole graphics stack to use BE instead of LE for color this could be avoided.  The buffer can be configured in size, but right now it is just set to the minimal of a single gram.  This also means that there is no real DMA advantage since the transfers are only 2 bytes at a time.




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

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