dri-devel Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
To: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: dri-devel@lists.freedesktop.org
Subject: Re: [PATCH 03/33] HACK: drm/omap: fix memory barrier bug in DMM driver
Date: Tue, 23 Feb 2016 23:13:49 +0200	[thread overview]
Message-ID: <21417920.F8D2RjGPET@avalon> (raw)
In-Reply-To: <1455875288-4370-4-git-send-email-tomi.valkeinen@ti.com>

Hi Tomi,

Thank you for the patch.

On Friday 19 February 2016 11:47:38 Tomi Valkeinen wrote:
> A DMM timeout "timed out waiting for done" has been observed on DRA7
> devices. The timeout happens rarely, and only when the system is under
> heavy load.
> 
> Debugging showed that the timeout can be made to happen much more
> frequently by optimizing the DMM driver, so that there's almost no code
> between writing the last DMM descriptors to RAM, and writing to DMM
> register which starts the DMM transaction.
> 
> The current theory is that a wmb() does not properly ensure that the
> data written to RAM is observable by all the components in the system.
> 
> This DMM timeout has caused interesting (and rare) bugs as the error
> handling was not functioning properly (the error handling has been fixed
> in previous commits):
> 
>  * If a DMM timeout happened when a GEM buffer was being pinned for
>    display on the screen, a timeout error would be shown, but the driver
>    would continue programming DSS HW with broken buffer, leading to
>    SYNCLOST floods and possible crashes.
> 
>  * If a DMM timeout happened when other user (say, video decoder) was
>    pinning a GEM buffer, a timeout would be shown but if the user
>    handled the error properly, no other issues followed.
> 
>  * If a DMM timeout happened when a GEM buffer was being released, the
>    driver does not even notice the error, leading to crashes or hang
>    later.
> 
> This patch adds wmb() and readl() calls after the last bit is written to
> RAM, which should ensure that the execution proceeds only after the data
> is actually in RAM, and thus observable by DMM.
> 
> This patch is a HACK, as a read-back should not be needed. Further study
> is required to understand if DMM is somehow special case and read-back
> is ok, or if DRA7's memory barriers do not work correctly.

CONFIG_SOC_DRA7XX selects OMAP_INTERCONNECT and OMAP_INTERCONNECT_BARRIER, but 
dra7xx_map_io() doesn't call omap_barriers_init(). Could that be the root 
cause of the issue ? I don't have access to a DRA7xx system, would you be able 
to test that ?

> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> ---
>  drivers/gpu/drm/omapdrm/omap_dmm_tiler.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c
> b/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c index 80526dec7b2c..4e04f9487375
> 100644
> --- a/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c
> +++ b/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c
> @@ -262,6 +262,17 @@ static int dmm_txn_commit(struct dmm_txn *txn, bool
> wait) }
> 
>  	txn->last_pat->next_pa = 0;
> +	/* ensure that the written descriptors are visible to DMM */
> +	wmb();
> +
> +	/*
> +	 * NOTE: the wmb() above should be enough, but there seems to be a bug
> +	 * in OMAP's memory barrier implementation, which in some rare cases may
> +	 * cause the writes not to be observable after wmb().
> +	 */
> +
> +	/* read back to ensure the data is in RAM */
> +	readl(&txn->last_pat->next_pa);
> 
>  	/* write to PAT_DESCR to clear out any pending transaction */
>  	writel(0x0, dmm->base + reg[PAT_DESCR][engine->id]);

-- 
Regards,

Laurent Pinchart

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

  reply	other threads:[~2016-02-23 21:13 UTC|newest]

Thread overview: 65+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-02-19  9:47 [PATCH 00/33] drm/omap: patches for v4.6 Tomi Valkeinen
2016-02-19  9:47 ` [PATCH 01/33] drm/omap: HDMI: change enable/disable to avoid sync-losts Tomi Valkeinen
2016-02-23 10:22   ` Laurent Pinchart
2016-02-23 11:33     ` Tomi Valkeinen
2016-02-19  9:47 ` [PATCH 02/33] HACK: drm/omap: always use blocking DMM fill Tomi Valkeinen
2016-02-23 10:27   ` Laurent Pinchart
2016-02-23 13:09     ` Tomi Valkeinen
2016-02-19  9:47 ` [PATCH 03/33] HACK: drm/omap: fix memory barrier bug in DMM driver Tomi Valkeinen
2016-02-23 21:13   ` Laurent Pinchart [this message]
2016-02-24 10:34     ` Tomi Valkeinen
2016-02-19  9:47 ` [PATCH 04/33] drm/omap: add dmm_read() and dmm_write() wrappers Tomi Valkeinen
2016-02-23 21:18   ` Laurent Pinchart
2016-02-19  9:47 ` [PATCH 05/33] drm/omap: partial workaround for DRA7 DMM errata i878 Tomi Valkeinen
2016-02-23 21:57   ` Laurent Pinchart
2016-02-24  9:14     ` Tomi Valkeinen
2017-09-29 12:29     ` Peter Ujfalusi
2016-02-19  9:47 ` [PATCH 06/33] drm/omap: drm_atomic_get_plane_state() may return ERR_PTR Tomi Valkeinen
2016-02-23 22:01   ` Laurent Pinchart
2016-02-19  9:47 ` [PATCH 07/33] drm/omap: tpd12s015: remove platform data support Tomi Valkeinen
2016-02-19  9:47 ` [PATCH 08/33] drm/omap: tpd12s015: gpio descriptor API Tomi Valkeinen
2016-02-19  9:47 ` [PATCH 09/33] drm/omap: tpd12s015: CT_CP_HPD as optional gpio Tomi Valkeinen
2016-02-19  9:47 ` [PATCH 10/33] drm/omap: add define for DISPC_IRQ_WBUNCOMPLETEERROR Tomi Valkeinen
2016-02-23 22:04   ` Laurent Pinchart
2016-02-19  9:47 ` [PATCH 11/33] drm/omap: use dma_mapping_error in omap_gem_attach_pages Tomi Valkeinen
2016-02-23 22:10   ` Laurent Pinchart
2016-02-25 15:39     ` Tomi Valkeinen
2016-02-26  8:52       ` Laurent Pinchart
2016-02-26  9:07         ` Tomi Valkeinen
2016-02-19  9:47 ` [PATCH 12/33] drm/omap: use dma_mapping_error in omap_gem_dma_sync Tomi Valkeinen
2016-02-23 22:14   ` Laurent Pinchart
2016-02-25 15:45     ` Tomi Valkeinen
2016-02-26  8:54       ` Laurent Pinchart
2016-02-19  9:47 ` [PATCH 13/33] drm/omap: print an error if display enable fails Tomi Valkeinen
2016-02-23 22:17   ` Laurent Pinchart
2016-02-19  9:47 ` [PATCH 14/33] drm/omap: gem: Clean up GEM objects memory flags Tomi Valkeinen
2016-02-19  9:47 ` [PATCH 15/33] drm/omap: gem: Refactor GEM object allocation Tomi Valkeinen
2016-02-19  9:47 ` [PATCH 16/33] drm/omap: gem: Implement dma_buf import Tomi Valkeinen
2016-02-19  9:47 ` [PATCH 17/33] drm/omap: remove support for ext mem & sync Tomi Valkeinen
2016-02-23 22:42   ` Laurent Pinchart
2016-02-24  9:38     ` Tomi Valkeinen
2016-02-19  9:47 ` [PATCH 18/33] drm/omap: increase vblank wait timeout Tomi Valkeinen
2016-02-23 22:43   ` Laurent Pinchart
2016-02-24  9:41     ` Tomi Valkeinen
2016-02-19  9:47 ` [PATCH 19/33] drm/omap: DISPC: support double-pixel mode Tomi Valkeinen
2016-02-19  9:47 ` [PATCH 20/33] drm/omap: support double-pixel Tomi Valkeinen
2016-02-19  9:47 ` [PATCH 21/33] drm/omap: HDMI: support double-pixel pixel clock Tomi Valkeinen
2016-02-19  9:47 ` [PATCH 22/33] drm/omap: HDMI: Fix HSW value Tomi Valkeinen
2016-02-19  9:47 ` [PATCH 23/33] drm/omap: HDMI: fix WP timings for ilace Tomi Valkeinen
2016-02-19  9:47 ` [PATCH 24/33] drm/omap: DISPC: Fix field order for HDMI Tomi Valkeinen
2016-02-19  9:48 ` [PATCH 25/33] drm/omap: HDMI5: Fix FC HSW value Tomi Valkeinen
2016-02-19  9:48 ` [PATCH 26/33] drm/omap: HDMI5: clean up timings copy Tomi Valkeinen
2016-02-19  9:48 ` [PATCH 27/33] drm/omap: HDMI5: Add interlace support Tomi Valkeinen
2016-02-19  9:48 ` [PATCH 28/33] drm/omap: HDMI5: allow interlace Tomi Valkeinen
2016-02-19  9:48 ` [PATCH 29/33] drm/omap: verify that display x-res is divisible by 8 Tomi Valkeinen
2016-02-19  9:48 ` [PATCH 30/33] drm/omap: verify that fb plane pitches are the same Tomi Valkeinen
2016-02-23 23:02   ` Laurent Pinchart
2016-02-25 15:56     ` Tomi Valkeinen
2016-02-26  8:55       ` Laurent Pinchart
2016-02-26  9:12         ` Tomi Valkeinen
2016-02-26  9:27           ` Laurent Pinchart
2016-02-19  9:48 ` [PATCH 31/33] drm/omap: EBUSY status handling in omap_gem_fault() Tomi Valkeinen
2016-02-19  9:48 ` [PATCH 32/33] drm/omap: fix crtc->plane property delegation Tomi Valkeinen
2016-02-23 23:24   ` Laurent Pinchart
2016-02-19  9:48 ` [PATCH 33/33] drm/omap: check if rotation is supported before commit Tomi Valkeinen
2016-02-23 23:30   ` Laurent Pinchart

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=21417920.F8D2RjGPET@avalon \
    --to=laurent.pinchart@ideasonboard.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=tomi.valkeinen@ti.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox