[PATCH v2 00/12] drm/exynos: rework vblank handling

[PATCH v2 00/12] drm/exynos: rework vblank handling

[Andrzej Hajda]

Hi Inki,

This patchset fixes long standing issue with occassional page faults
or vblank event timeouts on TM2 targets due to delayed vblank handling.
DECON driver should now handle properly all scenarios described in drm
docs [1][2], at least it was my intention.

The patchset also:
- adds frame counter callback,
- fixes VBLANK interrupt trigger time,
- fixes soft-trigger mask,
- removes redundant pipe related fields.

The patch is based on todays linux-next, the real dependency is on the Shawn patchset
'Add vblank hooks to struct drm_crtc_funcs'.

I have successfully tested it on TM2 panel, TV and both.

[1]: https://dri.freedesktop.org/docs/drm/gpu/drm-kms.html?highlight=drm_crtc_state#c.drm_crtc_arm_vblank_event
[2]: https://dri.freedesktop.org/docs/drm/gpu/drm-kms.html?highlight=drm_crtc_state#c.drm_crtc_state

Regards
Andrzej


Andrzej Hajda (12):
  drm/exynos: move crtc event handling to drivers callbacks
  drm/exynos: simplify completion event handling
  drm/exynos/decon5433: fix vblank event handling
  drm/exynos/decon5433: implement frame counter
  drm/exynos/decon5433: signal frame done interrupt at front porch
  drm/exynos/fimd: signal frame done interrupt at front porch
  drm/exynos/decon5433: fix software trigger mask
  drm/exynos: kill exynos_drm_crtc::pipe
  drm/exynos: kill exynos_drm_private::pipe
  drm/exynos: set plane possible_crtcs in exynos_plane_init
  drm/exynos: kill pipe field from drivers contexts
  drm/exynos: kill mode_set_nofb callback

 drivers/gpu/drm/exynos/exynos5433_drm_decon.c | 119 +++++++++++++++++++++-----
 drivers/gpu/drm/exynos/exynos7_drm_decon.c    |  20 ++---
 drivers/gpu/drm/exynos/exynos_drm_crtc.c      |  65 +++++++-------
 drivers/gpu/drm/exynos/exynos_drm_crtc.h      |   3 +-
 drivers/gpu/drm/exynos/exynos_drm_drv.h       |  14 +--
 drivers/gpu/drm/exynos/exynos_drm_fimd.c      |  28 ++----
 drivers/gpu/drm/exynos/exynos_drm_plane.c     |   5 +-
 drivers/gpu/drm/exynos/exynos_drm_plane.h     |   1 -
 drivers/gpu/drm/exynos/exynos_drm_vidi.c      |  23 +----
 drivers/gpu/drm/exynos/exynos_mixer.c         |  14 +--
 include/video/exynos5433_decon.h              |  12 +++
 11 files changed, 166 insertions(+), 138 deletions(-)

-- 
2.7.4

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

[PATCH v2 01/12] drm/exynos: move crtc event handling to drivers callbacks

[Andrzej Hajda]

CRTC event is currently send with next vblank, or instantly in case crtc
is being disabled. This approach usually works, but in corner cases it can
result in premature event generation. Only device driver is able to verify
if the event can be sent. This patch is a first step in that direction - it
moves event handling to the drivers.

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
 drivers/gpu/drm/exynos/exynos5433_drm_decon.c |  1 +
 drivers/gpu/drm/exynos/exynos7_drm_decon.c    |  1 +
 drivers/gpu/drm/exynos/exynos_drm_crtc.c      | 29 +++++++++++++++------------
 drivers/gpu/drm/exynos/exynos_drm_crtc.h      |  2 ++
 drivers/gpu/drm/exynos/exynos_drm_fimd.c      |  2 ++
 drivers/gpu/drm/exynos/exynos_drm_vidi.c      |  1 +
 drivers/gpu/drm/exynos/exynos_mixer.c         |  1 +
 7 files changed, 24 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
index 0fd6f7a..147911e 100644
--- a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
+++ b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
@@ -378,6 +378,7 @@ static void decon_atomic_flush(struct exynos_drm_crtc *crtc)
 
 	if (ctx->out_type & IFTYPE_I80)
 		set_bit(BIT_WIN_UPDATED, &ctx->flags);
+	exynos_crtc_handle_event(crtc);
 }
 
 static void decon_swreset(struct decon_context *ctx)
diff --git a/drivers/gpu/drm/exynos/exynos7_drm_decon.c b/drivers/gpu/drm/exynos/exynos7_drm_decon.c
index f9ab19e..4881180 100644
--- a/drivers/gpu/drm/exynos/exynos7_drm_decon.c
+++ b/drivers/gpu/drm/exynos/exynos7_drm_decon.c
@@ -526,6 +526,7 @@ static void decon_atomic_flush(struct exynos_drm_crtc *crtc)
 
 	for (i = 0; i < WINDOWS_NR; i++)
 		decon_shadow_protect_win(ctx, i, false);
+	exynos_crtc_handle_event(crtc);
 }
 
 static void decon_init(struct decon_context *ctx)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.c b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
index fa32091..0620d3c 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_crtc.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
@@ -85,16 +85,28 @@ static void exynos_crtc_atomic_flush(struct drm_crtc *crtc,
 				     struct drm_crtc_state *old_crtc_state)
 {
 	struct exynos_drm_crtc *exynos_crtc = to_exynos_crtc(crtc);
-	struct drm_pending_vblank_event *event;
-	unsigned long flags;
 
 	if (exynos_crtc->ops->atomic_flush)
 		exynos_crtc->ops->atomic_flush(exynos_crtc);
+}
+
+static const struct drm_crtc_helper_funcs exynos_crtc_helper_funcs = {
+	.enable		= exynos_drm_crtc_enable,
+	.disable	= exynos_drm_crtc_disable,
+	.mode_set_nofb	= exynos_drm_crtc_mode_set_nofb,
+	.atomic_check	= exynos_crtc_atomic_check,
+	.atomic_begin	= exynos_crtc_atomic_begin,
+	.atomic_flush	= exynos_crtc_atomic_flush,
+};
+
+void exynos_crtc_handle_event(struct exynos_drm_crtc *exynos_crtc)
+{
+	struct drm_crtc *crtc = &exynos_crtc->base;
+	struct drm_pending_vblank_event *event = crtc->state->event;
+	unsigned long flags;
 
-	event = crtc->state->event;
 	if (event) {
 		crtc->state->event = NULL;
-
 		spin_lock_irqsave(&crtc->dev->event_lock, flags);
 		if (drm_crtc_vblank_get(crtc) == 0)
 			drm_crtc_arm_vblank_event(crtc, event);
@@ -105,15 +117,6 @@ static void exynos_crtc_atomic_flush(struct drm_crtc *crtc,
 
 }
 
-static const struct drm_crtc_helper_funcs exynos_crtc_helper_funcs = {
-	.enable		= exynos_drm_crtc_enable,
-	.disable	= exynos_drm_crtc_disable,
-	.mode_set_nofb	= exynos_drm_crtc_mode_set_nofb,
-	.atomic_check	= exynos_crtc_atomic_check,
-	.atomic_begin	= exynos_crtc_atomic_begin,
-	.atomic_flush	= exynos_crtc_atomic_flush,
-};
-
 static void exynos_drm_crtc_destroy(struct drm_crtc *crtc)
 {
 	struct exynos_drm_crtc *exynos_crtc = to_exynos_crtc(crtc);
diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.h b/drivers/gpu/drm/exynos/exynos_drm_crtc.h
index 4e986ba..9634fe5 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_crtc.h
+++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.h
@@ -38,4 +38,6 @@ int exynos_drm_crtc_get_pipe_from_type(struct drm_device *drm_dev,
  */
 void exynos_drm_crtc_te_handler(struct drm_crtc *crtc);
 
+void exynos_crtc_handle_event(struct exynos_drm_crtc *exynos_crtc);
+
 #endif
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
index a9fa444..69ebed0 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
@@ -723,6 +723,8 @@ static void fimd_atomic_flush(struct exynos_drm_crtc *crtc)
 
 	for (i = 0; i < WINDOWS_NR; i++)
 		fimd_shadow_protect_win(ctx, i, false);
+
+	exynos_crtc_handle_event(crtc);
 }
 
 static void fimd_update_plane(struct exynos_drm_crtc *crtc,
diff --git a/drivers/gpu/drm/exynos/exynos_drm_vidi.c b/drivers/gpu/drm/exynos/exynos_drm_vidi.c
index 57fe514..5d9a62a 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_vidi.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_vidi.c
@@ -170,6 +170,7 @@ static const struct exynos_drm_crtc_ops vidi_crtc_ops = {
 	.enable_vblank = vidi_enable_vblank,
 	.disable_vblank = vidi_disable_vblank,
 	.update_plane = vidi_update_plane,
+	.atomic_flush = exynos_crtc_handle_event,
 };
 
 static void vidi_fake_vblank_timer(unsigned long arg)
diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c
index 72143ac..25edb63 100644
--- a/drivers/gpu/drm/exynos/exynos_mixer.c
+++ b/drivers/gpu/drm/exynos/exynos_mixer.c
@@ -1012,6 +1012,7 @@ static void mixer_atomic_flush(struct exynos_drm_crtc *crtc)
 		return;
 
 	mixer_vsync_set_update(mixer_ctx, true);
+	exynos_crtc_handle_event(crtc);
 }
 
 static void mixer_enable(struct exynos_drm_crtc *crtc)
-- 
2.7.4

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

[PATCH v2 02/12] drm/exynos: simplify completion event handling

[Andrzej Hajda]

All Exynos CRTC drivers shouldn't fail at referencing vblank events,
alternate path is actually dead code.

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

diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.c b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
index 0620d3c..9184974 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_crtc.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
@@ -105,16 +105,15 @@ void exynos_crtc_handle_event(struct exynos_drm_crtc *exynos_crtc)
 	struct drm_pending_vblank_event *event = crtc->state->event;
 	unsigned long flags;
 
-	if (event) {
-		crtc->state->event = NULL;
-		spin_lock_irqsave(&crtc->dev->event_lock, flags);
-		if (drm_crtc_vblank_get(crtc) == 0)
-			drm_crtc_arm_vblank_event(crtc, event);
-		else
-			drm_crtc_send_vblank_event(crtc, event);
-		spin_unlock_irqrestore(&crtc->dev->event_lock, flags);
-	}
+	if (!event)
+		return;
+	crtc->state->event = NULL;
+
+	WARN_ON(drm_crtc_vblank_get(crtc) != 0);
 
+	spin_lock_irqsave(&crtc->dev->event_lock, flags);
+	drm_crtc_arm_vblank_event(crtc, event);
+	spin_unlock_irqrestore(&crtc->dev->event_lock, flags);
 }
 
 static void exynos_drm_crtc_destroy(struct drm_crtc *crtc)
-- 
2.7.4

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

[PATCH v2 03/12] drm/exynos/decon5433: fix vblank event handling

[Andrzej Hajda]

Current implementation of event handling assumes that vblank interrupt is
always called at the right time. It is not true, it can be delayed due to
various reasons. As a result different races can happen. The patch fixes
the issue by using hardware frame counter present in DECON to serialize
vblank and commit completion events.

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
v2:
  - added internal decon_get_frame_count function,
  - updated frame counter on flush,
  - misc style fixes (thank Inki)
---
 drivers/gpu/drm/exynos/exynos5433_drm_decon.c | 77 ++++++++++++++++++++++++++-
 include/video/exynos5433_decon.h              |  8 +++
 2 files changed, 84 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
index 147911e..4ca3d6e 100644
--- a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
+++ b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
@@ -68,6 +68,8 @@ struct decon_context {
 	unsigned long			flags;
 	unsigned long			out_type;
 	int				first_win;
+	spinlock_t			vblank_lock;
+	u32				frame_id;
 };
 
 static const uint32_t decon_formats[] = {
@@ -122,6 +124,48 @@ static void decon_disable_vblank(struct exynos_drm_crtc *crtc)
 		writel(0, ctx->addr + DECON_VIDINTCON0);
 }
 
+/* return number of starts/ends of frame transmissions since reset */
+static u32 decon_get_frame_count(struct decon_context *ctx, bool end)
+{
+	u32 frm, pfrm, status, cnt = 2;
+
+	/* To get consistent result repeat read until frame id is stable.
+	 * Usually the loop will be executed once, in rare cases when the loop
+	 * is executed at frame change time 2nd pass will be needed.
+	 */
+	frm = readl(ctx->addr + DECON_CRFMID);
+	do {
+		status = readl(ctx->addr + DECON_VIDCON1);
+		pfrm = frm;
+		frm = readl(ctx->addr + DECON_CRFMID);
+	} while (frm != pfrm && --cnt);
+
+	/* CRFMID is incremented on BPORCH in case of I80 and on VSYNC in case
+	 * of RGB, it should be taken into account.
+	 */
+	if (!frm)
+		return 0;
+
+	switch (status & (VIDCON1_VSTATUS_MASK | VIDCON1_I80_ACTIVE)) {
+	case VIDCON1_VSTATUS_VS:
+		if (!(ctx->out_type & IFTYPE_I80))
+			--frm;
+		break;
+	case VIDCON1_VSTATUS_BP:
+		--frm;
+		break;
+	case VIDCON1_I80_ACTIVE:
+	case VIDCON1_VSTATUS_AC:
+		if (end)
+			--frm;
+		break;
+	default:
+		break;
+	}
+
+	return frm;
+}
+
 static void decon_setup_trigger(struct decon_context *ctx)
 {
 	if (!(ctx->out_type & (IFTYPE_I80 | I80_HW_TRG)))
@@ -365,11 +409,14 @@ static void decon_disable_plane(struct exynos_drm_crtc *crtc,
 static void decon_atomic_flush(struct exynos_drm_crtc *crtc)
 {
 	struct decon_context *ctx = crtc->ctx;
+	unsigned long flags;
 	int i;
 
 	if (test_bit(BIT_SUSPENDED, &ctx->flags))
 		return;
 
+	spin_lock_irqsave(&ctx->vblank_lock, flags);
+
 	for (i = ctx->first_win; i < WINDOWS_NR; i++)
 		decon_shadow_protect_win(ctx, i, false);
 
@@ -378,12 +425,18 @@ static void decon_atomic_flush(struct exynos_drm_crtc *crtc)
 
 	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);
+
+	spin_unlock_irqrestore(&ctx->vblank_lock, flags);
 }
 
 static void decon_swreset(struct decon_context *ctx)
 {
 	unsigned int tries;
+	unsigned long flags;
 
 	writel(0, ctx->addr + DECON_VIDCON0);
 	for (tries = 2000; tries; --tries) {
@@ -401,6 +454,10 @@ static void decon_swreset(struct decon_context *ctx)
 
 	WARN(tries == 0, "failed to software reset DECON\n");
 
+	spin_lock_irqsave(&ctx->vblank_lock, flags);
+	ctx->frame_id = 0;
+	spin_unlock_irqrestore(&ctx->vblank_lock, flags);
+
 	if (!(ctx->out_type & IFTYPE_HDMI))
 		return;
 
@@ -579,6 +636,23 @@ static const struct component_ops decon_component_ops = {
 	.unbind = decon_unbind,
 };
 
+static void decon_handle_vblank(struct decon_context *ctx)
+{
+	u32 frm;
+
+	spin_lock(&ctx->vblank_lock);
+
+	frm = decon_get_frame_count(ctx, true);
+
+	if (frm != ctx->frame_id) {
+		if (frm > ctx->frame_id)
+			drm_crtc_handle_vblank(&ctx->crtc->base);
+		ctx->frame_id = frm;
+	}
+
+	spin_unlock(&ctx->vblank_lock);
+}
+
 static irqreturn_t decon_irq_handler(int irq, void *dev_id)
 {
 	struct decon_context *ctx = dev_id;
@@ -599,7 +673,7 @@ static irqreturn_t decon_irq_handler(int irq, void *dev_id)
 			    (VIDOUT_INTERLACE_EN_F | VIDOUT_INTERLACE_FIELD_F))
 				return IRQ_HANDLED;
 		}
-		drm_crtc_handle_vblank(&ctx->crtc->base);
+		decon_handle_vblank(ctx);
 	}
 
 out:
@@ -672,6 +746,7 @@ static int exynos5433_decon_probe(struct platform_device *pdev)
 	__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);
 
 	if (ctx->out_type & IFTYPE_HDMI) {
 		ctx->first_win = 1;
diff --git a/include/video/exynos5433_decon.h b/include/video/exynos5433_decon.h
index ef8e2a8..352fc0d 100644
--- a/include/video/exynos5433_decon.h
+++ b/include/video/exynos5433_decon.h
@@ -46,6 +46,7 @@
 #define DECON_FRAMEFIFO_STATUS		0x0524
 #define DECON_CMU			0x1404
 #define DECON_UPDATE			0x1410
+#define DECON_CRFMID			0x1414
 #define DECON_UPDATE_SCHEME		0x1438
 #define DECON_VIDCON1			0x2000
 #define DECON_VIDCON2			0x2004
@@ -142,6 +143,13 @@
 #define STANDALONE_UPDATE_F		(1 << 0)
 
 /* DECON_VIDCON1 */
+#define VIDCON1_LINECNT_MASK		(0x0fff << 16)
+#define VIDCON1_I80_ACTIVE		(1 << 15)
+#define VIDCON1_VSTATUS_MASK		(0x3 << 13)
+#define VIDCON1_VSTATUS_VS		(0 << 13)
+#define VIDCON1_VSTATUS_BP		(1 << 13)
+#define VIDCON1_VSTATUS_AC		(2 << 13)
+#define VIDCON1_VSTATUS_FP		(3 << 13)
 #define VIDCON1_VCLK_MASK		(0x3 << 9)
 #define VIDCON1_VCLK_RUN_VDEN_DISABLE	(0x3 << 9)
 #define VIDCON1_VCLK_HOLD		(0x0 << 9)
-- 
2.7.4

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

[PATCH v2 04/12] drm/exynos/decon5433: implement frame counter

[Andrzej Hajda]

DECON in Exynos5433 has frame counter, it can be used to implement
get_vblank_counter callback.

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
v2:
  - reuse decon_get_frame_count function already implemented
    in previous patch
---
 drivers/gpu/drm/exynos/exynos5433_drm_decon.c | 12 ++++++++++++
 drivers/gpu/drm/exynos/exynos_drm_crtc.c      | 11 +++++++++++
 drivers/gpu/drm/exynos/exynos_drm_drv.h       |  1 +
 3 files changed, 24 insertions(+)

diff --git a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
index 4ca3d6e..60ff81b 100644
--- a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
+++ b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
@@ -166,6 +166,16 @@ static u32 decon_get_frame_count(struct decon_context *ctx, bool end)
 	return frm;
 }
 
+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);
+}
+
 static void decon_setup_trigger(struct decon_context *ctx)
 {
 	if (!(ctx->out_type & (IFTYPE_I80 | I80_HW_TRG)))
@@ -564,6 +574,7 @@ static const struct exynos_drm_crtc_ops decon_crtc_ops = {
 	.disable		= decon_disable,
 	.enable_vblank		= decon_enable_vblank,
 	.disable_vblank		= decon_disable_vblank,
+	.get_vblank_counter	= decon_get_vblank_counter,
 	.atomic_begin		= decon_atomic_begin,
 	.update_plane		= decon_update_plane,
 	.disable_plane		= decon_disable_plane,
@@ -583,6 +594,7 @@ static int decon_bind(struct device *dev, struct device *master, void *data)
 
 	ctx->drm_dev = drm_dev;
 	ctx->pipe = priv->pipe++;
+	drm_dev->max_vblank_count = 0xffffffff;
 
 	for (win = ctx->first_win; win < WINDOWS_NR; win++) {
 		int tmp = (win == ctx->first_win) ? 0 : win;
diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.c b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
index 9184974..c02378b 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_crtc.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
@@ -142,6 +142,16 @@ static void exynos_drm_crtc_disable_vblank(struct drm_crtc *crtc)
 		exynos_crtc->ops->disable_vblank(exynos_crtc);
 }
 
+static u32 exynos_drm_crtc_get_vblank_counter(struct drm_crtc *crtc)
+{
+	struct exynos_drm_crtc *exynos_crtc = to_exynos_crtc(crtc);
+
+	if (exynos_crtc->ops->get_vblank_counter)
+		return exynos_crtc->ops->get_vblank_counter(exynos_crtc);
+
+	return 0;
+}
+
 static const struct drm_crtc_funcs exynos_crtc_funcs = {
 	.set_config	= drm_atomic_helper_set_config,
 	.page_flip	= drm_atomic_helper_page_flip,
@@ -151,6 +161,7 @@ static const struct drm_crtc_funcs exynos_crtc_funcs = {
 	.atomic_destroy_state = drm_atomic_helper_crtc_destroy_state,
 	.enable_vblank = exynos_drm_crtc_enable_vblank,
 	.disable_vblank = exynos_drm_crtc_disable_vblank,
+	.get_vblank_counter = exynos_drm_crtc_get_vblank_counter,
 };
 
 struct exynos_drm_crtc *exynos_drm_crtc_create(struct drm_device *drm_dev,
diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.h b/drivers/gpu/drm/exynos/exynos_drm_drv.h
index cb31769..906ab7a 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_drv.h
+++ b/drivers/gpu/drm/exynos/exynos_drm_drv.h
@@ -133,6 +133,7 @@ struct exynos_drm_crtc_ops {
 	void (*commit)(struct exynos_drm_crtc *crtc);
 	int (*enable_vblank)(struct exynos_drm_crtc *crtc);
 	void (*disable_vblank)(struct exynos_drm_crtc *crtc);
+	u32 (*get_vblank_counter)(struct exynos_drm_crtc *crtc);
 	int (*atomic_check)(struct exynos_drm_crtc *crtc,
 			    struct drm_crtc_state *state);
 	void (*atomic_begin)(struct exynos_drm_crtc *crtc);
-- 
2.7.4

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

[PATCH v2 05/12] drm/exynos/decon5433: signal frame done interrupt at front porch

[Andrzej Hajda]

DECON in case of video mode generates interrupt by default at start
of vertical back porch. As this interrupt is used to generate VBLANK
events more optimal point is start of vertical front porch.

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
 drivers/gpu/drm/exynos/exynos5433_drm_decon.c | 2 +-
 include/video/exynos5433_decon.h              | 4 ++++
 2 files 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 60ff81b..319f557 100644
--- a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
+++ b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
@@ -105,7 +105,7 @@ static int decon_enable_vblank(struct exynos_drm_crtc *crtc)
 		if (ctx->out_type & IFTYPE_I80)
 			val |= VIDINTCON0_FRAMEDONE;
 		else
-			val |= VIDINTCON0_INTFRMEN;
+			val |= VIDINTCON0_INTFRMEN | VIDINTCON0_FRAMESEL_FP;
 
 		writel(val, ctx->addr + DECON_VIDINTCON0);
 	}
diff --git a/include/video/exynos5433_decon.h b/include/video/exynos5433_decon.h
index 352fc0d..6b083d3 100644
--- a/include/video/exynos5433_decon.h
+++ b/include/video/exynos5433_decon.h
@@ -127,6 +127,10 @@
 
 /* VIDINTCON0 */
 #define VIDINTCON0_FRAMEDONE		(1 << 17)
+#define VIDINTCON0_FRAMESEL_BP		(0 << 15)
+#define VIDINTCON0_FRAMESEL_VS		(1 << 15)
+#define VIDINTCON0_FRAMESEL_AC		(2 << 15)
+#define VIDINTCON0_FRAMESEL_FP		(3 << 15)
 #define VIDINTCON0_INTFRMEN		(1 << 12)
 #define VIDINTCON0_INTEN		(1 << 0)
 
-- 
2.7.4

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

[PATCH v2 06/12] drm/exynos/fimd: signal frame done interrupt at front porch

[Andrzej Hajda]

VBLANK interrupt should be signalled as soon as scanout ends, front porch
is the best moment.

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

diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
index 69ebed0..eb4cd82 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
@@ -257,7 +257,7 @@ static int fimd_enable_vblank(struct exynos_drm_crtc *crtc)
 			val |= VIDINTCON0_INT_FRAME;
 
 			val &= ~VIDINTCON0_FRAMESEL0_MASK;
-			val |= VIDINTCON0_FRAMESEL0_VSYNC;
+			val |= VIDINTCON0_FRAMESEL0_FRONTPORCH;
 			val &= ~VIDINTCON0_FRAMESEL1_MASK;
 			val |= VIDINTCON0_FRAMESEL1_NONE;
 		}
-- 
2.7.4

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

[PATCH v2 07/12] drm/exynos/decon5433: fix software trigger mask

[Andrzej Hajda]

The patch fixes copy/paste bug introduced during code refactoring.

Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Fixes: b93c2e8b5d9d ("drm/exynos/decon5433: configure sysreg in case of hardware trigger")Fixes:
Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
 drivers/gpu/drm/exynos/exynos5433_drm_decon.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
index 319f557..ee8d5cb 100644
--- a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
+++ b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
@@ -182,8 +182,8 @@ static void decon_setup_trigger(struct decon_context *ctx)
 		return;
 
 	if (!(ctx->out_type & I80_HW_TRG)) {
-		writel(TRIGCON_TE_AUTO_MASK | TRIGCON_SWTRIGEN
-		       | TRIGCON_TE_AUTO_MASK | TRIGCON_SWTRIGEN,
+		writel(TRIGCON_TRIGEN_PER_F | TRIGCON_TRIGEN_F |
+		       TRIGCON_TE_AUTO_MASK | TRIGCON_SWTRIGEN,
 		       ctx->addr + DECON_TRIGCON);
 		return;
 	}
-- 
2.7.4

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

[PATCH v2 08/12] drm/exynos: kill exynos_drm_crtc::pipe

[Andrzej Hajda]

Since crtc index is stored in drm_crtc pipe field became redundant.
The patch beside removing the field simplifies also
exynos_drm_crtc_get_pipe_from_type.

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
 drivers/gpu/drm/exynos/exynos5433_drm_decon.c |  3 +--
 drivers/gpu/drm/exynos/exynos7_drm_decon.c    |  3 +--
 drivers/gpu/drm/exynos/exynos_drm_crtc.c      | 12 +++---------
 drivers/gpu/drm/exynos/exynos_drm_crtc.h      |  1 -
 drivers/gpu/drm/exynos/exynos_drm_drv.h       |  8 --------
 drivers/gpu/drm/exynos/exynos_drm_fimd.c      |  3 +--
 drivers/gpu/drm/exynos/exynos_drm_vidi.c      |  3 +--
 drivers/gpu/drm/exynos/exynos_mixer.c         |  3 +--
 8 files changed, 8 insertions(+), 28 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
index ee8d5cb..2eb6997 100644
--- a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
+++ b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
@@ -614,8 +614,7 @@ static int decon_bind(struct device *dev, struct device *master, void *data)
 	out_type = (ctx->out_type & IFTYPE_HDMI) ? EXYNOS_DISPLAY_TYPE_HDMI
 						  : EXYNOS_DISPLAY_TYPE_LCD;
 	ctx->crtc = exynos_drm_crtc_create(drm_dev, &exynos_plane->base,
-					ctx->pipe, out_type,
-					&decon_crtc_ops, ctx);
+			out_type, &decon_crtc_ops, ctx);
 	if (IS_ERR(ctx->crtc)) {
 		ret = PTR_ERR(ctx->crtc);
 		goto err;
diff --git a/drivers/gpu/drm/exynos/exynos7_drm_decon.c b/drivers/gpu/drm/exynos/exynos7_drm_decon.c
index 4881180..0ccb334 100644
--- a/drivers/gpu/drm/exynos/exynos7_drm_decon.c
+++ b/drivers/gpu/drm/exynos/exynos7_drm_decon.c
@@ -656,8 +656,7 @@ static int decon_bind(struct device *dev, struct device *master, void *data)
 
 	exynos_plane = &ctx->planes[DEFAULT_WIN];
 	ctx->crtc = exynos_drm_crtc_create(drm_dev, &exynos_plane->base,
-					   ctx->pipe, EXYNOS_DISPLAY_TYPE_LCD,
-					   &decon_crtc_ops, ctx);
+			EXYNOS_DISPLAY_TYPE_LCD, &decon_crtc_ops, ctx);
 	if (IS_ERR(ctx->crtc)) {
 		decon_ctx_remove(ctx);
 		return PTR_ERR(ctx->crtc);
diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.c b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
index c02378b..2d0aa11 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_crtc.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
@@ -166,7 +166,6 @@ static const struct drm_crtc_funcs exynos_crtc_funcs = {
 
 struct exynos_drm_crtc *exynos_drm_crtc_create(struct drm_device *drm_dev,
 					struct drm_plane *plane,
-					int pipe,
 					enum exynos_drm_output_type type,
 					const struct exynos_drm_crtc_ops *ops,
 					void *ctx)
@@ -179,7 +178,6 @@ struct exynos_drm_crtc *exynos_drm_crtc_create(struct drm_device *drm_dev,
 	if (!exynos_crtc)
 		return ERR_PTR(-ENOMEM);
 
-	exynos_crtc->pipe = pipe;
 	exynos_crtc->type = type;
 	exynos_crtc->ops = ops;
 	exynos_crtc->ctx = ctx;
@@ -206,13 +204,9 @@ int exynos_drm_crtc_get_pipe_from_type(struct drm_device *drm_dev,
 {
 	struct drm_crtc *crtc;
 
-	list_for_each_entry(crtc, &drm_dev->mode_config.crtc_list, head) {
-		struct exynos_drm_crtc *exynos_crtc;
-
-		exynos_crtc = to_exynos_crtc(crtc);
-		if (exynos_crtc->type == out_type)
-			return exynos_crtc->pipe;
-	}
+	drm_for_each_crtc(crtc, drm_dev)
+		if (to_exynos_crtc(crtc)->type == out_type)
+			return drm_crtc_index(crtc);
 
 	return -EPERM;
 }
diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.h b/drivers/gpu/drm/exynos/exynos_drm_crtc.h
index 9634fe5..ef58b64 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_crtc.h
+++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.h
@@ -19,7 +19,6 @@
 
 struct exynos_drm_crtc *exynos_drm_crtc_create(struct drm_device *drm_dev,
 					struct drm_plane *plane,
-					int pipe,
 					enum exynos_drm_output_type type,
 					const struct exynos_drm_crtc_ops *ops,
 					void *context);
diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.h b/drivers/gpu/drm/exynos/exynos_drm_drv.h
index 906ab7a..7e54be1 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_drv.h
+++ b/drivers/gpu/drm/exynos/exynos_drm_drv.h
@@ -154,13 +154,6 @@ struct exynos_drm_clk {
  *
  * @base: crtc object.
  * @type: one of EXYNOS_DISPLAY_TYPE_LCD and HDMI.
- * @pipe: a crtc index created at load() with a new crtc object creation
- *	and the crtc object would be set to private->crtc array
- *	to get a crtc object corresponding to this pipe from private->crtc
- *	array when irq interrupt occurred. the reason of using this pipe is that
- *	drm framework doesn't support multiple irq yet.
- *	we can refer to the crtc to current hardware interrupt occurred through
- *	this pipe value.
  * @enabled: if the crtc is enabled or not
  * @event: vblank event that is currently queued for flip
  * @wait_update: wait all pending planes updates to finish
@@ -171,7 +164,6 @@ struct exynos_drm_clk {
 struct exynos_drm_crtc {
 	struct drm_crtc			base;
 	enum exynos_drm_output_type	type;
-	unsigned int			pipe;
 	const struct exynos_drm_crtc_ops	*ops;
 	void				*ctx;
 	struct exynos_drm_clk		*pipe_clk;
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
index eb4cd82..caaccd8 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
@@ -1017,8 +1017,7 @@ static int fimd_bind(struct device *dev, struct device *master, void *data)
 
 	exynos_plane = &ctx->planes[DEFAULT_WIN];
 	ctx->crtc = exynos_drm_crtc_create(drm_dev, &exynos_plane->base,
-					   ctx->pipe, EXYNOS_DISPLAY_TYPE_LCD,
-					   &fimd_crtc_ops, ctx);
+			EXYNOS_DISPLAY_TYPE_LCD, &fimd_crtc_ops, ctx);
 	if (IS_ERR(ctx->crtc))
 		return PTR_ERR(ctx->crtc);
 
diff --git a/drivers/gpu/drm/exynos/exynos_drm_vidi.c b/drivers/gpu/drm/exynos/exynos_drm_vidi.c
index 5d9a62a..67f365f 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_vidi.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_vidi.c
@@ -417,8 +417,7 @@ static int vidi_bind(struct device *dev, struct device *master, void *data)
 
 	exynos_plane = &ctx->planes[DEFAULT_WIN];
 	ctx->crtc = exynos_drm_crtc_create(drm_dev, &exynos_plane->base,
-					   ctx->pipe, EXYNOS_DISPLAY_TYPE_VIDI,
-					   &vidi_crtc_ops, ctx);
+			EXYNOS_DISPLAY_TYPE_VIDI, &vidi_crtc_ops, ctx);
 	if (IS_ERR(ctx->crtc)) {
 		DRM_ERROR("failed to create crtc.\n");
 		return PTR_ERR(ctx->crtc);
diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c
index 25edb63..3fb8cf3 100644
--- a/drivers/gpu/drm/exynos/exynos_mixer.c
+++ b/drivers/gpu/drm/exynos/exynos_mixer.c
@@ -1165,8 +1165,7 @@ static int mixer_bind(struct device *dev, struct device *manager, void *data)
 
 	exynos_plane = &ctx->planes[DEFAULT_WIN];
 	ctx->crtc = exynos_drm_crtc_create(drm_dev, &exynos_plane->base,
-					   ctx->pipe, EXYNOS_DISPLAY_TYPE_HDMI,
-					   &mixer_crtc_ops, ctx);
+			EXYNOS_DISPLAY_TYPE_HDMI, &mixer_crtc_ops, ctx);
 	if (IS_ERR(ctx->crtc)) {
 		mixer_ctx_remove(ctx);
 		ret = PTR_ERR(ctx->crtc);
-- 
2.7.4

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

[PATCH v2 09/12] drm/exynos: kill exynos_drm_private::pipe

[Andrzej Hajda]

The field duplicates drm_dev->mode_config.num_crtc.

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
 drivers/gpu/drm/exynos/exynos5433_drm_decon.c | 18 ++++--------------
 drivers/gpu/drm/exynos/exynos7_drm_decon.c    | 11 ++---------
 drivers/gpu/drm/exynos/exynos_drm_drv.h       |  3 ---
 drivers/gpu/drm/exynos/exynos_drm_fimd.c      |  9 ++-------
 drivers/gpu/drm/exynos/exynos_drm_vidi.c      |  4 +---
 drivers/gpu/drm/exynos/exynos_mixer.c         |  8 ++------
 6 files changed, 11 insertions(+), 42 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
index 2eb6997..d922b10 100644
--- a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
+++ b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
@@ -586,14 +586,13 @@ static int decon_bind(struct device *dev, struct device *master, void *data)
 {
 	struct decon_context *ctx = dev_get_drvdata(dev);
 	struct drm_device *drm_dev = data;
-	struct exynos_drm_private *priv = drm_dev->dev_private;
 	struct exynos_drm_plane *exynos_plane;
 	enum exynos_drm_output_type out_type;
 	unsigned int win;
 	int ret;
 
 	ctx->drm_dev = drm_dev;
-	ctx->pipe = priv->pipe++;
+	ctx->pipe = drm_dev->mode_config.num_crtc;
 	drm_dev->max_vblank_count = 0xffffffff;
 
 	for (win = ctx->first_win; win < WINDOWS_NR; win++) {
@@ -615,21 +614,12 @@ static int decon_bind(struct device *dev, struct device *master, void *data)
 						  : EXYNOS_DISPLAY_TYPE_LCD;
 	ctx->crtc = exynos_drm_crtc_create(drm_dev, &exynos_plane->base,
 			out_type, &decon_crtc_ops, ctx);
-	if (IS_ERR(ctx->crtc)) {
-		ret = PTR_ERR(ctx->crtc);
-		goto err;
-	}
+	if (IS_ERR(ctx->crtc))
+		return PTR_ERR(ctx->crtc);
 
 	decon_clear_channels(ctx->crtc);
 
-	ret = drm_iommu_attach_device(drm_dev, dev);
-	if (ret)
-		goto err;
-
-	return ret;
-err:
-	priv->pipe--;
-	return ret;
+	return drm_iommu_attach_device(drm_dev, dev);
 }
 
 static void decon_unbind(struct device *dev, struct device *master, void *data)
diff --git a/drivers/gpu/drm/exynos/exynos7_drm_decon.c b/drivers/gpu/drm/exynos/exynos7_drm_decon.c
index 0ccb334..206e779 100644
--- a/drivers/gpu/drm/exynos/exynos7_drm_decon.c
+++ b/drivers/gpu/drm/exynos/exynos7_drm_decon.c
@@ -130,19 +130,12 @@ static void decon_clear_channels(struct exynos_drm_crtc *crtc)
 static int decon_ctx_initialize(struct decon_context *ctx,
 			struct drm_device *drm_dev)
 {
-	struct exynos_drm_private *priv = drm_dev->dev_private;
-	int ret;
-
 	ctx->drm_dev = drm_dev;
-	ctx->pipe = priv->pipe++;
+	ctx->pipe = drm_dev->mode_config.num_crtc;
 
 	decon_clear_channels(ctx->crtc);
 
-	ret = drm_iommu_attach_device(drm_dev, ctx->dev);
-	if (ret)
-		priv->pipe--;
-
-	return ret;
+	return drm_iommu_attach_device(drm_dev, ctx->dev);
 }
 
 static void decon_ctx_remove(struct decon_context *ctx)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.h b/drivers/gpu/drm/exynos/exynos_drm_drv.h
index 7e54be1..7ef2b36 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_drv.h
+++ b/drivers/gpu/drm/exynos/exynos_drm_drv.h
@@ -196,7 +196,6 @@ struct drm_exynos_file_private {
  *	otherwise default one.
  * @da_space_size: size of device address space.
  *	if 0 then default value is used for it.
- * @pipe: the pipe number for this crtc/manager.
  * @pending: the crtcs that have pending updates to finish
  * @lock: protect access to @pending
  * @wait: wait an atomic commit to finish
@@ -207,8 +206,6 @@ struct exynos_drm_private {
 	struct device *dma_dev;
 	void *mapping;
 
-	unsigned int pipe;
-
 	/* for atomic commit */
 	u32			pending;
 	spinlock_t		lock;
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
index caaccd8..588723d 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
@@ -996,13 +996,12 @@ static int fimd_bind(struct device *dev, struct device *master, void *data)
 {
 	struct fimd_context *ctx = dev_get_drvdata(dev);
 	struct drm_device *drm_dev = data;
-	struct exynos_drm_private *priv = drm_dev->dev_private;
 	struct exynos_drm_plane *exynos_plane;
 	unsigned int i;
 	int ret;
 
 	ctx->drm_dev = drm_dev;
-	ctx->pipe = priv->pipe++;
+	ctx->pipe = drm_dev->mode_config.num_crtc;
 
 	for (i = 0; i < WINDOWS_NR; i++) {
 		ctx->configs[i].pixel_formats = fimd_formats;
@@ -1032,11 +1031,7 @@ static int fimd_bind(struct device *dev, struct device *master, void *data)
 	if (is_drm_iommu_supported(drm_dev))
 		fimd_clear_channels(ctx->crtc);
 
-	ret = drm_iommu_attach_device(drm_dev, dev);
-	if (ret)
-		priv->pipe--;
-
-	return ret;
+	return drm_iommu_attach_device(drm_dev, dev);
 }
 
 static void fimd_unbind(struct device *dev, struct device *master,
diff --git a/drivers/gpu/drm/exynos/exynos_drm_vidi.c b/drivers/gpu/drm/exynos/exynos_drm_vidi.c
index 67f365f..cb5e3c6 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_vidi.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_vidi.c
@@ -156,10 +156,8 @@ static void vidi_disable(struct exynos_drm_crtc *crtc)
 static int vidi_ctx_initialize(struct vidi_context *ctx,
 			struct drm_device *drm_dev)
 {
-	struct exynos_drm_private *priv = drm_dev->dev_private;
-
 	ctx->drm_dev = drm_dev;
-	ctx->pipe = priv->pipe++;
+	ctx->pipe = drm_dev->mode_config.num_crtc;
 
 	return 0;
 }
diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c
index 3fb8cf3..1474982 100644
--- a/drivers/gpu/drm/exynos/exynos_mixer.c
+++ b/drivers/gpu/drm/exynos/exynos_mixer.c
@@ -900,7 +900,7 @@ static int mixer_initialize(struct mixer_context *mixer_ctx,
 	priv = drm_dev->dev_private;
 
 	mixer_ctx->drm_dev = drm_dev;
-	mixer_ctx->pipe = priv->pipe++;
+	mixer_ctx->pipe = drm_dev->mode_config.num_crtc;
 
 	/* acquire resources: regs, irqs, clocks */
 	ret = mixer_resources_init(mixer_ctx);
@@ -918,11 +918,7 @@ static int mixer_initialize(struct mixer_context *mixer_ctx,
 		}
 	}
 
-	ret = drm_iommu_attach_device(drm_dev, mixer_ctx->dev);
-	if (ret)
-		priv->pipe--;
-
-	return ret;
+	return drm_iommu_attach_device(drm_dev, mixer_ctx->dev);
 }
 
 static void mixer_ctx_remove(struct mixer_context *mixer_ctx)
-- 
2.7.4

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

[PATCH v2 10/12] drm/exynos: set plane possible_crtcs in exynos_plane_init

[Andrzej Hajda]

All Exynos planes are assigned to exactly one CRTC, it allows to simplify
initialization by moving setting of possible_crtcs to exynos_plane_init.

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
 drivers/gpu/drm/exynos/exynos5433_drm_decon.c | 2 +-
 drivers/gpu/drm/exynos/exynos7_drm_decon.c    | 2 +-
 drivers/gpu/drm/exynos/exynos_drm_fimd.c      | 2 +-
 drivers/gpu/drm/exynos/exynos_drm_plane.c     | 5 ++---
 drivers/gpu/drm/exynos/exynos_drm_plane.h     | 1 -
 drivers/gpu/drm/exynos/exynos_drm_vidi.c      | 2 +-
 drivers/gpu/drm/exynos/exynos_mixer.c         | 2 +-
 7 files changed, 7 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
index d922b10..a60d461 100644
--- a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
+++ b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
@@ -604,7 +604,7 @@ static int decon_bind(struct device *dev, struct device *master, void *data)
 		ctx->configs[win].type = decon_win_types[tmp];
 
 		ret = exynos_plane_init(drm_dev, &ctx->planes[win], win,
-					1 << ctx->pipe, &ctx->configs[win]);
+					&ctx->configs[win]);
 		if (ret)
 			return ret;
 	}
diff --git a/drivers/gpu/drm/exynos/exynos7_drm_decon.c b/drivers/gpu/drm/exynos/exynos7_drm_decon.c
index 206e779..4320b3b 100644
--- a/drivers/gpu/drm/exynos/exynos7_drm_decon.c
+++ b/drivers/gpu/drm/exynos/exynos7_drm_decon.c
@@ -642,7 +642,7 @@ static int decon_bind(struct device *dev, struct device *master, void *data)
 		ctx->configs[i].type = decon_win_types[i];
 
 		ret = exynos_plane_init(drm_dev, &ctx->planes[i], i,
-					1 << ctx->pipe, &ctx->configs[i]);
+					&ctx->configs[i]);
 		if (ret)
 			return ret;
 	}
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
index 588723d..ef27757 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
@@ -1009,7 +1009,7 @@ static int fimd_bind(struct device *dev, struct device *master, void *data)
 		ctx->configs[i].zpos = i;
 		ctx->configs[i].type = fimd_win_types[i];
 		ret = exynos_plane_init(drm_dev, &ctx->planes[i], i,
-					1 << ctx->pipe, &ctx->configs[i]);
+					&ctx->configs[i]);
 		if (ret)
 			return ret;
 	}
diff --git a/drivers/gpu/drm/exynos/exynos_drm_plane.c b/drivers/gpu/drm/exynos/exynos_drm_plane.c
index c2f17f3..611b6fd 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_plane.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_plane.c
@@ -273,14 +273,13 @@ static void exynos_plane_attach_zpos_property(struct drm_plane *plane,
 }
 
 int exynos_plane_init(struct drm_device *dev,
-		      struct exynos_drm_plane *exynos_plane,
-		      unsigned int index, unsigned long possible_crtcs,
+		      struct exynos_drm_plane *exynos_plane, unsigned int index,
 		      const struct exynos_drm_plane_config *config)
 {
 	int err;
 
 	err = drm_universal_plane_init(dev, &exynos_plane->base,
-				       possible_crtcs,
+				       1 << dev->mode_config.num_crtc,
 				       &exynos_plane_funcs,
 				       config->pixel_formats,
 				       config->num_pixel_formats,
diff --git a/drivers/gpu/drm/exynos/exynos_drm_plane.h b/drivers/gpu/drm/exynos/exynos_drm_plane.h
index 9aafad1..497047b 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_plane.h
+++ b/drivers/gpu/drm/exynos/exynos_drm_plane.h
@@ -11,5 +11,4 @@
 
 int exynos_plane_init(struct drm_device *dev,
 		      struct exynos_drm_plane *exynos_plane, unsigned int index,
-		      unsigned long possible_crtcs,
 		      const struct exynos_drm_plane_config *config);
diff --git a/drivers/gpu/drm/exynos/exynos_drm_vidi.c b/drivers/gpu/drm/exynos/exynos_drm_vidi.c
index cb5e3c6..8860ff4 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_vidi.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_vidi.c
@@ -408,7 +408,7 @@ static int vidi_bind(struct device *dev, struct device *master, void *data)
 		plane_config.type = vidi_win_types[i];
 
 		ret = exynos_plane_init(drm_dev, &ctx->planes[i], i,
-					1 << ctx->pipe, &plane_config);
+					&plane_config);
 		if (ret)
 			return ret;
 	}
diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c
index 1474982..3722b5f 100644
--- a/drivers/gpu/drm/exynos/exynos_mixer.c
+++ b/drivers/gpu/drm/exynos/exynos_mixer.c
@@ -1154,7 +1154,7 @@ static int mixer_bind(struct device *dev, struct device *manager, void *data)
 			continue;
 
 		ret = exynos_plane_init(drm_dev, &ctx->planes[i], i,
-					1 << ctx->pipe, &plane_configs[i]);
+					&plane_configs[i]);
 		if (ret)
 			return ret;
 	}
-- 
2.7.4

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

[PATCH v2 11/12] drm/exynos: kill pipe field from drivers contexts

[Andrzej Hajda]

Since possible_crtcs are set by Exynos core helper pipe fields have no
raison d'etre. The only place it was used, as a hack, is
fimd_clear_channels, to avoid calling drm_crtc_handle_vblank, but DRM core
has already other protection mechanism (vblank->enabled), so it could be
safely removed.

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
 drivers/gpu/drm/exynos/exynos5433_drm_decon.c |  2 --
 drivers/gpu/drm/exynos/exynos7_drm_decon.c    |  4 +---
 drivers/gpu/drm/exynos/exynos_drm_fimd.c      | 11 ++---------
 drivers/gpu/drm/exynos/exynos_drm_vidi.c      | 15 +--------------
 drivers/gpu/drm/exynos/exynos_mixer.c         |  2 --
 5 files changed, 4 insertions(+), 30 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
index a60d461..999d952 100644
--- a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
+++ b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
@@ -64,7 +64,6 @@ struct decon_context {
 	void __iomem			*addr;
 	struct regmap			*sysreg;
 	struct clk			*clks[ARRAY_SIZE(decon_clks_name)];
-	int				pipe;
 	unsigned long			flags;
 	unsigned long			out_type;
 	int				first_win;
@@ -592,7 +591,6 @@ static int decon_bind(struct device *dev, struct device *master, void *data)
 	int ret;
 
 	ctx->drm_dev = drm_dev;
-	ctx->pipe = drm_dev->mode_config.num_crtc;
 	drm_dev->max_vblank_count = 0xffffffff;
 
 	for (win = ctx->first_win; win < WINDOWS_NR; win++) {
diff --git a/drivers/gpu/drm/exynos/exynos7_drm_decon.c b/drivers/gpu/drm/exynos/exynos7_drm_decon.c
index 4320b3b..1ffb0b1 100644
--- a/drivers/gpu/drm/exynos/exynos7_drm_decon.c
+++ b/drivers/gpu/drm/exynos/exynos7_drm_decon.c
@@ -55,7 +55,6 @@ struct decon_context {
 	unsigned long			irq_flags;
 	bool				i80_if;
 	bool				suspended;
-	int				pipe;
 	wait_queue_head_t		wait_vsync_queue;
 	atomic_t			wait_vsync_event;
 
@@ -131,7 +130,6 @@ static int decon_ctx_initialize(struct decon_context *ctx,
 			struct drm_device *drm_dev)
 {
 	ctx->drm_dev = drm_dev;
-	ctx->pipe = drm_dev->mode_config.num_crtc;
 
 	decon_clear_channels(ctx->crtc);
 
@@ -605,7 +603,7 @@ static irqreturn_t decon_irq_handler(int irq, void *dev_id)
 		writel(clear_bit, ctx->regs + VIDINTCON1);
 
 	/* check the crtc is detached already from encoder */
-	if (ctx->pipe < 0 || !ctx->drm_dev)
+	if (!ctx->drm_dev)
 		goto out;
 
 	if (!ctx->i80_if) {
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
index ef27757..0b74e57 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
@@ -191,7 +191,6 @@ struct fimd_context {
 	u32				i80ifcon;
 	bool				i80_if;
 	bool				suspended;
-	int				pipe;
 	wait_queue_head_t		wait_vsync_queue;
 	atomic_t			wait_vsync_event;
 	atomic_t			win_updated;
@@ -368,18 +367,13 @@ static void fimd_clear_channels(struct exynos_drm_crtc *crtc)
 
 	/* Wait for vsync, as disable channel takes effect at next vsync */
 	if (ch_enabled) {
-		int pipe = ctx->pipe;
-
-		/* ensure that vblank interrupt won't be reported to core */
 		ctx->suspended = false;
-		ctx->pipe = -1;
 
 		fimd_enable_vblank(ctx->crtc);
 		fimd_wait_for_vblank(ctx->crtc);
 		fimd_disable_vblank(ctx->crtc);
 
 		ctx->suspended = true;
-		ctx->pipe = pipe;
 	}
 
 	clk_disable_unprepare(ctx->lcd_clk);
@@ -913,7 +907,7 @@ static void fimd_te_handler(struct exynos_drm_crtc *crtc)
 	u32 trg_type = ctx->driver_data->trg_type;
 
 	/* Checks the crtc is detached already from encoder */
-	if (ctx->pipe < 0 || !ctx->drm_dev)
+	if (!ctx->drm_dev)
 		return;
 
 	if (trg_type == I80_HW_TRG)
@@ -971,7 +965,7 @@ static irqreturn_t fimd_irq_handler(int irq, void *dev_id)
 		writel(clear_bit, ctx->regs + VIDINTCON1);
 
 	/* check the crtc is detached already from encoder */
-	if (ctx->pipe < 0 || !ctx->drm_dev)
+	if (!ctx->drm_dev)
 		goto out;
 
 	if (!ctx->i80_if)
@@ -1001,7 +995,6 @@ static int fimd_bind(struct device *dev, struct device *master, void *data)
 	int ret;
 
 	ctx->drm_dev = drm_dev;
-	ctx->pipe = drm_dev->mode_config.num_crtc;
 
 	for (i = 0; i < WINDOWS_NR; i++) {
 		ctx->configs[i].pixel_formats = fimd_formats;
diff --git a/drivers/gpu/drm/exynos/exynos_drm_vidi.c b/drivers/gpu/drm/exynos/exynos_drm_vidi.c
index 8860ff4..17b3ef7 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_vidi.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_vidi.c
@@ -51,7 +51,6 @@ struct vidi_context {
 	bool				suspended;
 	struct timer_list		timer;
 	struct mutex			lock;
-	int				pipe;
 };
 
 static inline struct vidi_context *encoder_to_vidi(struct drm_encoder *e)
@@ -153,15 +152,6 @@ static void vidi_disable(struct exynos_drm_crtc *crtc)
 	mutex_unlock(&ctx->lock);
 }
 
-static int vidi_ctx_initialize(struct vidi_context *ctx,
-			struct drm_device *drm_dev)
-{
-	ctx->drm_dev = drm_dev;
-	ctx->pipe = drm_dev->mode_config.num_crtc;
-
-	return 0;
-}
-
 static const struct exynos_drm_crtc_ops vidi_crtc_ops = {
 	.enable = vidi_enable,
 	.disable = vidi_disable,
@@ -175,9 +165,6 @@ static void vidi_fake_vblank_timer(unsigned long arg)
 {
 	struct vidi_context *ctx = (void *)arg;
 
-	if (ctx->pipe < 0)
-		return;
-
 	if (drm_crtc_handle_vblank(&ctx->crtc->base))
 		mod_timer(&ctx->timer,
 			jiffies + msecs_to_jiffies(VIDI_REFRESH_TIME) - 1);
@@ -398,7 +385,7 @@ static int vidi_bind(struct device *dev, struct device *master, void *data)
 	unsigned int i;
 	int pipe, ret;
 
-	vidi_ctx_initialize(ctx, drm_dev);
+	ctx->drm_dev = drm_dev;
 
 	plane_config.pixel_formats = formats;
 	plane_config.num_pixel_formats = ARRAY_SIZE(formats);
diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c
index 3722b5f..1cd84cb 100644
--- a/drivers/gpu/drm/exynos/exynos_mixer.c
+++ b/drivers/gpu/drm/exynos/exynos_mixer.c
@@ -99,7 +99,6 @@ struct mixer_context {
 	struct drm_device	*drm_dev;
 	struct exynos_drm_crtc	*crtc;
 	struct exynos_drm_plane	planes[MIXER_WIN_NR];
-	int			pipe;
 	unsigned long		flags;
 
 	struct mixer_resources	mixer_res;
@@ -900,7 +899,6 @@ static int mixer_initialize(struct mixer_context *mixer_ctx,
 	priv = drm_dev->dev_private;
 
 	mixer_ctx->drm_dev = drm_dev;
-	mixer_ctx->pipe = drm_dev->mode_config.num_crtc;
 
 	/* acquire resources: regs, irqs, clocks */
 	ret = mixer_resources_init(mixer_ctx);
-- 
2.7.4

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

[PATCH v2 12/12] drm/exynos: kill mode_set_nofb callback

[Andrzej Hajda]

All Exynos CRTCs are fully configured by .enable callback. The only users
of mode_set_nofb actually did nothing in their callbacks - they immediately
returned because devices were in suspend state - mode_set_nofb is always
called on disabled device.

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
 drivers/gpu/drm/exynos/exynos7_drm_decon.c |  1 -
 drivers/gpu/drm/exynos/exynos_drm_crtc.c   | 10 ----------
 drivers/gpu/drm/exynos/exynos_drm_drv.h    |  2 --
 drivers/gpu/drm/exynos/exynos_drm_fimd.c   |  1 -
 4 files changed, 14 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos7_drm_decon.c b/drivers/gpu/drm/exynos/exynos7_drm_decon.c
index 1ffb0b1..3e88269 100644
--- a/drivers/gpu/drm/exynos/exynos7_drm_decon.c
+++ b/drivers/gpu/drm/exynos/exynos7_drm_decon.c
@@ -581,7 +581,6 @@ static void decon_disable(struct exynos_drm_crtc *crtc)
 static const struct exynos_drm_crtc_ops decon_crtc_ops = {
 	.enable = decon_enable,
 	.disable = decon_disable,
-	.commit = decon_commit,
 	.enable_vblank = decon_enable_vblank,
 	.disable_vblank = decon_disable_vblank,
 	.atomic_begin = decon_atomic_begin,
diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.c b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
index 2d0aa11..d72777f 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_crtc.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
@@ -49,15 +49,6 @@ static void exynos_drm_crtc_disable(struct drm_crtc *crtc)
 	}
 }
 
-static void
-exynos_drm_crtc_mode_set_nofb(struct drm_crtc *crtc)
-{
-	struct exynos_drm_crtc *exynos_crtc = to_exynos_crtc(crtc);
-
-	if (exynos_crtc->ops->commit)
-		exynos_crtc->ops->commit(exynos_crtc);
-}
-
 static int exynos_crtc_atomic_check(struct drm_crtc *crtc,
 				     struct drm_crtc_state *state)
 {
@@ -93,7 +84,6 @@ static void exynos_crtc_atomic_flush(struct drm_crtc *crtc,
 static const struct drm_crtc_helper_funcs exynos_crtc_helper_funcs = {
 	.enable		= exynos_drm_crtc_enable,
 	.disable	= exynos_drm_crtc_disable,
-	.mode_set_nofb	= exynos_drm_crtc_mode_set_nofb,
 	.atomic_check	= exynos_crtc_atomic_check,
 	.atomic_begin	= exynos_crtc_atomic_begin,
 	.atomic_flush	= exynos_crtc_atomic_flush,
diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.h b/drivers/gpu/drm/exynos/exynos_drm_drv.h
index 7ef2b36..527bf1d 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_drv.h
+++ b/drivers/gpu/drm/exynos/exynos_drm_drv.h
@@ -115,7 +115,6 @@ struct exynos_drm_plane_config {
  *
  * @enable: enable the device
  * @disable: disable the device
- * @commit: set current hw specific display mode to hw.
  * @enable_vblank: specific driver callback for enabling vblank interrupt.
  * @disable_vblank: specific driver callback for disabling vblank interrupt.
  * @atomic_check: validate state
@@ -130,7 +129,6 @@ struct exynos_drm_crtc;
 struct exynos_drm_crtc_ops {
 	void (*enable)(struct exynos_drm_crtc *crtc);
 	void (*disable)(struct exynos_drm_crtc *crtc);
-	void (*commit)(struct exynos_drm_crtc *crtc);
 	int (*enable_vblank)(struct exynos_drm_crtc *crtc);
 	void (*disable_vblank)(struct exynos_drm_crtc *crtc);
 	u32 (*get_vblank_counter)(struct exynos_drm_crtc *crtc);
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
index 0b74e57..bf5135c 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
@@ -942,7 +942,6 @@ static void fimd_dp_clock_enable(struct exynos_drm_clk *clk, bool enable)
 static const struct exynos_drm_crtc_ops fimd_crtc_ops = {
 	.enable = fimd_enable,
 	.disable = fimd_disable,
-	.commit = fimd_commit,
 	.enable_vblank = fimd_enable_vblank,
 	.disable_vblank = fimd_disable_vblank,
 	.atomic_begin = fimd_atomic_begin,
-- 
2.7.4

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

Re: [PATCH v2 03/12] drm/exynos/decon5433: fix vblank event handling

[Michel Dänzer]

On 08/03/17 11:58 PM, Andrzej Hajda wrote:
> Current implementation of event handling assumes that vblank interrupt is
> always called at the right time. It is not true, it can be delayed due to
> various reasons. As a result different races can happen. The patch fixes
> the issue by using hardware frame counter present in DECON to serialize
> vblank and commit completion events.
> 
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> ---
> v2:
>   - added internal decon_get_frame_count function,
>   - updated frame counter on flush,
>   - misc style fixes (thank Inki)

[...]

> @@ -579,6 +636,23 @@ static const struct component_ops decon_component_ops = {
>  	.unbind = decon_unbind,
>  };
>  
> +static void decon_handle_vblank(struct decon_context *ctx)
> +{
> +	u32 frm;
> +
> +	spin_lock(&ctx->vblank_lock);
> +
> +	frm = decon_get_frame_count(ctx, true);
> +
> +	if (frm != ctx->frame_id) {
> +		if (frm > ctx->frame_id)
> +			drm_crtc_handle_vblank(&ctx->crtc->base);

This comparison isn't safe WRT the counter value returned by
decon_get_frame_count wrapping around. If it goes all the way up to
0xffffffff before wrapping around to zero, it can be handled e.g. by

		if ((s32)(frm - ctx->frame_id) > 0)

otherwise it gets a bit more complicated.


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 03/12] drm/exynos/decon5433: fix vblank event handling

[Andrzej Hajda]

On 09.03.2017 04:54, Michel Dänzer wrote:
> On 08/03/17 11:58 PM, Andrzej Hajda wrote:
>> Current implementation of event handling assumes that vblank interrupt is
>> always called at the right time. It is not true, it can be delayed due to
>> various reasons. As a result different races can happen. The patch fixes
>> the issue by using hardware frame counter present in DECON to serialize
>> vblank and commit completion events.
>>
>> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
>> ---
>> v2:
>>   - added internal decon_get_frame_count function,
>>   - updated frame counter on flush,
>>   - misc style fixes (thank Inki)
> [...]
>
>> @@ -579,6 +636,23 @@ static const struct component_ops decon_component_ops = {
>>  	.unbind = decon_unbind,
>>  };
>>  
>> +static void decon_handle_vblank(struct decon_context *ctx)
>> +{
>> +	u32 frm;
>> +
>> +	spin_lock(&ctx->vblank_lock);
>> +
>> +	frm = decon_get_frame_count(ctx, true);
>> +
>> +	if (frm != ctx->frame_id) {
>> +		if (frm > ctx->frame_id)
>> +			drm_crtc_handle_vblank(&ctx->crtc->base);
> This comparison isn't safe WRT the counter value returned by
> decon_get_frame_count wrapping around.

And knowing that max framerate is 60fps it will happen after:

0xffffffff / 60fps / 60sec / 60min / 24h / 365days = 2.3 years

So after 2.3 years of uninterrupted work of panel/TV we will lose one or
two vblanks :) Highly improbable and even then low damage.

> If it goes all the way up to
> 0xffffffff before wrapping around to zero, it can be handled e.g. by
>
> 		if ((s32)(frm - ctx->frame_id) > 0)
>
> otherwise it gets a bit more complicated.
>
>
But the fix looks simple so I think it is worth to fix it. Thanks.

Regards

Andrzej




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

Re: [PATCH v2 03/12] drm/exynos/decon5433: fix vblank event handling

[Inki Dae]

2017년 03월 09일 15:54에 Andrzej Hajda 이(가) 쓴 글:
> On 09.03.2017 04:54, Michel Dänzer wrote:
>> On 08/03/17 11:58 PM, Andrzej Hajda wrote:
>>> Current implementation of event handling assumes that vblank interrupt is
>>> always called at the right time. It is not true, it can be delayed due to
>>> various reasons. As a result different races can happen. The patch fixes
>>> the issue by using hardware frame counter present in DECON to serialize
>>> vblank and commit completion events.
>>>
>>> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
>>> ---
>>> v2:
>>>   - added internal decon_get_frame_count function,
>>>   - updated frame counter on flush,
>>>   - misc style fixes (thank Inki)
>> [...]
>>
>>> @@ -579,6 +636,23 @@ static const struct component_ops decon_component_ops = {
>>>  	.unbind = decon_unbind,
>>>  };
>>>  
>>> +static void decon_handle_vblank(struct decon_context *ctx)
>>> +{
>>> +	u32 frm;
>>> +
>>> +	spin_lock(&ctx->vblank_lock);
>>> +
>>> +	frm = decon_get_frame_count(ctx, true);
>>> +
>>> +	if (frm != ctx->frame_id) {
>>> +		if (frm > ctx->frame_id)
>>> +			drm_crtc_handle_vblank(&ctx->crtc->base);
>> This comparison isn't safe WRT the counter value returned by
>> decon_get_frame_count wrapping around.
> 
> And knowing that max framerate is 60fps it will happen after:
> 
> 0xffffffff / 60fps / 60sec / 60min / 24h / 365days = 2.3 years
> 
> So after 2.3 years of uninterrupted work of panel/TV we will lose one or
> two vblanks :) Highly improbable and even then low damage.
> 
>> If it goes all the way up to
>> 0xffffffff before wrapping around to zero, it can be handled e.g. by
>>
>> 		if ((s32)(frm - ctx->frame_id) > 0)
>>
>> otherwise it gets a bit more complicated.
>>
>>
> But the fix looks simple so I think it is worth to fix it. Thanks.

Andrzej, v3?

> 
> Regards
> 
> Andrzej
> 
> 
> 
> 
> 
> 

[PATCH v3 03/12] drm/exynos/decon5433: fix vblank event handling

[Andrzej Hajda]

Current implementation of event handling assumes that vblank interrupt is
always called at the right time. It is not true, it can be delayed due to
various reasons. As a result different races can happen. The patch fixes
the issue by using hardware frame counter present in DECON to serialize
vblank and commit completion events.

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
v3:
  - handle wrap-around of frame counter
v2:
  - added internal decon_get_frame_count function,
  - updated frame counter on flush,
  - misc style fixes (thank Inki)
---
 drivers/gpu/drm/exynos/exynos5433_drm_decon.c | 78 ++++++++++++++++++++++++++-
 include/video/exynos5433_decon.h              |  8 +++
 2 files changed, 85 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
index 147911e..73482d3 100644
--- a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
+++ b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
@@ -68,6 +68,8 @@ struct decon_context {
 	unsigned long			flags;
 	unsigned long			out_type;
 	int				first_win;
+	spinlock_t			vblank_lock;
+	u32				frame_id;
 };
 
 static const uint32_t decon_formats[] = {
@@ -122,6 +124,48 @@ static void decon_disable_vblank(struct exynos_drm_crtc *crtc)
 		writel(0, ctx->addr + DECON_VIDINTCON0);
 }
 
+/* return number of starts/ends of frame transmissions since reset */
+static u32 decon_get_frame_count(struct decon_context *ctx, bool end)
+{
+	u32 frm, pfrm, status, cnt = 2;
+
+	/* To get consistent result repeat read until frame id is stable.
+	 * Usually the loop will be executed once, in rare cases when the loop
+	 * is executed at frame change time 2nd pass will be needed.
+	 */
+	frm = readl(ctx->addr + DECON_CRFMID);
+	do {
+		status = readl(ctx->addr + DECON_VIDCON1);
+		pfrm = frm;
+		frm = readl(ctx->addr + DECON_CRFMID);
+	} while (frm != pfrm && --cnt);
+
+	/* CRFMID is incremented on BPORCH in case of I80 and on VSYNC in case
+	 * of RGB, it should be taken into account.
+	 */
+	if (!frm)
+		return 0;
+
+	switch (status & (VIDCON1_VSTATUS_MASK | VIDCON1_I80_ACTIVE)) {
+	case VIDCON1_VSTATUS_VS:
+		if (!(ctx->out_type & IFTYPE_I80))
+			--frm;
+		break;
+	case VIDCON1_VSTATUS_BP:
+		--frm;
+		break;
+	case VIDCON1_I80_ACTIVE:
+	case VIDCON1_VSTATUS_AC:
+		if (end)
+			--frm;
+		break;
+	default:
+		break;
+	}
+
+	return frm;
+}
+
 static void decon_setup_trigger(struct decon_context *ctx)
 {
 	if (!(ctx->out_type & (IFTYPE_I80 | I80_HW_TRG)))
@@ -365,11 +409,14 @@ static void decon_disable_plane(struct exynos_drm_crtc *crtc,
 static void decon_atomic_flush(struct exynos_drm_crtc *crtc)
 {
 	struct decon_context *ctx = crtc->ctx;
+	unsigned long flags;
 	int i;
 
 	if (test_bit(BIT_SUSPENDED, &ctx->flags))
 		return;
 
+	spin_lock_irqsave(&ctx->vblank_lock, flags);
+
 	for (i = ctx->first_win; i < WINDOWS_NR; i++)
 		decon_shadow_protect_win(ctx, i, false);
 
@@ -378,12 +425,18 @@ static void decon_atomic_flush(struct exynos_drm_crtc *crtc)
 
 	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);
+
+	spin_unlock_irqrestore(&ctx->vblank_lock, flags);
 }
 
 static void decon_swreset(struct decon_context *ctx)
 {
 	unsigned int tries;
+	unsigned long flags;
 
 	writel(0, ctx->addr + DECON_VIDCON0);
 	for (tries = 2000; tries; --tries) {
@@ -401,6 +454,10 @@ static void decon_swreset(struct decon_context *ctx)
 
 	WARN(tries == 0, "failed to software reset DECON\n");
 
+	spin_lock_irqsave(&ctx->vblank_lock, flags);
+	ctx->frame_id = 0;
+	spin_unlock_irqrestore(&ctx->vblank_lock, flags);
+
 	if (!(ctx->out_type & IFTYPE_HDMI))
 		return;
 
@@ -579,6 +636,24 @@ static const struct component_ops decon_component_ops = {
 	.unbind = decon_unbind,
 };
 
+static void decon_handle_vblank(struct decon_context *ctx)
+{
+	u32 frm;
+
+	spin_lock(&ctx->vblank_lock);
+
+	frm = decon_get_frame_count(ctx, true);
+
+	if (frm != ctx->frame_id) {
+		/* handle only if incremented, take care of wrap-around */
+		if ((s32)(frm - ctx->frame_id) > 0)
+			drm_crtc_handle_vblank(&ctx->crtc->base);
+		ctx->frame_id = frm;
+	}
+
+	spin_unlock(&ctx->vblank_lock);
+}
+
 static irqreturn_t decon_irq_handler(int irq, void *dev_id)
 {
 	struct decon_context *ctx = dev_id;
@@ -599,7 +674,7 @@ static irqreturn_t decon_irq_handler(int irq, void *dev_id)
 			    (VIDOUT_INTERLACE_EN_F | VIDOUT_INTERLACE_FIELD_F))
 				return IRQ_HANDLED;
 		}
-		drm_crtc_handle_vblank(&ctx->crtc->base);
+		decon_handle_vblank(ctx);
 	}
 
 out:
@@ -672,6 +747,7 @@ static int exynos5433_decon_probe(struct platform_device *pdev)
 	__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);
 
 	if (ctx->out_type & IFTYPE_HDMI) {
 		ctx->first_win = 1;
diff --git a/include/video/exynos5433_decon.h b/include/video/exynos5433_decon.h
index ef8e2a8..352fc0d 100644
--- a/include/video/exynos5433_decon.h
+++ b/include/video/exynos5433_decon.h
@@ -46,6 +46,7 @@
 #define DECON_FRAMEFIFO_STATUS		0x0524
 #define DECON_CMU			0x1404
 #define DECON_UPDATE			0x1410
+#define DECON_CRFMID			0x1414
 #define DECON_UPDATE_SCHEME		0x1438
 #define DECON_VIDCON1			0x2000
 #define DECON_VIDCON2			0x2004
@@ -142,6 +143,13 @@
 #define STANDALONE_UPDATE_F		(1 << 0)
 
 /* DECON_VIDCON1 */
+#define VIDCON1_LINECNT_MASK		(0x0fff << 16)
+#define VIDCON1_I80_ACTIVE		(1 << 15)
+#define VIDCON1_VSTATUS_MASK		(0x3 << 13)
+#define VIDCON1_VSTATUS_VS		(0 << 13)
+#define VIDCON1_VSTATUS_BP		(1 << 13)
+#define VIDCON1_VSTATUS_AC		(2 << 13)
+#define VIDCON1_VSTATUS_FP		(3 << 13)
 #define VIDCON1_VCLK_MASK		(0x3 << 9)
 #define VIDCON1_VCLK_RUN_VDEN_DISABLE	(0x3 << 9)
 #define VIDCON1_VCLK_HOLD		(0x0 << 9)
-- 
2.7.4

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

Re: [PATCH v2 00/12] drm/exynos: rework vblank handling

[Inki Dae]

Hi Andrzej,

Thanks for fixing this.

As you mentioned, this patch set depends on Shawn patch set[1].
So can you separate this patch series into two sets?

One includes only the patch set related to fixing page fault issue and other includes cleanup code based on Shawn patchset.

I'd like to merge fixup series to -fixes and for other one to -next.

[1] http://www.spinics.net/lists/dri-devel/msg131903.html

Thanks.

2017년 03월 08일 23:58에 Andrzej Hajda 이(가) 쓴 글:
> Hi Inki,
> 
> This patchset fixes long standing issue with occassional page faults
> or vblank event timeouts on TM2 targets due to delayed vblank handling.
> DECON driver should now handle properly all scenarios described in drm
> docs [1][2], at least it was my intention.
> 
> The patchset also:
> - adds frame counter callback,
> - fixes VBLANK interrupt trigger time,
> - fixes soft-trigger mask,
> - removes redundant pipe related fields.
> 
> The patch is based on todays linux-next, the real dependency is on the Shawn patchset
> 'Add vblank hooks to struct drm_crtc_funcs'.
> 
> I have successfully tested it on TM2 panel, TV and both.
> 
> [1]: https://dri.freedesktop.org/docs/drm/gpu/drm-kms.html?highlight=drm_crtc_state#c.drm_crtc_arm_vblank_event
> [2]: https://dri.freedesktop.org/docs/drm/gpu/drm-kms.html?highlight=drm_crtc_state#c.drm_crtc_state
> 
> Regards
> Andrzej
> 
> 
> Andrzej Hajda (12):
>   drm/exynos: move crtc event handling to drivers callbacks
>   drm/exynos: simplify completion event handling
>   drm/exynos/decon5433: fix vblank event handling
>   drm/exynos/decon5433: implement frame counter
>   drm/exynos/decon5433: signal frame done interrupt at front porch
>   drm/exynos/fimd: signal frame done interrupt at front porch
>   drm/exynos/decon5433: fix software trigger mask
>   drm/exynos: kill exynos_drm_crtc::pipe
>   drm/exynos: kill exynos_drm_private::pipe
>   drm/exynos: set plane possible_crtcs in exynos_plane_init
>   drm/exynos: kill pipe field from drivers contexts
>   drm/exynos: kill mode_set_nofb callback
> 
>  drivers/gpu/drm/exynos/exynos5433_drm_decon.c | 119 +++++++++++++++++++++-----
>  drivers/gpu/drm/exynos/exynos7_drm_decon.c    |  20 ++---
>  drivers/gpu/drm/exynos/exynos_drm_crtc.c      |  65 +++++++-------
>  drivers/gpu/drm/exynos/exynos_drm_crtc.h      |   3 +-
>  drivers/gpu/drm/exynos/exynos_drm_drv.h       |  14 +--
>  drivers/gpu/drm/exynos/exynos_drm_fimd.c      |  28 ++----
>  drivers/gpu/drm/exynos/exynos_drm_plane.c     |   5 +-
>  drivers/gpu/drm/exynos/exynos_drm_plane.h     |   1 -
>  drivers/gpu/drm/exynos/exynos_drm_vidi.c      |  23 +----
>  drivers/gpu/drm/exynos/exynos_mixer.c         |  14 +--
>  include/video/exynos5433_decon.h              |  12 +++
>  11 files changed, 166 insertions(+), 138 deletions(-)
> 
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel