Skip to content

Commit

Permalink
drivers: display: ili9340: code cleanup
Browse files Browse the repository at this point in the history 
Multiple enhancements towards better code readability and consistency:

- sorted headers
- define and reference magic constants
- adjust some names
- add U suffix to unsigned constants
- move hw reset to a function
- remove non-needed initialization code from seeed tft

Signed-off-by: Gerard Marull-Paretas <[email protected]>
  • Loading branch information
@gmarull @MaureenHelm
gmarull authored and MaureenHelm committed Sep 29, 2020
1 parent 2d4ec8f commit b406527
Show file tree
Hide file tree
Showing 3 changed files with 83 additions and 73 deletions.
118 changes: 62 additions & 56 deletions drivers/display/display_ili9340.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,16 +9,16 @@
#define DT_DRV_COMPAT ilitek_ili9340

#include "display_ili9340.h"
#include <drivers/display.h>

#define LOG_LEVEL CONFIG_DISPLAY_LOG_LEVEL
#include <logging/log.h>
LOG_MODULE_REGISTER(display_ili9340);
#include <string.h>

#include <drivers/display.h>
#include <drivers/gpio.h>
#include <sys/byteorder.h>
#include <drivers/spi.h>
#include <string.h>
#include <sys/byteorder.h>

#include <logging/log.h>
LOG_MODULE_REGISTER(display_ili9340, CONFIG_DISPLAY_LOG_LEVEL);

struct ili9340_config {
const char *spi_name;
Expand All @@ -43,9 +43,6 @@ struct ili9340_data {
struct spi_cs_control cs_ctrl;
};

#define ILI9340_CMD_DATA_PIN_COMMAND 1
#define ILI9340_CMD_DATA_PIN_DATA 0

/* The number of bytes taken by a RGB pixel */
#ifdef CONFIG_ILI9340_RGB565
#define ILI9340_RGB_SIZE 2U
Expand All @@ -62,46 +59,50 @@ static int ili9340_exit_sleep(const struct device *dev)
return r;
}

k_sleep(K_MSEC(120));
k_sleep(K_MSEC(ILI9340_SLEEP_OUT_TIME));

return 0;
}

static void ili9340_hw_reset(const struct device *dev)
{
const struct ili9340_config *config = (struct ili9340_config *)dev->config;
struct ili9340_data *data = (struct ili9340_data *)dev->data;

if (data->reset_gpio == NULL) {
return;
}

gpio_pin_set(data->reset_gpio, config->reset_pin, 1);
k_sleep(K_MSEC(ILI9340_RESET_PULSE_TIME));
gpio_pin_set(data->reset_gpio, config->reset_pin, 0);

k_sleep(K_MSEC(ILI9340_RESET_WAIT_TIME));
}

static int ili9340_init(const struct device *dev)
{
const struct ili9340_config *config = (struct ili9340_config *)dev->config;
struct ili9340_data *data = (struct ili9340_data *)dev->data;

int r;

LOG_DBG("Initializing display driver");

data->spi_dev = device_get_binding(config->spi_name);
if (data->spi_dev == NULL) {
LOG_ERR("Could not get SPI device %s", config->spi_name);
return -ENODEV;
}

data->spi_config.frequency = config->spi_max_freq;
data->spi_config.operation = SPI_OP_MODE_MASTER | SPI_WORD_SET(8);
data->spi_config.operation = SPI_OP_MODE_MASTER | SPI_WORD_SET(8U);
data->spi_config.slave = config->spi_addr;

data->cs_ctrl.gpio_dev = device_get_binding(config->spi_cs_label);
if (data->cs_ctrl.gpio_dev) {
if (data->cs_ctrl.gpio_dev != NULL) {
data->cs_ctrl.gpio_pin = config->spi_cs_pin;
data->cs_ctrl.gpio_dt_flags = config->spi_cs_flags;
data->cs_ctrl.delay = 0U;
data->spi_config.cs = &(data->cs_ctrl);
}

data->reset_gpio = device_get_binding(config->reset_label);
if (data->reset_gpio) {
r = gpio_pin_configure(data->reset_gpio, config->reset_pin,
GPIO_OUTPUT_INACTIVE | config->reset_flags);
if (r < 0) {
LOG_ERR("Could not configure reset GPIO (%d)", r);
return r;
}
data->spi_config.cs = &data->cs_ctrl;
}

data->command_data_gpio = device_get_binding(config->cmd_data_label);
Expand All @@ -117,23 +118,24 @@ static int ili9340_init(const struct device *dev)
return r;
}

if (data->reset_gpio) {
LOG_DBG("Resetting display driver");
k_sleep(K_MSEC(1));
gpio_pin_set(data->reset_gpio, config->reset_pin, 1);
k_sleep(K_MSEC(1));
gpio_pin_set(data->reset_gpio, config->reset_pin, 0);
k_sleep(K_MSEC(5));
data->reset_gpio = device_get_binding(config->reset_label);
if (data->reset_gpio != NULL) {
r = gpio_pin_configure(data->reset_gpio, config->reset_pin,
GPIO_OUTPUT_INACTIVE | config->reset_flags);
if (r < 0) {
LOG_ERR("Could not configure reset GPIO (%d)", r);
return r;
}
}

LOG_DBG("Initializing LCD");
ili9340_hw_reset(dev);

r = ili9340_lcd_init(dev);
if (r < 0) {
LOG_ERR("Could not initialize LCD (%d)", r);
return r;
}

LOG_DBG("Exiting sleep mode");
r = ili9340_exit_sleep(dev);
if (r < 0) {
LOG_ERR("Could not exit sleep mode (%d)", r);
Expand All @@ -144,21 +146,21 @@ static int ili9340_init(const struct device *dev)
}

static int ili9340_set_mem_area(const struct device *dev, const uint16_t x,
const uint16_t y, const uint16_t w, const uint16_t h)
const uint16_t y, const uint16_t w, const uint16_t h)
{
int r;
uint16_t spi_data[2];

spi_data[0] = sys_cpu_to_be16(x);
spi_data[1] = sys_cpu_to_be16(x + w - 1);
r = ili9340_transmit(dev, ILI9340_CMD_COLUMN_ADDR, &spi_data[0], 4);
spi_data[1] = sys_cpu_to_be16(x + w - 1U);
r = ili9340_transmit(dev, ILI9340_CMD_COLUMN_ADDR, &spi_data[0], 4U);
if (r < 0) {
return r;
}

spi_data[0] = sys_cpu_to_be16(y);
spi_data[1] = sys_cpu_to_be16(y + h - 1);
r = ili9340_transmit(dev, ILI9340_CMD_PAGE_ADDR, &spi_data[0], 4);
spi_data[1] = sys_cpu_to_be16(y + h - 1U);
r = ili9340_transmit(dev, ILI9340_CMD_PAGE_ADDR, &spi_data[0], 4U);
if (r < 0) {
return r;
}
Expand All @@ -174,16 +176,16 @@ static int ili9340_write(const struct device *dev, const uint16_t x,
struct ili9340_data *data = (struct ili9340_data *)dev->data;

int r;
const uint8_t *write_data_start = (uint8_t *) buf;
const uint8_t *write_data_start = (const uint8_t *)buf;
struct spi_buf tx_buf;
struct spi_buf_set tx_bufs;
uint16_t write_cnt;
uint16_t nbr_of_writes;
uint16_t write_h;

__ASSERT(desc->width <= desc->pitch, "Pitch is smaller then width");
__ASSERT((desc->pitch * ILI9340_RGB_SIZE * desc->height)
<= desc->buf_size, "Input buffer to small");
__ASSERT(desc->width <= desc->pitch, "Pitch is smaller than width");
__ASSERT((desc->pitch * ILI9340_RGB_SIZE * desc->height) <= desc->buf_size,
"Input buffer to small");

LOG_DBG("Writing %dx%d (w,h) @ %dx%d (x,y)", desc->width, desc->height,
x, y);
Expand All @@ -201,7 +203,7 @@ static int ili9340_write(const struct device *dev, const uint16_t x,
}

r = ili9340_transmit(dev, ILI9340_CMD_MEM_WRITE,
(void *) write_data_start,
write_data_start,
desc->width * ILI9340_RGB_SIZE * write_h);
if (r < 0) {
return r;
Expand All @@ -220,7 +222,7 @@ static int ili9340_write(const struct device *dev, const uint16_t x,
return r;
}

write_data_start += (desc->pitch * ILI9340_RGB_SIZE);
write_data_start += desc->pitch * ILI9340_RGB_SIZE;
}

return 0;
Expand Down Expand Up @@ -256,7 +258,7 @@ static int ili9340_display_blanking_on(const struct device *dev)
static int ili9340_set_brightness(const struct device *dev,
const uint8_t brightness)
{
LOG_WRN("Set brightness not implemented");
LOG_ERR("Set brightness not implemented");
return -ENOTSUP;
}

Expand Down Expand Up @@ -295,8 +297,8 @@ static void ili9340_get_capabilities(const struct device *dev,
struct display_capabilities *capabilities)
{
memset(capabilities, 0, sizeof(struct display_capabilities));
capabilities->x_resolution = 320U;
capabilities->y_resolution = 240U;
capabilities->x_resolution = ILI9340_X_RES;
capabilities->y_resolution = ILI9340_Y_RES;
#ifdef CONFIG_ILI9340_RGB565
capabilities->supported_pixel_formats = PIXEL_FORMAT_RGB_565;
capabilities->current_pixel_format = PIXEL_FORMAT_RGB_565;
Expand All @@ -307,28 +309,32 @@ static void ili9340_get_capabilities(const struct device *dev,
capabilities->current_orientation = DISPLAY_ORIENTATION_NORMAL;
}

int ili9340_transmit(const struct device *dev, uint8_t cmd, void *tx_data,
int ili9340_transmit(const struct device *dev, uint8_t cmd, const void *tx_data,
size_t tx_len)
{
const struct ili9340_config *config = (struct ili9340_config *)dev->config;
struct ili9340_data *data = (struct ili9340_data *)dev->data;

int r;
struct spi_buf tx_buf = { .buf = &cmd, .len = 1 };
struct spi_buf_set tx_bufs = { .buffers = &tx_buf, .count = 1 };
struct spi_buf tx_buf;
struct spi_buf_set tx_bufs = { .buffers = &tx_buf, .count = 1U };

gpio_pin_set(data->command_data_gpio, config->cmd_data_pin,
ILI9340_CMD_DATA_PIN_COMMAND);
/* send command */
tx_buf.buf = &cmd;
tx_buf.len = 1U;

gpio_pin_set(data->command_data_gpio, config->cmd_data_pin, ILI9340_CMD);
r = spi_write(data->spi_dev, &data->spi_config, &tx_bufs);
if (r < 0) {
return r;
}

/* send data (if any) */
if (tx_data != NULL) {
tx_buf.buf = tx_data;
tx_buf.buf = (void *)tx_data;
tx_buf.len = tx_len;
gpio_pin_set(data->command_data_gpio, config->cmd_data_pin,
ILI9340_CMD_DATA_PIN_DATA);

gpio_pin_set(data->command_data_gpio, config->cmd_data_pin, ILI9340_DATA);
r = spi_write(data->spi_dev, &data->spi_config, &tx_bufs);
if (r < 0) {
return r;
Expand Down
22 changes: 21 additions & 1 deletion drivers/display/display_ili9340.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,26 @@
#define ILI9340_DATA_PIXEL_FORMAT_MCU_18_BIT 0x06
#define ILI9340_DATA_PIXEL_FORMAT_MCU_16_BIT 0x05

/** Command/data GPIO level for commands. */
#define ILI9340_CMD 1U
/** Command/data GPIO level for data. */
#define ILI9340_DATA 0U

/** Sleep out time (ms), ref. 8.2.12 of ILI9340 manual. */
#define ILI9340_SLEEP_OUT_TIME 120

/** Reset pulse time (ms), ref 15.4 of ILI9340 manual. */
#define ILI9340_RESET_PULSE_TIME 1

/** Reset wait time (ms), ref 15.4 of ILI9340 manual. */
#define ILI9340_RESET_WAIT_TIME 5

/** X resolution (pixels). */
#define ILI9340_X_RES 320U

/** Y resolution (pixels). */
#define ILI9340_Y_RES 240U

/**
* Send data to ILI9340 display controller
*
Expand All @@ -62,7 +82,7 @@
*
* @return 0 on success, errno otherwise.
*/
int ili9340_transmit(const struct device *dev, uint8_t cmd, void *tx_data,
int ili9340_transmit(const struct device *dev, uint8_t cmd, const void *tx_data,
size_t tx_len);

/**
Expand Down
16 changes: 0 additions & 16 deletions drivers/display/display_ili9340_seeed_tftv2.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -220,21 +220,5 @@ int ili9340_lcd_init(const struct device *dev)
return r;
}

/* Sleep Out */
cmd = ILI9340_CMD_EXIT_SLEEP;
r = ili9340_transmit(dev, cmd, NULL, 0);
if (r < 0) {
return r;
}

k_sleep(K_MSEC(120));

/* Display Off */
cmd = ILI9340_CMD_DISPLAY_OFF;
r = ili9340_transmit(dev, cmd, NULL, 0);
if (r < 0) {
return r;
}

return 0;
}

0 comments on commit b406527

Please sign in to comment.