Hello and a little bug report

[tubbutec]

Hi,

I just started using ugfx on my platform. I have been programming bare metal before and now switched to chibios and ugfx. Thanks for the great work, I will try to contribute as much as I can.

I am not sure, maybe I am using the library incorrectly, but I might have found a small bug in the ILI9341 driver.

I am using 16 bit FSMC and had to change in gdisp_lld_ILI9341.c

#define write_data16(g, data)	{ write_data(g, data >> 8); write_data(g, (uint8_t)data); }

to

#define write_data16(g, data)		write_data(g, data)

other wise it would display garbage (obviously)

Which is the correct place to switch from 8 bit to 16 bit mode? somehow in the boardfile?

Thanks

[inmarket]

It is not a bug so much as a difference in how the display is spoken to via a 16 bit interface versus a 8 bit interface.

The code you have changed is correct for a 16 bit hardware interface but will break an 8 bit bus interface. The code has previously only been used on such chips with an 8 bit processor interface. This driver is currently used successfully as-is on such boards as the Mikromedia STM32m4 board (which use an 8 bit cpu interface for the display).

Note that changing the bus interface width may also require changing some of the initialisation code to ensure the controller expects 16 bit wide data.

I will re-examine the code for the driver. It should be possible to define a MACRO that specifies 8 or 16 bit bus width and to alter the code correspondingly.

Can you please post your board file and your (modified) driver file and I will build an integrated solution for the master repository so we don't break boards that are using it with an 8 bit bus.

[tubbutec]

Sorry, wanted to attach these files, but .h is not an allowed filetype for uploading ..

I didn't change anything in the driver file except the above mentioned macro. The display is set to 16 bit data mode using external jumpers.

gdisp_lld_ILI9341.c

#define write_data16(g, data)		write_data(g, data)/*{ write_data(g, data >> 8); write_data(g, (uint8_t)data); }*/

Here is the board file. This is pretty much taken from board_SSD2119.h, but without the dma stuff:

board ILI9341.h

/*
 * This file is subject to the terms of the GFX License. If a copy of
 * the license was not distributed with this file, you can obtain one at:
 *
 *              http://ugfx.org/license.html
 */

#ifndef GDISP_LLD_BOARD_H
#define GDISP_LLD_BOARD_H

#include "stm32f4xx.h"

#define ILI9341_ROTATE180

// For a multiple display configuration we would put all this in a structure and then
//	set g->board to that structure.
#define GDISP_REG              (*((volatile uint16_t *) 0x60000000)) /* RS = 0 */
#define GDISP_RAM              (*((volatile uint16_t *) 0x60100000)) /* RS = 1 */

static inline void init_board(GDisplay *g) {

	// As we are not using multiple displays we set g->board to NULL as we don't use it.
	g->board = 0;

	switch(g->controllerdisplay) {
	case 0:											// Set up for Display 0
		/* FSMC setup for F4 */
		rccEnableAHB3(RCC_AHB3ENR_FSMCEN, 0);


		/* Group pins */
		IOBus busD = {GPIOD, (1 << 0) | (1 << 1) | (1 << 4) | (1 << 5) | (1 << 7) | (1 << 8) |
								(1 << 9) | (1 << 10) | (1 << 14) | (1 << 15), 0};

		IOBus busE = {GPIOE, (1 << 3) | (1 << 7) | (1 << 8) | (1 << 9) | (1 << 10) | (1 << 11) | (1 << 12) |
								(1 << 13) | (1 << 14) | (1 << 15), 0};

		/* FSMC is an alternate function 12 (AF12) */
		palSetBusMode(&busD, PAL_MODE_ALTERNATE(12));
		palSetBusMode(&busE, PAL_MODE_ALTERNATE(12));

		/* FSMC timing register configuration */
		FSMC_Bank1->BTCR[0 + 1] = (FSMC_BTR1_ADDSET_2 | FSMC_BTR1_ADDSET_1) \
				| (FSMC_BTR1_DATAST_2 | FSMC_BTR1_DATAST_1) \
				| FSMC_BTR1_BUSTURN_0;


		/* Bank1 NOR/PSRAM control register configuration
		 * Write enable, memory databus width set to 16 bit, memory bank enable */
		FSMC_Bank1->BTCR[0] = FSMC_BCR1_WREN | FSMC_BCR1_MWID_0 | FSMC_BCR1_MBKEN;
		break;
	}
}

static inline void post_init_board(GDisplay *g) {
	(void) g;
}

static inline void setpin_reset(GDisplay *g, bool_t state) {
	(void) g;

	//if(state)
	//	palClearPad(GPIOE, GPIOE_TFT_RST);
	//else
	//	palSetPad(GPIOE, GPIOE_TFT_RST);
}

static inline void set_backlight(GDisplay *g, uint8_t percent) {
	(void) g;

	//if(percent)
	//	palClearPad(GPIOD, GPIOD_TFT_LIGHT);
	//else
	//	palSetPad(GPIOD, GPIOD_TFT_LIGHT);
}

static inline void acquire_bus(GDisplay *g) {
	(void) g;
}

static inline void release_bus(GDisplay *g) {
	(void) g;
}

static inline void write_index(GDisplay *g, uint16_t index) {
	(void) g;

	GDISP_REG = index;
}

static inline void write_data(GDisplay *g, uint16_t data) {
	(void) g;

	GDISP_RAM = data;
}

static inline void setreadmode(GDisplay *g) {
	(void) g;
}

static inline void setwritemode(GDisplay *g) {
	(void) g;
}

static inline uint16_t read_data(GDisplay *g) {
	(void) g;

	return GDISP_RAM;
}

#endif /* GDISP_LLD_BOARD_H */

[Joel Bodenmann]

Sorry, wanted to attach these files, but .h is not an allowed filetype for uploading ..

That is correct. It is recommended to place all your files you want to upload into a .zip file as these files are attachable.

I'll let inmarket handle this driver issue.

~ Tectu

[sordfish]

I know its an old post but it was the only one to explain why I was only getting blue using the stock driver.

after the mod for 16bit I have all colours

Many thanks all!

[Joel Bodenmann]

Hello sordfish and welcome to the community!

It looks like we forgot to apply these changes. I put it on the ToDo list!

Thank you for reminding us

~ Tectu