Doing a bisect results in this commit:
commit 0c7018e232c5526869250e57da8043a86a45b5de
Author: Rajendra Nayak [off-list ref]
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 [off-list ref]
Signed-off-by: Tero Kristo [off-list ref]
Reviewed-by: Santosh Shilimkar [off-list ref]
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
Doing a bisect results in this commit:
commit 0c7018e232c5526869250e57da8043a86a45b5de
Author: Rajendra Nayak [off-list ref]
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 [off-list ref]
Signed-off-by: Tero Kristo [off-list ref]
Reviewed-by: Santosh Shilimkar [off-list ref]
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>
Doing a bisect results in this commit:
commit 0c7018e232c5526869250e57da8043a86a45b5de
Author: Rajendra Nayak [off-list ref]
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 [off-list ref]
Signed-off-by: Tero Kristo [off-list ref]
Reviewed-by: Santosh Shilimkar [off-list ref]
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
From: Tomi Valkeinen <hidden> Date: 2012-11-22 14:44:43
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)
quoted
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.
From: Tero Kristo <hidden> Date: 2012-11-23 09:34:38
On Thu, 2012-11-22 at 16:44 +0200, Tomi Valkeinen wrote:
On 2012-11-22 16:34, Tero Kristo wrote:
quoted
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)
quoted
quoted
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
On Friday 23 November 2012 03:04 PM, Tero Kristo wrote:
On Thu, 2012-11-22 at 16:44 +0200, Tomi Valkeinen wrote:
quoted
On 2012-11-22 16:34, Tero Kristo wrote:
quoted
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)
quoted
quoted
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
Hi,
On Monday 26 November 2012 12:18 PM, Archit Taneja wrote:
On Friday 23 November 2012 03:04 PM, Tero Kristo wrote:
quoted
On Thu, 2012-11-22 at 16:44 +0200, Tomi Valkeinen wrote:
quoted
On 2012-11-22 16:34, Tero Kristo wrote:
quoted
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)
quoted
quoted
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 <redacted>
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 <redacted>
---
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
From: Tomi Valkeinen <hidden> Date: 2012-11-27 11:23:35
On 2012-11-26 14:14, Archit Taneja wrote:
quoted hunk
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 <redacted>
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 <redacted>
---
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
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>
On Tuesday 27 November 2012 04:53 PM, Tomi Valkeinen wrote:
On 2012-11-26 14:14, Archit Taneja wrote:
quoted
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 <redacted>
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 <redacted>
---
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
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
From: Tomi Valkeinen <hidden> Date: 2012-11-27 12:21:25
On 2012-11-27 13:56, Archit Taneja wrote:
On Tuesday 27 November 2012 04:53 PM, Tomi Valkeinen wrote:
quoted
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.
From: Tero Kristo <hidden> Date: 2012-11-27 12:31:46
On Tue, 2012-11-27 at 14:21 +0200, Tomi Valkeinen wrote:
On 2012-11-27 13:56, Archit Taneja wrote:
quoted
On Tuesday 27 November 2012 04:53 PM, Tomi Valkeinen wrote:
quoted
quoted
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
On Tuesday 27 November 2012 06:01 PM, Tero Kristo wrote:
On Tue, 2012-11-27 at 14:21 +0200, Tomi Valkeinen wrote:
quoted
On 2012-11-27 13:56, Archit Taneja wrote:
quoted
On Tuesday 27 November 2012 04:53 PM, Tomi Valkeinen wrote:
quoted
quoted
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
quoted
quoted
quoted
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