Re: [alsa-devel] [PATCH 11/20] OMAP: McBSP: Add link DMA mode selection
On Wednesday 12 August 2009 14:48:33 Nurkkala Eero.An (EXT-Offcode/Oulu) wrote: On Wed, 2009-08-12 at 13:45 +0200, ext Jarkko Nikula wrote: The threshold based transfer will cause that omap_pcm_pointer will loose a bit its accuracy. Probably irrelevant but still better to play safe at least over one kernel release before making it default. No, it doesn't loose accuracy =) It's as accurate with both modes. The difference is, that the other does things in bursts; In element mode it is kind of easy to estimate where the hardware actually in the playback case (aplay -f dat /dev/zero): omap_pcm_pointer returns 669, than the HW is around 669-512=157 (plus few samples). In threshold mode you only know that the HW is playing in between 0-512, 512-1024 somewhere. I know neither of these are accurate and these examples are quite oversimplified, but there is a difference and that difference is quite significant. that's called evolution rather than regression =) Note that evolution can also introduce regression... That's how things will be in the future =) I agree with you on this, since the threshold mode provides quite good power saving benefits. But on the other hand it would be still better to keep the element mode as default for at least one release cycle. If no report is coming about problems, than we can make the threshold mode for McBSP2 as the default -- Péter -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [alsa-devel] [PATCH 11/20] OMAP: McBSP: Add link DMA mode selection
On Tue, 11 Aug 2009 09:18:01 +0300 Eero Nurkkala ext-eero.nurkk...@nokia.com wrote: I would like to see this new threshold based transfer functionality to be integrated so that projects can take the advantage of it and helps generic PM development too but I don't want that it would cause any regression now. Later on it is easy to switch threshold based transfer to be the default for McBSP2 on OMAP3 but it's safer to keep current mode default over one kernel release. What regression are you addressing to? Of course in OMAP2s there could be issues I'm not aware of. But does this have any effect on them? (isn't this only OMAP3). A possible regression to a SW which is using McBSP2 on OMAP3. There many of those boards. The threshold based transfer will cause that omap_pcm_pointer will loose a bit its accuracy. Probably irrelevant but still better to play safe at least over one kernel release before making it default. Return value prints from omap_pcm_pointer while playing aplay -f dat /dev/zero element: 614 669 691 712 734 755 threshold: 512 512 512 1024 1024 1536 -- Jarkko -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [alsa-devel] [PATCH 11/20] OMAP: McBSP: Add link DMA mode selection
On Wed, 2009-08-12 at 13:45 +0200, ext Jarkko Nikula wrote: The threshold based transfer will cause that omap_pcm_pointer will loose a bit its accuracy. Probably irrelevant but still better to play safe at least over one kernel release before making it default. No, it doesn't loose accuracy =) It's as accurate with both modes. The difference is, that the other does things in bursts; that's called evolution rather than regression =) That's how things will be in the future =) Return value prints from omap_pcm_pointer while playing aplay -f dat /dev/zero element: 614 669 691 712 734 755 threshold: 512 512 512 1024 1024 1536 -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [alsa-devel] [PATCH 11/20] OMAP: McBSP: Add link DMA mode selection
Sorry for the late reply. On Thu, 6 Aug 2009 20:15:40 +0200 ext-eero.nurkk...@nokia.com wrote: Well, the element mode is fine for !McBSP2. One doesn't really loose the buffer pointer accuracy, because you can get the last DMA irq timestamp with SNDRV_PCM_IOCTL_TTSTAMP (and compare that to current time). The accuracy is not far off from the element mode? (If you read the DMA portion of TRM, I think there were some issues as well) Well the threshold mode makes the offset returned from omap_pcm_pointer to behave gradually as the threshold size is set. This would affect a SW which is doing sub-period processing like mixing audio n samples ahead the current DMA pointer. Of course HW buffer in McBSP2 adds static latency but otherwise processing is similar compared to other ports and OMAPs. I would like to see this new threshold based transfer functionality to be integrated so that projects can take the advantage of it and helps generic PM development too but I don't want that it would cause any regression now. Later on it is easy to switch threshold based transfer to be the default for McBSP2 on OMAP3 but it's safer to keep current mode default over one kernel release. For !McBSP2, it doesn't even pay to have the threshold mode at all, because the effect on PM is actually adverse - too frequent wakeups will cause more adverse net effect on PM (IIRC) @ VBAT. This is good information and integrating these functionalities allow to collect more. Without regression of course :-) -- Jarkko -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [alsa-devel] [PATCH 11/20] OMAP: McBSP: Add link DMA mode selection
Hello guys, On Thu, Aug 06, 2009 at 08:15:40PM +0200, Nurkkala Eero.An (EXT-Offcode/Oulu) wrote: I would say the default mode for the omap34xx should be also element as it keeps the omap_pcm_pointer behavior the same than currently and avoids possible regression. Yes, the default mode should be the element mode in my opinion as well. -- Péter Well, the element mode is fine for !McBSP2. One doesn't really loose the buffer pointer accuracy, because you can get the last DMA irq timestamp with SNDRV_PCM_IOCTL_TTSTAMP (and compare that to current time). The accuracy is not far off from the element mode? (If you read the DMA portion of TRM, I think there were some issues as well) Don't want to be piccy, but McBSP2 with element mode is more or less like a joke? As there's a large HW bugger, why not take advantage of it..why bypass it as default? For !McBSP2, it doesn't even pay to have the threshold mode at all, because the effect on PM is actually adverse - too frequent wakeups will cause more adverse net effect on PM (IIRC) @ VBAT. I agree with Eero in this point. We can set mcbsp2 default op mode to threshold. I think we are even here :) two believe that must be element mode by default, while two want default threshold (at least for mcbsp2). We need another opinion. - Eero -- To unsubscribe from this list: send the line unsubscribe alsa-devel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- Eduardo Valentin -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [alsa-devel] [PATCH 11/20] OMAP: McBSP: Add link DMA mode selection
I would say the default mode for the omap34xx should be also element as it keeps the omap_pcm_pointer behavior the same than currently and avoids possible regression. Yes, the default mode should be the element mode in my opinion as well. -- Péter Well, the element mode is fine for !McBSP2. One doesn't really loose the buffer pointer accuracy, because you can get the last DMA irq timestamp with SNDRV_PCM_IOCTL_TTSTAMP (and compare that to current time). The accuracy is not far off from the element mode? (If you read the DMA portion of TRM, I think there were some issues as well) Don't want to be piccy, but McBSP2 with element mode is more or less like a joke? As there's a large HW bugger, why not take advantage of it..why bypass it as default? For !McBSP2, it doesn't even pay to have the threshold mode at all, because the effect on PM is actually adverse - too frequent wakeups will cause more adverse net effect on PM (IIRC) @ VBAT. - Eero -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 11/20] OMAP: McBSP: Add link DMA mode selection
On Thu, 30 Jul 2009 15:49:34 +0300
Eduardo Valentin eduardo.valen...@nokia.com wrote:
From: Peter Ujfalusi peter.ujfal...@nokia.com
It adds a new sysfs file, where the user can configure the mcbsp mode to use.
If the mcbsp channel is in use, it does not allow the change.
Than in omap_pcm_open we can call the omap_mcbsp_get_opmode to get the mode,
store it, than use it to implement the different modes.
...
+/** McBSP DMA operating modes **/
+#define MCBSP_DMA_MODE_ELEMENT 0
+#define MCBSP_DMA_MODE_THRESHOLD 1
+#define MCBSP_DMA_MODE_FRAME 2
...
+static ssize_t dma_op_mode_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct omap_mcbsp *mcbsp = dev_get_drvdata(dev);
+ int dma_op_mode;
+
+ spin_lock_irq(mcbsp-lock);
+ dma_op_mode = mcbsp-dma_op_mode;
+ spin_unlock_irq(mcbsp-lock);
+
+ return sprintf(buf, %d\n, dma_op_mode);
It would be good to handle this as an ascii like e.g /sys/power/state
does. element, threshold or frame. A number value doesn't tell
much (for me when testing).
@@ -1195,12 +1266,14 @@ static int __devinit omap_mcbsp_probe
(struct platform_device *pdev)
if (cpu_is_omap34xx()) {
mcbsp-max_tx_thres = max_thres(mcbsp);
mcbsp-max_rx_thres = max_thres(mcbsp);
+ mcbsp-dma_op_mode = MCBSP_DMA_MODE_THRESHOLD;
if (omap_additional_add(pdev))
dev_warn(pdev-dev,
Unable to create threshold controls\n);
} else {
mcbsp-max_tx_thres = -EINVAL;
mcbsp-max_rx_thres = -EINVAL;
+ mcbsp-dma_op_mode = MCBSP_DMA_MODE_ELEMENT;
}
I would say the default mode for the omap34xx should be also element as
it keeps the omap_pcm_pointer behavior the same than currently and
avoids possible regression.
--
Jarkko
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 11/20] OMAP: McBSP: Add link DMA mode selection
On Thu, Jul 30, 2009 at 03:47:25PM +0200, ext Mark Brown wrote: On Thu, Jul 30, 2009 at 04:28:08PM +0300, Eduardo Valentin wrote: On Thu, Jul 30, 2009 at 03:04:42PM +0200, ext Mark Brown wrote: Is this really something that people would want to tweak at runtime (except for test purposes)? Yes, test purposes, bug also, the same link could be sometime used for low-power playback while sometime for low-latency processing where as accurate as possible DMA pointer position info is needed. Latency you *should* be able to infer from the application behaviour. Yes that's true. But I think using ELEMENT mode, position reports are much more accurate, which will result in more precise inferences for latencies. -- Eduardo Valentin -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 11/20] OMAP: McBSP: Add link DMA mode selection
From: Peter Ujfalusi peter.ujfal...@nokia.com
It adds a new sysfs file, where the user can configure the mcbsp mode to use.
If the mcbsp channel is in use, it does not allow the change.
Than in omap_pcm_open we can call the omap_mcbsp_get_opmode to get the mode,
store it, than use it to implement the different modes.
Signed-off-by: Peter Ujfalusi peter.ujfal...@nokia.com
Signed-off-by: Eduardo Valentin eduardo.valen...@nokia.com
---
arch/arm/plat-omap/include/mach/mcbsp.h |8 +++
arch/arm/plat-omap/mcbsp.c | 73 +++
2 files changed, 81 insertions(+), 0 deletions(-)
diff --git a/arch/arm/plat-omap/include/mach/mcbsp.h
b/arch/arm/plat-omap/include/mach/mcbsp.h
index b0fc675..61c55ec 100644
--- a/arch/arm/plat-omap/include/mach/mcbsp.h
+++ b/arch/arm/plat-omap/include/mach/mcbsp.h
@@ -275,6 +275,11 @@
#define RSYNCERREN 0x0001
#define WAKEUPEN_ALL (XRDYEN | RRDYEN)
+/** McBSP DMA operating modes **/
+#define MCBSP_DMA_MODE_ELEMENT 0
+#define MCBSP_DMA_MODE_THRESHOLD 1
+#define MCBSP_DMA_MODE_FRAME 2
+
/* we don't do multichannel for now */
struct omap_mcbsp_reg_cfg {
u16 spcr2;
@@ -405,6 +410,7 @@ struct omap_mcbsp {
struct clk *iclk;
struct clk *fclk;
#ifdef CONFIG_ARCH_OMAP34XX
+ int dma_op_mode;
u16 max_tx_thres;
u16 max_rx_thres;
#endif
@@ -421,6 +427,7 @@ void omap_mcbsp_set_tx_threshold(unsigned int id, u16
threshold);
void omap_mcbsp_set_rx_threshold(unsigned int id, u16 threshold);
u16 omap_mcbsp_get_max_tx_threshold(unsigned int id);
u16 omap_mcbsp_get_max_rx_threshold(unsigned int id);
+int omap_mcbsp_get_dma_op_mode(unsigned int id);
#else
static inline void omap_mcbsp_set_tx_threshold(unsigned int id, u16 threshold)
{ }
@@ -428,6 +435,7 @@ static inline void omap_mcbsp_set_rx_threshold(unsigned int
id, u16 threshold)
{ }
static inline u16 omap_mcbsp_get_max_tx_threshold(unsigned int id) { return 0;
}
static inline u16 omap_mcbsp_get_max_rx_threshold(unsigned int id) { return 0;
}
+static inline int omap_mcbsp_get_dma_op_mode(unsigned int id) { return 0; }
#endif
int omap_mcbsp_request(unsigned int id);
void omap_mcbsp_free(unsigned int id);
diff --git a/arch/arm/plat-omap/mcbsp.c b/arch/arm/plat-omap/mcbsp.c
index 1b8ff9e..6c535b3 100644
--- a/arch/arm/plat-omap/mcbsp.c
+++ b/arch/arm/plat-omap/mcbsp.c
@@ -282,6 +282,29 @@ u16 omap_mcbsp_get_max_rx_threshold(unsigned int id)
return mcbsp-max_rx_thres;
}
EXPORT_SYMBOL(omap_mcbsp_get_max_rx_threshold);
+
+/*
+ * omap_mcbsp_get_dma_op_mode just return the current configured
+ * operating mode for the mcbsp channel
+ */
+int omap_mcbsp_get_dma_op_mode(unsigned int id)
+{
+ struct omap_mcbsp *mcbsp;
+ int dma_op_mode;
+
+ if (!omap_mcbsp_check_valid_id(id)) {
+ printk(KERN_ERR %s: Invalid id (%u)\n, __func__, id + 1);
+ return -ENODEV;
+ }
+ mcbsp = id_to_mcbsp_ptr(id);
+
+ spin_lock_irq(mcbsp-lock);
+ dma_op_mode = mcbsp-dma_op_mode;
+ spin_unlock_irq(mcbsp-lock);
+
+ return dma_op_mode;
+}
+EXPORT_SYMBOL(omap_mcbsp_get_dma_op_mode);
#endif
/*
@@ -1093,9 +1116,57 @@ static DEVICE_ATTR(prop, 0644, prop##_show,
prop##_store);
THRESHOLD_PROP_BUILDER(max_tx_thres);
THRESHOLD_PROP_BUILDER(max_rx_thres);
+static ssize_t dma_op_mode_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct omap_mcbsp *mcbsp = dev_get_drvdata(dev);
+ int dma_op_mode;
+
+ spin_lock_irq(mcbsp-lock);
+ dma_op_mode = mcbsp-dma_op_mode;
+ spin_unlock_irq(mcbsp-lock);
+
+ return sprintf(buf, %d\n, dma_op_mode);
+}
+
+static ssize_t dma_op_mode_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t size)
+{
+ struct omap_mcbsp *mcbsp = dev_get_drvdata(dev);
+ unsigned long val;
+ int status;
+
+ status = strict_strtoul(buf, 0, val);
+ if (status)
+ return status;
+
+ spin_lock_irq(mcbsp-lock);
+
+ if (!mcbsp-free) {
+ size = -EBUSY;
+ goto unlock;
+ }
+
+ if (val MCBSP_DMA_MODE_FRAME || val MCBSP_DMA_MODE_ELEMENT) {
+ size = -EINVAL;
+ goto unlock;
+ }
+
+ mcbsp-dma_op_mode = val;
+
+unlock:
+ spin_unlock_irq(mcbsp-lock);
+
+ return size;
+}
+
+static DEVICE_ATTR(dma_op_mode, 0644, dma_op_mode_show, dma_op_mode_store);
+
static const struct attribute *additional_attrs[] = {
dev_attr_max_tx_thres.attr,
dev_attr_max_rx_thres.attr,
+ dev_attr_dma_op_mode.attr,
NULL,
};
@@ -1195,12 +1266,14 @@ static int __devinit omap_mcbsp_probe(struct
platform_device *pdev)
if (cpu_is_omap34xx()) {
mcbsp-max_tx_thres =
Re: [PATCH 11/20] OMAP: McBSP: Add link DMA mode selection
On Thu, Jul 30, 2009 at 03:49:34PM +0300, Eduardo Valentin wrote: It adds a new sysfs file, where the user can configure the mcbsp mode to use. If the mcbsp channel is in use, it does not allow the change. Than in omap_pcm_open we can call the omap_mcbsp_get_opmode to get the mode, store it, than use it to implement the different modes. Is this really something that people would want to tweak at runtime (except for test purposes)? -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 11/20] OMAP: McBSP: Add link DMA mode selection
On Thu, Jul 30, 2009 at 03:04:42PM +0200, ext Mark Brown wrote: On Thu, Jul 30, 2009 at 03:49:34PM +0300, Eduardo Valentin wrote: It adds a new sysfs file, where the user can configure the mcbsp mode to use. If the mcbsp channel is in use, it does not allow the change. Than in omap_pcm_open we can call the omap_mcbsp_get_opmode to get the mode, store it, than use it to implement the different modes. Is this really something that people would want to tweak at runtime (except for test purposes)? Yes, test purposes, bug also, the same link could be sometime used for low-power playback while sometime for low-latency processing where as accurate as possible DMA pointer position info is needed. -- Eduardo Valentin -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 11/20] OMAP: McBSP: Add link DMA mode selection
On Thu, Jul 30, 2009 at 04:28:08PM +0300, Eduardo Valentin wrote: On Thu, Jul 30, 2009 at 03:04:42PM +0200, ext Mark Brown wrote: Is this really something that people would want to tweak at runtime (except for test purposes)? Yes, test purposes, bug also, the same link could be sometime used for low-power playback while sometime for low-latency processing where as accurate as possible DMA pointer position info is needed. Latency you *should* be able to infer from the application behaviour. -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
