Re: [v2] drm/komeda: Workaround for broken FLIP_COMPLETE timestamps

From: "james qian wang (Arm Technology China)" <james.qian.wang@arm.com>
To: Mihail Atanassov <Mihail.Atanassov@arm.com>
Cc: Maxime Ripard <maxime.ripard@bootlin.com>,
	Liviu Dudau <Liviu.Dudau@arm.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"dri-devel@lists.freedesktop.org"
	<dri-devel@lists.freedesktop.org>,
	David Airlie <airlied@linux.ie>, nd <nd@arm.com>,
	Sean Paul <sean@poorly.run>
Subject: Re: [v2] drm/komeda: Workaround for broken FLIP_COMPLETE timestamps
Date: Sun, 29 Sep 2019 06:18:56 +0000	[thread overview]
Message-ID: <20190929061849.GA29382@jamwan02-TSP300> (raw)
In-Reply-To: <20190923101017.35114-1-mihail.atanassov@arm.com>

On Mon, Sep 23, 2019 at 10:10:26AM +0000, Mihail Atanassov wrote:
> When initially turning a crtc on, drm_reset_vblank_timestamp will
> set the vblank timestamp to 0 for any driver that doesn't provide
> a ->get_vblank_timestamp() hook.
> 
> Unfortunately, the FLIP_COMPLETE event depends on that timestamp,
> and the only way to regenerate a valid one is to have vblank
> interrupts enabled and have a valid in-ISR call to
> drm_crtc_handle_vblank.
> 
> Additionally, if the user doesn't request vblanks but _does_ request
> FLIP_COMPLETE events, we still don't have a good timestamp: it'll be the
> same stamp as the last vblank one.
> 
> Work around the issue by always enabling vblanks when the CRTC is on.
> Reducing the amount of time that PL0 has to be unmasked would be nice to
> fix at a later time.
> 
> Changes since v1 [https://patchwork.freedesktop.org/patch/331727/]:
>  - moved drm_crtc_vblank_put call to the ->atomic_disable() hook
> 
> Cc: Daniel Vetter <daniel@ffwll.ch>
> Cc: Liviu Dudau <Liviu.Dudau@arm.com>
> Signed-off-by: Mihail Atanassov <mihail.atanassov@arm.com>
> ---
>  drivers/gpu/drm/arm/display/komeda/komeda_crtc.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
> index 34bc73ca18bc..d06679afb228 100644
> --- a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
> @@ -489,6 +489,7 @@ komeda_crtc_atomic_enable(struct drm_crtc *crtc,
>  	pm_runtime_get_sync(crtc->dev->dev);
>  	komeda_crtc_prepare(to_kcrtc(crtc));
>  	drm_crtc_vblank_on(crtc);
> +	WARN_ON(drm_crtc_vblank_get(crtc));
>  	komeda_crtc_do_flush(crtc, old);
>  }
>  
> @@ -581,6 +582,7 @@ komeda_crtc_atomic_disable(struct drm_crtc *crtc,
>  		komeda_crtc_flush_and_wait_for_flip_done(kcrtc, disable_done);
>  	}
>  
> +	drm_crtc_vblank_put(crtc);
>  	drm_crtc_vblank_off(crtc);
>  	komeda_crtc_unprepare(kcrtc);
>  	pm_runtime_put(crtc->dev->dev);

Looks good to me.

Reviewed-by: James Qian Wang (Arm Technology China) <james.qian.wang@arm.com>

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