[Intel-gfx] [PATCH 19/38] drm/hisilicon: Implement some semblance of vblank event handling
Daniel Vetter
daniel at ffwll.ch
Fri Jun 17 07:23:58 UTC 2016
On Fri, Jun 17, 2016 at 10:09:50AM +0800, Xinliang Liu wrote:
> Hi Daniel,
>
> I have tested your David's drm-next branch[1] which including this patch.
> In most time it is ok. But when switching modes or disable/re-enable
> mode, it will encounter bellow error msg:
> --
> [ 357.940728] [drm:drm_atomic_helper_commit_cleanup_done] *ERROR*
> [CRTC:24:crtc-0] flip_done timed out
> [ 368.004962] [drm:drm_atomic_helper_commit_cleanup_done] *ERROR*
> [CRTC:24:crtc-0] flip_done timed out
> [ 396.064871] INFO: rcu_preempt detected stalls on CPUs/tasks:
> [ 396.070548] 0-...: (3 GPs behind) idle=f4f/1/0 softirq=4253/4253 fqs=19
> [ 396.077335] 7-...: (1 GPs behind) idle=71f/140000000000000/0
> softirq=2444/2451 fqs=19
> [ 396.085332] (detected by 1, t=6028 jiffies, g=3924, c=3923, q=246)
> [ 396.091600] Task dump for CPU 0:
> [ 396.094821] swapper/0 R running task 0 0 0 0x00000002
> [ 396.101872] Call trace:
> [ 396.104323] [<ffff000008085bec>] __switch_to+0xa4/0xd4
> [ 396.109460] [<ffff000008c85000>] __boot_cpu_mode+0x0/0x80
> [ 396.114852] Task dump for CPU 7:
> [ 396.118072] Xorg R running task 0 1658 1646 0x00000002
> [ 396.125121] Call trace:
> [ 396.127562] [<ffff000008085c10>] __switch_to+0xc8/0xd4
> [ 396.132695] [<ffff800035567110>] 0xffff800035567110
> [ 396.137569] rcu_preempt kthread starved for 1000 jiffies! g3924
> c3923 f0x0 RCU_GP_WAIT_FQS(3) ->state=0x1
> [ 396.147130] rcu_preempt S ffff000008085c10 0 7 2 0x00000000
> [ 396.154180] Call trace:
> [ 396.156620] [<ffff000008085c10>] __switch_to+0xc8/0xd4
> [ 396.161758] [<ffff000008810490>] __schedule+0x188/0x590
> [ 396.166978] [<ffff0000088108d4>] schedule+0x3c/0xa0
> [ 396.171851] [<ffff00000881368c>] schedule_timeout+0x104/0x1a4
> [ 396.177595] [<ffff00000810549c>] rcu_gp_kthread+0x54c/0x814
> [ 396.183164] [<ffff0000080d3e5c>] kthread+0xd4/0xe8
> [ 396.187951] [<ffff000008084e10>] ret_from_fork+0x10/0x40
> --
>
> Then the console stuck. Any tips for addressing this issue?
> I am running a debian system.
>
> [1] git://people.freedesktop.org/~airlied/linux drm-next
hisilicon doesn't handle crtc_state->event correctly. Most likely when
shutting down a CRTC if fails to send out that flip event, which means the
waiting for flip_done times out. My patch tried to fix that (it's not
correct, but it did work on other drivers).
>From a quick look what's wrong with hisilicon vblank handling:
- You don't call drm_crtc_vblank_on/off, which means the core thinks
vblanks will keep working even when the CRTC is off. That throws off the
hack in my patch. You need to put a call to drm_crtc_vblank_off into
crtc->disable hook, and drm_crtc_vblank_on into crtc->enable.
- While at it please review that the event sending is placed correctly and
can't race with the new buffers showing up on the screen. The event
should be signalled at exactly the time the buffers start to get scanned
out. The important bit is to make sure that even if something races or
gets delayed that it still happens together.
Cheers, Daniel
>
> Thanks,
> -xinliang
>
> On 2 June 2016 at 06:06, Daniel Vetter <daniel.vetter at ffwll.ch> wrote:
> > atomic_flush seems to be the right place, but I'm not entirely sure
> > whether this will catch them all. It could be that when disabling the
> > crtc we'll miss the vblank.
> >
> > While at it nuke the dummy functions.
> >
> > v2: Be more robust and either arm, when the CRTC is on, or just send
> > the event out right away.
> >
> > Cc: Xinliang Liu <xinliang.liu at linaro.org>
> > Cc: Xinwei Kong <kong.kongxinwei at hisilicon.com>
> > Cc: Archit Taneja <architt at codeaurora.org>
> > Signed-off-by: Daniel Vetter <daniel.vetter at intel.com>
>
> Acked-by: Xinliang Liu <xinliang.liu at linaro.org>
>
> > ---
> > drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c | 20 ++++++++++++--------
> > 1 file changed, 12 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c
> > index fba6372d060e..ed76baad525f 100644
> > --- a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c
> > +++ b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c
> > @@ -502,13 +502,6 @@ static void ade_crtc_disable(struct drm_crtc *crtc)
> > acrtc->enable = false;
> > }
> >
> > -static int ade_crtc_atomic_check(struct drm_crtc *crtc,
> > - struct drm_crtc_state *state)
> > -{
> > - /* do nothing */
> > - return 0;
> > -}
> > -
> > static void ade_crtc_mode_set_nofb(struct drm_crtc *crtc)
> > {
> > struct ade_crtc *acrtc = to_ade_crtc(crtc);
> > @@ -537,6 +530,7 @@ static void ade_crtc_atomic_flush(struct drm_crtc *crtc,
> > {
> > struct ade_crtc *acrtc = to_ade_crtc(crtc);
> > struct ade_hw_ctx *ctx = acrtc->ctx;
> > + struct drm_pending_vblank_event *event = crtc->state->event;
> > void __iomem *base = ctx->base;
> >
> > /* only crtc is enabled regs take effect */
> > @@ -545,12 +539,22 @@ static void ade_crtc_atomic_flush(struct drm_crtc *crtc,
> > /* flush ade registers */
> > writel(ADE_ENABLE, base + ADE_EN);
> > }
> > +
> > + if (event) {
> > + crtc->state->event = NULL;
> > +
> > + spin_lock_irq(&crtc->dev->event_lock);
> > + if (drm_crtc_vblank_get(crtc) == 0)
> > + drm_crtc_arm_vblank_event(crtc, event);
> > + else
> > + drm_crtc_send_vblank_event(crtc, event);
> > + spin_unlock_irq(&crtc->dev->event_lock);
> > + }
> > }
> >
> > static const struct drm_crtc_helper_funcs ade_crtc_helper_funcs = {
> > .enable = ade_crtc_enable,
> > .disable = ade_crtc_disable,
> > - .atomic_check = ade_crtc_atomic_check,
> > .mode_set_nofb = ade_crtc_mode_set_nofb,
> > .atomic_begin = ade_crtc_atomic_begin,
> > .atomic_flush = ade_crtc_atomic_flush,
> > --
> > 2.8.1
> >
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
More information about the Intel-gfx
mailing list