[PATCH 0/3] Davinci DSP remoteproc device improvements

[PATCH 0/3] Davinci DSP remoteproc device improvements

[Suman Anna]

Hi Sekhar,

The following series makes some minor improvements to the legacy-mode
Davinci remoteproc device creation logic in preparation for adding the
DT support to the Davinci remoteproc driver. The patches are baselined
on 4.12-rc1. The changes include conditionally creating the DSP device
based on memory supplied to the remoteproc device, and adds resource
names and additional resources for publishing the DSP internal memory
regions to the davinci remoteproc driver.

Patches tested on OMAP-L138 LCDK board by booting it in legacy-mode.

regards
Suman

Suman Anna (3):
  ARM: davinci: da8xx: Create DSP device only when assigned memory
  ARM: davinci: da8xx: Add names to DSP IOMEM resources
  ARM: davinci: da8xx: Add DSP internal RAM memories as IOMEM resources

 arch/arm/mach-davinci/devices-da8xx.c      | 30 ++++++++++++++++++++++++++++++
 arch/arm/mach-davinci/include/mach/da8xx.h |  5 +++++
 2 files changed, 35 insertions(+)

-- 
2.12.0

[PATCH 1/3] ARM: davinci: da8xx: Create DSP device only when assigned memory

[Suman Anna]

The DSP device on Davinci platforms does not have an MMU and requires
specific DDR memory to boot. This memory is reserved using the rproc_mem
kernel boot parameter and is assigned to the device on non-DT boots.
The remoteproc core uses the DMA API and so will fall back to assigning
random memory if this memory is not assigned to the device, but the DSP
remote processor boot will not be successful in such cases. So, check
that memory has been reserved and assigned to the device specifically
before even creating the DSP device.

Signed-off-by: Suman Anna <s-anna@ti.com>
---
 arch/arm/mach-davinci/devices-da8xx.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/arm/mach-davinci/devices-da8xx.c b/arch/arm/mach-davinci/devices-da8xx.c
index 7cf529ffbe5a..1ccf52e49886 100644
--- a/arch/arm/mach-davinci/devices-da8xx.c
+++ b/arch/arm/mach-davinci/devices-da8xx.c
@@ -814,6 +814,8 @@ static struct platform_device da8xx_dsp = {
 	.resource	= da8xx_rproc_resources,
 };
 
+static bool rproc_mem_inited __initdata;
+
 #if IS_ENABLED(CONFIG_DA8XX_REMOTEPROC)
 
 static phys_addr_t rproc_base __initdata;
@@ -852,6 +854,8 @@ void __init da8xx_rproc_reserve_cma(void)
 	ret = dma_declare_contiguous(&da8xx_dsp.dev, rproc_size, rproc_base, 0);
 	if (ret)
 		pr_err("%s: dma_declare_contiguous failed %d\n", __func__, ret);
+	else
+		rproc_mem_inited = true;
 }
 
 #else
@@ -866,6 +870,12 @@ int __init da8xx_register_rproc(void)
 {
 	int ret;
 
+	if (!rproc_mem_inited) {
+		pr_warn("%s: memory not reserved for DSP, not registering DSP device\n",
+			__func__);
+		return -ENOMEM;
+	}
+
 	ret = platform_device_register(&da8xx_dsp);
 	if (ret)
 		pr_err("%s: can't register DSP device: %d\n", __func__, ret);
-- 
2.12.0

Re: [PATCH 1/3] ARM: davinci: da8xx: Create DSP device only when assigned memory

[Sekhar Nori]

On Wednesday 17 May 2017 03:43 AM, Suman Anna wrote:
> The DSP device on Davinci platforms does not have an MMU and requires
> specific DDR memory to boot. This memory is reserved using the rproc_mem
> kernel boot parameter and is assigned to the device on non-DT boots.
> The remoteproc core uses the DMA API and so will fall back to assigning
> random memory if this memory is not assigned to the device, but the DSP
> remote processor boot will not be successful in such cases. So, check
> that memory has been reserved and assigned to the device specifically
> before even creating the DSP device.
> 
> Signed-off-by: Suman Anna <s-anna@ti.com>
> ---
>  arch/arm/mach-davinci/devices-da8xx.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/arch/arm/mach-davinci/devices-da8xx.c b/arch/arm/mach-davinci/devices-da8xx.c
> index 7cf529ffbe5a..1ccf52e49886 100644
> --- a/arch/arm/mach-davinci/devices-da8xx.c
> +++ b/arch/arm/mach-davinci/devices-da8xx.c
> @@ -814,6 +814,8 @@ static struct platform_device da8xx_dsp = {
>  	.resource	= da8xx_rproc_resources,
>  };
>  
> +static bool rproc_mem_inited __initdata;
> +
>  #if IS_ENABLED(CONFIG_DA8XX_REMOTEPROC)
>  
>  static phys_addr_t rproc_base __initdata;
> @@ -852,6 +854,8 @@ void __init da8xx_rproc_reserve_cma(void)
>  	ret = dma_declare_contiguous(&da8xx_dsp.dev, rproc_size, rproc_base, 0);
>  	if (ret)
>  		pr_err("%s: dma_declare_contiguous failed %d\n", __func__, ret);
> +	else
> +		rproc_mem_inited = true;
>  }
>  
>  #else
> @@ -866,6 +870,12 @@ int __init da8xx_register_rproc(void)
>  {
>  	int ret;
>  
> +	if (!rproc_mem_inited) {
> +		pr_warn("%s: memory not reserved for DSP, not registering DSP device\n",
> +			__func__);

We now have a warning and an error if dma_declare_contiguous() fails. I
like this message better. So can you replace the existing error message
with this text instead ?

Thanks,
Sekhar

Re: [PATCH 1/3] ARM: davinci: da8xx: Create DSP device only when assigned memory

[Suman Anna]

Hi Sekhar,

On 05/18/2017 01:10 AM, Sekhar Nori wrote:
> On Wednesday 17 May 2017 03:43 AM, Suman Anna wrote:
>> The DSP device on Davinci platforms does not have an MMU and requires
>> specific DDR memory to boot. This memory is reserved using the rproc_mem
>> kernel boot parameter and is assigned to the device on non-DT boots.
>> The remoteproc core uses the DMA API and so will fall back to assigning
>> random memory if this memory is not assigned to the device, but the DSP
>> remote processor boot will not be successful in such cases. So, check
>> that memory has been reserved and assigned to the device specifically
>> before even creating the DSP device.
>>
>> Signed-off-by: Suman Anna <s-anna@ti.com>
>> ---
>>  arch/arm/mach-davinci/devices-da8xx.c | 10 ++++++++++
>>  1 file changed, 10 insertions(+)
>>
>> diff --git a/arch/arm/mach-davinci/devices-da8xx.c b/arch/arm/mach-davinci/devices-da8xx.c
>> index 7cf529ffbe5a..1ccf52e49886 100644
>> --- a/arch/arm/mach-davinci/devices-da8xx.c
>> +++ b/arch/arm/mach-davinci/devices-da8xx.c
>> @@ -814,6 +814,8 @@ static struct platform_device da8xx_dsp = {
>>  	.resource	= da8xx_rproc_resources,
>>  };
>>  
>> +static bool rproc_mem_inited __initdata;
>> +
>>  #if IS_ENABLED(CONFIG_DA8XX_REMOTEPROC)
>>  
>>  static phys_addr_t rproc_base __initdata;
>> @@ -852,6 +854,8 @@ void __init da8xx_rproc_reserve_cma(void)
>>  	ret = dma_declare_contiguous(&da8xx_dsp.dev, rproc_size, rproc_base, 0);
>>  	if (ret)
>>  		pr_err("%s: dma_declare_contiguous failed %d\n", __func__, ret);
>> +	else
>> +		rproc_mem_inited = true;
>>  }
>>  
>>  #else
>> @@ -866,6 +870,12 @@ int __init da8xx_register_rproc(void)
>>  {
>>  	int ret;
>>  
>> +	if (!rproc_mem_inited) {
>> +		pr_warn("%s: memory not reserved for DSP, not registering DSP device\n",
>> +			__func__);
> 
> We now have a warning and an error if dma_declare_contiguous() fails. I
> like this message better. So can you replace the existing error message
> with this text instead ?

Hmm, this trace is not just covering the dma_declare_contiguous failure.
There can be two different errors in da8xx_rproc_reserve_cma, and this
trace only shows up when the device is being registered. So not sure how
your suggestion improves anything by dropping/replacing one of them.

regards
Suman

Re: [PATCH 1/3] ARM: davinci: da8xx: Create DSP device only when assigned memory

[Sekhar Nori]

On Thursday 18 May 2017 09:29 PM, Suman Anna wrote:
> Hi Sekhar,
> 
> On 05/18/2017 01:10 AM, Sekhar Nori wrote:
>> On Wednesday 17 May 2017 03:43 AM, Suman Anna wrote:
>>> The DSP device on Davinci platforms does not have an MMU and requires
>>> specific DDR memory to boot. This memory is reserved using the rproc_mem
>>> kernel boot parameter and is assigned to the device on non-DT boots.
>>> The remoteproc core uses the DMA API and so will fall back to assigning
>>> random memory if this memory is not assigned to the device, but the DSP
>>> remote processor boot will not be successful in such cases. So, check
>>> that memory has been reserved and assigned to the device specifically
>>> before even creating the DSP device.
>>>
>>> Signed-off-by: Suman Anna <s-anna@ti.com>
>>> ---
>>>  arch/arm/mach-davinci/devices-da8xx.c | 10 ++++++++++
>>>  1 file changed, 10 insertions(+)
>>>
>>> diff --git a/arch/arm/mach-davinci/devices-da8xx.c b/arch/arm/mach-davinci/devices-da8xx.c
>>> index 7cf529ffbe5a..1ccf52e49886 100644
>>> --- a/arch/arm/mach-davinci/devices-da8xx.c
>>> +++ b/arch/arm/mach-davinci/devices-da8xx.c
>>> @@ -814,6 +814,8 @@ static struct platform_device da8xx_dsp = {
>>>  	.resource	= da8xx_rproc_resources,
>>>  };
>>>  
>>> +static bool rproc_mem_inited __initdata;
>>> +
>>>  #if IS_ENABLED(CONFIG_DA8XX_REMOTEPROC)
>>>  
>>>  static phys_addr_t rproc_base __initdata;
>>> @@ -852,6 +854,8 @@ void __init da8xx_rproc_reserve_cma(void)
>>>  	ret = dma_declare_contiguous(&da8xx_dsp.dev, rproc_size, rproc_base, 0);
>>>  	if (ret)
>>>  		pr_err("%s: dma_declare_contiguous failed %d\n", __func__, ret);
>>> +	else
>>> +		rproc_mem_inited = true;
>>>  }
>>>  
>>>  #else
>>> @@ -866,6 +870,12 @@ int __init da8xx_register_rproc(void)
>>>  {
>>>  	int ret;
>>>  
>>> +	if (!rproc_mem_inited) {
>>> +		pr_warn("%s: memory not reserved for DSP, not registering DSP device\n",
>>> +			__func__);
>>
>> We now have a warning and an error if dma_declare_contiguous() fails. I
>> like this message better. So can you replace the existing error message
>> with this text instead ?
> 
> Hmm, this trace is not just covering the dma_declare_contiguous failure.
> There can be two different errors in da8xx_rproc_reserve_cma, and this

I seem to have missed that part. I now applied this series to v4.13/soc.
I don't have a branch in linux-next. So, it may be a while before it
ends up there. But I do merge all the patches I apply to master branch
of my tree.

Thanks,
Sekhar

[PATCH 2/3] ARM: davinci: da8xx: Add names to DSP IOMEM resources

[Suman Anna]

Add names to the IOMEM resources for the DSP device present on
DA8xx SoCs. This will facilitate the driver to use the names and
be agnostic of the resource order, and facilitate scalable DT
support in follow-up patches.

Signed-off-by: Suman Anna <s-anna@ti.com>
---
 arch/arm/mach-davinci/devices-da8xx.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm/mach-davinci/devices-da8xx.c b/arch/arm/mach-davinci/devices-da8xx.c
index 1ccf52e49886..74c76538cda3 100644
--- a/arch/arm/mach-davinci/devices-da8xx.c
+++ b/arch/arm/mach-davinci/devices-da8xx.c
@@ -789,11 +789,13 @@ int __init da850_register_mmcsd1(struct davinci_mmc_config *config)
 
 static struct resource da8xx_rproc_resources[] = {
 	{ /* DSP boot address */
+		.name		= "host1cfg",
 		.start		= DA8XX_SYSCFG0_BASE + DA8XX_HOST1CFG_REG,
 		.end		= DA8XX_SYSCFG0_BASE + DA8XX_HOST1CFG_REG + 3,
 		.flags		= IORESOURCE_MEM,
 	},
 	{ /* DSP interrupt registers */
+		.name		= "chipsig",
 		.start		= DA8XX_SYSCFG0_BASE + DA8XX_CHIPSIG_REG,
 		.end		= DA8XX_SYSCFG0_BASE + DA8XX_CHIPSIG_REG + 7,
 		.flags		= IORESOURCE_MEM,
-- 
2.12.0

[PATCH 3/3] ARM: davinci: da8xx: Add DSP internal RAM memories as IOMEM resources

[Suman Anna]

The DSP subsystem on DA8xx has various internal RAM memories that can
accessed from the ARM side. These memories can be configured to be
used as either RAM or Cache. Add these memories as IOMEM resources
to the DSP device so that the driver can support loading of images
into internal memories.

Signed-off-by: Suman Anna <s-anna@ti.com>
---
 arch/arm/mach-davinci/devices-da8xx.c      | 18 ++++++++++++++++++
 arch/arm/mach-davinci/include/mach/da8xx.h |  5 +++++
 2 files changed, 23 insertions(+)

diff --git a/arch/arm/mach-davinci/devices-da8xx.c b/arch/arm/mach-davinci/devices-da8xx.c
index 74c76538cda3..22440c05d66a 100644
--- a/arch/arm/mach-davinci/devices-da8xx.c
+++ b/arch/arm/mach-davinci/devices-da8xx.c
@@ -800,6 +800,24 @@ static struct resource da8xx_rproc_resources[] = {
 		.end		= DA8XX_SYSCFG0_BASE + DA8XX_CHIPSIG_REG + 7,
 		.flags		= IORESOURCE_MEM,
 	},
+	{ /* DSP L2 RAM */
+		.name		= "l2sram",
+		.start		= DA8XX_DSP_L2_RAM_BASE,
+		.end		= DA8XX_DSP_L2_RAM_BASE + SZ_256K - 1,
+		.flags		= IORESOURCE_MEM,
+	},
+	{ /* DSP L1P RAM */
+		.name		= "l1pram",
+		.start		= DA8XX_DSP_L1P_RAM_BASE,
+		.end		= DA8XX_DSP_L1P_RAM_BASE + SZ_32K - 1,
+		.flags		= IORESOURCE_MEM,
+	},
+	{ /* DSP L1D RAM */
+		.name		= "l1dram",
+		.start		= DA8XX_DSP_L1D_RAM_BASE,
+		.end		= DA8XX_DSP_L1D_RAM_BASE + SZ_32K - 1,
+		.flags		= IORESOURCE_MEM,
+	},
 	{ /* dsp irq */
 		.start		= IRQ_DA8XX_CHIPINT0,
 		.end		= IRQ_DA8XX_CHIPINT0,
diff --git a/arch/arm/mach-davinci/include/mach/da8xx.h b/arch/arm/mach-davinci/include/mach/da8xx.h
index 7e464228948b..93ff1569cee5 100644
--- a/arch/arm/mach-davinci/include/mach/da8xx.h
+++ b/arch/arm/mach-davinci/include/mach/da8xx.h
@@ -75,6 +75,11 @@ extern unsigned int da850_max_speed;
 #define DA8XX_VPIF_BASE		0x01e17000
 #define DA8XX_GPIO_BASE		0x01e26000
 #define DA8XX_PSC1_BASE		0x01e27000
+
+#define DA8XX_DSP_L2_RAM_BASE	0x11800000
+#define DA8XX_DSP_L1P_RAM_BASE	(DA8XX_DSP_L2_RAM_BASE + 0x600000)
+#define DA8XX_DSP_L1D_RAM_BASE	(DA8XX_DSP_L2_RAM_BASE + 0x700000)
+
 #define DA8XX_AEMIF_CS2_BASE	0x60000000
 #define DA8XX_AEMIF_CS3_BASE	0x62000000
 #define DA8XX_AEMIF_CTL_BASE	0x68000000
-- 
2.12.0