notro / fbtft

Linux Framebuffer drivers for small TFT LCD display modules. Development has moved to https://git.kernel.org/cgit/linux/kernel/git/gregkh/staging.git/tree/drivers/staging/fbtft?h=staging-testing
1.86k stars 494 forks source link

Freetronics OLED128 support (SSD1351) #34

Closed projectgus closed 11 years ago

projectgus commented 11 years ago

Hi Notro,

Thanks very much for fbtft! I work for Freetronics and I've been looking at integrating a new open hardware OLED module (model "OLED128") into fbtft.

It's great to think "I wonder if anyone has done something like this..." and find a project as active as fbtft!

Unfortunately I didn't pay a lot of attention before I started integrating, and I've worked against the old non-generic ssd1351fb.c driver which has just been deleted: https://github.com/projectgus/fbtft/compare/ft_oled128

Before I rebase against fb_ssd1351.c, I was hoping I could ask some advice about the approach.

The main difference for our OLED128 is that we use one of the SSD1351's onboard GPIOs to enable/disable the onboard boost converter for the +15V panel power.

So the addition I made was to register two GPIOs as part of the ssd1351 driver which can then be assigned as the backlight "led" pin, which causes them to be turned on and off (the sequence isn't quite the same as the controller datasheet requests, but it seems to work perfectly well from what I can tell.)

This seemed like the easiest and most "Linux-like" way, but I thought I'd run it past you now before I go and change the code over to use fb_ssd1351.c. The alternative I could see would be to put the ssd1351 GPIO driver into a separate module, but that seemed like it might be overkill given the GPIOs are really part of the same ssd1351 unit.

What do you think? Do you have any comments on my branch, apart from "you based it on the wrong driver"? :)

Cheers,

PS If you are happy to merge support for the OLED128, Freetronics is committed to supporting the module and you can feel free to direct any module-specific queries to me/us. We're also happy to provide you with some sample hardware if that helps at all for testing future updates.

notro commented 11 years ago

The main difference for our OLED128 is that we use one of the SSD1351's onboard GPIOs to enable/disable the onboard boost converter for the +15V panel power.

Yes, I have seen that some LCD controllers have GPIOs available. But since I don't have a display that uses this for backlight, I haven't integrated that functionality into FBTFT. I'm not sure what will be the best solution for this, but since I'm rather busy right now, I can't look into it in detail. I recommend that you use fbtftops.register_backlight to register your own backlight device for now. Have a look at the default implementation: https://github.com/notro/fbtft/blob/master/fbtft-core.c#L230

But what do we do to make this configurable, so the driver can use both methods? fbtftops.register_backlight can be set in init_display() since it is called before register_backlight. I have a property display->backlight I was thinking should choose between backlight implementations, but this is not available in init_display(). Well, OK, I think maybe you have to spell out FBTFT_REGISTER_DRIVER() in you driver and in the probe() function you can set display.register_backlight() if display->backlight == 2. Then let your probe() function call fbtft_probe_common(). Then in fbtft_device set backlight=2 for your display device.

Let me know if any of this is unclear or plain unimplementable.

Also have a look at this: https://github.com/notro/fbtft/wiki/Development#submitting-patches

projectgus commented 11 years ago

Hi notro,

Thanks for all the pointers. Sorry it's taken me so long to reply (got busy with other things.)

Here's a patch using what I understood as the approach you outlined. Turned out I could use the boilerplate FBTFT_REGISTER_DRIVER, which was good.

From 3ff4b43ba3c471ff0de91e9926c0e02f5462864a Mon Sep 17 00:00:00 2001
From: Angus Gratton <angus@freetronics.com>
Date: Thu, 29 Aug 2013 11:00:24 +1000
Subject: [PATCH] Add support for Freetronics OLED128 module (SSD1351 based)
 Including support for driving backlight (display panel power) from a GPIO
 onboard the SSD1351

Signed-off-by: Angus Gratton <angus@freetronics.com>
---
 fb_ssd1351.c   | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 fbtft.h        |  1 +
 fbtft_device.c | 19 ++++++++++++++++++
 3 files changed, 82 insertions(+)

diff --git a/fb_ssd1351.c b/fb_ssd1351.c
index 7203d7b..aff00ae 100644
--- a/fb_ssd1351.c
+++ b/fb_ssd1351.c
@@ -21,11 +21,17 @@
            "2 2 2 2 2 2 2 2 " \
            "2 2 2 2 2 2 2" \

+static void register_onboard_backlight(struct fbtft_par *par);

 static int init_display(struct fbtft_par *par)
 {
    fbtft_par_dbg(DEBUG_INIT_DISPLAY, par, "%s()\n", __func__);

+   if (par->pdata->display.backlight == FBTFT_ONBOARD_BACKLIGHT) {
+       /* module uses onboard GPIO for panel power */
+       par->fbtftops.register_backlight = register_onboard_backlight;
+   }
+
    par->fbtftops.reset(par);

    write_reg(par, 0xfd, 0x12); /* Command Lock */
@@ -148,6 +154,62 @@ static struct fbtft_display display = {
        .blank = blank,
    },
 };
+
+static void update_onboard_backlight(struct backlight_device *bd)
+{
+   struct fbtft_par *par = bl_get_data(bd);
+   bool on;
+
+   fbtft_par_dbg(DEBUG_BACKLIGHT, par,
+       "%s: power=%d, fb_blank=%d\n",
+       __func__, bd->props.power, bd->props.fb_blank);
+
+   on = (bd->props.power == FB_BLANK_UNBLANK)
+       && (bd->props.fb_blank == FB_BLANK_UNBLANK);
+   /* Onboard backlight connected to GPIO0 on SSD1351, GPIO1 unused */
+   write_reg(par, 0xB5, on ? 0x03 : 0x02);
+}
+
+static void register_onboard_backlight(struct fbtft_par *par)
+{
+   struct backlight_device *bd;
+   struct backlight_properties bl_props = { 0, };
+   struct backlight_ops *bl_ops;
+
+   fbtft_par_dbg(DEBUG_BACKLIGHT, par, "%s()\n", __func__);
+
+   bl_ops = kzalloc(sizeof(struct backlight_ops), GFP_KERNEL);
+   if (!bl_ops) {
+       dev_err(par->info->device,
+           "%s: could not allocate memory for backlight operations.\n",
+           __func__);
+       return;
+   }
+
+   bl_ops->update_status = update_onboard_backlight;
+   bl_props.type = BACKLIGHT_RAW;
+   bl_props.power = FB_BLANK_POWERDOWN;
+
+   bd = backlight_device_register(dev_driver_string(par->info->device),
+               par->info->device, par, bl_ops, &bl_props);
+   if (IS_ERR(bd)) {
+       dev_err(par->info->device,
+           "cannot register backlight device (%ld)\n",
+           PTR_ERR(bd));
+       goto failed;
+   }
+   par->info->bl_dev = bd;
+
+   if (!par->fbtftops.unregister_backlight)
+       par->fbtftops.unregister_backlight = fbtft_unregister_backlight;
+
+   return;
+failed:
+   kfree(bl_ops);
+}
+
+
+
 FBTFT_REGISTER_DRIVER(DRVNAME, &display);

 MODULE_ALIAS("spi:" DRVNAME);
diff --git a/fbtft.h b/fbtft.h
index e0fb851..ca8cb71 100644
--- a/fbtft.h
+++ b/fbtft.h
@@ -33,6 +33,7 @@
 #define FBTFT_RASET        0x2B
 #define FBTFT_RAMWR        0x2C

+#define FBTFT_ONBOARD_BACKLIGHT 2

 #define FBTFT_GPIO_NO_MATCH        0xFFFF
 #define FBTFT_GPIO_NAME_SIZE   32
diff --git a/fbtft_device.c b/fbtft_device.c
index 74714ee..502c585 100644
--- a/fbtft_device.c
+++ b/fbtft_device.c
@@ -242,6 +242,25 @@ static struct fbtft_device_display displays[] = {
            }
        }
    }, {
+       .name = "freetronicsoled128",
+       .spi = &(struct spi_board_info) {
+           .modalias = "fb_ssd1351",
+           .max_speed_hz = 20000000,
+           .mode = SPI_MODE_0,
+           .platform_data = &(struct fbtft_platform_data) {
+               .display = {
+                   .buswidth = 8,
+                   .backlight = FBTFT_ONBOARD_BACKLIGHT,
+               },
+               .bgr = true,
+               .gpios = (const struct fbtft_gpio []) {
+                   { "reset", 24 },
+                   { "dc", 25 },
+                   {},
+               },
+           }
+       }
+   }, {
        .name = "hy28a",
        .spi = &(struct spi_board_info) {
            .modalias = "fb_ili9320",

I did do a bit of other work investigating a more generic implementation, adding support for "onboard GPIOs" (ie GPIOs on the display controller) as a generic property of any driver, so these are registered with the kernel GPIO framework at probe time and can then be controlled. I still couldn't find an elegant way to assign these to pins on the fbtft device itself though, things got a bit fiddly so I stopped working on it.

Please let me know what you think of this patch.

Regards,

Angus

notro commented 11 years ago

You proved me wrong about backlight not being available in init_display :-)

The patch looks fine except for a couple of things:

Regarding your other init sequence patch: I won't change the default init sequence in the driver (it is just a default which at this point matches another display), so you have to add a custom display.init_sequence to your panel in fbtft_device. Having a custom init_sequence means fbtft_init_display() will be used instead of fb_ssd1351: init_display(). You can move the register_backlight assignement to set_var() instead (see one of the other drivers). set_var() is called before register_backlight()

Please check that par->pdata is not NULL before using it: if (par->pdata && par->pdata->display.backlight == FBTFT_ONBOARD_BACKLIGHT) {

projectgus commented 11 years ago

Thanks for the comments notro.

Here's a revised version of the patch will the null check. I've decided to leave the clock divider for now to keep things simple, please see comment in other issue.

Thanks again,

Angus

From 6cdc56b95bcfc4c180bb464bff1a00a0347b71e0 Mon Sep 17 00:00:00 2001
From: Angus Gratton <angus@freetronics.com>
Date: Thu, 29 Aug 2013 11:00:24 +1000
Subject: [PATCH] Add support for Freetronics OLED128 module (SSD1351 based)
 Including support for driving backlight (display panel power) from a GPIO
 onboard the SSD1351

Signed-off-by: Angus Gratton <angus@freetronics.com>
---
 fb_ssd1351.c   | 63 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 fbtft.h        |  1 +
 fbtft_device.c | 19 ++++++++++++++++++
 3 files changed, 83 insertions(+)

diff --git a/fb_ssd1351.c b/fb_ssd1351.c
index 7203d7b..c693f2c 100644
--- a/fb_ssd1351.c
+++ b/fb_ssd1351.c
@@ -21,11 +21,18 @@
            "2 2 2 2 2 2 2 2 " \
            "2 2 2 2 2 2 2" \

+static void register_onboard_backlight(struct fbtft_par *par);

 static int init_display(struct fbtft_par *par)
 {
    fbtft_par_dbg(DEBUG_INIT_DISPLAY, par, "%s()\n", __func__);

+   if (par->pdata
+       && par->pdata->display.backlight == FBTFT_ONBOARD_BACKLIGHT) {
+       /* module uses onboard GPIO for panel power */
+       par->fbtftops.register_backlight = register_onboard_backlight;
+   }
+
    par->fbtftops.reset(par);

    write_reg(par, 0xfd, 0x12); /* Command Lock */
@@ -148,6 +155,62 @@ static struct fbtft_display display = {
        .blank = blank,
    },
 };
+
+static void update_onboard_backlight(struct backlight_device *bd)
+{
+   struct fbtft_par *par = bl_get_data(bd);
+   bool on;
+
+   fbtft_par_dbg(DEBUG_BACKLIGHT, par,
+       "%s: power=%d, fb_blank=%d\n",
+       __func__, bd->props.power, bd->props.fb_blank);
+
+   on = (bd->props.power == FB_BLANK_UNBLANK)
+       && (bd->props.fb_blank == FB_BLANK_UNBLANK);
+   /* Onboard backlight connected to GPIO0 on SSD1351, GPIO1 unused */
+   write_reg(par, 0xB5, on ? 0x03 : 0x02);
+}
+
+static void register_onboard_backlight(struct fbtft_par *par)
+{
+   struct backlight_device *bd;
+   struct backlight_properties bl_props = { 0, };
+   struct backlight_ops *bl_ops;
+
+   fbtft_par_dbg(DEBUG_BACKLIGHT, par, "%s()\n", __func__);
+
+   bl_ops = kzalloc(sizeof(struct backlight_ops), GFP_KERNEL);
+   if (!bl_ops) {
+       dev_err(par->info->device,
+           "%s: could not allocate memory for backlight operations.\n",
+           __func__);
+       return;
+   }
+
+   bl_ops->update_status = update_onboard_backlight;
+   bl_props.type = BACKLIGHT_RAW;
+   bl_props.power = FB_BLANK_POWERDOWN;
+
+   bd = backlight_device_register(dev_driver_string(par->info->device),
+               par->info->device, par, bl_ops, &bl_props);
+   if (IS_ERR(bd)) {
+       dev_err(par->info->device,
+           "cannot register backlight device (%ld)\n",
+           PTR_ERR(bd));
+       goto failed;
+   }
+   par->info->bl_dev = bd;
+
+   if (!par->fbtftops.unregister_backlight)
+       par->fbtftops.unregister_backlight = fbtft_unregister_backlight;
+
+   return;
+failed:
+   kfree(bl_ops);
+}
+
+
+
 FBTFT_REGISTER_DRIVER(DRVNAME, &display);

 MODULE_ALIAS("spi:" DRVNAME);
diff --git a/fbtft.h b/fbtft.h
index e0fb851..ca8cb71 100644
--- a/fbtft.h
+++ b/fbtft.h
@@ -33,6 +33,7 @@
 #define FBTFT_RASET        0x2B
 #define FBTFT_RAMWR        0x2C

+#define FBTFT_ONBOARD_BACKLIGHT 2

 #define FBTFT_GPIO_NO_MATCH        0xFFFF
 #define FBTFT_GPIO_NAME_SIZE   32
diff --git a/fbtft_device.c b/fbtft_device.c
index 74714ee..502c585 100644
--- a/fbtft_device.c
+++ b/fbtft_device.c
@@ -242,6 +242,25 @@ static struct fbtft_device_display displays[] = {
            }
        }
    }, {
+       .name = "freetronicsoled128",
+       .spi = &(struct spi_board_info) {
+           .modalias = "fb_ssd1351",
+           .max_speed_hz = 20000000,
+           .mode = SPI_MODE_0,
+           .platform_data = &(struct fbtft_platform_data) {
+               .display = {
+                   .buswidth = 8,
+                   .backlight = FBTFT_ONBOARD_BACKLIGHT,
+               },
+               .bgr = true,
+               .gpios = (const struct fbtft_gpio []) {
+                   { "reset", 24 },
+                   { "dc", 25 },
+                   {},
+               },
+           }
+       }
+   }, {
        .name = "hy28a",
        .spi = &(struct spi_board_info) {
            .modalias = "fb_ili9320",
notro commented 11 years ago

Committed: https://github.com/notro/fbtft/commit/5dc99c11c6487fda7aed13b317505dee259385ac