[PATCH 0/9] drm/exynos/decon5433: move TE handling to DECON

[PATCH 0/9] drm/exynos/decon5433: move TE handling to DECON

[Andrzej Hajda]

Hi Inki,

This patchset contains fixes and improvements related to SW-TRIGGER.
The core patch moves all TE related stuff from panel(bindings) and
HDMI(irq handler) to DECON. This move allows to fix races between interrupt
handlers and DECON disable. It allows also to eliminate all DECON flags and
related checks in subsequent patches. Additionally SW-TRIGGER can be enabled
by adding TE interrupt to DECON dts node.
Another important fix is uncoditional SW-TRIGGER during enabled VBLANKs, this
way no VBLANK timeouts happens when drm-core or userspace waits for VBLANK.

First 5 patches could be treated as fixes, but since they depends on my previous
work which is not yet merged, I have not separated it.

As I said before the patchset depends on my vblank patchset[1] and recent fix
of hdmi pipeline disable order[2].

[1]: https://marc.info/?l=dri-devel&m=148958888524665
[2]: https://marc.info/?l=dri-devel&m=149137560223242

Regards
Andrzej


Andrzej Hajda (9):
  drm/exynos/decon5433: always do sw-trigger when vblanks enabled
  dt-bindings: exynos5433-decon: fix interrupts bindings
  dt-bindings: exynos5433-decon: add TE interrupt binding
  drm/exynos/decon5433: move TE handling to DECON
  drm/exynos/decon5433: kill BIT_IRQS_ENABLED flag
  drm/exynos/decon5433: kill BIT_CLKS_ENABLED flag
  drm/exynos/decon5433: kill BIT_WIN_UPDATED flag
  drm/exynos/decon5433: kill BIT_SUSPENDED flag
  drm/exynos/decon5433: remove useless check

 .../bindings/display/exynos/exynos5433-decon.txt   |  13 +-
 drivers/gpu/drm/exynos/exynos5433_drm_decon.c      | 143 ++++++++++-----------
 2 files changed, 74 insertions(+), 82 deletions(-)

-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 1/9] drm/exynos/decon5433: always do sw-trigger when vblanks enabled

[Andrzej Hajda]

When vblanks are enabled userspace and/or kernel can expect vblank
interrupt at declared period of time. To generate vblank interrupt
image transfer must be triggered. This patch fixes vblank timeouts
in case of sw-trigger mode.

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
 drivers/gpu/drm/exynos/exynos5433_drm_decon.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
index 42e8f8c..028a657 100644
--- a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
+++ b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
@@ -49,6 +49,7 @@ static const char * const decon_clks_name[] = {
 
 enum decon_flag_bits {
 	BIT_CLKS_ENABLED,
+	BIT_IRQS_ENABLED,
 	BIT_WIN_UPDATED,
 	BIT_SUSPENDED
 };
@@ -104,6 +105,7 @@ static int decon_enable_vblank(struct exynos_drm_crtc *crtc)
 		val |= VIDINTCON0_INTFRMEN | VIDINTCON0_FRAMESEL_FP;
 
 	writel(val, ctx->addr + DECON_VIDINTCON0);
+	set_bit(BIT_IRQS_ENABLED, &ctx->flags);
 
 	return 0;
 }
@@ -112,6 +114,7 @@ static void decon_disable_vblank(struct exynos_drm_crtc *crtc)
 {
 	struct decon_context *ctx = crtc->ctx;
 
+	clear_bit(BIT_IRQS_ENABLED, &ctx->flags);
 	if (test_bit(BIT_SUSPENDED, &ctx->flags))
 		return;
 
@@ -518,7 +521,8 @@ static void decon_te_irq_handler(struct exynos_drm_crtc *crtc)
 	    (ctx->out_type & I80_HW_TRG))
 		return;
 
-	if (test_and_clear_bit(BIT_WIN_UPDATED, &ctx->flags))
+	if (test_and_clear_bit(BIT_WIN_UPDATED, &ctx->flags) ||
+	    test_bit(BIT_IRQS_ENABLED, &ctx->flags))
 		decon_set_bits(ctx, DECON_TRIGCON, TRIGCON_SWTRIGCMD, ~0);
 }
 
-- 
2.7.4

[PATCH 2/9] dt-bindings: exynos5433-decon: fix interrupts bindings

[Andrzej Hajda]

DECON requires different interrupts depending on mode of work, which
depends on panel it is connected to.

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
 .../devicetree/bindings/display/exynos/exynos5433-decon.txt  | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/Documentation/devicetree/bindings/display/exynos/exynos5433-decon.txt b/Documentation/devicetree/bindings/display/exynos/exynos5433-decon.txt
index c9fd7b3..ba6be99 100644
--- a/Documentation/devicetree/bindings/display/exynos/exynos5433-decon.txt
+++ b/Documentation/devicetree/bindings/display/exynos/exynos5433-decon.txt
@@ -8,12 +8,12 @@ Required properties:
 - compatible: value should be one of:
 	"samsung,exynos5433-decon", "samsung,exynos5433-decon-tv";
 - reg: physical base address and length of the DECON registers set.
-- interrupts: should contain a list of all DECON IP block interrupts in the
-	      order: VSYNC, LCD_SYSTEM. The interrupt specifier format
-	      depends on the interrupt controller used.
-- interrupt-names: should contain the interrupt names: "vsync", "lcd_sys"
-		   in the same order as they were listed in the interrupts
-		   property.
+- interrupt-names: should contain the interrupt names depending on mode of work:
+		video mode: "vsync",
+		command mode: "lcd_sys".
+- interrupts or interrupts-extended: list of interrupt specifiers corresponding
+		to names privided in interrupt-names, as described in
+		interrupt-controller/interrupts.txt
 - clocks: must include clock specifiers corresponding to entries in the
 	  clock-names property.
 - clock-names: list of clock names sorted in the same order as the clocks
-- 
2.7.4

[PATCH 3/9] dt-bindings: exynos5433-decon: add TE interrupt binding

[Andrzej Hajda]

DECON command mode can use hardware trigger where transmission
is triggered automatically, or software trigger - where TE interrupt
handler should trigger transmission.
DECON will use software trigger if TE interrupt is specified.

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
 Documentation/devicetree/bindings/display/exynos/exynos5433-decon.txt | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/display/exynos/exynos5433-decon.txt b/Documentation/devicetree/bindings/display/exynos/exynos5433-decon.txt
index ba6be99..549c538 100644
--- a/Documentation/devicetree/bindings/display/exynos/exynos5433-decon.txt
+++ b/Documentation/devicetree/bindings/display/exynos/exynos5433-decon.txt
@@ -10,7 +10,8 @@ Required properties:
 - reg: physical base address and length of the DECON registers set.
 - interrupt-names: should contain the interrupt names depending on mode of work:
 		video mode: "vsync",
-		command mode: "lcd_sys".
+		command mode: "lcd_sys",
+		command mode with software trigger: "lcd_sys", "te".
 - interrupts or interrupts-extended: list of interrupt specifiers corresponding
 		to names privided in interrupt-names, as described in
 		interrupt-controller/interrupts.txt
-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 4/9] drm/exynos/decon5433: move TE handling to DECON

[Andrzej Hajda]

DECON is the only user of TE signal, moving all TE related
code to DECON driver allows to precise control of IRQ handlers.
This control allows to fix race between IRQ handler and DECON disable
code - now it is possible to disable DECON during IRQ handling
which can result in kernel crash. Beside race fixing this change
allows further code simplification.

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
 drivers/gpu/drm/exynos/exynos5433_drm_decon.c | 94 ++++++++++++++++++++-------
 1 file changed, 71 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
index 028a657..5bdf1a0 100644
--- a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
+++ b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
@@ -63,6 +63,8 @@ struct decon_context {
 	void __iomem			*addr;
 	struct regmap			*sysreg;
 	struct clk			*clks[ARRAY_SIZE(decon_clks_name)];
+	unsigned int			irq;
+	unsigned int			te_irq;
 	unsigned long			flags;
 	unsigned long			out_type;
 	int				first_win;
@@ -105,6 +107,11 @@ static int decon_enable_vblank(struct exynos_drm_crtc *crtc)
 		val |= VIDINTCON0_INTFRMEN | VIDINTCON0_FRAMESEL_FP;
 
 	writel(val, ctx->addr + DECON_VIDINTCON0);
+
+	enable_irq(ctx->irq);
+	if (!(ctx->out_type & I80_HW_TRG))
+		enable_irq(ctx->te_irq);
+
 	set_bit(BIT_IRQS_ENABLED, &ctx->flags);
 
 	return 0;
@@ -118,6 +125,10 @@ static void decon_disable_vblank(struct exynos_drm_crtc *crtc)
 	if (test_bit(BIT_SUSPENDED, &ctx->flags))
 		return;
 
+	if (!(ctx->out_type & I80_HW_TRG))
+		disable_irq_nosync(ctx->te_irq);
+	disable_irq_nosync(ctx->irq);
+
 	writel(0, ctx->addr + DECON_VIDINTCON0);
 }
 
@@ -491,6 +502,10 @@ static void decon_disable(struct exynos_drm_crtc *crtc)
 	struct decon_context *ctx = crtc->ctx;
 	int i;
 
+	if (!(ctx->out_type & I80_HW_TRG))
+		synchronize_irq(ctx->te_irq);
+	synchronize_irq(ctx->irq);
+
 	if (test_bit(BIT_SUSPENDED, &ctx->flags))
 		return;
 
@@ -513,17 +528,19 @@ static void decon_disable(struct exynos_drm_crtc *crtc)
 	set_bit(BIT_SUSPENDED, &ctx->flags);
 }
 
-static void decon_te_irq_handler(struct exynos_drm_crtc *crtc)
+static irqreturn_t decon_te_irq_handler(int irq, void *dev_id)
 {
-	struct decon_context *ctx = crtc->ctx;
+	struct decon_context *ctx = dev_id;
 
 	if (!test_bit(BIT_CLKS_ENABLED, &ctx->flags) ||
 	    (ctx->out_type & I80_HW_TRG))
-		return;
+		return IRQ_HANDLED;
 
 	if (test_and_clear_bit(BIT_WIN_UPDATED, &ctx->flags) ||
 	    test_bit(BIT_IRQS_ENABLED, &ctx->flags))
 		decon_set_bits(ctx, DECON_TRIGCON, TRIGCON_SWTRIGCMD, ~0);
+
+	return IRQ_HANDLED;
 }
 
 static void decon_clear_channels(struct exynos_drm_crtc *crtc)
@@ -564,7 +581,6 @@ static const struct exynos_drm_crtc_ops decon_crtc_ops = {
 	.update_plane		= decon_update_plane,
 	.disable_plane		= decon_disable_plane,
 	.atomic_flush		= decon_atomic_flush,
-	.te_handler		= decon_te_irq_handler,
 };
 
 static int decon_bind(struct device *dev, struct device *master, void *data)
@@ -717,6 +733,31 @@ static const struct of_device_id exynos5433_decon_driver_dt_match[] = {
 };
 MODULE_DEVICE_TABLE(of, exynos5433_decon_driver_dt_match);
 
+static int decon_conf_irq(struct decon_context *ctx, const char *name,
+		irq_handler_t handler, unsigned long int flags, bool required)
+{
+	struct platform_device *pdev = to_platform_device(ctx->dev);
+	int ret, irq = platform_get_irq_byname(pdev, name);
+
+	if (irq < 0) {
+		if (irq == -EPROBE_DEFER)
+			return irq;
+		if (required)
+			dev_err(ctx->dev, "cannot get %s IRQ\n", name);
+		else
+			irq = 0;
+		return irq;
+	}
+	irq_set_status_flags(irq, IRQ_NOAUTOEN);
+	ret = devm_request_irq(ctx->dev, irq, handler, flags, "drm_decon", ctx);
+	if (ret < 0) {
+		dev_err(ctx->dev, "IRQ %s request failed\n", name);
+		return ret;
+	}
+
+	return irq;
+}
+
 static int exynos5433_decon_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -740,15 +781,6 @@ static int exynos5433_decon_probe(struct platform_device *pdev)
 		ctx->out_type |= IFTYPE_I80;
 	}
 
-	if (ctx->out_type & I80_HW_TRG) {
-		ctx->sysreg = syscon_regmap_lookup_by_phandle(dev->of_node,
-							"samsung,disp-sysreg");
-		if (IS_ERR(ctx->sysreg)) {
-			dev_err(dev, "failed to get system register\n");
-			return PTR_ERR(ctx->sysreg);
-		}
-	}
-
 	for (i = 0; i < ARRAY_SIZE(decon_clks_name); i++) {
 		struct clk *clk;
 
@@ -771,18 +803,34 @@ static int exynos5433_decon_probe(struct platform_device *pdev)
 		return PTR_ERR(ctx->addr);
 	}
 
-	res = platform_get_resource_byname(pdev, IORESOURCE_IRQ,
-			(ctx->out_type & IFTYPE_I80) ? "lcd_sys" : "vsync");
-	if (!res) {
-		dev_err(dev, "cannot find IRQ resource\n");
-		return -ENXIO;
+	if (ctx->out_type & IFTYPE_I80) {
+		ret = decon_conf_irq(ctx, "lcd_sys", decon_irq_handler, 0, true);
+		if (ret < 0)
+			return ret;
+		ctx->irq = ret;
+
+		ret = decon_conf_irq(ctx, "te", decon_te_irq_handler,
+				     IRQF_TRIGGER_RISING, false);
+		if (ret < 0)
+			return ret;
+		if (ret) {
+			ctx->te_irq = ret;
+			ctx->out_type &= ~I80_HW_TRG;
+		}
+	} else {
+		ret = decon_conf_irq(ctx, "vsync", decon_irq_handler, 0, true);
+		if (ret < 0)
+			return ret;
+		ctx->irq = ret;
 	}
 
-	ret = devm_request_irq(dev, res->start, decon_irq_handler, 0,
-			       "drm_decon", ctx);
-	if (ret < 0) {
-		dev_err(dev, "lcd_sys irq request failed\n");
-		return ret;
+	if (ctx->out_type & I80_HW_TRG) {
+		ctx->sysreg = syscon_regmap_lookup_by_phandle(dev->of_node,
+							"samsung,disp-sysreg");
+		if (IS_ERR(ctx->sysreg)) {
+			dev_err(dev, "failed to get system register\n");
+			return PTR_ERR(ctx->sysreg);
+		}
 	}
 
 	platform_set_drvdata(pdev, ctx);
-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 5/9] drm/exynos/decon5433: kill BIT_IRQS_ENABLED flag

[Andrzej Hajda]

Since DECON uses enable_irq/disable_irq to full control IRQs,
there is no point in having flags to trace it separately.
As a bonus condition for software trigger becomes always true,
so it can be removed.

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
 drivers/gpu/drm/exynos/exynos5433_drm_decon.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
index 5bdf1a0..dc2e69a 100644
--- a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
+++ b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
@@ -49,7 +49,6 @@ static const char * const decon_clks_name[] = {
 
 enum decon_flag_bits {
 	BIT_CLKS_ENABLED,
-	BIT_IRQS_ENABLED,
 	BIT_WIN_UPDATED,
 	BIT_SUSPENDED
 };
@@ -112,8 +111,6 @@ static int decon_enable_vblank(struct exynos_drm_crtc *crtc)
 	if (!(ctx->out_type & I80_HW_TRG))
 		enable_irq(ctx->te_irq);
 
-	set_bit(BIT_IRQS_ENABLED, &ctx->flags);
-
 	return 0;
 }
 
@@ -121,7 +118,6 @@ static void decon_disable_vblank(struct exynos_drm_crtc *crtc)
 {
 	struct decon_context *ctx = crtc->ctx;
 
-	clear_bit(BIT_IRQS_ENABLED, &ctx->flags);
 	if (test_bit(BIT_SUSPENDED, &ctx->flags))
 		return;
 
@@ -536,9 +532,7 @@ static irqreturn_t decon_te_irq_handler(int irq, void *dev_id)
 	    (ctx->out_type & I80_HW_TRG))
 		return IRQ_HANDLED;
 
-	if (test_and_clear_bit(BIT_WIN_UPDATED, &ctx->flags) ||
-	    test_bit(BIT_IRQS_ENABLED, &ctx->flags))
-		decon_set_bits(ctx, DECON_TRIGCON, TRIGCON_SWTRIGCMD, ~0);
+	decon_set_bits(ctx, DECON_TRIGCON, TRIGCON_SWTRIGCMD, ~0);
 
 	return IRQ_HANDLED;
 }
-- 
2.7.4

[PATCH 6/9] drm/exynos/decon5433: kill BIT_CLKS_ENABLED flag

[Andrzej Hajda]

The flag was used to check if IRQ handlers can touch HW. Since driver
enables IRQs only if hardware is enabled the flag becomes redundant.

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
 drivers/gpu/drm/exynos/exynos5433_drm_decon.c | 12 +-----------
 1 file changed, 1 insertion(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
index dc2e69a..2629a59 100644
--- a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
+++ b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
@@ -48,7 +48,6 @@ static const char * const decon_clks_name[] = {
 };
 
 enum decon_flag_bits {
-	BIT_CLKS_ENABLED,
 	BIT_WIN_UPDATED,
 	BIT_SUSPENDED
 };
@@ -486,8 +485,6 @@ static void decon_enable(struct exynos_drm_crtc *crtc)
 
 	exynos_drm_pipe_clk_enable(crtc, true);
 
-	set_bit(BIT_CLKS_ENABLED, &ctx->flags);
-
 	decon_swreset(ctx);
 
 	decon_commit(ctx->crtc);
@@ -515,8 +512,6 @@ static void decon_disable(struct exynos_drm_crtc *crtc)
 
 	decon_swreset(ctx);
 
-	clear_bit(BIT_CLKS_ENABLED, &ctx->flags);
-
 	exynos_drm_pipe_clk_enable(crtc, false);
 
 	pm_runtime_put_sync(ctx->dev);
@@ -528,8 +523,7 @@ static irqreturn_t decon_te_irq_handler(int irq, void *dev_id)
 {
 	struct decon_context *ctx = dev_id;
 
-	if (!test_bit(BIT_CLKS_ENABLED, &ctx->flags) ||
-	    (ctx->out_type & I80_HW_TRG))
+	if (ctx->out_type & I80_HW_TRG)
 		return IRQ_HANDLED;
 
 	decon_set_bits(ctx, DECON_TRIGCON, TRIGCON_SWTRIGCMD, ~0);
@@ -654,9 +648,6 @@ static irqreturn_t decon_irq_handler(int irq, void *dev_id)
 	struct decon_context *ctx = dev_id;
 	u32 val;
 
-	if (!test_bit(BIT_CLKS_ENABLED, &ctx->flags))
-		goto out;
-
 	val = readl(ctx->addr + DECON_VIDINTCON1);
 	val &= VIDINTCON1_INTFRMDONEPEND | VIDINTCON1_INTFRMPEND;
 
@@ -672,7 +663,6 @@ static irqreturn_t decon_irq_handler(int irq, void *dev_id)
 		decon_handle_vblank(ctx);
 	}
 
-out:
 	return IRQ_HANDLED;
 }
 
-- 
2.7.4

[PATCH 7/9] drm/exynos/decon5433: kill BIT_WIN_UPDATED flag

[Andrzej Hajda]

The flag was used to trigger software update in TE IRQ handler only
if framebuffers were replaced. Since TE update is triggered always
when VBLANKs are enabled and after framebuffer replacement VBLANKs
are always enabled the flag becomes redundant.

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
 drivers/gpu/drm/exynos/exynos5433_drm_decon.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
index 2629a59..b81c12b 100644
--- a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
+++ b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
@@ -48,7 +48,6 @@ static const char * const decon_clks_name[] = {
 };
 
 enum decon_flag_bits {
-	BIT_WIN_UPDATED,
 	BIT_SUSPENDED
 };
 
@@ -428,9 +427,6 @@ static void decon_atomic_flush(struct exynos_drm_crtc *crtc)
 
 	decon_set_bits(ctx, DECON_UPDATE, STANDALONE_UPDATE_F, ~0);
 
-	if (ctx->out_type & IFTYPE_I80)
-		set_bit(BIT_WIN_UPDATED, &ctx->flags);
-
 	ctx->frame_id = decon_get_frame_count(ctx, true);
 
 	exynos_crtc_handle_event(crtc);
-- 
2.7.4

[PATCH 8/9] drm/exynos/decon5433: kill BIT_SUSPENDED flag

[Andrzej Hajda]

Exynos tracked suspend state to prevent touching disabled HW. After
fixing disable order in HDMI and moving TE handling to DECON it is
not needed anymore - all IRQ handlers and callbacks touching HW
are called only with enabled DECON.

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
 drivers/gpu/drm/exynos/exynos5433_drm_decon.c | 38 ---------------------------
 1 file changed, 38 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
index b81c12b..1cb4b86 100644
--- a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
+++ b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
@@ -47,10 +47,6 @@ static const char * const decon_clks_name[] = {
 	"sclk_decon_eclk",
 };
 
-enum decon_flag_bits {
-	BIT_SUSPENDED
-};
-
 struct decon_context {
 	struct device			*dev;
 	struct drm_device		*drm_dev;
@@ -62,7 +58,6 @@ struct decon_context {
 	struct clk			*clks[ARRAY_SIZE(decon_clks_name)];
 	unsigned int			irq;
 	unsigned int			te_irq;
-	unsigned long			flags;
 	unsigned long			out_type;
 	int				first_win;
 	spinlock_t			vblank_lock;
@@ -94,9 +89,6 @@ static int decon_enable_vblank(struct exynos_drm_crtc *crtc)
 	struct decon_context *ctx = crtc->ctx;
 	u32 val;
 
-	if (test_bit(BIT_SUSPENDED, &ctx->flags))
-		return -EPERM;
-
 	val = VIDINTCON0_INTEN;
 	if (ctx->out_type & IFTYPE_I80)
 		val |= VIDINTCON0_FRAMEDONE;
@@ -116,9 +108,6 @@ static void decon_disable_vblank(struct exynos_drm_crtc *crtc)
 {
 	struct decon_context *ctx = crtc->ctx;
 
-	if (test_bit(BIT_SUSPENDED, &ctx->flags))
-		return;
-
 	if (!(ctx->out_type & I80_HW_TRG))
 		disable_irq_nosync(ctx->te_irq);
 	disable_irq_nosync(ctx->irq);
@@ -172,9 +161,6 @@ static u32 decon_get_vblank_counter(struct exynos_drm_crtc *crtc)
 {
 	struct decon_context *ctx = crtc->ctx;
 
-	if (test_bit(BIT_SUSPENDED, &ctx->flags))
-		return 0;
-
 	return decon_get_frame_count(ctx, false);
 }
 
@@ -205,9 +191,6 @@ static void decon_commit(struct exynos_drm_crtc *crtc)
 	bool interlaced = false;
 	u32 val;
 
-	if (test_bit(BIT_SUSPENDED, &ctx->flags))
-		return;
-
 	if (ctx->out_type & IFTYPE_HDMI) {
 		m->crtc_hsync_start = m->crtc_hdisplay + 10;
 		m->crtc_hsync_end = m->crtc_htotal - 92;
@@ -331,9 +314,6 @@ static void decon_atomic_begin(struct exynos_drm_crtc *crtc)
 {
 	struct decon_context *ctx = crtc->ctx;
 
-	if (test_bit(BIT_SUSPENDED, &ctx->flags))
-		return;
-
 	decon_shadow_protect(ctx, true);
 }
 
@@ -354,9 +334,6 @@ static void decon_update_plane(struct exynos_drm_crtc *crtc,
 	dma_addr_t dma_addr = exynos_drm_fb_dma_addr(fb, 0);
 	u32 val;
 
-	if (test_bit(BIT_SUSPENDED, &ctx->flags))
-		return;
-
 	if (crtc->base.mode.flags & DRM_MODE_FLAG_INTERLACE) {
 		val = COORDINATE_X(state->crtc.x) |
 			COORDINATE_Y(state->crtc.y / 2);
@@ -407,9 +384,6 @@ static void decon_disable_plane(struct exynos_drm_crtc *crtc,
 	struct decon_context *ctx = crtc->ctx;
 	unsigned int win = plane->index;
 
-	if (test_bit(BIT_SUSPENDED, &ctx->flags))
-		return;
-
 	decon_set_bits(ctx, DECON_WINCONx(win), WINCONx_ENWIN_F, 0);
 }
 
@@ -418,9 +392,6 @@ static void decon_atomic_flush(struct exynos_drm_crtc *crtc)
 	struct decon_context *ctx = crtc->ctx;
 	unsigned long flags;
 
-	if (test_bit(BIT_SUSPENDED, &ctx->flags))
-		return;
-
 	spin_lock_irqsave(&ctx->vblank_lock, flags);
 
 	decon_shadow_protect(ctx, false);
@@ -474,9 +445,6 @@ static void decon_enable(struct exynos_drm_crtc *crtc)
 {
 	struct decon_context *ctx = crtc->ctx;
 
-	if (!test_and_clear_bit(BIT_SUSPENDED, &ctx->flags))
-		return;
-
 	pm_runtime_get_sync(ctx->dev);
 
 	exynos_drm_pipe_clk_enable(crtc, true);
@@ -495,9 +463,6 @@ static void decon_disable(struct exynos_drm_crtc *crtc)
 		synchronize_irq(ctx->te_irq);
 	synchronize_irq(ctx->irq);
 
-	if (test_bit(BIT_SUSPENDED, &ctx->flags))
-		return;
-
 	/*
 	 * We need to make sure that all windows are disabled before we
 	 * suspend that connector. Otherwise we might try to scan from
@@ -511,8 +476,6 @@ static void decon_disable(struct exynos_drm_crtc *crtc)
 	exynos_drm_pipe_clk_enable(crtc, false);
 
 	pm_runtime_put_sync(ctx->dev);
-
-	set_bit(BIT_SUSPENDED, &ctx->flags);
 }
 
 static irqreturn_t decon_te_irq_handler(int irq, void *dev_id)
@@ -750,7 +713,6 @@ static int exynos5433_decon_probe(struct platform_device *pdev)
 	if (!ctx)
 		return -ENOMEM;
 
-	__set_bit(BIT_SUSPENDED, &ctx->flags);
 	ctx->dev = dev;
 	ctx->out_type = (unsigned long)of_device_get_match_data(dev);
 	spin_lock_init(&ctx->vblank_lock);
-- 
2.7.4

[PATCH 9/9] drm/exynos/decon5433: remove useless check

[Andrzej Hajda]

TE IRQ is enabled only in case of sw-trigger, so trigger check
is redundant.

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
 drivers/gpu/drm/exynos/exynos5433_drm_decon.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
index 1cb4b86..5792ca88 100644
--- a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
+++ b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
@@ -482,9 +482,6 @@ static irqreturn_t decon_te_irq_handler(int irq, void *dev_id)
 {
 	struct decon_context *ctx = dev_id;
 
-	if (ctx->out_type & I80_HW_TRG)
-		return IRQ_HANDLED;
-
 	decon_set_bits(ctx, DECON_TRIGCON, TRIGCON_SWTRIGCMD, ~0);
 
 	return IRQ_HANDLED;
-- 
2.7.4

Re: [PATCH 5/9] drm/exynos/decon5433: kill BIT_IRQS_ENABLED flag

[Inki Dae]

2017년 04월 05일 16:28에 Andrzej Hajda 이(가) 쓴 글:
> Since DECON uses enable_irq/disable_irq to full control IRQs,
> there is no point in having flags to trace it separately.
> As a bonus condition for software trigger becomes always true,
> so it can be removed.
> 
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> ---
>  drivers/gpu/drm/exynos/exynos5433_drm_decon.c | 8 +-------
>  1 file changed, 1 insertion(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
> index 5bdf1a0..dc2e69a 100644
> --- a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
> +++ b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
> @@ -49,7 +49,6 @@ static const char * const decon_clks_name[] = {
>  
>  enum decon_flag_bits {
>  	BIT_CLKS_ENABLED,
> -	BIT_IRQS_ENABLED,
>  	BIT_WIN_UPDATED,
>  	BIT_SUSPENDED
>  };
> @@ -112,8 +111,6 @@ static int decon_enable_vblank(struct exynos_drm_crtc *crtc)
>  	if (!(ctx->out_type & I80_HW_TRG))
>  		enable_irq(ctx->te_irq);
>  
> -	set_bit(BIT_IRQS_ENABLED, &ctx->flags);
> -
>  	return 0;
>  }
>  
> @@ -121,7 +118,6 @@ static void decon_disable_vblank(struct exynos_drm_crtc *crtc)
>  {
>  	struct decon_context *ctx = crtc->ctx;
>  
> -	clear_bit(BIT_IRQS_ENABLED, &ctx->flags);
>  	if (test_bit(BIT_SUSPENDED, &ctx->flags))
>  		return;
>  
> @@ -536,9 +532,7 @@ static irqreturn_t decon_te_irq_handler(int irq, void *dev_id)
>  	    (ctx->out_type & I80_HW_TRG))
>  		return IRQ_HANDLED;
>  
> -	if (test_and_clear_bit(BIT_WIN_UPDATED, &ctx->flags) ||
> -	    test_bit(BIT_IRQS_ENABLED, &ctx->flags))
> -		decon_set_bits(ctx, DECON_TRIGCON, TRIGCON_SWTRIGCMD, ~0);
> +	decon_set_bits(ctx, DECON_TRIGCON, TRIGCON_SWTRIGCMD, ~0);

This code would incur mulfunction if now decon driver uses sw trigger mode.
The panel device on TM2 and TM2E boards supports ALPM mode which makes Panel device to be keeped on with low power even ARM, crtc and encoder devices are off.
In this case, even if decon device is off te interrupt could happen and writing to decon register could be tried.

Thanks,
Inki Dae

>  
>  	return IRQ_HANDLED;
>  }
> 
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 5/9] drm/exynos/decon5433: kill BIT_IRQS_ENABLED flag

[Andrzej Hajda]

On 13.04.2017 10:33, Inki Dae wrote:
>
> 2017년 04월 05일 16:28에 Andrzej Hajda 이(가) 쓴 글:
>> Since DECON uses enable_irq/disable_irq to full control IRQs,
>> there is no point in having flags to trace it separately.
>> As a bonus condition for software trigger becomes always true,
>> so it can be removed.
>>
>> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
>> ---
>>  drivers/gpu/drm/exynos/exynos5433_drm_decon.c | 8 +-------
>>  1 file changed, 1 insertion(+), 7 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
>> index 5bdf1a0..dc2e69a 100644
>> --- a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
>> +++ b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
>> @@ -49,7 +49,6 @@ static const char * const decon_clks_name[] = {
>>  
>>  enum decon_flag_bits {
>>  	BIT_CLKS_ENABLED,
>> -	BIT_IRQS_ENABLED,
>>  	BIT_WIN_UPDATED,
>>  	BIT_SUSPENDED
>>  };
>> @@ -112,8 +111,6 @@ static int decon_enable_vblank(struct exynos_drm_crtc *crtc)
>>  	if (!(ctx->out_type & I80_HW_TRG))
>>  		enable_irq(ctx->te_irq);
>>  
>> -	set_bit(BIT_IRQS_ENABLED, &ctx->flags);
>> -
>>  	return 0;
>>  }
>>  
>> @@ -121,7 +118,6 @@ static void decon_disable_vblank(struct exynos_drm_crtc *crtc)
>>  {
>>  	struct decon_context *ctx = crtc->ctx;
>>  
>> -	clear_bit(BIT_IRQS_ENABLED, &ctx->flags);
>>  	if (test_bit(BIT_SUSPENDED, &ctx->flags))
>>  		return;
>>  
>> @@ -536,9 +532,7 @@ static irqreturn_t decon_te_irq_handler(int irq, void *dev_id)
>>  	    (ctx->out_type & I80_HW_TRG))
>>  		return IRQ_HANDLED;
>>  
>> -	if (test_and_clear_bit(BIT_WIN_UPDATED, &ctx->flags) ||
>> -	    test_bit(BIT_IRQS_ENABLED, &ctx->flags))
>> -		decon_set_bits(ctx, DECON_TRIGCON, TRIGCON_SWTRIGCMD, ~0);
>> +	decon_set_bits(ctx, DECON_TRIGCON, TRIGCON_SWTRIGCMD, ~0);
> This code would incur mulfunction if now decon driver uses sw trigger mode.
> The panel device on TM2 and TM2E boards supports ALPM mode which makes Panel device to be keeped on with low power even ARM, crtc and encoder devices are off.
> In this case, even if decon device is off te interrupt could happen and writing to decon register could be tried.

As commit message explains it should not happen because of precise
control of irqs enablement.
More precisely patch 4 prevents it - te irq is disabled by
decon_disable_vblank, and synchronized (ie ensured that there are no
pending irqs) in decon_disable.

Regards
Andrzej

>
> Thanks,
> Inki Dae
>
>>  
>>  	return IRQ_HANDLED;
>>  }
>>
>

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel