From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.1 (2015-04-28) on archive.lwn.net X-Spam-Level: X-Spam-Status: No, score=-4.7 required=5.0 tests=DKIM_SIGNED, HEADER_FROM_DIFFERENT_DOMAINS,RCVD_IN_DNSWL_HI,T_DKIM_INVALID, T_RP_MATCHES_RCVD autolearn=ham autolearn_force=no version=3.4.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by archive.lwn.net (Postfix) with ESMTP id 908617E66E for ; Fri, 9 Mar 2018 01:06:46 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751306AbeCIBGn (ORCPT ); Thu, 8 Mar 2018 20:06:43 -0500 Received: from smtp.codeaurora.org ([198.145.29.96]:47900 "EHLO smtp.codeaurora.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751212AbeCIBGm (ORCPT ); Thu, 8 Mar 2018 20:06:42 -0500 Received: by smtp.codeaurora.org (Postfix, from userid 1000) id 77F696076C; Fri, 9 Mar 2018 01:06:41 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=codeaurora.org; s=default; t=1520557601; bh=MXMr8k+lYT6O+bN5dTviaXFwoFzIbJjpXE5z/xt219Q=; h=Subject:To:Cc:References:From:Date:In-Reply-To:From; b=mKTTJjLLPhOUOqnDCNplId/xx39JZBr3XJyLZufDpMYnnPgLZNukabms64ZWl0AQO OqF7moxcxdM+Nf8rFA3WI7t/iPVIFK2u4mja5g13ai53BN6NP96RJ1ozvaxmK3FwkZ aGq46AQ4HVyAYmgryKAQvUPjGnZESFJBjQXz/OVQ= Received: from [10.226.58.86] (i-global254.qualcomm.com [199.106.103.254]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) (Authenticated sender: sdharia@smtp.codeaurora.org) by smtp.codeaurora.org (Postfix) with ESMTPSA id BB383602A0; Fri, 9 Mar 2018 01:06:38 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=codeaurora.org; s=default; t=1520557600; bh=MXMr8k+lYT6O+bN5dTviaXFwoFzIbJjpXE5z/xt219Q=; h=Subject:To:Cc:References:From:Date:In-Reply-To:From; b=ZG2Gm/HDW0c18DlWa23Qq/lE691S8rgwccAzrnVkE/+dQzlnY7mNuY6KEcQjwqBtT UdRynOkTWkkJOP7wU8ssWqiNDllldrfOtlqVKqXIM+xvfYx5MWumRbQfX4efDS4vn7 LmUJ6YUkds19p7lFOmOAI+onfIsGNVcEtToywQPM= DMARC-Filter: OpenDMARC Filter v1.3.2 smtp.codeaurora.org BB383602A0 Authentication-Results: pdx-caf-mail.web.codeaurora.org; dmarc=none (p=none dis=none) header.from=codeaurora.org Authentication-Results: pdx-caf-mail.web.codeaurora.org; spf=none smtp.mailfrom=sdharia@codeaurora.org Subject: Re: [v3, 3/4] i2c: i2c-qcom-geni: Add bus driver for the Qualcomm GENI I2C controller To: Doug Anderson Cc: Jonathan Corbet , Andy Gross , David Brown , Rob Herring , Mark Rutland , Wolfram Sang , Greg Kroah-Hartman , Karthikeyan Ramasubramanian , linux-doc@vger.kernel.org, linux-arm-msm@vger.kernel.org, devicetree@vger.kernel.org, linux-i2c@vger.kernel.org, linux-serial@vger.kernel.org, Jiri Slaby , evgreen@chromium.org, acourbot@chromium.org, Girish Mahadevan , swboyd@chromium.org References: <1519781889-16117-4-git-send-email-kramasub@codeaurora.org> <03c5ed5b-5160-bdd5-82f9-4a8beab33ca8@codeaurora.org> From: Sagar Dharia Message-ID: <65ae29b5-83da-22aa-f483-c3a25802239f@codeaurora.org> Date: Thu, 8 Mar 2018 18:06:38 -0700 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.6.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: linux-doc-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-doc@vger.kernel.org Hi Doug On 3/8/2018 2:12 PM, Doug Anderson wrote: > Hi, > > On Wed, Mar 7, 2018 at 9:19 PM, Doug Anderson wrote: >>>> DMA is hard and i2c transfers > 32 bytes are rare. Do we really gain >>>> a lot by transferring i2c commands over DMA compared to a FIFO? >>>> Enough to justify the code complexity and the set of bugs that will >>>> show up? I'm sure it will be a controversial assertion given that the >>>> code's already written, but personally I'd be supportive of ripping >>>> DMA mode out to simplify the driver. I'd be curious if anyone else >>>> agrees. To me it seems like premature optimization. >>> >>> >>> Yes, we have multiple clients (e.g. touch, NFC) using I2C for data transfers >>> bigger than 32 bytes (some transfers are 100s of bytes). The fifo size is >>> 32, so we can definitely avoid at least 1 interrupt when DMA mode is used >>> with data size > 32. >> >> Does that 1-2 interrupts make any real difference, though? In theory >> it really shouldn't affect the transfer rate. We should be able to >> service the interrupt plenty fast and if we were concerned we would >> tweak the watermark code a little bit. So I guess we're worried about >> the extra CPU cycles (and power cost) to service those extra couple >> interrupts? >> >> In theory when touch data is coming in or NFC data is coming in then >> we're probably not in a super low power state to begin with. If it's >> touch data we likely want to have the CPU boosted a bunch to respond >> to the user quickly. If we've got 8 cores available all of which can >> run at 1GHz or more a few interrupts won't kill us. NFC data is >> probably not common enough that we need to optimize power/CPU >> utilizatoin for that. >> >> >> So while i can believe that you do save an interrupt or two, I still >> am not convinced that those interrupts are worth a bunch of complex >> code (and a whole second code path) to save. >> >> >> ...also note that above you said that coming out of runtime suspend >> can take several msec. That seems like it dwarfs any slight >> difference in timing between a FIFO-based operation and DMA. > > One last note here (sorry for not thinking of this last night) is that > I would also be interested in considering _only_ supporting the DMA > path. Is it somehow intrinsically bad to use the DMA flow for a > 1-byte transfer? Is there a bunch of extra overhead or power draw? > > Specifically my main point is that maintaining two separate flows (the > FIFO flow vs the DMA flow) adds complexity, code size, and bugs. If > there's a really good reason to maintain both flows then fine, but we > should really consider if this is something that's really giving us > value before we agree to it. > FIFO mode gives us 2 advantages: 1. small transfers don't have to go through 'dma-map/unmap penalties. Some small buffers come from the stack of client caller and the dma-map/unmap may fail. 2. bring-ups are 'less eventful' (with temp. change to just not have DMA mode at all during bring-ups) since SMMU translation/DMA path from QUP (master) to memory slave may not always available when critical I2C peripherals need to be brought up (e.g. PMIC). CPU to QUP (slave) path is usually available. On the other side, DMA mode has other advantages: 1. Multiple android clients are still heavily using I2C in spite of faster peripheral buses being available in industry. As an example, some multi-finger Touch screens use I2C and the data to be transferred per transaction over the bus grows well beyond 70-100 bytes based on number of fingers. These transactions are very frequent when touch is being used, and in an environment where other heavy system users are also running (MM/graphics). Another example is, NFC uses I2C (as of now) to transfer as much as 700+ bytes. This can save us 20+ interrupts per transfer. These transfers are mostly in burst. So the RPMh penalty to resume the shared resources is only experienced for very first transfer. Remaining transfers in the burst benefit from DMA if they are too big. Goal here is to have common driver for upstream targets and android and android has seen proven advantages with both modes. If we end up keeping DMA only for downstream (or FIFO only for downstream), then we lose the advantage of having code in upstream since we have to maintain downstream patch with other mode. Thanks Sagar > > -Doug > -- > To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html