Re: [PATCH 7/7] ALSA: hda - Reduce verbs during generic parser initialization - Lars-Peter Clausen

From: Lars-Peter Clausen <lars@metafoo.de>
To: Takashi Iwai <tiwai@suse.de>
Cc: alsa-devel@alsa-project.org
Subject: Re: [PATCH 7/7] ALSA: hda - Reduce verbs during generic parser initialization
Date: Sat, 28 Feb 2015 11:27:42 +0100	[thread overview]
Message-ID: <54F1981E.7070103@metafoo.de> (raw)
In-Reply-To: <s5hwq32gxrf.wl-tiwai@suse.de>

On 02/28/2015 11:01 AM, Takashi Iwai wrote:
> At Sat, 28 Feb 2015 10:51:29 +0100,
> Lars-Peter Clausen wrote:
>>
>> On 02/27/2015 10:28 PM, Takashi Iwai wrote:
>>> For reducing the number of verbs performed during the initialization
>>> and the resume of the generic parser, this patch (re-)introduces the
>>> mechanism to cache everything in init and sync later.
>>>
>>> However, it became a bit tricky in the end: we can't use regcache's
>>> cache_only flag straightforwardly here because we do want the actual
>>> hardware access for reads if needed, but forbids only the writes.
>>> So, instead, the regmap reg_write callback checks the own flag
>>> (reusing the existing codec->cached_write) and skips the actual
>>> access.  This works by assumption of regmap dumping the whole
>>> registers via regcache_sync() at a later point.
>>>
>>> Signed-off-by: Takashi Iwai <tiwai@suse.de>
>>> ---
>>>    sound/pci/hda/hda_generic.c | 4 ++++
>>>    sound/pci/hda/hda_regmap.c  | 6 ++++++
>>>    2 files changed, 10 insertions(+)
>>>
>>> diff --git a/sound/pci/hda/hda_generic.c b/sound/pci/hda/hda_generic.c
>>> index 33de31c83d6e..bcaed93f8795 100644
>>> --- a/sound/pci/hda/hda_generic.c
>>> +++ b/sound/pci/hda/hda_generic.c
>>> @@ -5411,6 +5411,8 @@ int snd_hda_gen_init(struct hda_codec *codec)
>>>
>>>    	snd_hda_apply_verbs(codec);
>>>
>>> +	codec->cached_write = 1;
>>> +
>>
>> How about using regcache_cache_only(), this handles this internally in
>> regmap and will also make sure that the cache gets marked as dirty so that a
>> regcache_sync() will properly write out the dirty registers.
>
> I tried this as a first option, too, but it doesn't work because we
> still want reading the value from hardware if not read beforehand.
> regcache_cache_only() prevents also from reading a h/w value.  It's
> the point I mentioned this being tricky.

Hm, right, I skipped right past that. But I think it will still be better to 
implement this write-only caching in the regmap layer.

- Lars