public inbox for linux-arm-kernel@lists.infradead.org 
 help / color / mirror / Atom feed
From: marc.zyngier@arm•com (Marc Zyngier)
To: linux-arm-kernel@lists•infradead.org
Subject: [PATCH V17 2/3] dmaengine: qcom_hidma: add debugfs hooks
Date: Wed, 27 Apr 2016 09:47:45 +0100	[thread overview]
Message-ID: <57207CB1.4040307@arm.com> (raw)
In-Reply-To: <20160427081501.GV2274@localhost>

On 27/04/16 09:15, Vinod Koul wrote:
> On Tue, Apr 26, 2016 at 12:55:18PM -0400, Sinan Kaya wrote:
>> On 4/26/2016 12:25 PM, Vinod Koul wrote:
>>> On Tue, Apr 26, 2016 at 08:08:16AM -0400, okaya at codeaurora.org wrote:
>>>> On 2016-04-25 23:30, Vinod Koul wrote:
>>>>> On Mon, Apr 11, 2016 at 10:21:12AM -0400, Sinan Kaya wrote:
>>>>>
>>>>>> +static int hidma_chan_stats(struct seq_file *s, void *unused)
>>>>>> +{
>>>>>> +	struct hidma_chan *mchan = s->private;
>>>>>> +	struct hidma_desc *mdesc;
>>>>>> +	struct hidma_dev *dmadev = mchan->dmadev;
>>>>>> +
>>>>>> +	pm_runtime_get_sync(dmadev->ddev.dev);
>>>>>
>>>>> debug shouldn't power up device, why do you want to do that
>>>>
>>>>
>>>> Clocks are turned off while the hw is idle. I can?t reach hw
>>>> registers without restoring power.
>>>
>>> Hmm, have you thought about using regmap?
>>>
>>
>> To be honest, I didn't know what regmap is but I just read some code
>> and looked at how it is used. Feel free to correct me if I got it 
>> wrong. 
>>
>> Regmap seems to be designed for *slow* speed peripherals to improve frequent
>> accesses by the SW. It looks like it is used by MFD, SPI and I2C drivers.
>>
>> It seems to cache the register contents and flush/invalidate them only when
>> needed.
>>
>> The MMIO version seems to be assuming the presence of device-tree like CLK
>> API which doesn't exist on ACPI systems and is not portable.
>>
>> My reaction is that it is a lot of code with no added functionality to what
>> HIDMA driver is trying to achieve. 
>>
>> Given that the use case here is only for debug purposes; I think it is OK 
>> to keep this runtime call here. I don't want to add any overhead into the
>> existing code just to support the debug use case.  
>>
>> None of my register read/writes are slow. This file will only be used to 
>> troubleshoot customer issues.

I'd recommend you actually run perf on a a few of your MMIO accesses. I
believe the result will be eye opening. On the KVM side, we've trimmed
our MMIO access as much as possible, using a memory-based cache (similar
to regmap in concept). This has made some code paths about 40% faster.

> $ is always faster than MMIO. This way you can give reg contents to users
> without waking up hw.

Indeed. MMIO access sucks rocks, even on a very fast box. Actually, the
faster the box is, the slower MMIO feels (compared to memory).

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...

  reply	other threads:[~2016-04-27  8:47 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-04-11 14:21 [PATCH V17 0/3] dmaengine: add Qualcomm Technologies HIDMA driver Sinan Kaya
2016-04-11 14:21 ` [PATCH V17 1/3] dmaengine: qcom_hidma: implement lower level hardware interface Sinan Kaya
2016-04-26  3:28   ` Vinod Koul
2016-04-26 15:04     ` Sinan Kaya
2016-04-26 15:10       ` Andy Shevchenko
2016-04-26 15:23         ` Sinan Kaya
2016-04-26 16:24       ` Vinod Koul
2016-04-28 19:30         ` Sinan Kaya
2016-05-01  4:38           ` Sinan Kaya
2016-04-11 14:21 ` [PATCH V17 2/3] dmaengine: qcom_hidma: add debugfs hooks Sinan Kaya
2016-04-26  3:30   ` Vinod Koul
2016-04-26 12:08     ` okaya at codeaurora.org
2016-04-26 16:25       ` Vinod Koul
2016-04-26 16:55         ` Sinan Kaya
2016-04-27  8:15           ` Vinod Koul
2016-04-27  8:47             ` Marc Zyngier [this message]
2016-04-27 13:25               ` okaya at codeaurora.org
2016-04-27 12:51             ` okaya at codeaurora.org
2016-05-01  4:35               ` Sinan Kaya
2016-05-02  9:25                 ` Vinod Koul
2016-05-02 10:40                   ` Mark Brown
2016-04-11 14:21 ` [PATCH V17 3/3] dmaengine: qcom_hidma: add support for object hierarchy Sinan Kaya

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=57207CB1.4040307@arm.com \
    --to=marc.zyngier@arm$(echo .)com \
    --cc=linux-arm-kernel@lists$(echo .)infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox