From: Lori Hikichi <lhikichi-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
To: Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Cc: Scott Branden <sbranden-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>,
Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>,
Pawel Moll <pawel.moll-5wv7dgnIgG8@public.gmane.org>,
Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>,
Ian Campbell
<ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg@public.gmane.org>,
Kumar Gala <galak-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>,
Liam Girdwood <lgirdwood-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
Jaroslav Kysela <perex-/Fr2/VpizcU@public.gmane.org>,
Takashi Iwai <tiwai-l3A5Bk7waGM@public.gmane.org>,
alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
bcm-kernel-feedback-list-dY08KVG/lbpWk0Htik3J/w@public.gmane.org,
Dmitry Torokhov <dtor-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>,
Anatol Pomazao <anatol-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>,
abrestic-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org,
bryeung-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org,
olofj-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org,
pwestin-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org
Subject: Re: [PATCH 2/2] ASoC: add core audio driver for Broadcom Cygnus SOC.
Date: Thu, 9 Apr 2015 19:06:22 -0700 [thread overview]
Message-ID: <5527301E.6070906@broadcom.com> (raw)
In-Reply-To: <20150408192309.GI6023-GFdadSzt00ze9xe1eoZjHA@public.gmane.org>
On 15-04-08 12:23 PM, Mark Brown wrote:
> On Tue, Apr 07, 2015 at 07:30:57PM -0700, Lori Hikichi wrote:
>> On 15-04-06 09:19 AM, Mark Brown wrote:
>
>>> OK, so that seems fragile - what happens if we're slightly late
>>> processing an interrupt or miss one entirely? Most hardware has some
>>> way to read back the current position which tends to be more reliable,
>>> is that not an option here?
>
>> The hardware updates a read pointer (rdaddr) which we feed to ALSA via the
>> ".pointer" hook. So yes, the hardware does have a register that tells us its
>> progress. This routine (ringbuf_inc) actually updates a write pointer
>> (wraddr) which is a misnomer. The write pointer doesn’t actually tell us
>> where we are writing to – ALSA keeps track of that. The wraddr only prevents
>> the hardware from reading past it. We just use it, along with a low water
>> mark configuration register, to keep the periodic interrupts firing. The
>> hardware is tracking the distance between rdaddr and wraddr and comparing
>> that to the low water mark.
>
> The code has handling for both read and write so it's not just updating
> a write pointer. Is there no flexibility in the hardware regarding
> interrupt generation?
>
>> Being late processing the interrupt is okay since there are more samples to
>> read. That is, it was only a low water mark interrupt and we have one
>> period of valid data still (we configure low water to be one period).
>> Missing an interrupt is okay since the hardware will just stop reading from
>> buffer when rdaddr has hit wraddr.
>
> Stopping if we miss an interrupt is precisely the sort of situation we
> want to avoid if we can - if the application is sufficiently far ahead
> of the hardware everything should continue to work fine. The minimal
> period size appears to be very small so this is a potential issue, if an
> application tries to use many very small periods it's both more
> vulnerable to some other interrupt taking longer than might be desirable
> and likely that things would be fine as the application is hopefully
> more than one period ahead of where the hardware is at.
>
> If the hardware is always going to halt at the end of the period there's
> not a huge amount we can do about this except possibly raise the minimum
> period if systems are running into trouble but if there's a way to avoid
> doing that then that would be even better.
>
Makes sense, thanks for clarifying the desired behaviour, I will make the
necessary adjustments to keep the hardware supplied with valid data even if
interrupts are held off past a whole period.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
WARNING: multiple messages have this Message-ID (diff)
From: lhikichi@broadcom.com (Lori Hikichi)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 2/2] ASoC: add core audio driver for Broadcom Cygnus SOC.
Date: Thu, 9 Apr 2015 19:06:22 -0700 [thread overview]
Message-ID: <5527301E.6070906@broadcom.com> (raw)
In-Reply-To: <20150408192309.GI6023@sirena.org.uk>
On 15-04-08 12:23 PM, Mark Brown wrote:
> On Tue, Apr 07, 2015 at 07:30:57PM -0700, Lori Hikichi wrote:
>> On 15-04-06 09:19 AM, Mark Brown wrote:
>
>>> OK, so that seems fragile - what happens if we're slightly late
>>> processing an interrupt or miss one entirely? Most hardware has some
>>> way to read back the current position which tends to be more reliable,
>>> is that not an option here?
>
>> The hardware updates a read pointer (rdaddr) which we feed to ALSA via the
>> ".pointer" hook. So yes, the hardware does have a register that tells us its
>> progress. This routine (ringbuf_inc) actually updates a write pointer
>> (wraddr) which is a misnomer. The write pointer doesn?t actually tell us
>> where we are writing to ? ALSA keeps track of that. The wraddr only prevents
>> the hardware from reading past it. We just use it, along with a low water
>> mark configuration register, to keep the periodic interrupts firing. The
>> hardware is tracking the distance between rdaddr and wraddr and comparing
>> that to the low water mark.
>
> The code has handling for both read and write so it's not just updating
> a write pointer. Is there no flexibility in the hardware regarding
> interrupt generation?
>
>> Being late processing the interrupt is okay since there are more samples to
>> read. That is, it was only a low water mark interrupt and we have one
>> period of valid data still (we configure low water to be one period).
>> Missing an interrupt is okay since the hardware will just stop reading from
>> buffer when rdaddr has hit wraddr.
>
> Stopping if we miss an interrupt is precisely the sort of situation we
> want to avoid if we can - if the application is sufficiently far ahead
> of the hardware everything should continue to work fine. The minimal
> period size appears to be very small so this is a potential issue, if an
> application tries to use many very small periods it's both more
> vulnerable to some other interrupt taking longer than might be desirable
> and likely that things would be fine as the application is hopefully
> more than one period ahead of where the hardware is at.
>
> If the hardware is always going to halt at the end of the period there's
> not a huge amount we can do about this except possibly raise the minimum
> period if systems are running into trouble but if there's a way to avoid
> doing that then that would be even better.
>
Makes sense, thanks for clarifying the desired behaviour, I will make the
necessary adjustments to keep the hardware supplied with valid data even if
interrupts are held off past a whole period.
WARNING: multiple messages have this Message-ID (diff)
From: Lori Hikichi <lhikichi@broadcom.com>
To: Mark Brown <broonie@kernel.org>
Cc: Scott Branden <sbranden@broadcom.com>,
Rob Herring <robh+dt@kernel.org>, Pawel Moll <pawel.moll@arm.com>,
Mark Rutland <mark.rutland@arm.com>,
"Ian Campbell" <ijc+devicetree@hellion.org.uk>,
Kumar Gala <galak@codeaurora.org>,
Liam Girdwood <lgirdwood@gmail.com>,
Jaroslav Kysela <perex@perex.cz>, Takashi Iwai <tiwai@suse.de>,
<alsa-devel@alsa-project.org>, <linux-kernel@vger.kernel.org>,
<devicetree@vger.kernel.org>,
<linux-arm-kernel@lists.infradead.org>,
<bcm-kernel-feedback-list@broadcom.com>,
Dmitry Torokhov <dtor@google.com>,
Anatol Pomazao <anatol@google.com>, <abrestic@google.com>,
<bryeung@google.com>, <olofj@google.com>, <pwestin@google.com>
Subject: Re: [PATCH 2/2] ASoC: add core audio driver for Broadcom Cygnus SOC.
Date: Thu, 9 Apr 2015 19:06:22 -0700 [thread overview]
Message-ID: <5527301E.6070906@broadcom.com> (raw)
In-Reply-To: <20150408192309.GI6023@sirena.org.uk>
On 15-04-08 12:23 PM, Mark Brown wrote:
> On Tue, Apr 07, 2015 at 07:30:57PM -0700, Lori Hikichi wrote:
>> On 15-04-06 09:19 AM, Mark Brown wrote:
>
>>> OK, so that seems fragile - what happens if we're slightly late
>>> processing an interrupt or miss one entirely? Most hardware has some
>>> way to read back the current position which tends to be more reliable,
>>> is that not an option here?
>
>> The hardware updates a read pointer (rdaddr) which we feed to ALSA via the
>> ".pointer" hook. So yes, the hardware does have a register that tells us its
>> progress. This routine (ringbuf_inc) actually updates a write pointer
>> (wraddr) which is a misnomer. The write pointer doesn’t actually tell us
>> where we are writing to – ALSA keeps track of that. The wraddr only prevents
>> the hardware from reading past it. We just use it, along with a low water
>> mark configuration register, to keep the periodic interrupts firing. The
>> hardware is tracking the distance between rdaddr and wraddr and comparing
>> that to the low water mark.
>
> The code has handling for both read and write so it's not just updating
> a write pointer. Is there no flexibility in the hardware regarding
> interrupt generation?
>
>> Being late processing the interrupt is okay since there are more samples to
>> read. That is, it was only a low water mark interrupt and we have one
>> period of valid data still (we configure low water to be one period).
>> Missing an interrupt is okay since the hardware will just stop reading from
>> buffer when rdaddr has hit wraddr.
>
> Stopping if we miss an interrupt is precisely the sort of situation we
> want to avoid if we can - if the application is sufficiently far ahead
> of the hardware everything should continue to work fine. The minimal
> period size appears to be very small so this is a potential issue, if an
> application tries to use many very small periods it's both more
> vulnerable to some other interrupt taking longer than might be desirable
> and likely that things would be fine as the application is hopefully
> more than one period ahead of where the hardware is at.
>
> If the hardware is always going to halt at the end of the period there's
> not a huge amount we can do about this except possibly raise the minimum
> period if systems are running into trouble but if there's a way to avoid
> doing that then that would be even better.
>
Makes sense, thanks for clarifying the desired behaviour, I will make the
necessary adjustments to keep the hardware supplied with valid data even if
interrupts are held off past a whole period.
next prev parent reply other threads:[~2015-04-10 2:06 UTC|newest]
Thread overview: 55+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-03-31 3:16 [PATCH 0/2] Cygnus Audio Driver Scott Branden
2015-03-31 3:16 ` Scott Branden
2015-03-31 3:16 ` Scott Branden
2015-03-31 3:16 ` [PATCH 1/2] ASoC: cygnus-audio: adding device tree bindings Scott Branden
2015-03-31 3:16 ` Scott Branden
2015-03-31 3:16 ` Scott Branden
2015-03-31 5:58 ` Mark Brown
2015-03-31 5:58 ` Mark Brown
2015-04-02 18:47 ` Lori Hikichi
2015-04-02 18:47 ` Lori Hikichi
2015-04-02 18:47 ` Lori Hikichi
2015-03-31 7:00 ` Mark Brown
2015-03-31 7:00 ` Mark Brown
2015-03-31 7:26 ` [alsa-devel] " Lars-Peter Clausen
2015-03-31 7:26 ` Lars-Peter Clausen
[not found] ` <551A4C37.5090903-Qo5EllUWu/uELgA04lAiVw@public.gmane.org>
2015-04-02 22:44 ` Lori Hikichi
2015-04-02 22:44 ` Lori Hikichi
2015-04-02 22:44 ` Lori Hikichi
[not found] ` <1427771784-29950-1-git-send-email-sbranden-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
2015-03-31 3:16 ` [PATCH 2/2] ASoC: add core audio driver for Broadcom Cygnus SOC Scott Branden
2015-03-31 3:16 ` Scott Branden
2015-03-31 3:16 ` Scott Branden
[not found] ` <1427771784-29950-3-git-send-email-sbranden-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
2015-03-31 6:42 ` Mark Brown
2015-03-31 6:42 ` Mark Brown
2015-03-31 6:42 ` Mark Brown
2015-04-02 18:47 ` Lori Hikichi
2015-04-02 18:47 ` Lori Hikichi
2015-04-02 18:47 ` Lori Hikichi
[not found] ` <551D8EB6.1050004-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
2015-04-06 16:19 ` Mark Brown
2015-04-06 16:19 ` Mark Brown
2015-04-06 16:19 ` Mark Brown
2015-04-08 2:30 ` Lori Hikichi
2015-04-08 2:30 ` Lori Hikichi
2015-04-08 2:30 ` Lori Hikichi
2015-04-08 19:23 ` Mark Brown
2015-04-08 19:23 ` Mark Brown
[not found] ` <20150408192309.GI6023-GFdadSzt00ze9xe1eoZjHA@public.gmane.org>
2015-04-10 2:06 ` Lori Hikichi [this message]
2015-04-10 2:06 ` Lori Hikichi
2015-04-10 2:06 ` Lori Hikichi
2015-03-31 6:43 ` [PATCH 0/2] Cygnus Audio Driver Mark Brown
2015-03-31 6:43 ` Mark Brown
2015-03-31 6:43 ` Mark Brown
[not found] ` <20150331064338.GE2869-GFdadSzt00ze9xe1eoZjHA@public.gmane.org>
2015-04-03 19:33 ` Scott Branden
2015-04-03 19:33 ` Scott Branden
2015-04-03 19:33 ` Scott Branden
[not found] ` <551EEAF8.7050108-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
2015-04-06 9:58 ` Mark Brown
2015-04-06 9:58 ` Mark Brown
2015-04-06 9:58 ` Mark Brown
[not found] ` <20150406095803.GD6023-GFdadSzt00ze9xe1eoZjHA@public.gmane.org>
2015-04-08 2:28 ` Lori Hikichi
2015-04-08 2:28 ` Lori Hikichi
2015-04-08 2:28 ` Lori Hikichi
2015-04-08 18:54 ` Mark Brown
2015-04-08 18:54 ` Mark Brown
[not found] ` <20150408185420.GH6023-GFdadSzt00ze9xe1eoZjHA@public.gmane.org>
2015-04-11 3:06 ` Lori Hikichi
2015-04-11 3:06 ` Lori Hikichi
2015-04-11 3:06 ` Lori Hikichi
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=5527301E.6070906@broadcom.com \
--to=lhikichi-dy08kvg/lbpwk0htik3j/w@public.gmane.org \
--cc=abrestic-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org \
--cc=alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw@public.gmane.org \
--cc=anatol-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org \
--cc=bcm-kernel-feedback-list-dY08KVG/lbpWk0Htik3J/w@public.gmane.org \
--cc=broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org \
--cc=bryeung-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org \
--cc=devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
--cc=dtor-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org \
--cc=galak-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org \
--cc=ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg@public.gmane.org \
--cc=lgirdwood-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org \
--cc=linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org \
--cc=linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
--cc=mark.rutland-5wv7dgnIgG8@public.gmane.org \
--cc=olofj-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org \
--cc=pawel.moll-5wv7dgnIgG8@public.gmane.org \
--cc=perex-/Fr2/VpizcU@public.gmane.org \
--cc=pwestin-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org \
--cc=robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org \
--cc=sbranden-dY08KVG/lbpWk0Htik3J/w@public.gmane.org \
--cc=tiwai-l3A5Bk7waGM@public.gmane.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.