From: zhangxuezhi <[email protected]>
For st7789v ic,add tearing signal detect to avoid screen tearing
Signed-off-by: zhangxuezhi <[email protected]>
---
v5:fix log print
---
drivers/staging/fbtft/fb_st7789v.c | 128 ++++++++++++++++++++++++++++++++++++-
drivers/staging/fbtft/fbtft.h | 1 +
2 files changed, 128 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/fbtft/fb_st7789v.c b/drivers/staging/fbtft/fb_st7789v.c
index 3a280cc..ab10235 100644
--- a/drivers/staging/fbtft/fb_st7789v.c
+++ b/drivers/staging/fbtft/fb_st7789v.c
@@ -9,9 +9,12 @@
#include <linux/delay.h>
#include <linux/init.h>
#include <linux/kernel.h>
+#include <linux/mutex.h>
+#include <linux/interrupt.h>
+#include <linux/completion.h>
#include <linux/module.h>
#include <video/mipi_display.h>
-
+#include <linux/gpio/consumer.h>
#include "fbtft.h"
#define DRVNAME "fb_st7789v"
@@ -66,6 +69,32 @@ enum st7789v_command {
#define MADCTL_MX BIT(6) /* bitmask for column address order */
#define MADCTL_MY BIT(7) /* bitmask for page address order */
+#define SPI_PANEL_TE_TIMEOUT 400
+static struct mutex te_mutex;/*mutex for tearing line*/
+static struct completion spi_panel_te;
+
+static irqreturn_t spi_panel_te_handler(int irq, void *data)
+{
+ complete(&spi_panel_te);
+ return IRQ_HANDLED;
+}
+
+static void set_spi_panel_te_irq_status(struct fbtft_par *par, bool enable)
+{
+ static int te_irq_count;
+
+ mutex_lock(&te_mutex);
+
+ if (enable) {
+ if (++te_irq_count == 1)
+ enable_irq(gpiod_to_irq(par->gpio.te));
+ } else {
+ if (--te_irq_count == 0)
+ disable_irq(gpiod_to_irq(par->gpio.te));
+ }
+ mutex_unlock(&te_mutex);
+}
+
/**
* init_display() - initialize the display controller
*
@@ -82,6 +111,29 @@ enum st7789v_command {
*/
static int init_display(struct fbtft_par *par)
{
+ int rc;
+ struct device *dev = par->info->device;
+
+ par->gpio.te = devm_gpiod_get_index_optional(dev, "te", 0, GPIOD_IN);
+ if (par->gpio.te) {
+ init_completion(&spi_panel_te);
+ mutex_init(&te_mutex);
+ rc = devm_request_irq(dev,
+ gpiod_to_irq(par->gpio.te),
+ spi_panel_te_handler, IRQF_TRIGGER_RISING,
+ "TE_GPIO", par);
+ if (rc) {
+ pr_err("TE request_irq failed.\n");
+ devm_gpiod_put(dev, par->gpio.te);
+ par->gpio.te = NULL;
+ } else {
+ disable_irq_nosync(gpiod_to_irq(par->gpio.te));
+ pr_info("TE request_irq completion.\n");
+ }
+ } else {
+ pr_info("%s:%d, TE gpio not specified\n",
+ __func__, __LINE__);
+ }
/* turn off sleep mode */
write_reg(par, MIPI_DCS_EXIT_SLEEP_MODE);
mdelay(120);
@@ -137,6 +189,9 @@ static int init_display(struct fbtft_par *par)
*/
write_reg(par, PWCTRL1, 0xA4, 0xA1);
+ /*Tearing Effect Line On*/
+ if (par->gpio.te)
+ write_reg(par, 0x35, 0x00);
write_reg(par, MIPI_DCS_SET_DISPLAY_ON);
if (HSD20_IPS)
@@ -145,6 +200,76 @@ static int init_display(struct fbtft_par *par)
return 0;
}
+/*****************************************************************************
+ *
+ * int (*write_vmem)(struct fbtft_par *par);
+ *
+ *****************************************************************************/
+
+/* 16 bit pixel over 8-bit databus */
+static int st7789v_write_vmem16_bus8(struct fbtft_par *par, size_t offset, size_t len)
+{
+ u16 *vmem16;
+ __be16 *txbuf16 = par->txbuf.buf;
+ size_t remain;
+ size_t to_copy;
+ size_t tx_array_size;
+ int i;
+ int ret = 0;
+ size_t startbyte_size = 0;
+
+ fbtft_par_dbg(DEBUG_WRITE_VMEM, par, "st7789v ---%s(offset=%zu, len=%zu)\n",
+ __func__, offset, len);
+
+ remain = len / 2;
+ vmem16 = (u16 *)(par->info->screen_buffer + offset);
+
+ if (par->gpio.dc)
+ gpiod_set_value(par->gpio.dc, 1);
+
+ /* non buffered write */
+ if (!par->txbuf.buf)
+ return par->fbtftops.write(par, vmem16, len);
+
+ /* buffered write */
+ tx_array_size = par->txbuf.len / 2;
+
+ if (par->startbyte) {
+ txbuf16 = par->txbuf.buf + 1;
+ tx_array_size -= 2;
+ *(u8 *)(par->txbuf.buf) = par->startbyte | 0x2;
+ startbyte_size = 1;
+ }
+
+ while (remain) {
+ to_copy = min(tx_array_size, remain);
+ dev_dbg(par->info->device, " to_copy=%zu, remain=%zu\n",
+ to_copy, remain - to_copy);
+
+ for (i = 0; i < to_copy; i++)
+ txbuf16[i] = cpu_to_be16(vmem16[i]);
+
+ vmem16 = vmem16 + to_copy;
+ if (par->gpio.te) {
+ set_spi_panel_te_irq_status(par, true);
+ reinit_completion(&spi_panel_te);
+ ret = wait_for_completion_timeout(&spi_panel_te,
+ msecs_to_jiffies(SPI_PANEL_TE_TIMEOUT));
+ if (ret == 0)
+ pr_err("wait panel TE time out\n");
+ }
+ ret = par->fbtftops.write(par, par->txbuf.buf,
+ startbyte_size + to_copy * 2);
+ if (par->gpio.te)
+ set_spi_panel_te_irq_status(par, false);
+ if (ret < 0)
+ return ret;
+ remain -= to_copy;
+ }
+
+ return ret;
+}
+
/**
* set_var() - apply LCD properties like rotation and BGR mode
*
@@ -259,6 +384,7 @@ static int blank(struct fbtft_par *par, bool on)
.gamma = HSD20_IPS_GAMMA,
.fbtftops = {
.init_display = init_display,
+ .write_vmem = st7789v_write_vmem16_bus8,
.set_var = set_var,
.set_gamma = set_gamma,
.blank = blank,
diff --git a/drivers/staging/fbtft/fbtft.h b/drivers/staging/fbtft/fbtft.h
index 76f8c09..93bac05 100644
--- a/drivers/staging/fbtft/fbtft.h
+++ b/drivers/staging/fbtft/fbtft.h
@@ -212,6 +212,7 @@ struct fbtft_par {
struct gpio_desc *wr;
struct gpio_desc *latch;
struct gpio_desc *cs;
+ struct gpio_desc *te;
struct gpio_desc *db[16];
struct gpio_desc *led[16];
struct gpio_desc *aux[16];
--
1.9.1
On Wed, Jan 27, 2021 at 09:32:20AM +0800, Carlis wrote:
> @@ -82,6 +111,29 @@ enum st7789v_command {
> */
> static int init_display(struct fbtft_par *par)
> {
> + int rc;
> + struct device *dev = par->info->device;
> +
> + par->gpio.te = devm_gpiod_get_index_optional(dev, "te", 0, GPIOD_IN);
> + if (par->gpio.te) {
> + init_completion(&spi_panel_te);
> + mutex_init(&te_mutex);
> + rc = devm_request_irq(dev,
> + gpiod_to_irq(par->gpio.te),
> + spi_panel_te_handler, IRQF_TRIGGER_RISING,
> + "TE_GPIO", par);
> + if (rc) {
> + pr_err("TE request_irq failed.\n");
> + devm_gpiod_put(dev, par->gpio.te);
> + par->gpio.te = NULL;
> + } else {
> + disable_irq_nosync(gpiod_to_irq(par->gpio.te));
> + pr_info("TE request_irq completion.\n");
> + }
> + } else {
> + pr_info("%s:%d, TE gpio not specified\n",
> + __func__, __LINE__);
> + }
I'm sorry that I was not clear before. This code will crash if
devm_gpiod_get_index_optional() returns an error. You *NEED* to check
for error pointers and return the error code. Write it exactly like
this:
par->gpio.te = devm_gpiod_get_index_optional(dev, "te", 0, GPIOD_IN);
if (IS_ERR(par->gpio.te))
return PTR_ERR(par->gpio.te);
if (par->gpio.te) {
init_completion(&spi_panel_te);
regards,
dan carpenter
On Wed, 27 Jan 2021 08:45:23 +0300
Dan Carpenter <[email protected]> wrote:
> On Wed, Jan 27, 2021 at 09:32:20AM +0800, Carlis wrote:
> > @@ -82,6 +111,29 @@ enum st7789v_command {
> > */
> > static int init_display(struct fbtft_par *par)
> > {
> > + int rc;
> > + struct device *dev = par->info->device;
> > +
> > + par->gpio.te = devm_gpiod_get_index_optional(dev, "te", 0,
> > GPIOD_IN);
> > + if (par->gpio.te) {
> > + init_completion(&spi_panel_te);
> > + mutex_init(&te_mutex);
> > + rc = devm_request_irq(dev,
> > + gpiod_to_irq(par->gpio.te),
> > + spi_panel_te_handler,
> > IRQF_TRIGGER_RISING,
> > + "TE_GPIO", par);
> > + if (rc) {
> > + pr_err("TE request_irq failed.\n");
> > + devm_gpiod_put(dev, par->gpio.te);
> > + par->gpio.te = NULL;
> > + } else {
> > +
> > disable_irq_nosync(gpiod_to_irq(par->gpio.te));
> > + pr_info("TE request_irq completion.\n");
> > + }
> > + } else {
> > + pr_info("%s:%d, TE gpio not specified\n",
> > + __func__, __LINE__);
> > + }
>
> I'm sorry that I was not clear before. This code will crash if
> devm_gpiod_get_index_optional() returns an error. You *NEED* to check
> for error pointers and return the error code. Write it exactly like
> this:
>
> par->gpio.te = devm_gpiod_get_index_optional(dev, "te", 0,
> GPIOD_IN); if (IS_ERR(par->gpio.te))
> return PTR_ERR(par->gpio.te);
>
> if (par->gpio.te) {
> init_completion(&spi_panel_te);
>
>
> regards,
> dan carpenter
>
hi,i will fix it like below:
par->gpio.te = devm_gpiod_get_index_optional(dev, "te", 0,
GPIOD_IN); if (IS_ERR(par->gpio.te)) {
rc = PTR_ERR(par->gpio.te);
pr_err("Failed to request te gpio: %d\n", rc);
par->gpio.te = NULL;
}
if (par->gpio.te) {
init_completion(&spi_panel_te);
mutex_init(&te_mutex);
rc = devm_request_irq(dev,
gpiod_to_irq(par->gpio.te),
spi_panel_te_handler,
IRQF_TRIGGER_RISING, "TE_GPIO", par);
if (rc) {
pr_err("TE request_irq failed.\n");
devm_gpiod_put(dev, par->gpio.te);
par->gpio.te = NULL;
} else {
disable_irq_nosync(gpiod_to_irq(par->gpio.te));
pr_info("TE request_irq completion.\n");
}
} else {
pr_info("%s:%d, TE gpio not specified\n",
__func__, __LINE__);
}
regards,
zhangxuezhi
On Wed, Jan 27, 2021 at 09:32:20AM +0800, Carlis wrote:
> @@ -82,6 +111,29 @@ enum st7789v_command {
> */
> static int init_display(struct fbtft_par *par)
> {
> + int rc;
> + struct device *dev = par->info->device;
> +
> + par->gpio.te = devm_gpiod_get_index_optional(dev, "te", 0, GPIOD_IN);
> + if (par->gpio.te) {
> + init_completion(&spi_panel_te);
> + mutex_init(&te_mutex);
> + rc = devm_request_irq(dev,
> + gpiod_to_irq(par->gpio.te),
> + spi_panel_te_handler, IRQF_TRIGGER_RISING,
> + "TE_GPIO", par);
> + if (rc) {
> + pr_err("TE request_irq failed.\n");
> + devm_gpiod_put(dev, par->gpio.te);
> + par->gpio.te = NULL;
> + } else {
> + disable_irq_nosync(gpiod_to_irq(par->gpio.te));
> + pr_info("TE request_irq completion.\n");
This printk adds no value.
Don't print anything on the success path. You should add that to your
code while your debugging the feature, but don't push it to the upstream
kernel.
> + }
> + } else {
> + pr_info("%s:%d, TE gpio not specified\n",
> + __func__, __LINE__);
> + }
regards,
dan carpenter
On Wed, Jan 27, 2021 at 02:19:27PM +0800, carlis wrote:
> hi,i will fix it like below:
> par->gpio.te = devm_gpiod_get_index_optional(dev, "te", 0,
> GPIOD_IN); if (IS_ERR(par->gpio.te)) {
> rc = PTR_ERR(par->gpio.te);
> pr_err("Failed to request te gpio: %d\n", rc);
> par->gpio.te = NULL;
> }
I wish you would just copy and paste the code that I sent you instead,
but this also fixes the crash...
regards,
dan carpenter
Hi Dan, Carlis,
On Wed, Jan 27, 2021 at 9:32 AM Dan Carpenter <[email protected]> wrote:
> On Wed, Jan 27, 2021 at 02:19:27PM +0800, carlis wrote:
> > hi,i will fix it like below:
> > par->gpio.te = devm_gpiod_get_index_optional(dev, "te", 0,
> > GPIOD_IN); if (IS_ERR(par->gpio.te)) {
> > rc = PTR_ERR(par->gpio.te);
> > pr_err("Failed to request te gpio: %d\n", rc);
> > par->gpio.te = NULL;
> > }
>
> I wish you would just copy and paste the code that I sent you instead,
> but this also fixes the crash...
While this fixes the crash, it does not propagate the error condition
(which may be -EPROBE_DEFER) upstream.
Same for devm_request_irq().
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds