Received: by 2002:a05:6a10:a0d1:0:0:0:0 with SMTP id j17csp1585215pxa; Thu, 6 Aug 2020 10:53:26 -0700 (PDT) X-Google-Smtp-Source: ABdhPJzbEbShXF1chkfcq5CB/uhti8Epm7MIST9N+SVXJPstujnckZ9YB441i7qsnd6m7fi9haNw X-Received: by 2002:a50:eac5:: with SMTP id u5mr5279028edp.6.1596736406223; Thu, 06 Aug 2020 10:53:26 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1596736406; cv=none; d=google.com; s=arc-20160816; b=HnyiVtmdxDcaiSRfS5U20GLmcWuQWXnEpWQOqXDgwhpPzAqJxiHq4o6+q88CmAwFhw xq4aHLS4GcwEHFwusYe5sFU9hkZTSw2raDbb9cLs0b9ug/D69QV7Tk/+GYA7ZVTb35yg CTZbj5Ae2UIWIpdCWJg4MUpa5Bxgm8V40bBKG19iOpZdVttXMDAP6Mj/ohTJza6/vKpv sgCBttNMo3i2UbwhhmjcHiyERo3A05W4ZUpsrGMJSW4r+3xzXx2MxzabTTf+bJH/p+cz xT2vzCbhUQbWUtrRrmgo9Tv+DrfUhFcPyQyvOxbhyGC8e9nFHz9isNci3dKWiWKUobN0 CSXg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:dkim-signature:content-language :content-transfer-encoding:in-reply-to:mime-version:user-agent:date :message-id:references:cc:to:from:subject; bh=CS+W4xiYVifNlDvQtAoKvNJ1SxgXVP8asa+INgRDbKA=; b=VaE5QPgcjE9m8iLvFJWGvOojk7kwTpcdB6618BJP7WRG3Ls6X6p1PYkfbFCjReeIvd Vn0ExbNdph0JLbjbwhq6XVdd0CRMQJxCf7JLVhgtkYLZaVeYE5mpwfMIWUbQgMxeRqU2 uqrEv0WGn14uBAVn43MTlrRt73+zWGHLCPgBowg4pyMU6RzoXQunqHeZMnFwMrqAIZGS sgt4ctR/63Z3mi/S8oZcdA7f8XiwMp5GY+sy4hnXVI9ORxMPuFTBOF5ZqVklqw096NDo VYHptLwHoazzMBcBS+AV2aoQkAuYRW8VRD3Rx1ZNbsfuQtgVppqLeof3SxRlDQPHANQX gMJw== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@nvidia.com header.s=n1 header.b=K6nhdIPU; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=nvidia.com Return-Path: Received: from vger.kernel.org (vger.kernel.org. [23.128.96.18]) by mx.google.com with ESMTP id bt2si3629920ejb.597.2020.08.06.10.53.03; Thu, 06 Aug 2020 10:53:26 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18; Authentication-Results: mx.google.com; dkim=pass header.i=@nvidia.com header.s=n1 header.b=K6nhdIPU; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=nvidia.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1730426AbgHFRwl (ORCPT + 99 others); Thu, 6 Aug 2020 13:52:41 -0400 Received: from hqnvemgate24.nvidia.com ([216.228.121.143]:2682 "EHLO hqnvemgate24.nvidia.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1728399AbgHFRwj (ORCPT ); Thu, 6 Aug 2020 13:52:39 -0400 Received: from hqpgpgate101.nvidia.com (Not Verified[216.228.121.13]) by hqnvemgate24.nvidia.com (using TLS: TLSv1.2, DES-CBC3-SHA) id ; Thu, 06 Aug 2020 10:50:56 -0700 Received: from hqmail.nvidia.com ([172.20.161.6]) by hqpgpgate101.nvidia.com (PGP Universal service); Thu, 06 Aug 2020 10:52:37 -0700 X-PGP-Universal: processed; by hqpgpgate101.nvidia.com on Thu, 06 Aug 2020 10:52:37 -0700 Received: from [10.2.172.190] (10.124.1.5) by HQMAIL107.nvidia.com (172.20.187.13) with Microsoft SMTP Server (TLS) id 15.0.1473.3; Thu, 6 Aug 2020 17:52:36 +0000 Subject: Re: [PATCH v8 08/10] gpu: host1x: mipi: Keep MIPI clock enabled till calibration is done From: Sowjanya Komatineni To: Dmitry Osipenko , Thierry Reding CC: , , , , , , , , , , References: <1596469346-937-1-git-send-email-skomatineni@nvidia.com>

<6eede805-80fd-016f-22f8-b6d25f6587af@nvidia.com> <1c12e40e-de7f-0599-a941-82760b4c7668@gmail.com> <9ef0b875-e826-43e2-207e-168d2081ff6a@nvidia.com> <4689cfe9-e7c4-48bf-217f-3a31b59b8bda@nvidia.com> <0e78c5ca-c529-1e98-891d-30351c9aae81@gmail.com> <309e3b66-9288-91ef-71b4-be73eacbbd62@nvidia.com> <4025a458-fa78-924d-c84f-166f82df0f8e@gmail.com> <4f15d655-3d62-cf9f-82da-eae379d60fa6@nvidia.com> <412f8c53-1aca-db31-99a1-a0ecb2081ca5@nvidia.com> <61275bd6-58e7-887f-aa7d-8e60895e7b2b@nvidia.com> <6ff57c38-9847-42b0-643b-0d167c13779f@gmail.com> Message-ID: Date: Thu, 6 Aug 2020 10:52:38 -0700 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.10.0 MIME-Version: 1.0 In-Reply-To: X-Originating-IP: [10.124.1.5] X-ClientProxiedBy: HQMAIL101.nvidia.com (172.20.187.10) To HQMAIL107.nvidia.com (172.20.187.13) Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: quoted-printable Content-Language: en-US DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=nvidia.com; s=n1; t=1596736256; bh=CS+W4xiYVifNlDvQtAoKvNJ1SxgXVP8asa+INgRDbKA=; h=X-PGP-Universal:Subject:From:To:CC:References:Message-ID:Date: User-Agent:MIME-Version:In-Reply-To:X-Originating-IP: X-ClientProxiedBy:Content-Type:Content-Transfer-Encoding: Content-Language; b=K6nhdIPU86mcumxZoU9u24TPt4lhoYatIyHCBBHXSuGOa21WFcwdL8nwnBkAeUxKD EW9RTS/+PXiKpJsVBqhs9gy/PEpqXddrFTSpoWE+Ebm9ep5EX86+khaLWRWzfKyrUW m1bg3xIVfYXMYGYdILfuS1R03yh2VtHN+AItP1Ddyxxx8+1saSZQuAvNUNJ/jc2PgN zSjC/0/czumePpKc1Zm4bZ4/yb1yzXQUlSP8RdbO7z7BCcr6Ed7dW5uaGpb5/5v8hZ Jnqg4obHOggVcX5xoFbniTd0I1Mwaf5st48dk3mEk9gITCBObnwkvBtBHo7/s22JBf wadD5eQywyG5g== Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 8/6/20 10:44 AM, Sowjanya Komatineni wrote: > > On 8/6/20 10:27 AM, Dmitry Osipenko wrote: >> 06.08.2020 20:12, Sowjanya Komatineni =D0=BF=D0=B8=D1=88=D0=B5=D1=82: >>> On 8/6/20 9:41 AM, Sowjanya Komatineni wrote: >>>> On 8/6/20 9:10 AM, Dmitry Osipenko wrote: >>>>> 06.08.2020 18:59, Sowjanya Komatineni =D0=BF=D0=B8=D1=88=D0=B5=D1=82: >>>>> ... >>>>>>>> Confirmed from HW designer, calibration FSM to finish takes worst >>>>>>>> case >>>>>>>> 72uS so by the time it gets to sensor stream it will be done its >>>>>>>> sequence and will be waiting for DONE bit. >>>>>>>> >>>>>>>> So disabling MIPI CAL clock on sensor stream fails is safe. >>>>>>> 72us is quite a lot of time, what will happen if LP-11 happens=20 >>>>>>> before >>>>>>> FSM finished calibration? >>>>>>> >>>>>>> Maybe the finish_calibration() needs to split into two parts: >>>>>>> >>>>>>> =C2=A0=C2=A0=C2=A0 1. wait for CAL_STATUS_ACTIVE before enabling se= nsor >>>>>>> =C2=A0=C2=A0=C2=A0 2. wait for CAL_STATUS_DONE after enabling senso= r >>>>>> I don't think we need to split for active and done. Active will=20 >>>>>> be 1 as >>>>>> long as other pads are in calibration as well. >>>>>> >>>>>> We cant use active status check for specific pads under calibration. >>>>>> This is common bit for all pads. >>>>> Does hardware have a single FSM block shared by all pads or there=20 >>>>> is FSM >>>>> per group of pads? >>>> MIPI CAL status register has DONE bits for individual pads status and >>>> single ACTIVE bit. >>>> >>>> ACTIVE bit set to 1 indicates auto calibration is active which is the >>>> case even when other pads (other CSI pads from other ports streaming >>>> in case of parallel stream) are under calibration. Also DSI is shared >>>> as well. >>>> >>>> We do calibration for individual pads. So, we should not rely on >>>> ACTIVE bit. >>>> >>>> >>>> MIPI driver checks for condition ACTIVE =3D=3D 1 && DONE =3D=3D 1 from= the >>>> beginning. >>>> >>>> But I think this also should be fixed as in case of parallel streams >>>> calibration can happen in parallel waiting for ACTIVE to be cleared >>>> makes all calibration callers to wait for longer than needed as ACTIVE >>>> is common for all pads. >>>> >>>>>> Unfortunately HW don't have separate status indicating when=20 >>>>>> sequence is >>>>>> done to indicate its waiting for LP11. >>>>>> >>>>>> >>>>>> To avoid all this, will remove cancel_calibration() totally and use >>>>>> same >>>>>> finish calibration even in case of stream failure then. >>>>>> >>>>> What about to add 72us delay to the end of start_calibration() in=20 >>>>> order >>>>> to ensure that FSM is finished before LP-11? >>>> Why we should add 72uS in start_calibration() when can use same >>>> finish_calibration() for both pass/fail cases? >>>> >>>> Only timing loose we see is in case of failure we still wait for 250ms >>>> and as this is failing case I hope should be ok. >>>> >>> Also as we don't need cancel_calibration(), keeping tegra_mipi_wait() >>> like earlier makes sense I believe as we are letting it finish going >>> thru sequence. >>> >>> So I think below are fixes, >>> >>> 1. Existing MIPI driver, tegra_mipi_wait() to not use status ACTIVE bit >>> to be 0 and use only DONE bit to be 1 for wait condition=C2=A0 as we ar= e >>> calibrating separately for individual pads and this ACTIVE bit is=20 >>> common >>> for all pads where it will not be 0 in case of other parallel streams >>> which may also be under calibration. >> Yes, looks like it's a mistake of the current MIPI driver that it polls >> the ACTIVE bit. >> >>> 2. No need for separate cancel_calibration. So, probably earlier names >>> tegra_mipi_calibrate() and tegra_mipi_wait() hols good as we are=20 >>> waiting >>> for calibration sequence to finish irrespective of fail/pass. >> The new names reflect better what those functions actually do, IMO. > ok Will keep same names. >> >> What about to make finish_calibration() to take an additional argument >> which corresponds to the awaited HW bits? For example if it's CSIA, then >> it could be: >> >> =C2=A0=C2=A0 tegra_mipi_finish_calibration(csi_chan->mipi, MIPI_CAL_CSIA= ); > MIPI device is separate for each stream so waiting for only those=20 > corresponding DONE bits happen currently and no need to pass argument. >> >> >> Also, is it okay that DSI and CSI could change MIPI_CAL_CTRL after DSI >> or CSI already started calibration? >> >> Looking at the current start_calibration(), I think the mutex should be >> kept locked and then finish_calibration() should unlock it. Right mutex_unlock should happen at end of finish_calibration. With keeping mutex locked in start, we dont have to check for active to=20 be 0 to issue start as mutex will keep it locked and other pads=20 calibration can only go thru when current one is done. So instead of below sequence, its simpler to do this way? start_calibration() - mutex_lock - wait for 72uS after start finish_calibration() - keep check for ACTIVE =3D 0 and DONE =3D 1 - mutex_unlock() > > Confirmed with HW designer. > > ACTIVE is common bit for all pads where we see it 1 as long as all=20 > pads (DSI + all CSI Pads) are under calibration. > > While MIPI CAL is doing calibration for certain pads, before issuing=20 > other start it has to wait for ACTIVE to be 0. > > > Earlier driver (before split) checks for ACTIVE to be 0 along with=20 > DONE bit to be 1 as it does both calibrate and wait in same API. > > With the split, looks like we need below sequence to be safe. > > 1. tegra_mipi_start_calibration(): wait for ACTIVE to be 0 before=20 > issuing START and after issuing start wait for 72uS to let calibration=20 > code sequence finish so it will be ready to see LP-11 after that. > > In case of parallel streams, call to start_calibration can happen when=20 > pads of other stream are under calibration. > > 2. tegra_mipi_finish_calibration(): check for DONE bit to be 1 > > >