Re: [PATCH] media: davinci: vpbe: fix build warning
Hi Hans,
On Wed, Oct 3, 2012 at 12:10 PM, Hans Verkuil wrote:
> On Wed October 3 2012 08:27:38 Prabhakar wrote:
>> From: Lad, Prabhakar
>>
>> recent patch with commit id 4f996594ceaf6c3f9bc42b40c40b0f7f87b79c86
>> which makes vidioc_s_crop const, was causing a following build warning,
>>
>> vpbe_display.c: In function 'vpbe_display_s_crop':
>> vpbe_display.c:640: warning: initialization discards qualifiers from pointer
>> target type
>>
>> This patch fixes the above build warning.
>
> Acked-by: Hans Verkuil
>
> Prabhaker, can you make a pull request for this patch, the "CUSTOM_TIMINGS"
> rename patch and the earlier "vpfe: fix build error" patch? These things
> should be fast-tracked.
>
Yes I will issue a pull request today.
Regards,
--Prabhakar Lad
> Regards,
>
> Hans
>
>> Signed-off-by: Lad, Prabhakar
>> Signed-off-by: Manjunath Hadli
>> Cc: Hans Verkuil
>> Cc: Mauro Carvalho Chehab
>> ---
>> drivers/media/platform/davinci/vpbe_display.c | 20 ++--
>> 1 files changed, 10 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/media/platform/davinci/vpbe_display.c
>> b/drivers/media/platform/davinci/vpbe_display.c
>> index 1b238fe..161c776 100644
>> --- a/drivers/media/platform/davinci/vpbe_display.c
>> +++ b/drivers/media/platform/davinci/vpbe_display.c
>> @@ -637,7 +637,7 @@ static int vpbe_display_s_crop(struct file *file, void
>> *priv,
>> struct vpbe_device *vpbe_dev = disp_dev->vpbe_dev;
>> struct osd_layer_config *cfg = &layer->layer_info.config;
>> struct osd_state *osd_device = disp_dev->osd_device;
>> - struct v4l2_rect *rect = &crop->c;
>> + struct v4l2_rect rect = crop->c;
>> int ret;
>>
>> v4l2_dbg(1, debug, &vpbe_dev->v4l2_dev,
>> @@ -648,21 +648,21 @@ static int vpbe_display_s_crop(struct file *file, void
>> *priv,
>> return -EINVAL;
>> }
>>
>> - if (rect->top < 0)
>> - rect->top = 0;
>> - if (rect->left < 0)
>> - rect->left = 0;
>> + if (rect.top < 0)
>> + rect.top = 0;
>> + if (rect.left < 0)
>> + rect.left = 0;
>>
>> - vpbe_disp_check_window_params(disp_dev, rect);
>> + vpbe_disp_check_window_params(disp_dev, &rect);
>>
>> osd_device->ops.get_layer_config(osd_device,
>> layer->layer_info.id, cfg);
>>
>> vpbe_disp_calculate_scale_factor(disp_dev, layer,
>> - rect->width,
>> - rect->height);
>> - vpbe_disp_adj_position(disp_dev, layer, rect->top,
>> - rect->left);
>> + rect.width,
>> + rect.height);
>> + vpbe_disp_adj_position(disp_dev, layer, rect.top,
>> + rect.left);
>> ret = osd_device->ops.set_layer_config(osd_device,
>> layer->layer_info.id, cfg);
>> if (ret < 0) {
>>
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH] media: davinci: vpbe: fix build warning
On Wed October 3 2012 08:27:38 Prabhakar wrote:
> From: Lad, Prabhakar
>
> recent patch with commit id 4f996594ceaf6c3f9bc42b40c40b0f7f87b79c86
> which makes vidioc_s_crop const, was causing a following build warning,
>
> vpbe_display.c: In function 'vpbe_display_s_crop':
> vpbe_display.c:640: warning: initialization discards qualifiers from pointer
> target type
>
> This patch fixes the above build warning.
Acked-by: Hans Verkuil
Prabhaker, can you make a pull request for this patch, the "CUSTOM_TIMINGS"
rename patch and the earlier "vpfe: fix build error" patch? These things
should be fast-tracked.
Regards,
Hans
> Signed-off-by: Lad, Prabhakar
> Signed-off-by: Manjunath Hadli
> Cc: Hans Verkuil
> Cc: Mauro Carvalho Chehab
> ---
> drivers/media/platform/davinci/vpbe_display.c | 20 ++--
> 1 files changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/media/platform/davinci/vpbe_display.c
> b/drivers/media/platform/davinci/vpbe_display.c
> index 1b238fe..161c776 100644
> --- a/drivers/media/platform/davinci/vpbe_display.c
> +++ b/drivers/media/platform/davinci/vpbe_display.c
> @@ -637,7 +637,7 @@ static int vpbe_display_s_crop(struct file *file, void
> *priv,
> struct vpbe_device *vpbe_dev = disp_dev->vpbe_dev;
> struct osd_layer_config *cfg = &layer->layer_info.config;
> struct osd_state *osd_device = disp_dev->osd_device;
> - struct v4l2_rect *rect = &crop->c;
> + struct v4l2_rect rect = crop->c;
> int ret;
>
> v4l2_dbg(1, debug, &vpbe_dev->v4l2_dev,
> @@ -648,21 +648,21 @@ static int vpbe_display_s_crop(struct file *file, void
> *priv,
> return -EINVAL;
> }
>
> - if (rect->top < 0)
> - rect->top = 0;
> - if (rect->left < 0)
> - rect->left = 0;
> + if (rect.top < 0)
> + rect.top = 0;
> + if (rect.left < 0)
> + rect.left = 0;
>
> - vpbe_disp_check_window_params(disp_dev, rect);
> + vpbe_disp_check_window_params(disp_dev, &rect);
>
> osd_device->ops.get_layer_config(osd_device,
> layer->layer_info.id, cfg);
>
> vpbe_disp_calculate_scale_factor(disp_dev, layer,
> - rect->width,
> - rect->height);
> - vpbe_disp_adj_position(disp_dev, layer, rect->top,
> - rect->left);
> + rect.width,
> + rect.height);
> + vpbe_disp_adj_position(disp_dev, layer, rect.top,
> + rect.left);
> ret = osd_device->ops.set_layer_config(osd_device,
> layer->layer_info.id, cfg);
> if (ret < 0) {
>
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH] media: davinci: vpfe: fix build error
On Mon October 1 2012 14:52:48 Prabhakar wrote:
> From: Lad, Prabhakar
>
> recent patch with commit id 4f996594ceaf6c3f9bc42b40c40b0f7f87b79c86
> which makes vidioc_s_crop const, was causing a following build error.
>
> vpfe_capture.c: In function 'vpfe_s_crop':
> vpfe_capture.c:1695: error: assignment of read-only location '*crop'
> vpfe_capture.c:1706: warning: passing argument 1 of
> 'ccdc_dev->hw_ops.set_image_window' discards qualifiers from pointer target
> type
> vpfe_capture.c:1706: note: expected 'struct v4l2_rect *' but argument is of
> type 'const struct v4l2_rect *'
> make[4]: *** [drivers/media/platform/davinci/vpfe_capture.o] Error 1
Acked-by: Hans Verkuil
> Signed-off-by: Lad, Prabhakar
> Signed-off-by: Manjunath Hadli
> Cc: Hans Verkuil
> ---
> drivers/media/platform/davinci/vpfe_capture.c | 17 +
> 1 files changed, 9 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/media/platform/davinci/vpfe_capture.c
> b/drivers/media/platform/davinci/vpfe_capture.c
> index 48052cb..8be492c 100644
> --- a/drivers/media/platform/davinci/vpfe_capture.c
> +++ b/drivers/media/platform/davinci/vpfe_capture.c
> @@ -1669,6 +1669,7 @@ static int vpfe_s_crop(struct file *file, void *priv,
>const struct v4l2_crop *crop)
> {
> struct vpfe_device *vpfe_dev = video_drvdata(file);
> + struct v4l2_rect rect = crop->c;
> int ret = 0;
>
> v4l2_dbg(1, debug, &vpfe_dev->v4l2_dev, "vpfe_s_crop\n");
> @@ -1684,7 +1685,7 @@ static int vpfe_s_crop(struct file *file, void *priv,
> if (ret)
> return ret;
>
> - if (crop->c.top < 0 || crop->c.left < 0) {
> + if (rect.top < 0 || rect.left < 0) {
> v4l2_err(&vpfe_dev->v4l2_dev,
> "doesn't support negative values for top & left\n");
> ret = -EINVAL;
> @@ -1692,26 +1693,26 @@ static int vpfe_s_crop(struct file *file, void *priv,
> }
>
> /* adjust the width to 16 pixel boundary */
> - crop->c.width = ((crop->c.width + 15) & ~0xf);
> + rect.width = ((rect.width + 15) & ~0xf);
>
> /* make sure parameters are valid */
> - if ((crop->c.left + crop->c.width >
> + if ((rect.left + rect.width >
> vpfe_dev->std_info.active_pixels) ||
> - (crop->c.top + crop->c.height >
> + (rect.top + rect.height >
> vpfe_dev->std_info.active_lines)) {
> v4l2_err(&vpfe_dev->v4l2_dev, "Error in S_CROP params\n");
> ret = -EINVAL;
> goto unlock_out;
> }
> - ccdc_dev->hw_ops.set_image_window(&crop->c);
> - vpfe_dev->fmt.fmt.pix.width = crop->c.width;
> - vpfe_dev->fmt.fmt.pix.height = crop->c.height;
> + ccdc_dev->hw_ops.set_image_window(&rect);
> + vpfe_dev->fmt.fmt.pix.width = rect.width;
> + vpfe_dev->fmt.fmt.pix.height = rect.height;
> vpfe_dev->fmt.fmt.pix.bytesperline =
> ccdc_dev->hw_ops.get_line_length();
> vpfe_dev->fmt.fmt.pix.sizeimage =
> vpfe_dev->fmt.fmt.pix.bytesperline *
> vpfe_dev->fmt.fmt.pix.height;
> - vpfe_dev->crop = crop->c;
> + vpfe_dev->crop = rect;
> unlock_out:
> mutex_unlock(&vpfe_dev->lock);
> return ret;
>
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
[PATCH] ARM: davinci: dm644x: fix out range signal for ED
From: Lad, Prabhakar
while testing display on dm644x, for ED out-range signals
was observed. This patch fixes appropriate clock setting
for ED.
Signed-off-by: Lad, Prabhakar
Signed-off-by: Manjunath Hadli
Cc: Sekhar Nori
---
arch/arm/mach-davinci/dm644x.c |3 +--
1 files changed, 1 insertions(+), 2 deletions(-)
diff --git a/arch/arm/mach-davinci/dm644x.c b/arch/arm/mach-davinci/dm644x.c
index 79d2880..688484b 100644
--- a/arch/arm/mach-davinci/dm644x.c
+++ b/arch/arm/mach-davinci/dm644x.c
@@ -713,8 +713,7 @@ static int dm644x_venc_setup_clock(enum
vpbe_enc_timings_type type,
break;
case VPBE_ENC_CUSTOM_TIMINGS:
if (pclock <= 2700) {
- v |= DM644X_VPSS_MUXSEL_PLL2_MODE |
-DM644X_VPSS_DACCLKEN;
+ v |= DM644X_VPSS_DACCLKEN;
writel(v, DAVINCI_SYSMOD_VIRT(SYSMOD_VPSS_CLKCTL));
} else {
/*
--
1.7.4.1
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
[PATCH] davinci: vpbe: replace V4L2_OUT_CAP_CUSTOM_TIMINGS with V4L2_OUT_CAP_DV_TIMINGS
From: Lad, Prabhakar
This patch replaces V4L2_OUT_CAP_CUSTOM_TIMINGS macro with
V4L2_OUT_CAP_DV_TIMINGS. As V4L2_OUT_CAP_CUSTOM_TIMINGS is being phased
out.
Signed-off-by: Lad, Prabhakar
Signed-off-by: Manjunath Hadli
Cc: Sekhar Nori
Cc: Hans Verkuil
Cc: Mauro Carvalho Chehab
---
arch/arm/mach-davinci/board-dm644x-evm.c |2 +-
drivers/media/platform/davinci/vpbe.c|4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/arm/mach-davinci/board-dm644x-evm.c
b/arch/arm/mach-davinci/board-dm644x-evm.c
index 3baf56d..a79dc1e 100644
--- a/arch/arm/mach-davinci/board-dm644x-evm.c
+++ b/arch/arm/mach-davinci/board-dm644x-evm.c
@@ -699,7 +699,7 @@ static struct vpbe_output dm644xevm_vpbe_outputs[] = {
.index = 1,
.name = "Component",
.type = V4L2_OUTPUT_TYPE_ANALOG,
- .capabilities = V4L2_OUT_CAP_CUSTOM_TIMINGS,
+ .capabilities = V4L2_OUT_CAP_DV_TIMINGS,
},
.subdev_name= VPBE_VENC_SUBDEV_NAME,
.default_mode = "480p59_94",
diff --git a/drivers/media/platform/davinci/vpbe.c
b/drivers/media/platform/davinci/vpbe.c
index d03f452..9b623bc 100644
--- a/drivers/media/platform/davinci/vpbe.c
+++ b/drivers/media/platform/davinci/vpbe.c
@@ -340,7 +340,7 @@ static int vpbe_s_dv_timings(struct vpbe_device *vpbe_dev,
if (!(cfg->outputs[out_index].output.capabilities &
- V4L2_OUT_CAP_CUSTOM_TIMINGS))
+ V4L2_OUT_CAP_DV_TIMINGS))
return -EINVAL;
for (i = 0; i < output->num_modes; i++) {
@@ -408,7 +408,7 @@ static int vpbe_enum_dv_timings(struct vpbe_device
*vpbe_dev,
int j = 0;
int i;
- if (!(output->output.capabilities & V4L2_OUT_CAP_CUSTOM_TIMINGS))
+ if (!(output->output.capabilities & V4L2_OUT_CAP_DV_TIMINGS))
return -EINVAL;
for (i = 0; i < output->num_modes; i++) {
--
1.7.4.1
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
[PATCH] media: davinci: vpbe: fix build warning
From: Lad, Prabhakar
recent patch with commit id 4f996594ceaf6c3f9bc42b40c40b0f7f87b79c86
which makes vidioc_s_crop const, was causing a following build warning,
vpbe_display.c: In function 'vpbe_display_s_crop':
vpbe_display.c:640: warning: initialization discards qualifiers from pointer
target type
This patch fixes the above build warning.
Signed-off-by: Lad, Prabhakar
Signed-off-by: Manjunath Hadli
Cc: Hans Verkuil
Cc: Mauro Carvalho Chehab
---
drivers/media/platform/davinci/vpbe_display.c | 20 ++--
1 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/drivers/media/platform/davinci/vpbe_display.c
b/drivers/media/platform/davinci/vpbe_display.c
index 1b238fe..161c776 100644
--- a/drivers/media/platform/davinci/vpbe_display.c
+++ b/drivers/media/platform/davinci/vpbe_display.c
@@ -637,7 +637,7 @@ static int vpbe_display_s_crop(struct file *file, void
*priv,
struct vpbe_device *vpbe_dev = disp_dev->vpbe_dev;
struct osd_layer_config *cfg = &layer->layer_info.config;
struct osd_state *osd_device = disp_dev->osd_device;
- struct v4l2_rect *rect = &crop->c;
+ struct v4l2_rect rect = crop->c;
int ret;
v4l2_dbg(1, debug, &vpbe_dev->v4l2_dev,
@@ -648,21 +648,21 @@ static int vpbe_display_s_crop(struct file *file, void
*priv,
return -EINVAL;
}
- if (rect->top < 0)
- rect->top = 0;
- if (rect->left < 0)
- rect->left = 0;
+ if (rect.top < 0)
+ rect.top = 0;
+ if (rect.left < 0)
+ rect.left = 0;
- vpbe_disp_check_window_params(disp_dev, rect);
+ vpbe_disp_check_window_params(disp_dev, &rect);
osd_device->ops.get_layer_config(osd_device,
layer->layer_info.id, cfg);
vpbe_disp_calculate_scale_factor(disp_dev, layer,
- rect->width,
- rect->height);
- vpbe_disp_adj_position(disp_dev, layer, rect->top,
- rect->left);
+ rect.width,
+ rect.height);
+ vpbe_disp_adj_position(disp_dev, layer, rect.top,
+ rect.left);
ret = osd_device->ops.set_layer_config(osd_device,
layer->layer_info.id, cfg);
if (ret < 0) {
--
1.7.4.1
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [alsa-devel] [PATCH 0/3] ASoC: Davinci: McASP: add support new McASP IP Variant
On 31.08.2012 14:50, Hebbar, Gururaja wrote: > The OMAP2+ variant of McASP is different from Davinci variant w.r.to some > register offset and missing generic SRAM APIs support > > Changes > - Add new MCASP_VERSION_3 to identify new variant. New DT compatible > "ti,omap2-mcasp-audio" to identify version 3 controller. > - The register offsets are handled depending on the version. > - Provide a config option to indicate missing SRAM API support. Could you give some insight which hardware this was tested on? I'm trying to get this up and running on a AM33xx board, and the patches look all reasonable to me. My problem is that I can't make the DMA engine move forward, I fail to receive a single interrupt on this peripheral after the stream starts. I will continue searching for the reason of this tomorrow, but maybe you can give me some hint by explaining your setup? Note that I'm using your patches together with Matt's from this series: https://github.com/ohporter/linux/tree/edma-dmaengine-am33xx-rfc-v1 ... but it doesn't work without those either. Thanks, Daniel ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [alsa-devel] [PATCH 2/3] ASoC: Davinci: pcm: add support for sram-support-less platforms
On Tue, Oct 02, 2012 at 06:50:14PM +0200, Daniel Mack wrote: > On 02.10.2012 18:41, Matt Porter wrote: > > On Tue, Oct 02, 2012 at 03:42:47PM +0200, Daniel Mack wrote: > >> On 02.10.2012 13:06, Sekhar Nori wrote: > >>> On 10/2/2012 4:03 PM, Daniel Mack wrote: > On 02.10.2012 11:37, Mark Brown wrote: > > On Tue, Oct 02, 2012 at 10:48:53AM +0300, Peter Ujfalusi wrote: > > > >> I also agree that ifdef is not a good solution. > >> It is better to have this information passed as device_data and via DT > >> it can > >> be decided based on the compatible property for the device. > > > > That's not really the problem here - the problem is that the APIs used > > to get the SRAM are DaVinci only so it's not possible to build on OMAP > > or other platforms. The SRAM code needs to move to a standard API. > > What about following Matt Porter's idea and ignore the SRAM code > entirely and port the entire PCM code to generic dmaengine code first? > The EDMA driver needs to learn support for cyclic DMA for that, and I > might give that a try in near future. > > Later on, the SRAM ping-pong code can get added back using genalloc > functions, as Sekhar proposed. That needs to be done by someone who has > access to a Davinci board though, I only have a AM33xx/OMAP here. > >>> > >>> We cannot "get rid" of SRAM code and add it back "later". It is required > >>> for most DaVinci parts. The SRAM code can be converted to use genalloc > >>> (conversion should be straightforward and I can help test it) and the > >>> code that uses SRAM can probably keep using the private EDMA API till > >>> the dmaengine EDMA driver has cyclic DMA support. Matt has already > >>> posted patches to move private EDMA APIs to a common location. That > >>> should keep AM335x build from breaking. Is this something that is > >>> feasible? > >> > >> Yes - by "later" I just meant in a subsequent patch. But you're probably > >> right, we can also do that first. > >> > >> I'm looking at that right now and the problem seems that we don't have a > >> sane way to dynamically look up gen_pools independently of the actual > >> run-time platform. All users of gen_pools seem to know which platform > >> they run on and access a platform-specific pool. So I don't currently > >> see how we could implement multi-platform code, gen_pools are fine but > >> don't solve the problem here. > >> > >> Would it be an idea to add a char* member to gen_pools and a function > >> that can be used to dynamically look it up again? If a buffer with a > >> certain name exists, we can use it and install that ping-pong buffer, > >> otherwise we just don't. While that would be easy to do, it's a > >> tree-wide change. > >> > >> Is there a better way that I miss? > > > > At the high level there's two platform models we have to handle, the > > boot from board file !DT case, and then the boot from DT case. Since > > Davinci is just starting DT conversion, we mostly care about the !DT > > base in which the struct gen_pool * is passed in pdata to the ASoC > > driver. It is then selectable on a per-platform basis where the decision > > should be made. > > > > Given a separate discussion with Sekhar, we're only going to have one > > SRAM pool on any DaVinci part right now...this was only a question on > > L138 anyway. But regardless, the created gen_pool will be passed via > > pdata. > > I thought about this too, as mmp does it that way. > > > Since DT conversion is starting and we need to consider that now, > > the idea there is to use the DT-based generic sram driver [1] such that > > when we do boot from DT on Davinci, the genpool is provided via phandle > > and the pointer extracted with the OF helpers that are part of the > > series. > > A phandle is the cleanest way I think, yes. See the of_get_named_gen_pool() helper example in the series http://comments.gmane.org/gmane.linux.kernel/1352210 > > That's pretty much it. I'm reworking the backend support as discussed > > with Sekhar wrt to my uio_pruss series. I can post a standalone series > > that just replaces sram_* with genalloc for davinci ASoC. > > As you can also test it, it would be easiest if you came up with a patch > for that, yes. I can have a look at the dma bits laters, once my OMAP > board finally works with the code as it currently stands. I'm still > fighting with the mcasp driver right now ... Ok. Also, as Sekhar pointed out, dmaengine itself isn't a blocker since we can have AM33xx use the private EDMA API for ASoC until I finish cyclic DMA support. Handling the ping-pong and normal case transparently within the dmaengine driver will require a further look but at least genalloc is our first step there. -Matt ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [alsa-devel] [PATCH 2/3] ASoC: Davinci: pcm: add support for sram-support-less platforms
On 02.10.2012 18:41, Matt Porter wrote: > On Tue, Oct 02, 2012 at 03:42:47PM +0200, Daniel Mack wrote: >> On 02.10.2012 13:06, Sekhar Nori wrote: >>> On 10/2/2012 4:03 PM, Daniel Mack wrote: On 02.10.2012 11:37, Mark Brown wrote: > On Tue, Oct 02, 2012 at 10:48:53AM +0300, Peter Ujfalusi wrote: > >> I also agree that ifdef is not a good solution. >> It is better to have this information passed as device_data and via DT >> it can >> be decided based on the compatible property for the device. > > That's not really the problem here - the problem is that the APIs used > to get the SRAM are DaVinci only so it's not possible to build on OMAP > or other platforms. The SRAM code needs to move to a standard API. What about following Matt Porter's idea and ignore the SRAM code entirely and port the entire PCM code to generic dmaengine code first? The EDMA driver needs to learn support for cyclic DMA for that, and I might give that a try in near future. Later on, the SRAM ping-pong code can get added back using genalloc functions, as Sekhar proposed. That needs to be done by someone who has access to a Davinci board though, I only have a AM33xx/OMAP here. >>> >>> We cannot "get rid" of SRAM code and add it back "later". It is required >>> for most DaVinci parts. The SRAM code can be converted to use genalloc >>> (conversion should be straightforward and I can help test it) and the >>> code that uses SRAM can probably keep using the private EDMA API till >>> the dmaengine EDMA driver has cyclic DMA support. Matt has already >>> posted patches to move private EDMA APIs to a common location. That >>> should keep AM335x build from breaking. Is this something that is feasible? >> >> Yes - by "later" I just meant in a subsequent patch. But you're probably >> right, we can also do that first. >> >> I'm looking at that right now and the problem seems that we don't have a >> sane way to dynamically look up gen_pools independently of the actual >> run-time platform. All users of gen_pools seem to know which platform >> they run on and access a platform-specific pool. So I don't currently >> see how we could implement multi-platform code, gen_pools are fine but >> don't solve the problem here. >> >> Would it be an idea to add a char* member to gen_pools and a function >> that can be used to dynamically look it up again? If a buffer with a >> certain name exists, we can use it and install that ping-pong buffer, >> otherwise we just don't. While that would be easy to do, it's a >> tree-wide change. >> >> Is there a better way that I miss? > > At the high level there's two platform models we have to handle, the > boot from board file !DT case, and then the boot from DT case. Since > Davinci is just starting DT conversion, we mostly care about the !DT > base in which the struct gen_pool * is passed in pdata to the ASoC > driver. It is then selectable on a per-platform basis where the decision > should be made. > > Given a separate discussion with Sekhar, we're only going to have one > SRAM pool on any DaVinci part right now...this was only a question on > L138 anyway. But regardless, the created gen_pool will be passed via > pdata. I thought about this too, as mmp does it that way. > Since DT conversion is starting and we need to consider that now, > the idea there is to use the DT-based generic sram driver [1] such that > when we do boot from DT on Davinci, the genpool is provided via phandle > and the pointer extracted with the OF helpers that are part of the > series. A phandle is the cleanest way I think, yes. > That's pretty much it. I'm reworking the backend support as discussed > with Sekhar wrt to my uio_pruss series. I can post a standalone series > that just replaces sram_* with genalloc for davinci ASoC. As you can also test it, it would be easiest if you came up with a patch for that, yes. I can have a look at the dma bits laters, once my OMAP board finally works with the code as it currently stands. I'm still fighting with the mcasp driver right now ... Daniel ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [alsa-devel] [PATCH 2/3] ASoC: Davinci: pcm: add support for sram-support-less platforms
On Tue, Oct 02, 2012 at 03:42:47PM +0200, Daniel Mack wrote: > On 02.10.2012 13:06, Sekhar Nori wrote: > > On 10/2/2012 4:03 PM, Daniel Mack wrote: > >> On 02.10.2012 11:37, Mark Brown wrote: > >>> On Tue, Oct 02, 2012 at 10:48:53AM +0300, Peter Ujfalusi wrote: > >>> > I also agree that ifdef is not a good solution. > It is better to have this information passed as device_data and via DT > it can > be decided based on the compatible property for the device. > >>> > >>> That's not really the problem here - the problem is that the APIs used > >>> to get the SRAM are DaVinci only so it's not possible to build on OMAP > >>> or other platforms. The SRAM code needs to move to a standard API. > >> > >> What about following Matt Porter's idea and ignore the SRAM code > >> entirely and port the entire PCM code to generic dmaengine code first? > >> The EDMA driver needs to learn support for cyclic DMA for that, and I > >> might give that a try in near future. > >> > >> Later on, the SRAM ping-pong code can get added back using genalloc > >> functions, as Sekhar proposed. That needs to be done by someone who has > >> access to a Davinci board though, I only have a AM33xx/OMAP here. > > > > We cannot "get rid" of SRAM code and add it back "later". It is required > > for most DaVinci parts. The SRAM code can be converted to use genalloc > > (conversion should be straightforward and I can help test it) and the > > code that uses SRAM can probably keep using the private EDMA API till > > the dmaengine EDMA driver has cyclic DMA support. Matt has already > > posted patches to move private EDMA APIs to a common location. That > > should keep AM335x build from breaking. Is this something that is feasible? > > Yes - by "later" I just meant in a subsequent patch. But you're probably > right, we can also do that first. > > I'm looking at that right now and the problem seems that we don't have a > sane way to dynamically look up gen_pools independently of the actual > run-time platform. All users of gen_pools seem to know which platform > they run on and access a platform-specific pool. So I don't currently > see how we could implement multi-platform code, gen_pools are fine but > don't solve the problem here. > > Would it be an idea to add a char* member to gen_pools and a function > that can be used to dynamically look it up again? If a buffer with a > certain name exists, we can use it and install that ping-pong buffer, > otherwise we just don't. While that would be easy to do, it's a > tree-wide change. > > Is there a better way that I miss? At the high level there's two platform models we have to handle, the boot from board file !DT case, and then the boot from DT case. Since Davinci is just starting DT conversion, we mostly care about the !DT base in which the struct gen_pool * is passed in pdata to the ASoC driver. It is then selectable on a per-platform basis where the decision should be made. Given a separate discussion with Sekhar, we're only going to have one SRAM pool on any DaVinci part right now...this was only a question on L138 anyway. But regardless, the created gen_pool will be passed via pdata. Since DT conversion is starting and we need to consider that now, the idea there is to use the DT-based generic sram driver [1] such that when we do boot from DT on Davinci, the genpool is provided via phandle and the pointer extracted with the OF helpers that are part of the series. That's pretty much it. I'm reworking the backend support as discussed with Sekhar wrt to my uio_pruss series. I can post a standalone series that just replaces sram_* with genalloc for davinci ASoC. -Matt ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [alsa-devel] [PATCH 2/3] ASoC: Davinci: pcm: add support for sram-support-less platforms
On Tue, Oct 02, 2012 at 12:33:39PM +0200, Daniel Mack wrote: > On 02.10.2012 11:37, Mark Brown wrote: > > On Tue, Oct 02, 2012 at 10:48:53AM +0300, Peter Ujfalusi wrote: > > > >> I also agree that ifdef is not a good solution. > >> It is better to have this information passed as device_data and via DT it > >> can > >> be decided based on the compatible property for the device. > > > > That's not really the problem here - the problem is that the APIs used > > to get the SRAM are DaVinci only so it's not possible to build on OMAP > > or other platforms. The SRAM code needs to move to a standard API. > > What about following Matt Porter's idea and ignore the SRAM code > entirely and port the entire PCM code to generic dmaengine code first? > The EDMA driver needs to learn support for cyclic DMA for that, and I > might give that a try in near future. > > Later on, the SRAM ping-pong code can get added back using genalloc > functions, as Sekhar proposed. That needs to be done by someone who has > access to a Davinci board though, I only have a AM33xx/OMAP here. I already backed away from that idea since the older SoCs without a FIFO absolutely need ping-pong buffering in SRAM. -Matt ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [alsa-devel] [PATCH 2/3] ASoC: Davinci: pcm: add support for sram-support-less platforms
On Tue, Oct 02, 2012 at 10:48:53AM +0300, Peter Ujfalusi wrote: > On 09/22/2012 06:33 PM, Mark Brown wrote: > > On Fri, Aug 31, 2012 at 06:20:58PM +0530, Hebbar, Gururaja wrote: > > > >> +config SND_DAVINCI_HAVE_SRAM > >> + bool > >> + default y if ARCH_DAVINCI=y > >> + default n if ARCH_OMAP=y > >> + > > > > I've been sitting on this mostly since it seems like a step back from > > multi-platform kernels (which is where we're trying to get to) and I've > > been trying to decide what the best approach is. I'm thinking that we > > do want a generic API for allocating this stuff, it's a fairly generic > > feature (there's TCMs as well). > > > > Adding ifdefs like this does just doesn't seem good. > > I also agree that ifdef is not a good solution. > It is better to have this information passed as device_data and via DT it can > be decided based on the compatible property for the device. The driver is going to be used by both !DT and DT only platforms for a while so DT-centric solutions just don't make sense. There's a clean way to do this that was used for uio_pruss.c. When davinci is further along on DT conversion we can make use of the devicetree-based generic sram driver that's progressing along right now [1]. It needs some minor help to allow specifying the gen_pool allocation order, but it will work nicely for getting access to the right sram pool. -Matt [1] https://patchwork.kernel.org/patch/1421961/ ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH v2 4/7] ARM: davinci: Add support for an L3RAM gen_pool
On Tue, Oct 02, 2012 at 04:43:59PM +0530, Sekhar Nori wrote: > On 10/1/2012 7:20 PM, Ben Gardiner wrote: > > On Mon, Oct 1, 2012 at 8:32 AM, Matt Porter wrote: > >> On Mon, Oct 01, 2012 at 05:34:02PM +0530, Sekhar Nori wrote: > >>> Hi Matt, > >>> > >>> On 9/29/2012 1:07 AM, Matt Porter wrote: > L3RAM (shared SRAM) is needed for use by several drivers. > This creates a genalloc pool and a hook for the platform code > to provide the struct gen_pool * in platform data. > > Signed-off-by: Matt Porter > >>> > >>> I am not sure if any of the DaVinci devices have a need to allocate from > >>> *both* ARM RAM and shared RAM. Shared RAM is not present on all DaVinci > >>> devices AFAIR, and on DA850, there is just 8KB ARM RAM so I am not sure > >>> if there is much point in trying to allocate from there. > >>> > >>> Can you instead see if Ben's earlier patch[1] to use shared RAM for SRAM > >>> allocation on DA850 makes sense for your case? If yes, can you repost > >>> with Ben's patch included in your series instead of this patch? I would > >>> prefer that over creating a new pool for shared RAM. > >> > >> Hrm, I did look at Ben's earlier patch. The reason I added a separate > >> pool mostly was so I didn't have to touch the PM code at all. That can > >> continue using the private SRAM API with the ARM RAM as it is now. The > >> idea here was to allow that to be separate since no other bus masters > >> can access the ARM RAM anyway and do something that didn't require > >> regression testing PM. Also, I figured there's really no reason to use > >> even a tiny bit of the shared SRAM on PM if we have that ARM RAM there > >> and working fine for that use case. > >> [...] > > > > I agree with Matt. Preserving the use of the ARM RAM (8K on L138 -- as > > you said, Sekhar) in any fashion is preferable to moving suspend > > support into shared RAM. There is more of it (128K on L138) but also > > more pressure on allocations there since there are more clients. > > There is where I would like to see more information on who the potential > clients are. Even if DSP takes away 64K of the shared RAM on OMAP-L138, > there should be more than enough for PM, Audio and PRU. I haven't > checked the PM code size lately but it should be fairly small and I can > check the actual number if that helps. So, adding a new pool just to > save on those bytes doesn't sound like helping a lot. The only wildcard is PRU usage due to the small per-PRU memory on PRUss v1...though no sense in speculating further until somebody demonstrates a need. > > I appreciate that you are trying to preserve prior efforts in > > attempted merging of SRAM support -- thank you for that; however, that > > patch [1] was just an import of Subashish Ghosh's patch [2] -- I > > chose _that_ implementation option then mainly because I imagined it > > would be the least risky to get accepted upstream and not because of > > any particular technical merits. > > Its not a question of prior effort since Matt has already put in the > effort too. I am yet unconvinced that we need to add support to manage > two blocks of SoC internal RAM on DA850 in the kernel today. That's all. Yeah, I've come to agree with that. If somebody ends up needing that extra 8K of local ram on L138 then they can add support. -Matt ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH v2 4/7] ARM: davinci: Add support for an L3RAM gen_pool
On Tue, Oct 02, 2012 at 03:32:55PM +0530, Sekhar Nori wrote: > On 10/1/2012 6:02 PM, Matt Porter wrote: > > On Mon, Oct 01, 2012 at 05:34:02PM +0530, Sekhar Nori wrote: > >> Hi Matt, > >> > >> On 9/29/2012 1:07 AM, Matt Porter wrote: > >>> L3RAM (shared SRAM) is needed for use by several drivers. > >>> This creates a genalloc pool and a hook for the platform code > >>> to provide the struct gen_pool * in platform data. > >>> > >>> Signed-off-by: Matt Porter > >> > >> I am not sure if any of the DaVinci devices have a need to allocate from > >> *both* ARM RAM and shared RAM. Shared RAM is not present on all DaVinci > >> devices AFAIR, and on DA850, there is just 8KB ARM RAM so I am not sure > >> if there is much point in trying to allocate from there. > >> > >> Can you instead see if Ben's earlier patch[1] to use shared RAM for SRAM > >> allocation on DA850 makes sense for your case? If yes, can you repost > >> with Ben's patch included in your series instead of this patch? I would > >> prefer that over creating a new pool for shared RAM. > > > > Hrm, I did look at Ben's earlier patch. The reason I added a separate > > pool mostly was so I didn't have to touch the PM code at all. That can > > continue using the private SRAM API with the ARM RAM as it is now. The > > But you dont have to touch the PM code. PM code can continue using SRAM > API. I have verified in the past that PM can work using shared RAM. > > > idea here was to allow that to be separate since no other bus masters > > can access the ARM RAM anyway and do something that didn't require > > regression testing PM. Also, I figured there's really no reason to use > > even a tiny bit of the shared SRAM on PM if we have that ARM RAM there > > and working fine for that use case. > > I see no reason why PM would break with shared RAM. I have not even seen > reports of shared RAM being short of size so we need to save space by > having PM code in ARM RAM. I can test the changes before the code is > committed and it will get tested in linux-next as well. Ok, sounds good to me. > > The other thing is that Ben's patch needs to be rewritten to at least > > have the hook I added so we can provide the gen_pool in platform data. > > If you prefer this path still, I can add the needed hook on top of his > > original patch. Ultimately, I only *need* genalloc support for the > > shared sram so I can remove the private SRAM API from uio_pruss...so I'm > > happy with any way to get at it. > > Right, I prefer just adding the hook so that genalloc can be used along > with SRAM API. Ok. > > Oh, and to be honest...it's not just for uio_pruss, but also to cleanly > > remove the private SRAM API usage from the davinci ASoC driver too. > > Audio can use the shared RAM too. And once all users of the SRAM API are > gone, only the hook to help pass the gen_pool as platform data needs to > remain. Right, I think we are on the same page now. I'll post an update to Ben's original patch with required gen_pool hook for pdata use. I noticed the beginning of DT support for davinci and the DT-based genalloc driver, https://patchwork.kernel.org/patch/1421961/, fits into that well. -Matt ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH v2 4/7] ARM: davinci: Add support for an L3RAM gen_pool
On Tue, Oct 2, 2012 at 7:13 AM, Sekhar Nori wrote: > On 10/1/2012 7:20 PM, Ben Gardiner wrote: >> On Mon, Oct 1, 2012 at 8:32 AM, Matt Porter wrote: >>> On Mon, Oct 01, 2012 at 05:34:02PM +0530, Sekhar Nori wrote: Hi Matt, On 9/29/2012 1:07 AM, Matt Porter wrote: > L3RAM (shared SRAM) is needed for use by several drivers. > This creates a genalloc pool and a hook for the platform code > to provide the struct gen_pool * in platform data. > > Signed-off-by: Matt Porter I am not sure if any of the DaVinci devices have a need to allocate from *both* ARM RAM and shared RAM. Shared RAM is not present on all DaVinci devices AFAIR, and on DA850, there is just 8KB ARM RAM so I am not sure if there is much point in trying to allocate from there. Can you instead see if Ben's earlier patch[1] to use shared RAM for SRAM allocation on DA850 makes sense for your case? If yes, can you repost with Ben's patch included in your series instead of this patch? I would prefer that over creating a new pool for shared RAM. >>> >>> Hrm, I did look at Ben's earlier patch. The reason I added a separate >>> pool mostly was so I didn't have to touch the PM code at all. That can >>> continue using the private SRAM API with the ARM RAM as it is now. The >>> idea here was to allow that to be separate since no other bus masters >>> can access the ARM RAM anyway and do something that didn't require >>> regression testing PM. Also, I figured there's really no reason to use >>> even a tiny bit of the shared SRAM on PM if we have that ARM RAM there >>> and working fine for that use case. >>> [...] >> >> I agree with Matt. Preserving the use of the ARM RAM (8K on L138 -- as >> you said, Sekhar) in any fashion is preferable to moving suspend >> support into shared RAM. There is more of it (128K on L138) but also >> more pressure on allocations there since there are more clients. > > There is where I would like to see more information on who the potential > clients are. Even if DSP takes away 64K of the shared RAM on OMAP-L138, > there should be more than enough for PM, Audio and PRU. I haven't > checked the PM code size lately but it should be fairly small and I can > check the actual number if that helps. So, adding a new pool just to > save on those bytes doesn't sound like helping a lot. Good points. I suppose that the list would decrease also on L138 if davinci-pcm ping-pong buffers were removed from SRAM. >> I appreciate that you are trying to preserve prior efforts in >> attempted merging of SRAM support -- thank you for that; however, that >> patch [1] was just an import of Subashish Ghosh's patch [2] -- I >> chose _that_ implementation option then mainly because I imagined it >> would be the least risky to get accepted upstream and not because of >> any particular technical merits. > > Its not a question of prior effort since Matt has already put in the > effort too. I am yet unconvinced that we need to add support to manage > two blocks of SoC internal RAM on DA850 in the kernel today. That's all. Understood. Thank you, Sekhar. Best Regards, Ben Gardiner --- Nanometrics Inc. http://www.nanometrics.ca ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [alsa-devel] [PATCH 2/3] ASoC: Davinci: pcm: add support for sram-support-less platforms
On 02.10.2012 13:06, Sekhar Nori wrote: > On 10/2/2012 4:03 PM, Daniel Mack wrote: >> On 02.10.2012 11:37, Mark Brown wrote: >>> On Tue, Oct 02, 2012 at 10:48:53AM +0300, Peter Ujfalusi wrote: >>> I also agree that ifdef is not a good solution. It is better to have this information passed as device_data and via DT it can be decided based on the compatible property for the device. >>> >>> That's not really the problem here - the problem is that the APIs used >>> to get the SRAM are DaVinci only so it's not possible to build on OMAP >>> or other platforms. The SRAM code needs to move to a standard API. >> >> What about following Matt Porter's idea and ignore the SRAM code >> entirely and port the entire PCM code to generic dmaengine code first? >> The EDMA driver needs to learn support for cyclic DMA for that, and I >> might give that a try in near future. >> >> Later on, the SRAM ping-pong code can get added back using genalloc >> functions, as Sekhar proposed. That needs to be done by someone who has >> access to a Davinci board though, I only have a AM33xx/OMAP here. > > We cannot "get rid" of SRAM code and add it back "later". It is required > for most DaVinci parts. The SRAM code can be converted to use genalloc > (conversion should be straightforward and I can help test it) and the > code that uses SRAM can probably keep using the private EDMA API till > the dmaengine EDMA driver has cyclic DMA support. Matt has already > posted patches to move private EDMA APIs to a common location. That > should keep AM335x build from breaking. Is this something that is feasible? Yes - by "later" I just meant in a subsequent patch. But you're probably right, we can also do that first. I'm looking at that right now and the problem seems that we don't have a sane way to dynamically look up gen_pools independently of the actual run-time platform. All users of gen_pools seem to know which platform they run on and access a platform-specific pool. So I don't currently see how we could implement multi-platform code, gen_pools are fine but don't solve the problem here. Would it be an idea to add a char* member to gen_pools and a function that can be used to dynamically look it up again? If a buffer with a certain name exists, we can use it and install that ping-pong buffer, otherwise we just don't. While that would be easy to do, it's a tree-wide change. Is there a better way that I miss? Daniel ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [RFC PATCH 08/13] mmc: omap_hsmmc: limit max_segs with the EDMA DMAC
On Mon, 2012-10-01 at 12:39 -0400, Matt Porter wrote:
> Anything you can show at this point? ;) I'd be happy to drop the
> half-hack
> for a real API. If not, I'm going to carry that to v2 atm.
This is what I had done sometime back. Feel free to update
diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
index 9c02a45..94ae006 100644
--- a/include/linux/dmaengine.h
+++ b/include/linux/dmaengine.h
@@ -86,11 +86,11 @@ enum dma_transaction_type {
* @DMA_DEV_TO_DEV: Slave mode & From Device to Device
*/
enum dma_transfer_direction {
- DMA_MEM_TO_MEM,
- DMA_MEM_TO_DEV,
- DMA_DEV_TO_MEM,
- DMA_DEV_TO_DEV,
- DMA_TRANS_NONE,
+ DMA_MEM_TO_MEM = 0x01,
+ DMA_MEM_TO_DEV = 0x02,
+ DMA_DEV_TO_MEM = 0x04,
+ DMA_DEV_TO_DEV = 0x08,
+ DMA_TRANS_NONE = 0x10,
};
/**
@@ -371,6 +371,41 @@ struct dma_slave_config {
unsigned int slave_id;
};
+enum dmaengine_apis {
+ DMAENGINE_MEMCPY= 0x0001,
+ DMAENGINE_XOR = 0x0002,
+ DMAENGINE_XOR_VAL = 0x0004,
+ DMAENGINE_PQ= 0x0008,
+ DMAENGINE_PQ_VAL= 0x0010,
+ DMAENGINE_MEMSET= 0x0020,
+ DMAENGINE_SLAVE = 0x0040,
+ DMAENGINE_CYCLIC= 0x0080,
+ DMAENGINE_INTERLEAVED = 0x0100,
+ DMAENGINE_SG= 0x0200,
+};
+
+/* struct dmaengine_chan_caps - expose capability of a channel
+ * Note: each channel can have same or different capabilities
+ *
+ * This primarily classifies capabilities into
+ * a) APIs/ops supported
+ * b) channel physical capabilities
+ *
+ * @ops: or'ed api capability
+ * @widths: channel widths supported
+ * @dirn: channel directions supported
+ * @bursts: bitmask of burst lengths supported
+ * @mux: configurable slave id or hard wired
+ * -1 for hard wired, otherwise valid positive slave id (including zero)
+ */
+struct dmaengine_chan_caps {
+ enum dmaengine_apis ops;
+ enum dma_slave_buswidth widths;
+ enum dma_transfer_direction dirn;
+ unsigned int dma_bursts;
+ int mux;
+};
+
static inline const char *dma_chan_name(struct dma_chan *chan)
{
return dev_name(&chan->dev->device);
@@ -534,6 +569,7 @@ struct dma_tx_state {
* struct with auxiliary transfer status information, otherwise the call
* will just return a simple status code
* @device_issue_pending: push pending transactions to hardware
+ * @device_channel_caps: return the capablities of channel
*/
struct dma_device {
@@ -602,6 +638,9 @@ struct dma_device {
dma_cookie_t cookie,
struct dma_tx_state *txstate);
void (*device_issue_pending)(struct dma_chan *chan);
+
+ struct dmaengine_chan_caps *(*device_channel_caps)(
+ struct dma_chan *chan);
};
static inline int dmaengine_device_control(struct dma_chan *chan,
--
~Vinod
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH v2 4/7] ARM: davinci: Add support for an L3RAM gen_pool
On 10/1/2012 7:20 PM, Ben Gardiner wrote: > On Mon, Oct 1, 2012 at 8:32 AM, Matt Porter wrote: >> On Mon, Oct 01, 2012 at 05:34:02PM +0530, Sekhar Nori wrote: >>> Hi Matt, >>> >>> On 9/29/2012 1:07 AM, Matt Porter wrote: L3RAM (shared SRAM) is needed for use by several drivers. This creates a genalloc pool and a hook for the platform code to provide the struct gen_pool * in platform data. Signed-off-by: Matt Porter >>> >>> I am not sure if any of the DaVinci devices have a need to allocate from >>> *both* ARM RAM and shared RAM. Shared RAM is not present on all DaVinci >>> devices AFAIR, and on DA850, there is just 8KB ARM RAM so I am not sure >>> if there is much point in trying to allocate from there. >>> >>> Can you instead see if Ben's earlier patch[1] to use shared RAM for SRAM >>> allocation on DA850 makes sense for your case? If yes, can you repost >>> with Ben's patch included in your series instead of this patch? I would >>> prefer that over creating a new pool for shared RAM. >> >> Hrm, I did look at Ben's earlier patch. The reason I added a separate >> pool mostly was so I didn't have to touch the PM code at all. That can >> continue using the private SRAM API with the ARM RAM as it is now. The >> idea here was to allow that to be separate since no other bus masters >> can access the ARM RAM anyway and do something that didn't require >> regression testing PM. Also, I figured there's really no reason to use >> even a tiny bit of the shared SRAM on PM if we have that ARM RAM there >> and working fine for that use case. >> [...] > > I agree with Matt. Preserving the use of the ARM RAM (8K on L138 -- as > you said, Sekhar) in any fashion is preferable to moving suspend > support into shared RAM. There is more of it (128K on L138) but also > more pressure on allocations there since there are more clients. There is where I would like to see more information on who the potential clients are. Even if DSP takes away 64K of the shared RAM on OMAP-L138, there should be more than enough for PM, Audio and PRU. I haven't checked the PM code size lately but it should be fairly small and I can check the actual number if that helps. So, adding a new pool just to save on those bytes doesn't sound like helping a lot. > I appreciate that you are trying to preserve prior efforts in > attempted merging of SRAM support -- thank you for that; however, that > patch [1] was just an import of Subashish Ghosh's patch [2] -- I > chose _that_ implementation option then mainly because I imagined it > would be the least risky to get accepted upstream and not because of > any particular technical merits. Its not a question of prior effort since Matt has already put in the effort too. I am yet unconvinced that we need to add support to manage two blocks of SoC internal RAM on DA850 in the kernel today. That's all. Thanks, Sekhar ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [alsa-devel] [PATCH 2/3] ASoC: Davinci: pcm: add support for sram-support-less platforms
On 10/2/2012 4:03 PM, Daniel Mack wrote: > On 02.10.2012 11:37, Mark Brown wrote: >> On Tue, Oct 02, 2012 at 10:48:53AM +0300, Peter Ujfalusi wrote: >> >>> I also agree that ifdef is not a good solution. >>> It is better to have this information passed as device_data and via DT it >>> can >>> be decided based on the compatible property for the device. >> >> That's not really the problem here - the problem is that the APIs used >> to get the SRAM are DaVinci only so it's not possible to build on OMAP >> or other platforms. The SRAM code needs to move to a standard API. > > What about following Matt Porter's idea and ignore the SRAM code > entirely and port the entire PCM code to generic dmaengine code first? > The EDMA driver needs to learn support for cyclic DMA for that, and I > might give that a try in near future. > > Later on, the SRAM ping-pong code can get added back using genalloc > functions, as Sekhar proposed. That needs to be done by someone who has > access to a Davinci board though, I only have a AM33xx/OMAP here. We cannot "get rid" of SRAM code and add it back "later". It is required for most DaVinci parts. The SRAM code can be converted to use genalloc (conversion should be straightforward and I can help test it) and the code that uses SRAM can probably keep using the private EDMA API till the dmaengine EDMA driver has cyclic DMA support. Matt has already posted patches to move private EDMA APIs to a common location. That should keep AM335x build from breaking. Is this something that is feasible? Thanks, Sekhar ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [alsa-devel] [PATCH 2/3] ASoC: Davinci: pcm: add support for sram-support-less platforms
On 02.10.2012 11:37, Mark Brown wrote: > On Tue, Oct 02, 2012 at 10:48:53AM +0300, Peter Ujfalusi wrote: > >> I also agree that ifdef is not a good solution. >> It is better to have this information passed as device_data and via DT it can >> be decided based on the compatible property for the device. > > That's not really the problem here - the problem is that the APIs used > to get the SRAM are DaVinci only so it's not possible to build on OMAP > or other platforms. The SRAM code needs to move to a standard API. What about following Matt Porter's idea and ignore the SRAM code entirely and port the entire PCM code to generic dmaengine code first? The EDMA driver needs to learn support for cyclic DMA for that, and I might give that a try in near future. Later on, the SRAM ping-pong code can get added back using genalloc functions, as Sekhar proposed. That needs to be done by someone who has access to a Davinci board though, I only have a AM33xx/OMAP here. Daniel ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH v2 4/7] ARM: davinci: Add support for an L3RAM gen_pool
On 10/1/2012 6:02 PM, Matt Porter wrote: > On Mon, Oct 01, 2012 at 05:34:02PM +0530, Sekhar Nori wrote: >> Hi Matt, >> >> On 9/29/2012 1:07 AM, Matt Porter wrote: >>> L3RAM (shared SRAM) is needed for use by several drivers. >>> This creates a genalloc pool and a hook for the platform code >>> to provide the struct gen_pool * in platform data. >>> >>> Signed-off-by: Matt Porter >> >> I am not sure if any of the DaVinci devices have a need to allocate from >> *both* ARM RAM and shared RAM. Shared RAM is not present on all DaVinci >> devices AFAIR, and on DA850, there is just 8KB ARM RAM so I am not sure >> if there is much point in trying to allocate from there. >> >> Can you instead see if Ben's earlier patch[1] to use shared RAM for SRAM >> allocation on DA850 makes sense for your case? If yes, can you repost >> with Ben's patch included in your series instead of this patch? I would >> prefer that over creating a new pool for shared RAM. > > Hrm, I did look at Ben's earlier patch. The reason I added a separate > pool mostly was so I didn't have to touch the PM code at all. That can > continue using the private SRAM API with the ARM RAM as it is now. The But you dont have to touch the PM code. PM code can continue using SRAM API. I have verified in the past that PM can work using shared RAM. > idea here was to allow that to be separate since no other bus masters > can access the ARM RAM anyway and do something that didn't require > regression testing PM. Also, I figured there's really no reason to use > even a tiny bit of the shared SRAM on PM if we have that ARM RAM there > and working fine for that use case. I see no reason why PM would break with shared RAM. I have not even seen reports of shared RAM being short of size so we need to save space by having PM code in ARM RAM. I can test the changes before the code is committed and it will get tested in linux-next as well. > The other thing is that Ben's patch needs to be rewritten to at least > have the hook I added so we can provide the gen_pool in platform data. > If you prefer this path still, I can add the needed hook on top of his > original patch. Ultimately, I only *need* genalloc support for the > shared sram so I can remove the private SRAM API from uio_pruss...so I'm > happy with any way to get at it. Right, I prefer just adding the hook so that genalloc can be used along with SRAM API. > Oh, and to be honest...it's not just for uio_pruss, but also to cleanly > remove the private SRAM API usage from the davinci ASoC driver too. Audio can use the shared RAM too. And once all users of the SRAM API are gone, only the hook to help pass the gen_pool as platform data needs to remain. Thanks, Sekhar ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [alsa-devel] [PATCH 2/3] ASoC: Davinci: pcm: add support for sram-support-less platforms
On Tue, Oct 02, 2012 at 10:48:53AM +0300, Peter Ujfalusi wrote: > I also agree that ifdef is not a good solution. > It is better to have this information passed as device_data and via DT it can > be decided based on the compatible property for the device. That's not really the problem here - the problem is that the APIs used to get the SRAM are DaVinci only so it's not possible to build on OMAP or other platforms. The SRAM code needs to move to a standard API. ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH v3] media: davinci: vpif: display: separate out subdev from output
On 9/25/2012 4:41 PM, Prabhakar wrote: > From: Lad, Prabhakar > > vpif_display relied on a 1-1 mapping of output and subdev. This is not > necessarily the case. Separate the two. So there is a list of subdevs > and a list of outputs. Each output refers to a subdev and has routing > information. An output does not have to have a subdev. > > The initial output for each channel is set to the fist output. > > Currently missing is support for associating multiple subdevs with > an output. > > Signed-off-by: Lad, Prabhakar > Signed-off-by: Manjunath Hadli > Cc: Hans Verkuil > Cc: Sekhar Nori For the DaVinci platform changes: Acked-by: Sekhar Nori Thanks, Sekhar ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH 2/3] ASoC: Davinci: pcm: add support for sram-support-less platforms
Hi Mark, On 9/22/2012 9:03 PM, Mark Brown wrote: > On Fri, Aug 31, 2012 at 06:20:58PM +0530, Hebbar, Gururaja wrote: > >> +config SND_DAVINCI_HAVE_SRAM >> +bool >> +default y if ARCH_DAVINCI=y >> +default n if ARCH_OMAP=y >> + > > I've been sitting on this mostly since it seems like a step back from > multi-platform kernels (which is where we're trying to get to) and I've > been trying to decide what the best approach is. I'm thinking that we > do want a generic API for allocating this stuff, it's a fairly generic > feature (there's TCMs as well). > > Adding ifdefs like this does just doesn't seem good. How about converting to use genalloc instead of the DaVinci private SRAM API and passing the pool to be used via platform data? Matt Porter just used this approach to make the uio PRUSS driver usable on both DaVinci and AM335x. It suffered from the exact same problem. https://patchwork.kernel.org/patch/1522481/ Thanks, Sekhar ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [alsa-devel] [PATCH 2/3] ASoC: Davinci: pcm: add support for sram-support-less platforms
On 09/22/2012 06:33 PM, Mark Brown wrote: > On Fri, Aug 31, 2012 at 06:20:58PM +0530, Hebbar, Gururaja wrote: > >> +config SND_DAVINCI_HAVE_SRAM >> +bool >> +default y if ARCH_DAVINCI=y >> +default n if ARCH_OMAP=y >> + > > I've been sitting on this mostly since it seems like a step back from > multi-platform kernels (which is where we're trying to get to) and I've > been trying to decide what the best approach is. I'm thinking that we > do want a generic API for allocating this stuff, it's a fairly generic > feature (there's TCMs as well). > > Adding ifdefs like this does just doesn't seem good. I also agree that ifdef is not a good solution. It is better to have this information passed as device_data and via DT it can be decided based on the compatible property for the device. -- Péter ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
