From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
To: Sebastian Reichel <sre@kernel.org>
Cc: Tony Lindgren <tony@atomide.com>, Aaro Koskinen <aaro.koskinen@iki.fi>,
        Tomi Valkeinen <tomi.valkeinen@ti.com>,
        David Airlie <airlied@linux.ie>, linux-omap@vger.kernel.org,
        dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 06/23] drm: omapdrm: wait for pending operations before updating plane
Date: Sat, 26 Mar 2016 11:20 +0200
Message-ID: <2247354.2fBg1ifxEX@avalon>
User-Agent: KMail/4.14.10 (Linux/4.1.15-gentoo-r1; KDE/4.14.16; x86_64; ; )
In-Reply-To: <1457455195-1938-7-git-send-email-sre@kernel.org>
References: <1457455195-1938-1-git-send-email-sre@kernel.org> <1457455195-1938-7-git-send-email-sre@kernel.org>
MIME-Version: 1.0
Content-Transfer-Encoding: 7Bit
Content-Type: text/plain; charset="us-ascii"
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1286
Lines: 39

Hi Sabastian,

Thank you for the patch.

On Tuesday 08 Mar 2016 17:39:38 Sebastian Reichel wrote:
> Updating the plane may interrupt ongoing display
> updates, so wait for any pending operations.

There's already an omap_atomic_wait_for_completion() call a couple of lines 
below, do we need two of them ? Why can display update be ongoing there, given 
that the previous omap_atomic_complete() call did wait for completion before 
returning ?

> Signed-off-By: Sebastian Reichel <sre@kernel.org>
> ---
>  drivers/gpu/drm/omapdrm/omap_drv.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c
> b/drivers/gpu/drm/omapdrm/omap_drv.c index a3ff35f5f6cd..e142a4245766
> 100644
> --- a/drivers/gpu/drm/omapdrm/omap_drv.c
> +++ b/drivers/gpu/drm/omapdrm/omap_drv.c
> @@ -95,6 +95,10 @@ static void omap_atomic_complete(struct
> omap_atomic_state_commit *commit) /* Apply the atomic update. */
>  	dispc_runtime_get();
> 
> +	dev_dbg(dev->dev, "omap_atomic_complete");
> +
> +	omap_atomic_wait_for_completion(dev, old_state);
> +
>  	drm_atomic_helper_commit_modeset_disables(dev, old_state);
>  	drm_atomic_helper_commit_planes(dev, old_state, false);
>  	drm_atomic_helper_commit_modeset_enables(dev, old_state);

-- 
Regards,

Laurent Pinchart