Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756259AbbLAObU (ORCPT ); Tue, 1 Dec 2015 09:31:20 -0500 Received: from mailgw02.mediatek.com ([210.61.82.184]:48474 "EHLO mailgw02.mediatek.com" rhost-flags-OK-FAIL-OK-FAIL) by vger.kernel.org with ESMTP id S1755605AbbLAObS (ORCPT ); Tue, 1 Dec 2015 09:31:18 -0500 X-Listener-Flag: 11101 Message-ID: <1448980272.25447.6.camel@mtksdaap41> Subject: Re: [RESEND RFC/PATCH 3/8] media: platform: mtk-vpu: Support Mediatek VPU From: andrew-ct chen To: Daniel Thompson CC: Mark Rutland , James Liao , Catalin Marinas , Will Deacon , Darren Etheridge , Laurent Pinchart , Eddie Huang , Pawel Moll , Hongzhou Yang , Mauro Carvalho Chehab , Fabien Dessenne , "Peter Griffin" , Geert Uytterhoeven , Mikhail Ulyanov , Hans Verkuil , , "devicetree@vger.kernel.org" , Arnd Bergmann , Ian Campbell , Sascha Hauer , Benoit Parrot , Rob Herring , , Yingjoe Chen , Matthias Brugger , Tiffany Lin , "linux-arm-kernel@lists.infradead.org" , Daniel Hsiao , lkml , "Sakari Ailus" , Kumar Gala Date: Tue, 1 Dec 2015 22:31:12 +0800 In-Reply-To: References: <1447764885-23100-1-git-send-email-tiffany.lin@mediatek.com> <1447764885-23100-4-git-send-email-tiffany.lin@mediatek.com> <5655DDB4.2080002@linaro.org> <1448626209.7734.26.camel@mtksdaap41> <56584AC5.7020704@linaro.org> <1448883819.15961.7.camel@mtksdaap41> Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.2.3-0ubuntu6 Content-Transfer-Encoding: 7bit MIME-Version: 1.0 X-MTK: N Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3512 Lines: 90 On Mon, 2015-11-30 at 15:36 +0000, Daniel Thompson wrote: > On 30 November 2015 at 11:43, andrew-ct chen > wrote: > > On Fri, 2015-11-27 at 12:21 +0000, Daniel Thompson wrote: > >> On 27/11/15 12:10, andrew-ct chen wrote: > >> >>> + > >> >>> > >+ memcpy((void *)send_obj->share_buf, buf, len); > >> >>> > >+ send_obj->len = len; > >> >>> > >+ send_obj->id = id; > >> >>> > >+ vpu_cfg_writel(vpu, 0x1, HOST_TO_VPU); > >> >>> > >+ > >> >>> > >+ /* Wait until VPU receives the command */ > >> >>> > >+ timeout = jiffies + msecs_to_jiffies(IPI_TIMEOUT_MS); > >> >>> > >+ do { > >> >>> > >+ if (time_after(jiffies, timeout)) { > >> >>> > >+ dev_err(vpu->dev, "vpu_ipi_send: IPI timeout!\n"); > >> >>> > >+ return -EIO; > >> >>> > >+ } > >> >>> > >+ } while (vpu_cfg_readl(vpu, HOST_TO_VPU)); > >> >> > > >> >> >Do we need to busy wait every time we communicate with the co-processor? > >> >> >Couldn't we put this wait*before* we write to HOST_TO_VPU above. > >> >> > > >> >> >That way we only spin when there is a need to. > >> >> > > >> > Since the hardware VPU only allows that one client sends the command to > >> > it each time. > >> > We need the wait to make sure VPU accepted the command and cleared the > >> > interrupt and then the next command would be served. > >> > >> I understand that the VPU can only have on message outstanding at once. > >> > >> I just wonder why we busy wait *after* sending the first command rather > >> than *before* sending the second one. > > > > No other special reasons. Just send one command and wait until VPU gets > > the command. Then, I think this wait also can be put before we write to > > HOST_TO_VPU.Is this better than former? May I know the reason? > > Busy waiting is bad; it is a waste of host CPU processor time and/or power. > > When the busy wait occurs after queuing then we will busy wait for > every command we send. > > If busy wait occurs before next queuing then we will wait for a > shorter time in total because we have the chance to do something > useful on the host before we have to wait. > Got it. Thanks a lot for the explanation. I'll put the busy wait before next queuing in next version. > > >> Streamed decode/encode typically ends up being rate controlled by > >> capture or display meaning that in these cases we don't need to busy > >> wait at all (because by the time we send the next frame the VPU has > >> already accepted the previous message). > > > > For now, only one device "encoder" exists, it is true. > > But, we'll have encoder and decoder devices, the decode and encode > > requested to VPU are simultaneous. > > Sure, I accept that lock and busy-wait is an appropriate way to ensure > safety (assuming the VPU is fairly quick clearing the HOST_TO_VPU > flag). > The busy wait time is less than 1ms in average. > > > Is this supposed to be removed for this patches and we can add it back > > if the another device(decoder) is ready for review? > > No I'm not suggesting that. > > I only recommend moving the busy wait *before* end sending of command > (for efficiency reasons mentioned above). Ok. Thanks. > > > Daniel. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/