OMAP4430 produces boot warnings

OMAP4430 produces boot warnings

[Russell King - ARM Linux]

This one is nice and long, from last nights boot test.  Looks like it was
introduced sometime in the last couple of weeks.  Full log at:

http://www.arm.linux.org.uk/developer/build/result.php?type=boot&idx=518

and config:
http://www.arm.linux.org.uk/developer/build/file.php?type=config&idx=2786

taal display1: taal panel revision e3.83.7d
------------[ cut here ]------------
WARNING: at drivers/bus/omap_l3_noc.c:97 l3_interrupt_handler+0x100/0x150()
L3 standard error: TARGET:DMM2 at address 0x0
Modules linked in:
Backtrace: 
[<c0016e90>] (dump_backtrace+0x0/0x110) from [<c035d964>] (dump_stack+0x18/0x1c)
 r6:c0395088 r5:00000061 r4:df03fb30 r3:c04d0d54
[<c035d94c>] (dump_stack+0x0/0x1c) from [<c003519c>] (warn_slowpath_common+0x54/0x6c)
[<c0035148>] (warn_slowpath_common+0x0/0x6c) from [<c0035258>] (warn_slowpath_fmt+0x38/0x40)
 r8:00000000 r7:00000000 r6:c0394f74 r5:00080001 r4:f8000200
r3:00000009
[<c0035220>] (warn_slowpath_fmt+0x0/0x40) from [<c01ac168>] (l3_interrupt_handler+0x100/0x150)
 r3:c0395121 r2:c03950bc
[<c01ac068>] (l3_interrupt_handler+0x0/0x150) from [<c007a210>] (handle_irq_event_percpu+0x38/0x17c)
 r6:0000002a r5:df006500 r4:df14a6c0
[<c007a1d8>] (handle_irq_event_percpu+0x0/0x17c) from [<c007a3ac>] (handle_irq_event+0x58/0x78)
[<c007a354>] (handle_irq_event+0x0/0x78) from [<c007d23c>] (handle_fasteoi_irq+0xcc/0x138)
 r6:c04b4558 r5:00000000 r4:df006500 r3:00000000
[<c007d170>] (handle_fasteoi_irq+0x0/0x138) from [<c0079bb8>] (generic_handle_irq+0x28/0x38)
 r4:0000002a r3:c007d170
[<c0079b90>] (generic_handle_irq+0x0/0x38) from [<c0014358>] (handle_IRQ+0x80/0xc0)
 r4:0000002a r3:000001bc
[<c00142d8>] (handle_IRQ+0x0/0xc0) from [<c0008684>] (gic_handle_irq+0x3c/0x60)
 r5:df03fc38 r4:fa240100
[<c0008648>] (gic_handle_irq+0x0/0x60) from [<c0012fc0>] (__irq_svc+0x40/0x50)
Exception stack(0xdf03fc38 to 0xdf03fc80)
fc20:                                                       c0509360 60000113
fc40: 00000000 00200020 df29e600 60000113 c0508f0c df195400 fa044104 4012fde0
fc60: 000a2139 df03fc8c df03fc90 df03fc80 c01c9708 c03604a8 60000113 ffffffff
 r6:ffffffff r5:60000113 r4:c03604a8 r3:c01c9708
[<c0360474>] (_raw_spin_unlock_irqrestore+0x0/0x38) from [<c01c9708>] (dss_mgr_start_update+0xc4/0xd8)
[<c01c9644>] (dss_mgr_start_update+0x0/0xd8) from [<c01d0ecc>] (dsi_update_screen_dispc.clone.9+0x1c4/0x22c)
 r6:00000000 r5:df195410 r4:df340410 r3:001f001f
[<c01d0d08>] (dsi_update_screen_dispc.clone.9+0x0/0x22c) from [<c01d0f74>] (omap_dsi_update+0x40/0x48)
[<c01d0f34>] (omap_dsi_update+0x0/0x48) from [<c01dd0ec>] (taal_update+0xb8/0xe4)
 r7:c04d9798 r6:00000000 r5:df340800 r4:df29ea10
[<c01dd034>] (taal_update+0x0/0xe4) from [<c01d8cc0>] (omapfb_init_display+0x110/0x14c)
 r6:00000000 r5:df340800 r4:df042000
[<c01d8bb0>] (omapfb_init_display+0x0/0x14c) from [<c0493918>] (omapfb_probe+0x378/0x408)
 r8:df042708 r7:c04d0a80 r6:00000003 r5:df340800 r4:df042000
[<c04935a0>] (omapfb_probe+0x0/0x408) from [<c0210a7c>] (platform_drv_probe+0x1c/0x20)
[<c0210a60>] (platform_drv_probe+0x0/0x20) from [<c020f654>] (really_probe+0xa4/0x1c4)
[<c020f5b0>] (really_probe+0x0/0x1c4) from [<c020f894>] (driver_probe_device+0x38/0x50)
 r7:00000000 r6:c04d907c r5:c04d907c r4:c04d0a90
[<c020f85c>] (driver_probe_device+0x0/0x50) from [<c020f914>] (__driver_attach+0x68/0x8c)
 r5:c04d0ac4 r4:c04d0a90
[<c020f8ac>] (__driver_attach+0x0/0x8c) from [<c020df48>] (bus_for_each_dev+0x58/0x88)
 r6:c020f8ac r5:df03fe50 r4:c04d907c r3:c020f8ac
[<c020def0>] (bus_for_each_dev+0x0/0x88) from [<c020f3a8>] (driver_attach+0x20/0x28)
 r7:00000000 r6:c04ddeb0 r5:df167180 r4:c04d907c
[<c020f388>] (driver_attach+0x0/0x28) from [<c020ee34>] (bus_add_driver+0xb4/0x228)
[<c020ed80>] (bus_add_driver+0x0/0x228) from [<c020fec8>] (driver_register+0xa4/0x134)
 r8:00000000 r7:c0493564 r6:c04a238c r5:c04a23ac r4:c04d907c
[<c020fe24>] (driver_register+0x0/0x134) from [<c0210d74>] (platform_driver_register+0x4c/0x60)
[<c0210d28>] (platform_driver_register+0x0/0x60) from [<c0210da8>] (platform_driver_probe+0x20/0xb4)
[<c0210d88>] (platform_driver_probe+0x0/0xb4) from [<c049357c>] (omapfb_init+0x18/0x3c)
 r6:c04a238c r5:c04a23ac r4:00000007 r3:df03e000
[<c0493564>] (omapfb_init+0x0/0x3c) from [<c00088d4>] (do_one_initcall+0xa4/0x174)
[<c0008830>] (do_one_initcall+0x0/0x174) from [<c0479960>] (kernel_init_freeable+0x104/0x1c8)
[<c047985c>] (kernel_init_freeable+0x0/0x1c8) from [<c03535b4>] (kernel_init+0x10/0x10c)
[<c03535a4>] (kernel_init+0x0/0x10c) from [<c0013458>] (ret_from_fork+0x14/0x3c)
 r4:00000000 r3:00000000
---[ end trace e317d608bf587b3d ]---
omapdss DISPC error: OCP_ERR

OMAP4430 produces boot warnings

[Archit Taneja]

Hi,

On Thursday 22 November 2012 04:33 AM, Russell King - ARM Linux wrote:
> This one is nice and long, from last nights boot test.  Looks like it was
> introduced sometime in the last couple of weeks.  Full log at:
>
> http://www.arm.linux.org.uk/developer/build/result.php?type=boot&idx=518
>
> and config:
> http://www.arm.linux.org.uk/developer/build/file.php?type=config&idx=2786

Doing a bisect results in this commit:

commit 0c7018e232c5526869250e57da8043a86a45b5de
Author: Rajendra Nayak <rnayak@ti.com>
Date:   Thu Oct 18 12:20:06 2012 +0300

     ARM: OMAP4: suspend: Program all domains to retention

     Remove the FIXME's in the suspend sequence since
     we now intend to support system level RET support.

     Signed-off-by: Rajendra Nayak <rnayak@ti.com>
     Signed-off-by: Tero Kristo <t-kristo@ti.com>
     Reviewed-by: Santosh Shilimkar <santosh.shilimkar@ti.com>

I guess this commit will allow DSS to go to a lower power state. So what 
might be happening is:

- After returning back from the lower power state, the DISPC base 
address register hasn't been restored. Leading to a fetch from a bad 
address. Resulting in an OCP error.

or

-  DSS never came back to ON state, and it's not able to access 
registers. I doubt this possibility because we got an OCP error 
interrupt from DISPC.

Archit

>
> taal display1: taal panel revision e3.83.7d
> ------------[ cut here ]------------
> WARNING: at drivers/bus/omap_l3_noc.c:97 l3_interrupt_handler+0x100/0x150()
> L3 standard error: TARGET:DMM2 at address 0x0
> Modules linked in:
> Backtrace:
> [<c0016e90>] (dump_backtrace+0x0/0x110) from [<c035d964>] (dump_stack+0x18/0x1c)
>   r6:c0395088 r5:00000061 r4:df03fb30 r3:c04d0d54
> [<c035d94c>] (dump_stack+0x0/0x1c) from [<c003519c>] (warn_slowpath_common+0x54/0x6c)
> [<c0035148>] (warn_slowpath_common+0x0/0x6c) from [<c0035258>] (warn_slowpath_fmt+0x38/0x40)
>   r8:00000000 r7:00000000 r6:c0394f74 r5:00080001 r4:f8000200
> r3:00000009
> [<c0035220>] (warn_slowpath_fmt+0x0/0x40) from [<c01ac168>] (l3_interrupt_handler+0x100/0x150)
>   r3:c0395121 r2:c03950bc
> [<c01ac068>] (l3_interrupt_handler+0x0/0x150) from [<c007a210>] (handle_irq_event_percpu+0x38/0x17c)
>   r6:0000002a r5:df006500 r4:df14a6c0
> [<c007a1d8>] (handle_irq_event_percpu+0x0/0x17c) from [<c007a3ac>] (handle_irq_event+0x58/0x78)
> [<c007a354>] (handle_irq_event+0x0/0x78) from [<c007d23c>] (handle_fasteoi_irq+0xcc/0x138)
>   r6:c04b4558 r5:00000000 r4:df006500 r3:00000000
> [<c007d170>] (handle_fasteoi_irq+0x0/0x138) from [<c0079bb8>] (generic_handle_irq+0x28/0x38)
>   r4:0000002a r3:c007d170
> [<c0079b90>] (generic_handle_irq+0x0/0x38) from [<c0014358>] (handle_IRQ+0x80/0xc0)
>   r4:0000002a r3:000001bc
> [<c00142d8>] (handle_IRQ+0x0/0xc0) from [<c0008684>] (gic_handle_irq+0x3c/0x60)
>   r5:df03fc38 r4:fa240100
> [<c0008648>] (gic_handle_irq+0x0/0x60) from [<c0012fc0>] (__irq_svc+0x40/0x50)
> Exception stack(0xdf03fc38 to 0xdf03fc80)
> fc20:                                                       c0509360 60000113
> fc40: 00000000 00200020 df29e600 60000113 c0508f0c df195400 fa044104 4012fde0
> fc60: 000a2139 df03fc8c df03fc90 df03fc80 c01c9708 c03604a8 60000113 ffffffff
>   r6:ffffffff r5:60000113 r4:c03604a8 r3:c01c9708
> [<c0360474>] (_raw_spin_unlock_irqrestore+0x0/0x38) from [<c01c9708>] (dss_mgr_start_update+0xc4/0xd8)
> [<c01c9644>] (dss_mgr_start_update+0x0/0xd8) from [<c01d0ecc>] (dsi_update_screen_dispc.clone.9+0x1c4/0x22c)
>   r6:00000000 r5:df195410 r4:df340410 r3:001f001f
> [<c01d0d08>] (dsi_update_screen_dispc.clone.9+0x0/0x22c) from [<c01d0f74>] (omap_dsi_update+0x40/0x48)
> [<c01d0f34>] (omap_dsi_update+0x0/0x48) from [<c01dd0ec>] (taal_update+0xb8/0xe4)
>   r7:c04d9798 r6:00000000 r5:df340800 r4:df29ea10
> [<c01dd034>] (taal_update+0x0/0xe4) from [<c01d8cc0>] (omapfb_init_display+0x110/0x14c)
>   r6:00000000 r5:df340800 r4:df042000
> [<c01d8bb0>] (omapfb_init_display+0x0/0x14c) from [<c0493918>] (omapfb_probe+0x378/0x408)
>   r8:df042708 r7:c04d0a80 r6:00000003 r5:df340800 r4:df042000
> [<c04935a0>] (omapfb_probe+0x0/0x408) from [<c0210a7c>] (platform_drv_probe+0x1c/0x20)
> [<c0210a60>] (platform_drv_probe+0x0/0x20) from [<c020f654>] (really_probe+0xa4/0x1c4)
> [<c020f5b0>] (really_probe+0x0/0x1c4) from [<c020f894>] (driver_probe_device+0x38/0x50)
>   r7:00000000 r6:c04d907c r5:c04d907c r4:c04d0a90
> [<c020f85c>] (driver_probe_device+0x0/0x50) from [<c020f914>] (__driver_attach+0x68/0x8c)
>   r5:c04d0ac4 r4:c04d0a90
> [<c020f8ac>] (__driver_attach+0x0/0x8c) from [<c020df48>] (bus_for_each_dev+0x58/0x88)
>   r6:c020f8ac r5:df03fe50 r4:c04d907c r3:c020f8ac
> [<c020def0>] (bus_for_each_dev+0x0/0x88) from [<c020f3a8>] (driver_attach+0x20/0x28)
>   r7:00000000 r6:c04ddeb0 r5:df167180 r4:c04d907c
> [<c020f388>] (driver_attach+0x0/0x28) from [<c020ee34>] (bus_add_driver+0xb4/0x228)
> [<c020ed80>] (bus_add_driver+0x0/0x228) from [<c020fec8>] (driver_register+0xa4/0x134)
>   r8:00000000 r7:c0493564 r6:c04a238c r5:c04a23ac r4:c04d907c
> [<c020fe24>] (driver_register+0x0/0x134) from [<c0210d74>] (platform_driver_register+0x4c/0x60)
> [<c0210d28>] (platform_driver_register+0x0/0x60) from [<c0210da8>] (platform_driver_probe+0x20/0xb4)
> [<c0210d88>] (platform_driver_probe+0x0/0xb4) from [<c049357c>] (omapfb_init+0x18/0x3c)
>   r6:c04a238c r5:c04a23ac r4:00000007 r3:df03e000
> [<c0493564>] (omapfb_init+0x0/0x3c) from [<c00088d4>] (do_one_initcall+0xa4/0x174)
> [<c0008830>] (do_one_initcall+0x0/0x174) from [<c0479960>] (kernel_init_freeable+0x104/0x1c8)
> [<c047985c>] (kernel_init_freeable+0x0/0x1c8) from [<c03535b4>] (kernel_init+0x10/0x10c)
> [<c03535a4>] (kernel_init+0x0/0x10c) from [<c0013458>] (ret_from_fork+0x14/0x3c)
>   r4:00000000 r3:00000000
> ---[ end trace e317d608bf587b3d ]---
> omapdss DISPC error: OCP_ERR
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>

OMAP4430 produces boot warnings

[Tomi Valkeinen]

On 2012-11-22 14:42, Archit Taneja wrote:
> Hi,
> 
> On Thursday 22 November 2012 04:33 AM, Russell King - ARM Linux wrote:
>> This one is nice and long, from last nights boot test.  Looks like it was
>> introduced sometime in the last couple of weeks.  Full log at:
>>
>> http://www.arm.linux.org.uk/developer/build/result.php?type=boot&idx=518
>>
>> and config:
>> http://www.arm.linux.org.uk/developer/build/file.php?type=config&idx=2786
> 
> Doing a bisect results in this commit:
> 
> commit 0c7018e232c5526869250e57da8043a86a45b5de
> Author: Rajendra Nayak <rnayak@ti.com>
> Date:   Thu Oct 18 12:20:06 2012 +0300
> 
>     ARM: OMAP4: suspend: Program all domains to retention
> 
>     Remove the FIXME's in the suspend sequence since
>     we now intend to support system level RET support.
> 
>     Signed-off-by: Rajendra Nayak <rnayak@ti.com>
>     Signed-off-by: Tero Kristo <t-kristo@ti.com>
>     Reviewed-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
> 
> I guess this commit will allow DSS to go to a lower power state. So what
> might be happening is:
> 
> - After returning back from the lower power state, the DISPC base
> address register hasn't been restored. Leading to a fetch from a bad
> address. Resulting in an OCP error.
> 
> or
> 
> -  DSS never came back to ON state, and it's not able to access
> registers. I doubt this possibility because we got an OCP error
> interrupt from DISPC.

It seems that the problem is that dispc never restores the context,
because get_ctx_loss_count always returns 1. I enabled pwrdm debug
prints, and pwrdm_get_context_loss_count() always returns 1 for dss,
even if the register contents have obviously been lost.

Does the pwrdm mistakenly think that in RET state the DSS still keeps
the register contents?

 Tomi


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 899 bytes
Desc: OpenPGP digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20121122/40c241c6/attachment.sig>

OMAP4430 produces boot warnings

[Tero Kristo]

On Thu, 2012-11-22 at 15:42 +0200, Tomi Valkeinen wrote:
> On 2012-11-22 14:42, Archit Taneja wrote:
> > Hi,
> > 
> > On Thursday 22 November 2012 04:33 AM, Russell King - ARM Linux wrote:
> >> This one is nice and long, from last nights boot test.  Looks like it was
> >> introduced sometime in the last couple of weeks.  Full log at:
> >>
> >> http://www.arm.linux.org.uk/developer/build/result.php?type=boot&idx=518
> >>
> >> and config:
> >> http://www.arm.linux.org.uk/developer/build/file.php?type=config&idx=2786
> > 
> > Doing a bisect results in this commit:
> > 
> > commit 0c7018e232c5526869250e57da8043a86a45b5de
> > Author: Rajendra Nayak <rnayak@ti.com>
> > Date:   Thu Oct 18 12:20:06 2012 +0300
> > 
> >     ARM: OMAP4: suspend: Program all domains to retention
> > 
> >     Remove the FIXME's in the suspend sequence since
> >     we now intend to support system level RET support.
> > 
> >     Signed-off-by: Rajendra Nayak <rnayak@ti.com>
> >     Signed-off-by: Tero Kristo <t-kristo@ti.com>
> >     Reviewed-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
> > 
> > I guess this commit will allow DSS to go to a lower power state. So what
> > might be happening is:
> > 
> > - After returning back from the lower power state, the DISPC base
> > address register hasn't been restored. Leading to a fetch from a bad
> > address. Resulting in an OCP error.
> > 
> > or
> > 
> > -  DSS never came back to ON state, and it's not able to access
> > registers. I doubt this possibility because we got an OCP error
> > interrupt from DISPC.
> 
> It seems that the problem is that dispc never restores the context,
> because get_ctx_loss_count always returns 1. I enabled pwrdm debug
> prints, and pwrdm_get_context_loss_count() always returns 1 for dss,
> even if the register contents have obviously been lost.

I guess you checked that DSS pwrdm is switching between RET and ON in
your setup?

> Does the pwrdm mistakenly think that in RET state the DSS still keeps
> the register contents?

This might be the case, however the pwrdm code should be generic and
handle all domains properly. What is the tree / branch / commit you are
using for testing this stuff? I can take a look at this also.

-Tero

OMAP4430 produces boot warnings

[Tomi Valkeinen]

On 2012-11-22 16:34, Tero Kristo wrote:

> I guess you checked that DSS pwrdm is switching between RET and ON in
> your setup?

Yes:

# cat /debug/pm_debug/count |grep dss
[   35.356567] pwrdm state mismatch(l3init_pwrdm) 3 != 1
[   35.361938] pwrdm state mismatch(cam_pwrdm) 3 != 0
[   35.366973] pwrdm state mismatch(ivahd_pwrdm) 3 != 1
[   35.372253] pwrdm state mismatch(tesla_pwrdm) 3 != 1
[   35.377532] pwrdm state mismatch(abe_pwrdm) 3 != 1
dss_pwrdm (RET),OFF:1,RET:11,INA:0,ON:11,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0
l3_dss_clkdm->dss_pwrdm (0)

then I load and unload the dss modules, and then:

# cat /debug/pm_debug/count |grep dss
[   60.813629] pwrdm state mismatch(l3init_pwrdm) 3 != 1
[   60.819000] pwrdm state mismatch(cam_pwrdm) 3 != 0
[   60.824127] pwrdm state mismatch(ivahd_pwrdm) 3 != 1
[   60.829376] pwrdm state mismatch(tesla_pwrdm) 3 != 1
[   60.834625] pwrdm state mismatch(abe_pwrdm) 3 != 1
dss_pwrdm (ON),OFF:1,RET:21,INA:0,ON:22,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0
l3_dss_clkdm->dss_pwrdm (0)

>> Does the pwrdm mistakenly think that in RET state the DSS still keeps
>> the register contents?
> 
> This might be the case, however the pwrdm code should be generic and
> handle all domains properly. What is the tree / branch / commit you are
> using for testing this stuff? I can take a look at this also.

arm-soc/for-next

 Tomi


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 899 bytes
Desc: OpenPGP digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20121122/54916d83/attachment.sig>

OMAP4430 produces boot warnings

[Tero Kristo]

On Thu, 2012-11-22 at 16:44 +0200, Tomi Valkeinen wrote:
> On 2012-11-22 16:34, Tero Kristo wrote:
> 
> > I guess you checked that DSS pwrdm is switching between RET and ON in
> > your setup?
> 
> Yes:
> 
> # cat /debug/pm_debug/count |grep dss
> [   35.356567] pwrdm state mismatch(l3init_pwrdm) 3 != 1
> [   35.361938] pwrdm state mismatch(cam_pwrdm) 3 != 0
> [   35.366973] pwrdm state mismatch(ivahd_pwrdm) 3 != 1
> [   35.372253] pwrdm state mismatch(tesla_pwrdm) 3 != 1
> [   35.377532] pwrdm state mismatch(abe_pwrdm) 3 != 1
> dss_pwrdm (RET),OFF:1,RET:11,INA:0,ON:11,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0
> l3_dss_clkdm->dss_pwrdm (0)
> 
> then I load and unload the dss modules, and then:
> 
> # cat /debug/pm_debug/count |grep dss
> [   60.813629] pwrdm state mismatch(l3init_pwrdm) 3 != 1
> [   60.819000] pwrdm state mismatch(cam_pwrdm) 3 != 0
> [   60.824127] pwrdm state mismatch(ivahd_pwrdm) 3 != 1
> [   60.829376] pwrdm state mismatch(tesla_pwrdm) 3 != 1
> [   60.834625] pwrdm state mismatch(abe_pwrdm) 3 != 1
> dss_pwrdm (ON),OFF:1,RET:21,INA:0,ON:22,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0
> l3_dss_clkdm->dss_pwrdm (0)
> 
> >> Does the pwrdm mistakenly think that in RET state the DSS still keeps
> >> the register contents?
> > 
> > This might be the case, however the pwrdm code should be generic and
> > handle all domains properly. What is the tree / branch / commit you are
> > using for testing this stuff? I can take a look at this also.
> 
> arm-soc/for-next

I guess this is caused because some of the patches are still not in the
for-next branch, it looks like at least this is missing:

https://patchwork.kernel.org/patch/1608901/

...or the latest update done by Paul to that one.

The patch I posted appears to have a small merge induced bug, it is
registering the context loss soc_ops for am33xx when it should actually
register those for omap4. This might explain another bug I've been
looking at in a different branch recently... The update Paul posted does
not seem to have this problem, but I haven't tested it myself.

-Tero

OMAP4430 produces boot warnings

[Archit Taneja]

On Friday 23 November 2012 03:04 PM, Tero Kristo wrote:
> On Thu, 2012-11-22 at 16:44 +0200, Tomi Valkeinen wrote:
>> On 2012-11-22 16:34, Tero Kristo wrote:
>>
>>> I guess you checked that DSS pwrdm is switching between RET and ON in
>>> your setup?
>>
>> Yes:
>>
>> # cat /debug/pm_debug/count |grep dss
>> [   35.356567] pwrdm state mismatch(l3init_pwrdm) 3 != 1
>> [   35.361938] pwrdm state mismatch(cam_pwrdm) 3 != 0
>> [   35.366973] pwrdm state mismatch(ivahd_pwrdm) 3 != 1
>> [   35.372253] pwrdm state mismatch(tesla_pwrdm) 3 != 1
>> [   35.377532] pwrdm state mismatch(abe_pwrdm) 3 != 1
>> dss_pwrdm (RET),OFF:1,RET:11,INA:0,ON:11,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0
>> l3_dss_clkdm->dss_pwrdm (0)
>>
>> then I load and unload the dss modules, and then:
>>
>> # cat /debug/pm_debug/count |grep dss
>> [   60.813629] pwrdm state mismatch(l3init_pwrdm) 3 != 1
>> [   60.819000] pwrdm state mismatch(cam_pwrdm) 3 != 0
>> [   60.824127] pwrdm state mismatch(ivahd_pwrdm) 3 != 1
>> [   60.829376] pwrdm state mismatch(tesla_pwrdm) 3 != 1
>> [   60.834625] pwrdm state mismatch(abe_pwrdm) 3 != 1
>> dss_pwrdm (ON),OFF:1,RET:21,INA:0,ON:22,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0
>> l3_dss_clkdm->dss_pwrdm (0)
>>
>>>> Does the pwrdm mistakenly think that in RET state the DSS still keeps
>>>> the register contents?
>>>
>>> This might be the case, however the pwrdm code should be generic and
>>> handle all domains properly. What is the tree / branch / commit you are
>>> using for testing this stuff? I can take a look at this also.
>>
>> arm-soc/for-next
>
> I guess this is caused because some of the patches are still not in the
> for-next branch, it looks like at least this is missing:
>
> https://patchwork.kernel.org/patch/1608901/
>
> ...or the latest update done by Paul to that one.
>
> The patch I posted appears to have a small merge induced bug, it is
> registering the context loss soc_ops for am33xx when it should actually
> register those for omap4. This might explain another bug I've been
> looking at in a different branch recently... The update Paul posted does
> not seem to have this problem, but I haven't tested it myself.


Applying the patch above, and registering the soc_ops for omap4 instead 
of am33xx doesn't seem to help. The context lost count now always returns 0.

And to verify Tomi's observation, if we don't rely on the context lost 
count, and restore the registers always, we don't see the problem.

Btw, we use the function omap_pm_get_dev_context_loss_count to get the 
count, do we use this or is there a new func to get the count?

Thanks,
Archit

OMAP4430 produces boot warnings

[Archit Taneja]

Hi,

On Monday 26 November 2012 12:18 PM, Archit Taneja wrote:
> On Friday 23 November 2012 03:04 PM, Tero Kristo wrote:
>> On Thu, 2012-11-22 at 16:44 +0200, Tomi Valkeinen wrote:
>>> On 2012-11-22 16:34, Tero Kristo wrote:
>>>
>>>> I guess you checked that DSS pwrdm is switching between RET and ON in
>>>> your setup?
>>>
>>> Yes:
>>>
>>> # cat /debug/pm_debug/count |grep dss
>>> [   35.356567] pwrdm state mismatch(l3init_pwrdm) 3 != 1
>>> [   35.361938] pwrdm state mismatch(cam_pwrdm) 3 != 0
>>> [   35.366973] pwrdm state mismatch(ivahd_pwrdm) 3 != 1
>>> [   35.372253] pwrdm state mismatch(tesla_pwrdm) 3 != 1
>>> [   35.377532] pwrdm state mismatch(abe_pwrdm) 3 != 1
>>> dss_pwrdm
>>> (RET),OFF:1,RET:11,INA:0,ON:11,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0
>>> l3_dss_clkdm->dss_pwrdm (0)
>>>
>>> then I load and unload the dss modules, and then:
>>>
>>> # cat /debug/pm_debug/count |grep dss
>>> [   60.813629] pwrdm state mismatch(l3init_pwrdm) 3 != 1
>>> [   60.819000] pwrdm state mismatch(cam_pwrdm) 3 != 0
>>> [   60.824127] pwrdm state mismatch(ivahd_pwrdm) 3 != 1
>>> [   60.829376] pwrdm state mismatch(tesla_pwrdm) 3 != 1
>>> [   60.834625] pwrdm state mismatch(abe_pwrdm) 3 != 1
>>> dss_pwrdm
>>> (ON),OFF:1,RET:21,INA:0,ON:22,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0
>>> l3_dss_clkdm->dss_pwrdm (0)
>>>
>>>>> Does the pwrdm mistakenly think that in RET state the DSS still keeps
>>>>> the register contents?
>>>>
>>>> This might be the case, however the pwrdm code should be generic and
>>>> handle all domains properly. What is the tree / branch / commit you are
>>>> using for testing this stuff? I can take a look at this also.
>>>
>>> arm-soc/for-next
>>
>> I guess this is caused because some of the patches are still not in the
>> for-next branch, it looks like at least this is missing:
>>
>> https://patchwork.kernel.org/patch/1608901/
>>
>> ...or the latest update done by Paul to that one.
>>
>> The patch I posted appears to have a small merge induced bug, it is
>> registering the context loss soc_ops for am33xx when it should actually
>> register those for omap4. This might explain another bug I've been
>> looking at in a different branch recently... The update Paul posted does
>> not seem to have this problem, but I haven't tested it myself.
>
>
> Applying the patch above, and registering the soc_ops for omap4 instead
> of am33xx doesn't seem to help. The context lost count now always
> returns 0.
>
> And to verify Tomi's observation, if we don't rely on the context lost
> count, and restore the registers always, we don't see the problem.

So Rajendra and I found the problem.

The function _omap4_update_context_lost() reads the register 
RM_DSS_DSS_CONTEXT for all DSS hwmods, and increments the count if we 
read a non zero value. The issue is that the DSS's parent platform 
device (tied to dss_core hwmod) is called first when resuming, it 
correctly reads the register and observes that DSS lost context, and 
then clears the register.

When the children hwmods are enabled, the see that the registers are 
cleared, and hence never increment their count.

One option is to make the DSS driver use the context lost count of the 
hwmod corresponding to the parent platform device. It sort of makes a 
bit of sense as all the DSS platform devices belong to the same power 
domain, so considering only the parent's context lost count is not so bad.

The second option would be to have some usecounting mechanism in 
omap_hwmod where different hwmods belonging to the same power domain 
don't have their PM_CONTEXT registers cleared until all the hwmods are 
enabled.

The first option is easier to implement, here is a patch for the DISPC 
driver:


 From 619276fa0e62b90875475eb345a310f1223e82f6 Mon Sep 17 00:00:00 2001
From: Archit Taneja <archit@ti.com>
Date: Mon, 26 Nov 2012 17:22:27 +0530
Subject: [PATCH] OMAPDSS: DISPC: Use DISPC's parent device to get context
  lost count

When enabling a hwmod, omap_hwmod refers to the register mentioned in the
hwmod struct's member 'prcm.omap4.context_offs' to see whether context was
lost or not. It increments the context lost count for the hwmod and then 
clears
the register.

All the DSS hwmods have the same register(RM_DSS_DSS_CONTEXT) as 
context_offs.
When DSS is enabled, the first hwmod to be enabled is the "dss_core" 
hwmod since
it's the parent platform device. The dss_core hwmod updates it's context 
lost
count correctly and clears the register. When the hwmods corresponding 
to the
children platform devices are enabled. They see that the register is 
clear, and
don't increment their context lost count. Therefore, all the children 
platfrom
devices never report a change in context.

The DISPC driver currently gets the context lost count for DSS from it's
corresponsing platform device instance. The DISPC platform device is one 
of the
child devices, and doesn't report the context lost count correctly.

Make the DISPC driver get the context lost count from it's parent. The 
parent
platform device's hwmod is the only one which correctly updates the 
context lost
count.

Signed-off-by: Archit Taneja <archit@ti.com>
---
  drivers/video/omap2/dss/dispc.c |    4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/video/omap2/dss/dispc.c 
b/drivers/video/omap2/dss/dispc.c
index a5ab354..d9dfc4ad 100644
--- a/drivers/video/omap2/dss/dispc.c
+++ b/drivers/video/omap2/dss/dispc.c
@@ -374,7 +374,7 @@ static void dispc_save_context(void)
  	if (dss_has_feature(FEAT_CORE_CLK_DIV))
  		SR(DIVISOR);

-	dispc.ctx_loss_cnt = dss_get_ctx_loss_count(&dispc.pdev->dev);
+	dispc.ctx_loss_cnt = dss_get_ctx_loss_count(dispc.pdev->dev.parent);
  	dispc.ctx_valid = true;

  	DSSDBG("context saved, ctx_loss_count %d\n", dispc.ctx_loss_cnt);
@@ -389,7 +389,7 @@ static void dispc_restore_context(void)
  	if (!dispc.ctx_valid)
  		return;

-	ctx = dss_get_ctx_loss_count(&dispc.pdev->dev);
+	ctx = dss_get_ctx_loss_count(dispc.pdev->dev.parent);

  	if (ctx >= 0 && ctx == dispc.ctx_loss_cnt)
  		return;
-- 
1.7.9.5

OMAP4430 produces boot warnings

[Tomi Valkeinen]

On 2012-11-26 14:14, Archit Taneja wrote:

> So Rajendra and I found the problem.
> 
> The function _omap4_update_context_lost() reads the register
> RM_DSS_DSS_CONTEXT for all DSS hwmods, and increments the count if we
> read a non zero value. The issue is that the DSS's parent platform
> device (tied to dss_core hwmod) is called first when resuming, it
> correctly reads the register and observes that DSS lost context, and
> then clears the register.
> 
> When the children hwmods are enabled, the see that the registers are
> cleared, and hence never increment their count.
> 
> One option is to make the DSS driver use the context lost count of the
> hwmod corresponding to the parent platform device. It sort of makes a
> bit of sense as all the DSS platform devices belong to the same power
> domain, so considering only the parent's context lost count is not so bad.
> 
> The second option would be to have some usecounting mechanism in
> omap_hwmod where different hwmods belonging to the same power domain
> don't have their PM_CONTEXT registers cleared until all the hwmods are
> enabled.
> 
> The first option is easier to implement, here is a patch for the DISPC
> driver:
> 
> 
> From 619276fa0e62b90875475eb345a310f1223e82f6 Mon Sep 17 00:00:00 2001
> From: Archit Taneja <archit@ti.com>
> Date: Mon, 26 Nov 2012 17:22:27 +0530
> Subject: [PATCH] OMAPDSS: DISPC: Use DISPC's parent device to get context
>  lost count
> 
> When enabling a hwmod, omap_hwmod refers to the register mentioned in the
> hwmod struct's member 'prcm.omap4.context_offs' to see whether context was
> lost or not. It increments the context lost count for the hwmod and then
> clears
> the register.
> 
> All the DSS hwmods have the same register(RM_DSS_DSS_CONTEXT) as
> context_offs.
> When DSS is enabled, the first hwmod to be enabled is the "dss_core"
> hwmod since
> it's the parent platform device. The dss_core hwmod updates it's context
> lost
> count correctly and clears the register. When the hwmods corresponding
> to the
> children platform devices are enabled. They see that the register is
> clear, and
> don't increment their context lost count. Therefore, all the children
> platfrom
> devices never report a change in context.
> 
> The DISPC driver currently gets the context lost count for DSS from it's
> corresponsing platform device instance. The DISPC platform device is one
> of the
> child devices, and doesn't report the context lost count correctly.
> 
> Make the DISPC driver get the context lost count from it's parent. The
> parent
> platform device's hwmod is the only one which correctly updates the
> context lost
> count.
> 
> Signed-off-by: Archit Taneja <archit@ti.com>
> ---
>  drivers/video/omap2/dss/dispc.c |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/video/omap2/dss/dispc.c
> b/drivers/video/omap2/dss/dispc.c
> index a5ab354..d9dfc4ad 100644
> --- a/drivers/video/omap2/dss/dispc.c
> +++ b/drivers/video/omap2/dss/dispc.c
> @@ -374,7 +374,7 @@ static void dispc_save_context(void)
>      if (dss_has_feature(FEAT_CORE_CLK_DIV))
>          SR(DIVISOR);
> 
> -    dispc.ctx_loss_cnt = dss_get_ctx_loss_count(&dispc.pdev->dev);
> +    dispc.ctx_loss_cnt = dss_get_ctx_loss_count(dispc.pdev->dev.parent);
>      dispc.ctx_valid = true;
> 
>      DSSDBG("context saved, ctx_loss_count %d\n", dispc.ctx_loss_cnt);
> @@ -389,7 +389,7 @@ static void dispc_restore_context(void)
>      if (!dispc.ctx_valid)
>          return;
> 
> -    ctx = dss_get_ctx_loss_count(&dispc.pdev->dev);
> +    ctx = dss_get_ctx_loss_count(dispc.pdev->dev.parent);
> 
>      if (ctx >= 0 && ctx == dispc.ctx_loss_cnt)
>          return;

Hmm, well this feels like a hack. DISPC driver doesn't know how the DSS
modules are arranged, which module belongs to which power domain, etc.

If it cannot be fixed in the arch code, I guess we could just have
dss_get_ctx_loss_count(void) function which always returns the
dss_core's ctx loss count, and define that on all the platforms omapdss
is used, the dss_core's ctx loss count is the same as ctx loss count for
all the dss submodules.

I think the above is true for all OMAPs. But it feels like a hack too,
but not as bad as the above patch.

 Tomi


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 899 bytes
Desc: OpenPGP digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20121127/3cc8cff2/attachment.sig>

OMAP4430 produces boot warnings

[Archit Taneja]

On Tuesday 27 November 2012 04:53 PM, Tomi Valkeinen wrote:
> On 2012-11-26 14:14, Archit Taneja wrote:
>
>> So Rajendra and I found the problem.
>>
>> The function _omap4_update_context_lost() reads the register
>> RM_DSS_DSS_CONTEXT for all DSS hwmods, and increments the count if we
>> read a non zero value. The issue is that the DSS's parent platform
>> device (tied to dss_core hwmod) is called first when resuming, it
>> correctly reads the register and observes that DSS lost context, and
>> then clears the register.
>>
>> When the children hwmods are enabled, the see that the registers are
>> cleared, and hence never increment their count.
>>
>> One option is to make the DSS driver use the context lost count of the
>> hwmod corresponding to the parent platform device. It sort of makes a
>> bit of sense as all the DSS platform devices belong to the same power
>> domain, so considering only the parent's context lost count is not so bad.
>>
>> The second option would be to have some usecounting mechanism in
>> omap_hwmod where different hwmods belonging to the same power domain
>> don't have their PM_CONTEXT registers cleared until all the hwmods are
>> enabled.
>>
>> The first option is easier to implement, here is a patch for the DISPC
>> driver:
>>
>>
>>  From 619276fa0e62b90875475eb345a310f1223e82f6 Mon Sep 17 00:00:00 2001
>> From: Archit Taneja <archit@ti.com>
>> Date: Mon, 26 Nov 2012 17:22:27 +0530
>> Subject: [PATCH] OMAPDSS: DISPC: Use DISPC's parent device to get context
>>   lost count
>>
>> When enabling a hwmod, omap_hwmod refers to the register mentioned in the
>> hwmod struct's member 'prcm.omap4.context_offs' to see whether context was
>> lost or not. It increments the context lost count for the hwmod and then
>> clears
>> the register.
>>
>> All the DSS hwmods have the same register(RM_DSS_DSS_CONTEXT) as
>> context_offs.
>> When DSS is enabled, the first hwmod to be enabled is the "dss_core"
>> hwmod since
>> it's the parent platform device. The dss_core hwmod updates it's context
>> lost
>> count correctly and clears the register. When the hwmods corresponding
>> to the
>> children platform devices are enabled. They see that the register is
>> clear, and
>> don't increment their context lost count. Therefore, all the children
>> platfrom
>> devices never report a change in context.
>>
>> The DISPC driver currently gets the context lost count for DSS from it's
>> corresponsing platform device instance. The DISPC platform device is one
>> of the
>> child devices, and doesn't report the context lost count correctly.
>>
>> Make the DISPC driver get the context lost count from it's parent. The
>> parent
>> platform device's hwmod is the only one which correctly updates the
>> context lost
>> count.
>>
>> Signed-off-by: Archit Taneja <archit@ti.com>
>> ---
>>   drivers/video/omap2/dss/dispc.c |    4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/video/omap2/dss/dispc.c
>> b/drivers/video/omap2/dss/dispc.c
>> index a5ab354..d9dfc4ad 100644
>> --- a/drivers/video/omap2/dss/dispc.c
>> +++ b/drivers/video/omap2/dss/dispc.c
>> @@ -374,7 +374,7 @@ static void dispc_save_context(void)
>>       if (dss_has_feature(FEAT_CORE_CLK_DIV))
>>           SR(DIVISOR);
>>
>> -    dispc.ctx_loss_cnt = dss_get_ctx_loss_count(&dispc.pdev->dev);
>> +    dispc.ctx_loss_cnt = dss_get_ctx_loss_count(dispc.pdev->dev.parent);
>>       dispc.ctx_valid = true;
>>
>>       DSSDBG("context saved, ctx_loss_count %d\n", dispc.ctx_loss_cnt);
>> @@ -389,7 +389,7 @@ static void dispc_restore_context(void)
>>       if (!dispc.ctx_valid)
>>           return;
>>
>> -    ctx = dss_get_ctx_loss_count(&dispc.pdev->dev);
>> +    ctx = dss_get_ctx_loss_count(dispc.pdev->dev.parent);
>>
>>       if (ctx >= 0 && ctx == dispc.ctx_loss_cnt)
>>           return;
>
> Hmm, well this feels like a hack. DISPC driver doesn't know how the DSS
> modules are arranged, which module belongs to which power domain, etc.
>
> If it cannot be fixed in the arch code, I guess we could just have
> dss_get_ctx_loss_count(void) function which always returns the
> dss_core's ctx loss count, and define that on all the platforms omapdss
> is used, the dss_core's ctx loss count is the same as ctx loss count for
> all the dss submodules.
>
> I think the above is true for all OMAPs. But it feels like a hack too,
> but not as bad as the above patch.

Yes, a function taking in no platform device in dss's core.c would be 
less hacky. I guess we would need this for now, because a solution in 
omap_hwmod would be more complex and it may not be ready by the merge 
window.

Archit

OMAP4430 produces boot warnings

[Tomi Valkeinen]

On 2012-11-27 13:56, Archit Taneja wrote:
> On Tuesday 27 November 2012 04:53 PM, Tomi Valkeinen wrote:

>> Hmm, well this feels like a hack. DISPC driver doesn't know how the DSS
>> modules are arranged, which module belongs to which power domain, etc.
>>
>> If it cannot be fixed in the arch code, I guess we could just have
>> dss_get_ctx_loss_count(void) function which always returns the
>> dss_core's ctx loss count, and define that on all the platforms omapdss
>> is used, the dss_core's ctx loss count is the same as ctx loss count for
>> all the dss submodules.
>>
>> I think the above is true for all OMAPs. But it feels like a hack too,
>> but not as bad as the above patch.
> 
> Yes, a function taking in no platform device in dss's core.c would be
> less hacky. I guess we would need this for now, because a solution in
> omap_hwmod would be more complex and it may not be ready by the merge
> window.

Ok. Can you cook up a patch and test it?

PM guys, does the above sound like an acceptable work-around?

 Tomi


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 899 bytes
Desc: OpenPGP digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20121127/3dc952da/attachment.sig>

OMAP4430 produces boot warnings

[Tero Kristo]

On Tue, 2012-11-27 at 14:21 +0200, Tomi Valkeinen wrote:
> On 2012-11-27 13:56, Archit Taneja wrote:
> > On Tuesday 27 November 2012 04:53 PM, Tomi Valkeinen wrote:
> 
> >> Hmm, well this feels like a hack. DISPC driver doesn't know how the DSS
> >> modules are arranged, which module belongs to which power domain, etc.
> >>
> >> If it cannot be fixed in the arch code, I guess we could just have
> >> dss_get_ctx_loss_count(void) function which always returns the
> >> dss_core's ctx loss count, and define that on all the platforms omapdss
> >> is used, the dss_core's ctx loss count is the same as ctx loss count for
> >> all the dss submodules.
> >>
> >> I think the above is true for all OMAPs. But it feels like a hack too,
> >> but not as bad as the above patch.
> > 
> > Yes, a function taking in no platform device in dss's core.c would be
> > less hacky. I guess we would need this for now, because a solution in
> > omap_hwmod would be more complex and it may not be ready by the merge
> > window.
> 
> Ok. Can you cook up a patch and test it?
> 
> PM guys, does the above sound like an acceptable work-around?

This sounds like a good approach to me at least.

-Tero

OMAP4430 produces boot warnings

[Archit Taneja]

On Tuesday 27 November 2012 06:01 PM, Tero Kristo wrote:
> On Tue, 2012-11-27 at 14:21 +0200, Tomi Valkeinen wrote:
>> On 2012-11-27 13:56, Archit Taneja wrote:
>>> On Tuesday 27 November 2012 04:53 PM, Tomi Valkeinen wrote:
>>
>>>> Hmm, well this feels like a hack. DISPC driver doesn't know how the DSS
>>>> modules are arranged, which module belongs to which power domain, etc.
>>>>
>>>> If it cannot be fixed in the arch code, I guess we could just have
>>>> dss_get_ctx_loss_count(void) function which always returns the
>>>> dss_core's ctx loss count, and define that on all the platforms omapdss
>>>> is used, the dss_core's ctx loss count is the same as ctx loss count for
>>>> all the dss submodules.

Well the context lost count we are interested in isn't the "omapdss" 
platform device in core.c, it's the "omapdss_dss" platform device in dss.c

I was considering moving the dss_get_ctx_lost_count() to dss.c, as it 
needs the "omapdss_dss" platform_device. It looks better in core.c, but 
we would need a dirty way to give it the "omapdss_dss". What do you think?

Archit

>>>>
>>>> I think the above is true for all OMAPs. But it feels like a hack too,
>>>> but not as bad as the above patch.
>>>
>>> Yes, a function taking in no platform device in dss's core.c would be
>>> less hacky. I guess we would need this for now, because a solution in
>>> omap_hwmod would be more complex and it may not be ready by the merge
>>> window.
>>
>> Ok. Can you cook up a patch and test it?
>>
>> PM guys, does the above sound like an acceptable work-around?
>
> This sounds like a good approach to me at least.
>
> -Tero
>
>
>
>