linux-fbdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v5] fbtft: add tearing signal detect
@ 2021-01-27  1:32 Carlis
  2021-01-27  5:45 ` Dan Carpenter
  2021-01-27  6:36 ` Dan Carpenter
  0 siblings, 2 replies; 6+ messages in thread
From: Carlis @ 2021-01-27  1:32 UTC (permalink / raw)
  To: gregkh
  Cc: colin.king, oliver.graute, zhangxuezhi1, mh12gx2825, sbrivio,
	dri-devel, linux-fbdev, devel, linux-kernel

From: zhangxuezhi <zhangxuezhi1@yulong.com>

For st7789v ic,add tearing signal detect to avoid screen tearing

Signed-off-by: zhangxuezhi <zhangxuezhi1@yulong.com>
---
v5:fix log print
---
 drivers/staging/fbtft/fb_st7789v.c | 128 ++++++++++++++++++++++++++++++++++++-
 drivers/staging/fbtft/fbtft.h      |   1 +
 2 files changed, 128 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/fbtft/fb_st7789v.c b/drivers/staging/fbtft/fb_st7789v.c
index 3a280cc..ab10235 100644
--- a/drivers/staging/fbtft/fb_st7789v.c
+++ b/drivers/staging/fbtft/fb_st7789v.c
@@ -9,9 +9,12 @@
 #include <linux/delay.h>
 #include <linux/init.h>
 #include <linux/kernel.h>
+#include <linux/mutex.h>
+#include <linux/interrupt.h>
+#include <linux/completion.h>
 #include <linux/module.h>
 #include <video/mipi_display.h>
-
+#include <linux/gpio/consumer.h>
 #include "fbtft.h"
 
 #define DRVNAME "fb_st7789v"
@@ -66,6 +69,32 @@ enum st7789v_command {
 #define MADCTL_MX BIT(6) /* bitmask for column address order */
 #define MADCTL_MY BIT(7) /* bitmask for page address order */
 
+#define SPI_PANEL_TE_TIMEOUT	400
+static struct mutex te_mutex;/*mutex for tearing line*/
+static struct completion spi_panel_te;
+
+static irqreturn_t spi_panel_te_handler(int irq, void *data)
+{
+	complete(&spi_panel_te);
+	return IRQ_HANDLED;
+}
+
+static void set_spi_panel_te_irq_status(struct fbtft_par *par, bool enable)
+{
+	static int te_irq_count;
+
+	mutex_lock(&te_mutex);
+
+	if (enable) {
+		if (++te_irq_count == 1)
+			enable_irq(gpiod_to_irq(par->gpio.te));
+	} else {
+		if (--te_irq_count == 0)
+			disable_irq(gpiod_to_irq(par->gpio.te));
+	}
+	mutex_unlock(&te_mutex);
+}
+
 /**
  * init_display() - initialize the display controller
  *
@@ -82,6 +111,29 @@ enum st7789v_command {
  */
 static int init_display(struct fbtft_par *par)
 {
+	int rc;
+	struct device *dev = par->info->device;
+
+	par->gpio.te = devm_gpiod_get_index_optional(dev, "te", 0, GPIOD_IN);
+	if (par->gpio.te) {
+		init_completion(&spi_panel_te);
+		mutex_init(&te_mutex);
+		rc = devm_request_irq(dev,
+				      gpiod_to_irq(par->gpio.te),
+				     spi_panel_te_handler, IRQF_TRIGGER_RISING,
+				     "TE_GPIO", par);
+		if (rc) {
+			pr_err("TE request_irq failed.\n");
+			devm_gpiod_put(dev, par->gpio.te);
+			par->gpio.te = NULL;
+		} else {
+			disable_irq_nosync(gpiod_to_irq(par->gpio.te));
+			pr_info("TE request_irq completion.\n");
+		}
+	} else {
+		pr_info("%s:%d, TE gpio not specified\n",
+			__func__, __LINE__);
+	}
 	/* turn off sleep mode */
 	write_reg(par, MIPI_DCS_EXIT_SLEEP_MODE);
 	mdelay(120);
@@ -137,6 +189,9 @@ static int init_display(struct fbtft_par *par)
 	 */
 	write_reg(par, PWCTRL1, 0xA4, 0xA1);
 
+    /*Tearing Effect Line On*/
+	if (par->gpio.te)
+		write_reg(par, 0x35, 0x00);
 	write_reg(par, MIPI_DCS_SET_DISPLAY_ON);
 
 	if (HSD20_IPS)
@@ -145,6 +200,76 @@ static int init_display(struct fbtft_par *par)
 	return 0;
 }
 
+/*****************************************************************************
+ *
+ *   int (*write_vmem)(struct fbtft_par *par);
+ *
+ *****************************************************************************/
+
+/* 16 bit pixel over 8-bit databus */
+static int st7789v_write_vmem16_bus8(struct fbtft_par *par, size_t offset, size_t len)
+{
+	u16 *vmem16;
+	__be16 *txbuf16 = par->txbuf.buf;
+	size_t remain;
+	size_t to_copy;
+	size_t tx_array_size;
+	int i;
+	int ret = 0;
+	size_t startbyte_size = 0;
+
+	fbtft_par_dbg(DEBUG_WRITE_VMEM, par, "st7789v ---%s(offset=%zu, len=%zu)\n",
+		      __func__, offset, len);
+
+	remain = len / 2;
+	vmem16 = (u16 *)(par->info->screen_buffer + offset);
+
+	if (par->gpio.dc)
+		gpiod_set_value(par->gpio.dc, 1);
+
+	/* non buffered write */
+	if (!par->txbuf.buf)
+		return par->fbtftops.write(par, vmem16, len);
+
+	/* buffered write */
+	tx_array_size = par->txbuf.len / 2;
+
+	if (par->startbyte) {
+		txbuf16 = par->txbuf.buf + 1;
+		tx_array_size -= 2;
+		*(u8 *)(par->txbuf.buf) = par->startbyte | 0x2;
+		startbyte_size = 1;
+	}
+
+	while (remain) {
+		to_copy = min(tx_array_size, remain);
+		dev_dbg(par->info->device, "    to_copy=%zu, remain=%zu\n",
+			to_copy, remain - to_copy);
+
+		for (i = 0; i < to_copy; i++)
+			txbuf16[i] = cpu_to_be16(vmem16[i]);
+
+		vmem16 = vmem16 + to_copy;
+		if (par->gpio.te) {
+			set_spi_panel_te_irq_status(par, true);
+			reinit_completion(&spi_panel_te);
+			ret = wait_for_completion_timeout(&spi_panel_te,
+							  msecs_to_jiffies(SPI_PANEL_TE_TIMEOUT));
+			if (ret == 0)
+				pr_err("wait panel TE time out\n");
+		}
+		ret = par->fbtftops.write(par, par->txbuf.buf,
+					 startbyte_size + to_copy * 2);
+		if (par->gpio.te)
+			set_spi_panel_te_irq_status(par, false);
+		if (ret < 0)
+			return ret;
+		remain -= to_copy;
+	}
+
+	return ret;
+}
+
 /**
  * set_var() - apply LCD properties like rotation and BGR mode
  *
@@ -259,6 +384,7 @@ static int blank(struct fbtft_par *par, bool on)
 	.gamma = HSD20_IPS_GAMMA,
 	.fbtftops = {
 		.init_display = init_display,
+		.write_vmem = st7789v_write_vmem16_bus8,
 		.set_var = set_var,
 		.set_gamma = set_gamma,
 		.blank = blank,
diff --git a/drivers/staging/fbtft/fbtft.h b/drivers/staging/fbtft/fbtft.h
index 76f8c09..93bac05 100644
--- a/drivers/staging/fbtft/fbtft.h
+++ b/drivers/staging/fbtft/fbtft.h
@@ -212,6 +212,7 @@ struct fbtft_par {
 		struct gpio_desc *wr;
 		struct gpio_desc *latch;
 		struct gpio_desc *cs;
+		struct gpio_desc *te;
 		struct gpio_desc *db[16];
 		struct gpio_desc *led[16];
 		struct gpio_desc *aux[16];
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v5] fbtft: add tearing signal detect
  2021-01-27  1:32 [PATCH v5] fbtft: add tearing signal detect Carlis
@ 2021-01-27  5:45 ` Dan Carpenter
  2021-01-27  6:19   ` carlis
  2021-01-27  6:36 ` Dan Carpenter
  1 sibling, 1 reply; 6+ messages in thread
From: Dan Carpenter @ 2021-01-27  5:45 UTC (permalink / raw)
  To: Carlis
  Cc: gregkh, devel, linux-fbdev, mh12gx2825, oliver.graute,
	linux-kernel, dri-devel, sbrivio, colin.king, zhangxuezhi1

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


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v5] fbtft: add tearing signal detect
  2021-01-27  5:45 ` Dan Carpenter
@ 2021-01-27  6:19   ` carlis
  2021-01-27  6:49     ` Dan Carpenter
  0 siblings, 1 reply; 6+ messages in thread
From: carlis @ 2021-01-27  6:19 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: gregkh, devel, linux-fbdev, mh12gx2825, oliver.graute,
	linux-kernel, dri-devel, sbrivio, colin.king, zhangxuezhi1

On Wed, 27 Jan 2021 08:45:23 +0300
Dan Carpenter <dan.carpenter@oracle.com> 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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v5] fbtft: add tearing signal detect
  2021-01-27  1:32 [PATCH v5] fbtft: add tearing signal detect Carlis
  2021-01-27  5:45 ` Dan Carpenter
@ 2021-01-27  6:36 ` Dan Carpenter
  1 sibling, 0 replies; 6+ messages in thread
From: Dan Carpenter @ 2021-01-27  6:36 UTC (permalink / raw)
  To: Carlis
  Cc: gregkh, devel, linux-fbdev, mh12gx2825, oliver.graute,
	linux-kernel, dri-devel, sbrivio, colin.king, zhangxuezhi1

On Wed, Jan 27, 2021 at 09:32:20AM +0800, Carlis wrote:
> @@ -82,6 +111,29 @@ enum st7789v_command {
>   */
>  static int init_display(struct fbtft_par *par)
>  {
> +	int rc;
> +	struct device *dev = par->info->device;
> +
> +	par->gpio.te = devm_gpiod_get_index_optional(dev, "te", 0, GPIOD_IN);
> +	if (par->gpio.te) {
> +		init_completion(&spi_panel_te);
> +		mutex_init(&te_mutex);
> +		rc = devm_request_irq(dev,
> +				      gpiod_to_irq(par->gpio.te),
> +				     spi_panel_te_handler, IRQF_TRIGGER_RISING,
> +				     "TE_GPIO", par);
> +		if (rc) {
> +			pr_err("TE request_irq failed.\n");
> +			devm_gpiod_put(dev, par->gpio.te);
> +			par->gpio.te = NULL;
> +		} else {
> +			disable_irq_nosync(gpiod_to_irq(par->gpio.te));
> +			pr_info("TE request_irq completion.\n");


This printk adds no value.

Don't print anything on the success path.  You should add that to your
code while your debugging the feature, but don't push it to the upstream
kernel.

> +		}
> +	} else {
> +		pr_info("%s:%d, TE gpio not specified\n",
> +			__func__, __LINE__);
> +	}

regards,
dan carpenter


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v5] fbtft: add tearing signal detect
  2021-01-27  6:19   ` carlis
@ 2021-01-27  6:49     ` Dan Carpenter
  2021-01-27  8:48       ` Geert Uytterhoeven
  0 siblings, 1 reply; 6+ messages in thread
From: Dan Carpenter @ 2021-01-27  6:49 UTC (permalink / raw)
  To: carlis
  Cc: devel, linux-fbdev, mh12gx2825, gregkh, oliver.graute,
	linux-kernel, dri-devel, sbrivio, colin.king, zhangxuezhi1

On Wed, Jan 27, 2021 at 02:19:27PM +0800, carlis wrote:
> hi,i will fix it like below:
> 	par->gpio.te = devm_gpiod_get_index_optional(dev, "te", 0,
> GPIOD_IN); if (IS_ERR(par->gpio.te)) {
> 		rc = PTR_ERR(par->gpio.te);
> 		pr_err("Failed to request  te gpio: %d\n", rc);
> 		par->gpio.te = NULL;
> 	}

I wish you would just copy and paste the code that I sent you instead,
but this also fixes the crash...

regards,
dan carpenter



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v5] fbtft: add tearing signal detect
  2021-01-27  6:49     ` Dan Carpenter
@ 2021-01-27  8:48       ` Geert Uytterhoeven
  0 siblings, 0 replies; 6+ messages in thread
From: Geert Uytterhoeven @ 2021-01-27  8:48 UTC (permalink / raw)
  To: Dan Carpenter, carlis
  Cc: driverdevel, Linux Fbdev development list, mh12gx2825, Greg KH,
	oliver.graute, Linux Kernel Mailing List, DRI Development,
	Stefano Brivio, Colin King, zhangxuezhi1

Hi Dan, Carlis,

On Wed, Jan 27, 2021 at 9:32 AM Dan Carpenter <dan.carpenter@oracle.com> wrote:
> On Wed, Jan 27, 2021 at 02:19:27PM +0800, carlis wrote:
> > hi,i will fix it like below:
> >       par->gpio.te = devm_gpiod_get_index_optional(dev, "te", 0,
> > GPIOD_IN); if (IS_ERR(par->gpio.te)) {
> >               rc = PTR_ERR(par->gpio.te);
> >               pr_err("Failed to request  te gpio: %d\n", rc);
> >               par->gpio.te = NULL;
> >       }
>
> I wish you would just copy and paste the code that I sent you instead,
> but this also fixes the crash...

While this fixes the crash, it does not propagate the error condition
(which may be -EPROBE_DEFER) upstream.
Same for devm_request_irq().

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2021-01-27  8:52 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-27  1:32 [PATCH v5] fbtft: add tearing signal detect Carlis
2021-01-27  5:45 ` Dan Carpenter
2021-01-27  6:19   ` carlis
2021-01-27  6:49     ` Dan Carpenter
2021-01-27  8:48       ` Geert Uytterhoeven
2021-01-27  6:36 ` Dan Carpenter

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).