[PATCH] mtd: nand: raw: atmel: add module param to avoid using dma

[PATCH] mtd: nand: raw: atmel: add module param to avoid using dma

[Peter Rosin]

On a sama5d31 with a Full-HD dual LVDS panel (132MHz pixel clock) NAND
flash accesses have a tendency to cause display disturbances. Add a
module param to disable DMA from the NAND controller, since that fixes
the display problem for me.

Signed-off-by: Peter Rosin <peda@axentia.se>
---
 drivers/mtd/nand/raw/atmel/nand-controller.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/nand/raw/atmel/nand-controller.c b/drivers/mtd/nand/raw/atmel/nand-controller.c
index b2f00b398490..2ff7a77c7b8e 100644
--- a/drivers/mtd/nand/raw/atmel/nand-controller.c
+++ b/drivers/mtd/nand/raw/atmel/nand-controller.c
@@ -129,6 +129,11 @@
 #define DEFAULT_TIMEOUT_MS			1000
 #define MIN_DMA_LEN				128
 
+static bool atmel_nand_avoid_dma __read_mostly;
+
+MODULE_PARM_DESC(avoiddma, "Avoid using DMA");
+module_param_named(avoiddma, atmel_nand_avoid_dma, bool, 0400);
+
 enum atmel_nand_rb_type {
 	ATMEL_NAND_NO_RB,
 	ATMEL_NAND_NATIVE_RB,
@@ -1977,7 +1982,7 @@ static int atmel_nand_controller_init(struct atmel_nand_controller *nc,
 		return ret;
 	}
 
-	if (nc->caps->has_dma) {
+	if (nc->caps->has_dma && !atmel_nand_avoid_dma) {
 		dma_cap_mask_t mask;
 
 		dma_cap_zero(mask);
-- 
2.11.0

Re: [PATCH] mtd: nand: raw: atmel: add module param to avoid using dma

[Boris Brezillon]

On Thu, 29 Mar 2018 15:10:54 +0200
Peter Rosin <peda@axentia.se> wrote:

> On a sama5d31 with a Full-HD dual LVDS panel (132MHz pixel clock) NAND
> flash accesses have a tendency to cause display disturbances. Add a
> module param to disable DMA from the NAND controller, since that fixes
> the display problem for me.
> 
> Signed-off-by: Peter Rosin <peda@axentia.se>
> ---
>  drivers/mtd/nand/raw/atmel/nand-controller.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/nand/raw/atmel/nand-controller.c b/drivers/mtd/nand/raw/atmel/nand-controller.c
> index b2f00b398490..2ff7a77c7b8e 100644
> --- a/drivers/mtd/nand/raw/atmel/nand-controller.c
> +++ b/drivers/mtd/nand/raw/atmel/nand-controller.c
> @@ -129,6 +129,11 @@
>  #define DEFAULT_TIMEOUT_MS			1000
>  #define MIN_DMA_LEN				128
>  
> +static bool atmel_nand_avoid_dma __read_mostly;
> +
> +MODULE_PARM_DESC(avoiddma, "Avoid using DMA");
> +module_param_named(avoiddma, atmel_nand_avoid_dma, bool, 0400);

I'm not a big fan of those driver specific cmdline parameters. Can't we
instead give an higher priority to HLCDC master using the bus matrix?

> +
>  enum atmel_nand_rb_type {
>  	ATMEL_NAND_NO_RB,
>  	ATMEL_NAND_NATIVE_RB,
> @@ -1977,7 +1982,7 @@ static int atmel_nand_controller_init(struct atmel_nand_controller *nc,
>  		return ret;
>  	}
>  
> -	if (nc->caps->has_dma) {
> +	if (nc->caps->has_dma && !atmel_nand_avoid_dma) {
>  		dma_cap_mask_t mask;
>  
>  		dma_cap_zero(mask);



-- 
Boris Brezillon, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH] mtd: nand: raw: atmel: add module param to avoid using dma

[Peter Rosin]

On 2018-03-29 15:33, Boris Brezillon wrote:
> On Thu, 29 Mar 2018 15:10:54 +0200
> Peter Rosin <peda@axentia.se> wrote:
> 
>> On a sama5d31 with a Full-HD dual LVDS panel (132MHz pixel clock) NAND
>> flash accesses have a tendency to cause display disturbances. Add a
>> module param to disable DMA from the NAND controller, since that fixes
>> the display problem for me.
>>
>> Signed-off-by: Peter Rosin <peda@axentia.se>
>> ---
>>  drivers/mtd/nand/raw/atmel/nand-controller.c | 7 ++++++-
>>  1 file changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/mtd/nand/raw/atmel/nand-controller.c b/drivers/mtd/nand/raw/atmel/nand-controller.c
>> index b2f00b398490..2ff7a77c7b8e 100644
>> --- a/drivers/mtd/nand/raw/atmel/nand-controller.c
>> +++ b/drivers/mtd/nand/raw/atmel/nand-controller.c
>> @@ -129,6 +129,11 @@
>>  #define DEFAULT_TIMEOUT_MS			1000
>>  #define MIN_DMA_LEN				128
>>  
>> +static bool atmel_nand_avoid_dma __read_mostly;
>> +
>> +MODULE_PARM_DESC(avoiddma, "Avoid using DMA");
>> +module_param_named(avoiddma, atmel_nand_avoid_dma, bool, 0400);
> 
> I'm not a big fan of those driver specific cmdline parameters. Can't we
> instead give an higher priority to HLCDC master using the bus matrix?

I don't know if it will be enough, but we sure can try. However, I have
no idea how to do that. I will happily test stuff though...

Cheers,
Peter

Re: [PATCH] mtd: nand: raw: atmel: add module param to avoid using dma

[Boris Brezillon]

On Thu, 29 Mar 2018 15:37:43 +0200
Peter Rosin <peda@axentia.se> wrote:

> On 2018-03-29 15:33, Boris Brezillon wrote:
> > On Thu, 29 Mar 2018 15:10:54 +0200
> > Peter Rosin <peda@axentia.se> wrote:
> >   
> >> On a sama5d31 with a Full-HD dual LVDS panel (132MHz pixel clock) NAND
> >> flash accesses have a tendency to cause display disturbances. Add a
> >> module param to disable DMA from the NAND controller, since that fixes
> >> the display problem for me.
> >>
> >> Signed-off-by: Peter Rosin <peda@axentia.se>
> >> ---
> >>  drivers/mtd/nand/raw/atmel/nand-controller.c | 7 ++++++-
> >>  1 file changed, 6 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/mtd/nand/raw/atmel/nand-controller.c b/drivers/mtd/nand/raw/atmel/nand-controller.c
> >> index b2f00b398490..2ff7a77c7b8e 100644
> >> --- a/drivers/mtd/nand/raw/atmel/nand-controller.c
> >> +++ b/drivers/mtd/nand/raw/atmel/nand-controller.c
> >> @@ -129,6 +129,11 @@
> >>  #define DEFAULT_TIMEOUT_MS			1000
> >>  #define MIN_DMA_LEN				128
> >>  
> >> +static bool atmel_nand_avoid_dma __read_mostly;
> >> +
> >> +MODULE_PARM_DESC(avoiddma, "Avoid using DMA");
> >> +module_param_named(avoiddma, atmel_nand_avoid_dma, bool, 0400);  
> > 
> > I'm not a big fan of those driver specific cmdline parameters. Can't we
> > instead give an higher priority to HLCDC master using the bus matrix?  
> 
> I don't know if it will be enough, but we sure can try. However, I have
> no idea how to do that. I will happily test stuff though...

There's no interface to configure that from Linux, but you can try to
tweak it with devmem and if that does the trick, maybe we can expose a
way to configure that from Linux. For more details, see the "Bus Matrix
(MATRIX)" section in Atmel datasheets.

-- 
Boris Brezillon, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH] mtd: nand: raw: atmel: add module param to avoid using dma

[Peter Rosin]

On 2018-03-29 15:44, Boris Brezillon wrote:
> On Thu, 29 Mar 2018 15:37:43 +0200
> Peter Rosin <peda@axentia.se> wrote:
> 
>> On 2018-03-29 15:33, Boris Brezillon wrote:
>>> On Thu, 29 Mar 2018 15:10:54 +0200
>>> Peter Rosin <peda@axentia.se> wrote:
>>>   
>>>> On a sama5d31 with a Full-HD dual LVDS panel (132MHz pixel clock) NAND
>>>> flash accesses have a tendency to cause display disturbances. Add a
>>>> module param to disable DMA from the NAND controller, since that fixes
>>>> the display problem for me.
>>>>
>>>> Signed-off-by: Peter Rosin <peda@axentia.se>
>>>> ---
>>>>  drivers/mtd/nand/raw/atmel/nand-controller.c | 7 ++++++-
>>>>  1 file changed, 6 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/mtd/nand/raw/atmel/nand-controller.c b/drivers/mtd/nand/raw/atmel/nand-controller.c
>>>> index b2f00b398490..2ff7a77c7b8e 100644
>>>> --- a/drivers/mtd/nand/raw/atmel/nand-controller.c
>>>> +++ b/drivers/mtd/nand/raw/atmel/nand-controller.c
>>>> @@ -129,6 +129,11 @@
>>>>  #define DEFAULT_TIMEOUT_MS			1000
>>>>  #define MIN_DMA_LEN				128
>>>>  
>>>> +static bool atmel_nand_avoid_dma __read_mostly;
>>>> +
>>>> +MODULE_PARM_DESC(avoiddma, "Avoid using DMA");
>>>> +module_param_named(avoiddma, atmel_nand_avoid_dma, bool, 0400);  
>>>
>>> I'm not a big fan of those driver specific cmdline parameters. Can't we
>>> instead give an higher priority to HLCDC master using the bus matrix?  
>>
>> I don't know if it will be enough, but we sure can try. However, I have
>> no idea how to do that. I will happily test stuff though...
> 
> There's no interface to configure that from Linux, but you can try to
> tweak it with devmem and if that does the trick, maybe we can expose a
> way to configure that from Linux. For more details, see the "Bus Matrix
> (MATRIX)" section in Atmel datasheets.

I don't seem to succeed in changing the registers I think I need to change.
I can poke the "Write Protection Mode Register" by writing MAT0 and MAT1 to
it. But when I try to write to "Priority Registers B For Slaves" it doesn't
take, regardless of write protect mode.

Can the relevant bits only be written when the HLCDC is inactive or
something?

Cheers,
Peter

Re: [PATCH] mtd: nand: raw: atmel: add module param to avoid using dma

[Peter Rosin]

On 2018-03-29 16:27, Peter Rosin wrote:
> On 2018-03-29 15:44, Boris Brezillon wrote:
>> On Thu, 29 Mar 2018 15:37:43 +0200
>> Peter Rosin <peda@axentia.se> wrote:
>>
>>> On 2018-03-29 15:33, Boris Brezillon wrote:
>>>> On Thu, 29 Mar 2018 15:10:54 +0200
>>>> Peter Rosin <peda@axentia.se> wrote:
>>>>   
>>>>> On a sama5d31 with a Full-HD dual LVDS panel (132MHz pixel clock) NAND
>>>>> flash accesses have a tendency to cause display disturbances. Add a
>>>>> module param to disable DMA from the NAND controller, since that fixes
>>>>> the display problem for me.
>>>>>
>>>>> Signed-off-by: Peter Rosin <peda@axentia.se>
>>>>> ---
>>>>>  drivers/mtd/nand/raw/atmel/nand-controller.c | 7 ++++++-
>>>>>  1 file changed, 6 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/drivers/mtd/nand/raw/atmel/nand-controller.c b/drivers/mtd/nand/raw/atmel/nand-controller.c
>>>>> index b2f00b398490..2ff7a77c7b8e 100644
>>>>> --- a/drivers/mtd/nand/raw/atmel/nand-controller.c
>>>>> +++ b/drivers/mtd/nand/raw/atmel/nand-controller.c
>>>>> @@ -129,6 +129,11 @@
>>>>>  #define DEFAULT_TIMEOUT_MS			1000
>>>>>  #define MIN_DMA_LEN				128
>>>>>  
>>>>> +static bool atmel_nand_avoid_dma __read_mostly;
>>>>> +
>>>>> +MODULE_PARM_DESC(avoiddma, "Avoid using DMA");
>>>>> +module_param_named(avoiddma, atmel_nand_avoid_dma, bool, 0400);  
>>>>
>>>> I'm not a big fan of those driver specific cmdline parameters. Can't we
>>>> instead give an higher priority to HLCDC master using the bus matrix?  
>>>
>>> I don't know if it will be enough, but we sure can try. However, I have
>>> no idea how to do that. I will happily test stuff though...
>>
>> There's no interface to configure that from Linux, but you can try to
>> tweak it with devmem and if that does the trick, maybe we can expose a
>> way to configure that from Linux. For more details, see the "Bus Matrix
>> (MATRIX)" section in Atmel datasheets.
> 
> I don't seem to succeed in changing the registers I think I need to change.
> I can poke the "Write Protection Mode Register" by writing MAT0 and MAT1 to
> it. But when I try to write to "Priority Registers B For Slaves" it doesn't
> take, regardless of write protect mode.
> 
> Can the relevant bits only be written when the HLCDC is inactive or
> something?

Here is someone else with what seems like very similar problems[1], however
also with USB disturbing the display. I can't test that (easily) since a
mux in front of the USB port was unfortunately wired incorrectly in this
first attempt. And USB disturbing the display is not a showstopper around
here, since USB will only be used for debugging etc in my case.

[1] https://www.avrfreaks.net/forum/sama5d31-ahb-bus-matrix-lcd

Cheers,
Peter

Re: [PATCH] mtd: nand: raw: atmel: add module param to avoid using dma

[Boris Brezillon]

On Thu, 29 Mar 2018 16:27:12 +0200
Peter Rosin <peda@axentia.se> wrote:

> On 2018-03-29 15:44, Boris Brezillon wrote:
> > On Thu, 29 Mar 2018 15:37:43 +0200
> > Peter Rosin <peda@axentia.se> wrote:
> >   
> >> On 2018-03-29 15:33, Boris Brezillon wrote:  
> >>> On Thu, 29 Mar 2018 15:10:54 +0200
> >>> Peter Rosin <peda@axentia.se> wrote:
> >>>     
> >>>> On a sama5d31 with a Full-HD dual LVDS panel (132MHz pixel clock) NAND
> >>>> flash accesses have a tendency to cause display disturbances. Add a
> >>>> module param to disable DMA from the NAND controller, since that fixes
> >>>> the display problem for me.
> >>>>
> >>>> Signed-off-by: Peter Rosin <peda@axentia.se>
> >>>> ---
> >>>>  drivers/mtd/nand/raw/atmel/nand-controller.c | 7 ++++++-
> >>>>  1 file changed, 6 insertions(+), 1 deletion(-)
> >>>>
> >>>> diff --git a/drivers/mtd/nand/raw/atmel/nand-controller.c b/drivers/mtd/nand/raw/atmel/nand-controller.c
> >>>> index b2f00b398490..2ff7a77c7b8e 100644
> >>>> --- a/drivers/mtd/nand/raw/atmel/nand-controller.c
> >>>> +++ b/drivers/mtd/nand/raw/atmel/nand-controller.c
> >>>> @@ -129,6 +129,11 @@
> >>>>  #define DEFAULT_TIMEOUT_MS			1000
> >>>>  #define MIN_DMA_LEN				128
> >>>>  
> >>>> +static bool atmel_nand_avoid_dma __read_mostly;
> >>>> +
> >>>> +MODULE_PARM_DESC(avoiddma, "Avoid using DMA");
> >>>> +module_param_named(avoiddma, atmel_nand_avoid_dma, bool, 0400);    
> >>>
> >>> I'm not a big fan of those driver specific cmdline parameters. Can't we
> >>> instead give an higher priority to HLCDC master using the bus matrix?    
> >>
> >> I don't know if it will be enough, but we sure can try. However, I have
> >> no idea how to do that. I will happily test stuff though...  
> > 
> > There's no interface to configure that from Linux, but you can try to
> > tweak it with devmem and if that does the trick, maybe we can expose a
> > way to configure that from Linux. For more details, see the "Bus Matrix
> > (MATRIX)" section in Atmel datasheets.  
> 
> I don't seem to succeed in changing the registers I think I need to change.
> I can poke the "Write Protection Mode Register" by writing MAT0 and MAT1 to
> it.

You mean 0x4D415400, right? ("MAT0" != 0x4D415400).

> But when I try to write to "Priority Registers B For Slaves" it doesn't
> take, regardless of write protect mode.

Did you check MATRIX_WPSR after writing to MATRIX_PRXSY?

> 
> Can the relevant bits only be written when the HLCDC is inactive or
> something?

I don't know, but maybe Nicolas does.

-- 
Boris Brezillon, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH] mtd: nand: raw: atmel: add module param to avoid using dma

[Peter Rosin]

On 2018-04-02 14:22, Boris Brezillon wrote:
> On Thu, 29 Mar 2018 16:27:12 +0200
> Peter Rosin <peda@axentia.se> wrote:
> 
>> On 2018-03-29 15:44, Boris Brezillon wrote:
>>> On Thu, 29 Mar 2018 15:37:43 +0200
>>> Peter Rosin <peda@axentia.se> wrote:
>>>   
>>>> On 2018-03-29 15:33, Boris Brezillon wrote:  
>>>>> On Thu, 29 Mar 2018 15:10:54 +0200
>>>>> Peter Rosin <peda@axentia.se> wrote:
>>>>>     
>>>>>> On a sama5d31 with a Full-HD dual LVDS panel (132MHz pixel clock) NAND
>>>>>> flash accesses have a tendency to cause display disturbances. Add a
>>>>>> module param to disable DMA from the NAND controller, since that fixes
>>>>>> the display problem for me.
>>>>>>
>>>>>> Signed-off-by: Peter Rosin <peda@axentia.se>
>>>>>> ---
>>>>>>  drivers/mtd/nand/raw/atmel/nand-controller.c | 7 ++++++-
>>>>>>  1 file changed, 6 insertions(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/drivers/mtd/nand/raw/atmel/nand-controller.c b/drivers/mtd/nand/raw/atmel/nand-controller.c
>>>>>> index b2f00b398490..2ff7a77c7b8e 100644
>>>>>> --- a/drivers/mtd/nand/raw/atmel/nand-controller.c
>>>>>> +++ b/drivers/mtd/nand/raw/atmel/nand-controller.c
>>>>>> @@ -129,6 +129,11 @@
>>>>>>  #define DEFAULT_TIMEOUT_MS			1000
>>>>>>  #define MIN_DMA_LEN				128
>>>>>>  
>>>>>> +static bool atmel_nand_avoid_dma __read_mostly;
>>>>>> +
>>>>>> +MODULE_PARM_DESC(avoiddma, "Avoid using DMA");
>>>>>> +module_param_named(avoiddma, atmel_nand_avoid_dma, bool, 0400);    
>>>>>
>>>>> I'm not a big fan of those driver specific cmdline parameters. Can't we
>>>>> instead give an higher priority to HLCDC master using the bus matrix?    
>>>>
>>>> I don't know if it will be enough, but we sure can try. However, I have
>>>> no idea how to do that. I will happily test stuff though...  
>>>
>>> There's no interface to configure that from Linux, but you can try to
>>> tweak it with devmem and if that does the trick, maybe we can expose a
>>> way to configure that from Linux. For more details, see the "Bus Matrix
>>> (MATRIX)" section in Atmel datasheets.  
>>
>> I don't seem to succeed in changing the registers I think I need to change.
>> I can poke the "Write Protection Mode Register" by writing MAT0 and MAT1 to
>> it.
> 
> You mean 0x4D415400, right? ("MAT0" != 0x4D415400).

Bits 1 through 7 do not matter, so even though not equal they are (or
should be) equivalent. But I did use 0x4d415400. I simply used the
shorter syntax since that was easier to type and conveyed the relevant
info.

>> But when I try to write to "Priority Registers B For Slaves" it doesn't
>> take, regardless of write protect mode.
> 
> Did you check MATRIX_WPSR after writing to MATRIX_PRXSY?

No, but did it again and checked, see transcript below. BTW, how do I
know which master is in use for the LCD controller? 8 or 9? Both? And
which DDR slave is the target? 7, 8, 9 or 10? More than one?

Cheers,
Peter

# devmem2 0xffffede4 w
/dev/mem opened.
Memory mapped at address 0xb6f50000.
Value at address 0xFFFFEDE4 (0xb6f50de4): 0x0
# devmem2 0xffffede4 w 0x4d415401
/dev/mem opened.
Memory mapped at address 0xb6f0d000.
Value at address 0xFFFFEDE4 (0xb6f0dde4): 0x0
Written 0x4D415401; readback 0x4D415401
# devmem2 0xffffede4 w
/dev/mem opened.
Memory mapped at address 0xb6f55000.
Value at address 0xFFFFEDE4 (0xb6f55de4): 0x1
# devmem2 0xffffede4 w 0x4d415400
/dev/mem opened.
Memory mapped at address 0xb6fb5000.
Value at address 0xFFFFEDE4 (0xb6fb5de4): 0x1
Written 0x4D415400; readback 0x4D415400
# devmem2 0xffffede4 w
/dev/mem opened.
Memory mapped at address 0xb6fef000.
Value at address 0xFFFFEDE4 (0xb6fefde4): 0x0


# devmem2 0xffffede8 w
/dev/mem opened.
Memory mapped at address 0xb6fe9000.
Value at address 0xFFFFEDE8 (0xb6fe9de8): 0x0


# devmem2 0xffffecbc w
/dev/mem opened.
Memory mapped at address 0xb6ff0000.
Value at address 0xFFFFECBC (0xb6ff0cbc): 0x0
# devmem2 0xffffecbc w 0x33
/dev/mem opened.
Memory mapped at address 0xb6f79000.
Value at address 0xFFFFECBC (0xb6f79cbc): 0x0
Written 0x33; readback 0x33
# devmem2 0xffffecbc w
/dev/mem opened.
Memory mapped at address 0xb6efe000.
Value at address 0xFFFFECBC (0xb6efecbc): 0x0


# devmem2 0xffffede8 w
/dev/mem opened.
Memory mapped at address 0xb6f9e000.
Value at address 0xFFFFEDE8 (0xb6f9ede8): 0x0

Re: [PATCH] mtd: nand: raw: atmel: add module param to avoid using dma

[Boris Brezillon]

On Mon, 2 Apr 2018 19:59:39 +0200
Peter Rosin <peda@axentia.se> wrote:

> On 2018-04-02 14:22, Boris Brezillon wrote:
> > On Thu, 29 Mar 2018 16:27:12 +0200
> > Peter Rosin <peda@axentia.se> wrote:
> >   
> >> On 2018-03-29 15:44, Boris Brezillon wrote:  
> >>> On Thu, 29 Mar 2018 15:37:43 +0200
> >>> Peter Rosin <peda@axentia.se> wrote:
> >>>     
> >>>> On 2018-03-29 15:33, Boris Brezillon wrote:    
> >>>>> On Thu, 29 Mar 2018 15:10:54 +0200
> >>>>> Peter Rosin <peda@axentia.se> wrote:
> >>>>>       
> >>>>>> On a sama5d31 with a Full-HD dual LVDS panel (132MHz pixel clock) NAND
> >>>>>> flash accesses have a tendency to cause display disturbances. Add a
> >>>>>> module param to disable DMA from the NAND controller, since that fixes
> >>>>>> the display problem for me.
> >>>>>>
> >>>>>> Signed-off-by: Peter Rosin <peda@axentia.se>
> >>>>>> ---
> >>>>>>  drivers/mtd/nand/raw/atmel/nand-controller.c | 7 ++++++-
> >>>>>>  1 file changed, 6 insertions(+), 1 deletion(-)
> >>>>>>
> >>>>>> diff --git a/drivers/mtd/nand/raw/atmel/nand-controller.c b/drivers/mtd/nand/raw/atmel/nand-controller.c
> >>>>>> index b2f00b398490..2ff7a77c7b8e 100644
> >>>>>> --- a/drivers/mtd/nand/raw/atmel/nand-controller.c
> >>>>>> +++ b/drivers/mtd/nand/raw/atmel/nand-controller.c
> >>>>>> @@ -129,6 +129,11 @@
> >>>>>>  #define DEFAULT_TIMEOUT_MS			1000
> >>>>>>  #define MIN_DMA_LEN				128
> >>>>>>  
> >>>>>> +static bool atmel_nand_avoid_dma __read_mostly;
> >>>>>> +
> >>>>>> +MODULE_PARM_DESC(avoiddma, "Avoid using DMA");
> >>>>>> +module_param_named(avoiddma, atmel_nand_avoid_dma, bool, 0400);      
> >>>>>
> >>>>> I'm not a big fan of those driver specific cmdline parameters. Can't we
> >>>>> instead give an higher priority to HLCDC master using the bus matrix?      
> >>>>
> >>>> I don't know if it will be enough, but we sure can try. However, I have
> >>>> no idea how to do that. I will happily test stuff though...    
> >>>
> >>> There's no interface to configure that from Linux, but you can try to
> >>> tweak it with devmem and if that does the trick, maybe we can expose a
> >>> way to configure that from Linux. For more details, see the "Bus Matrix
> >>> (MATRIX)" section in Atmel datasheets.    
> >>
> >> I don't seem to succeed in changing the registers I think I need to change.
> >> I can poke the "Write Protection Mode Register" by writing MAT0 and MAT1 to
> >> it.  
> > 
> > You mean 0x4D415400, right? ("MAT0" != 0x4D415400).  
> 
> Bits 1 through 7 do not matter, so even though not equal they are (or
> should be) equivalent. But I did use 0x4d415400. I simply used the
> shorter syntax since that was easier to type and conveyed the relevant
> info.

Ok.

> 
> >> But when I try to write to "Priority Registers B For Slaves" it doesn't
> >> take, regardless of write protect mode.  
> > 
> > Did you check MATRIX_WPSR after writing to MATRIX_PRXSY?  
> 
> No, but did it again and checked, see transcript below.

I don't use devmem2. Is 'readback' information accurate or is it
always what's been written? Because when you write 0x33 to 0xFFFFECBC,
0x33 is read back, but just after that, when you read it again it's 0.

> BTW, how do I
> know which master is in use for the LCD controller? 8 or 9? Both?

It's configurable on a per-layer basis through the SIF bit in
LCDC_<layer>CFG0. The driver tries to dispatch the load on those 2 AHB
masters [1].

> And
> which DDR slave is the target? 7, 8, 9 or 10? More than one?

This, I don't know. I guess all of them can be used.

> 
> Cheers,
> Peter
> 
> # devmem2 0xffffede4 w
> /dev/mem opened.
> Memory mapped at address 0xb6f50000.
> Value at address 0xFFFFEDE4 (0xb6f50de4): 0x0
> # devmem2 0xffffede4 w 0x4d415401
> /dev/mem opened.
> Memory mapped at address 0xb6f0d000.
> Value at address 0xFFFFEDE4 (0xb6f0dde4): 0x0
> Written 0x4D415401; readback 0x4D415401
> # devmem2 0xffffede4 w
> /dev/mem opened.
> Memory mapped at address 0xb6f55000.
> Value at address 0xFFFFEDE4 (0xb6f55de4): 0x1
> # devmem2 0xffffede4 w 0x4d415400
> /dev/mem opened.
> Memory mapped at address 0xb6fb5000.
> Value at address 0xFFFFEDE4 (0xb6fb5de4): 0x1
> Written 0x4D415400; readback 0x4D415400
> # devmem2 0xffffede4 w
> /dev/mem opened.
> Memory mapped at address 0xb6fef000.
> Value at address 0xFFFFEDE4 (0xb6fefde4): 0x0
> 
> 
> # devmem2 0xffffede8 w
> /dev/mem opened.
> Memory mapped at address 0xb6fe9000.
> Value at address 0xFFFFEDE8 (0xb6fe9de8): 0x0
> 
> 
> # devmem2 0xffffecbc w
> /dev/mem opened.
> Memory mapped at address 0xb6ff0000.
> Value at address 0xFFFFECBC (0xb6ff0cbc): 0x0
> # devmem2 0xffffecbc w 0x33
> /dev/mem opened.
> Memory mapped at address 0xb6f79000.
> Value at address 0xFFFFECBC (0xb6f79cbc): 0x0
> Written 0x33; readback 0x33
> # devmem2 0xffffecbc w
> /dev/mem opened.
> Memory mapped at address 0xb6efe000.
> Value at address 0xFFFFECBC (0xb6efecbc): 0x0
> 
> 
> # devmem2 0xffffede8 w
> /dev/mem opened.
> Memory mapped at address 0xb6f9e000.
> Value at address 0xFFFFEDE8 (0xb6f9ede8): 0x0
> 

[1]https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c#L498

-- 
Boris Brezillon, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH] mtd: nand: raw: atmel: add module param to avoid using dma

[Boris Brezillon]

On Mon, 2 Apr 2018 21:28:43 +0200
Boris Brezillon <boris.brezillon@bootlin.com> wrote:

> On Mon, 2 Apr 2018 19:59:39 +0200
> Peter Rosin <peda@axentia.se> wrote:
> 
> > On 2018-04-02 14:22, Boris Brezillon wrote:  
> > > On Thu, 29 Mar 2018 16:27:12 +0200
> > > Peter Rosin <peda@axentia.se> wrote:
> > >     
> > >> On 2018-03-29 15:44, Boris Brezillon wrote:    
> > >>> On Thu, 29 Mar 2018 15:37:43 +0200
> > >>> Peter Rosin <peda@axentia.se> wrote:
> > >>>       
> > >>>> On 2018-03-29 15:33, Boris Brezillon wrote:      
> > >>>>> On Thu, 29 Mar 2018 15:10:54 +0200
> > >>>>> Peter Rosin <peda@axentia.se> wrote:
> > >>>>>         
> > >>>>>> On a sama5d31 with a Full-HD dual LVDS panel (132MHz pixel clock) NAND
> > >>>>>> flash accesses have a tendency to cause display disturbances. Add a
> > >>>>>> module param to disable DMA from the NAND controller, since that fixes
> > >>>>>> the display problem for me.
> > >>>>>>
> > >>>>>> Signed-off-by: Peter Rosin <peda@axentia.se>
> > >>>>>> ---
> > >>>>>>  drivers/mtd/nand/raw/atmel/nand-controller.c | 7 ++++++-
> > >>>>>>  1 file changed, 6 insertions(+), 1 deletion(-)
> > >>>>>>
> > >>>>>> diff --git a/drivers/mtd/nand/raw/atmel/nand-controller.c b/drivers/mtd/nand/raw/atmel/nand-controller.c
> > >>>>>> index b2f00b398490..2ff7a77c7b8e 100644
> > >>>>>> --- a/drivers/mtd/nand/raw/atmel/nand-controller.c
> > >>>>>> +++ b/drivers/mtd/nand/raw/atmel/nand-controller.c
> > >>>>>> @@ -129,6 +129,11 @@
> > >>>>>>  #define DEFAULT_TIMEOUT_MS			1000
> > >>>>>>  #define MIN_DMA_LEN				128
> > >>>>>>  
> > >>>>>> +static bool atmel_nand_avoid_dma __read_mostly;
> > >>>>>> +
> > >>>>>> +MODULE_PARM_DESC(avoiddma, "Avoid using DMA");
> > >>>>>> +module_param_named(avoiddma, atmel_nand_avoid_dma, bool, 0400);        
> > >>>>>
> > >>>>> I'm not a big fan of those driver specific cmdline parameters. Can't we
> > >>>>> instead give an higher priority to HLCDC master using the bus matrix?        
> > >>>>
> > >>>> I don't know if it will be enough, but we sure can try. However, I have
> > >>>> no idea how to do that. I will happily test stuff though...      
> > >>>
> > >>> There's no interface to configure that from Linux, but you can try to
> > >>> tweak it with devmem and if that does the trick, maybe we can expose a
> > >>> way to configure that from Linux. For more details, see the "Bus Matrix
> > >>> (MATRIX)" section in Atmel datasheets.      
> > >>
> > >> I don't seem to succeed in changing the registers I think I need to change.
> > >> I can poke the "Write Protection Mode Register" by writing MAT0 and MAT1 to
> > >> it.    
> > > 
> > > You mean 0x4D415400, right? ("MAT0" != 0x4D415400).    
> > 
> > Bits 1 through 7 do not matter, so even though not equal they are (or
> > should be) equivalent. But I did use 0x4d415400. I simply used the
> > shorter syntax since that was easier to type and conveyed the relevant
> > info.  
> 
> Ok.
> 
> >   
> > >> But when I try to write to "Priority Registers B For Slaves" it doesn't
> > >> take, regardless of write protect mode.    
> > > 
> > > Did you check MATRIX_WPSR after writing to MATRIX_PRXSY?    
> > 
> > No, but did it again and checked, see transcript below.  
> 
> I don't use devmem2. Is 'readback' information accurate or is it
> always what's been written? Because when you write 0x33 to 0xFFFFECBC,
> 0x33 is read back, but just after that, when you read it again it's 0.
> 
> > BTW, how do I
> > know which master is in use for the LCD controller? 8 or 9? Both?  
> 
> It's configurable on a per-layer basis through the SIF bit in
> LCDC_<layer>CFG0. The driver tries to dispatch the load on those 2 AHB
> masters [1].
> 
> > And
> > which DDR slave is the target? 7, 8, 9 or 10? More than one?  
> 
> This, I don't know. I guess all of them can be used.

Looks like I was wrong. According to "Table 15-3. SAMA5D3 Master to
Slave Access", LCDC port 0 can only access DDR port 2 and LCDC port 1
can only access DDR port 3.

Can you try to write 0x3 to 0xFFFFECCC and 0x30 to 0xFFFFECD4?

> 
> > 
> > Cheers,
> > Peter
> > 
> > # devmem2 0xffffede4 w
> > /dev/mem opened.
> > Memory mapped at address 0xb6f50000.
> > Value at address 0xFFFFEDE4 (0xb6f50de4): 0x0
> > # devmem2 0xffffede4 w 0x4d415401
> > /dev/mem opened.
> > Memory mapped at address 0xb6f0d000.
> > Value at address 0xFFFFEDE4 (0xb6f0dde4): 0x0
> > Written 0x4D415401; readback 0x4D415401
> > # devmem2 0xffffede4 w
> > /dev/mem opened.
> > Memory mapped at address 0xb6f55000.
> > Value at address 0xFFFFEDE4 (0xb6f55de4): 0x1
> > # devmem2 0xffffede4 w 0x4d415400
> > /dev/mem opened.
> > Memory mapped at address 0xb6fb5000.
> > Value at address 0xFFFFEDE4 (0xb6fb5de4): 0x1
> > Written 0x4D415400; readback 0x4D415400
> > # devmem2 0xffffede4 w
> > /dev/mem opened.
> > Memory mapped at address 0xb6fef000.
> > Value at address 0xFFFFEDE4 (0xb6fefde4): 0x0
> > 
> > 
> > # devmem2 0xffffede8 w
> > /dev/mem opened.
> > Memory mapped at address 0xb6fe9000.
> > Value at address 0xFFFFEDE8 (0xb6fe9de8): 0x0
> > 
> > 
> > # devmem2 0xffffecbc w
> > /dev/mem opened.
> > Memory mapped at address 0xb6ff0000.
> > Value at address 0xFFFFECBC (0xb6ff0cbc): 0x0
> > # devmem2 0xffffecbc w 0x33
> > /dev/mem opened.
> > Memory mapped at address 0xb6f79000.
> > Value at address 0xFFFFECBC (0xb6f79cbc): 0x0
> > Written 0x33; readback 0x33
> > # devmem2 0xffffecbc w
> > /dev/mem opened.
> > Memory mapped at address 0xb6efe000.
> > Value at address 0xFFFFECBC (0xb6efecbc): 0x0
> > 
> > 
> > # devmem2 0xffffede8 w
> > /dev/mem opened.
> > Memory mapped at address 0xb6f9e000.
> > Value at address 0xFFFFEDE8 (0xb6f9ede8): 0x0
> >   
> 
> [1]https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c#L498
> 



-- 
Boris Brezillon, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH] mtd: nand: raw: atmel: add module param to avoid using dma

[Boris Brezillon]

On Mon, 2 Apr 2018 22:20:20 +0200
Boris Brezillon <boris.brezillon@bootlin.com> wrote:

> >   
> > > And
> > > which DDR slave is the target? 7, 8, 9 or 10? More than one?    
> > 
> > This, I don't know. I guess all of them can be used.  
> 
> Looks like I was wrong. According to "Table 15-3. SAMA5D3 Master to
> Slave Access", LCDC port 0 can only access DDR port 2 and LCDC port 1
> can only access DDR port 3.
> 
> Can you try to write 0x3 to 0xFFFFECCC and 0x30 to 0xFFFFECD4?
> 

Oh, one more thing. Changing the priority won't necessarily solve your
problem because of that:

"
If more than one master requests the slave bus, regardless of the
respective masters priorities, no master will be granted
the slave bus for two consecutive runs. A master can only get
back-to-back grants so long as it is the only requesting
master.
"

To solve that, you'll have to play with MATRIX_MCFGy.ULBT (make sure
DMAC0 and DMAC1 have a small enough ULBT that is not 0) or
MATRIX_SCFGx.SLOT_CYCLE (that one is probably harder to get right
since it's expressed in AHB clock cycles).

-- 
Boris Brezillon, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH] mtd: nand: raw: atmel: add module param to avoid using dma

[Peter Rosin]

On 2018-04-02 22:20, Boris Brezillon wrote:
> On Mon, 2 Apr 2018 21:28:43 +0200
> Boris Brezillon <boris.brezillon@bootlin.com> wrote:
> 
>> On Mon, 2 Apr 2018 19:59:39 +0200
>> Peter Rosin <peda@axentia.se> wrote:
>>
>>> On 2018-04-02 14:22, Boris Brezillon wrote:  
>>>> On Thu, 29 Mar 2018 16:27:12 +0200
>>>> Peter Rosin <peda@axentia.se> wrote:
>>>>     
>>>>> On 2018-03-29 15:44, Boris Brezillon wrote:    
>>>>>> On Thu, 29 Mar 2018 15:37:43 +0200
>>>>>> Peter Rosin <peda@axentia.se> wrote:
>>>>>>       
>>>>>>> On 2018-03-29 15:33, Boris Brezillon wrote:      
>>>>>>>> On Thu, 29 Mar 2018 15:10:54 +0200
>>>>>>>> Peter Rosin <peda@axentia.se> wrote:
>>>>>>>>         
>>>>>>>>> On a sama5d31 with a Full-HD dual LVDS panel (132MHz pixel clock) NAND
>>>>>>>>> flash accesses have a tendency to cause display disturbances. Add a
>>>>>>>>> module param to disable DMA from the NAND controller, since that fixes
>>>>>>>>> the display problem for me.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Peter Rosin <peda@axentia.se>
>>>>>>>>> ---
>>>>>>>>>  drivers/mtd/nand/raw/atmel/nand-controller.c | 7 ++++++-
>>>>>>>>>  1 file changed, 6 insertions(+), 1 deletion(-)
>>>>>>>>>
>>>>>>>>> diff --git a/drivers/mtd/nand/raw/atmel/nand-controller.c b/drivers/mtd/nand/raw/atmel/nand-controller.c
>>>>>>>>> index b2f00b398490..2ff7a77c7b8e 100644
>>>>>>>>> --- a/drivers/mtd/nand/raw/atmel/nand-controller.c
>>>>>>>>> +++ b/drivers/mtd/nand/raw/atmel/nand-controller.c
>>>>>>>>> @@ -129,6 +129,11 @@
>>>>>>>>>  #define DEFAULT_TIMEOUT_MS			1000
>>>>>>>>>  #define MIN_DMA_LEN				128
>>>>>>>>>  
>>>>>>>>> +static bool atmel_nand_avoid_dma __read_mostly;
>>>>>>>>> +
>>>>>>>>> +MODULE_PARM_DESC(avoiddma, "Avoid using DMA");
>>>>>>>>> +module_param_named(avoiddma, atmel_nand_avoid_dma, bool, 0400);        
>>>>>>>>
>>>>>>>> I'm not a big fan of those driver specific cmdline parameters. Can't we
>>>>>>>> instead give an higher priority to HLCDC master using the bus matrix?        
>>>>>>>
>>>>>>> I don't know if it will be enough, but we sure can try. However, I have
>>>>>>> no idea how to do that. I will happily test stuff though...      
>>>>>>
>>>>>> There's no interface to configure that from Linux, but you can try to
>>>>>> tweak it with devmem and if that does the trick, maybe we can expose a
>>>>>> way to configure that from Linux. For more details, see the "Bus Matrix
>>>>>> (MATRIX)" section in Atmel datasheets.      
>>>>>
>>>>> I don't seem to succeed in changing the registers I think I need to change.
>>>>> I can poke the "Write Protection Mode Register" by writing MAT0 and MAT1 to
>>>>> it.    
>>>>
>>>> You mean 0x4D415400, right? ("MAT0" != 0x4D415400).    
>>>
>>> Bits 1 through 7 do not matter, so even though not equal they are (or
>>> should be) equivalent. But I did use 0x4d415400. I simply used the
>>> shorter syntax since that was easier to type and conveyed the relevant
>>> info.  
>>
>> Ok.
>>
>>>   
>>>>> But when I try to write to "Priority Registers B For Slaves" it doesn't
>>>>> take, regardless of write protect mode.    
>>>>
>>>> Did you check MATRIX_WPSR after writing to MATRIX_PRXSY?    
>>>
>>> No, but did it again and checked, see transcript below.  
>>
>> I don't use devmem2. Is 'readback' information accurate or is it
>> always what's been written? Because when you write 0x33 to 0xFFFFECBC,
>> 0x33 is read back, but just after that, when you read it again it's 0.
>>
>>> BTW, how do I
>>> know which master is in use for the LCD controller? 8 or 9? Both?  
>>
>> It's configurable on a per-layer basis through the SIF bit in
>> LCDC_<layer>CFG0. The driver tries to dispatch the load on those 2 AHB
>> masters [1].
>>
>>> And
>>> which DDR slave is the target? 7, 8, 9 or 10? More than one?  
>>
>> This, I don't know. I guess all of them can be used.
> 
> Looks like I was wrong. According to "Table 15-3. SAMA5D3 Master to
> Slave Access", LCDC port 0 can only access DDR port 2 and LCDC port 1
> can only access DDR port 3.

About that table, someone with HW-knowledge should have a real close
look at it! Why?

I peeked at all the PRxSy registers and there are a lot of '3' entries
for all the MxPR fields. In fact, the '3' entries align very neatly
with the checks in this "Master to Slave Access" table. Except they
don't, after a while.

Here's how the table looks in my datasheet:

 0 vv--v--v--vvvv-
 1 vv--v--v--vvvv-
 2 vv-------------
 3 vv--------vvv--
 4 vv-------------
 5 v--------------
 6 vv--vv-vvvvvvvv
   v--------------
 7 v--------------
 8 --v-v--v-------
 9 -v---v--v--v---
10 ---------vv-vvv
11 v--v-----------
12 v-----v--------

And here's the '3' entries when digging in the registers (the extra
dash at the end is for the 16th non-existent slave):

 0 33--3--3--3333--
 1 33--3--3--3333--
 2 33--------------
 3 -3--------333---
 4 33--------------
 5 3---------------
 6 33--33-33333333-
 7 --3-3--3--------
 8 -3---3--3--3----
 9 --3-3--3-33-333-
10 3--3------------
11 3-----3---------
12 ----------------
13 ----------------
14 ----------------
15 ----------------

There's a big mismatch for the four DDR2 lines in the table; they
seem to map to only three registers. Other than that, the only tweak
or anomaly is that first entry (Cortex A5) for master 3 (Int ROM).

*time passes*

Arrrgh!! You say "Table 15-3". This is Table 14-3 for me! I believe
I'm using the latest datasheet (02-Feb-16). What are you reading???!?
Is that something that adds to the confusion?

> Can you try to write 0x3 to 0xFFFFECCC and 0x30 to 0xFFFFECD4?

Will continue experimenting...

Cheers,
Peter

Re: [PATCH] mtd: nand: raw: atmel: add module param to avoid using dma

[Boris Brezillon]

On Tue, 3 Apr 2018 08:11:30 +0200
Peter Rosin <peda@axentia.se> wrote:

> On 2018-04-02 22:20, Boris Brezillon wrote:
> > On Mon, 2 Apr 2018 21:28:43 +0200
> > Boris Brezillon <boris.brezillon@bootlin.com> wrote:
> >   
> >> On Mon, 2 Apr 2018 19:59:39 +0200
> >> Peter Rosin <peda@axentia.se> wrote:
> >>  
> >>> On 2018-04-02 14:22, Boris Brezillon wrote:    
> >>>> On Thu, 29 Mar 2018 16:27:12 +0200
> >>>> Peter Rosin <peda@axentia.se> wrote:
> >>>>       
> >>>>> On 2018-03-29 15:44, Boris Brezillon wrote:      
> >>>>>> On Thu, 29 Mar 2018 15:37:43 +0200
> >>>>>> Peter Rosin <peda@axentia.se> wrote:
> >>>>>>         
> >>>>>>> On 2018-03-29 15:33, Boris Brezillon wrote:        
> >>>>>>>> On Thu, 29 Mar 2018 15:10:54 +0200
> >>>>>>>> Peter Rosin <peda@axentia.se> wrote:
> >>>>>>>>           
> >>>>>>>>> On a sama5d31 with a Full-HD dual LVDS panel (132MHz pixel clock) NAND
> >>>>>>>>> flash accesses have a tendency to cause display disturbances. Add a
> >>>>>>>>> module param to disable DMA from the NAND controller, since that fixes
> >>>>>>>>> the display problem for me.
> >>>>>>>>>
> >>>>>>>>> Signed-off-by: Peter Rosin <peda@axentia.se>
> >>>>>>>>> ---
> >>>>>>>>>  drivers/mtd/nand/raw/atmel/nand-controller.c | 7 ++++++-
> >>>>>>>>>  1 file changed, 6 insertions(+), 1 deletion(-)
> >>>>>>>>>
> >>>>>>>>> diff --git a/drivers/mtd/nand/raw/atmel/nand-controller.c b/drivers/mtd/nand/raw/atmel/nand-controller.c
> >>>>>>>>> index b2f00b398490..2ff7a77c7b8e 100644
> >>>>>>>>> --- a/drivers/mtd/nand/raw/atmel/nand-controller.c
> >>>>>>>>> +++ b/drivers/mtd/nand/raw/atmel/nand-controller.c
> >>>>>>>>> @@ -129,6 +129,11 @@
> >>>>>>>>>  #define DEFAULT_TIMEOUT_MS			1000
> >>>>>>>>>  #define MIN_DMA_LEN				128
> >>>>>>>>>  
> >>>>>>>>> +static bool atmel_nand_avoid_dma __read_mostly;
> >>>>>>>>> +
> >>>>>>>>> +MODULE_PARM_DESC(avoiddma, "Avoid using DMA");
> >>>>>>>>> +module_param_named(avoiddma, atmel_nand_avoid_dma, bool, 0400);          
> >>>>>>>>
> >>>>>>>> I'm not a big fan of those driver specific cmdline parameters. Can't we
> >>>>>>>> instead give an higher priority to HLCDC master using the bus matrix?          
> >>>>>>>
> >>>>>>> I don't know if it will be enough, but we sure can try. However, I have
> >>>>>>> no idea how to do that. I will happily test stuff though...        
> >>>>>>
> >>>>>> There's no interface to configure that from Linux, but you can try to
> >>>>>> tweak it with devmem and if that does the trick, maybe we can expose a
> >>>>>> way to configure that from Linux. For more details, see the "Bus Matrix
> >>>>>> (MATRIX)" section in Atmel datasheets.        
> >>>>>
> >>>>> I don't seem to succeed in changing the registers I think I need to change.
> >>>>> I can poke the "Write Protection Mode Register" by writing MAT0 and MAT1 to
> >>>>> it.      
> >>>>
> >>>> You mean 0x4D415400, right? ("MAT0" != 0x4D415400).      
> >>>
> >>> Bits 1 through 7 do not matter, so even though not equal they are (or
> >>> should be) equivalent. But I did use 0x4d415400. I simply used the
> >>> shorter syntax since that was easier to type and conveyed the relevant
> >>> info.    
> >>
> >> Ok.
> >>  
> >>>     
> >>>>> But when I try to write to "Priority Registers B For Slaves" it doesn't
> >>>>> take, regardless of write protect mode.      
> >>>>
> >>>> Did you check MATRIX_WPSR after writing to MATRIX_PRXSY?      
> >>>
> >>> No, but did it again and checked, see transcript below.    
> >>
> >> I don't use devmem2. Is 'readback' information accurate or is it
> >> always what's been written? Because when you write 0x33 to 0xFFFFECBC,
> >> 0x33 is read back, but just after that, when you read it again it's 0.
> >>  
> >>> BTW, how do I
> >>> know which master is in use for the LCD controller? 8 or 9? Both?    
> >>
> >> It's configurable on a per-layer basis through the SIF bit in
> >> LCDC_<layer>CFG0. The driver tries to dispatch the load on those 2 AHB
> >> masters [1].
> >>  
> >>> And
> >>> which DDR slave is the target? 7, 8, 9 or 10? More than one?    
> >>
> >> This, I don't know. I guess all of them can be used.  
> > 
> > Looks like I was wrong. According to "Table 15-3. SAMA5D3 Master to
> > Slave Access", LCDC port 0 can only access DDR port 2 and LCDC port 1
> > can only access DDR port 3.  
> 
> About that table, someone with HW-knowledge should have a real close
> look at it! Why?
> 
> I peeked at all the PRxSy registers and there are a lot of '3' entries
> for all the MxPR fields. In fact, the '3' entries align very neatly
> with the checks in this "Master to Slave Access" table. Except they
> don't, after a while.
> 
> Here's how the table looks in my datasheet:
> 
>  0 vv--v--v--vvvv-
>  1 vv--v--v--vvvv-
>  2 vv-------------
>  3 vv--------vvv--
>  4 vv-------------
>  5 v--------------
>  6 vv--vv-vvvvvvvv
>    v--------------
>  7 v--------------
>  8 --v-v--v-------
>  9 -v---v--v--v---
> 10 ---------vv-vvv
> 11 v--v-----------
> 12 v-----v--------
> 
> And here's the '3' entries when digging in the registers (the extra
> dash at the end is for the 16th non-existent slave):
> 
>  0 33--3--3--3333--
>  1 33--3--3--3333--
>  2 33--------------
>  3 -3--------333---
>  4 33--------------
>  5 3---------------
>  6 33--33-33333333-
>  7 --3-3--3--------
>  8 -3---3--3--3----
>  9 --3-3--3-33-333-
> 10 3--3------------
> 11 3-----3---------
> 12 ----------------
> 13 ----------------
> 14 ----------------
> 15 ----------------
> 
> There's a big mismatch for the four DDR2 lines in the table; they
> seem to map to only three registers. Other than that, the only tweak
> or anomaly is that first entry (Cortex A5) for master 3 (Int ROM).
> 
> *time passes*
> 
> Arrrgh!! You say "Table 15-3". This is Table 14-3 for me! I believe
> I'm using the latest datasheet (02-Feb-16). What are you reading???!?

Oops, I was reading an old datasheet (from 2014).

> Is that something that adds to the confusion?
> 
> > Can you try to write 0x3 to 0xFFFFECCC and 0x30 to 0xFFFFECD4?  
> 
> Will continue experimenting...
> 
> Cheers,
> Peter



-- 
Boris Brezillon, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH] mtd: nand: raw: atmel: add module param to avoid using dma

[Peter Rosin]

Hi Nicolas,

Boris asked for your input on this (the datasheet difference appears to
have no bearing on the issue) elsewhere in the tree of messages. It's
now been a week or so and I'm starting to wonder if you missed this
altogether or if you are simply out of office or something?

Cheers,
Peter

On 2018-04-03 09:18, Boris Brezillon wrote:
> On Tue, 3 Apr 2018 08:11:30 +0200
> Peter Rosin <peda@axentia.se> wrote:
> 
>> On 2018-04-02 22:20, Boris Brezillon wrote:
>>> On Mon, 2 Apr 2018 21:28:43 +0200
>>> Boris Brezillon <boris.brezillon@bootlin.com> wrote:
>>>   
>>>> On Mon, 2 Apr 2018 19:59:39 +0200
>>>> Peter Rosin <peda@axentia.se> wrote:
>>>>  
>>>>> On 2018-04-02 14:22, Boris Brezillon wrote:    
>>>>>> On Thu, 29 Mar 2018 16:27:12 +0200
>>>>>> Peter Rosin <peda@axentia.se> wrote:
>>>>>>       
>>>>>>> On 2018-03-29 15:44, Boris Brezillon wrote:      
>>>>>>>> On Thu, 29 Mar 2018 15:37:43 +0200
>>>>>>>> Peter Rosin <peda@axentia.se> wrote:
>>>>>>>>         
>>>>>>>>> On 2018-03-29 15:33, Boris Brezillon wrote:        
>>>>>>>>>> On Thu, 29 Mar 2018 15:10:54 +0200
>>>>>>>>>> Peter Rosin <peda@axentia.se> wrote:
>>>>>>>>>>           
>>>>>>>>>>> On a sama5d31 with a Full-HD dual LVDS panel (132MHz pixel clock) NAND
>>>>>>>>>>> flash accesses have a tendency to cause display disturbances. Add a
>>>>>>>>>>> module param to disable DMA from the NAND controller, since that fixes
>>>>>>>>>>> the display problem for me.
>>>>>>>>>>>
>>>>>>>>>>> Signed-off-by: Peter Rosin <peda@axentia.se>
>>>>>>>>>>> ---
>>>>>>>>>>>  drivers/mtd/nand/raw/atmel/nand-controller.c | 7 ++++++-
>>>>>>>>>>>  1 file changed, 6 insertions(+), 1 deletion(-)
>>>>>>>>>>>
>>>>>>>>>>> diff --git a/drivers/mtd/nand/raw/atmel/nand-controller.c b/drivers/mtd/nand/raw/atmel/nand-controller.c
>>>>>>>>>>> index b2f00b398490..2ff7a77c7b8e 100644
>>>>>>>>>>> --- a/drivers/mtd/nand/raw/atmel/nand-controller.c
>>>>>>>>>>> +++ b/drivers/mtd/nand/raw/atmel/nand-controller.c
>>>>>>>>>>> @@ -129,6 +129,11 @@
>>>>>>>>>>>  #define DEFAULT_TIMEOUT_MS			1000
>>>>>>>>>>>  #define MIN_DMA_LEN				128
>>>>>>>>>>>  
>>>>>>>>>>> +static bool atmel_nand_avoid_dma __read_mostly;
>>>>>>>>>>> +
>>>>>>>>>>> +MODULE_PARM_DESC(avoiddma, "Avoid using DMA");
>>>>>>>>>>> +module_param_named(avoiddma, atmel_nand_avoid_dma, bool, 0400);          
>>>>>>>>>>
>>>>>>>>>> I'm not a big fan of those driver specific cmdline parameters. Can't we
>>>>>>>>>> instead give an higher priority to HLCDC master using the bus matrix?          
>>>>>>>>>
>>>>>>>>> I don't know if it will be enough, but we sure can try. However, I have
>>>>>>>>> no idea how to do that. I will happily test stuff though...        
>>>>>>>>
>>>>>>>> There's no interface to configure that from Linux, but you can try to
>>>>>>>> tweak it with devmem and if that does the trick, maybe we can expose a
>>>>>>>> way to configure that from Linux. For more details, see the "Bus Matrix
>>>>>>>> (MATRIX)" section in Atmel datasheets.        
>>>>>>>
>>>>>>> I don't seem to succeed in changing the registers I think I need to change.
>>>>>>> I can poke the "Write Protection Mode Register" by writing MAT0 and MAT1 to
>>>>>>> it.      
>>>>>>
>>>>>> You mean 0x4D415400, right? ("MAT0" != 0x4D415400).      
>>>>>
>>>>> Bits 1 through 7 do not matter, so even though not equal they are (or
>>>>> should be) equivalent. But I did use 0x4d415400. I simply used the
>>>>> shorter syntax since that was easier to type and conveyed the relevant
>>>>> info.    
>>>>
>>>> Ok.
>>>>  
>>>>>     
>>>>>>> But when I try to write to "Priority Registers B For Slaves" it doesn't
>>>>>>> take, regardless of write protect mode.      
>>>>>>
>>>>>> Did you check MATRIX_WPSR after writing to MATRIX_PRXSY?      
>>>>>
>>>>> No, but did it again and checked, see transcript below.    
>>>>
>>>> I don't use devmem2. Is 'readback' information accurate or is it
>>>> always what's been written? Because when you write 0x33 to 0xFFFFECBC,
>>>> 0x33 is read back, but just after that, when you read it again it's 0.
>>>>  
>>>>> BTW, how do I
>>>>> know which master is in use for the LCD controller? 8 or 9? Both?    
>>>>
>>>> It's configurable on a per-layer basis through the SIF bit in
>>>> LCDC_<layer>CFG0. The driver tries to dispatch the load on those 2 AHB
>>>> masters [1].
>>>>  
>>>>> And
>>>>> which DDR slave is the target? 7, 8, 9 or 10? More than one?    
>>>>
>>>> This, I don't know. I guess all of them can be used.  
>>>
>>> Looks like I was wrong. According to "Table 15-3. SAMA5D3 Master to
>>> Slave Access", LCDC port 0 can only access DDR port 2 and LCDC port 1
>>> can only access DDR port 3.  
>>
>> About that table, someone with HW-knowledge should have a real close
>> look at it! Why?
>>
>> I peeked at all the PRxSy registers and there are a lot of '3' entries
>> for all the MxPR fields. In fact, the '3' entries align very neatly
>> with the checks in this "Master to Slave Access" table. Except they
>> don't, after a while.
>>
>> Here's how the table looks in my datasheet:
>>
>>  0 vv--v--v--vvvv-
>>  1 vv--v--v--vvvv-
>>  2 vv-------------
>>  3 vv--------vvv--
>>  4 vv-------------
>>  5 v--------------
>>  6 vv--vv-vvvvvvvv
>>    v--------------
>>  7 v--------------
>>  8 --v-v--v-------
>>  9 -v---v--v--v---
>> 10 ---------vv-vvv
>> 11 v--v-----------
>> 12 v-----v--------
>>
>> And here's the '3' entries when digging in the registers (the extra
>> dash at the end is for the 16th non-existent slave):
>>
>>  0 33--3--3--3333--
>>  1 33--3--3--3333--
>>  2 33--------------
>>  3 -3--------333---
>>  4 33--------------
>>  5 3---------------
>>  6 33--33-33333333-
>>  7 --3-3--3--------
>>  8 -3---3--3--3----
>>  9 --3-3--3-33-333-
>> 10 3--3------------
>> 11 3-----3---------
>> 12 ----------------
>> 13 ----------------
>> 14 ----------------
>> 15 ----------------
>>
>> There's a big mismatch for the four DDR2 lines in the table; they
>> seem to map to only three registers. Other than that, the only tweak
>> or anomaly is that first entry (Cortex A5) for master 3 (Int ROM).
>>
>> *time passes*
>>
>> Arrrgh!! You say "Table 15-3". This is Table 14-3 for me! I believe
>> I'm using the latest datasheet (02-Feb-16). What are you reading???!?
> 
> Oops, I was reading an old datasheet (from 2014).

Re: [PATCH] mtd: nand: raw: atmel: add module param to avoid using dma

[Boris Brezillon]

On Wed, 11 Apr 2018 16:44:10 +0200
Peter Rosin <peda@axentia.se> wrote:

> Hi Nicolas,
> 
> Boris asked for your input on this (the datasheet difference appears to
> have no bearing on the issue) elsewhere in the tree of messages. It's
> now been a week or so and I'm starting to wonder if you missed this
> altogether or if you are simply out of office or something?

I was wondering if you had given up on this problem, it seems you did
not. Did you try forcing the HLCDC to use the 2nd interface (ahb_id=1)
instead of the first one?

Re: [PATCH] mtd: nand: raw: atmel: add module param to avoid using dma

[Peter Rosin]

On 2018-04-11 16:59, Boris Brezillon wrote:
> On Wed, 11 Apr 2018 16:44:10 +0200
> Peter Rosin <peda@axentia.se> wrote:
> 
>> Hi Nicolas,
>>
>> Boris asked for your input on this (the datasheet difference appears to
>> have no bearing on the issue) elsewhere in the tree of messages. It's
>> now been a week or so and I'm starting to wonder if you missed this
>> altogether or if you are simply out of office or something?
> 
> I was wondering if you had given up on this problem, it seems you did
> not.

I have my local patch to disable dma for the flash, but local patches
are always a disappointment.

>      Did you try forcing the HLCDC to use the 2nd interface (ahb_id=1)
> instead of the first one?

Just tried, and it's better that way, but the problem still exist and is
very visible on some (but apparently not all) flash accesses.

Cheers,
Peter

Re: [PATCH] mtd: nand: raw: atmel: add module param to avoid using dma

[Boris Brezillon]

On Wed, 11 Apr 2018 17:10:43 +0200
Peter Rosin <peda@axentia.se> wrote:

> On 2018-04-11 16:59, Boris Brezillon wrote:
> > On Wed, 11 Apr 2018 16:44:10 +0200
> > Peter Rosin <peda@axentia.se> wrote:
> >   
> >> Hi Nicolas,
> >>
> >> Boris asked for your input on this (the datasheet difference appears to
> >> have no bearing on the issue) elsewhere in the tree of messages. It's
> >> now been a week or so and I'm starting to wonder if you missed this
> >> altogether or if you are simply out of office or something?  
> > 
> > I was wondering if you had given up on this problem, it seems you did
> > not.  
> 
> I have my local patch to disable dma for the flash, but local patches
> are always a disappointment.

I understand that.

> 
> >      Did you try forcing the HLCDC to use the 2nd interface (ahb_id=1)
> > instead of the first one?  
> 
> Just tried, and it's better that way, but the problem still exist and is
> very visible on some (but apparently not all) flash accesses.

Then your problems are unlikely to go away even with the priority
adjustments because the DMAC do not use port 3, and priority stuff are
only useful to enforce priority between masters accessing the same
slave.

I guess the real limitation comes the DRAM link, and asking the CPU
to copy data from the NFC SRAM to the the DRAM is probably slowing
things enough to let the HLCDC go through with its data transfers. Or
maybe it has to do with the CPU data caches that are not immediately
flushed to the DRAM when you copy things through the CPU.

Re: [PATCH] mtd: nand: raw: atmel: add module param to avoid using dma

[Nicolas Ferre]

On 11/04/2018 at 16:44, Peter Rosin wrote:
> Hi Nicolas,
> 
> Boris asked for your input on this (the datasheet difference appears to
> have no bearing on the issue) elsewhere in the tree of messages. It's
> now been a week or so and I'm starting to wonder if you missed this
> altogether or if you are simply out of office or something?

Hi Peter,

I didn't dig into this issue with matrix datasheet reset values and your 
findings below. I'll try to move forward with your detailed explanation 
and with my contacts within the "product" team internally.

However, I have the feeling that this sounds a little bit familiar to me 
and that the pins drive strength for the NAND Flash *and* LCD must be 
positioned to "Medium drive" at least for these interfaces (register 
PIO_CFGR).

We use this particular setting for our own vendor branch and found that 
the LCD and NAND Flash was far more sable on *some HW boards*. Here is 
an example for NAND but you can find the same for LCD:
https://github.com/linux4sam/linux-at91/commit/99d9e4c8848a2f16cc5b34bb27e588ca7504b695
Obviously the "drive strength" property added by Ludovic has been 
proposed but is not accepted yet in Mainline and this is why you don't 
see it positioned here.

If it feels like an issue with "crosstalk" it might be the reason why. 
For overruns or underruns, it's true that I would say that it's not 
related and that dealing with the matrix is the way to go.

You can simply test this using devmem2 and see if it's better.

Hope that it helps.
Best regards,
   Nicolas

> On 2018-04-03 09:18, Boris Brezillon wrote:
>> On Tue, 3 Apr 2018 08:11:30 +0200
>> Peter Rosin <peda@axentia.se> wrote:
>>
>>> On 2018-04-02 22:20, Boris Brezillon wrote:
>>>> On Mon, 2 Apr 2018 21:28:43 +0200
>>>> Boris Brezillon <boris.brezillon@bootlin.com> wrote:
>>>>    
>>>>> On Mon, 2 Apr 2018 19:59:39 +0200
>>>>> Peter Rosin <peda@axentia.se> wrote:
>>>>>   
>>>>>> On 2018-04-02 14:22, Boris Brezillon wrote:
>>>>>>> On Thu, 29 Mar 2018 16:27:12 +0200
>>>>>>> Peter Rosin <peda@axentia.se> wrote:
>>>>>>>        
>>>>>>>> On 2018-03-29 15:44, Boris Brezillon wrote:
>>>>>>>>> On Thu, 29 Mar 2018 15:37:43 +0200
>>>>>>>>> Peter Rosin <peda@axentia.se> wrote:
>>>>>>>>>          
>>>>>>>>>> On 2018-03-29 15:33, Boris Brezillon wrote:
>>>>>>>>>>> On Thu, 29 Mar 2018 15:10:54 +0200
>>>>>>>>>>> Peter Rosin <peda@axentia.se> wrote:
>>>>>>>>>>>            
>>>>>>>>>>>> On a sama5d31 with a Full-HD dual LVDS panel (132MHz pixel clock) NAND
>>>>>>>>>>>> flash accesses have a tendency to cause display disturbances. Add a
>>>>>>>>>>>> module param to disable DMA from the NAND controller, since that fixes
>>>>>>>>>>>> the display problem for me.
>>>>>>>>>>>>
>>>>>>>>>>>> Signed-off-by: Peter Rosin <peda@axentia.se>
>>>>>>>>>>>> ---
>>>>>>>>>>>>   drivers/mtd/nand/raw/atmel/nand-controller.c | 7 ++++++-
>>>>>>>>>>>>   1 file changed, 6 insertions(+), 1 deletion(-)
>>>>>>>>>>>>
>>>>>>>>>>>> diff --git a/drivers/mtd/nand/raw/atmel/nand-controller.c b/drivers/mtd/nand/raw/atmel/nand-controller.c
>>>>>>>>>>>> index b2f00b398490..2ff7a77c7b8e 100644
>>>>>>>>>>>> --- a/drivers/mtd/nand/raw/atmel/nand-controller.c
>>>>>>>>>>>> +++ b/drivers/mtd/nand/raw/atmel/nand-controller.c
>>>>>>>>>>>> @@ -129,6 +129,11 @@
>>>>>>>>>>>>   #define DEFAULT_TIMEOUT_MS			1000
>>>>>>>>>>>>   #define MIN_DMA_LEN				128
>>>>>>>>>>>>   
>>>>>>>>>>>> +static bool atmel_nand_avoid_dma __read_mostly;
>>>>>>>>>>>> +
>>>>>>>>>>>> +MODULE_PARM_DESC(avoiddma, "Avoid using DMA");
>>>>>>>>>>>> +module_param_named(avoiddma, atmel_nand_avoid_dma, bool, 0400);
>>>>>>>>>>>
>>>>>>>>>>> I'm not a big fan of those driver specific cmdline parameters. Can't we
>>>>>>>>>>> instead give an higher priority to HLCDC master using the bus matrix?
>>>>>>>>>>
>>>>>>>>>> I don't know if it will be enough, but we sure can try. However, I have
>>>>>>>>>> no idea how to do that. I will happily test stuff though...
>>>>>>>>>
>>>>>>>>> There's no interface to configure that from Linux, but you can try to
>>>>>>>>> tweak it with devmem and if that does the trick, maybe we can expose a
>>>>>>>>> way to configure that from Linux. For more details, see the "Bus Matrix
>>>>>>>>> (MATRIX)" section in Atmel datasheets.
>>>>>>>>
>>>>>>>> I don't seem to succeed in changing the registers I think I need to change.
>>>>>>>> I can poke the "Write Protection Mode Register" by writing MAT0 and MAT1 to
>>>>>>>> it.
>>>>>>>
>>>>>>> You mean 0x4D415400, right? ("MAT0" != 0x4D415400).
>>>>>>
>>>>>> Bits 1 through 7 do not matter, so even though not equal they are (or
>>>>>> should be) equivalent. But I did use 0x4d415400. I simply used the
>>>>>> shorter syntax since that was easier to type and conveyed the relevant
>>>>>> info.
>>>>>
>>>>> Ok.
>>>>>   
>>>>>>      
>>>>>>>> But when I try to write to "Priority Registers B For Slaves" it doesn't
>>>>>>>> take, regardless of write protect mode.
>>>>>>>
>>>>>>> Did you check MATRIX_WPSR after writing to MATRIX_PRXSY?
>>>>>>
>>>>>> No, but did it again and checked, see transcript below.
>>>>>
>>>>> I don't use devmem2. Is 'readback' information accurate or is it
>>>>> always what's been written? Because when you write 0x33 to 0xFFFFECBC,
>>>>> 0x33 is read back, but just after that, when you read it again it's 0.
>>>>>   
>>>>>> BTW, how do I
>>>>>> know which master is in use for the LCD controller? 8 or 9? Both?
>>>>>
>>>>> It's configurable on a per-layer basis through the SIF bit in
>>>>> LCDC_<layer>CFG0. The driver tries to dispatch the load on those 2 AHB
>>>>> masters [1].
>>>>>   
>>>>>> And
>>>>>> which DDR slave is the target? 7, 8, 9 or 10? More than one?
>>>>>
>>>>> This, I don't know. I guess all of them can be used.
>>>>
>>>> Looks like I was wrong. According to "Table 15-3. SAMA5D3 Master to
>>>> Slave Access", LCDC port 0 can only access DDR port 2 and LCDC port 1
>>>> can only access DDR port 3.
>>>
>>> About that table, someone with HW-knowledge should have a real close
>>> look at it! Why?
>>>
>>> I peeked at all the PRxSy registers and there are a lot of '3' entries
>>> for all the MxPR fields. In fact, the '3' entries align very neatly
>>> with the checks in this "Master to Slave Access" table. Except they
>>> don't, after a while.
>>>
>>> Here's how the table looks in my datasheet:
>>>
>>>   0 vv--v--v--vvvv-
>>>   1 vv--v--v--vvvv-
>>>   2 vv-------------
>>>   3 vv--------vvv--
>>>   4 vv-------------
>>>   5 v--------------
>>>   6 vv--vv-vvvvvvvv
>>>     v--------------
>>>   7 v--------------
>>>   8 --v-v--v-------
>>>   9 -v---v--v--v---
>>> 10 ---------vv-vvv
>>> 11 v--v-----------
>>> 12 v-----v--------
>>>
>>> And here's the '3' entries when digging in the registers (the extra
>>> dash at the end is for the 16th non-existent slave):
>>>
>>>   0 33--3--3--3333--
>>>   1 33--3--3--3333--
>>>   2 33--------------
>>>   3 -3--------333---
>>>   4 33--------------
>>>   5 3---------------
>>>   6 33--33-33333333-
>>>   7 --3-3--3--------
>>>   8 -3---3--3--3----
>>>   9 --3-3--3-33-333-
>>> 10 3--3------------
>>> 11 3-----3---------
>>> 12 ----------------
>>> 13 ----------------
>>> 14 ----------------
>>> 15 ----------------
>>>
>>> There's a big mismatch for the four DDR2 lines in the table; they
>>> seem to map to only three registers. Other than that, the only tweak
>>> or anomaly is that first entry (Cortex A5) for master 3 (Int ROM).
>>>
>>> *time passes*
>>>
>>> Arrrgh!! You say "Table 15-3". This is Table 14-3 for me! I believe
>>> I'm using the latest datasheet (02-Feb-16). What are you reading???!?
>>
>> Oops, I was reading an old datasheet (from 2014).
> 


-- 
Nicolas Ferre

Re: [PATCH] mtd: nand: raw: atmel: add module param to avoid using dma

[Peter Rosin]

On 2018-04-11 17:34, Nicolas Ferre wrote:
> On 11/04/2018 at 16:44, Peter Rosin wrote:
>> Hi Nicolas,
>>
>> Boris asked for your input on this (the datasheet difference appears to
>> have no bearing on the issue) elsewhere in the tree of messages. It's
>> now been a week or so and I'm starting to wonder if you missed this
>> altogether or if you are simply out of office or something?
> 
> Hi Peter,
> 
> I didn't dig into this issue with matrix datasheet reset values and your 
> findings below. I'll try to move forward with your detailed explanation 
> and with my contacts within the "product" team internally.

Thanks, I feel that experimenting with the matrix with bogus documentation
is harder than needed, so I'm waiting for that information.

> However, I have the feeling that this sounds a little bit familiar to me 
> and that the pins drive strength for the NAND Flash *and* LCD must be 
> positioned to "Medium drive" at least for these interfaces (register 
> PIO_CFGR).
> 
> We use this particular setting for our own vendor branch and found that 
> the LCD and NAND Flash was far more sable on *some HW boards*. Here is 
> an example for NAND but you can find the same for LCD:
> https://github.com/linux4sam/linux-at91/commit/99d9e4c8848a2f16cc5b34bb27e588ca7504b695
> Obviously the "drive strength" property added by Ludovic has been 
> proposed but is not accepted yet in Mainline and this is why you don't 
> see it positioned here.

Ok, translating this from SAMA5D2 to SAMA5D3 (which is what I have), I
assume this is PIO_DRIVER1 and PIO_DRIVER2 instead. Peeking at those
registers they all contain 0xaaaaaaaa, so I guess all pins are already
"Medium drive" on my board. Also, looking at that sama5d2-ptc-ek patch
it seems possible to adjust the drive strength of the nand D<x> lines,
and I don't think that's possible on the SAMA5D3? The NAND uses D0-D15
on our board, but there is no alternative use for those pins.

So I can change the drive-strength for these LCD pins: LCDDAT0-15 (only
using rgb565), LCDVSYNC, LCDHSYNC, LCDPCK and LCDDEN (LCDDISP is not
used). And for the NAND I can fiddle with NANDALE and NANDCLE.

I.e. PA0-15, PA26-29 and PE21-22

I tried to change the drive strength of these pins to both "Low Drive"
and "High Drive" and it didn't have any visible effect on the display
artifacts during NAND accesses.

> If it feels like an issue with "crosstalk" it might be the reason why. 
> For overruns or underruns, it's true that I would say that it's not 
> related and that dealing with the matrix is the way to go.

It does feel like underruns and that the LCD controller isn't able to
keep up with the needed tempo of the display output. At least that is
consistent with how the artifacts look.

> You can simply test this using devmem2 and see if it's better.

See above.

> Hope that it helps.

Sorry, but no disco. Thanks though!

Cheers,
Peter

> Best regards,
>    Nicolas
> 
>> On 2018-04-03 09:18, Boris Brezillon wrote:
>>> On Tue, 3 Apr 2018 08:11:30 +0200
>>> Peter Rosin <peda@axentia.se> wrote:
>>>
>>>> On 2018-04-02 22:20, Boris Brezillon wrote:
>>>>> On Mon, 2 Apr 2018 21:28:43 +0200
>>>>> Boris Brezillon <boris.brezillon@bootlin.com> wrote:
>>>>>    
>>>>>> On Mon, 2 Apr 2018 19:59:39 +0200
>>>>>> Peter Rosin <peda@axentia.se> wrote:
>>>>>>   
>>>>>>> On 2018-04-02 14:22, Boris Brezillon wrote:
>>>>>>>> On Thu, 29 Mar 2018 16:27:12 +0200
>>>>>>>> Peter Rosin <peda@axentia.se> wrote:
>>>>>>>>        
>>>>>>>>> On 2018-03-29 15:44, Boris Brezillon wrote:
>>>>>>>>>> On Thu, 29 Mar 2018 15:37:43 +0200
>>>>>>>>>> Peter Rosin <peda@axentia.se> wrote:
>>>>>>>>>>          
>>>>>>>>>>> On 2018-03-29 15:33, Boris Brezillon wrote:
>>>>>>>>>>>> On Thu, 29 Mar 2018 15:10:54 +0200
>>>>>>>>>>>> Peter Rosin <peda@axentia.se> wrote:
>>>>>>>>>>>>            
>>>>>>>>>>>>> On a sama5d31 with a Full-HD dual LVDS panel (132MHz pixel clock) NAND
>>>>>>>>>>>>> flash accesses have a tendency to cause display disturbances. Add a
>>>>>>>>>>>>> module param to disable DMA from the NAND controller, since that fixes
>>>>>>>>>>>>> the display problem for me.
>>>>>>>>>>>>>
>>>>>>>>>>>>> Signed-off-by: Peter Rosin <peda@axentia.se>
>>>>>>>>>>>>> ---
>>>>>>>>>>>>>   drivers/mtd/nand/raw/atmel/nand-controller.c | 7 ++++++-
>>>>>>>>>>>>>   1 file changed, 6 insertions(+), 1 deletion(-)
>>>>>>>>>>>>>
>>>>>>>>>>>>> diff --git a/drivers/mtd/nand/raw/atmel/nand-controller.c b/drivers/mtd/nand/raw/atmel/nand-controller.c
>>>>>>>>>>>>> index b2f00b398490..2ff7a77c7b8e 100644
>>>>>>>>>>>>> --- a/drivers/mtd/nand/raw/atmel/nand-controller.c
>>>>>>>>>>>>> +++ b/drivers/mtd/nand/raw/atmel/nand-controller.c
>>>>>>>>>>>>> @@ -129,6 +129,11 @@
>>>>>>>>>>>>>   #define DEFAULT_TIMEOUT_MS			1000
>>>>>>>>>>>>>   #define MIN_DMA_LEN				128
>>>>>>>>>>>>>   
>>>>>>>>>>>>> +static bool atmel_nand_avoid_dma __read_mostly;
>>>>>>>>>>>>> +
>>>>>>>>>>>>> +MODULE_PARM_DESC(avoiddma, "Avoid using DMA");
>>>>>>>>>>>>> +module_param_named(avoiddma, atmel_nand_avoid_dma, bool, 0400);
>>>>>>>>>>>>
>>>>>>>>>>>> I'm not a big fan of those driver specific cmdline parameters. Can't we
>>>>>>>>>>>> instead give an higher priority to HLCDC master using the bus matrix?
>>>>>>>>>>>
>>>>>>>>>>> I don't know if it will be enough, but we sure can try. However, I have
>>>>>>>>>>> no idea how to do that. I will happily test stuff though...
>>>>>>>>>>
>>>>>>>>>> There's no interface to configure that from Linux, but you can try to
>>>>>>>>>> tweak it with devmem and if that does the trick, maybe we can expose a
>>>>>>>>>> way to configure that from Linux. For more details, see the "Bus Matrix
>>>>>>>>>> (MATRIX)" section in Atmel datasheets.
>>>>>>>>>
>>>>>>>>> I don't seem to succeed in changing the registers I think I need to change.
>>>>>>>>> I can poke the "Write Protection Mode Register" by writing MAT0 and MAT1 to
>>>>>>>>> it.
>>>>>>>>
>>>>>>>> You mean 0x4D415400, right? ("MAT0" != 0x4D415400).
>>>>>>>
>>>>>>> Bits 1 through 7 do not matter, so even though not equal they are (or
>>>>>>> should be) equivalent. But I did use 0x4d415400. I simply used the
>>>>>>> shorter syntax since that was easier to type and conveyed the relevant
>>>>>>> info.
>>>>>>
>>>>>> Ok.
>>>>>>   
>>>>>>>      
>>>>>>>>> But when I try to write to "Priority Registers B For Slaves" it doesn't
>>>>>>>>> take, regardless of write protect mode.
>>>>>>>>
>>>>>>>> Did you check MATRIX_WPSR after writing to MATRIX_PRXSY?
>>>>>>>
>>>>>>> No, but did it again and checked, see transcript below.
>>>>>>
>>>>>> I don't use devmem2. Is 'readback' information accurate or is it
>>>>>> always what's been written? Because when you write 0x33 to 0xFFFFECBC,
>>>>>> 0x33 is read back, but just after that, when you read it again it's 0.
>>>>>>   
>>>>>>> BTW, how do I
>>>>>>> know which master is in use for the LCD controller? 8 or 9? Both?
>>>>>>
>>>>>> It's configurable on a per-layer basis through the SIF bit in
>>>>>> LCDC_<layer>CFG0. The driver tries to dispatch the load on those 2 AHB
>>>>>> masters [1].
>>>>>>   
>>>>>>> And
>>>>>>> which DDR slave is the target? 7, 8, 9 or 10? More than one?
>>>>>>
>>>>>> This, I don't know. I guess all of them can be used.
>>>>>
>>>>> Looks like I was wrong. According to "Table 15-3. SAMA5D3 Master to
>>>>> Slave Access", LCDC port 0 can only access DDR port 2 and LCDC port 1
>>>>> can only access DDR port 3.
>>>>
>>>> About that table, someone with HW-knowledge should have a real close
>>>> look at it! Why?
>>>>
>>>> I peeked at all the PRxSy registers and there are a lot of '3' entries
>>>> for all the MxPR fields. In fact, the '3' entries align very neatly
>>>> with the checks in this "Master to Slave Access" table. Except they
>>>> don't, after a while.
>>>>
>>>> Here's how the table looks in my datasheet:
>>>>
>>>>   0 vv--v--v--vvvv-
>>>>   1 vv--v--v--vvvv-
>>>>   2 vv-------------
>>>>   3 vv--------vvv--
>>>>   4 vv-------------
>>>>   5 v--------------
>>>>   6 vv--vv-vvvvvvvv
>>>>     v--------------
>>>>   7 v--------------
>>>>   8 --v-v--v-------
>>>>   9 -v---v--v--v---
>>>> 10 ---------vv-vvv
>>>> 11 v--v-----------
>>>> 12 v-----v--------
>>>>
>>>> And here's the '3' entries when digging in the registers (the extra
>>>> dash at the end is for the 16th non-existent slave):
>>>>
>>>>   0 33--3--3--3333--
>>>>   1 33--3--3--3333--
>>>>   2 33--------------
>>>>   3 -3--------333---
>>>>   4 33--------------
>>>>   5 3---------------
>>>>   6 33--33-33333333-
>>>>   7 --3-3--3--------
>>>>   8 -3---3--3--3----
>>>>   9 --3-3--3-33-333-
>>>> 10 3--3------------
>>>> 11 3-----3---------
>>>> 12 ----------------
>>>> 13 ----------------
>>>> 14 ----------------
>>>> 15 ----------------
>>>>
>>>> There's a big mismatch for the four DDR2 lines in the table; they
>>>> seem to map to only three registers. Other than that, the only tweak
>>>> or anomaly is that first entry (Cortex A5) for master 3 (Int ROM).
>>>>
>>>> *time passes*
>>>>
>>>> Arrrgh!! You say "Table 15-3". This is Table 14-3 for me! I believe
>>>> I'm using the latest datasheet (02-Feb-16). What are you reading???!?
>>>
>>> Oops, I was reading an old datasheet (from 2014).
>>
> 
> 

Re: [PATCH] mtd: nand: raw: atmel: add module param to avoid using dma

[Peter Rosin]

On 2018-04-11 17:34, Nicolas Ferre wrote:
> On 11/04/2018 at 16:44, Peter Rosin wrote:
>> Hi Nicolas,
>>
>> Boris asked for your input on this (the datasheet difference appears to
>> have no bearing on the issue) elsewhere in the tree of messages. It's
>> now been a week or so and I'm starting to wonder if you missed this
>> altogether or if you are simply out of office or something?
> 
> Hi Peter,
> 
> I didn't dig into this issue with matrix datasheet reset values and your 
> findings below. I'll try to move forward with your detailed explanation 
> and with my contacts within the "product" team internally.

Hi again, just checking in to see if there is any progress? If not, maybe
it's time to just take the patch as-is, warts and imperfections included,
and even if we all hate it. But what to do...

Cheers,
Peter

Re: [PATCH] mtd: nand: raw: atmel: add module param to avoid using dma

[Boris Brezillon]

On Tue, 22 May 2018 20:03:41 +0200
Peter Rosin <peda@axentia.se> wrote:

> On 2018-04-11 17:34, Nicolas Ferre wrote:
> > On 11/04/2018 at 16:44, Peter Rosin wrote:  
> >> Hi Nicolas,
> >>
> >> Boris asked for your input on this (the datasheet difference appears to
> >> have no bearing on the issue) elsewhere in the tree of messages. It's
> >> now been a week or so and I'm starting to wonder if you missed this
> >> altogether or if you are simply out of office or something?  
> > 
> > Hi Peter,
> > 
> > I didn't dig into this issue with matrix datasheet reset values and your 
> > findings below. I'll try to move forward with your detailed explanation 
> > and with my contacts within the "product" team internally.  
> 
> Hi again, just checking in to see if there is any progress? If not, maybe
> it's time to just take the patch as-is, warts and imperfections included,
> and even if we all hate it. But what to do...

Well, it's not really that the fix is ugly, but I'm pretty sure it's
actually not fixing the problem, just make it less likely to happen.
And, as soon as you'll have more traffic going through the DRAM
controller you'll experience the same problem again.

Re: [PATCH] mtd: nand: raw: atmel: add module param to avoid using dma

[Tudor Ambarus]

Hi, Peter,

On 04/11/2018 06:34 PM, Nicolas Ferre wrote:
> I'll try to move forward with your detailed explanation and with my 
> contacts within the "product" team internally.

We have talked with the hardware team, looks like there is an error in
the description of the Master to Slave Access matrix. CPU accesses DDR2
port0 through AXI matrix and not AHB. There is no conflict between CPU
and LCDC DMA when accessing DDR2 ports. This explains why using CPU
helps.

The slave numbers from "Table 14-3 Master to Slave Access" are wrong.
The 7th row  should be removed and all the other rows from below it,
shifted up with one level (DDR2 Port 1 is Slave no 7, DDR2 port 2 is
Slave no 8, ... , APB1 is slave no 11).

We think the best way is to keep LCD on DDR Ports 2 and 3 (8th and 9th
slaves), to have maximum bandwidth and to use DMA on DDR port 1 for NAND
(7th slave).

Also, some information about your configuration is useful. Can you
please tell us what NAND DMA configuration did you use?  Are you using
NAND storage for the videos that you are playing on the LCD screen?

Thanks,
ta

Re: [PATCH] mtd: nand: raw: atmel: add module param to avoid using dma

[Peter Rosin]

On 2018-05-25 16:51, Tudor Ambarus wrote:
> Hi, Peter,
> 
> On 04/11/2018 06:34 PM, Nicolas Ferre wrote:
>> I'll try to move forward with your detailed explanation and with my 
>> contacts within the "product" team internally.
> 
> We have talked with the hardware team, looks like there is an error in
> the description of the Master to Slave Access matrix. CPU accesses DDR2
> port0 through AXI matrix and not AHB. There is no conflict between CPU
> and LCDC DMA when accessing DDR2 ports. This explains why using CPU
> helps.
> 
> The slave numbers from "Table 14-3 Master to Slave Access" are wrong.
> The 7th row  should be removed and all the other rows from below it,
> shifted up with one level (DDR2 Port 1 is Slave no 7, DDR2 port 2 is
> Slave no 8, ... , APB1 is slave no 11).

Yes, the fact that row 7 (the 8th row, since there is a row 0, but I get
what you mean) should be removed was pretty much what I had guessed. But
what about the other strange things I found? To reiterate, this is my
PRxSy register content (zeroes shown as -):

PRxS0  33--3--3--3333--
PRxS1  33--3--3--3333--
PRxS2  33--------------
PRxS3  -3--------333---
PRxS4  33--------------
PRxS5  3---------------
PRxS6  33--33-33333333-
PRxS7  --3-3--3--------
PRxS8  -3---3--3--3----
PRxS9  --3-3--3-33-333-
PRxS10 3--3------------
PRxS11 3-----3---------
PRxS12 ----------------
PRxS13 ----------------
PRxS14 ----------------
PRxS15 ----------------

So, PRxS7 matches row 8 (DDR2 port 1) in the datasheet.
And PRxS8 matches row 9 (DDR2 port 2) in the datasheet.
But PRxS9 is not matching row 10 (DDR2 port 3)!
And PRxS10-11 match rows 11-12 (APB 0-1) in the datasheet.

But also look at PRxS3 (Internal ROM), there's a missing 3 in the
first slot (A5)! Maybe the priority of that one can't be changed
for some fundamental reason? But there is no hint to that effect
in the datasheet AFAICT...

> We think the best way is to keep LCD on DDR Ports 2 and 3 (8th and 9th
> slaves), to have maximum bandwidth and to use DMA on DDR port 1 for NAND
> (7th slave).

I think I tried to arrange for that last time around. Not sure though,
so I will try again next week...

> Also, some information about your configuration is useful. Can you
> please tell us what NAND DMA configuration did you use?  Are you using
> NAND storage for the videos that you are playing on the LCD screen?

Not sure what level of detail you are after?

We use W949D2CBJX5E LPDDR1 memories from Winbond. Years ago, I wrote some
LPDDR1 patches for at91bootstrap to driver/ddramc.c (which predate the
current upstream support) and to board/sama5d3xek/sama5d3xek.c (which doesn't
to this day support LPDDR1 upstream, but I guess it shouldn't and that we
should have a board of our own, so that part isn't really clean and also the
reason I never upstreamed it). Anyway, some other trivial glue was also
needed, but the meat are in those files. I just had a brief look at the
LPDDR1 support in the upstream ddramc.c file and our code is very similar.
But I did notice some differences and I will look into if that difference
is something that matters. I can show provide patches if you think they
are relevant?

We use this in the sam-ba .qml file when we flash the devices with sam-ba and
the nandflash applet (don't know if it's relevant, but it shows that we have
a 16-bit bus):

        device: SAMA5D3 {
		name: "sama5d3-linea"

		aliases: []

		description: "SAMA5D3 Linea"

		config {
			nandflash {
				ioset: 1
				busWidth: 16
				header: 0xc0902405
			}
		}
	}

*time passes*

Oh dear ... you said NAND *DMA* configuration. Nothing special that I know
of. So, the default? Anything specific I should check?

Further, we are not playing any video. The artifacts are visible on a
static screen on an otherwise "idle" system (i.e. just showing whatever
on the screen and accessing the DRAM with DMA is enough to trigger the
visual glitches).

Cheers,
Peter

Re: [PATCH] mtd: nand: raw: atmel: add module param to avoid using dma

[Peter Rosin]

On 2018-05-25 16:51, Tudor Ambarus wrote:
> We think the best way is to keep LCD on DDR Ports 2 and 3 (8th and 9th
> slaves), to have maximum bandwidth and to use DMA on DDR port 1 for NAND
> (7th slave).

Exactly how do I accomplish that?

I can see how I can move the LCD between slave DDR port 2 and 3 by
selecting LCDC DMA master 8 or 9 (but according to the above it should
not matter). The big question is how I control what slave the NAND flash
is going to use? I find nothing in the datasheet, and the code is also
non-transparent enough for me to figure it out by myself without
throwing out this question first...

Cheers,
Peter

Re: [PATCH] mtd: nand: raw: atmel: add module param to avoid using dma

[Peter Rosin]

On 2018-05-27 11:18, Peter Rosin wrote:
> On 2018-05-25 16:51, Tudor Ambarus wrote:
>> We think the best way is to keep LCD on DDR Ports 2 and 3 (8th and 9th
>> slaves), to have maximum bandwidth and to use DMA on DDR port 1 for NAND
>> (7th slave).
> 
> Exactly how do I accomplish that?
> 
> I can see how I can move the LCD between slave DDR port 2 and 3 by
> selecting LCDC DMA master 8 or 9 (but according to the above it should
> not matter). The big question is how I control what slave the NAND flash
> is going to use? I find nothing in the datasheet, and the code is also
> non-transparent enough for me to figure it out by myself without
> throwing out this question first...

I added this:

diff --git a/drivers/mtd/nand/raw/atmel/nand-controller.c b/drivers/mtd/nand/raw/atmel/nand-controller.c
index e686fe73159e..3b33c63d2ed4 100644
--- a/drivers/mtd/nand/raw/atmel/nand-controller.c
+++ b/drivers/mtd/nand/raw/atmel/nand-controller.c
@@ -1991,6 +1991,9 @@ static int atmel_nand_controller_init(struct atmel_nand_controller *nc,
 		nc->dmac = dma_request_channel(mask, NULL, NULL);
 		if (!nc->dmac)
 			dev_err(nc->dev, "Failed to request DMA channel\n");
+
+		dev_info(nc->dev, "using %s for DMA transfers\n",
+			 dma_chan_name(nc->dmac));
 	}
 
 	/* We do not retrieve the SMC syscon when parsing old DTs. */



and the output is

atmel-nand-controller 10000000.ebi:nand-controller: using dma0chan5 for DMA transfers

So, DMA controller 0 is in use. I still don't know if IF0, IF1 or IF2 is used
or how to find out. I guess IF2 is not in use since that does not allow any
DDR2 port as slave...

>From the datasheet, DMAC0/IF0 uses DDR2 Port 2, and DMAC0/IF1 uses DDR2 Port 1.
But, by the looks of the register content in my other mail, it seems as if
DMA0/IF1 can also use DDR2 Port 3.

So, I think I want either

A) the NAND controller to use DMAC0/IF0 (i.e. DDR2 port 1, and possibly 3) and
   the LCDC to use master 9 (i.e. DDR2 Port 2)

or

B) the NAND controller to use DMAC1/IF1 (i.e. DDR2 port 2) and the LCDC to use
   master 8 (i.e. DDR2 Port 3)

But, again, how do I limit DMAC0 to either of IF0 or IF1 for NAND accesses?

Note that I have previously tried to move LCDC DMA from master 8 (the default)
to master 9, and it got better, but not good enough. I.e. the visual glitches
remained, but were a little bit harder to trigger. That makes me suspect DMAC0
uses both IF0 and IF1 for its DMAs, but that it prefers IF0.

Cheers,
Peter

Re: [PATCH] mtd: nand: raw: atmel: add module param to avoid using dma

[Peter Rosin]

[-- Attachment #1: Type: text/plain, Size: 5091 bytes --]

On 2018-05-28 00:11, Peter Rosin wrote:
> On 2018-05-27 11:18, Peter Rosin wrote:
>> On 2018-05-25 16:51, Tudor Ambarus wrote:
>>> We think the best way is to keep LCD on DDR Ports 2 and 3 (8th and 9th
>>> slaves), to have maximum bandwidth and to use DMA on DDR port 1 for NAND
>>> (7th slave).
>>
>> Exactly how do I accomplish that?
>>
>> I can see how I can move the LCD between slave DDR port 2 and 3 by
>> selecting LCDC DMA master 8 or 9 (but according to the above it should
>> not matter). The big question is how I control what slave the NAND flash
>> is going to use? I find nothing in the datasheet, and the code is also
>> non-transparent enough for me to figure it out by myself without
>> throwing out this question first...
> 
> I added this:
> 
> diff --git a/drivers/mtd/nand/raw/atmel/nand-controller.c b/drivers/mtd/nand/raw/atmel/nand-controller.c
> index e686fe73159e..3b33c63d2ed4 100644
> --- a/drivers/mtd/nand/raw/atmel/nand-controller.c
> +++ b/drivers/mtd/nand/raw/atmel/nand-controller.c
> @@ -1991,6 +1991,9 @@ static int atmel_nand_controller_init(struct atmel_nand_controller *nc,
>  		nc->dmac = dma_request_channel(mask, NULL, NULL);
>  		if (!nc->dmac)
>  			dev_err(nc->dev, "Failed to request DMA channel\n");
> +
> +		dev_info(nc->dev, "using %s for DMA transfers\n",
> +			 dma_chan_name(nc->dmac));
>  	}
>  
>  	/* We do not retrieve the SMC syscon when parsing old DTs. */
> 
> 
> 
> and the output is
> 
> atmel-nand-controller 10000000.ebi:nand-controller: using dma0chan5 for DMA transfers
> 
> So, DMA controller 0 is in use. I still don't know if IF0, IF1 or IF2 is used
> or how to find out. I guess IF2 is not in use since that does not allow any
> DDR2 port as slave...
> 
> From the datasheet, DMAC0/IF0 uses DDR2 Port 2, and DMAC0/IF1 uses DDR2 Port 1.
> But, by the looks of the register content in my other mail, it seems as if
> DMA0/IF1 can also use DDR2 Port 3.
> 
> So, I think I want either
> 
> A) the NAND controller to use DMAC0/IF0 (i.e. DDR2 port 1, and possibly 3) and
>    the LCDC to use master 9 (i.e. DDR2 Port 2)
> 
> or
> 
> B) the NAND controller to use DMAC1/IF1 (i.e. DDR2 port 2) and the LCDC to use
>    master 8 (i.e. DDR2 Port 3)

Crap, that was not what I meant to express. Sorry for the confusion. This is
better.

So, I think I want either

A) the NAND controller to use master 1 DMAC0/IF0 (i.e. slave 8 DDR2 port 2) and
   the LCDC to use master 9 (i.e. slave 9 DDR2 Port 3)

or

B) the NAND controller to use master 2 DMAC0/IF1 (i.e. slave 7 DDR2 port 1, and
   possibly slave 9 DDR2 port 3 (if my previous findings are relevant) and the
   LCDC to use master 8 (i.e. slave 8 DDR2 Port 2)

> But, again, how do I limit DMAC0 to either of IF0 or IF1 for NAND accesses?

So, I added a horrid patch (attached), which mainly adds printk lines, but
additionally does one more thing in atc_prep_dma_memcpy. It changes the DSCR_IF
field (from 0) to 1 for DMA-memcpy for dma0chan5 (i.e. the NAND). At least I
think it does that?

Running with that patch gets me this:

# dmesg | grep -i dma
at_hdmac ffffe600.dma-controller: Atmel AHB DMA Controller ( cpy set slave ), 8 channels
at_hdmac ffffe800.dma-controller: Atmel AHB DMA Controller ( cpy set slave ), 8 channels
dma dma0chan0: xlate 0 2
dma dma0chan1: xlate 0 2
at91_i2c f0014000.i2c: using dma0chan0 (tx) and dma0chan1 (rx) for DMA transfers
dma dma1chan0: xlate 0 2
dma dma1chan1: xlate 0 2
at91_i2c f801c000.i2c: using dma1chan0 (tx) and dma1chan1 (rx) for DMA transfers
dma dma0chan2: xlate 0 2
dma dma0chan3: xlate 0 2
dma dma0chan4: xlate 0 2
atmel_mci f0000000.mmc: using dma0chan4 for DMA transfers
dma dma1chan2: xlate 0 2
dma dma1chan3: xlate 0 2
atmel_aes f8038000.aes: Atmel AES - Using dma1chan2, dma1chan3 for DMA transfers
dma dma1chan4: xlate 0 2
atmel_sha f8034000.sha: using dma1chan4 for DMA transfers
dma dma1chan5: xlate 0 2
dma dma1chan6: xlate 0 2
atmel_tdes f803c000.tdes: using dma1chan5, dma1chan6 for DMA transfers
atmel-nand-controller 10000000.ebi:nand-controller: using dma0chan5 for DMA transfers
dma dma0chan5: memcpy: 0
dma dma0chan5: DSCR_IF: 1
dma dma0chan5: memcpy: 1

So, output is as expected and I believe that the patch makes the NAND DMA
accesses use master 2 DMAC0/IF1 and are thus forced to use slave 7 DDR2 Port 1
(and possibly 9). The LCDC is using slave 8 DDR2 Port 2. So there should be no
slave conflict?

But the on-screen crap remains during NAND accesses.

But pressing on.

I then changed the priorities for all accesses to 0 in the PRxSy registers, except
the ones for masters 8/9 LCDC (slaves 8/9) which I left at priority 3.

But the on-screen crap remains during NAND accesses.

My guess is that the NAND DMA is doing too long bursts and that the LCDC therefore
has to wait too long and simply fails to keep the pipeline from running short?

So I tried to reduce the maximum SLOT_CYCLE for slaves 7 and 9 in the SCFGx
registers. No noticeable effect either.

I then tried to split bursts from master 2 (DMAC0/IF1) with small values in the
MCFG2 register. No effect.

I'm getting nowhere.

Cheers,
Peter

[-- Attachment #2: horrid-dmac-if1.patch --]
[-- Type: text/plain, Size: 4636 bytes --]

diff --git a/drivers/dma/at_hdmac.c b/drivers/dma/at_hdmac.c
index 75f38d19fcbe..6cb58197bd29 100644
--- a/drivers/dma/at_hdmac.c
+++ b/drivers/dma/at_hdmac.c
@@ -243,6 +243,18 @@ static void atc_dostart(struct at_dma_chan *atchan, struct at_desc *first)
 
 	vdbg_dump_regs(atchan);
 
+	if (atchan->chan_common.chan_id == 5 &&
+	    atchan->chan_common.device->dev_id == 0)
+	{
+		static u32 last_if = 4;
+		u32 this_if = first->txd.phys & 3;
+		if (this_if != last_if) {
+			dev_info(chan2dev(&atchan->chan_common),
+				 "DSCR_IF: %u\n", this_if);
+			last_if = this_if;
+		}
+	}
+
 	channel_writel(atchan, SADDR, 0);
 	channel_writel(atchan, DADDR, 0);
 	channel_writel(atchan, CTRLA, 0);
@@ -854,6 +866,19 @@ atc_prep_dma_memcpy(struct dma_chan *chan, dma_addr_t dest, dma_addr_t src,
 		desc->lli.ctrlb = ctrlb;
 
 		desc->txd.cookie = 0;
+		if (chan->chan_id == 5 &&
+		    chan->device->dev_id == 0)
+		{
+			static u32 last_if = 4;
+			u32 this_if = desc->txd.phys & 3;
+			if (this_if != last_if) {
+				dev_info(chan2dev(chan),
+					 "memcpy: %u\n", this_if);
+				last_if = this_if;
+			}
+			desc->txd.phys = (desc->txd.phys & ~3) | 1;
+		}
+
 		desc->len = xfer_count << src_width;
 
 		atc_desc_chain(&first, &prev, desc);
@@ -1107,6 +1132,8 @@ atc_prep_slave_sg(struct dma_chan *chan, struct scatterlist *sgl,
 			| ATC_SRC_ADDR_MODE_INCR
 			| ATC_FC_MEM2PER
 			| ATC_SIF(atchan->mem_if) | ATC_DIF(atchan->per_if);
+		dev_info(chan2dev(chan), "slave_sg: mem2dev %d %d\n",
+			 atchan->mem_if, atchan->per_if);
 		reg = sconfig->dst_addr;
 		for_each_sg(sgl, sg, sg_len, i) {
 			struct at_desc	*desc;
@@ -1147,6 +1174,8 @@ atc_prep_slave_sg(struct dma_chan *chan, struct scatterlist *sgl,
 			| ATC_SRC_ADDR_MODE_FIXED
 			| ATC_FC_PER2MEM
 			| ATC_SIF(atchan->per_if) | ATC_DIF(atchan->mem_if);
+		dev_info(chan2dev(chan), "slave_sg: dev2mem %d %d\n",
+			 atchan->mem_if, atchan->per_if);
 
 		reg = sconfig->src_addr;
 		for_each_sg(sgl, sg, sg_len, i) {
@@ -1255,6 +1284,8 @@ atc_dma_cyclic_fill_desc(struct dma_chan *chan, struct at_desc *desc,
 				| ATC_FC_MEM2PER
 				| ATC_SIF(atchan->mem_if)
 				| ATC_DIF(atchan->per_if);
+		dev_info(chan2dev(chan), "fill_desc: mem2dev %d %d\n",
+			 atchan->mem_if, atchan->per_if);
 		desc->len = period_len;
 		break;
 
@@ -1267,6 +1298,8 @@ atc_dma_cyclic_fill_desc(struct dma_chan *chan, struct at_desc *desc,
 				| ATC_FC_PER2MEM
 				| ATC_SIF(atchan->per_if)
 				| ATC_DIF(atchan->mem_if);
+		dev_info(chan2dev(chan), "fill_desc: dev2mem %d %d\n",
+			 atchan->mem_if, atchan->per_if);
 		desc->len = period_len;
 		break;
 
@@ -1344,6 +1377,18 @@ atc_prep_dma_cyclic(struct dma_chan *chan, dma_addr_t buf_addr, size_t buf_len,
 		atc_desc_chain(&first, &prev, desc);
 	}
 
+	if (chan->chan_id == 5 &&
+	    chan->device->dev_id == 0)
+	{
+		static u32 last_if = 4;
+		u32 this_if = first->txd.phys & 3;
+		if (this_if != last_if) {
+			dev_info(chan2dev(chan),
+				 "cyclic: %u\n", this_if);
+			last_if = this_if;
+		}
+	}
+
 	/* lets make a cyclic list */
 	prev->lli.dscr = first->txd.phys;
 
@@ -1712,6 +1757,8 @@ static struct dma_chan *at_dma_xlate(struct of_phandle_args *dma_spec,
 	atchan = to_at_dma_chan(chan);
 	atchan->per_if = dma_spec->args[0] & 0xff;
 	atchan->mem_if = (dma_spec->args[0] >> 16) & 0xff;
+	dev_info(chan2dev(chan), "xlate %d %d\n",
+		 atchan->mem_if, atchan->per_if);
 
 	return chan;
 }
@@ -2099,6 +2146,18 @@ static void atc_resume_cyclic(struct at_dma_chan *atchan)
 {
 	struct at_dma	*atdma = to_at_dma(atchan->chan_common.device);
 
+	if (atchan->chan_common.chan_id == 5 &&
+	    atchan->chan_common.device->dev_id == 0)
+	{
+		static u32 last_if = 4;
+		u32 this_if = atchan->save_dscr;
+		if (this_if != last_if) {
+			dev_info(chan2dev(&atchan->chan_common),
+				 "resume_cyclic: %u\n", this_if);
+			last_if = this_if;
+		}
+	}
+
 	/* restore channel status for cyclic descriptors list:
 	 * next descriptor in the cyclic list at the time of suspend */
 	channel_writel(atchan, SADDR, 0);
diff --git a/drivers/mtd/nand/raw/atmel/nand-controller.c b/drivers/mtd/nand/raw/atmel/nand-controller.c
index e686fe73159e..3b33c63d2ed4 100644
--- a/drivers/mtd/nand/raw/atmel/nand-controller.c
+++ b/drivers/mtd/nand/raw/atmel/nand-controller.c
@@ -1991,6 +1991,9 @@ static int atmel_nand_controller_init(struct atmel_nand_controller *nc,
 		nc->dmac = dma_request_channel(mask, NULL, NULL);
 		if (!nc->dmac)
 			dev_err(nc->dev, "Failed to request DMA channel\n");
+
+		dev_info(nc->dev, "using %s for DMA transfers\n",
+			 dma_chan_name(nc->dmac));
 	}
 
 	/* We do not retrieve the SMC syscon when parsing old DTs. */

Re: [PATCH] mtd: nand: raw: atmel: add module param to avoid using dma

[Boris Brezillon]

Hi Peter,

On Mon, 28 May 2018 12:10:02 +0200
Peter Rosin <peda@axentia.se> wrote:

> On 2018-05-28 00:11, Peter Rosin wrote:
> > On 2018-05-27 11:18, Peter Rosin wrote:  
> >> On 2018-05-25 16:51, Tudor Ambarus wrote:  
> >>> We think the best way is to keep LCD on DDR Ports 2 and 3 (8th and 9th
> >>> slaves), to have maximum bandwidth and to use DMA on DDR port 1 for NAND
> >>> (7th slave).  
> >>
> >> Exactly how do I accomplish that?
> >>
> >> I can see how I can move the LCD between slave DDR port 2 and 3 by
> >> selecting LCDC DMA master 8 or 9 (but according to the above it should
> >> not matter). The big question is how I control what slave the NAND flash
> >> is going to use? I find nothing in the datasheet, and the code is also
> >> non-transparent enough for me to figure it out by myself without
> >> throwing out this question first...  
> > 
> > I added this:
> > 
> > diff --git a/drivers/mtd/nand/raw/atmel/nand-controller.c b/drivers/mtd/nand/raw/atmel/nand-controller.c
> > index e686fe73159e..3b33c63d2ed4 100644
> > --- a/drivers/mtd/nand/raw/atmel/nand-controller.c
> > +++ b/drivers/mtd/nand/raw/atmel/nand-controller.c
> > @@ -1991,6 +1991,9 @@ static int atmel_nand_controller_init(struct atmel_nand_controller *nc,
> >  		nc->dmac = dma_request_channel(mask, NULL, NULL);
> >  		if (!nc->dmac)
> >  			dev_err(nc->dev, "Failed to request DMA channel\n");
> > +
> > +		dev_info(nc->dev, "using %s for DMA transfers\n",
> > +			 dma_chan_name(nc->dmac));
> >  	}
> >  
> >  	/* We do not retrieve the SMC syscon when parsing old DTs. */
> > 
> > 
> > 
> > and the output is
> > 
> > atmel-nand-controller 10000000.ebi:nand-controller: using dma0chan5 for DMA transfers
> > 
> > So, DMA controller 0 is in use. I still don't know if IF0, IF1 or IF2 is used
> > or how to find out. I guess IF2 is not in use since that does not allow any
> > DDR2 port as slave...
> > 
> > From the datasheet, DMAC0/IF0 uses DDR2 Port 2, and DMAC0/IF1 uses DDR2 Port 1.
> > But, by the looks of the register content in my other mail, it seems as if
> > DMA0/IF1 can also use DDR2 Port 3.
> > 
> > So, I think I want either
> > 
> > A) the NAND controller to use DMAC0/IF0 (i.e. DDR2 port 1, and possibly 3) and
> >    the LCDC to use master 9 (i.e. DDR2 Port 2)
> > 
> > or
> > 
> > B) the NAND controller to use DMAC1/IF1 (i.e. DDR2 port 2) and the LCDC to use
> >    master 8 (i.e. DDR2 Port 3)  
> 
> Crap, that was not what I meant to express. Sorry for the confusion. This is
> better.
> 
> So, I think I want either
> 
> A) the NAND controller to use master 1 DMAC0/IF0 (i.e. slave 8 DDR2 port 2) and
>    the LCDC to use master 9 (i.e. slave 9 DDR2 Port 3)
> 
> or
> 
> B) the NAND controller to use master 2 DMAC0/IF1 (i.e. slave 7 DDR2 port 1, and
>    possibly slave 9 DDR2 port 3 (if my previous findings are relevant) and the
>    LCDC to use master 8 (i.e. slave 8 DDR2 Port 2)
> 
> > But, again, how do I limit DMAC0 to either of IF0 or IF1 for NAND accesses?  
> 
> So, I added a horrid patch (attached), which mainly adds printk lines, but
> additionally does one more thing in atc_prep_dma_memcpy. It changes the DSCR_IF
> field (from 0) to 1 for DMA-memcpy for dma0chan5 (i.e. the NAND). At least I
> think it does that?
> 
> Running with that patch gets me this:
> 
> # dmesg | grep -i dma
> at_hdmac ffffe600.dma-controller: Atmel AHB DMA Controller ( cpy set slave ), 8 channels
> at_hdmac ffffe800.dma-controller: Atmel AHB DMA Controller ( cpy set slave ), 8 channels
> dma dma0chan0: xlate 0 2
> dma dma0chan1: xlate 0 2
> at91_i2c f0014000.i2c: using dma0chan0 (tx) and dma0chan1 (rx) for DMA transfers
> dma dma1chan0: xlate 0 2
> dma dma1chan1: xlate 0 2
> at91_i2c f801c000.i2c: using dma1chan0 (tx) and dma1chan1 (rx) for DMA transfers
> dma dma0chan2: xlate 0 2
> dma dma0chan3: xlate 0 2
> dma dma0chan4: xlate 0 2
> atmel_mci f0000000.mmc: using dma0chan4 for DMA transfers
> dma dma1chan2: xlate 0 2
> dma dma1chan3: xlate 0 2
> atmel_aes f8038000.aes: Atmel AES - Using dma1chan2, dma1chan3 for DMA transfers
> dma dma1chan4: xlate 0 2
> atmel_sha f8034000.sha: using dma1chan4 for DMA transfers
> dma dma1chan5: xlate 0 2
> dma dma1chan6: xlate 0 2
> atmel_tdes f803c000.tdes: using dma1chan5, dma1chan6 for DMA transfers
> atmel-nand-controller 10000000.ebi:nand-controller: using dma0chan5 for DMA transfers
> dma dma0chan5: memcpy: 0
> dma dma0chan5: DSCR_IF: 1
> dma dma0chan5: memcpy: 1
> 
> So, output is as expected and I believe that the patch makes the NAND DMA
> accesses use master 2 DMAC0/IF1 and are thus forced to use slave 7 DDR2 Port 1
> (and possibly 9). The LCDC is using slave 8 DDR2 Port 2. So there should be no
> slave conflict?
> 
> But the on-screen crap remains during NAND accesses.
> 
> But pressing on.
> 
> I then changed the priorities for all accesses to 0 in the PRxSy registers, except
> the ones for masters 8/9 LCDC (slaves 8/9) which I left at priority 3.
> 
> But the on-screen crap remains during NAND accesses.
> 
> My guess is that the NAND DMA is doing too long bursts and that the LCDC therefore
> has to wait too long and simply fails to keep the pipeline from running short?
> 
> So I tried to reduce the maximum SLOT_CYCLE for slaves 7 and 9 in the SCFGx
> registers. No noticeable effect either.
> 
> I then tried to split bursts from master 2 (DMAC0/IF1) with small values in the
> MCFG2 register. No effect.
> 
> I'm getting nowhere.

Could it just be that you're reaching the DDR bus limit. As I said
previously, when you go through the CPU, and assuming you're consuming
the data directly, you have:

1/ NFC SRAM -> CPU
2/ CPU -> L1 data cache --write-back--> DRAM
3/ L1-cache -> CPU

While, if you use DMA you get:

1/ NFC SRAM -> DRAM
2/ SDRAM -> L1 data cache -> CPU

So, if you're approaching the limit of (LP)DDR bandwidth, using the CPU
might make things a bit better. Still, if the limitation really comes
from the DDR bus, my opinion is that you should maybe use a smaller
resolution or use a more compact pixel format (RGB565?).

Did you calculate how much of the bandwidth is taken by the HLCDC
block and compared it to the max (LP)DDR bandwidth?

Regards,

Boris

Re: [PATCH] mtd: nand: raw: atmel: add module param to avoid using dma

[Peter Rosin]

On 2018-05-28 16:27, Boris Brezillon wrote:
> Hi Peter,
> 
> On Mon, 28 May 2018 12:10:02 +0200
> Peter Rosin <peda@axentia.se> wrote:
> 
>> On 2018-05-28 00:11, Peter Rosin wrote:
>>> On 2018-05-27 11:18, Peter Rosin wrote:  
>>>> On 2018-05-25 16:51, Tudor Ambarus wrote:  
>>>>> We think the best way is to keep LCD on DDR Ports 2 and 3 (8th and 9th
>>>>> slaves), to have maximum bandwidth and to use DMA on DDR port 1 for NAND
>>>>> (7th slave).  
>>>>
>>>> Exactly how do I accomplish that?
>>>>
>>>> I can see how I can move the LCD between slave DDR port 2 and 3 by
>>>> selecting LCDC DMA master 8 or 9 (but according to the above it should
>>>> not matter). The big question is how I control what slave the NAND flash
>>>> is going to use? I find nothing in the datasheet, and the code is also
>>>> non-transparent enough for me to figure it out by myself without
>>>> throwing out this question first...  
>>>
>>> I added this:
>>>
>>> diff --git a/drivers/mtd/nand/raw/atmel/nand-controller.c b/drivers/mtd/nand/raw/atmel/nand-controller.c
>>> index e686fe73159e..3b33c63d2ed4 100644
>>> --- a/drivers/mtd/nand/raw/atmel/nand-controller.c
>>> +++ b/drivers/mtd/nand/raw/atmel/nand-controller.c
>>> @@ -1991,6 +1991,9 @@ static int atmel_nand_controller_init(struct atmel_nand_controller *nc,
>>>  		nc->dmac = dma_request_channel(mask, NULL, NULL);
>>>  		if (!nc->dmac)
>>>  			dev_err(nc->dev, "Failed to request DMA channel\n");
>>> +
>>> +		dev_info(nc->dev, "using %s for DMA transfers\n",
>>> +			 dma_chan_name(nc->dmac));
>>>  	}
>>>  
>>>  	/* We do not retrieve the SMC syscon when parsing old DTs. */
>>>
>>>
>>>
>>> and the output is
>>>
>>> atmel-nand-controller 10000000.ebi:nand-controller: using dma0chan5 for DMA transfers
>>>
>>> So, DMA controller 0 is in use. I still don't know if IF0, IF1 or IF2 is used
>>> or how to find out. I guess IF2 is not in use since that does not allow any
>>> DDR2 port as slave...
>>>
>>> From the datasheet, DMAC0/IF0 uses DDR2 Port 2, and DMAC0/IF1 uses DDR2 Port 1.
>>> But, by the looks of the register content in my other mail, it seems as if
>>> DMA0/IF1 can also use DDR2 Port 3.
>>>
>>> So, I think I want either
>>>
>>> A) the NAND controller to use DMAC0/IF0 (i.e. DDR2 port 1, and possibly 3) and
>>>    the LCDC to use master 9 (i.e. DDR2 Port 2)
>>>
>>> or
>>>
>>> B) the NAND controller to use DMAC1/IF1 (i.e. DDR2 port 2) and the LCDC to use
>>>    master 8 (i.e. DDR2 Port 3)  
>>
>> Crap, that was not what I meant to express. Sorry for the confusion. This is
>> better.
>>
>> So, I think I want either
>>
>> A) the NAND controller to use master 1 DMAC0/IF0 (i.e. slave 8 DDR2 port 2) and
>>    the LCDC to use master 9 (i.e. slave 9 DDR2 Port 3)
>>
>> or
>>
>> B) the NAND controller to use master 2 DMAC0/IF1 (i.e. slave 7 DDR2 port 1, and
>>    possibly slave 9 DDR2 port 3 (if my previous findings are relevant) and the
>>    LCDC to use master 8 (i.e. slave 8 DDR2 Port 2)
>>
>>> But, again, how do I limit DMAC0 to either of IF0 or IF1 for NAND accesses?  
>>
>> So, I added a horrid patch (attached), which mainly adds printk lines, but
>> additionally does one more thing in atc_prep_dma_memcpy. It changes the DSCR_IF
>> field (from 0) to 1 for DMA-memcpy for dma0chan5 (i.e. the NAND). At least I
>> think it does that?
>>
>> Running with that patch gets me this:
>>
>> # dmesg | grep -i dma
>> at_hdmac ffffe600.dma-controller: Atmel AHB DMA Controller ( cpy set slave ), 8 channels
>> at_hdmac ffffe800.dma-controller: Atmel AHB DMA Controller ( cpy set slave ), 8 channels
>> dma dma0chan0: xlate 0 2
>> dma dma0chan1: xlate 0 2
>> at91_i2c f0014000.i2c: using dma0chan0 (tx) and dma0chan1 (rx) for DMA transfers
>> dma dma1chan0: xlate 0 2
>> dma dma1chan1: xlate 0 2
>> at91_i2c f801c000.i2c: using dma1chan0 (tx) and dma1chan1 (rx) for DMA transfers
>> dma dma0chan2: xlate 0 2
>> dma dma0chan3: xlate 0 2
>> dma dma0chan4: xlate 0 2
>> atmel_mci f0000000.mmc: using dma0chan4 for DMA transfers
>> dma dma1chan2: xlate 0 2
>> dma dma1chan3: xlate 0 2
>> atmel_aes f8038000.aes: Atmel AES - Using dma1chan2, dma1chan3 for DMA transfers
>> dma dma1chan4: xlate 0 2
>> atmel_sha f8034000.sha: using dma1chan4 for DMA transfers
>> dma dma1chan5: xlate 0 2
>> dma dma1chan6: xlate 0 2
>> atmel_tdes f803c000.tdes: using dma1chan5, dma1chan6 for DMA transfers
>> atmel-nand-controller 10000000.ebi:nand-controller: using dma0chan5 for DMA transfers
>> dma dma0chan5: memcpy: 0
>> dma dma0chan5: DSCR_IF: 1
>> dma dma0chan5: memcpy: 1
>>
>> So, output is as expected and I believe that the patch makes the NAND DMA
>> accesses use master 2 DMAC0/IF1 and are thus forced to use slave 7 DDR2 Port 1
>> (and possibly 9). The LCDC is using slave 8 DDR2 Port 2. So there should be no
>> slave conflict?
>>
>> But the on-screen crap remains during NAND accesses.
>>
>> But pressing on.
>>
>> I then changed the priorities for all accesses to 0 in the PRxSy registers, except
>> the ones for masters 8/9 LCDC (slaves 8/9) which I left at priority 3.
>>
>> But the on-screen crap remains during NAND accesses.
>>
>> My guess is that the NAND DMA is doing too long bursts and that the LCDC therefore
>> has to wait too long and simply fails to keep the pipeline from running short?
>>
>> So I tried to reduce the maximum SLOT_CYCLE for slaves 7 and 9 in the SCFGx
>> registers. No noticeable effect either.
>>
>> I then tried to split bursts from master 2 (DMAC0/IF1) with small values in the
>> MCFG2 register. No effect.
>>
>> I'm getting nowhere.
> 
> Could it just be that you're reaching the DDR bus limit. As I said
> previously, when you go through the CPU, and assuming you're consuming
> the data directly, you have:
> 
> 1/ NFC SRAM -> CPU
> 2/ CPU -> L1 data cache --write-back--> DRAM
> 3/ L1-cache -> CPU
> 
> While, if you use DMA you get:
> 
> 1/ NFC SRAM -> DRAM
> 2/ SDRAM -> L1 data cache -> CPU
> 
> So, if you're approaching the limit of (LP)DDR bandwidth, using the CPU
> might make things a bit better. Still, if the limitation really comes
> from the DDR bus, my opinion is that you should maybe use a smaller
> resolution or use a more compact pixel format (RGB565?).

The issue is still there if I use a CLUT mode instead of rgb565, which is
what I normally use (and what I would like to use, CLUT is just alien and
a pain these days).

The panels we are using only supports one resolution (each), but the issue
is there with both 1920x1080@16bpp and 1024x768@8bpp (~60Hz).

> Did you calculate how much of the bandwidth is taken by the HLCDC
> block and compared it to the max (LP)DDR bandwidth?

I did, but don't remember the exact details. There is some room even for
1920x1080@16bpp, but not oceans of it. We were a bit uncertain if 16bpp
would be possible, and in fact that was the reason I worked on CLUT
support for atmel-hlcdc last year. But since the problem persists with
much less memory pressure as well, I don't think that's it either.

Admittedly I have not tested these AHB matrix tricks with a smaller
panel (it would take a bit of work to arrange for those tests), but the
issue was there when I last tried (using defaults).

Cheers,
Peter

Re: [PATCH] mtd: nand: raw: atmel: add module param to avoid using dma

[Boris Brezillon]

On Mon, 28 May 2018 17:52:53 +0200
Peter Rosin <peda@axentia.se> wrote:

> On 2018-05-28 16:27, Boris Brezillon wrote:
> > Hi Peter,
> > 
> > On Mon, 28 May 2018 12:10:02 +0200
> > Peter Rosin <peda@axentia.se> wrote:
> >   
> >> On 2018-05-28 00:11, Peter Rosin wrote:  
> >>> On 2018-05-27 11:18, Peter Rosin wrote:    
> >>>> On 2018-05-25 16:51, Tudor Ambarus wrote:    
> >>>>> We think the best way is to keep LCD on DDR Ports 2 and 3 (8th and 9th
> >>>>> slaves), to have maximum bandwidth and to use DMA on DDR port 1 for NAND
> >>>>> (7th slave).    
> >>>>
> >>>> Exactly how do I accomplish that?
> >>>>
> >>>> I can see how I can move the LCD between slave DDR port 2 and 3 by
> >>>> selecting LCDC DMA master 8 or 9 (but according to the above it should
> >>>> not matter). The big question is how I control what slave the NAND flash
> >>>> is going to use? I find nothing in the datasheet, and the code is also
> >>>> non-transparent enough for me to figure it out by myself without
> >>>> throwing out this question first...    
> >>>
> >>> I added this:
> >>>
> >>> diff --git a/drivers/mtd/nand/raw/atmel/nand-controller.c b/drivers/mtd/nand/raw/atmel/nand-controller.c
> >>> index e686fe73159e..3b33c63d2ed4 100644
> >>> --- a/drivers/mtd/nand/raw/atmel/nand-controller.c
> >>> +++ b/drivers/mtd/nand/raw/atmel/nand-controller.c
> >>> @@ -1991,6 +1991,9 @@ static int atmel_nand_controller_init(struct atmel_nand_controller *nc,
> >>>  		nc->dmac = dma_request_channel(mask, NULL, NULL);
> >>>  		if (!nc->dmac)
> >>>  			dev_err(nc->dev, "Failed to request DMA channel\n");
> >>> +
> >>> +		dev_info(nc->dev, "using %s for DMA transfers\n",
> >>> +			 dma_chan_name(nc->dmac));
> >>>  	}
> >>>  
> >>>  	/* We do not retrieve the SMC syscon when parsing old DTs. */
> >>>
> >>>
> >>>
> >>> and the output is
> >>>
> >>> atmel-nand-controller 10000000.ebi:nand-controller: using dma0chan5 for DMA transfers
> >>>
> >>> So, DMA controller 0 is in use. I still don't know if IF0, IF1 or IF2 is used
> >>> or how to find out. I guess IF2 is not in use since that does not allow any
> >>> DDR2 port as slave...
> >>>
> >>> From the datasheet, DMAC0/IF0 uses DDR2 Port 2, and DMAC0/IF1 uses DDR2 Port 1.
> >>> But, by the looks of the register content in my other mail, it seems as if
> >>> DMA0/IF1 can also use DDR2 Port 3.
> >>>
> >>> So, I think I want either
> >>>
> >>> A) the NAND controller to use DMAC0/IF0 (i.e. DDR2 port 1, and possibly 3) and
> >>>    the LCDC to use master 9 (i.e. DDR2 Port 2)
> >>>
> >>> or
> >>>
> >>> B) the NAND controller to use DMAC1/IF1 (i.e. DDR2 port 2) and the LCDC to use
> >>>    master 8 (i.e. DDR2 Port 3)    
> >>
> >> Crap, that was not what I meant to express. Sorry for the confusion. This is
> >> better.
> >>
> >> So, I think I want either
> >>
> >> A) the NAND controller to use master 1 DMAC0/IF0 (i.e. slave 8 DDR2 port 2) and
> >>    the LCDC to use master 9 (i.e. slave 9 DDR2 Port 3)
> >>
> >> or
> >>
> >> B) the NAND controller to use master 2 DMAC0/IF1 (i.e. slave 7 DDR2 port 1, and
> >>    possibly slave 9 DDR2 port 3 (if my previous findings are relevant) and the
> >>    LCDC to use master 8 (i.e. slave 8 DDR2 Port 2)
> >>  
> >>> But, again, how do I limit DMAC0 to either of IF0 or IF1 for NAND accesses?    
> >>
> >> So, I added a horrid patch (attached), which mainly adds printk lines, but
> >> additionally does one more thing in atc_prep_dma_memcpy. It changes the DSCR_IF
> >> field (from 0) to 1 for DMA-memcpy for dma0chan5 (i.e. the NAND). At least I
> >> think it does that?
> >>
> >> Running with that patch gets me this:
> >>
> >> # dmesg | grep -i dma
> >> at_hdmac ffffe600.dma-controller: Atmel AHB DMA Controller ( cpy set slave ), 8 channels
> >> at_hdmac ffffe800.dma-controller: Atmel AHB DMA Controller ( cpy set slave ), 8 channels
> >> dma dma0chan0: xlate 0 2
> >> dma dma0chan1: xlate 0 2
> >> at91_i2c f0014000.i2c: using dma0chan0 (tx) and dma0chan1 (rx) for DMA transfers
> >> dma dma1chan0: xlate 0 2
> >> dma dma1chan1: xlate 0 2
> >> at91_i2c f801c000.i2c: using dma1chan0 (tx) and dma1chan1 (rx) for DMA transfers
> >> dma dma0chan2: xlate 0 2
> >> dma dma0chan3: xlate 0 2
> >> dma dma0chan4: xlate 0 2
> >> atmel_mci f0000000.mmc: using dma0chan4 for DMA transfers
> >> dma dma1chan2: xlate 0 2
> >> dma dma1chan3: xlate 0 2
> >> atmel_aes f8038000.aes: Atmel AES - Using dma1chan2, dma1chan3 for DMA transfers
> >> dma dma1chan4: xlate 0 2
> >> atmel_sha f8034000.sha: using dma1chan4 for DMA transfers
> >> dma dma1chan5: xlate 0 2
> >> dma dma1chan6: xlate 0 2
> >> atmel_tdes f803c000.tdes: using dma1chan5, dma1chan6 for DMA transfers
> >> atmel-nand-controller 10000000.ebi:nand-controller: using dma0chan5 for DMA transfers
> >> dma dma0chan5: memcpy: 0
> >> dma dma0chan5: DSCR_IF: 1
> >> dma dma0chan5: memcpy: 1
> >>
> >> So, output is as expected and I believe that the patch makes the NAND DMA
> >> accesses use master 2 DMAC0/IF1 and are thus forced to use slave 7 DDR2 Port 1
> >> (and possibly 9). The LCDC is using slave 8 DDR2 Port 2. So there should be no
> >> slave conflict?
> >>
> >> But the on-screen crap remains during NAND accesses.
> >>
> >> But pressing on.
> >>
> >> I then changed the priorities for all accesses to 0 in the PRxSy registers, except
> >> the ones for masters 8/9 LCDC (slaves 8/9) which I left at priority 3.
> >>
> >> But the on-screen crap remains during NAND accesses.
> >>
> >> My guess is that the NAND DMA is doing too long bursts and that the LCDC therefore
> >> has to wait too long and simply fails to keep the pipeline from running short?
> >>
> >> So I tried to reduce the maximum SLOT_CYCLE for slaves 7 and 9 in the SCFGx
> >> registers. No noticeable effect either.
> >>
> >> I then tried to split bursts from master 2 (DMAC0/IF1) with small values in the
> >> MCFG2 register. No effect.
> >>
> >> I'm getting nowhere.  
> > 
> > Could it just be that you're reaching the DDR bus limit. As I said
> > previously, when you go through the CPU, and assuming you're consuming
> > the data directly, you have:
> > 
> > 1/ NFC SRAM -> CPU
> > 2/ CPU -> L1 data cache --write-back--> DRAM
> > 3/ L1-cache -> CPU
> > 
> > While, if you use DMA you get:
> > 
> > 1/ NFC SRAM -> DRAM
> > 2/ SDRAM -> L1 data cache -> CPU
> > 
> > So, if you're approaching the limit of (LP)DDR bandwidth, using the CPU
> > might make things a bit better. Still, if the limitation really comes
> > from the DDR bus, my opinion is that you should maybe use a smaller
> > resolution or use a more compact pixel format (RGB565?).  
> 
> The issue is still there if I use a CLUT mode instead of rgb565, which is
> what I normally use (and what I would like to use, CLUT is just alien and
> a pain these days).
> 
> The panels we are using only supports one resolution (each), but the issue
> is there with both 1920x1080@16bpp and 1024x768@8bpp (~60Hz).

Duh! This adds to the weirdness of this issue. I'd thought that by
dividing the required bandwidth by 2 you would get a reliable setup.

> 
> > Did you calculate how much of the bandwidth is taken by the HLCDC
> > block and compared it to the max (LP)DDR bandwidth?  
> 
> I did, but don't remember the exact details. There is some room even for
> 1920x1080@16bpp, but not oceans of it. We were a bit uncertain if 16bpp
> would be possible, and in fact that was the reason I worked on CLUT
> support for atmel-hlcdc last year. But since the problem persists with
> much less memory pressure as well, I don't think that's it either.
> 
> Admittedly I have not tested these AHB matrix tricks with a smaller
> panel (it would take a bit of work to arrange for those tests), but the
> issue was there when I last tried (using defaults).

Okay. I think I'll take your initial patch, but I'd really like to
understand the root cause of this problem. Tudor, any idea why the
various stuff Peter tried did not work?

Re: [PATCH] mtd: nand: raw: atmel: add module param to avoid using dma

[Nicolas Ferre]

On 28/05/2018 at 17:52, Peter Rosin wrote:
> On 2018-05-28 16:27, Boris Brezillon wrote:

[..]

>> Could it just be that you're reaching the DDR bus limit. As I said
>> previously, when you go through the CPU, and assuming you're consuming
>> the data directly, you have:
>>
>> 1/ NFC SRAM -> CPU
>> 2/ CPU -> L1 data cache --write-back--> DRAM
>> 3/ L1-cache -> CPU
>>
>> While, if you use DMA you get:
>>
>> 1/ NFC SRAM -> DRAM
>> 2/ SDRAM -> L1 data cache -> CPU
>>
>> So, if you're approaching the limit of (LP)DDR bandwidth, using the CPU
>> might make things a bit better. Still, if the limitation really comes
>> from the DDR bus, my opinion is that you should maybe use a smaller
>> resolution or use a more compact pixel format (RGB565?).
> 
> The issue is still there if I use a CLUT mode instead of rgb565, which is
> what I normally use (and what I would like to use, CLUT is just alien and
> a pain these days).
> 
> The panels we are using only supports one resolution (each), but the issue
> is there with both 1920x1080@16bpp and 1024x768@8bpp (~60Hz).
> 
>> Did you calculate how much of the bandwidth is taken by the HLCDC
>> block and compared it to the max (LP)DDR bandwidth?
> 
> I did, but don't remember the exact details. There is some room even for
> 1920x1080@16bpp, but not oceans of it. We were a bit uncertain if 16bpp
> would be possible, and in fact that was the reason I worked on CLUT
> support for atmel-hlcdc last year. But since the problem persists with
> much less memory pressure as well, I don't think that's it either.

Just jumping in the conversation with another perspective (maybe already 
tried actually).

Can you try to make all that you can to maximize the blanking period of 
your screen (some are more tolerant than others according to that). By 
doing so, you would allow the LCD FIFO to recover better after each 
line. You might loose some columns on the side of your display but it 
would give us a good idea of how far we are from getting rid of those 
annoying LCD reset glitches (that are due to underruns on LCD FIFO).

> Admittedly I have not tested these AHB matrix tricks with a smaller
> panel (it would take a bit of work to arrange for those tests), but the
> issue was there when I last tried (using defaults).

If what I said earlier has an impact, reducing the panel size will also 
make a difference. But the question is how small is "smaller" ;-)

Best regards,
-- 
Nicolas Ferre

Re: [PATCH] mtd: nand: raw: atmel: add module param to avoid using dma

[Eugen Hristev]

On 28.05.2018 13:10, Peter Rosin wrote:
> On 2018-05-28 00:11, Peter Rosin wrote:
>> On 2018-05-27 11:18, Peter Rosin wrote:
>>> On 2018-05-25 16:51, Tudor Ambarus wrote:
>>>> We think the best way is to keep LCD on DDR Ports 2 and 3 (8th and 9th
>>>> slaves), to have maximum bandwidth and to use DMA on DDR port 1 for NAND
>>>> (7th slave).
>>>
>>> Exactly how do I accomplish that?
>>>
>>> I can see how I can move the LCD between slave DDR port 2 and 3 by
>>> selecting LCDC DMA master 8 or 9 (but according to the above it should
>>> not matter). The big question is how I control what slave the NAND flash
>>> is going to use? I find nothing in the datasheet, and the code is also
>>> non-transparent enough for me to figure it out by myself without
>>> throwing out this question first...

 >> [...]

>> and the output is
>>
>> atmel-nand-controller 10000000.ebi:nand-controller: using dma0chan5 for DMA transfers
>>
>> So, DMA controller 0 is in use. I still don't know if IF0, IF1 or IF2 is used
>> or how to find out. I guess IF2 is not in use since that does not allow any
>> DDR2 port as slave...

Hello Peter,

Thank you for all the information, I will chip in to help a little bit.
The Master/channel is described in the device tree. The channel has a 
controller, a mem/periph interface and a periph ID, plus a FIFO 
configuration.

The dma chan number reported in the dmesg is just software. Here is an 
example from DT:
dmas = <&dma0 2 AT91_DMA_CFG_PER_ID(1)>,
        <&dma0 2 AT91_DMA_CFG_PER_ID(2)>;

you can match this with the help from 
Documentation/devicetree/bindings/dma/atmel-dma.txt:

1. A phandle pointing to the DMA controller. 

2. The memory interface (16 most significant bits), the peripheral 
interface
(16 less significant bits). 

3. Parameters for the at91 DMA configuration register which are device 

dependent: 

   - bit 7-0: peripheral identifier for the hardware handshaking 
interface. The
   identifier can be different for tx and rx. 

   - bit 11-8: FIFO configuration. 0 for half FIFO, 1 for ALAP, 2 for ASAP.


So , what was Tudor asking for, is your DT for the ebi node (if you are 
using ebi), or, your NFC SRAM (Nand Flash Controller SRAM) DMA 
devicetree chunk, so, we can figure out which type of DMA are you using.

Normally, the ebi should be connected to both DMA0 and DMA1 on those 
interfaces specified in DT. Which ones you want to use, depends on your 
setup (and contention on the bus/accesses, like in your case, the HLCDC)

Thats why we have multiple choices, to pick the right one for each case.
In our vanilla DT sama5d3.dtsi we do not have DMA described for ebi 
interface.

Eugen

 >> [...]

Re: [PATCH] mtd: nand: raw: atmel: add module param to avoid using dma

[Peter Rosin]

On 2018-05-29 08:30, Eugen Hristev wrote:
> 
> 
> On 28.05.2018 13:10, Peter Rosin wrote:
>> On 2018-05-28 00:11, Peter Rosin wrote:
>>> On 2018-05-27 11:18, Peter Rosin wrote:
>>>> On 2018-05-25 16:51, Tudor Ambarus wrote:
>>>>> We think the best way is to keep LCD on DDR Ports 2 and 3 (8th and 9th
>>>>> slaves), to have maximum bandwidth and to use DMA on DDR port 1 for NAND
>>>>> (7th slave).
>>>>
>>>> Exactly how do I accomplish that?
>>>>
>>>> I can see how I can move the LCD between slave DDR port 2 and 3 by
>>>> selecting LCDC DMA master 8 or 9 (but according to the above it should
>>>> not matter). The big question is how I control what slave the NAND flash
>>>> is going to use? I find nothing in the datasheet, and the code is also
>>>> non-transparent enough for me to figure it out by myself without
>>>> throwing out this question first...
> 
>  >> [...]
> 
>>> and the output is
>>>
>>> atmel-nand-controller 10000000.ebi:nand-controller: using dma0chan5 for DMA transfers
>>>
>>> So, DMA controller 0 is in use. I still don't know if IF0, IF1 or IF2 is used
>>> or how to find out. I guess IF2 is not in use since that does not allow any
>>> DDR2 port as slave...
> 
> Hello Peter,
> 
> Thank you for all the information, I will chip in to help a little bit.
> The Master/channel is described in the device tree. The channel has a 
> controller, a mem/periph interface and a periph ID, plus a FIFO 
> configuration.
> 
> The dma chan number reported in the dmesg is just software.

Got that, that was why I added the various additional traces in that
horrid patch in the mail you are responding to :-)

>                                                             Here is an 
> example from DT:
> dmas = <&dma0 2 AT91_DMA_CFG_PER_ID(1)>,
>         <&dma0 2 AT91_DMA_CFG_PER_ID(2)>;
> 
> you can match this with the help from 
> Documentation/devicetree/bindings/dma/atmel-dma.txt:
> 
> 1. A phandle pointing to the DMA controller. 
> 
> 2. The memory interface (16 most significant bits), the peripheral 
> interface
> (16 less significant bits). 
> 
> 3. Parameters for the at91 DMA configuration register which are device 
> 
> dependent: 
> 
>    - bit 7-0: peripheral identifier for the hardware handshaking 
> interface. The
>    identifier can be different for tx and rx. 
> 
>    - bit 11-8: FIFO configuration. 0 for half FIFO, 1 for ALAP, 2 for ASAP.
> 
> 
> So , what was Tudor asking for, is your DT for the ebi node (if you are 
> using ebi), or, your NFC SRAM (Nand Flash Controller SRAM) DMA 
> devicetree chunk, so, we can figure out which type of DMA are you using.
> 
> Normally, the ebi should be connected to both DMA0 and DMA1 on those 
> interfaces specified in DT. Which ones you want to use, depends on your 
> setup (and contention on the bus/accesses, like in your case, the HLCDC)
> 
> Thats why we have multiple choices, to pick the right one for each case.
> In our vanilla DT sama5d3.dtsi we do not have DMA described for ebi 
> interface.

Ahh, *that* NAND DMA configuration. Of course, how silly of me...

This is a setup based on the at91-linea.dtsi "CPU" board. That dtsi should
have the relevant NAND/DMA info. Also, at91-nattis-2-natte-2.dts describes
the older HW (with a 1024x768 panel) that is also affected, if you want a
full device tree to look at.

Looks like I didn't make a selection, quoting from at91-linea.dtsi:

&ebi {
	pinctrl-0 = <&pinctrl_ebi_nand_addr>;
	pinctrl-names = "default";
	status = "okay";
};

&nand_controller {
	status = "okay";

	nand: nand@3 {
		reg = <0x3 0x0 0x2>;
		atmel,rb = <0>;
		nand-bus-width = <8>;
		nand-ecc-mode = "hw";
		nand-ecc-strength = <4>;
		nand-ecc-step-size = <512>;
		nand-on-flash-bbt;
		label = "atmel_nand";
	};
};

The reason is probably because the sama5d3xek device-trees didn't at the
time of "fork". Does anybody have any suggestion for some extra properties
to try in the above nodes?

Further, I forgot that I had actually upstreamed linea support for
at91bootstrap, so relevant NAND timings etc can be found in lpddr1_init()
in

	at91bootstrap/contrib/board/axentia/sama5d3_linea/sama5d3_linea.c

Cheers,
Peter

Re: [PATCH] mtd: nand: raw: atmel: add module param to avoid using dma

[Eugen Hristev]

On 29.05.2018 10:10, Peter Rosin wrote:
> On 2018-05-29 08:30, Eugen Hristev wrote:
>>
>>
>> On 28.05.2018 13:10, Peter Rosin wrote:
>>> On 2018-05-28 00:11, Peter Rosin wrote:
>>>> On 2018-05-27 11:18, Peter Rosin wrote:
>>>>> On 2018-05-25 16:51, Tudor Ambarus wrote:
>>>>>> We think the best way is to keep LCD on DDR Ports 2 and 3 (8th and 9th
>>>>>> slaves), to have maximum bandwidth and to use DMA on DDR port 1 for NAND
>>>>>> (7th slave).
>>>>>
>>>>> Exactly how do I accomplish that?
>>>>>
>>>>> I can see how I can move the LCD between slave DDR port 2 and 3 by
>>>>> selecting LCDC DMA master 8 or 9 (but according to the above it should
>>>>> not matter). The big question is how I control what slave the NAND flash
>>>>> is going to use? I find nothing in the datasheet, and the code is also
>>>>> non-transparent enough for me to figure it out by myself without
>>>>> throwing out this question first...
>>
>>   >> [...]
>>
>>>> and the output is
>>>>
>>>> atmel-nand-controller 10000000.ebi:nand-controller: using dma0chan5 for DMA transfers
>>>>
>>>> So, DMA controller 0 is in use. I still don't know if IF0, IF1 or IF2 is used
>>>> or how to find out. I guess IF2 is not in use since that does not allow any
>>>> DDR2 port as slave...
>>
>> Hello Peter,
>>
>> Thank you for all the information, I will chip in to help a little bit.
>> The Master/channel is described in the device tree. The channel has a
>> controller, a mem/periph interface and a periph ID, plus a FIFO
>> configuration.
>>
>> The dma chan number reported in the dmesg is just software.
> 
> Got that, that was why I added the various additional traces in that
> horrid patch in the mail you are responding to :-)
> 
>>                                                              Here is an
>> example from DT:
>> dmas = <&dma0 2 AT91_DMA_CFG_PER_ID(1)>,
>>          <&dma0 2 AT91_DMA_CFG_PER_ID(2)>;
>>
>> you can match this with the help from
>> Documentation/devicetree/bindings/dma/atmel-dma.txt:
>>
>> 1. A phandle pointing to the DMA controller.
>>
>> 2. The memory interface (16 most significant bits), the peripheral
>> interface
>> (16 less significant bits).
>>
>> 3. Parameters for the at91 DMA configuration register which are device
>>
>> dependent:
>>
>>     - bit 7-0: peripheral identifier for the hardware handshaking
>> interface. The
>>     identifier can be different for tx and rx.
>>
>>     - bit 11-8: FIFO configuration. 0 for half FIFO, 1 for ALAP, 2 for ASAP.
>>
>>
>> So , what was Tudor asking for, is your DT for the ebi node (if you are
>> using ebi), or, your NFC SRAM (Nand Flash Controller SRAM) DMA
>> devicetree chunk, so, we can figure out which type of DMA are you using.
>>
>> Normally, the ebi should be connected to both DMA0 and DMA1 on those
>> interfaces specified in DT. Which ones you want to use, depends on your
>> setup (and contention on the bus/accesses, like in your case, the HLCDC)
>>
>> Thats why we have multiple choices, to pick the right one for each case.
>> In our vanilla DT sama5d3.dtsi we do not have DMA described for ebi
>> interface.
> 
> Ahh, *that* NAND DMA configuration. Of course, how silly of me...
> 
> This is a setup based on the at91-linea.dtsi "CPU" board. That dtsi should
> have the relevant NAND/DMA info. Also, at91-nattis-2-natte-2.dts describes
> the older HW (with a 1024x768 panel) that is also affected, if you want a
> full device tree to look at.
> 
> Looks like I didn't make a selection, quoting from at91-linea.dtsi:

Ok, so try to force the nand to use DDR port 1 : use DMAC0 or DMAC1 on 
the interfaces corresponding just to DDR port 1, and see if helps (while 
leaving HLCDC on both masters - DDR port 2 and 3).

One more thing: what are the actual nand commands which you use when you 
get the glitches? read/write/erase ... ?
What happens if you try to minimize the nand access? you also said at 
some point that only *some* nand accesses cause glitches.

Another thing : even if the LCD displays a still image, the DMA still 
feeds data to the LCD right ?

> 
> &ebi {
> 	pinctrl-0 = <&pinctrl_ebi_nand_addr>;
> 	pinctrl-names = "default";
> 	status = "okay";
> };
> 
> &nand_controller {
> 	status = "okay";
> 
> 	nand: nand@3 {
> 		reg = <0x3 0x0 0x2>;
> 		atmel,rb = <0>;
> 		nand-bus-width = <8>;
> 		nand-ecc-mode = "hw";
> 		nand-ecc-strength = <4>;
> 		nand-ecc-step-size = <512>;
> 		nand-on-flash-bbt;
> 		label = "atmel_nand";
> 	};
> };
> 
> The reason is probably because the sama5d3xek device-trees didn't at the
> time of "fork". Does anybody have any suggestion for some extra properties
> to try in the above nodes?
> 
> Further, I forgot that I had actually upstreamed linea support for
> at91bootstrap, so relevant NAND timings etc can be found in lpddr1_init()
> in
> 
> 	at91bootstrap/contrib/board/axentia/sama5d3_linea/sama5d3_linea.c
> 
> Cheers,
> Peter
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

Re: [PATCH] mtd: nand: raw: atmel: add module param to avoid using dma

[Boris Brezillon]

Hi Eugen,

On Tue, 29 May 2018 09:30:54 +0300
Eugen Hristev <eugen.hristev@microchip.com> wrote:

> On 28.05.2018 13:10, Peter Rosin wrote:
> > On 2018-05-28 00:11, Peter Rosin wrote:  
> >> On 2018-05-27 11:18, Peter Rosin wrote:  
> >>> On 2018-05-25 16:51, Tudor Ambarus wrote:  
> >>>> We think the best way is to keep LCD on DDR Ports 2 and 3 (8th and 9th
> >>>> slaves), to have maximum bandwidth and to use DMA on DDR port 1 for NAND
> >>>> (7th slave).  
> >>>
> >>> Exactly how do I accomplish that?
> >>>
> >>> I can see how I can move the LCD between slave DDR port 2 and 3 by
> >>> selecting LCDC DMA master 8 or 9 (but according to the above it should
> >>> not matter). The big question is how I control what slave the NAND flash
> >>> is going to use? I find nothing in the datasheet, and the code is also
> >>> non-transparent enough for me to figure it out by myself without
> >>> throwing out this question first...  
> 
>  >> [...]  
> 
> >> and the output is
> >>
> >> atmel-nand-controller 10000000.ebi:nand-controller: using dma0chan5 for DMA transfers
> >>
> >> So, DMA controller 0 is in use. I still don't know if IF0, IF1 or IF2 is used
> >> or how to find out. I guess IF2 is not in use since that does not allow any
> >> DDR2 port as slave...  
> 
> Hello Peter,
> 
> Thank you for all the information, I will chip in to help a little bit.
> The Master/channel is described in the device tree. The channel has a 
> controller, a mem/periph interface and a periph ID, plus a FIFO 
> configuration.
> 
> The dma chan number reported in the dmesg is just software. Here is an 
> example from DT:
> dmas = <&dma0 2 AT91_DMA_CFG_PER_ID(1)>,
>         <&dma0 2 AT91_DMA_CFG_PER_ID(2)>;
> 
> you can match this with the help from 
> Documentation/devicetree/bindings/dma/atmel-dma.txt:
> 
> 1. A phandle pointing to the DMA controller. 
> 
> 2. The memory interface (16 most significant bits), the peripheral 
> interface
> (16 less significant bits). 
> 
> 3. Parameters for the at91 DMA configuration register which are device 
> 
> dependent: 
> 
>    - bit 7-0: peripheral identifier for the hardware handshaking 
> interface. The
>    identifier can be different for tx and rx. 
> 
>    - bit 11-8: FIFO configuration. 0 for half FIFO, 1 for ALAP, 2 for ASAP.
> 
> 
> So , what was Tudor asking for, is your DT for the ebi node (if you are 
> using ebi), or, your NFC SRAM (Nand Flash Controller SRAM) DMA 
> devicetree chunk, so, we can figure out which type of DMA are you using.

I think you're missing something here. We use the DMA engine in memcpy
mode (SRAM -> DRAM), not in device mode (dev -> DRAM or DRAM -> dev).
So there's no dmas prop defined in the DT and there should not be.

Regards,

Boris

> 
> Normally, the ebi should be connected to both DMA0 and DMA1 on those 
> interfaces specified in DT. Which ones you want to use, depends on your 
> setup (and contention on the bus/accesses, like in your case, the HLCDC)
> 
> Thats why we have multiple choices, to pick the right one for each case.
> In our vanilla DT sama5d3.dtsi we do not have DMA described for ebi 
> interface.
> 
> Eugen
> 
>  >> [...]  

Re: [PATCH] mtd: nand: raw: atmel: add module param to avoid using dma

[Eugen Hristev]

[...]


> 
> I think you're missing something here. We use the DMA engine in memcpy
> mode (SRAM -> DRAM), not in device mode (dev -> DRAM or DRAM -> dev).
> So there's no dmas prop defined in the DT and there should not be.
> 
> Regards,
> 
> Boris
> 

Ok, so the memcpy SRAM <-> DRAM will hog the transfer between DRAM and 
LCD if my understanding is correct. That's the DMA that Peter wants to 
disable with his patch ?

Then we can then try to force NFC SRAM DMA channels to use just DDR port 
1 or 2 for memcpy ?

I have also received a suggestion to try to increase the porches in 
LCDC_LCDCFG3 .

>>
>>   >> [...]
> 
> 

Re: [PATCH] mtd: nand: raw: atmel: add module param to avoid using dma

[Boris Brezillon]

On Tue, 29 May 2018 18:01:40 +0300
Eugen Hristev <eugen.hristev@microchip.com> wrote:

> [...]
> 
> 
> > 
> > I think you're missing something here. We use the DMA engine in memcpy
> > mode (SRAM -> DRAM), not in device mode (dev -> DRAM or DRAM -> dev).
> > So there's no dmas prop defined in the DT and there should not be.
> > 
> > Regards,
> > 
> > Boris
> >   
> 
> Ok, so the memcpy SRAM <-> DRAM will hog the transfer between DRAM and 
> LCD if my understanding is correct. That's the DMA that Peter wants to 
> disable with his patch ?
> 
> Then we can then try to force NFC SRAM DMA channels to use just DDR port 
> 1 or 2 for memcpy ?

You mean the dmaengine? According to "14.1.3 Master to Slave Access"
that's already the case.

Only DMAC0 can access the NFC SRAM and it's done through DMAC0:IF0,
then access to DDR is going through port DDR port 1 (DMAC0:IF1) or 2
(DMAC0:IF0).

> 
> I have also received a suggestion to try to increase the porches in 
> LCDC_LCDCFG3 .

Yep, Nicolas suggested something similar. Peter, can you try that?

Re: [PATCH] mtd: nand: raw: atmel: add module param to avoid using dma

[Eugen Hristev]

On 29.05.2018 18:15, Boris Brezillon wrote:
> On Tue, 29 May 2018 18:01:40 +0300
> Eugen Hristev <eugen.hristev@microchip.com> wrote:
> 
>> [...]
>>
>>
>>>
>>> I think you're missing something here. We use the DMA engine in memcpy
>>> mode (SRAM -> DRAM), not in device mode (dev -> DRAM or DRAM -> dev).
>>> So there's no dmas prop defined in the DT and there should not be.
>>>
>>> Regards,
>>>
>>> Boris
>>>    
>>
>> Ok, so the memcpy SRAM <-> DRAM will hog the transfer between DRAM and
>> LCD if my understanding is correct. That's the DMA that Peter wants to
>> disable with his patch ?
>>
>> Then we can then try to force NFC SRAM DMA channels to use just DDR port
>> 1 or 2 for memcpy ?
> 
> You mean the dmaengine? According to "14.1.3 Master to Slave Access"
> that's already the case.
> 
> Only DMAC0 can access the NFC SRAM and it's done through DMAC0:IF0,
> then access to DDR is going through port DDR port 1 (DMAC0:IF1) or 2
> (DMAC0:IF0).

If we can make NFC use port 1 only, then HLCDC could have two ports as 
master 8 & 9, maybe a better bandwidth.

> 
>>
>> I have also received a suggestion to try to increase the porches in
>> LCDC_LCDCFG3 .
> 
> Yep, Nicolas suggested something similar. Peter, can you try that?
> 

Re: [PATCH] mtd: nand: raw: atmel: add module param to avoid using dma

[Boris Brezillon]

On Tue, 29 May 2018 18:21:40 +0300
Eugen Hristev <eugen.hristev@microchip.com> wrote:

> On 29.05.2018 18:15, Boris Brezillon wrote:
> > On Tue, 29 May 2018 18:01:40 +0300
> > Eugen Hristev <eugen.hristev@microchip.com> wrote:
> >   
> >> [...]
> >>
> >>  
> >>>
> >>> I think you're missing something here. We use the DMA engine in memcpy
> >>> mode (SRAM -> DRAM), not in device mode (dev -> DRAM or DRAM -> dev).
> >>> So there's no dmas prop defined in the DT and there should not be.
> >>>
> >>> Regards,
> >>>
> >>> Boris
> >>>      
> >>
> >> Ok, so the memcpy SRAM <-> DRAM will hog the transfer between DRAM and
> >> LCD if my understanding is correct. That's the DMA that Peter wants to
> >> disable with his patch ?
> >>
> >> Then we can then try to force NFC SRAM DMA channels to use just DDR port
> >> 1 or 2 for memcpy ?  
> > 
> > You mean the dmaengine? According to "14.1.3 Master to Slave Access"
> > that's already the case.
> > 
> > Only DMAC0 can access the NFC SRAM and it's done through DMAC0:IF0,
> > then access to DDR is going through port DDR port 1 (DMAC0:IF1) or 2
> > (DMAC0:IF0).  
> 
> If we can make NFC use port 1 only, then HLCDC could have two ports as 
> master 8 & 9, maybe a better bandwidth.

Peter, can you try with the following patch?

--->8---
diff --git a/drivers/dma/at_hdmac_regs.h b/drivers/dma/at_hdmac_regs.h
index ef3f227ce3e6..2a48e870f292 100644
--- a/drivers/dma/at_hdmac_regs.h
+++ b/drivers/dma/at_hdmac_regs.h
@@ -124,8 +124,8 @@
 #define        ATC_SIF(i)              (0x3 & (i))     /* Src tx done via AHB-Lite Interface i */
 #define        ATC_DIF(i)              ((0x3 & (i)) <<  4)     /* Dst tx done via AHB-Lite Interface i */
                                  /* Specify AHB interfaces */
-#define AT_DMA_MEM_IF          0 /* interface 0 as memory interface */
-#define AT_DMA_PER_IF          1 /* interface 1 as peripheral interface */
+#define AT_DMA_MEM_IF          1 /* interface 0 as memory interface */
+#define AT_DMA_PER_IF          0 /* interface 1 as peripheral interface */
 
 #define        ATC_SRC_PIP             (0x1 <<  8)     /* Source Picture-in-Picture enabled */
 #define        ATC_DST_PIP             (0x1 << 12)     /* Destination Picture-in-Picture enabled */

Re: [PATCH] mtd: nand: raw: atmel: add module param to avoid using dma

[Boris Brezillon]

On Tue, 29 May 2018 17:46:21 +0200
Boris Brezillon <boris.brezillon@bootlin.com> wrote:

> On Tue, 29 May 2018 18:21:40 +0300
> Eugen Hristev <eugen.hristev@microchip.com> wrote:
> 
> > On 29.05.2018 18:15, Boris Brezillon wrote:  
> > > On Tue, 29 May 2018 18:01:40 +0300
> > > Eugen Hristev <eugen.hristev@microchip.com> wrote:
> > >     
> > >> [...]
> > >>
> > >>    
> > >>>
> > >>> I think you're missing something here. We use the DMA engine in memcpy
> > >>> mode (SRAM -> DRAM), not in device mode (dev -> DRAM or DRAM -> dev).
> > >>> So there's no dmas prop defined in the DT and there should not be.
> > >>>
> > >>> Regards,
> > >>>
> > >>> Boris
> > >>>        
> > >>
> > >> Ok, so the memcpy SRAM <-> DRAM will hog the transfer between DRAM and
> > >> LCD if my understanding is correct. That's the DMA that Peter wants to
> > >> disable with his patch ?
> > >>
> > >> Then we can then try to force NFC SRAM DMA channels to use just DDR port
> > >> 1 or 2 for memcpy ?    
> > > 
> > > You mean the dmaengine? According to "14.1.3 Master to Slave Access"
> > > that's already the case.
> > > 
> > > Only DMAC0 can access the NFC SRAM and it's done through DMAC0:IF0,
> > > then access to DDR is going through port DDR port 1 (DMAC0:IF1) or 2
> > > (DMAC0:IF0).    
> > 
> > If we can make NFC use port 1 only, then HLCDC could have two ports as 
> > master 8 & 9, maybe a better bandwidth.  
> 
> Peter, can you try with the following patch?

Actually that won't work because all SRAMs are on IF0, and here we use
DMA memcpy to copy things from/to the SRAM to/from the DRAM. I have no
simple solution to force usage of IF1 when accessing the DRAM but I'm
also not sure this will solve Peter's problem since forcing LCDC to use
DDR port 3 did not make things better. 

> 
> --->8---  
> diff --git a/drivers/dma/at_hdmac_regs.h b/drivers/dma/at_hdmac_regs.h
> index ef3f227ce3e6..2a48e870f292 100644
> --- a/drivers/dma/at_hdmac_regs.h
> +++ b/drivers/dma/at_hdmac_regs.h
> @@ -124,8 +124,8 @@
>  #define        ATC_SIF(i)              (0x3 & (i))     /* Src tx done via AHB-Lite Interface i */
>  #define        ATC_DIF(i)              ((0x3 & (i)) <<  4)     /* Dst tx done via AHB-Lite Interface i */
>                                   /* Specify AHB interfaces */
> -#define AT_DMA_MEM_IF          0 /* interface 0 as memory interface */
> -#define AT_DMA_PER_IF          1 /* interface 1 as peripheral interface */
> +#define AT_DMA_MEM_IF          1 /* interface 0 as memory interface */
> +#define AT_DMA_PER_IF          0 /* interface 1 as peripheral interface */
>  
>  #define        ATC_SRC_PIP             (0x1 <<  8)     /* Source Picture-in-Picture enabled */
>  #define        ATC_DST_PIP             (0x1 << 12)     /* Destination Picture-in-Picture enabled */

Re: [PATCH] mtd: nand: raw: atmel: add module param to avoid using dma

[Peter Rosin]

Hi again!

I have spent some hours bringing out the old hardware with the 1024x768
panel from underneath the usual piles of junk and layers of dust...and
tried the current kernel on that one. And the display was stable even
when stressing with lots of NAND accesses.  *boggle*

Then I remembered that I had lowered the pixel clock (from 71.1 MHz to
65 MHz (and reduced the vertical blanking to maintain the refresh rate).
I didn't notice that this fixed the NAND interference, probably because
I ran NAND without DMA at the time? Anyway, if I reset the pixel clock
to 71.1 MHz (without increasing the vertical blanking, just to be nasty)
I can get the artifacts easily. But running with a pixel clock of 65 MHz
is not a problem at all, so we can consider NAND-DMA with that panel
solved.

However, now we know that this setup needs relatively little to start
working, and that might be good if we want to see if other changes has
any effect. I will look into that tomorrow. And we can also get a grip
on the critical bandwidth.

But first, answers to some random questions...


On 2018-05-29 09:25, Eugen Hristev wrote:
> One more thing: what are the actual nand commands which you use when you 
> get the glitches? read/write/erase ... ?

Erase seems to be least sensitive, read or write are worse (and similar)
according to my unscientific observations.

> What happens if you try to minimize the nand access? you also said at 
> some point that only *some* nand accesses cause glitches.

These systems will normally not access the NAND, but the displays look
like total crap when this happens. It can happen even when sync()ing
small files, but doesn't happen for every little file. Writing out or
reading a large file to/from NAND invariably triggers the issue.

> Another thing : even if the LCD displays a still image, the DMA still 
> feeds data to the LCD right ?

Absolutely. But since we are not playing some large video file (which
could have been stored on the NAND) we typically don't see the problem. It
only turns up in special circumstances. But these circumstances can't be
avoided and the display looks so freaking ugly when it happens...



On 2018-05-29 17:01, Eugen Hristev wrote:
> Then we can then try to force NFC SRAM DMA channels to use just DDR port 
> 1 or 2 for memcpy ?

I *think* my "horrid" patch does that. Specifically this line

+			desc->txd.phys = (desc->txd.phys & ~3) | 1;




On 2018-05-28 18:09, Nicolas Ferre wrote:
> Can you try to make all that you can to maximize the blanking period of 
> your screen (some are more tolerant than others according to that). By 
> doing so, you would allow the LCD FIFO to recover better after each 
> line. You might loose some columns on the side of your display but it 
> would give us a good idea of how far we are from getting rid of those 
> annoying LCD reset glitches (that are due to underruns on LCD FIFO).

I noticed that the 1024x768 panel is using 24bpp, not 16bpp as I
stated previously. Also, the horizontal blanking is 320 pixels, so a
total of 1024+320=1344 pixels/row and a pixel clock of 71.1 Mhz yields
18.9 us/row. The needed data during that time is 1024*24 bits so
1.30 Gbit/s. For the 65 MHz pixel clock, I get 1.19 Gbit/s. Assuming,
of course, that the pixel clock is actually what was requested... What
is the granularity of the pixel clock anyway?

For the bigger 1920x1080 panel, I have a horizontal blanking of 200
pixels and a pixel clock of 144 MHz, so 14.7 us/row -> 2.09 Gbit/s.
I suspect that no amount of fiddling with blanking is going to get
that anyway near the needed ~1.25 Gbit/s. Besides, the specs of the
panel say that the maximum horizontal blanking time is 280 pixels.
Seems futile to even try since this horizontal blanking time is so
much shorter for the larger panel (fewer and faster pixels) and the
longer time wasn't enough for the smaller panel to catch up. But ok,
in combination with something else it might be just enough. Will try
tomorrow...


On 2018-05-28 18:09, Boris Brezillon wrote:
> On Mon, 28 May 2018 17:52:53 +0200 Peter Rosin <peda@axentia.se> wrote:
>> The panels we are using only supports one resolution (each), but the issue
>> is there with both 1920x1080@16bpp and 1024x768@8bpp (~60Hz).
> 
> Duh! This adds to the weirdness of this issue. I'd thought that by
> dividing the required bandwidth by 2 you would get a reliable setup.

I think I might have misremembered seeing the issue with 1024x768@8bpp.
Sorry. But it *is* there for (the old variant of) 1024x768@24bpp, and
that is still only 60% or so of the bandwidth compared to 1920x1080@16bpp.

Cheers,
Peter

Re: [PATCH] mtd: nand: raw: atmel: add module param to avoid using dma

[Tudor Ambarus]

Hi, Peter,

On 05/28/2018 01:10 PM, Peter Rosin wrote:

[cut]

> So, I think I want either
> 
> A) the NAND controller to use master 1 DMAC0/IF0 (i.e. slave 8 DDR2 port 2) and
>     the LCDC to use master 9 (i.e. slave 9 DDR2 Port 3)
> 
> or
> 
> B) the NAND controller to use master 2 DMAC0/IF1 (i.e. slave 7 DDR2 port 1, and
>     possibly slave 9 DDR2 port 3 (if my previous findings are relevant) and the
>     LCDC to use master 8 (i.e. slave 8 DDR2 Port 2)

My understanding is that "Table 14-3. Master to Slave Access" describes
what connections are allowed between the masters and slaves, while the
PRxSy registers just set the priorities. What happens when you assign
the highest priority to a master to slave connection that is not
allowed? Probably it is ignored, but I'll check with the hardware team.
So I expect that the NAND controller can not use DDR2 port 3 regardless
of the priority set.

[cut]

> So, output is as expected and I believe that the patch makes the NAND DMA
> accesses use master 2 DMAC0/IF1 and are thus forced to use slave 7 DDR2 Port 1
> (and possibly 9). The LCDC is using slave 8 DDR2 Port 2. So there should be no
> slave conflict?
> 
> But the on-screen crap remains during NAND accesses.

No conflict, but you missed to dispatch the load on the LCDC DMA
masters, if I understood correctly.

So, I think we want to test the following:
- NAND controller to use DMAC0/IF1 (slave 7 DDR2 port 1)
- LCDC to use master 8 (slave 8 DDR2 Port 2) and master 9 (slave 9 DDR2
Port 3).

Best,
ta

Re: [PATCH] mtd: nand: raw: atmel: add module param to avoid using dma

[Boris Brezillon]

On Mon, 4 Jun 2018 18:46:56 +0300
Tudor Ambarus <tudor.ambarus@microchip.com> wrote:

> Hi, Peter,
> 
> On 05/28/2018 01:10 PM, Peter Rosin wrote:
> 
> [cut]
> 
> > So, I think I want either
> > 
> > A) the NAND controller to use master 1 DMAC0/IF0 (i.e. slave 8 DDR2 port 2) and
> >     the LCDC to use master 9 (i.e. slave 9 DDR2 Port 3)
> > 
> > or
> > 
> > B) the NAND controller to use master 2 DMAC0/IF1 (i.e. slave 7 DDR2 port 1, and
> >     possibly slave 9 DDR2 port 3 (if my previous findings are relevant) and the
> >     LCDC to use master 8 (i.e. slave 8 DDR2 Port 2)  
> 
> My understanding is that "Table 14-3. Master to Slave Access" describes
> what connections are allowed between the masters and slaves, while the
> PRxSy registers just set the priorities. What happens when you assign
> the highest priority to a master to slave connection that is not
> allowed? Probably it is ignored, but I'll check with the hardware team.
> So I expect that the NAND controller can not use DDR2 port 3 regardless
> of the priority set.
> 
> [cut]
> 
> > So, output is as expected and I believe that the patch makes the NAND DMA
> > accesses use master 2 DMAC0/IF1 and are thus forced to use slave 7 DDR2 Port 1
> > (and possibly 9). The LCDC is using slave 8 DDR2 Port 2. So there should be no
> > slave conflict?
> > 
> > But the on-screen crap remains during NAND accesses.  
> 
> No conflict, but you missed to dispatch the load on the LCDC DMA
> masters, if I understood correctly.
> 
> So, I think we want to test the following:
> - NAND controller to use DMAC0/IF1 (slave 7 DDR2 port 1)

As I explained in one of my previous email, it's not that easy to set
up, because the SRAM is connected to IF0, and we're using DMA memcpy
here. Also, I don't see how it could solve Peter's problem if, even
when he switches to LCDC master 9 for the primary overlay, he still
keeps experiencing FIFO underruns.

> - LCDC to use master 8 (slave 8 DDR2 Port 2) and master 9 (slave 9 DDR2
> Port 3).

Except that only works if you have several overlays activated, which
AFAIR, is not the case in Peter's setup.

Re: [PATCH] mtd: nand: raw: atmel: add module param to avoid using dma

[Peter Rosin]

On 2018-04-02 22:20, Boris Brezillon wrote:
> On Mon, 2 Apr 2018 21:28:43 +0200
> Boris Brezillon <boris.brezillon@bootlin.com> wrote:
> 
>> On Mon, 2 Apr 2018 19:59:39 +0200
>> Peter Rosin <peda@axentia.se> wrote:
>>
>>> On 2018-04-02 14:22, Boris Brezillon wrote:  
>>>> On Thu, 29 Mar 2018 16:27:12 +0200
>>>> Peter Rosin <peda@axentia.se> wrote:
>>>>     
>>>>> On 2018-03-29 15:44, Boris Brezillon wrote:    
>>>>>> On Thu, 29 Mar 2018 15:37:43 +0200
>>>>>> Peter Rosin <peda@axentia.se> wrote:
>>>>>>       
>>>>>>> On 2018-03-29 15:33, Boris Brezillon wrote:      
>>>>>>>> On Thu, 29 Mar 2018 15:10:54 +0200
>>>>>>>> Peter Rosin <peda@axentia.se> wrote:
>>>>>>>>         
>>>>>>>>> On a sama5d31 with a Full-HD dual LVDS panel (132MHz pixel clock) NAND
>>>>>>>>> flash accesses have a tendency to cause display disturbances. Add a
>>>>>>>>> module param to disable DMA from the NAND controller, since that fixes
>>>>>>>>> the display problem for me.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Peter Rosin <peda@axentia.se>
>>>>>>>>> ---
>>>>>>>>>  drivers/mtd/nand/raw/atmel/nand-controller.c | 7 ++++++-
>>>>>>>>>  1 file changed, 6 insertions(+), 1 deletion(-)
>>>>>>>>>
>>>>>>>>> diff --git a/drivers/mtd/nand/raw/atmel/nand-controller.c b/drivers/mtd/nand/raw/atmel/nand-controller.c
>>>>>>>>> index b2f00b398490..2ff7a77c7b8e 100644
>>>>>>>>> --- a/drivers/mtd/nand/raw/atmel/nand-controller.c
>>>>>>>>> +++ b/drivers/mtd/nand/raw/atmel/nand-controller.c
>>>>>>>>> @@ -129,6 +129,11 @@
>>>>>>>>>  #define DEFAULT_TIMEOUT_MS			1000
>>>>>>>>>  #define MIN_DMA_LEN				128
>>>>>>>>>  
>>>>>>>>> +static bool atmel_nand_avoid_dma __read_mostly;
>>>>>>>>> +
>>>>>>>>> +MODULE_PARM_DESC(avoiddma, "Avoid using DMA");
>>>>>>>>> +module_param_named(avoiddma, atmel_nand_avoid_dma, bool, 0400);        
>>>>>>>>
>>>>>>>> I'm not a big fan of those driver specific cmdline parameters. Can't we
>>>>>>>> instead give an higher priority to HLCDC master using the bus matrix?        
>>>>>>>
>>>>>>> I don't know if it will be enough, but we sure can try. However, I have
>>>>>>> no idea how to do that. I will happily test stuff though...      
>>>>>>
>>>>>> There's no interface to configure that from Linux, but you can try to
>>>>>> tweak it with devmem and if that does the trick, maybe we can expose a
>>>>>> way to configure that from Linux. For more details, see the "Bus Matrix
>>>>>> (MATRIX)" section in Atmel datasheets.      
>>>>>
>>>>> I don't seem to succeed in changing the registers I think I need to change.
>>>>> I can poke the "Write Protection Mode Register" by writing MAT0 and MAT1 to
>>>>> it.    
>>>>
>>>> You mean 0x4D415400, right? ("MAT0" != 0x4D415400).    
>>>
>>> Bits 1 through 7 do not matter, so even though not equal they are (or
>>> should be) equivalent. But I did use 0x4d415400. I simply used the
>>> shorter syntax since that was easier to type and conveyed the relevant
>>> info.  
>>
>> Ok.
>>
>>>   
>>>>> But when I try to write to "Priority Registers B For Slaves" it doesn't
>>>>> take, regardless of write protect mode.    
>>>>
>>>> Did you check MATRIX_WPSR after writing to MATRIX_PRXSY?    
>>>
>>> No, but did it again and checked, see transcript below.  
>>
>> I don't use devmem2. Is 'readback' information accurate or is it
>> always what's been written? Because when you write 0x33 to 0xFFFFECBC,
>> 0x33 is read back, but just after that, when you read it again it's 0.
>>
>>> BTW, how do I
>>> know which master is in use for the LCD controller? 8 or 9? Both?  
>>
>> It's configurable on a per-layer basis through the SIF bit in
>> LCDC_<layer>CFG0. The driver tries to dispatch the load on those 2 AHB
>> masters [1].
>>
>>> And
>>> which DDR slave is the target? 7, 8, 9 or 10? More than one?  
>>
>> This, I don't know. I guess all of them can be used.
> 
> Looks like I was wrong. According to "Table 15-3. SAMA5D3 Master to
> Slave Access", LCDC port 0 can only access DDR port 2 and LCDC port 1
> can only access DDR port 3.
> 
> Can you try to write 0x3 to 0xFFFFECCC and 0x30 to 0xFFFFECD4?

Given the matrix dump in the other mail, it is not surprising that I
can't. But if I change the matrix from the default

 0 33--3--3--3333--
 1 33--3--3--3333--
 2 33--------------
 3 -3--------333---
 4 33--------------
 5 3---------------
 6 33--33-33333333-
 7 --3-3--3--------
 8 -3---3--3--3----
 9 --3-3--3-33-333-
10 3--3------------
11 3-----3---------
12 ----------------
13 ----------------
14 ----------------
15 ----------------

to

 0 33--3--3--3333--
 1 33--3--3--3333--
 2 33--------------
 3 -3--------333---
 4 33--------------
 5 3---------------
 6 33--33-33333333-
 7 --1-1--3--------
 8 -1---1--3--3----
 9 --1-1--3-33-333-
10 3--3------------
11 3-----3---------
12 ----------------
13 ----------------
14 ----------------
15 ----------------

which I *think* is reducing the prio of accesses from all DMAC masters
to all DDR slaves, and then change the ULBT to 1 (SINGLE) for all
six DMAC masters, I still get the same display disturbances on nand
accesses. And I can't seem to tweak the LQOSENx bits, at least not for
the DMAC/DDR case.

Cheers,
Peter

Re: [PATCH] mtd: nand: raw: atmel: add module param to avoid using dma

[Boris Brezillon]

On Tue, 3 Apr 2018 08:51:10 +0200
Peter Rosin <peda@axentia.se> wrote:

> On 2018-04-02 22:20, Boris Brezillon wrote:
> > On Mon, 2 Apr 2018 21:28:43 +0200
> > Boris Brezillon <boris.brezillon@bootlin.com> wrote:
> >   
> >> On Mon, 2 Apr 2018 19:59:39 +0200
> >> Peter Rosin <peda@axentia.se> wrote:
> >>  
> >>> On 2018-04-02 14:22, Boris Brezillon wrote:    
> >>>> On Thu, 29 Mar 2018 16:27:12 +0200
> >>>> Peter Rosin <peda@axentia.se> wrote:
> >>>>       
> >>>>> On 2018-03-29 15:44, Boris Brezillon wrote:      
> >>>>>> On Thu, 29 Mar 2018 15:37:43 +0200
> >>>>>> Peter Rosin <peda@axentia.se> wrote:
> >>>>>>         
> >>>>>>> On 2018-03-29 15:33, Boris Brezillon wrote:        
> >>>>>>>> On Thu, 29 Mar 2018 15:10:54 +0200
> >>>>>>>> Peter Rosin <peda@axentia.se> wrote:
> >>>>>>>>           
> >>>>>>>>> On a sama5d31 with a Full-HD dual LVDS panel (132MHz pixel clock) NAND
> >>>>>>>>> flash accesses have a tendency to cause display disturbances. Add a
> >>>>>>>>> module param to disable DMA from the NAND controller, since that fixes
> >>>>>>>>> the display problem for me.
> >>>>>>>>>
> >>>>>>>>> Signed-off-by: Peter Rosin <peda@axentia.se>
> >>>>>>>>> ---
> >>>>>>>>>  drivers/mtd/nand/raw/atmel/nand-controller.c | 7 ++++++-
> >>>>>>>>>  1 file changed, 6 insertions(+), 1 deletion(-)
> >>>>>>>>>
> >>>>>>>>> diff --git a/drivers/mtd/nand/raw/atmel/nand-controller.c b/drivers/mtd/nand/raw/atmel/nand-controller.c
> >>>>>>>>> index b2f00b398490..2ff7a77c7b8e 100644
> >>>>>>>>> --- a/drivers/mtd/nand/raw/atmel/nand-controller.c
> >>>>>>>>> +++ b/drivers/mtd/nand/raw/atmel/nand-controller.c
> >>>>>>>>> @@ -129,6 +129,11 @@
> >>>>>>>>>  #define DEFAULT_TIMEOUT_MS			1000
> >>>>>>>>>  #define MIN_DMA_LEN				128
> >>>>>>>>>  
> >>>>>>>>> +static bool atmel_nand_avoid_dma __read_mostly;
> >>>>>>>>> +
> >>>>>>>>> +MODULE_PARM_DESC(avoiddma, "Avoid using DMA");
> >>>>>>>>> +module_param_named(avoiddma, atmel_nand_avoid_dma, bool, 0400);          
> >>>>>>>>
> >>>>>>>> I'm not a big fan of those driver specific cmdline parameters. Can't we
> >>>>>>>> instead give an higher priority to HLCDC master using the bus matrix?          
> >>>>>>>
> >>>>>>> I don't know if it will be enough, but we sure can try. However, I have
> >>>>>>> no idea how to do that. I will happily test stuff though...        
> >>>>>>
> >>>>>> There's no interface to configure that from Linux, but you can try to
> >>>>>> tweak it with devmem and if that does the trick, maybe we can expose a
> >>>>>> way to configure that from Linux. For more details, see the "Bus Matrix
> >>>>>> (MATRIX)" section in Atmel datasheets.        
> >>>>>
> >>>>> I don't seem to succeed in changing the registers I think I need to change.
> >>>>> I can poke the "Write Protection Mode Register" by writing MAT0 and MAT1 to
> >>>>> it.      
> >>>>
> >>>> You mean 0x4D415400, right? ("MAT0" != 0x4D415400).      
> >>>
> >>> Bits 1 through 7 do not matter, so even though not equal they are (or
> >>> should be) equivalent. But I did use 0x4d415400. I simply used the
> >>> shorter syntax since that was easier to type and conveyed the relevant
> >>> info.    
> >>
> >> Ok.
> >>  
> >>>     
> >>>>> But when I try to write to "Priority Registers B For Slaves" it doesn't
> >>>>> take, regardless of write protect mode.      
> >>>>
> >>>> Did you check MATRIX_WPSR after writing to MATRIX_PRXSY?      
> >>>
> >>> No, but did it again and checked, see transcript below.    
> >>
> >> I don't use devmem2. Is 'readback' information accurate or is it
> >> always what's been written? Because when you write 0x33 to 0xFFFFECBC,
> >> 0x33 is read back, but just after that, when you read it again it's 0.
> >>  
> >>> BTW, how do I
> >>> know which master is in use for the LCD controller? 8 or 9? Both?    
> >>
> >> It's configurable on a per-layer basis through the SIF bit in
> >> LCDC_<layer>CFG0. The driver tries to dispatch the load on those 2 AHB
> >> masters [1].
> >>  
> >>> And
> >>> which DDR slave is the target? 7, 8, 9 or 10? More than one?    
> >>
> >> This, I don't know. I guess all of them can be used.  
> > 
> > Looks like I was wrong. According to "Table 15-3. SAMA5D3 Master to
> > Slave Access", LCDC port 0 can only access DDR port 2 and LCDC port 1
> > can only access DDR port 3.
> > 
> > Can you try to write 0x3 to 0xFFFFECCC and 0x30 to 0xFFFFECD4?  
> 
> Given the matrix dump in the other mail, it is not surprising that I
> can't. But if I change the matrix from the default
> 
>  0 33--3--3--3333--
>  1 33--3--3--3333--
>  2 33--------------
>  3 -3--------333---
>  4 33--------------
>  5 3---------------
>  6 33--33-33333333-
>  7 --3-3--3--------
>  8 -3---3--3--3----
>  9 --3-3--3-33-333-
> 10 3--3------------
> 11 3-----3---------
> 12 ----------------
> 13 ----------------
> 14 ----------------
> 15 ----------------

Is it really the default? I thought all entries were set to 0 by
default.

> 
> to
> 
>  0 33--3--3--3333--
>  1 33--3--3--3333--
>  2 33--------------
>  3 -3--------333---
>  4 33--------------
>  5 3---------------
>  6 33--33-33333333-
>  7 --1-1--3--------
>  8 -1---1--3--3----

Why do you change priority on slave 7 and 8 (AKA DDR port 0 and 1)?
They don't seem to be used by the LCDC.

>  9 --1-1--3-33-333-

Shouldn't we have

   9 -1---1--3--1----

here?

> 10 3--3------------
> 11 3-----3---------
> 12 ----------------
> 13 ----------------
> 14 ----------------
> 15 ----------------
> 
> which I *think* is reducing the prio of accesses from all DMAC masters
> to all DDR slaves, and then change the ULBT to 1 (SINGLE) for all
> six DMAC masters, I still get the same display disturbances on nand
> accesses. And I can't seem to tweak the LQOSENx bits, at least not for
> the DMAC/DDR case.

Can you try forcing ->ahb_id to 1 in the hlcdc driver, just to make
sure it solves the problem if we go through DDR port 3?


-- 
Boris Brezillon, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH] mtd: nand: raw: atmel: add module param to avoid using dma

[Boris Brezillon]

On Tue, 3 Apr 2018 09:15:22 +0200
Boris Brezillon <boris.brezillon@bootlin.com> wrote:
> 
> > 
> > to
> > 
> >  0 33--3--3--3333--
> >  1 33--3--3--3333--
> >  2 33--------------
> >  3 -3--------333---
> >  4 33--------------
> >  5 3---------------
> >  6 33--33-33333333-
> >  7 --1-1--3--------
> >  8 -1---1--3--3----  
> 
> Why do you change priority on slave 7 and 8 (AKA DDR port 0 and 1)?
> They don't seem to be used by the LCDC.
> 
> >  9 --1-1--3-33-333-  
> 
> Shouldn't we have
> 
>    9 -1---1--3--1----
> 
> here?

Never mind. I just read the email where you explain the datasheet vs
reality mismatch after this one.

I guess we'll have to wait for a Nicolas' feedback here.

-- 
Boris Brezillon, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH] mtd: nand: raw: atmel: add module param to avoid using dma

[Peter Rosin]

On 2018-04-03 09:15, Boris Brezillon wrote:
> On Tue, 3 Apr 2018 08:51:10 +0200
> Peter Rosin <peda@axentia.se> wrote:
> 
>> On 2018-04-02 22:20, Boris Brezillon wrote:
>>> On Mon, 2 Apr 2018 21:28:43 +0200
>>> Boris Brezillon <boris.brezillon@bootlin.com> wrote:
>>>   
>>>> On Mon, 2 Apr 2018 19:59:39 +0200
>>>> Peter Rosin <peda@axentia.se> wrote:
>>>>  
>>>>> On 2018-04-02 14:22, Boris Brezillon wrote:    
>>>>>> On Thu, 29 Mar 2018 16:27:12 +0200
>>>>>> Peter Rosin <peda@axentia.se> wrote:
>>>>>>       
>>>>>>> On 2018-03-29 15:44, Boris Brezillon wrote:      
>>>>>>>> On Thu, 29 Mar 2018 15:37:43 +0200
>>>>>>>> Peter Rosin <peda@axentia.se> wrote:
>>>>>>>>         
>>>>>>>>> On 2018-03-29 15:33, Boris Brezillon wrote:        
>>>>>>>>>> On Thu, 29 Mar 2018 15:10:54 +0200
>>>>>>>>>> Peter Rosin <peda@axentia.se> wrote:
>>>>>>>>>>           
>>>>>>>>>>> On a sama5d31 with a Full-HD dual LVDS panel (132MHz pixel clock) NAND
>>>>>>>>>>> flash accesses have a tendency to cause display disturbances. Add a
>>>>>>>>>>> module param to disable DMA from the NAND controller, since that fixes
>>>>>>>>>>> the display problem for me.
>>>>>>>>>>>
>>>>>>>>>>> Signed-off-by: Peter Rosin <peda@axentia.se>
>>>>>>>>>>> ---
>>>>>>>>>>>  drivers/mtd/nand/raw/atmel/nand-controller.c | 7 ++++++-
>>>>>>>>>>>  1 file changed, 6 insertions(+), 1 deletion(-)
>>>>>>>>>>>
>>>>>>>>>>> diff --git a/drivers/mtd/nand/raw/atmel/nand-controller.c b/drivers/mtd/nand/raw/atmel/nand-controller.c
>>>>>>>>>>> index b2f00b398490..2ff7a77c7b8e 100644
>>>>>>>>>>> --- a/drivers/mtd/nand/raw/atmel/nand-controller.c
>>>>>>>>>>> +++ b/drivers/mtd/nand/raw/atmel/nand-controller.c
>>>>>>>>>>> @@ -129,6 +129,11 @@
>>>>>>>>>>>  #define DEFAULT_TIMEOUT_MS			1000
>>>>>>>>>>>  #define MIN_DMA_LEN				128
>>>>>>>>>>>  
>>>>>>>>>>> +static bool atmel_nand_avoid_dma __read_mostly;
>>>>>>>>>>> +
>>>>>>>>>>> +MODULE_PARM_DESC(avoiddma, "Avoid using DMA");
>>>>>>>>>>> +module_param_named(avoiddma, atmel_nand_avoid_dma, bool, 0400);          
>>>>>>>>>>
>>>>>>>>>> I'm not a big fan of those driver specific cmdline parameters. Can't we
>>>>>>>>>> instead give an higher priority to HLCDC master using the bus matrix?          
>>>>>>>>>
>>>>>>>>> I don't know if it will be enough, but we sure can try. However, I have
>>>>>>>>> no idea how to do that. I will happily test stuff though...        
>>>>>>>>
>>>>>>>> There's no interface to configure that from Linux, but you can try to
>>>>>>>> tweak it with devmem and if that does the trick, maybe we can expose a
>>>>>>>> way to configure that from Linux. For more details, see the "Bus Matrix
>>>>>>>> (MATRIX)" section in Atmel datasheets.        
>>>>>>>
>>>>>>> I don't seem to succeed in changing the registers I think I need to change.
>>>>>>> I can poke the "Write Protection Mode Register" by writing MAT0 and MAT1 to
>>>>>>> it.      
>>>>>>
>>>>>> You mean 0x4D415400, right? ("MAT0" != 0x4D415400).      
>>>>>
>>>>> Bits 1 through 7 do not matter, so even though not equal they are (or
>>>>> should be) equivalent. But I did use 0x4d415400. I simply used the
>>>>> shorter syntax since that was easier to type and conveyed the relevant
>>>>> info.    
>>>>
>>>> Ok.
>>>>  
>>>>>     
>>>>>>> But when I try to write to "Priority Registers B For Slaves" it doesn't
>>>>>>> take, regardless of write protect mode.      
>>>>>>
>>>>>> Did you check MATRIX_WPSR after writing to MATRIX_PRXSY?      
>>>>>
>>>>> No, but did it again and checked, see transcript below.    
>>>>
>>>> I don't use devmem2. Is 'readback' information accurate or is it
>>>> always what's been written? Because when you write 0x33 to 0xFFFFECBC,
>>>> 0x33 is read back, but just after that, when you read it again it's 0.
>>>>  
>>>>> BTW, how do I
>>>>> know which master is in use for the LCD controller? 8 or 9? Both?    
>>>>
>>>> It's configurable on a per-layer basis through the SIF bit in
>>>> LCDC_<layer>CFG0. The driver tries to dispatch the load on those 2 AHB
>>>> masters [1].
>>>>  
>>>>> And
>>>>> which DDR slave is the target? 7, 8, 9 or 10? More than one?    
>>>>
>>>> This, I don't know. I guess all of them can be used.  
>>>
>>> Looks like I was wrong. According to "Table 15-3. SAMA5D3 Master to
>>> Slave Access", LCDC port 0 can only access DDR port 2 and LCDC port 1
>>> can only access DDR port 3.
>>>
>>> Can you try to write 0x3 to 0xFFFFECCC and 0x30 to 0xFFFFECD4?  
>>
>> Given the matrix dump in the other mail, it is not surprising that I
>> can't. But if I change the matrix from the default
>>
>>  0 33--3--3--3333--
>>  1 33--3--3--3333--
>>  2 33--------------
>>  3 -3--------333---
>>  4 33--------------
>>  5 3---------------
>>  6 33--33-33333333-
>>  7 --3-3--3--------
>>  8 -3---3--3--3----
>>  9 --3-3--3-33-333-
>> 10 3--3------------
>> 11 3-----3---------
>> 12 ----------------
>> 13 ----------------
>> 14 ----------------
>> 15 ----------------
> 
> Is it really the default? I thought all entries were set to 0 by
> default.

Yes, the datasheet claims 0 to be the reset default, but there is
a note in my datasheet that "Values in the Bus Matrix Priority
Registers are product dependent". I was referring to what I see
with devmem2 from the shell directly after boot. I have no idea
where the threes are coming from; they could be reset default or
from some loop somewhere that simply tries to write the highest
priority everywhere, but that the write then only sticks in the
allowed entries.

BTW, the sanity of everybody screaming "I'm super-important" is
a bit questionable...

Cheers,
Peter

Re: [PATCH] mtd: nand: raw: atmel: add module param to avoid using dma

[Boris Brezillon]

On Tue, 3 Apr 2018 10:14:31 +0200
Peter Rosin <peda@axentia.se> wrote:

> On 2018-04-03 09:15, Boris Brezillon wrote:
> > On Tue, 3 Apr 2018 08:51:10 +0200
> > Peter Rosin <peda@axentia.se> wrote:
> >   
> >> On 2018-04-02 22:20, Boris Brezillon wrote:  
> >>> On Mon, 2 Apr 2018 21:28:43 +0200
> >>> Boris Brezillon <boris.brezillon@bootlin.com> wrote:
> >>>     
> >>>> On Mon, 2 Apr 2018 19:59:39 +0200
> >>>> Peter Rosin <peda@axentia.se> wrote:
> >>>>    
> >>>>> On 2018-04-02 14:22, Boris Brezillon wrote:      
> >>>>>> On Thu, 29 Mar 2018 16:27:12 +0200
> >>>>>> Peter Rosin <peda@axentia.se> wrote:
> >>>>>>         
> >>>>>>> On 2018-03-29 15:44, Boris Brezillon wrote:        
> >>>>>>>> On Thu, 29 Mar 2018 15:37:43 +0200
> >>>>>>>> Peter Rosin <peda@axentia.se> wrote:
> >>>>>>>>           
> >>>>>>>>> On 2018-03-29 15:33, Boris Brezillon wrote:          
> >>>>>>>>>> On Thu, 29 Mar 2018 15:10:54 +0200
> >>>>>>>>>> Peter Rosin <peda@axentia.se> wrote:
> >>>>>>>>>>             
> >>>>>>>>>>> On a sama5d31 with a Full-HD dual LVDS panel (132MHz pixel clock) NAND
> >>>>>>>>>>> flash accesses have a tendency to cause display disturbances. Add a
> >>>>>>>>>>> module param to disable DMA from the NAND controller, since that fixes
> >>>>>>>>>>> the display problem for me.
> >>>>>>>>>>>
> >>>>>>>>>>> Signed-off-by: Peter Rosin <peda@axentia.se>
> >>>>>>>>>>> ---
> >>>>>>>>>>>  drivers/mtd/nand/raw/atmel/nand-controller.c | 7 ++++++-
> >>>>>>>>>>>  1 file changed, 6 insertions(+), 1 deletion(-)
> >>>>>>>>>>>
> >>>>>>>>>>> diff --git a/drivers/mtd/nand/raw/atmel/nand-controller.c b/drivers/mtd/nand/raw/atmel/nand-controller.c
> >>>>>>>>>>> index b2f00b398490..2ff7a77c7b8e 100644
> >>>>>>>>>>> --- a/drivers/mtd/nand/raw/atmel/nand-controller.c
> >>>>>>>>>>> +++ b/drivers/mtd/nand/raw/atmel/nand-controller.c
> >>>>>>>>>>> @@ -129,6 +129,11 @@
> >>>>>>>>>>>  #define DEFAULT_TIMEOUT_MS			1000
> >>>>>>>>>>>  #define MIN_DMA_LEN				128
> >>>>>>>>>>>  
> >>>>>>>>>>> +static bool atmel_nand_avoid_dma __read_mostly;
> >>>>>>>>>>> +
> >>>>>>>>>>> +MODULE_PARM_DESC(avoiddma, "Avoid using DMA");
> >>>>>>>>>>> +module_param_named(avoiddma, atmel_nand_avoid_dma, bool, 0400);            
> >>>>>>>>>>
> >>>>>>>>>> I'm not a big fan of those driver specific cmdline parameters. Can't we
> >>>>>>>>>> instead give an higher priority to HLCDC master using the bus matrix?            
> >>>>>>>>>
> >>>>>>>>> I don't know if it will be enough, but we sure can try. However, I have
> >>>>>>>>> no idea how to do that. I will happily test stuff though...          
> >>>>>>>>
> >>>>>>>> There's no interface to configure that from Linux, but you can try to
> >>>>>>>> tweak it with devmem and if that does the trick, maybe we can expose a
> >>>>>>>> way to configure that from Linux. For more details, see the "Bus Matrix
> >>>>>>>> (MATRIX)" section in Atmel datasheets.          
> >>>>>>>
> >>>>>>> I don't seem to succeed in changing the registers I think I need to change.
> >>>>>>> I can poke the "Write Protection Mode Register" by writing MAT0 and MAT1 to
> >>>>>>> it.        
> >>>>>>
> >>>>>> You mean 0x4D415400, right? ("MAT0" != 0x4D415400).        
> >>>>>
> >>>>> Bits 1 through 7 do not matter, so even though not equal they are (or
> >>>>> should be) equivalent. But I did use 0x4d415400. I simply used the
> >>>>> shorter syntax since that was easier to type and conveyed the relevant
> >>>>> info.      
> >>>>
> >>>> Ok.
> >>>>    
> >>>>>       
> >>>>>>> But when I try to write to "Priority Registers B For Slaves" it doesn't
> >>>>>>> take, regardless of write protect mode.        
> >>>>>>
> >>>>>> Did you check MATRIX_WPSR after writing to MATRIX_PRXSY?        
> >>>>>
> >>>>> No, but did it again and checked, see transcript below.      
> >>>>
> >>>> I don't use devmem2. Is 'readback' information accurate or is it
> >>>> always what's been written? Because when you write 0x33 to 0xFFFFECBC,
> >>>> 0x33 is read back, but just after that, when you read it again it's 0.
> >>>>    
> >>>>> BTW, how do I
> >>>>> know which master is in use for the LCD controller? 8 or 9? Both?      
> >>>>
> >>>> It's configurable on a per-layer basis through the SIF bit in
> >>>> LCDC_<layer>CFG0. The driver tries to dispatch the load on those 2 AHB
> >>>> masters [1].
> >>>>    
> >>>>> And
> >>>>> which DDR slave is the target? 7, 8, 9 or 10? More than one?      
> >>>>
> >>>> This, I don't know. I guess all of them can be used.    
> >>>
> >>> Looks like I was wrong. According to "Table 15-3. SAMA5D3 Master to
> >>> Slave Access", LCDC port 0 can only access DDR port 2 and LCDC port 1
> >>> can only access DDR port 3.
> >>>
> >>> Can you try to write 0x3 to 0xFFFFECCC and 0x30 to 0xFFFFECD4?    
> >>
> >> Given the matrix dump in the other mail, it is not surprising that I
> >> can't. But if I change the matrix from the default
> >>
> >>  0 33--3--3--3333--
> >>  1 33--3--3--3333--
> >>  2 33--------------
> >>  3 -3--------333---
> >>  4 33--------------
> >>  5 3---------------
> >>  6 33--33-33333333-
> >>  7 --3-3--3--------
> >>  8 -3---3--3--3----
> >>  9 --3-3--3-33-333-
> >> 10 3--3------------
> >> 11 3-----3---------
> >> 12 ----------------
> >> 13 ----------------
> >> 14 ----------------
> >> 15 ----------------  
> > 
> > Is it really the default? I thought all entries were set to 0 by
> > default.  
> 
> Yes, the datasheet claims 0 to be the reset default, but there is
> a note in my datasheet that "Values in the Bus Matrix Priority
> Registers are product dependent". I was referring to what I see
> with devmem2 from the shell directly after boot. I have no idea
> where the threes are coming from; they could be reset default or
> from some loop somewhere that simply tries to write the highest
> priority everywhere, but that the write then only sticks in the
> allowed entries.
> 
> BTW, the sanity of everybody screaming "I'm super-important" is
> a bit questionable...

It is indeed.

Re: [PATCH] mtd: nand: raw: atmel: add module param to avoid using dma

[Peter Rosin]

On 2018-04-02 21:28, Boris Brezillon wrote:
> On Mon, 2 Apr 2018 19:59:39 +0200
> Peter Rosin <peda@axentia.se> wrote:
> 
>> On 2018-04-02 14:22, Boris Brezillon wrote:
>>> On Thu, 29 Mar 2018 16:27:12 +0200
>>> Peter Rosin <peda@axentia.se> wrote:
>>>   
>>>> On 2018-03-29 15:44, Boris Brezillon wrote:  
>>>>> On Thu, 29 Mar 2018 15:37:43 +0200
>>>>> Peter Rosin <peda@axentia.se> wrote:
>>>>>     
>>>>>> On 2018-03-29 15:33, Boris Brezillon wrote:    
>>>>>>> On Thu, 29 Mar 2018 15:10:54 +0200
>>>>>>> Peter Rosin <peda@axentia.se> wrote:
>>>>>>>       
>>>>>>>> On a sama5d31 with a Full-HD dual LVDS panel (132MHz pixel clock) NAND
>>>>>>>> flash accesses have a tendency to cause display disturbances. Add a
>>>>>>>> module param to disable DMA from the NAND controller, since that fixes
>>>>>>>> the display problem for me.
>>>>>>>>
>>>>>>>> Signed-off-by: Peter Rosin <peda@axentia.se>
>>>>>>>> ---
>>>>>>>>  drivers/mtd/nand/raw/atmel/nand-controller.c | 7 ++++++-
>>>>>>>>  1 file changed, 6 insertions(+), 1 deletion(-)
>>>>>>>>
>>>>>>>> diff --git a/drivers/mtd/nand/raw/atmel/nand-controller.c b/drivers/mtd/nand/raw/atmel/nand-controller.c
>>>>>>>> index b2f00b398490..2ff7a77c7b8e 100644
>>>>>>>> --- a/drivers/mtd/nand/raw/atmel/nand-controller.c
>>>>>>>> +++ b/drivers/mtd/nand/raw/atmel/nand-controller.c
>>>>>>>> @@ -129,6 +129,11 @@
>>>>>>>>  #define DEFAULT_TIMEOUT_MS			1000
>>>>>>>>  #define MIN_DMA_LEN				128
>>>>>>>>  
>>>>>>>> +static bool atmel_nand_avoid_dma __read_mostly;
>>>>>>>> +
>>>>>>>> +MODULE_PARM_DESC(avoiddma, "Avoid using DMA");
>>>>>>>> +module_param_named(avoiddma, atmel_nand_avoid_dma, bool, 0400);      
>>>>>>>
>>>>>>> I'm not a big fan of those driver specific cmdline parameters. Can't we
>>>>>>> instead give an higher priority to HLCDC master using the bus matrix?      
>>>>>>
>>>>>> I don't know if it will be enough, but we sure can try. However, I have
>>>>>> no idea how to do that. I will happily test stuff though...    
>>>>>
>>>>> There's no interface to configure that from Linux, but you can try to
>>>>> tweak it with devmem and if that does the trick, maybe we can expose a
>>>>> way to configure that from Linux. For more details, see the "Bus Matrix
>>>>> (MATRIX)" section in Atmel datasheets.    
>>>>
>>>> I don't seem to succeed in changing the registers I think I need to change.
>>>> I can poke the "Write Protection Mode Register" by writing MAT0 and MAT1 to
>>>> it.  
>>>
>>> You mean 0x4D415400, right? ("MAT0" != 0x4D415400).  
>>
>> Bits 1 through 7 do not matter, so even though not equal they are (or
>> should be) equivalent. But I did use 0x4d415400. I simply used the
>> shorter syntax since that was easier to type and conveyed the relevant
>> info.
> 
> Ok.
> 
>>
>>>> But when I try to write to "Priority Registers B For Slaves" it doesn't
>>>> take, regardless of write protect mode.  
>>>
>>> Did you check MATRIX_WPSR after writing to MATRIX_PRXSY?  
>>
>> No, but did it again and checked, see transcript below.
> 
> I don't use devmem2. Is 'readback' information accurate or is it
> always what's been written? Because when you write 0x33 to 0xFFFFECBC,
> 0x33 is read back, but just after that, when you read it again it's 0.

Looking at the devmem2 source, it seems very likely that the compiler
optimizes out the read and thus outputs what has been written.

>> BTW, how do I
>> know which master is in use for the LCD controller? 8 or 9? Both?
> 
> It's configurable on a per-layer basis through the SIF bit in
> LCDC_<layer>CFG0. The driver tries to dispatch the load on those 2 AHB
> masters [1].

Ok, I only have one plane (in this case, i.e. no cursor, no overlays etc),
would that mean that only one master is used?

>> And
>> which DDR slave is the target? 7, 8, 9 or 10? More than one?
> 
> This, I don't know. I guess all of them can be used.

Right.

Re: [PATCH] mtd: nand: raw: atmel: add module param to avoid using dma

[Boris Brezillon]

On Mon, 2 Apr 2018 22:23:17 +0200
Peter Rosin <peda@axentia.se> wrote:


> > I don't use devmem2. Is 'readback' information accurate or is it
> > always what's been written? Because when you write 0x33 to 0xFFFFECBC,
> > 0x33 is read back, but just after that, when you read it again it's 0.  
> 
> Looking at the devmem2 source, it seems very likely that the compiler
> optimizes out the read and thus outputs what has been written.

Yep, had a look too, and it's missing a volatile specifier to prevent
that sort of optimizations.

> 
> >> BTW, how do I
> >> know which master is in use for the LCD controller? 8 or 9? Both?  
> > 
> > It's configurable on a per-layer basis through the SIF bit in
> > LCDC_<layer>CFG0. The driver tries to dispatch the load on those 2 AHB
> > masters [1].  
> 
> Ok, I only have one plane (in this case, i.e. no cursor, no overlays etc),
> would that mean that only one master is used?

Yep, it's always using the first one (master 8 on a sama5d3).

-- 
Boris Brezillon, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH] mtd: nand: raw: atmel: add module param to avoid using dma

[Alexandre Belloni]

On 02/04/2018 at 22:23:17 +0200, Peter Rosin wrote:
> >> No, but did it again and checked, see transcript below.
> > 
> > I don't use devmem2. Is 'readback' information accurate or is it
> > always what's been written? Because when you write 0x33 to 0xFFFFECBC,
> > 0x33 is read back, but just after that, when you read it again it's 0.
> 
> Looking at the devmem2 source, it seems very likely that the compiler
> optimizes out the read and thus outputs what has been written.
> 

At least on x86, it is not optimized out.


-- 
Alexandre Belloni, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH] mtd: nand: raw: atmel: add module param to avoid using dma

[Peter Rosin]

On 2018-04-03 09:18, Alexandre Belloni wrote:
> On 02/04/2018 at 22:23:17 +0200, Peter Rosin wrote:
>>>> No, but did it again and checked, see transcript below.
>>>
>>> I don't use devmem2. Is 'readback' information accurate or is it
>>> always what's been written? Because when you write 0x33 to 0xFFFFECBC,
>>> 0x33 is read back, but just after that, when you read it again it's 0.
>>
>> Looking at the devmem2 source, it seems very likely that the compiler
>> optimizes out the read and thus outputs what has been written.
>>
> 
> At least on x86, it is not optimized out.

Looking at the disassembly, they are gone here (not that I'm fluent).
I then tried to compile devmem2 with -O0 (instead of -Os) and the read
is then fine. So, I guess the devmem2 devs could claim that it's a
buildroot issue, but a volatile would certainly have helped...

Cheers,
Peter

Re: [PATCH] mtd: nand: raw: atmel: add module param to avoid using dma

[Nicolas Ferre]

On 29/03/2018 at 15:10, Peter Rosin wrote:
> On a sama5d31 with a Full-HD dual LVDS panel (132MHz pixel clock) NAND
> flash accesses have a tendency to cause display disturbances. Add a
> module param to disable DMA from the NAND controller, since that fixes
> the display problem for me.
> 
> Signed-off-by: Peter Rosin <peda@axentia.se>
> ---
>   drivers/mtd/nand/raw/atmel/nand-controller.c | 7 ++++++-
>   1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/nand/raw/atmel/nand-controller.c b/drivers/mtd/nand/raw/atmel/nand-controller.c
> index b2f00b398490..2ff7a77c7b8e 100644
> --- a/drivers/mtd/nand/raw/atmel/nand-controller.c
> +++ b/drivers/mtd/nand/raw/atmel/nand-controller.c
> @@ -129,6 +129,11 @@
>   #define DEFAULT_TIMEOUT_MS			1000
>   #define MIN_DMA_LEN				128
>   
> +static bool atmel_nand_avoid_dma __read_mostly;
> +
> +MODULE_PARM_DESC(avoiddma, "Avoid using DMA");
> +module_param_named(avoiddma, atmel_nand_avoid_dma, bool, 0400);

We had the DT parameter "atmel,nand-has-dma" for this purpose and it was 
associated with a module parameter as well (use_dma,).

Is it only managed by old DT binding?

Best regards,
   Nicoals

>   enum atmel_nand_rb_type {
>   	ATMEL_NAND_NO_RB,
>   	ATMEL_NAND_NATIVE_RB,
> @@ -1977,7 +1982,7 @@ static int atmel_nand_controller_init(struct atmel_nand_controller *nc,
>   		return ret;
>   	}
>   
> -	if (nc->caps->has_dma) {
> +	if (nc->caps->has_dma && !atmel_nand_avoid_dma) {
>   		dma_cap_mask_t mask;
>   
>   		dma_cap_zero(mask);
> 


-- 
Nicolas Ferre

Re: [PATCH] mtd: nand: raw: atmel: add module param to avoid using dma

[Peter Rosin]

On 2018-03-29 16:20, Nicolas Ferre wrote:
> On 29/03/2018 at 15:10, Peter Rosin wrote:
>> On a sama5d31 with a Full-HD dual LVDS panel (132MHz pixel clock) NAND
>> flash accesses have a tendency to cause display disturbances. Add a
>> module param to disable DMA from the NAND controller, since that fixes
>> the display problem for me.
>>
>> Signed-off-by: Peter Rosin <peda@axentia.se>
>> ---
>>   drivers/mtd/nand/raw/atmel/nand-controller.c | 7 ++++++-
>>   1 file changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/mtd/nand/raw/atmel/nand-controller.c b/drivers/mtd/nand/raw/atmel/nand-controller.c
>> index b2f00b398490..2ff7a77c7b8e 100644
>> --- a/drivers/mtd/nand/raw/atmel/nand-controller.c
>> +++ b/drivers/mtd/nand/raw/atmel/nand-controller.c
>> @@ -129,6 +129,11 @@
>>   #define DEFAULT_TIMEOUT_MS			1000
>>   #define MIN_DMA_LEN				128
>>   
>> +static bool atmel_nand_avoid_dma __read_mostly;
>> +
>> +MODULE_PARM_DESC(avoiddma, "Avoid using DMA");
>> +module_param_named(avoiddma, atmel_nand_avoid_dma, bool, 0400);
> 
> We had the DT parameter "atmel,nand-has-dma" for this purpose and it was 
> associated with a module parameter as well (use_dma,).
> 
> Is it only managed by old DT binding?

Saw it, but I need the reverse. I.e. *not* using DMA.

And I didn't think this belonged in DT since in some sense it doesn't really
describe HW so I went with a module parameter.

Cheers,
Peter

Re: [PATCH] mtd: nand: raw: atmel: add module param to avoid using dma

[Boris Brezillon]

On Thu, 29 Mar 2018 16:20:38 +0200
Nicolas Ferre <nicolas.ferre@microchip.com> wrote:

> On 29/03/2018 at 15:10, Peter Rosin wrote:
> > On a sama5d31 with a Full-HD dual LVDS panel (132MHz pixel clock) NAND
> > flash accesses have a tendency to cause display disturbances. Add a
> > module param to disable DMA from the NAND controller, since that fixes
> > the display problem for me.
> > 
> > Signed-off-by: Peter Rosin <peda@axentia.se>
> > ---
> >   drivers/mtd/nand/raw/atmel/nand-controller.c | 7 ++++++-
> >   1 file changed, 6 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/mtd/nand/raw/atmel/nand-controller.c b/drivers/mtd/nand/raw/atmel/nand-controller.c
> > index b2f00b398490..2ff7a77c7b8e 100644
> > --- a/drivers/mtd/nand/raw/atmel/nand-controller.c
> > +++ b/drivers/mtd/nand/raw/atmel/nand-controller.c
> > @@ -129,6 +129,11 @@
> >   #define DEFAULT_TIMEOUT_MS			1000
> >   #define MIN_DMA_LEN				128
> >   
> > +static bool atmel_nand_avoid_dma __read_mostly;
> > +
> > +MODULE_PARM_DESC(avoiddma, "Avoid using DMA");
> > +module_param_named(avoiddma, atmel_nand_avoid_dma, bool, 0400);  
> 
> We had the DT parameter "atmel,nand-has-dma" for this purpose and it was

Actually, that was not my understanding of this property. To me it
sounds like something to flag whether the platform supports DMA or
not, no whether it should be enabled on a per-board basis.

> associated with a module parameter as well (use_dma,).

That's true for the use_dma module param, I didn't add it back when
reworking the driver.

> 
> Is it only managed by old DT binding?

It is. I'm really not in favor of adding the DT prop back, but let's
re-introduce the module param if you think it's needed. BTW, the module
name has changed, so the param name will be different.

-- 
Boris Brezillon, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH] mtd: nand: raw: atmel: add module param to avoid using dma

[Boris Brezillon]

On Thu, 29 Mar 2018 15:10:54 +0200
Peter Rosin <peda@axentia.se> wrote:

> On a sama5d31 with a Full-HD dual LVDS panel (132MHz pixel clock) NAND
> flash accesses have a tendency to cause display disturbances. Add a
> module param to disable DMA from the NAND controller, since that fixes
> the display problem for me.
> 
> Signed-off-by: Peter Rosin <peda@axentia.se>

Reviewed-by: Boris Brezillon <boris.brezillon@bootlin.com>

Miquel, can you queue this one to nand/next.

> ---
>  drivers/mtd/nand/raw/atmel/nand-controller.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/nand/raw/atmel/nand-controller.c b/drivers/mtd/nand/raw/atmel/nand-controller.c
> index b2f00b398490..2ff7a77c7b8e 100644
> --- a/drivers/mtd/nand/raw/atmel/nand-controller.c
> +++ b/drivers/mtd/nand/raw/atmel/nand-controller.c
> @@ -129,6 +129,11 @@
>  #define DEFAULT_TIMEOUT_MS			1000
>  #define MIN_DMA_LEN				128
>  
> +static bool atmel_nand_avoid_dma __read_mostly;
> +
> +MODULE_PARM_DESC(avoiddma, "Avoid using DMA");
> +module_param_named(avoiddma, atmel_nand_avoid_dma, bool, 0400);
> +
>  enum atmel_nand_rb_type {
>  	ATMEL_NAND_NO_RB,
>  	ATMEL_NAND_NATIVE_RB,
> @@ -1977,7 +1982,7 @@ static int atmel_nand_controller_init(struct atmel_nand_controller *nc,
>  		return ret;
>  	}
>  
> -	if (nc->caps->has_dma) {
> +	if (nc->caps->has_dma && !atmel_nand_avoid_dma) {
>  		dma_cap_mask_t mask;
>  
>  		dma_cap_zero(mask);

Re: [PATCH] mtd: nand: raw: atmel: add module param to avoid using dma

[Miquel Raynal]

Hi Boris,

On Mon, 18 Jun 2018 10:39:08 +0200, Boris Brezillon
<boris.brezillon@bootlin.com> wrote:

> On Thu, 29 Mar 2018 15:10:54 +0200
> Peter Rosin <peda@axentia.se> wrote:
> 
> > On a sama5d31 with a Full-HD dual LVDS panel (132MHz pixel clock) NAND
> > flash accesses have a tendency to cause display disturbances. Add a
> > module param to disable DMA from the NAND controller, since that fixes
> > the display problem for me.
> > 
> > Signed-off-by: Peter Rosin <peda@axentia.se>  
> 
> Reviewed-by: Boris Brezillon <boris.brezillon@bootlin.com>
> 
> Miquel, can you queue this one to nand/next.

Yes of course.

I'll just change the subject to "mtd: rawnand: atmel:".

Thanks,
Miquèl

Re: [PATCH] mtd: nand: raw: atmel: add module param to avoid using dma

[Miquel Raynal]

Hi Peter,

On Mon, 18 Jun 2018 10:39:08 +0200, Boris Brezillon
<boris.brezillon@bootlin.com> wrote:

> On Thu, 29 Mar 2018 15:10:54 +0200
> Peter Rosin <peda@axentia.se> wrote:
> 
> > On a sama5d31 with a Full-HD dual LVDS panel (132MHz pixel clock) NAND
> > flash accesses have a tendency to cause display disturbances. Add a
> > module param to disable DMA from the NAND controller, since that fixes
> > the display problem for me.
> > 
> > Signed-off-by: Peter Rosin <peda@axentia.se>  
> 
> Reviewed-by: Boris Brezillon <boris.brezillon@bootlin.com>
> 
> Miquel, can you queue this one to nand/next.
> 

Applied to nand/next with the prefix changed to "mtd: rawnand: atmel:".

Thanks,
Miquèl