[PATCH v14] staging: fbtft: add tearing signal detect
From: Carlis
For st7789v IC, when we need continuous full screen refresh, it is best to
wait for the tearing effect line signal to arrive to avoid screen tearing.
Signed-off-by: Carlis
---
v14: change to define TE completion and TE irq only in st7789v.
v13: change TE completion to par data struct member and add a new
function to deal te gpio request, add new write_vmem function.
v12: change dev_err to dev_err_probe and add space in comments start, and
delete te_mutex, change te wait logic.
v11: remove devm_gpio_put and change a dev_err to dev_info.
v10: additional notes.
v9: change pr_* to dev_*.
v8: delete a log line.
v7: return error value when request fail.
v6: add te gpio request fail deal logic.
v5: fix log print.
v4: modify some code style and change te irq set function name.
v3: modify author and signed-off-by name.
v2: add release te gpio after irq request fail.
---
drivers/staging/fbtft/fb_st7789v.c | 115 +
1 file changed, 115 insertions(+)
diff --git a/drivers/staging/fbtft/fb_st7789v.c
b/drivers/staging/fbtft/fb_st7789v.c
index 3a280cc..ee8866d 100644
--- a/drivers/staging/fbtft/fb_st7789v.c
+++ b/drivers/staging/fbtft/fb_st7789v.c
@@ -7,9 +7,13 @@
#include
#include
+#include
#include
#include
+#include
+#include
#include
+
#include
#include "fbtft.h"
@@ -66,6 +70,62 @@ enum st7789v_command {
#define MADCTL_MX BIT(6) /* bitmask for column address order */
#define MADCTL_MY BIT(7) /* bitmask for page address order */
+/* 60Hz for 16.6ms, configured as 2*16.6ms */
+#define PANEL_TE_TIMEOUT_MS 33
+
+static struct completion panel_te; /* completion for panel TE line */
+static int irq_te; /* Linux IRQ for LCD TE line */
+
+static irqreturn_t panel_te_handler(int irq, void *data)
+{
+ complete(&panel_te);
+ return IRQ_HANDLED;
+}
+
+/*
+ * init_tearing_effect_line() - init tearing effect line.
+ * @par: FBTFT parameter object.
+ *
+ * Return: 0 on success, or a negative error code otherwise.
+ */
+static int init_tearing_effect_line(struct fbtft_par *par)
+{
+ struct device *dev = par->info->device;
+ struct gpio_desc *te;
+ int rc, irq;
+
+ te = gpiod_get_optional(dev, "te", GPIOD_IN);
+ if (IS_ERR(te))
+ return dev_err_probe(dev, PTR_ERR(te), "Failed to request te
GPIO\n");
+
+ /* if te is NULL, indicating no configuration, directly return success
*/
+ if (!te) {
+ irq_te = 0;
+ return 0;
+ }
+
+ irq = gpiod_to_irq(te);
+
+ /* GPIO is locked as an IRQ, we may drop the reference */
+ gpiod_put(te);
+
+ if (irq < 0)
+ return irq;
+
+ irq_te = irq;
+ init_completion(&panel_te);
+
+ /* The effective state is high and lasts no more than 1000 microseconds
*/
+ rc = devm_request_irq(dev, irq_te, panel_te_handler,
+ IRQF_TRIGGER_RISING, "TE_GPIO", par);
+ if (rc)
+ return dev_err_probe(dev, rc, "TE IRQ request failed.\n");
+
+ disable_irq_nosync(irq_te);
+
+ return 0;
+}
+
/**
* init_display() - initialize the display controller
*
@@ -82,6 +142,12 @@ enum st7789v_command {
*/
static int init_display(struct fbtft_par *par)
{
+ int rc;
+
+ rc = init_tearing_effect_line(par);
+ if (rc)
+ return rc;
+
/* turn off sleep mode */
write_reg(par, MIPI_DCS_EXIT_SLEEP_MODE);
mdelay(120);
@@ -137,6 +203,10 @@ static int init_display(struct fbtft_par *par)
*/
write_reg(par, PWCTRL1, 0xA4, 0xA1);
+ /* TE line output is off by default when powering on */
+ if (irq_te)
+ write_reg(par, MIPI_DCS_SET_TEAR_ON, 0x00);
+
write_reg(par, MIPI_DCS_SET_DISPLAY_ON);
if (HSD20_IPS)
@@ -145,6 +215,50 @@ static int init_display(struct fbtft_par *par)
return 0;
}
+/*
+ * write_vmem() - write data to display.
+ * @par: FBTFT parameter object.
+ * @offset: offset from screen_buffer.
+ * @len: the length of data to be writte.
+ *
+ * Return: 0 on success, or a negative error code otherwise.
+ */
+static int write_vmem(struct fbtft_par *par, size_t offset, size_t len)
+{
+ struct device *dev = par->info->device;
+ int ret;
+
+ if (irq_te) {
+ enable_irq(irq_te);
+ reinit_completion(&panel_te);
+ ret = wait_for_completion_timeout(&panel_te,
+
msecs_to_jiffies(PANEL_TE_TIMEOUT_MS));
+ if (ret == 0)
+ dev_err(dev, "wait panel TE timeout\n");
+
+ disable_irq(irq_te);
+ }
+
+ ret = 0;
+ switch (par->pdata->display.buswidth) {
+ case 8:
+ ret = fbtft_write_vmem16_bus8(par, offset, len);
+ break;
+
[PATCH v15] staging: fbtft: add tearing signal detect
From: Carlis
For st7789v IC, when we need continuous full screen refresh, it is best to
wait for the tearing effect line signal to arrive to avoid screen tearing.
Signed-off-by: Carlis
---
v15: change ret value return logic in write_vmem.
v14: change to define TE completion and TE irq only in st7789v.
v13: change TE completion to par data struct member and add a new
function to deal te gpio request, add new write_vmem function.
v12: change dev_err to dev_err_probe and add space in comments start, and
delete te_mutex, change te wait logic.
v11: remove devm_gpio_put and change a dev_err to dev_info.
v10: additional notes.
v9: change pr_* to dev_*.
v8: delete a log line.
v7: return error value when request fail.
v6: add te gpio request fail deal logic.
v5: fix log print.
v4: modify some code style and change te irq set function name.
v3: modify author and signed-off-by name.
v2: add release te gpio after irq request fail.
---
drivers/staging/fbtft/fb_st7789v.c | 115 +
1 file changed, 115 insertions(+)
diff --git a/drivers/staging/fbtft/fb_st7789v.c
b/drivers/staging/fbtft/fb_st7789v.c
index 3a280cc..abe9395 100644
--- a/drivers/staging/fbtft/fb_st7789v.c
+++ b/drivers/staging/fbtft/fb_st7789v.c
@@ -7,9 +7,13 @@
#include
#include
+#include
#include
#include
+#include
+#include
#include
+
#include
#include "fbtft.h"
@@ -66,6 +70,62 @@ enum st7789v_command {
#define MADCTL_MX BIT(6) /* bitmask for column address order */
#define MADCTL_MY BIT(7) /* bitmask for page address order */
+/* 60Hz for 16.6ms, configured as 2*16.6ms */
+#define PANEL_TE_TIMEOUT_MS 33
+
+static struct completion panel_te; /* completion for panel TE line */
+static int irq_te; /* Linux IRQ for LCD TE line */
+
+static irqreturn_t panel_te_handler(int irq, void *data)
+{
+ complete(&panel_te);
+ return IRQ_HANDLED;
+}
+
+/*
+ * init_tearing_effect_line() - init tearing effect line.
+ * @par: FBTFT parameter object.
+ *
+ * Return: 0 on success, or a negative error code otherwise.
+ */
+static int init_tearing_effect_line(struct fbtft_par *par)
+{
+ struct device *dev = par->info->device;
+ struct gpio_desc *te;
+ int rc, irq;
+
+ te = gpiod_get_optional(dev, "te", GPIOD_IN);
+ if (IS_ERR(te))
+ return dev_err_probe(dev, PTR_ERR(te), "Failed to request te
GPIO\n");
+
+ /* if te is NULL, indicating no configuration, directly return success
*/
+ if (!te) {
+ irq_te = 0;
+ return 0;
+ }
+
+ irq = gpiod_to_irq(te);
+
+ /* GPIO is locked as an IRQ, we may drop the reference */
+ gpiod_put(te);
+
+ if (irq < 0)
+ return irq;
+
+ irq_te = irq;
+ init_completion(&panel_te);
+
+ /* The effective state is high and lasts no more than 1000 microseconds
*/
+ rc = devm_request_irq(dev, irq_te, panel_te_handler,
+ IRQF_TRIGGER_RISING, "TE_GPIO", par);
+ if (rc)
+ return dev_err_probe(dev, rc, "TE IRQ request failed.\n");
+
+ disable_irq_nosync(irq_te);
+
+ return 0;
+}
+
/**
* init_display() - initialize the display controller
*
@@ -82,6 +142,12 @@ enum st7789v_command {
*/
static int init_display(struct fbtft_par *par)
{
+ int rc;
+
+ rc = init_tearing_effect_line(par);
+ if (rc)
+ return rc;
+
/* turn off sleep mode */
write_reg(par, MIPI_DCS_EXIT_SLEEP_MODE);
mdelay(120);
@@ -137,6 +203,10 @@ static int init_display(struct fbtft_par *par)
*/
write_reg(par, PWCTRL1, 0xA4, 0xA1);
+ /* TE line output is off by default when powering on */
+ if (irq_te)
+ write_reg(par, MIPI_DCS_SET_TEAR_ON, 0x00);
+
write_reg(par, MIPI_DCS_SET_DISPLAY_ON);
if (HSD20_IPS)
@@ -145,6 +215,50 @@ static int init_display(struct fbtft_par *par)
return 0;
}
+/*
+ * write_vmem() - write data to display.
+ * @par: FBTFT parameter object.
+ * @offset: offset from screen_buffer.
+ * @len: the length of data to be writte.
+ *
+ * Return: 0 on success, or a negative error code otherwise.
+ */
+static int write_vmem(struct fbtft_par *par, size_t offset, size_t len)
+{
+ struct device *dev = par->info->device;
+ int ret;
+
+ if (irq_te) {
+ enable_irq(irq_te);
+ reinit_completion(&panel_te);
+ ret = wait_for_completion_timeout(&panel_te,
+
msecs_to_jiffies(PANEL_TE_TIMEOUT_MS));
+ if (ret == 0)
+ dev_err(dev, "wait panel TE timeout\n");
+
+ disable_irq(irq_te);
+ }
+
+ switch (par->pdata->display.buswidth) {
+ case 8:
+ ret = fbtft_write_vmem16_bus8(par, offset, len);
[PATCH] staging: fbtft: change snprintf() to scnprintf()
From: Xuezhi Zhang show() must not use snprintf() when formatting the value to be returned to user space. Signed-off-by: Xuezhi Zhang --- drivers/staging/fbtft/fbtft-sysfs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/fbtft/fbtft-sysfs.c b/drivers/staging/fbtft/fbtft-sysfs.c index 26e52cc2de64..7df92db648d6 100644 --- a/drivers/staging/fbtft/fbtft-sysfs.c +++ b/drivers/staging/fbtft/fbtft-sysfs.c @@ -199,7 +199,7 @@ static ssize_t show_debug(struct device *device, struct fb_info *fb_info = dev_get_drvdata(device); struct fbtft_par *par = fb_info->par; - return snprintf(buf, PAGE_SIZE, "%lu\n", par->debug); + return scnprintf(buf, PAGE_SIZE, "%lu\n", par->debug); } static struct device_attribute debug_device_attr = -- 2.25.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: fbtft: convert sysfs snprintf to sysfs_emit
From: Xuezhi Zhang Fix the following coccicheck warning: drivers/staging/fbtft//fbtft-sysfs.c:202:8-16: WARNING: use scnprintf or sprintf Signed-off-by: Xuezhi Zhang --- drivers/staging/fbtft/fbtft-sysfs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/fbtft/fbtft-sysfs.c b/drivers/staging/fbtft/fbtft-sysfs.c index 26e52cc2d..39e8d2806 100644 --- a/drivers/staging/fbtft/fbtft-sysfs.c +++ b/drivers/staging/fbtft/fbtft-sysfs.c @@ -199,7 +199,7 @@ static ssize_t show_debug(struct device *device, struct fb_info *fb_info = dev_get_drvdata(device); struct fbtft_par *par = fb_info->par; - return snprintf(buf, PAGE_SIZE, "%lu\n", par->debug); + return sysfs_emit(buf, "%lu\n", par->debug); } static struct device_attribute debug_device_attr = -- 2.25.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] fbtft: add tearing signal detect
From: "carlis.zhang_cp"
For st7789v ic,add tearing signal detect to avoid screen tearing
Signed-off-by: carlis.zhang_cp
---
drivers/staging/fbtft/fb_st7789v.c | 133 -
drivers/staging/fbtft/fbtft.h | 1 +
2 files changed, 133 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/fbtft/fb_st7789v.c
b/drivers/staging/fbtft/fb_st7789v.c
index 3a280cc..c687b58 100644
--- a/drivers/staging/fbtft/fb_st7789v.c
+++ b/drivers/staging/fbtft/fb_st7789v.c
@@ -9,9 +9,12 @@
#include
#include
#include
+#include
+#include
+#include
#include
#include
-
+#include
#include "fbtft.h"
#define DRVNAME "fb_st7789v"
@@ -66,6 +69,38 @@ 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 enable_spi_panel_te_irq(struct fbtft_par *par, bool enable)
+{
+ static int te_irq_count;
+
+ if (!par->gpio.te) {
+ pr_err("%s:%d,SPI panel TE GPIO not configured\n",
+ __func__, __LINE__);
+ return;
+ }
+
+ 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 +117,28 @@ 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");
+ par->gpio.te = NULL;
+ } else {
+ disable_irq_nosync(gpiod_to_irq(par->gpio.te));
+ pr_err("TE request_irq completion.\n");
+ }
+ } else {
+ pr_err("%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 +194,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 +205,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 */
+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 rc, 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-
[PATCH v2] fbtft: add tearing signal detect
From: "carlis.zhang_cp"
For st7789v ic,add tearing signal detect to avoid screen tearing
Signed-off-by: carlis.zhang_cp
---
v2:add release te gpio after irq request fail
---
drivers/staging/fbtft/fb_st7789v.c | 134 -
drivers/staging/fbtft/fbtft.h | 1 +
2 files changed, 134 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/fbtft/fb_st7789v.c
b/drivers/staging/fbtft/fb_st7789v.c
index 3a280cc..5426276 100644
--- a/drivers/staging/fbtft/fb_st7789v.c
+++ b/drivers/staging/fbtft/fb_st7789v.c
@@ -9,9 +9,12 @@
#include
#include
#include
+#include
+#include
+#include
#include
#include
-
+#include
#include "fbtft.h"
#define DRVNAME "fb_st7789v"
@@ -66,6 +69,38 @@ 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 enable_spi_panel_te_irq(struct fbtft_par *par, bool enable)
+{
+ static int te_irq_count;
+
+ if (!par->gpio.te) {
+ pr_err("%s:%d,SPI panel TE GPIO not configured\n",
+ __func__, __LINE__);
+ return;
+ }
+
+ 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 +117,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_err("%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 +195,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 +206,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 */
+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 rc, 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++)
+
[PATCH v3] fbtft: add tearing signal detect
From: zhangxuezhi
For st7789v ic,add tearing signal detect to avoid screen tearing
Signed-off-by: zhangxuezhi
---
v3:modify author name
---
drivers/staging/fbtft/fb_st7789v.c | 134 -
drivers/staging/fbtft/fbtft.h | 1 +
2 files changed, 134 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/fbtft/fb_st7789v.c
b/drivers/staging/fbtft/fb_st7789v.c
index 3a280cc..5426276 100644
--- a/drivers/staging/fbtft/fb_st7789v.c
+++ b/drivers/staging/fbtft/fb_st7789v.c
@@ -9,9 +9,12 @@
#include
#include
#include
+#include
+#include
+#include
#include
#include
-
+#include
#include "fbtft.h"
#define DRVNAME "fb_st7789v"
@@ -66,6 +69,38 @@ 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 enable_spi_panel_te_irq(struct fbtft_par *par, bool enable)
+{
+ static int te_irq_count;
+
+ if (!par->gpio.te) {
+ pr_err("%s:%d,SPI panel TE GPIO not configured\n",
+ __func__, __LINE__);
+ return;
+ }
+
+ 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 +117,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_err("%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 +195,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 +206,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 */
+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 rc, 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(vmem1
[PATCH v4] fbtft: add tearing signal detect
From: zhangxuezhi
For st7789v ic,add tearing signal detect to avoid screen tearing
Signed-off-by: zhangxuezhi
---
v4:modify some code style and change te irq set function name
---
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..11a490de 100644
--- a/drivers/staging/fbtft/fb_st7789v.c
+++ b/drivers/staging/fbtft/fb_st7789v.c
@@ -9,9 +9,12 @@
#include
#include
#include
+#include
+#include
+#include
#include
#include
-
+#include
#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_err("%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 */
+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_st
Re: [PATCH] fbtft: add tearing signal detect
On Tue, 26 Jan 2021 10:54:41 +0300
Dan Carpenter wrote:
> On Sun, Jan 24, 2021 at 11:35:37PM +0800, Carlis wrote:
> > +static irqreturn_t spi_panel_te_handler(int irq, void *data)
> > +{
> > + complete(&spi_panel_te);
> > + return IRQ_HANDLED;
> > +}
> > +
> > +static void enable_spi_panel_te_irq(struct fbtft_par *par, bool
> > enable)
>
> It quite confused me that enable actually disables. I always feel
> like it's clearer to write these as two separate functions.
>
> > +{
> > + static int te_irq_count;
> > +
> > + if (!par->gpio.te) {
>
> This is always checked in the caller. And it's when it's NULL that
> means it's deliberate so don't print a message.
>
> > + pr_err("%s:%d,SPI panel TE GPIO not configured\n",
> > + __func__, __LINE__);
> > + return;
> > + }
> > +
> > + 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 +117,28 @@ 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) {
>
> devm_gpiod_get_index_optional() can return NULL or error pointers. If
> it returns NULL then don't print an error message. NULL reports are
> deliberate.
>
> par->gpio.te = devm_gpiod_get_index_optional(dev, "te", 0,
> GPIOD_IN); if (IS_ERR(par->gpio.te)) {
> pr_err("%s:%d, TE gpio not specified\n", __func__,
> __LINE__); return PTR_ERR(par->gpio.te);
> }
>
> 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");
> > + par->gpio.te = NULL;
> > + } else {
> > +
> > disable_irq_nosync(gpiod_to_irq(par->gpio.te));
> > + pr_err("TE request_irq completion.\n");
>
> Why is this printing an error message if devm_request_irq() succeeds?
>
> > + }
> > + } else {
> > + pr_err("%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 +194,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 +205,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 */
> > +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 rc, ret = 0;
>
> Delete one of these "rc" or "rec" variables.
>
> > + size_t startbyte_size = 0;
> > +
> > + fbtft_par_dbg(DEBUG_WRITE_VMEM, par, "st7789v
> > ---%s(offset=%zu, len=%zu)\n",
> > +
Re: [PATCH v2] fbtft: add tearing signal detect
On Tue, 26 Jan 2021 11:17:45 +0300 Dan Carpenter wrote: > On Mon, Jan 25, 2021 at 04:44:12PM +0800, Carlis wrote: > > From: "carlis.zhang_cp" > > I was really expecting that you would fix this and Signed-off-by as > well. > > regards, > dan carpenter > I have fix this in patch v3 > regards, > zhangxuezhi ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v4] fbtft: add tearing signal detect
On Tue, 26 Jan 2021 20:51:41 +0300
Dan Carpenter wrote:
> On Tue, Jan 26, 2021 at 08:40:35PM +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) {
>
> I explained in my earlier review that devm_gpiod_get_index_optional()
> can return error pointers... There was quite a bit of detail about
> how to handle this correctly in my earlier review, but I think you
> might not have noticed it. Please read it again.
>
> > + 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_err("%s:%d, TE gpio not specified\n",
> > + __func__, __LINE__);
> > + }
>
> regards,
> dan carpenter
>
Thank you for your correction,i will change pr_err to pr_info in next
patch v5
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v5] fbtft: add tearing signal detect
From: zhangxuezhi
For st7789v ic,add tearing signal detect to avoid screen tearing
Signed-off-by: zhangxuezhi
---
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
#include
#include
+#include
+#include
+#include
#include
#include
-
+#include
#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);
+
Re: [PATCH v5] fbtft: add tearing signal detect
On Wed, 27 Jan 2021 08:45:23 +0300
Dan Carpenter 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
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v6] fbtft: add tearing signal detect
From: zhangxuezhi
For st7789v ic,add tearing signal detect to avoid screen tearing
Signed-off-by: zhangxuezhi
---
v6: add te gpio request fail deal logic
---
drivers/staging/fbtft/fb_st7789v.c | 133 -
drivers/staging/fbtft/fbtft.h | 1 +
2 files changed, 133 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/fbtft/fb_st7789v.c
b/drivers/staging/fbtft/fb_st7789v.c
index 3a280cc..777391e 100644
--- a/drivers/staging/fbtft/fb_st7789v.c
+++ b/drivers/staging/fbtft/fb_st7789v.c
@@ -9,9 +9,12 @@
#include
#include
#include
+#include
+#include
+#include
#include
#include
-
+#include
#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,34 @@ 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 (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__);
+ }
/* turn off sleep mode */
write_reg(par, MIPI_DCS_EXIT_SLEEP_MODE);
mdelay(120);
@@ -137,6 +194,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 +205,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++)
+
Re: [PATCH v6] fbtft: add tearing signal detect
On Wed, 27 Jan 2021 10:00:13 +0100
Geert Uytterhoeven wrote:
> Hi Carlis,
>
> On Wed, Jan 27, 2021 at 9:52 AM Carlis wrote:
> > From: zhangxuezhi
> >
> > For st7789v ic,add tearing signal detect to avoid screen tearing
> >
> > Signed-off-by: zhangxuezhi
>
> Thanks for your patch!
>
> > --- a/drivers/staging/fbtft/fb_st7789v.c
> > +++ b/drivers/staging/fbtft/fb_st7789v.c
>
> > @@ -82,6 +111,34 @@ 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 (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;
>
> Errors (e.g. -EPROBE_DEFER) should be propagated upstream,
> not ignored.
>
> > + }
> > + 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;
>
> Errors (e.g. -EPROBE_DEFER) should be propagated upstream,
> not ignored.
>
> > + } 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);
>
> Gr{oetje,eeting}s,
>
> Geert
>
hi,i will fix in the patch v7
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v7] fbtft: add tearing signal detect
From: zhangxuezhi
For st7789v ic,add tearing signal detect to avoid screen tearing
Signed-off-by: zhangxuezhi
---
v7: request fail to return error value
---
drivers/staging/fbtft/fb_st7789v.c | 133 -
drivers/staging/fbtft/fbtft.h | 1 +
2 files changed, 133 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/fbtft/fb_st7789v.c
b/drivers/staging/fbtft/fb_st7789v.c
index 3a280cc..c964cc1 100644
--- a/drivers/staging/fbtft/fb_st7789v.c
+++ b/drivers/staging/fbtft/fb_st7789v.c
@@ -9,9 +9,12 @@
#include
#include
#include
+#include
+#include
+#include
#include
#include
-
+#include
#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,34 @@ 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 (IS_ERR(par->gpio.te)) {
+ rc = PTR_ERR(par->gpio.te);
+ pr_err("Failed to request te gpio: %d\n", rc);
+ return rc;
+ }
+ 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);
+ return rc;
+ }
+
+ 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 +194,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 +205,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]);
+
+
Re: [PATCH v6] fbtft: add tearing signal detect
On Wed, 27 Jan 2021 11:59:51 +0300
Dan Carpenter wrote:
> On Wed, Jan 27, 2021 at 03:28:22PM +0800, Carlis wrote:
> > 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 (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");
>
> #SadFaceEmoji
>
> > + }
> > + } else {
> > + pr_info("%s:%d, TE gpio not specified\n",
> > + __func__, __LINE__);
> > + }
>
> regards,
> dan carpenter
>
Sorry,i will delete this log in patch v8
regards
zhangxuezhi
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v8] fbtft: add tearing signal detect
From: zhangxuezhi
For st7789v ic,add tearing signal detect to avoid screen tearing
Signed-off-by: zhangxuezhi
---
v8: delete a log line
---
drivers/staging/fbtft/fb_st7789v.c | 132 -
drivers/staging/fbtft/fbtft.h | 1 +
2 files changed, 132 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/fbtft/fb_st7789v.c
b/drivers/staging/fbtft/fb_st7789v.c
index 3a280cc..de7460c 100644
--- a/drivers/staging/fbtft/fb_st7789v.c
+++ b/drivers/staging/fbtft/fb_st7789v.c
@@ -9,9 +9,12 @@
#include
#include
#include
+#include
+#include
+#include
#include
#include
-
+#include
#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,33 @@ 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 (IS_ERR(par->gpio.te)) {
+ rc = PTR_ERR(par->gpio.te);
+ pr_err("Failed to request te gpio: %d\n", rc);
+ return rc;
+ }
+ 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);
+ return rc;
+ }
+
+ disable_irq_nosync(gpiod_to_irq(par->gpio.te));
+ } 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 +193,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 +204,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) {
+
[PATCH v9] staging: fbtft: add tearing signal detect
From: zhangxuezhi
For st7789v ic,add tearing signal detect to avoid screen tearing
Signed-off-by: zhangxuezhi
---
v9: change pr_* to dev_*
---
drivers/staging/fbtft/fb_st7789v.c | 132 -
drivers/staging/fbtft/fbtft.h | 1 +
2 files changed, 132 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/fbtft/fb_st7789v.c
b/drivers/staging/fbtft/fb_st7789v.c
index 3a280cc..9aa2f36 100644
--- a/drivers/staging/fbtft/fb_st7789v.c
+++ b/drivers/staging/fbtft/fb_st7789v.c
@@ -9,9 +9,12 @@
#include
#include
#include
+#include
+#include
+#include
#include
#include
-
+#include
#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,33 @@ 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 (IS_ERR(par->gpio.te)) {
+ rc = PTR_ERR(par->gpio.te);
+ dev_err(par->info->device, "Failed to request te gpio: %d\n",
rc);
+ return rc;
+ }
+ 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) {
+ dev_err(par->info->device, "TE request_irq failed.\n");
+ devm_gpiod_put(dev, par->gpio.te);
+ return rc;
+ }
+
+ disable_irq_nosync(gpiod_to_irq(par->gpio.te));
+ } else {
+ dev_info(par->info->device, "%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 +193,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 +204,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]);
+
+ vm
[PATCH v10] staging: fbtft: add tearing signal detect
From: zhangxuezhi
For st7789v ic,when we need continuous full screen refresh, it is best to
wait for the TE signal arrive to avoid screen tearing
Signed-off-by: zhangxuezhi
---
v10: additional notes
v9: change pr_* to dev_*
v8: delete a log line
v7: return error value when request fail
v6: add te gpio request fail deal logic
v5: fix log print
v4: modify some code style and change te irq set function name
v3: modify author and signed-off-by name
v2: add release te gpio after irq request fail
---
drivers/staging/fbtft/fb_st7789v.c | 132 -
drivers/staging/fbtft/fbtft.h | 1 +
2 files changed, 132 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/fbtft/fb_st7789v.c
b/drivers/staging/fbtft/fb_st7789v.c
index 3a280cc..cba08a8 100644
--- a/drivers/staging/fbtft/fb_st7789v.c
+++ b/drivers/staging/fbtft/fb_st7789v.c
@@ -9,9 +9,12 @@
#include
#include
#include
+#include
+#include
+#include
#include
#include
-
+#include
#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 /* msecs */
+static struct mutex te_mutex;/* mutex for set te gpio irq status */
+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,33 @@ 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 (IS_ERR(par->gpio.te)) {
+ rc = PTR_ERR(par->gpio.te);
+ dev_err(par->info->device, "Failed to request te gpio: %d\n",
rc);
+ return rc;
+ }
+ 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) {
+ dev_err(par->info->device, "TE request_irq failed.\n");
+ devm_gpiod_put(dev, par->gpio.te);
+ return rc;
+ }
+
+ disable_irq_nosync(gpiod_to_irq(par->gpio.te));
+ } else {
+ dev_info(par->info->device, "%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 +193,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 +204,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;
+
Re: [PATCH v9] staging: fbtft: add tearing signal detect
On Wed, 27 Jan 2021 14:11:31 +0100
Greg KH wrote:
> On Wed, Jan 27, 2021 at 08:57:37PM +0800, Carlis wrote:
> > From: zhangxuezhi
> >
> > For st7789v ic,add tearing signal detect to avoid screen tearing
>
> I need a much better changelog description here please.
>
> >
> > Signed-off-by: zhangxuezhi
> > ---
> > v9: change pr_* to dev_*
> > ---
>
> What changed in all of your previous versions? All of them should be
> listed here, right?
>
>
>
> > drivers/staging/fbtft/fb_st7789v.c | 132
> > - drivers/staging/fbtft/fbtft.h
> > | 1 + 2 files changed, 132 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/staging/fbtft/fb_st7789v.c
> > b/drivers/staging/fbtft/fb_st7789v.c index 3a280cc..9aa2f36 100644
> > --- a/drivers/staging/fbtft/fb_st7789v.c
> > +++ b/drivers/staging/fbtft/fb_st7789v.c
> > @@ -9,9 +9,12 @@
> > #include
> > #include
> > #include
> > +#include
> > +#include
> > +#include
> > #include
> > #include
> > -
> > +#include
> > #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
>
> What is the units here? Where did this value come from?
hi,the units is msecs,and i got this value from a qcom mdp spi
drivers,and i will add the notes you need in the patch v10
>
> > +static struct mutex te_mutex;/*mutex for tearing line*/
>
> Does that look correct???
>
> thanks,
>
> greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v11] staging: fbtft: add tearing signal detect
From: zhangxuezhi
For st7789v ic,when we need continuous full screen refresh, it is best to
wait for the TE signal arrive to avoid screen tearing
Signed-off-by: zhangxuezhi
---
v11: remove devm_gpio_put and change a dev_err to dev_info
v10: additional notes
v9: change pr_* to dev_*
v8: delete a log line
v7: return error value when request fail
v6: add te gpio request fail deal logic
v5: fix log print
v4: modify some code style and change te irq set function name
v3: modify author and signed-off-by name
v2: add release te gpio after irq request fail
---
drivers/staging/fbtft/fb_st7789v.c | 131 -
drivers/staging/fbtft/fbtft.h | 1 +
2 files changed, 131 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/fbtft/fb_st7789v.c
b/drivers/staging/fbtft/fb_st7789v.c
index 3a280cc..cfb54a4 100644
--- a/drivers/staging/fbtft/fb_st7789v.c
+++ b/drivers/staging/fbtft/fb_st7789v.c
@@ -9,9 +9,12 @@
#include
#include
#include
+#include
+#include
+#include
#include
#include
-
+#include
#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 /* msecs */
+static struct mutex te_mutex;/* mutex for set te gpio irq status */
+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,32 @@ 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 (IS_ERR(par->gpio.te)) {
+ rc = PTR_ERR(par->gpio.te);
+ dev_info(par->info->device, "Failed to request te gpio: %d\n",
rc);
+ return rc;
+ }
+ 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) {
+ dev_err(par->info->device, "TE request_irq failed.\n");
+ return rc;
+ }
+
+ disable_irq_nosync(gpiod_to_irq(par->gpio.te));
+ } else {
+ dev_info(par->info->device, "%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 +192,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 +203,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;
+
Re: [PATCH v9] staging: fbtft: add tearing signal detect
On Wed, 27 Jan 2021 14:47:04 +0100
Geert Uytterhoeven wrote:
> Hi Carlis,
>
> On Wed, Jan 27, 2021 at 2:03 PM Carlis wrote:
> > From: zhangxuezhi
> >
> > For st7789v ic,add tearing signal detect to avoid screen tearing
> >
> > Signed-off-by: zhangxuezhi
> > ---
> > v9: change pr_* to dev_*
>
> Thanks for the update!
>
> > --- a/drivers/staging/fbtft/fb_st7789v.c
> > +++ b/drivers/staging/fbtft/fb_st7789v.c
> > @@ -9,9 +9,12 @@
> > #include
> > #include
> > #include
> > +#include
> > +#include
> > +#include
> > #include
> > #include
> > -
> > +#include
> > #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,33 @@ 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 (IS_ERR(par->gpio.te)) {
> > + rc = PTR_ERR(par->gpio.te);
> > + dev_err(par->info->device, "Failed to request te
> > gpio: %d\n", rc);
>
> This also prints an error in case of -EPROBE_DEFER.
> dev_err_probe()?
>
> > + return rc;
> > + }
> > + 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) {
> > + dev_err(par->info->device, "TE request_irq
> > failed.\n");
> > + devm_gpiod_put(dev, par->gpio.te);
>
> No need to call devm_gpiod_put() here, as it's managed automatically.
>
> Gr{oetje,eeting}s,
>
> Geert
>
hi,i will fix in patch v11
regards
zhangxuezhi1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v10] staging: fbtft: add tearing signal detect
On Wed, 27 Jan 2021 14:51:55 +0100 Greg KH wrote: > On Wed, Jan 27, 2021 at 09:42:52PM +0800, Carlis wrote: > > From: zhangxuezhi > > > > For st7789v ic,when we need continuous full screen refresh, it is > > best to wait for the TE signal arrive to avoid screen tearing > > > > Signed-off-by: zhangxuezhi > > Please slow down and wait at least a day between patch submissions, > there is no rush here. > > And also, ALWAYS run scripts/checkpatch.pl on your submissions, so > that you don't have a maintainer asking you about basic problems, > like are in this current patch :( > > thanks, > > greg k-h hi, This is my first patch contribution to Linux, so some of the rules are not very clear .In addition, I can confirm that before sending patch, I check it with checkPatch.py every time.Thank you very much for your help regards zhangxuezhi ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v10] staging: fbtft: add tearing signal detect
On Wed, 27 Jan 2021 15:13:05 +0100 Greg KH wrote: > On Wed, Jan 27, 2021 at 10:08:09PM +0800, carlis wrote: > > On Wed, 27 Jan 2021 14:51:55 +0100 > > Greg KH wrote: > > > > > On Wed, Jan 27, 2021 at 09:42:52PM +0800, Carlis wrote: > > > > From: zhangxuezhi > > > > > > > > For st7789v ic,when we need continuous full screen refresh, it > > > > is best to wait for the TE signal arrive to avoid screen tearing > > > > > > > > Signed-off-by: zhangxuezhi > > > > > > Please slow down and wait at least a day between patch > > > submissions, there is no rush here. > > > > > > And also, ALWAYS run scripts/checkpatch.pl on your submissions, so > > > that you don't have a maintainer asking you about basic problems, > > > like are in this current patch :( > > > > > > thanks, > > > > > > greg k-h > > > > hi, > > This is my first patch contribution to Linux, so some of the rules > > are not very clear .In addition, I can confirm that before sending > > patch, I check it with checkPatch.py every time.Thank you very much > > for your help > > Please read Documentation/SubmittingPatches which has a link to the > checklist and other documentation you should read. > > And I doubt you are running checkpatch on your submission, as there is > obvious coding style issues in it. If so, please provide the output > as it must be broken :( > > thanks, > > greg k-h hi, the patch v11 checkpatch.pl output is below: carlis@bf-rmsz-10:~/work/linux-kernel/linux$ ./scripts/checkpatch.pl 0001-staging-fbtft-add-tearing-signal-detect.patch total: 0 errors, 0 warnings, 0 checks, 176 lines checked 0001-staging-fbtft-add-tearing-signal-detect.patch has no obvious style problems and is ready for submission. regards zhangxuezhi ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v10] staging: fbtft: add tearing signal detect
On Wed, 27 Jan 2021 16:02:35 +0100 Greg KH wrote: > On Wed, Jan 27, 2021 at 05:49:46PM +0300, Dan Carpenter wrote: > > On Wed, Jan 27, 2021 at 03:25:20PM +0100, Greg KH wrote: > > > On Wed, Jan 27, 2021 at 10:17:08PM +0800, carlis wrote: > > > > On Wed, 27 Jan 2021 15:13:05 +0100 > > > > Greg KH wrote: > > > > > > > > > On Wed, Jan 27, 2021 at 10:08:09PM +0800, carlis wrote: > > > > > > On Wed, 27 Jan 2021 14:51:55 +0100 > > > > > > Greg KH wrote: > > > > > > > > > > > > > On Wed, Jan 27, 2021 at 09:42:52PM +0800, Carlis wrote: > > > > > > > > > > > > > > > From: zhangxuezhi > > > > > > > > > > > > > > > > For st7789v ic,when we need continuous full screen > > > > > > > > refresh, it is best to wait for the TE signal arrive to > > > > > > > > avoid screen tearing > > > > > > > > > > > > > > > > Signed-off-by: zhangxuezhi > > > > > > > > > > > > > > > > > > > > > > Please slow down and wait at least a day between patch > > > > > > > submissions, there is no rush here. > > > > > > > > > > > > > > And also, ALWAYS run scripts/checkpatch.pl on your > > > > > > > submissions, so that you don't have a maintainer asking > > > > > > > you about basic problems, like are in this current patch > > > > > > > :( > > > > > > > > > > > > > > thanks, > > > > > > > > > > > > > > greg k-h > > > > > > > > > > > > hi, > > > > > > This is my first patch contribution to Linux, so some of > > > > > > the rules are not very clear .In addition, I can confirm > > > > > > that before sending patch, I check it with checkPatch.py > > > > > > every time.Thank you very much for your help > > > > > > > > > > Please read Documentation/SubmittingPatches which has a link > > > > > to the checklist and other documentation you should read. > > > > > > > > > > And I doubt you are running checkpatch on your submission, as > > > > > there is obvious coding style issues in it. If so, please > > > > > provide the output as it must be broken :( > > > > > > > > > > thanks, > > > > > > > > > > greg k-h > > > > hi, the patch v11 checkpatch.pl output is below: > > > > > > > > carlis@bf-rmsz-10:~/work/linux-kernel/linux$ > > > > ./scripts/checkpatch.pl > > > > 0001-staging-fbtft-add-tearing-signal-detect.patch total: 0 > > > > errors, 0 warnings, 0 checks, 176 lines checked > > > > > > > > 0001-staging-fbtft-add-tearing-signal-detect.patch has no > > > > obvious style problems and is ready for submission. > > > > > > Wow, my apologies! > > > > > > Andy and Joe, there's something wrong here that is missing the > > > fact that a line is being indented with spaces and not tabs in > > > the patch at > > > https://lore.kernel.org/r/1611754972-151016-1-git-send-email-zhangxuez...@gmail.com > > > > > > Any ideas what broke? > > > > > > > /*Tearing Effect Line On*/ > > > > Comments are the exception to the "no spaces at the start of a line" > > rule. I was expecting that the kbuild-bot would send a Smatch > > warning for inconsistent indenting, but comments are not counted > > there either. > > > > I'm sort of surprised that we don't have checkpatch rule about the > > missing space characters. It should be: "/* Tearing Effect Line On > > */". > > That was going to be my next question, lots of comments added in this > patch don't have spaces... > > thanks, > > greg k-h Ok,i will fix it in patch v12 tomorrow regards, zhangxuezhi ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v10] staging: fbtft: add tearing signal detect
On Thu, 28 Jan 2021 00:32:22 +0200
Kari Argillander wrote:
> On Wed, Jan 27, 2021 at 09:42:52PM +0800, Carlis wrote:
> > For st7789v ic,when we need continuous full screen refresh, it is
> > best to wait for the TE signal arrive to avoid screen tearing
>
> > diff --git a/drivers/staging/fbtft/fb_st7789v.c
> > b/drivers/staging/fbtft/fb_st7789v.c index 3a280cc..cba08a8 100644
> > --- a/drivers/staging/fbtft/fb_st7789v.c
> > +++ b/drivers/staging/fbtft/fb_st7789v.c
> > @@ -9,9 +9,12 @@
> > #include
> > #include
> > #include
> > +#include
> > +#include
> > +#include
> > #include
> > #include
> > -
> > +#include
>
> Space after local headers. Also this should one up so all Linux
> headers are group together. You agreed?
>
OK,i will fix it in patch v12 tomorrow
> > #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 /* msecs */
> > +static struct mutex te_mutex;/* mutex for set te gpio irq status
> > */
>
> Space after ;
hi, i have fix it in the patch v11
>
> > +static struct completion spi_panel_te;
>
> What if multiple displays? Is this possible for user?
I will check it carefully again about this logic.
>
> > +
> > +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;
>
> Same here. Maybe you can think better way and then this code would
> also be cleaner.
>
> > +
> > + mutex_lock(&te_mutex);
>
> So locking should be done if we really do action and not just in case.
>
> > +
> > + 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,33 @@ 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 (IS_ERR(par->gpio.te)) {
> > + rc = PTR_ERR(par->gpio.te);
> > + dev_err(par->info->device, "Failed to request te
> > gpio: %d\n", rc);
> > + return rc;
> > + }
>
> You request with optinal and you still want to error out? We could
> just continue and not care about that error. User will be happier if
> device still works somehow.
You mean i just delete this dev_err print ?!
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);
> > + 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) {
> > + dev_err(par->info->device, "TE request_irq
> > failed.\n");
> > + devm_gpiod_put(dev, par->gpio.te);
> > + return rc;
> > + }
> > +
> > + disable_irq_nosync(gpiod_to_irq(par->gpio.te));
> > + } else {
> > + dev_info(par->info->device, "%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 +193,9 @@ static int init_display(struct fbtft_par *par)
> > */
> > write_reg(par, PWCTRL1, 0xA4, 0xA1);
> >
&
Re: [PATCH v10] staging: fbtft: add tearing signal detect
On Thu, 28 Jan 2021 08:52:33 +0200
Kari Argillander wrote:
> On Thu, Jan 28, 2021 at 09:42:58AM +0800, carlis wrote:
> > On Thu, 28 Jan 2021 00:32:22 +0200
> > Kari Argillander wrote:
> > > > #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 /* msecs */
> > > > +static struct mutex te_mutex;/* mutex for set te gpio irq
> > > > status */
> > >
> > > Space after ;
> > hi, i have fix it in the patch v11
> > >
>
> Yeah sorry. I accidentally review wrong patch. But mostly stuff are
> still relevant.
>
> > > > @@ -82,6 +111,33 @@ 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 (IS_ERR(par->gpio.te)) {
> > > > + rc = PTR_ERR(par->gpio.te);
> > > > + dev_err(par->info->device, "Failed to request
> > > > te gpio: %d\n", rc);
> > > > + return rc;
> > > > + }
> > >
> > > You request with optinal and you still want to error out? We could
> > > just continue and not care about that error. User will be happier
> > > if device still works somehow.
> > You mean i just delete this dev_err print ?!
> > 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);
>
> Not exactly. I'm suggesting something like this.
>
> if (IS_ERR(par->gpio.te) == -EPROBE_DEFER) {
> return -EPROBE_DEFER;
>
> if (IS_ERR(par->gpio.te))
> par-gpio.te = NULL;
>
> This like beginning of your patch series but the difference is that if
> EPROBE_DEFER then we will try again later. Any other error and we will
> just ignore TE gpio. But this is up to you what you want to do. To me
> this just seems place where this kind of logic can work.
>
> > > > + 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)
> > >
> > > !ret
> > >
> > > > + dev_err(par->info->device,
> > > > "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;
> > >
> > > Do we want to return something over 0? If not then this can be
> > > return 0. And then you do not need to even init ret value at the
> > > beginning.
> > >
> > > Also wait little bit like Greg sayd before sending new version.
> > > Someone might nack about what I say or say something more.
> > >
> > hi, i copy fbtft_write_vmem16_bus8 from file fbtft_bus.c and modify
> > it ,just add te wait logic, i will take more time to check this
> > original function.
>
> It might be ok or not. You should still check.
hi, i will check more carefully, now i have a new problem, Is there a
way to clear the interrupt pending state before opening it again?
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v10] staging: fbtft: add tearing signal detect
On Thu, 28 Jan 2021 10:42:54 +0100
Geert Uytterhoeven wrote:
> Hi Kari,
>
> On Thu, Jan 28, 2021 at 7:53 AM Kari Argillander
> wrote:
> > On Thu, Jan 28, 2021 at 09:42:58AM +0800, carlis wrote:
> > > On Thu, 28 Jan 2021 00:32:22 +0200
> > > Kari Argillander wrote:
> > > > > #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 /* msecs */
> > > > > +static struct mutex te_mutex;/* mutex for set te gpio irq
> > > > > status */
> > > >
> > > > Space after ;
> > > hi, i have fix it in the patch v11
> > > >
> >
> > Yeah sorry. I accidentally review wrong patch. But mostly stuff are
> > still relevant.
> >
> > > > > @@ -82,6 +111,33 @@ 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 (IS_ERR(par->gpio.te)) {
> > > > > + rc = PTR_ERR(par->gpio.te);
> > > > > + dev_err(par->info->device, "Failed to request te
> > > > > gpio: %d\n", rc);
> > > > > + return rc;
> > > > > + }
> > > >
> > > > You request with optinal and you still want to error out? We
> > > > could just continue and not care about that error. User will be
> > > > happier if device still works somehow.
>
> devm_gpiod_get_index_optional() returns NULL, not an error, if the
> GPIO is not found. So if IS_ERR() is the right check.
>
> And checks for -EPROBE_DEFER can be handled automatically
> by using dev_err_probe() instead of dev_err().
>
hi, i 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);
dev_err_probe(par->info->device, rc, "Failed to request
te gpio\n"); return rc;
}
if (par->gpio.te) {
init_completion(&spi_panel_te);
rc = devm_request_irq(dev,
gpiod_to_irq(par->gpio.te),
spi_panel_te_handler,
IRQF_TRIGGER_RISING, "TE_GPIO", par);
if (rc) {
dev_err(par->info->device, "TE request_irq
failed.\n"); return rc;
}
disable_irq_nosync(gpiod_to_irq(par->gpio.te));
} else {
dev_info(par->info->device, "%s:%d, TE gpio not
specified\n", __func__, __LINE__);
}
> > > You mean i just delete this dev_err print ?!
> > > 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);
> >
> > Not exactly. I'm suggesting something like this.
> >
> > if (IS_ERR(par->gpio.te) == -EPROBE_DEFER) {
> > return -EPROBE_DEFER;
> >
> > if (IS_ERR(par->gpio.te))
> > par-gpio.te = NULL;
> >
> > This like beginning of your patch series but the difference is that
> > if EPROBE_DEFER then we will try again later. Any other error and
> > we will just ignore TE gpio. But this is up to you what you want to
> > do. To me this just seems place where this kind of logic can work.
>
> Gr{oetje,eeting}s,
>
> Geert
>
regards,
zhangxuezhi
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v10] staging: fbtft: add tearing signal detect
On Thu, 28 Jan 2021 12:15:28 +0100
Geert Uytterhoeven wrote:
> Hi Carlis,
>
> On Thu, Jan 28, 2021 at 12:03 PM carlis
> wrote:
> > On Thu, 28 Jan 2021 10:42:54 +0100
> > Geert Uytterhoeven wrote:
> > > On Thu, Jan 28, 2021 at 7:53 AM Kari Argillander
> > > wrote:
> > > > On Thu, Jan 28, 2021 at 09:42:58AM +0800, carlis wrote:
> > > > > On Thu, 28 Jan 2021 00:32:22 +0200
> > > > > Kari Argillander wrote:
> > > > > > > #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 /* msecs */
> > > > > > > +static struct mutex te_mutex;/* mutex for set te gpio irq
> > > > > > > status */
> > > > > >
> > > > > > Space after ;
> > > > > hi, i have fix it in the patch v11
> > > > > >
> > > >
> > > > Yeah sorry. I accidentally review wrong patch. But mostly stuff
> > > > are still relevant.
> > > >
> > > > > > > @@ -82,6 +111,33 @@ 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 (IS_ERR(par->gpio.te)) {
> > > > > > > + rc = PTR_ERR(par->gpio.te);
> > > > > > > + dev_err(par->info->device, "Failed to request te
> > > > > > > gpio: %d\n", rc);
> > > > > > > + return rc;
> > > > > > > + }
> > > > > >
> > > > > > You request with optinal and you still want to error out? We
> > > > > > could just continue and not care about that error. User
> > > > > > will be happier if device still works somehow.
> > >
> > > devm_gpiod_get_index_optional() returns NULL, not an error, if the
> > > GPIO is not found. So if IS_ERR() is the right check.
> > >
> > > And checks for -EPROBE_DEFER can be handled automatically
> > > by using dev_err_probe() instead of dev_err().
> > >
> > hi, i 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);
> > dev_err_probe(par->info->device, rc, "Failed to
> > request te gpio\n"); return rc;
> > }
> > if (par->gpio.te) {
> > init_completion(&spi_panel_te);
> > rc = devm_request_irq(dev,
> > gpiod_to_irq(par->gpio.te),
> > spi_panel_te_handler,
> > IRQF_TRIGGER_RISING, "TE_GPIO", par);
> > if (rc) {
> > dev_err(par->info->device, "TE request_irq
> > failed.\n"); return rc;
>
> dev_err_probe()
>
> > }
> >
> > disable_irq_nosync(gpiod_to_irq(par->gpio.te));
> > } else {
> > dev_info(par->info->device, "%s:%d, TE gpio not
> > specified\n", __func__, __LINE__);
> > }
>
> Gr{oetje,eeting}s,
>
> Geert
>
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))
return dev_err_probe(par->info->device,
PTR_ERR(par->gpio.te), "Failed to request te gpio\n");
if (par->gpio.te) {
init_completion(&spi_panel_te);
rc = devm_request_irq(dev,
gpiod_to_irq(par->gpio.te),
spi_panel_te_handler,
IRQF_TRIGGER_RISING, "TE_GPIO", par);
if (IS_ERR(rc))
return dev_err_probe(par->info->device,
PTR_ERR(rc), "TE request_irq failed.\n");
disable_irq_nosync(gpiod_to_irq(par->gpio.te));
} else {
dev_info(par->info->device, "%s:%d, TE gpio not
specified\n", __func__, __LINE__);
}
regards,
zhangxuezhi
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v12] staging: fbtft: add tearing signal detect
From: zhangxuezhi
For st7789v ic,when we need continuous full screen refresh, it is best to
wait for the TE signal arrive to avoid screen tearing
Signed-off-by: zhangxuezhi
---
v12: change dev_err to dev_err_probe and add space in comments start, and
delete te_mutex, change te wait logic
v11: remove devm_gpio_put and change a dev_err to dev_info
v10: additional notes
v9: change pr_* to dev_*
v8: delete a log line
v7: return error value when request fail
v6: add te gpio request fail deal logic
v5: fix log print
v4: modify some code style and change te irq set function name
v3: modify author and signed-off-by name
v2: add release te gpio after irq request fail
---
drivers/staging/fbtft/fb_st7789v.c | 116 +
drivers/staging/fbtft/fbtft.h | 1 +
2 files changed, 117 insertions(+)
diff --git a/drivers/staging/fbtft/fb_st7789v.c
b/drivers/staging/fbtft/fb_st7789v.c
index 3a280cc..f08e9da 100644
--- a/drivers/staging/fbtft/fb_st7789v.c
+++ b/drivers/staging/fbtft/fb_st7789v.c
@@ -9,7 +9,11 @@
#include
#include
#include
+#include
+#include
#include
+#include
+
#include
#include "fbtft.h"
@@ -66,6 +70,15 @@ 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 /* msecs */
+static struct completion spi_panel_te;
+
+static irqreturn_t spi_panel_te_handler(int irq, void *data)
+{
+ complete(&spi_panel_te);
+ return IRQ_HANDLED;
+}
+
/**
* init_display() - initialize the display controller
*
@@ -82,6 +95,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 (IS_ERR(par->gpio.te))
+ return dev_err_probe(par->info->device, PTR_ERR(par->gpio.te),
+"Failed to request te gpio\n");
+
+ if (par->gpio.te) {
+ init_completion(&spi_panel_te);
+ rc = devm_request_irq(dev,
+ gpiod_to_irq(par->gpio.te),
+spi_panel_te_handler, IRQF_TRIGGER_RISING,
+"TE_GPIO", par);
+ if (IS_ERR(rc))
+ return dev_err_probe(par->info->device, PTR_ERR(rc),
+"TE request_irq failed.\n");
+
+ disable_irq_nosync(gpiod_to_irq(par->gpio.te));
+ } else {
+ dev_info(par->info->device, "%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 +173,10 @@ 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)
@@ -146,6 +186,81 @@ static int init_display(struct fbtft_par *par)
}
/**
+ * st7789v_write_vmem16_bus8() - write data to display
+ *
+ * @par: FBTFT parameter object
+ * @offset: offset from screen_buffer
+ * @len: the length of data to be written
+ *
+ * 16 bit pixel over 8-bit databus
+ *
+ * Return: 0 on success, < 0 if error occurred.
+ */
+
+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);
+
+ if (par->gpio.te) {
+ enable_irq(gpiod_to_irq(par->gpio.te));
+ reinit_completion(&spi_panel_te);
+ ret = wait_for_completion_timeout(&spi_panel_te,
+
msecs_to_jiffies(SPI_PANEL_TE_TIMEOUT));
+ if (ret == 0)
+ dev_err(par->info->device, "wait panel TE time out\n");
+
+ disable_irq(gpiod_to_irq(par->gpio.te));
+ }
+ /* 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;
+
Re: [PATCH v12] staging: fbtft: add tearing signal detect
On Thu, 28 Jan 2021 16:33:02 +0200
Andy Shevchenko wrote:
> On Thu, Jan 28, 2021 at 2:58 PM Carlis wrote:
>
> Thanks for your contribution, my comments below.
>
> > From: zhangxuezhi
>
> You probably have to configure your Git to use the same account for
> author and committer.
hi,you mean like below:
Carlis
?
>
> > For st7789v ic,when we need continuous full screen refresh, it is
> > best to
>
> 'ic,when' -> 'IC, when'
>
> > wait for the TE signal arrive to avoid screen tearing
>
> Decode TE for people who are not familiar with the abbreviation.
>
> Missed period at the end of sentence.
>
> ...
>
> > #include
> > #include
> > #include
> > +#include
> > +#include
> > #include
> > +#include
> > +
>
> Good, but I would rather squeeze it above to be more or less ordered,
> like just after delay.h inclusion.
>
> > #include
>
> ...
>
> > +#define SPI_PANEL_TE_TIMEOUT 400 /* msecs */
>
> Useless comment. Instead use _MS suffix in the name of constant.
> Besides that please add a comment explaining why this value has been
> chosen.
>
> ...
>
> > +static struct completion spi_panel_te;
>
> As Greg said.
>
> ...
>
> > static int init_display(struct fbtft_par *par)
> > {
> > + int rc;
> > + struct device *dev = par->info->device;
>
> Keep reversed xmas tree order:
>
>struct device *dev = par->info->device;
>int rc;
>
> ...
>
> > + par->gpio.te = devm_gpiod_get_index_optional(dev, "te", 0,
> > GPIOD_IN);
>
> No need to have it requested for all time since you use it as an IRQ
> later on. The IRQ chip will call the GPIO library framework to lock a
> pin as IRQ anyway.
>
> > + if (IS_ERR(par->gpio.te))
> > + return dev_err_probe(par->info->device,
> > PTR_ERR(par->gpio.te),
> > +"Failed to request te
> > gpio\n");
>
> > + if (par->gpio.te) {
>
> Instead you should probably do the following:
>
> int irq;
>
> irq = gpiod_to_irq(...);
> if (irq > 0)
>
> > + init_completion(&spi_panel_te);
> > + rc = devm_request_irq(dev,
>
> > + gpiod_to_irq(par->gpio.te),
>
> ...and here simply use irq.
>
> > +spi_panel_te_handler,
> > IRQF_TRIGGER_RISING,
> > +"TE_GPIO", par);
>
> > + if (IS_ERR(rc))
>
> This is wrong. rc is integer no IS_ERR() is required. Ditto for
> PTR_ERR(). Have you even looked for these macros implementations?
>
> > + return dev_err_probe(par->info->device,
> > PTR_ERR(rc),
>
> Use your temporary variable and move...
>
> > +"TE request_irq
> > failed.\n");
>
> ...this on the previous line.
>
> > + disable_irq_nosync(gpiod_to_irq(par->gpio.te));
>
> Why do you call gpio_to_irq() twice?
>
>
> > + } else {
> > + dev_info(par->info->device, "%s:%d, TE gpio not
> > specified\n",
> > +__func__, __LINE__);
>
> Remove this noise (besides the fact that we don't use __file__ and
> __LINE__ in messages like this.
>
> > + }
>
> Taking all together you probably need to create a helper and use it
> inside init_display(), like
>
> static int init_tearing_effect_line(struct fbtft_par *par)
> {
> struct device *dev = par->info->device;
> struct gpio_desc *te;
> int irq, rc;
>
> te = gpiod_get_optional(dev, "te", GPIOD_IN);
> if (IS_ERR(te))
>return dev_err_probe(dev, PTR_ERR(te), "Failed to request
> te GPIO\n");
>
> irq = gpiod_to_irq(te); // this value you have to save in the
> driver's (per device) data structure.
>
> /* GPIO is locked as an IRQ, we may drop the reference */
> gpiod_put(te);
>
> init_completion(&spi_panel_te); // should be in the (per device)
> data structure
> rc = devm_request_irq(dev, irq, spi_panel_te_handler,
> IRQF_TRIGGER_RISING, "TE_GPIO", par);
> if (rc)
> return dev_err_probe(dev, rc, "TE IRQ request
> failed.\n"); disable_irq_nosync(irq);
> return irq;
> }
>
h
Re: [PATCH v12] staging: fbtft: add tearing signal detect
On Fri, 29 Jan 2021 12:23:08 +0200
Andy Shevchenko wrote:
> On Fri, Jan 29, 2021 at 7:01 AM carlis wrote:
> > On Thu, 28 Jan 2021 16:33:02 +0200
> > Andy Shevchenko wrote:
> > > On Thu, Jan 28, 2021 at 2:58 PM Carlis
> > > wrote:
> > >
> > > Thanks for your contribution, my comments below.
> > >
> > > > From: zhangxuezhi
> > >
> > > You probably have to configure your Git to use the same account
> > > for author and committer.
> >
> > hi,you mean like below:
> > Carlis
> > ?
>
> I meant that you shouldn't probably have a From: line in the commit
> message.
>
> ...
>
> > hi, i have modified it according to your suggestion like below:
>
> Please, go again thru my comments and comments from others and
> carefully address all of them everywhere in your contribution. If you
> have questions, ask them in reply in the corresponding context.
>
> ...
>
> > /**
> > * init_tearing_effect_line() - init tearing effect line
>
> > *
>
> For example, above was commented on and hasn't been addressed here.
>
hi,here i can not get you.
> > * @par: FBTFT parameter object
> > *
> > * Return: 0 on success, < 0 if error occurred.
> > */
> > static int init_tearing_effect_line(struct fbtft_par *par)
> > {
> > struct device *dev = par->info->device;
> > struct gpio_desc *te;
> > int rc;
> >
> > te = gpiod_get_optional(dev, "te", GPIOD_IN);
> > if (IS_ERR(te))
> > return dev_err_probe(dev, PTR_ERR(te), "Failed to
> > request te GPIO\n");
> >
>
> > if (te) {
>
> This one is not like I suggested.
Why? My thinking is that if the TE is not configured and NULL is
returned, the initialization can still proceed.
>
> > par->irq_te = gpiod_to_irq(te);
> > gpiod_put(te);
> >
>
> > if (par->irq_te) {
>
> This is wrong.
Why? i have read gpiod_to_irq code, if an error occurs, a negative
value is returned, and zero is not possible,so I need this value to
determine if TE IRQ is configured
>
> > rc = devm_request_irq(dev,
> > par->irq_te,
> > panel_te_handler,
> > IRQF_TRIGGER_RISING,
> > "TE_GPIO", par);
>
> Try to use less LOCs.
>
> > if (rc)
> > return dev_err_probe(dev, rc, "TE
> > IRQ request failed.\n");
> >
> > disable_irq_nosync(par->irq_te);
> > init_completion(&par->panel_te);
>
> > } else {
> > return dev_err_probe(dev, par->irq_te,
> > "gpiod to TE IRQ failed.\n");
> > }
>
> Again, it is not what had been suggested.
>
> > }
> >
> > return 0;
> > }
>
> The rest is better, but we will see later on when you submit a new
> version (And I feel it won't be last).
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v12] staging: fbtft: add tearing signal detect
On Fri, 29 Jan 2021 12:23:08 +0200
Andy Shevchenko wrote:
> On Fri, Jan 29, 2021 at 7:01 AM carlis wrote:
> > On Thu, 28 Jan 2021 16:33:02 +0200
> > Andy Shevchenko wrote:
> > > On Thu, Jan 28, 2021 at 2:58 PM Carlis
> > > wrote:
> > >
> > > Thanks for your contribution, my comments below.
> > >
> > > > From: zhangxuezhi
> > >
> > > You probably have to configure your Git to use the same account
> > > for author and committer.
> >
> > hi,you mean like below:
> > Carlis
> > ?
>
> I meant that you shouldn't probably have a From: line in the commit
> message.
>
> ...
>
> > hi, i have modified it according to your suggestion like below:
>
> Please, go again thru my comments and comments from others and
> carefully address all of them everywhere in your contribution. If you
> have questions, ask them in reply in the corresponding context.
>
> ...
>
> > /**
> > * init_tearing_effect_line() - init tearing effect line
>
> > *
>
> For example, above was commented on and hasn't been addressed here.
>
> > * @par: FBTFT parameter object
> > *
> > * Return: 0 on success, < 0 if error occurred.
> > */
> > static int init_tearing_effect_line(struct fbtft_par *par)
> > {
> > struct device *dev = par->info->device;
> > struct gpio_desc *te;
> > int rc;
> >
> > te = gpiod_get_optional(dev, "te", GPIOD_IN);
> > if (IS_ERR(te))
> > return dev_err_probe(dev, PTR_ERR(te), "Failed to
> > request te GPIO\n");
> >
>
> > if (te) {
>
> This one is not like I suggested.
I don't think I have a problem here, if te GPIO is not configured, it
should return NULL, if it is configured, it should be greater than 0
>
> > par->irq_te = gpiod_to_irq(te);
> > gpiod_put(te);
> >
>
> > if (par->irq_te) {
>
> This is wrong.
>
> > rc = devm_request_irq(dev,
> > par->irq_te,
> > panel_te_handler,
> > IRQF_TRIGGER_RISING,
> > "TE_GPIO", par);
>
> Try to use less LOCs.
LOCs i can not get you
>
> > if (rc)
> > return dev_err_probe(dev, rc, "TE
> > IRQ request failed.\n");
> >
> > disable_irq_nosync(par->irq_te);
> > init_completion(&par->panel_te);
>
> > } else {
> > return dev_err_probe(dev, par->irq_te,
> > "gpiod to TE IRQ failed.\n");
> > }
>
> Again, it is not what had been suggested.
>
> > }
> >
> > return 0;
> > }
>
> The rest is better, but we will see later on when you submit a new
> version (And I feel it won't be last).
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v12] staging: fbtft: add tearing signal detect
On Fri, 29 Jan 2021 12:23:08 +0200
Andy Shevchenko wrote:
> On Fri, Jan 29, 2021 at 7:01 AM carlis wrote:
> > On Thu, 28 Jan 2021 16:33:02 +0200
> > Andy Shevchenko wrote:
> > > On Thu, Jan 28, 2021 at 2:58 PM Carlis
> > > wrote:
> > >
> > > Thanks for your contribution, my comments below.
> > >
> > > > From: zhangxuezhi
> > >
> > > You probably have to configure your Git to use the same account
> > > for author and committer.
> >
> > hi,you mean like below:
> > Carlis
> > ?
>
> I meant that you shouldn't probably have a From: line in the commit
> message.
>
> ...
>
> > hi, i have modified it according to your suggestion like below:
>
> Please, go again thru my comments and comments from others and
> carefully address all of them everywhere in your contribution. If you
> have questions, ask them in reply in the corresponding context.
>
> ...
>
> > /**
> > * init_tearing_effect_line() - init tearing effect line
>
> > *
>
> For example, above was commented on and hasn't been addressed here.
>
> > * @par: FBTFT parameter object
> > *
> > * Return: 0 on success, < 0 if error occurred.
> > */
> > static int init_tearing_effect_line(struct fbtft_par *par)
> > {
> > struct device *dev = par->info->device;
> > struct gpio_desc *te;
> > int rc;
> >
> > te = gpiod_get_optional(dev, "te", GPIOD_IN);
> > if (IS_ERR(te))
> > return dev_err_probe(dev, PTR_ERR(te), "Failed to
> > request te GPIO\n");
> >
>
> > if (te) {
>
> This one is not like I suggested.
>
> > par->irq_te = gpiod_to_irq(te);
> > gpiod_put(te);
> >
>
> > if (par->irq_te) {
>
> This is wrong.
>
> > rc = devm_request_irq(dev,
> > par->irq_te,
> > panel_te_handler,
> > IRQF_TRIGGER_RISING,
> > "TE_GPIO", par);
>
> Try to use less LOCs.
>
> > if (rc)
> > return dev_err_probe(dev, rc, "TE
> > IRQ request failed.\n");
> >
> > disable_irq_nosync(par->irq_te);
> > init_completion(&par->panel_te);
>
> > } else {
> > return dev_err_probe(dev, par->irq_te,
> > "gpiod to TE IRQ failed.\n");
> > }
>
> Again, it is not what had been suggested.
>
> > }
> >
> > return 0;
> > }
>
> The rest is better, but we will see later on when you submit a new
> version (And I feel it won't be last).
>
Hi, I apologize for what I said in the previous two emails. I missed
one email you sent before, and now I probably understand what you
meant. Here is a version I modified according to your suggestion:
>From 399e7fb91d1dcba4924cd38cc8283393c80b97e4 Mon Sep 17 00:00:00 2001
From: Carlis
Date: Sun, 24 Jan 2021 22:43:21 +0800
Subject: [PATCH v13] staging: fbtft: add tearing signal detect
For st7789v IC,when we need continuous full screen refresh, it is best
to wait for the tearing effect line signal arrive to avoid screen
tearing.
Signed-off-by: Carlis
---
v13: change te completion to par data struct member and add a new
function to deal te gpio request.
v12: change dev_err to dev_err_probe and add space in comments start,
and delete te_mutex, change te wait logic.
v11: remove devm_gpio_put and change a dev_err to dev_info.
v10: additional notes.
v9: change pr_* to dev_*.
v8: delete a log line.
v7: return error value when request fail.
v6: add te gpio request fail deal logic.
v5: fix log print.
v4: modify some code style and change te irq set function name.
v3: modify author and signed-off-by name.
v2: add release te gpio after irq request fail.
---
drivers/staging/fbtft/fb_st7789v.c | 138
+ drivers/staging/fbtft/fbtft.h
| 5 ++ 2 files changed, 143 insertions(+)
diff --git a/drivers/staging/fbtft/fb_st7789v.c
b/drivers/staging/fbtft/fb_st7789v.c index 3a280cc..8faae7a 100644
--- a/drivers/staging/fbtft/fb_st7789v.c
+++ b/drivers/staging/fbtft/fb_st7789v.c
@@ -7,9 +7,13 @@
#include
#include
+#include
#include
#include
+#include
+#include
#include
+
#include
#include "fbtft.h"
@@ -66,6 +70,55 @@ enum s
Re: [PATCH v12] staging: fbtft: add tearing signal detect
On Fri, 29 Jan 2021 16:26:12 +0200
Andy Shevchenko wrote:
> On Fri, Jan 29, 2021 at 3:56 PM carlis wrote:
> > On Fri, 29 Jan 2021 12:23:08 +0200
> > Andy Shevchenko wrote:
>
> We are almost there, I have no idea what Noralf or others are going to
> say though.
>
> ...
>
> > Hi, I apologize for what I said in the previous two emails. I missed
> > one email you sent before, and now I probably understand what you
> > meant. Here is a version I modified according to your suggestion:
> >
> > From 399e7fb91d1dcba4924cd38cc8283393c80b97e4 Mon Sep 17 00:00:00
> > 2001 From: Carlis
> > Date: Sun, 24 Jan 2021 22:43:21 +0800
> > Subject: [PATCH v13] staging: fbtft: add tearing signal detect
> >
> > For st7789v IC,when we need continuous full screen refresh, it is
> > best
>
> Missed space after comma.
>
> > to wait for the tearing effect line signal arrive to avoid screen
>
> to arrive
>
> > tearing.
>
> ...
>
> > +#define PANEL_TE_TIMEOUT_MS 34 /* 60Hz for 16.6ms, configured as
> > 2*16.6ms */ +
>
> Move comment before the definition
> /* comment */
> #define DEFINITION
>
> Also consider to use 33 ms as closest to what you mentioned in the
> comment. Or leave it with mention that you are using ceil() value.
>
> ...
>
> > +/**
> > + * init_tearing_effect_line() - init tearing effect line
>
> > + *
>
> As per a few previous reviews.
> Okay, I have noticed that the existing kernel-doc is written like
> this, but it doesn't prevent you from avoiding this little mistake.
>
> > + * @par: FBTFT parameter object
> > + *
> > + * Return: 0 on success, or a negative error code otherwise.
> > + */
> > +static int init_tearing_effect_line(struct fbtft_par *par)
> > +{
> > + struct device *dev = par->info->device;
> > + struct gpio_desc *te;
> > + int rc;
> > +
> > + te = gpiod_get_optional(dev, "te", GPIOD_IN);
> > + if (IS_ERR(te))
> > + return dev_err_probe(dev, PTR_ERR(te), "Failed to
> > request te GPIO\n"); +
>
> Below is okay, but needs a comment explaining why we return a success.
>
> > + if (!te)
> > + return 0;
> > +
> > + par->irq_te = gpiod_to_irq(te);
> > +
> > + /* GPIO is locked as an IRQ, we may drop the reference */
> > + gpiod_put(te);
> > +
> > + if (par->irq_te < 0)
> > + return par->irq_te;
>
> I recommend using a temporary variable. In such a case you won't need
> to specifically check for negative error code. So, something like
>
> int irq;
>
> irq = ...
>
> if (irq < 0)
> return irq;
>
> ->irq_te = irq;
>
> > + init_completion(&par->panel_te);
> > + rc = devm_request_irq(dev, par->irq_te, panel_te_handler,
> > + IRQF_TRIGGER_RISING, "TE_GPIO", par);
> >
>
> Right. Now it needs a comment explaining the choice of rising edge
> type of IRQ.
>
> > + if (rc)
> > + return dev_err_probe(dev, rc, "TE IRQ request
> > failed.\n"); +
> > + disable_irq_nosync(par->irq_te);
> > +
> > + return 0;
> > +}
>
> ...
>
> > + rc = init_tearing_effect_line(par);
>
> > + if (rc < 0)
>
> Here is no need to specifically check against less than 0,
> if (ret)
> will work nicely.
>
> > + return rc;
>
> ...
>
> > + if (par->irq_te)
> > + write_reg(par, MIPI_DCS_SET_TEAR_ON, 0x00);
>
> Do you need to call MIPI_DCS_SET_TEAR_SCANLINE in this case?
>
Hi, TE line output is off by default when powering on, and I use
Qualcomm SDX55 chip, SPI top speed is only 50MHz, its data transmission
speed( It takes about 24ms to transmit a frame) is slower than 60Hz
refresh(a frame only need 16.6ms), so i think there is no need to call
MIPI_DCS_SET_TEAR_SCANLINE
> Alos, when there is no IRQ, shouldn't we explicitly call
>write_reg(par, MIPI_DCS_SET_TEAR_OFF);
> ?
>
> ...
>
> > /**
> > + * st7789v_write_vmem16_bus8() - write data to display
>
> > + *
>
> Redundant.
>
> > + * @par: FBTFT parameter object
> > + * @offset: offset from screen_buffer
> > + * @len: the length of data to be written
> > + *
>
> > + * 16 bit pixel over 8-bit databus
>
> Write 16-bit pix
[PATCH v13] staging: fbtft: add tearing signal detect
From: Carlis
For st7789v IC, when we need continuous full screen refresh, it is best to
wait for the tearing effect line signal to arrive to avoid screen tearing.
Signed-off-by: Carlis
---
v13: change TE completion to par data struct member and add a new
function to deal te gpio request, move wait logic to
fbtft_write_vmem16_bus8.
v12: change dev_err to dev_err_probe and add space in comments start, and
delete te_mutex, change te wait logic.
v11: remove devm_gpio_put and change a dev_err to dev_info.
v10: additional notes.
v9: change pr_* to dev_*.
v8: delete a log line.
v7: return error value when request fail.
v6: add te gpio request fail deal logic.
v5: fix log print.
v4: modify some code style and change te irq set function name.
v3: modify author and signed-off-by name.
v2: add release te gpio after irq request fail.
---
drivers/staging/fbtft/fb_st7789v.c | 112 +
drivers/staging/fbtft/fbtft.h | 5 ++
2 files changed, 117 insertions(+)
diff --git a/drivers/staging/fbtft/fb_st7789v.c
b/drivers/staging/fbtft/fb_st7789v.c
index 3a280cc..7b41bad 100644
--- a/drivers/staging/fbtft/fb_st7789v.c
+++ b/drivers/staging/fbtft/fb_st7789v.c
@@ -7,9 +7,13 @@
#include
#include
+#include
#include
#include
+#include
+#include
#include
+
#include
#include "fbtft.h"
@@ -66,6 +70,59 @@ enum st7789v_command {
#define MADCTL_MX BIT(6) /* bitmask for column address order */
#define MADCTL_MY BIT(7) /* bitmask for page address order */
+/* 60Hz for 16.6ms, configured as 2*16.6ms */
+#define PANEL_TE_TIMEOUT_MS 33
+
+static irqreturn_t panel_te_handler(int irq, void *data)
+{
+ struct fbtft_par *par = (struct fbtft_par *)data;
+
+ complete(&par->panel_te);
+ return IRQ_HANDLED;
+}
+
+/*
+ * init_tearing_effect_line() - init tearing effect line.
+ * @par: FBTFT parameter object.
+ *
+ * Return: 0 on success, or a negative error code otherwise.
+ */
+static int init_tearing_effect_line(struct fbtft_par *par)
+{
+ struct device *dev = par->info->device;
+ struct gpio_desc *te;
+ int rc, irq;
+
+ te = gpiod_get_optional(dev, "te", GPIOD_IN);
+ if (IS_ERR(te))
+ return dev_err_probe(dev, PTR_ERR(te), "Failed to request te
GPIO\n");
+
+ /* if te is NULL, indicating no configuration, directly return success
*/
+ if (!te)
+ return 0;
+
+ irq = gpiod_to_irq(te);
+
+ /* GPIO is locked as an IRQ, we may drop the reference */
+ gpiod_put(te);
+
+ if (irq < 0)
+ return irq;
+
+ par->irq_te = irq;
+ init_completion(&par->panel_te);
+
+ /* The effective state is high and lasts no more than 1000 microseconds
*/
+ rc = devm_request_irq(dev, par->irq_te, panel_te_handler,
+ IRQF_TRIGGER_RISING, "TE_GPIO", par);
+ if (rc)
+ return dev_err_probe(dev, rc, "TE IRQ request failed.\n");
+
+ disable_irq_nosync(par->irq_te);
+
+ return 0;
+}
+
/**
* init_display() - initialize the display controller
*
@@ -82,6 +139,12 @@ enum st7789v_command {
*/
static int init_display(struct fbtft_par *par)
{
+ int rc;
+
+ rc = init_tearing_effect_line(par);
+ if (rc)
+ return rc;
+
/* turn off sleep mode */
write_reg(par, MIPI_DCS_EXIT_SLEEP_MODE);
mdelay(120);
@@ -137,6 +200,10 @@ static int init_display(struct fbtft_par *par)
*/
write_reg(par, PWCTRL1, 0xA4, 0xA1);
+ /* TE line output is off by default when powering on */
+ if (par->irq_te)
+ write_reg(par, MIPI_DCS_SET_TEAR_ON, 0x00);
+
write_reg(par, MIPI_DCS_SET_DISPLAY_ON);
if (HSD20_IPS)
@@ -145,6 +212,50 @@ static int init_display(struct fbtft_par *par)
return 0;
}
+/*
+ * write_vmem() - write data to display.
+ * @par: FBTFT parameter object.
+ * @offset: offset from screen_buffer.
+ * @len: the length of data to be writte.
+ *
+ * Return: 0 on success, or a negative error code otherwise.
+ */
+static int write_vmem(struct fbtft_par *par, size_t offset, size_t len)
+{
+ struct device *dev = par->info->device;
+ int ret;
+
+ if (par->irq_te) {
+ enable_irq(par->irq_te);
+ reinit_completion(&par->panel_te);
+ ret = wait_for_completion_timeout(&par->panel_te,
+
msecs_to_jiffies(PANEL_TE_TIMEOUT_MS));
+ if (ret == 0)
+ dev_err(dev, "wait panel TE timeout\n");
+
+ disable_irq(par->irq_te);
+ }
+
+ ret = 0;
+ switch (par->pdata->display.buswidth) {
+ case 8:
+ ret = fbtft_write_vmem16_bus8(par, offset, len);
+ break;
+ ca
Re: [PATCH v12] staging: fbtft: add tearing signal detect
On Mon, 1 Feb 2021 19:40:21 +0200 Andy Shevchenko wrote: > On Sat, Jan 30, 2021 at 8:39 AM carlis wrote: > > On Fri, 29 Jan 2021 16:26:12 +0200 > > Andy Shevchenko wrote: > > > On Fri, Jan 29, 2021 at 3:56 PM carlis > > > wrote: > > > > On Fri, 29 Jan 2021 12:23:08 +0200 > > > > Andy Shevchenko wrote: > > ... > > > > > Hi, I apologize for what I said in the previous two emails. I > > > > missed one email you sent before, and now I probably understand > > > > what you meant. Here is a version I modified according to your > > > > suggestion: > > I have realized that you are mocking stuff in the generic fbtft > structure for all drivers while only a single one is going to use > that. Consider moving everything to the driver in question. > > hi, Do you mean that i define the TE completion and irq_te in the fb_st7789v.c as i did before? ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
