All of lore.kernel.org
 help / color / mirror / Atom feed
From: Philipp Zabel <p.zabel@pengutronix.de>
To: Ying Liu <gnuiyl@gmail.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>,
	Peter Senna Tschudin <peter.senna@gmail.com>,
	Russell King <linux@armlinux.org.uk>,
	DRI Development <dri-devel@lists.freedesktop.org>
Subject: Re: [PATCH v4 4/7] gpu: ipu-v3: Do not wait for DMFC FIFO to clear when disabling DMFC channel
Date: Mon, 29 Aug 2016 11:46:27 +0200	[thread overview]
Message-ID: <1472463987.3054.26.camel@pengutronix.de> (raw)
In-Reply-To: <CAOcKUNUnaQA9-VNuzemFptPzjVU5UyYv3xB3NfPe43DXfjf=jg@mail.gmail.com>

Am Montag, den 29.08.2016, 17:36 +0800 schrieb Ying Liu:
> On Mon, Aug 29, 2016 at 4:46 PM, Philipp Zabel <p.zabel@pengutronix.de> wrote:
> > Am Freitag, den 26.08.2016, 15:30 +0800 schrieb Liu Ying:
> >> According to basic tests, it looks there is no issue if we don't wait for
> >> DMFC FIFO to clear when disabling DMFC channel.  NXP BSP doesn't do that,
> >> either.  This patch is needed to avoid the annoying warning caused by a
> >> timeout on waiting for the FIFO to clear after we add the new
> >> DRM_PLANE_COMMIT_NO_DISABLE_AFTER_MODESET flag to the imx-drm driver
> >> which changes the procedure to disable display channel slightly.
> >
> > I suppose the reason this happens is that now DC/DI are disabled first,
> > so the DC can't drain the FIFO anymore. If the FIFO is properly reset
> > when reenabling the DMFC, this shouldn't have any ill effects.
> 
> I found the timeout warning issue by blanking the framebuffer.
> Ofc, the framebuffer is supported by the fbdev emulation.
> Before applying this patch set, the planes are not even disabled
> when the framebuffer is blanked, that is to say, plane_funcs->
> atomic_disable is not called - the CRTC is disabled alone.
> After applying this patch set, the planes are always disabled
> together with the CRTC.  And, yes, DC/DI are disabled first,
> then the timeout warning happens.
> 
> Please note the warning happens when the planes are disabled
> instead of reenabled.  So, I don't get your point by resetting
> FIFO when _reenabling_  DMFC.

If we disable the DMFC with data still in the FIFO and then reenable it
and the DC first reads the stale data from the FIFO, that should cause a
visible artifact in the first frame after reenabling the plane. If that
doesn't happen, I trust that the hardware resets the FIFO state
somewhere along the way.

> And, I don't see the way to reset FIFO.

We could reset the DMFC_WR memory after disabling the DMFC, but I'm not
sure this is necessary at all.

regards
Philipp

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

  reply	other threads:[~2016-08-29  9:46 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-08-26  7:30 [PATCH v4 0/7] drm/imx: Add active plane reconfiguration support Liu Ying
2016-08-26  7:30 ` [PATCH v4 1/7] drm/atomic-helper: Add atomic_disable CRTC helper callback Liu Ying
2016-08-26  7:30 ` [PATCH v4 2/7] drm/atomic-helper: Disable appropriate planes in disable_planes_on_crtc() Liu Ying
2016-08-26  7:30 ` [PATCH v4 3/7] drm/atomic-helper: Add NO_DISABLE_AFTER_MODESET flag support for plane commit Liu Ying
2016-08-29  8:25   ` Daniel Vetter
2016-08-29  8:50     ` Ying Liu
2016-08-26  7:30 ` [PATCH v4 4/7] gpu: ipu-v3: Do not wait for DMFC FIFO to clear when disabling DMFC channel Liu Ying
2016-08-29  8:46   ` Philipp Zabel
2016-08-29  9:36     ` Ying Liu
2016-08-29  9:46       ` Philipp Zabel [this message]
2016-08-29  9:57         ` Ying Liu
2016-08-29 10:34           ` Philipp Zabel
2016-08-26  7:30 ` [PATCH v4 5/7] drm/imx: ipuv3-crtc: Use the callback ->atomic_disable instead of ->disable Liu Ying
2016-08-26  7:30 ` [PATCH v4 6/7] drm/imx: Use DRM_PLANE_COMMIT_NO_DISABLE_AFTER_MODESET flag Liu Ying
2016-08-26  7:30 ` [PATCH v4 7/7] drm/imx: Add active plane reconfiguration support Liu Ying
2016-08-29 10:53 ` [PATCH v4 0/7] " Philipp Zabel
2016-08-29 14:59   ` Philipp Zabel
2016-08-29 15:44     ` Daniel Vetter
2016-09-14 11:05       ` Philipp Zabel
2016-09-19  7:22         ` Daniel Vetter

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1472463987.3054.26.camel@pengutronix.de \
    --to=p.zabel@pengutronix.de \
    --cc=daniel.vetter@ffwll.ch \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=gnuiyl@gmail.com \
    --cc=linux@armlinux.org.uk \
    --cc=peter.senna@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.