From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753406AbcARBkk (ORCPT ); Sun, 17 Jan 2016 20:40:40 -0500 Received: from regular1.263xmail.com ([211.150.99.138]:60573 "EHLO regular1.263xmail.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753159AbcARBkh (ORCPT ); Sun, 17 Jan 2016 20:40:37 -0500 X-263anti-spam: KSV:0;BIG:0;ABS:1;DNS:0;ATT:0;SPF:S; X-MAIL-GRAY: 0 X-MAIL-DELIVERY: 1 X-KSVirus-check: 0 X-ABS-CHECKED: 1 X-SKE-CHECKED: 1 X-ADDR-CHECKED: 0 X-RL-SENDER: mark.yao@rock-chips.com X-FST-TO: linux-arm-kernel@lists.infradead.org X-SENDER-IP: 58.22.7.114 X-LOGIN-NAME: mark.yao@rock-chips.com X-UNIQUE-TAG: <7882ddb88f23732f7f891e67b0a6b6ba> X-ATTACHMENT-NUM: 0 X-DNS-TYPE: 0 Message-ID: <569C4290.1030902@rock-chips.com> Date: Mon, 18 Jan 2016 09:40:32 +0800 From: Mark yao User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.8.0 MIME-Version: 1.0 To: John Keeping , Thierry Reding CC: Linux Kernel Mailing List , dri-devel , "open list:ARM/Rockchip SoC..." , "linux-arm-kernel@lists.infradead.org" Subject: Re: [PATCH 3/3] drm/rockchip: explain why we can't wait_for_vblanks References: <20160114142047.GD19130@phenom.ffwll.local> <107bbc36a316ed0ddc7b5a8bcd9b6db6cbc71d4f.1452782114.git.john@metanate.com> <20160114145705.GA24005@ulmo> <20160114162619.233ab39c.john@metanate.com> In-Reply-To: <20160114162619.233ab39c.john@metanate.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 2016年01月15日 00:26, John Keeping wrote: > On Thu, 14 Jan 2016 15:57:05 +0100, Thierry Reding wrote: > >> On Thu, Jan 14, 2016 at 02:39:42PM +0000, John Keeping wrote: >>> Signed-off-by: John Keeping >>> --- >>> drivers/gpu/drm/rockchip/rockchip_drm_fb.c | 6 ++++++ >>> 1 file changed, 6 insertions(+) >>> >>> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c >>> index 679d23a..b267ce4 100644 >>> --- a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c >>> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c >>> @@ -177,6 +177,12 @@ static void rockchip_crtc_wait_for_update(struct drm_crtc *crtc) >>> crtc_funcs->wait_for_update(crtc); >>> } >>> >>> +/* >>> + * We can't use drm_atomic_helper_wait_for_vblanks() because rk3288 and rk3066 >>> + * have hardware counters for neither vblanks nor scanlines. This function is >>> + * equivalent but uses rockchip_crtc_wait_for_update() instead of waiting for >>> + * vblank_count to change. >>> + */ >> This is kind of misleading. From reading earlier parts of the thread the >> reason why drm_atomic_helper_wait_for_vblanks() won't work is because it >> has a potential race condition that can't be detected unless you also >> have a vblank counter. However, the above comment makes it work like >> drm_atomic_helper_wait_for_vblanks() doesn't work in the absence of a >> vblank counter, which isn't quite true. > How about something like this (using the sequence from Mark's message): > > /* > * We can't use drm_atomic_helper_wait_for_vblanks() because rk3288 and rk3066 > * have hardware counters for neither vblanks nor scanlines, which results in > * a race where: > * | <-- HW vsync irq and reg take effect > * plane_commit --> | > * get_vblank and wait --> | > * | <-- handle_vblank, vblank->count + 1 > * cleanup_fb --> | > * iommu crash --> | > * | <-- HW vsync irq and reg take effect > * > * This function is equivalent but uses rockchip_crtc_wait_for_update() instead > * of waiting for vblank_count to change. > */ > > > Looks good for me, but maybe Thierry has some more advices. :-) -- Ｍark Yao