* [PATCH 0/4] drm/exynos: rework vblank handling [not found] <CGME20170222160515eucas1p207f5f7b3a956b56405f547befd4daf63@eucas1p2.samsung.com> @ 2017-02-22 16:05 ` Andrzej Hajda [not found] ` <CGME20170222160515eucas1p2ced846577ae3e435ffb2e17fa60fbd6e@eucas1p2.samsung.com> ` (3 more replies) 0 siblings, 4 replies; 10+ messages in thread From: Andrzej Hajda @ 2017-02-22 16:05 UTC (permalink / raw) To: Inki Dae Cc: Andrzej Hajda, Bartlomiej Zolnierkiewicz, Marek Szyprowski, dri-devel, linux-samsung-soc, Krzysztof Kozlowski, Javier Martinez Canillas 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. 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 (4): 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: signal frame done interrupt at VSYNC drivers/gpu/drm/exynos/exynos5433_drm_decon.c | 63 +++++++++++++++++++++++++-- drivers/gpu/drm/exynos/exynos7_drm_decon.c | 1 + drivers/gpu/drm/exynos/exynos_drm_crtc.c | 32 +++++++------- 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 + include/video/exynos5433_decon.h | 13 ++++++ 8 files changed, 96 insertions(+), 19 deletions(-) -- 2.7.4 ^ permalink raw reply [flat|nested] 10+ messages in thread
[parent not found: <CGME20170222160515eucas1p2ced846577ae3e435ffb2e17fa60fbd6e@eucas1p2.samsung.com>]
* [PATCH 1/4] drm/exynos: move crtc event handling to drivers callbacks [not found] ` <CGME20170222160515eucas1p2ced846577ae3e435ffb2e17fa60fbd6e@eucas1p2.samsung.com> @ 2017-02-22 16:05 ` Andrzej Hajda 0 siblings, 0 replies; 10+ messages in thread From: Andrzej Hajda @ 2017-02-22 16:05 UTC (permalink / raw) To: Inki Dae Cc: Andrzej Hajda, Bartlomiej Zolnierkiewicz, Marek Szyprowski, dri-devel, linux-samsung-soc, Krzysztof Kozlowski, Javier Martinez Canillas 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 5367b66..c65f450 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 6a581a8..abd5d6c 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_crtc.h +++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.h @@ -40,4 +40,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 ^ permalink raw reply related [flat|nested] 10+ messages in thread
[parent not found: <CGME20170222160516eucas1p298a994725044878022fece1600f9e36a@eucas1p2.samsung.com>]
* [PATCH 2/4] drm/exynos: simplify completion event handling [not found] ` <CGME20170222160516eucas1p298a994725044878022fece1600f9e36a@eucas1p2.samsung.com> @ 2017-02-22 16:05 ` Andrzej Hajda 0 siblings, 0 replies; 10+ messages in thread From: Andrzej Hajda @ 2017-02-22 16:05 UTC (permalink / raw) To: Inki Dae Cc: Andrzej Hajda, Bartlomiej Zolnierkiewicz, Marek Szyprowski, dri-devel, linux-samsung-soc, Krzysztof Kozlowski, Javier Martinez Canillas 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 c65f450..5b7cd77 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 ^ permalink raw reply related [flat|nested] 10+ messages in thread
[parent not found: <CGME20170222160516eucas1p102bcb93b7f0356cfafbb707987b9e85f@eucas1p1.samsung.com>]
* [PATCH 3/4] drm/exynos/decon5433: fix vblank event handling [not found] ` <CGME20170222160516eucas1p102bcb93b7f0356cfafbb707987b9e85f@eucas1p1.samsung.com> @ 2017-02-22 16:05 ` Andrzej Hajda 2017-03-07 9:12 ` Inki Dae 0 siblings, 1 reply; 10+ messages in thread From: Andrzej Hajda @ 2017-02-22 16:05 UTC (permalink / raw) To: Inki Dae Cc: Andrzej Hajda, Bartlomiej Zolnierkiewicz, Marek Szyprowski, dri-devel, linux-samsung-soc, Krzysztof Kozlowski, Javier Martinez Canillas 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> --- drivers/gpu/drm/exynos/exynos5433_drm_decon.c | 62 +++++++++++++++++++++++++-- include/video/exynos5433_decon.h | 9 ++++ 2 files changed, 67 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c index 147911e..bfa9396 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[] = { @@ -365,25 +367,32 @@ 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); + if (ctx->out_type & IFTYPE_I80) + set_bit(BIT_WIN_UPDATED, &ctx->flags); + if (test_and_clear_bit(BIT_REQUEST_UPDATE, &ctx->flags)) decon_set_bits(ctx, DECON_UPDATE, STANDALONE_UPDATE_F, ~0); - if (ctx->out_type & IFTYPE_I80) - set_bit(BIT_WIN_UPDATED, &ctx->flags); - exynos_crtc_handle_event(crtc); + exynos_crtc_handle_event(ctx->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 +410,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 +592,46 @@ static const struct component_ops decon_component_ops = { .unbind = decon_unbind, }; +static void decon_handle_vblank(struct decon_context *ctx) +{ + u32 frm, pfrm, status, cnt; + + spin_lock(&ctx->vblank_lock); + + /* To get consistent result repeat read until frame id is stable. */ + frm = readl(ctx->addr + DECON_CRFMID); + cnt = 3; + do { + status = readl(ctx->addr + DECON_VIDCON1); + pfrm = frm; + frm = readl(ctx->addr + DECON_CRFMID); + } while (frm != pfrm && --cnt); + + status &= VIDCON1_VSTATUS_MASK | VIDCON1_I80_ACTIVE; + + /* In case of delayed vblank CRFMID could be already incremented, + * it should be taken into account. + */ + if (frm > 0) + switch (status) { + case VIDCON1_VSTATUS_VS: + if (ctx->out_type & IFTYPE_I80) + break; + case VIDCON1_I80_ACTIVE: + case VIDCON1_VSTATUS_BP: + case VIDCON1_VSTATUS_AC: + --frm; + } + + 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 +652,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 +725,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..beefc62 100644 --- a/include/video/exynos5433_decon.h +++ b/include/video/exynos5433_decon.h @@ -46,6 +46,8 @@ #define DECON_FRAMEFIFO_STATUS 0x0524 #define DECON_CMU 0x1404 #define DECON_UPDATE 0x1410 +#define DECON_CRFMID 0x1414 +#define DECON_RRFRMID 0x1418 #define DECON_UPDATE_SCHEME 0x1438 #define DECON_VIDCON1 0x2000 #define DECON_VIDCON2 0x2004 @@ -142,6 +144,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 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 3/4] drm/exynos/decon5433: fix vblank event handling 2017-02-22 16:05 ` [PATCH 3/4] drm/exynos/decon5433: fix vblank " Andrzej Hajda @ 2017-03-07 9:12 ` Inki Dae 2017-03-07 9:58 ` Andrzej Hajda 0 siblings, 1 reply; 10+ messages in thread From: Inki Dae @ 2017-03-07 9:12 UTC (permalink / raw) To: Andrzej Hajda Cc: linux-samsung-soc, Bartlomiej Zolnierkiewicz, Krzysztof Kozlowski, Javier Martinez Canillas, dri-devel, Marek Szyprowski Thanks for fixing it. Andrzej, DECON_CRFMID register is new to me. Where did you refer this register description from? I couldn't find this register in datasheet I have for Exynos5433. Below are a little bit trivial comments. 2017년 02월 23일 01:05에 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> > --- > drivers/gpu/drm/exynos/exynos5433_drm_decon.c | 62 +++++++++++++++++++++++++-- > include/video/exynos5433_decon.h | 9 ++++ > 2 files changed, 67 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c > index 147911e..bfa9396 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[] = { > @@ -365,25 +367,32 @@ 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); > > + if (ctx->out_type & IFTYPE_I80) > + set_bit(BIT_WIN_UPDATED, &ctx->flags); > + > if (test_and_clear_bit(BIT_REQUEST_UPDATE, &ctx->flags)) > decon_set_bits(ctx, DECON_UPDATE, STANDALONE_UPDATE_F, ~0); > > - if (ctx->out_type & IFTYPE_I80) > - set_bit(BIT_WIN_UPDATED, &ctx->flags); > - exynos_crtc_handle_event(crtc); > + exynos_crtc_handle_event(ctx->crtc); You don't have to change 'crtc' to 'ctx->crtc'. Keep '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 +410,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 +592,46 @@ static const struct component_ops decon_component_ops = { > .unbind = decon_unbind, > }; > > +static void decon_handle_vblank(struct decon_context *ctx) > +{ > + u32 frm, pfrm, status, cnt; > + > + spin_lock(&ctx->vblank_lock); > + > + /* To get consistent result repeat read until frame id is stable. */ > + frm = readl(ctx->addr + DECON_CRFMID); > + cnt = 3; Is there some guide that initial value of cnt should be 3? > + do { > + status = readl(ctx->addr + DECON_VIDCON1); > + pfrm = frm; > + frm = readl(ctx->addr + DECON_CRFMID); > + } while (frm != pfrm && --cnt); > + > + status &= VIDCON1_VSTATUS_MASK | VIDCON1_I80_ACTIVE; I couldn't find I80_ACTIVE field on DECON_VIDCON1 register descrption. Do you have other datasheet, new one? > + > + /* In case of delayed vblank CRFMID could be already incremented, > + * it should be taken into account. > + */ > + if (frm > 0) > + switch (status) { > + case VIDCON1_VSTATUS_VS: > + if (ctx->out_type & IFTYPE_I80) > + break; > + case VIDCON1_I80_ACTIVE: > + case VIDCON1_VSTATUS_BP: > + case VIDCON1_VSTATUS_AC: > + --frm; Let's add 'break;' and 'default: break;' explicitly. > + } > + > + 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 +652,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 +725,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..beefc62 100644 > --- a/include/video/exynos5433_decon.h > +++ b/include/video/exynos5433_decon.h > @@ -46,6 +46,8 @@ > #define DECON_FRAMEFIFO_STATUS 0x0524 > #define DECON_CMU 0x1404 > #define DECON_UPDATE 0x1410 > +#define DECON_CRFMID 0x1414 > +#define DECON_RRFRMID 0x1418 Above definition is not used in this patch. Thanks. > #define DECON_UPDATE_SCHEME 0x1438 > #define DECON_VIDCON1 0x2000 > #define DECON_VIDCON2 0x2004 > @@ -142,6 +144,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) > _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 3/4] drm/exynos/decon5433: fix vblank event handling 2017-03-07 9:12 ` Inki Dae @ 2017-03-07 9:58 ` Andrzej Hajda 2017-03-08 1:12 ` Inki Dae 0 siblings, 1 reply; 10+ messages in thread From: Andrzej Hajda @ 2017-03-07 9:58 UTC (permalink / raw) To: Inki Dae Cc: linux-samsung-soc, Bartlomiej Zolnierkiewicz, Krzysztof Kozlowski, Javier Martinez Canillas, dri-devel, Marek Szyprowski On 07.03.2017 10:12, Inki Dae wrote: > Thanks for fixing it. > > Andrzej, > DECON_CRFMID register is new to me. Where did you refer this register description from? I couldn't find this register in datasheet I have for Exynos5433. I have found it in android sources. > > Below are a little bit trivial comments. > > 2017년 02월 23일 01:05에 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> >> --- >> drivers/gpu/drm/exynos/exynos5433_drm_decon.c | 62 +++++++++++++++++++++++++-- >> include/video/exynos5433_decon.h | 9 ++++ >> 2 files changed, 67 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c >> index 147911e..bfa9396 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[] = { >> @@ -365,25 +367,32 @@ 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); >> >> + if (ctx->out_type & IFTYPE_I80) >> + set_bit(BIT_WIN_UPDATED, &ctx->flags); >> + >> if (test_and_clear_bit(BIT_REQUEST_UPDATE, &ctx->flags)) >> decon_set_bits(ctx, DECON_UPDATE, STANDALONE_UPDATE_F, ~0); >> >> - if (ctx->out_type & IFTYPE_I80) >> - set_bit(BIT_WIN_UPDATED, &ctx->flags); >> - exynos_crtc_handle_event(crtc); >> + exynos_crtc_handle_event(ctx->crtc); > You don't have to change 'crtc' to 'ctx->crtc'. Keep 'crtc'. OK > >> + >> + 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 +410,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 +592,46 @@ static const struct component_ops decon_component_ops = { >> .unbind = decon_unbind, >> }; >> >> +static void decon_handle_vblank(struct decon_context *ctx) >> +{ >> + u32 frm, pfrm, status, cnt; >> + >> + spin_lock(&ctx->vblank_lock); >> + >> + /* To get consistent result repeat read until frame id is stable. */ >> + frm = readl(ctx->addr + DECON_CRFMID); >> + cnt = 3; > Is there some guide that initial value of cnt should be 3? No, this is my arbitrary choice. In general the loop will be passed only once. In rare case when code runs at frame change time it will be run twice. It never should run more than two times - it would be sign of HW error, incorrect DECON programming or serious bottleneck. I initially left cnt=3 to detect such case, but after all I did not report such situation, so I can either change cnt to 2, or add error log if after loop cnt is 0, what do you prefer? > >> + do { >> + status = readl(ctx->addr + DECON_VIDCON1); >> + pfrm = frm; >> + frm = readl(ctx->addr + DECON_CRFMID); >> + } while (frm != pfrm && --cnt); >> + >> + status &= VIDCON1_VSTATUS_MASK | VIDCON1_I80_ACTIVE; > I couldn't find I80_ACTIVE field on DECON_VIDCON1 register descrption. Do you have other datasheet, new one? No, this is just result of my hardware analysis/debugging. > >> + >> + /* In case of delayed vblank CRFMID could be already incremented, >> + * it should be taken into account. >> + */ >> + if (frm > 0) >> + switch (status) { >> + case VIDCON1_VSTATUS_VS: >> + if (ctx->out_type & IFTYPE_I80) >> + break; >> + case VIDCON1_I80_ACTIVE: >> + case VIDCON1_VSTATUS_BP: >> + case VIDCON1_VSTATUS_AC: >> + --frm; > Let's add 'break;' and 'default: break;' explicitly. OK, anyway this code will be superseded by frame counter patch. > >> + } >> + >> + 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 +652,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 +725,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..beefc62 100644 >> --- a/include/video/exynos5433_decon.h >> +++ b/include/video/exynos5433_decon.h >> @@ -46,6 +46,8 @@ >> #define DECON_FRAMEFIFO_STATUS 0x0524 >> #define DECON_CMU 0x1404 >> #define DECON_UPDATE 0x1410 >> +#define DECON_CRFMID 0x1414 >> +#define DECON_RRFRMID 0x1418 > Above definition is not used in this patch. I have added it to make DECON registry space better documented. Of course I will remove it if you like, as it does not serves any purpose at the moment. Regards Andrzej > > Thanks. > >> #define DECON_UPDATE_SCHEME 0x1438 >> #define DECON_VIDCON1 0x2000 >> #define DECON_VIDCON2 0x2004 >> @@ -142,6 +144,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) >> > _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 3/4] drm/exynos/decon5433: fix vblank event handling 2017-03-07 9:58 ` Andrzej Hajda @ 2017-03-08 1:12 ` Inki Dae 0 siblings, 0 replies; 10+ messages in thread From: Inki Dae @ 2017-03-08 1:12 UTC (permalink / raw) To: Andrzej Hajda Cc: linux-samsung-soc, Bartlomiej Zolnierkiewicz, Krzysztof Kozlowski, Javier Martinez Canillas, dri-devel, Marek Szyprowski 2017년 03월 07일 18:58에 Andrzej Hajda 이(가) 쓴 글: > On 07.03.2017 10:12, Inki Dae wrote: >> Thanks for fixing it. >> >> Andrzej, >> DECON_CRFMID register is new to me. Where did you refer this register description from? I couldn't find this register in datasheet I have for Exynos5433. > > I have found it in android sources. > >> >> Below are a little bit trivial comments. >> >> 2017년 02월 23일 01:05에 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> >>> --- >>> drivers/gpu/drm/exynos/exynos5433_drm_decon.c | 62 +++++++++++++++++++++++++-- >>> include/video/exynos5433_decon.h | 9 ++++ >>> 2 files changed, 67 insertions(+), 4 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c >>> index 147911e..bfa9396 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[] = { >>> @@ -365,25 +367,32 @@ 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); >>> >>> + if (ctx->out_type & IFTYPE_I80) >>> + set_bit(BIT_WIN_UPDATED, &ctx->flags); >>> + >>> if (test_and_clear_bit(BIT_REQUEST_UPDATE, &ctx->flags)) >>> decon_set_bits(ctx, DECON_UPDATE, STANDALONE_UPDATE_F, ~0); >>> >>> - if (ctx->out_type & IFTYPE_I80) >>> - set_bit(BIT_WIN_UPDATED, &ctx->flags); >>> - exynos_crtc_handle_event(crtc); >>> + exynos_crtc_handle_event(ctx->crtc); >> You don't have to change 'crtc' to 'ctx->crtc'. Keep 'crtc'. > > OK > >> >>> + >>> + 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 +410,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 +592,46 @@ static const struct component_ops decon_component_ops = { >>> .unbind = decon_unbind, >>> }; >>> >>> +static void decon_handle_vblank(struct decon_context *ctx) >>> +{ >>> + u32 frm, pfrm, status, cnt; >>> + >>> + spin_lock(&ctx->vblank_lock); >>> + >>> + /* To get consistent result repeat read until frame id is stable. */ >>> + frm = readl(ctx->addr + DECON_CRFMID); >>> + cnt = 3; >> Is there some guide that initial value of cnt should be 3? > > No, this is my arbitrary choice. In general the loop will be passed only > once. In rare case when code runs at frame change time it will be run > twice. It never should run more than two times - it would be sign of HW > error, incorrect DECON programming or serious bottleneck. > I initially left cnt=3 to detect such case, but after all I did not > report such situation, so I can either change cnt to 2, or add error log > if after loop cnt is 0, what do you prefer? I don't care. I just wondered why cnt should be 3. :) It'd be better to comment why you set cnt to 3. Seems enough with above your comment. Thanks. > >> >>> + do { >>> + status = readl(ctx->addr + DECON_VIDCON1); >>> + pfrm = frm; >>> + frm = readl(ctx->addr + DECON_CRFMID); >>> + } while (frm != pfrm && --cnt); >>> + >>> + status &= VIDCON1_VSTATUS_MASK | VIDCON1_I80_ACTIVE; >> I couldn't find I80_ACTIVE field on DECON_VIDCON1 register descrption. Do you have other datasheet, new one? > > No, this is just result of my hardware analysis/debugging. > >> >>> + >>> + /* In case of delayed vblank CRFMID could be already incremented, >>> + * it should be taken into account. >>> + */ >>> + if (frm > 0) >>> + switch (status) { >>> + case VIDCON1_VSTATUS_VS: >>> + if (ctx->out_type & IFTYPE_I80) >>> + break; >>> + case VIDCON1_I80_ACTIVE: >>> + case VIDCON1_VSTATUS_BP: >>> + case VIDCON1_VSTATUS_AC: >>> + --frm; >> Let's add 'break;' and 'default: break;' explicitly. > > OK, anyway this code will be superseded by frame counter patch. > >> >>> + } >>> + >>> + 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 +652,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 +725,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..beefc62 100644 >>> --- a/include/video/exynos5433_decon.h >>> +++ b/include/video/exynos5433_decon.h >>> @@ -46,6 +46,8 @@ >>> #define DECON_FRAMEFIFO_STATUS 0x0524 >>> #define DECON_CMU 0x1404 >>> #define DECON_UPDATE 0x1410 >>> +#define DECON_CRFMID 0x1414 >>> +#define DECON_RRFRMID 0x1418 >> Above definition is not used in this patch. > > I have added it to make DECON registry space better documented. > Of course I will remove it if you like, as it does not serves any > purpose at the moment. > > Regards > Andrzej > > >> >> Thanks. >> >>> #define DECON_UPDATE_SCHEME 0x1438 >>> #define DECON_VIDCON1 0x2000 >>> #define DECON_VIDCON2 0x2004 >>> @@ -142,6 +144,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) >>> >> > > > _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 10+ messages in thread
[parent not found: <CGME20170222160517eucas1p1a254801abe01ae5ee77681ff4f955ba8@eucas1p1.samsung.com>]
* [PATCH 4/4] drm/exynos/decon5433: signal frame done interrupt at VSYNC [not found] ` <CGME20170222160517eucas1p1a254801abe01ae5ee77681ff4f955ba8@eucas1p1.samsung.com> @ 2017-02-22 16:05 ` Andrzej Hajda 2017-03-07 9:14 ` Inki Dae 0 siblings, 1 reply; 10+ messages in thread From: Andrzej Hajda @ 2017-02-22 16:05 UTC (permalink / raw) To: Inki Dae Cc: Andrzej Hajda, Bartlomiej Zolnierkiewicz, Marek Szyprowski, dri-devel, linux-samsung-soc, Krzysztof Kozlowski, Javier Martinez Canillas 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 bfa9396..2694b32 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 beefc62..2391b03 100644 --- a/include/video/exynos5433_decon.h +++ b/include/video/exynos5433_decon.h @@ -128,6 +128,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 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 4/4] drm/exynos/decon5433: signal frame done interrupt at VSYNC 2017-02-22 16:05 ` [PATCH 4/4] drm/exynos/decon5433: signal frame done interrupt at VSYNC Andrzej Hajda @ 2017-03-07 9:14 ` Inki Dae 2017-03-08 10:47 ` Andrzej Hajda 0 siblings, 1 reply; 10+ messages in thread From: Inki Dae @ 2017-03-07 9:14 UTC (permalink / raw) To: Andrzej Hajda Cc: Bartlomiej Zolnierkiewicz, Marek Szyprowski, dri-devel, linux-samsung-soc, Krzysztof Kozlowski, Javier Martinez Canillas 2017년 02월 23일 01:05에 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 bfa9396..2694b32 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 beefc62..2391b03 100644 > --- a/include/video/exynos5433_decon.h > +++ b/include/video/exynos5433_decon.h > @@ -128,6 +128,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) Above definitions are not used. Thanks. > +#define VIDINTCON0_FRAMESEL_FP (3 << 15) > #define VIDINTCON0_INTFRMEN (1 << 12) > #define VIDINTCON0_INTEN (1 << 0) > > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 4/4] drm/exynos/decon5433: signal frame done interrupt at VSYNC 2017-03-07 9:14 ` Inki Dae @ 2017-03-08 10:47 ` Andrzej Hajda 0 siblings, 0 replies; 10+ messages in thread From: Andrzej Hajda @ 2017-03-08 10:47 UTC (permalink / raw) To: Inki Dae Cc: linux-samsung-soc, Bartlomiej Zolnierkiewicz, Krzysztof Kozlowski, Javier Martinez Canillas, dri-devel, Marek Szyprowski On 07.03.2017 10:14, Inki Dae wrote: > > 2017년 02월 23일 01:05에 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 bfa9396..2694b32 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 beefc62..2391b03 100644 >> --- a/include/video/exynos5433_decon.h >> +++ b/include/video/exynos5433_decon.h >> @@ -128,6 +128,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) > Above definitions are not used. Yes, similarly to multiple other definitions in this file and as in many other files with HW registers related definitions. I can remove it if you prefer, but maybe it would be better to leave it as it documents the hardware. Regards Andrzej > > Thanks. > >> +#define VIDINTCON0_FRAMESEL_FP (3 << 15) >> #define VIDINTCON0_INTFRMEN (1 << 12) >> #define VIDINTCON0_INTEN (1 << 0) >> >> > _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2017-03-08 10:47 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <CGME20170222160515eucas1p207f5f7b3a956b56405f547befd4daf63@eucas1p2.samsung.com>
2017-02-22 16:05 ` [PATCH 0/4] drm/exynos: rework vblank handling Andrzej Hajda
[not found] ` <CGME20170222160515eucas1p2ced846577ae3e435ffb2e17fa60fbd6e@eucas1p2.samsung.com>
2017-02-22 16:05 ` [PATCH 1/4] drm/exynos: move crtc event handling to drivers callbacks Andrzej Hajda
[not found] ` <CGME20170222160516eucas1p298a994725044878022fece1600f9e36a@eucas1p2.samsung.com>
2017-02-22 16:05 ` [PATCH 2/4] drm/exynos: simplify completion event handling Andrzej Hajda
[not found] ` <CGME20170222160516eucas1p102bcb93b7f0356cfafbb707987b9e85f@eucas1p1.samsung.com>
2017-02-22 16:05 ` [PATCH 3/4] drm/exynos/decon5433: fix vblank " Andrzej Hajda
2017-03-07 9:12 ` Inki Dae
2017-03-07 9:58 ` Andrzej Hajda
2017-03-08 1:12 ` Inki Dae
[not found] ` <CGME20170222160517eucas1p1a254801abe01ae5ee77681ff4f955ba8@eucas1p1.samsung.com>
2017-02-22 16:05 ` [PATCH 4/4] drm/exynos/decon5433: signal frame done interrupt at VSYNC Andrzej Hajda
2017-03-07 9:14 ` Inki Dae
2017-03-08 10:47 ` Andrzej Hajda
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.