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

Commit Message

Peter Rosin March 29, 2018, 1:10 p.m. UTC

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 <[e-mail]>
---
 drivers/mtd/nand/raw/atmel/nand-controller.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

Comments

Boris Brezillon March 29, 2018, 1:33 p.m. UTC | #1

On Thu, 29 Mar 2018 15:10:54 +0200
Peter Rosin <[e-mail]> 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 <[e-mail]>
> ---
>  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);

Peter Rosin March 29, 2018, 1:37 p.m. UTC | #2

On 2018-03-29 15:33, Boris Brezillon wrote:
> On Thu, 29 Mar 2018 15:10:54 +0200
> Peter Rosin <[e-mail]> 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 <[e-mail]>
>> ---
>>  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

Boris Brezillon March 29, 2018, 1:44 p.m. UTC | #3

On Thu, 29 Mar 2018 15:37:43 +0200
Peter Rosin <[e-mail]> wrote:

> On 2018-03-29 15:33, Boris Brezillon wrote:
> > On Thu, 29 Mar 2018 15:10:54 +0200
> > Peter Rosin <[e-mail]> 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 <[e-mail]>
> >> ---
> >>  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.

Nicolas Ferre March 29, 2018, 2:20 p.m. UTC | #4

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 <[e-mail]>
> ---
>   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);
>

Peter Rosin March 29, 2018, 2:23 p.m. UTC | #5

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 <[e-mail]>
>> ---
>>   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

Peter Rosin March 29, 2018, 2:27 p.m. UTC | #6

On 2018-03-29 15:44, Boris Brezillon wrote:
> On Thu, 29 Mar 2018 15:37:43 +0200
> Peter Rosin <[e-mail]> wrote:
> 
>> On 2018-03-29 15:33, Boris Brezillon wrote:
>>> On Thu, 29 Mar 2018 15:10:54 +0200
>>> Peter Rosin <[e-mail]> 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 <[e-mail]>
>>>> ---
>>>>  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

Boris Brezillon March 29, 2018, 2:29 p.m. UTC | #7

On Thu, 29 Mar 2018 16:20:38 +0200
Nicolas Ferre <[e-mail]> 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 <[e-mail]>
> > ---
> >   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.

Peter Rosin March 30, 2018, 9:43 p.m. UTC | #8

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 <[e-mail]> wrote:
>>
>>> On 2018-03-29 15:33, Boris Brezillon wrote:
>>>> On Thu, 29 Mar 2018 15:10:54 +0200
>>>> Peter Rosin <[e-mail]> 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 <[e-mail]>
>>>>> ---
>>>>>  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

Boris Brezillon April 2, 2018, 12:22 p.m. UTC | #9

On Thu, 29 Mar 2018 16:27:12 +0200
Peter Rosin <[e-mail]> wrote:

> On 2018-03-29 15:44, Boris Brezillon wrote:
> > On Thu, 29 Mar 2018 15:37:43 +0200
> > Peter Rosin <[e-mail]> wrote:
> >   
> >> On 2018-03-29 15:33, Boris Brezillon wrote:  
> >>> On Thu, 29 Mar 2018 15:10:54 +0200
> >>> Peter Rosin <[e-mail]> 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 <[e-mail]>
> >>>> ---
> >>>>  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.

Peter Rosin April 2, 2018, 5:59 p.m. UTC | #10

On 2018-04-02 14:22, Boris Brezillon wrote:
> On Thu, 29 Mar 2018 16:27:12 +0200
> Peter Rosin <[e-mail]> wrote:
> 
>> On 2018-03-29 15:44, Boris Brezillon wrote:
>>> On Thu, 29 Mar 2018 15:37:43 +0200
>>> Peter Rosin <[e-mail]> wrote:
>>>   
>>>> On 2018-03-29 15:33, Boris Brezillon wrote:  
>>>>> On Thu, 29 Mar 2018 15:10:54 +0200
>>>>> Peter Rosin <[e-mail]> 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 <[e-mail]>
>>>>>> ---
>>>>>>  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

Boris Brezillon April 2, 2018, 7:28 p.m. UTC | #11

On Mon, 2 Apr 2018 19:59:39 +0200
Peter Rosin <[e-mail]> wrote:

> On 2018-04-02 14:22, Boris Brezillon wrote:
> > On Thu, 29 Mar 2018 16:27:12 +0200
> > Peter Rosin <[e-mail]> wrote:
> >   
> >> On 2018-03-29 15:44, Boris Brezillon wrote:  
> >>> On Thu, 29 Mar 2018 15:37:43 +0200
> >>> Peter Rosin <[e-mail]> wrote:
> >>>     
> >>>> On 2018-03-29 15:33, Boris Brezillon wrote:    
> >>>>> On Thu, 29 Mar 2018 15:10:54 +0200
> >>>>> Peter Rosin <[e-mail]> 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 <[e-mail]>
> >>>>>> ---
> >>>>>>  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 April 2, 2018, 8:20 p.m. UTC | #12

On Mon, 2 Apr 2018 21:28:43 +0200
Boris Brezillon <[e-mail]> wrote:

> On Mon, 2 Apr 2018 19:59:39 +0200
> Peter Rosin <[e-mail]> wrote:
> 
> > On 2018-04-02 14:22, Boris Brezillon wrote:  
> > > On Thu, 29 Mar 2018 16:27:12 +0200
> > > Peter Rosin <[e-mail]> wrote:
> > >     
> > >> On 2018-03-29 15:44, Boris Brezillon wrote:    
> > >>> On Thu, 29 Mar 2018 15:37:43 +0200
> > >>> Peter Rosin <[e-mail]> wrote:
> > >>>       
> > >>>> On 2018-03-29 15:33, Boris Brezillon wrote:      
> > >>>>> On Thu, 29 Mar 2018 15:10:54 +0200
> > >>>>> Peter Rosin <[e-mail]> 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 <[e-mail]>
> > >>>>>> ---
> > >>>>>>  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
>

Peter Rosin April 2, 2018, 8:23 p.m. UTC | #13

On 2018-04-02 21:28, Boris Brezillon wrote:
> On Mon, 2 Apr 2018 19:59:39 +0200
> Peter Rosin <[e-mail]> wrote:
> 
>> On 2018-04-02 14:22, Boris Brezillon wrote:
>>> On Thu, 29 Mar 2018 16:27:12 +0200
>>> Peter Rosin <[e-mail]> wrote:
>>>   
>>>> On 2018-03-29 15:44, Boris Brezillon wrote:  
>>>>> On Thu, 29 Mar 2018 15:37:43 +0200
>>>>> Peter Rosin <[e-mail]> wrote:
>>>>>     
>>>>>> On 2018-03-29 15:33, Boris Brezillon wrote:    
>>>>>>> On Thu, 29 Mar 2018 15:10:54 +0200
>>>>>>> Peter Rosin <[e-mail]> 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 <[e-mail]>
>>>>>>>> ---
>>>>>>>>  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.

Boris Brezillon April 2, 2018, 8:32 p.m. UTC | #14

On Mon, 2 Apr 2018 22:20:20 +0200
Boris Brezillon <[e-mail]> 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 April 2, 2018, 8:35 p.m. UTC | #15

On Mon, 2 Apr 2018 22:23:17 +0200
Peter Rosin <[e-mail]> 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).

Peter Rosin April 3, 2018, 6:11 a.m. UTC | #16

On 2018-04-02 22:20, Boris Brezillon wrote:
> On Mon, 2 Apr 2018 21:28:43 +0200
> Boris Brezillon <[e-mail]> wrote:
> 
>> On Mon, 2 Apr 2018 19:59:39 +0200
>> Peter Rosin <[e-mail]> wrote:
>>
>>> On 2018-04-02 14:22, Boris Brezillon wrote:  
>>>> On Thu, 29 Mar 2018 16:27:12 +0200
>>>> Peter Rosin <[e-mail]> wrote:
>>>>     
>>>>> On 2018-03-29 15:44, Boris Brezillon wrote:    
>>>>>> On Thu, 29 Mar 2018 15:37:43 +0200
>>>>>> Peter Rosin <[e-mail]> wrote:
>>>>>>       
>>>>>>> On 2018-03-29 15:33, Boris Brezillon wrote:      
>>>>>>>> On Thu, 29 Mar 2018 15:10:54 +0200
>>>>>>>> Peter Rosin <[e-mail]> 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 <[e-mail]>
>>>>>>>>> ---
>>>>>>>>>  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

Peter Rosin April 3, 2018, 6:51 a.m. UTC | #17

On 2018-04-02 22:20, Boris Brezillon wrote:
> On Mon, 2 Apr 2018 21:28:43 +0200
> Boris Brezillon <[e-mail]> wrote:
> 
>> On Mon, 2 Apr 2018 19:59:39 +0200
>> Peter Rosin <[e-mail]> wrote:
>>
>>> On 2018-04-02 14:22, Boris Brezillon wrote:  
>>>> On Thu, 29 Mar 2018 16:27:12 +0200
>>>> Peter Rosin <[e-mail]> wrote:
>>>>     
>>>>> On 2018-03-29 15:44, Boris Brezillon wrote:    
>>>>>> On Thu, 29 Mar 2018 15:37:43 +0200
>>>>>> Peter Rosin <[e-mail]> wrote:
>>>>>>       
>>>>>>> On 2018-03-29 15:33, Boris Brezillon wrote:      
>>>>>>>> On Thu, 29 Mar 2018 15:10:54 +0200
>>>>>>>> Peter Rosin <[e-mail]> 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 <[e-mail]>
>>>>>>>>> ---
>>>>>>>>>  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

Boris Brezillon April 3, 2018, 7:15 a.m. UTC | #18

On Tue, 3 Apr 2018 08:51:10 +0200
Peter Rosin <[e-mail]> wrote:

> On 2018-04-02 22:20, Boris Brezillon wrote:
> > On Mon, 2 Apr 2018 21:28:43 +0200
> > Boris Brezillon <[e-mail]> wrote:
> >   
> >> On Mon, 2 Apr 2018 19:59:39 +0200
> >> Peter Rosin <[e-mail]> wrote:
> >>  
> >>> On 2018-04-02 14:22, Boris Brezillon wrote:    
> >>>> On Thu, 29 Mar 2018 16:27:12 +0200
> >>>> Peter Rosin <[e-mail]> wrote:
> >>>>       
> >>>>> On 2018-03-29 15:44, Boris Brezillon wrote:      
> >>>>>> On Thu, 29 Mar 2018 15:37:43 +0200
> >>>>>> Peter Rosin <[e-mail]> wrote:
> >>>>>>         
> >>>>>>> On 2018-03-29 15:33, Boris Brezillon wrote:        
> >>>>>>>> On Thu, 29 Mar 2018 15:10:54 +0200
> >>>>>>>> Peter Rosin <[e-mail]> 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 <[e-mail]>
> >>>>>>>>> ---
> >>>>>>>>>  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 April 3, 2018, 7:18 a.m. UTC | #19

On Tue, 3 Apr 2018 08:11:30 +0200
Peter Rosin <[e-mail]> wrote:

> On 2018-04-02 22:20, Boris Brezillon wrote:
> > On Mon, 2 Apr 2018 21:28:43 +0200
> > Boris Brezillon <[e-mail]> wrote:
> >   
> >> On Mon, 2 Apr 2018 19:59:39 +0200
> >> Peter Rosin <[e-mail]> wrote:
> >>  
> >>> On 2018-04-02 14:22, Boris Brezillon wrote:    
> >>>> On Thu, 29 Mar 2018 16:27:12 +0200
> >>>> Peter Rosin <[e-mail]> wrote:
> >>>>       
> >>>>> On 2018-03-29 15:44, Boris Brezillon wrote:      
> >>>>>> On Thu, 29 Mar 2018 15:37:43 +0200
> >>>>>> Peter Rosin <[e-mail]> wrote:
> >>>>>>         
> >>>>>>> On 2018-03-29 15:33, Boris Brezillon wrote:        
> >>>>>>>> On Thu, 29 Mar 2018 15:10:54 +0200
> >>>>>>>> Peter Rosin <[e-mail]> 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 <[e-mail]>
> >>>>>>>>> ---
> >>>>>>>>>  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

Alexandre Belloni April 3, 2018, 7:18 a.m. UTC | #20

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.

Boris Brezillon April 3, 2018, 7:32 a.m. UTC | #21

On Tue, 3 Apr 2018 09:15:22 +0200
Boris Brezillon <[e-mail]> 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.

Peter Rosin April 3, 2018, 8:14 a.m. UTC | #22

On 2018-04-03 09:15, Boris Brezillon wrote:
> On Tue, 3 Apr 2018 08:51:10 +0200
> Peter Rosin <[e-mail]> wrote:
> 
>> On 2018-04-02 22:20, Boris Brezillon wrote:
>>> On Mon, 2 Apr 2018 21:28:43 +0200
>>> Boris Brezillon <[e-mail]> wrote:
>>>   
>>>> On Mon, 2 Apr 2018 19:59:39 +0200
>>>> Peter Rosin <[e-mail]> wrote:
>>>>  
>>>>> On 2018-04-02 14:22, Boris Brezillon wrote:    
>>>>>> On Thu, 29 Mar 2018 16:27:12 +0200
>>>>>> Peter Rosin <[e-mail]> wrote:
>>>>>>       
>>>>>>> On 2018-03-29 15:44, Boris Brezillon wrote:      
>>>>>>>> On Thu, 29 Mar 2018 15:37:43 +0200
>>>>>>>> Peter Rosin <[e-mail]> wrote:
>>>>>>>>         
>>>>>>>>> On 2018-03-29 15:33, Boris Brezillon wrote:        
>>>>>>>>>> On Thu, 29 Mar 2018 15:10:54 +0200
>>>>>>>>>> Peter Rosin <[e-mail]> 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 <[e-mail]>
>>>>>>>>>>> ---
>>>>>>>>>>>  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

Boris Brezillon April 3, 2018, 8:30 a.m. UTC | #23

On Tue, 3 Apr 2018 10:14:31 +0200
Peter Rosin <[e-mail]> wrote:

> On 2018-04-03 09:15, Boris Brezillon wrote:
> > On Tue, 3 Apr 2018 08:51:10 +0200
> > Peter Rosin <[e-mail]> wrote:
> >   
> >> On 2018-04-02 22:20, Boris Brezillon wrote:  
> >>> On Mon, 2 Apr 2018 21:28:43 +0200
> >>> Boris Brezillon <[e-mail]> wrote:
> >>>     
> >>>> On Mon, 2 Apr 2018 19:59:39 +0200
> >>>> Peter Rosin <[e-mail]> wrote:
> >>>>    
> >>>>> On 2018-04-02 14:22, Boris Brezillon wrote:      
> >>>>>> On Thu, 29 Mar 2018 16:27:12 +0200
> >>>>>> Peter Rosin <[e-mail]> wrote:
> >>>>>>         
> >>>>>>> On 2018-03-29 15:44, Boris Brezillon wrote:        
> >>>>>>>> On Thu, 29 Mar 2018 15:37:43 +0200
> >>>>>>>> Peter Rosin <[e-mail]> wrote:
> >>>>>>>>           
> >>>>>>>>> On 2018-03-29 15:33, Boris Brezillon wrote:          
> >>>>>>>>>> On Thu, 29 Mar 2018 15:10:54 +0200
> >>>>>>>>>> Peter Rosin <[e-mail]> 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 <[e-mail]>
> >>>>>>>>>>> ---
> >>>>>>>>>>>  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.

Peter Rosin April 3, 2018, 8:37 a.m. UTC | #24

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

Peter Rosin April 11, 2018, 2:44 p.m. UTC | #25

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 <[e-mail]> wrote:
> 
>> On 2018-04-02 22:20, Boris Brezillon wrote:
>>> On Mon, 2 Apr 2018 21:28:43 +0200
>>> Boris Brezillon <[e-mail]> wrote:
>>>   
>>>> On Mon, 2 Apr 2018 19:59:39 +0200
>>>> Peter Rosin <[e-mail]> wrote:
>>>>  
>>>>> On 2018-04-02 14:22, Boris Brezillon wrote:    
>>>>>> On Thu, 29 Mar 2018 16:27:12 +0200
>>>>>> Peter Rosin <[e-mail]> wrote:
>>>>>>       
>>>>>>> On 2018-03-29 15:44, Boris Brezillon wrote:      
>>>>>>>> On Thu, 29 Mar 2018 15:37:43 +0200
>>>>>>>> Peter Rosin <[e-mail]> wrote:
>>>>>>>>         
>>>>>>>>> On 2018-03-29 15:33, Boris Brezillon wrote:        
>>>>>>>>>> On Thu, 29 Mar 2018 15:10:54 +0200
>>>>>>>>>> Peter Rosin <[e-mail]> 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 <[e-mail]>
>>>>>>>>>>> ---
>>>>>>>>>>>  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).

Boris Brezillon April 11, 2018, 2:59 p.m. UTC | #26

On Wed, 11 Apr 2018 16:44:10 +0200
Peter Rosin <[e-mail]> 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?

Peter Rosin April 11, 2018, 3:10 p.m. UTC | #27

On 2018-04-11 16:59, Boris Brezillon wrote:
> On Wed, 11 Apr 2018 16:44:10 +0200
> Peter Rosin <[e-mail]> 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

Boris Brezillon April 11, 2018, 3:34 p.m. UTC | #28

On Wed, 11 Apr 2018 17:10:43 +0200
Peter Rosin <[e-mail]> wrote:

> On 2018-04-11 16:59, Boris Brezillon wrote:
> > On Wed, 11 Apr 2018 16:44:10 +0200
> > Peter Rosin <[e-mail]> 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.

Nicolas Ferre April 11, 2018, 3:34 p.m. UTC | #29

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 <[e-mail]> wrote:
>>
>>> On 2018-04-02 22:20, Boris Brezillon wrote:
>>>> On Mon, 2 Apr 2018 21:28:43 +0200
>>>> Boris Brezillon <[e-mail]> wrote:
>>>>    
>>>>> On Mon, 2 Apr 2018 19:59:39 +0200
>>>>> Peter Rosin <[e-mail]> wrote:
>>>>>   
>>>>>> On 2018-04-02 14:22, Boris Brezillon wrote:
>>>>>>> On Thu, 29 Mar 2018 16:27:12 +0200
>>>>>>> Peter Rosin <[e-mail]> wrote:
>>>>>>>        
>>>>>>>> On 2018-03-29 15:44, Boris Brezillon wrote:
>>>>>>>>> On Thu, 29 Mar 2018 15:37:43 +0200
>>>>>>>>> Peter Rosin <[e-mail]> wrote:
>>>>>>>>>          
>>>>>>>>>> On 2018-03-29 15:33, Boris Brezillon wrote:
>>>>>>>>>>> On Thu, 29 Mar 2018 15:10:54 +0200
>>>>>>>>>>> Peter Rosin <[e-mail]> 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 <[e-mail]>
>>>>>>>>>>>> ---
>>>>>>>>>>>>   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).
>

Peter Rosin April 12, 2018, 7:18 a.m. UTC | #30

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 <[e-mail]> wrote:
>>>
>>>> On 2018-04-02 22:20, Boris Brezillon wrote:
>>>>> On Mon, 2 Apr 2018 21:28:43 +0200
>>>>> Boris Brezillon <[e-mail]> wrote:
>>>>>    
>>>>>> On Mon, 2 Apr 2018 19:59:39 +0200
>>>>>> Peter Rosin <[e-mail]> wrote:
>>>>>>   
>>>>>>> On 2018-04-02 14:22, Boris Brezillon wrote:
>>>>>>>> On Thu, 29 Mar 2018 16:27:12 +0200
>>>>>>>> Peter Rosin <[e-mail]> wrote:
>>>>>>>>        
>>>>>>>>> On 2018-03-29 15:44, Boris Brezillon wrote:
>>>>>>>>>> On Thu, 29 Mar 2018 15:37:43 +0200
>>>>>>>>>> Peter Rosin <[e-mail]> wrote:
>>>>>>>>>>          
>>>>>>>>>>> On 2018-03-29 15:33, Boris Brezillon wrote:
>>>>>>>>>>>> On Thu, 29 Mar 2018 15:10:54 +0200
>>>>>>>>>>>> Peter Rosin <[e-mail]> 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 <[e-mail]>
>>>>>>>>>>>>> ---
>>>>>>>>>>>>>   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).
>>
> 
>

Peter Rosin May 22, 2018, 6:03 p.m. UTC | #31

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

Boris Brezillon May 23, 2018, 10:42 a.m. UTC | #32

On Tue, 22 May 2018 20:03:41 +0200
Peter Rosin <[e-mail]> 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.

Tudor Ambarus May 25, 2018, 2:51 p.m. UTC | #33

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

Peter Rosin May 26, 2018, 5:40 p.m. UTC | #34

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

Peter Rosin May 27, 2018, 9:18 a.m. UTC | #35

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

Boris Brezillon June 18, 2018, 8:39 a.m. UTC | #36

On Thu, 29 Mar 2018 15:10:54 +0200
Peter Rosin <[e-mail]> 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 <[e-mail]>

Reviewed-by: Boris Brezillon <[e-mail]>

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);

Miquel Raynal June 18, 2018, 2 p.m. UTC | #37

Hi Boris,

On Mon, 18 Jun 2018 10:39:08 +0200, Boris Brezillon
<[e-mail]> wrote:

> On Thu, 29 Mar 2018 15:10:54 +0200
> Peter Rosin <[e-mail]> 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 <[e-mail]>  
> 
> Reviewed-by: Boris Brezillon <[e-mail]>
> 
> 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

Miquel Raynal June 25, 2018, 12:31 p.m. UTC | #38

Hi Peter,

On Mon, 18 Jun 2018 10:39:08 +0200, Boris Brezillon
<[e-mail]> wrote:

> On Thu, 29 Mar 2018 15:10:54 +0200
> Peter Rosin <[e-mail]> 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 <[e-mail]>  
> 
> Reviewed-by: Boris Brezillon <[e-mail]>
> 
> Miquel, can you queue this one to nand/next.
> 

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

Thanks,
Miquèl

Ahmad Fatoum June 16, 2022, 3:54 p.m. UTC | #39

Hello Peter,

Hardware has found its way to my desk that has the same issue
that you experienced. Disabling NAND DMA also fixed the display
FIFO underflow, but on another variant, it didn't help and we
need to dig deeper. Were there perhaps any new findings since
this thread concluded in 2018?

On 25.05.18 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.

@Tudor, @Nicolas, Like Peter, I also have a lot of 3s (i.e. highest
priority for all DMA masters with access to a given slave) preset in
my matrix configuration. It seems not possible to change these priorities.
I have yet to try more of the suggestions in this thread. Has there been
a reply from your hardware team, if there are constraints regarding when
it's allowed to change MATRIX_PRBS9/10 on the SAMA5D3?

Thanks in advance,
Ahmad


> 
> 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

Ahmad Fatoum July 25, 2022, 2:17 p.m. UTC | #40

Hello everyone,

On 16.06.22 17:54, Ahmad Fatoum wrote:
> Hello Peter,
> 
> Hardware has found its way to my desk that has the same issue
> that you experienced. Disabling NAND DMA also fixed the display
> FIFO underflow, but on another variant, it didn't help and we
> need to dig deeper. Were there perhaps any new findings since
> this thread concluded in 2018?

We've since looked deeper into this and found that changing the
AHB port of the LCDC worked around the issue. The other AHB port
connects to a different DDR port and presumably the DDR port
scheduling is less disadvantageous to the LCDC than the
interconnect arbitration on the DDR port shared with the
DMA controller used for NAND.

> On 25.05.18 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.
> 
> @Tudor, @Nicolas, Like Peter, I also have a lot of 3s (i.e. highest
> priority for all DMA masters with access to a given slave) preset in
> my matrix configuration. It seems not possible to change these priorities.

I tested again and changing priorities was possible, just not for
the first LCDC master. That one was at the lowest priority and
the best one can do is decreasing other master priorities, but
this did not resolve the issue.

I also tried other things like breaking burst length and setting
default master for the port, but nothing I configured in the MATRIX
registers helped.

> I have yet to try more of the suggestions in this thread. Has there been
> a reply from your hardware team, if there are constraints regarding when
> it's allowed to change MATRIX_PRBS9/10 on the SAMA5D3?

While I have a workaround now, the proper way would still be configuring
PRBS9. @Tudor, @Nicolas, do you know why changing the LCDC bit in that
register is not possible on the SAMA5D3? I tried setting it prior to
Linux boot (and LCDC enabling), but it did not help.

Thanks,
Ahmad

> 
> Thanks in advance,
> Ahmad
> 
> 
>>
>> 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
> 
>

Tudor Ambarus July 28, 2022, 8:03 a.m. UTC | #41

On 7/25/22 17:17, Ahmad Fatoum wrote:
> @Tudor, @Nicolas, do you know why changing the LCDC bit in that
> register is not possible on the SAMA5D3? I tried setting it prior to
> Linux boot (and LCDC enabling), but it did not help.

Hi, Ahmad,

I don't, but I plan to look into this. Until then, would you please
retest on your side with the following patch applied?

https://lore.kernel.org/linux-mtd/[e-mail]/T/#u

Patch

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);