Re: [PATCH 05/32] OMAPDSS: fix dss_get_ctx_loss_count for DT

2013-05-31 Thread Grygorii Strashko

On 05/30/2013 07:36 PM, Kevin Hilman wrote:

Tomi Valkeinen tomi.valkei...@ti.com writes:


When using DT, dss device does not have platform data. However,
dss_get_ctx_loss_count() uses dss device's platform data to find the
get_ctx_loss_count function pointer.

To fix this, dss_get_ctx_loss_count() needs to be changed to get the
platform data from the omapdss device, which is a virtual device and
always has platform data.

Dumb question (since the DSS device model has always been beyond my
grasp): how/why does the virtual device still have platform_data on a DT
boot?

Also, this context_loss_count and DT boot is starting to get cumbersome,
and there's currently no (good) way of handling the context loss in a
generic way without pdata function pointers.

I'm starting to think we should move towards dropping this OMAP-specific
context loss counting, and just assume context is always lost.  If there
are performance problems, runtime PM autosuspend timeouts can be used to
avoid the transitions.

Or, on some devices, I suspect comparing a few registers against their
power-on reset values might be a quicker way of detecting lost context
and would avoid having to call into platform code all together.

Hi Kevin,
I've a question:
Why don't add API in Kernel/Base PM framework for context lost detection?
Like pm_was_ctx_lost().

If this topic has been discussed already (many times)) - sorry for the 
noise.
And I'll be very appreciate if you can point me on corresponding 
discussions.


Kevin


Signed-off-by: Tomi Valkeinen tomi.valkei...@ti.com
---
  drivers/video/omap2/dss/dss.c | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/video/omap2/dss/dss.c b/drivers/video/omap2/dss/dss.c
index 94f66f9..bd01608 100644
--- a/drivers/video/omap2/dss/dss.c
+++ b/drivers/video/omap2/dss/dss.c
@@ -157,7 +157,8 @@ static void dss_restore_context(void)
  
  int dss_get_ctx_loss_count(void)

  {
-   struct omap_dss_board_info *board_data = dss.pdev-dev.platform_data;
+   struct platform_device *core_pdev = dss_get_core_pdev();
+   struct omap_dss_board_info *board_data = core_pdev-dev.platform_data;
int cnt;
  
  	if (!board_data-get_context_loss_count)

--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 05/32] OMAPDSS: fix dss_get_ctx_loss_count for DT

2013-05-31 Thread Kevin Hilman
Hi Grygorii,

Grygorii Strashko grygorii.stras...@ti.com writes:

[...]

 I've a question:
 Why don't add API in Kernel/Base PM framework for context lost detection?
 Like pm_was_ctx_lost().

I'm not aware of any proposals to do this in a generic way.  Feel free
to propose one if you're so moved.

One complexity is that such a call is not needed on platforms that use
generic power domains (not OMAP). With gen_pd, the runtime PM callbacks
do not get called unless the power domains actually lose power.  So for
gen_pd, you assume context has been lost when your runtime PM callbacks
are called.

This is a bit different than how we've done it on OMAP, and there's been
some work in exploring how to adapt OMAP to gen_pd, but there's not
anyone actively working on it at the moment.

That being said, I'm still leaning towards simply removing the context
loss logic from all the OMAP drivers.  This feature adds a relatively
minor optimization, and a more configurable optimization could be done
using runtime PM autosuspend timeouts (configurable from userspace.)
Combined with the fact that adoption of gen_pd means we don't need it,
I'm included to drop it all together.

Kevin
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 05/32] OMAPDSS: fix dss_get_ctx_loss_count for DT

2013-05-30 Thread Tomi Valkeinen
When using DT, dss device does not have platform data. However,
dss_get_ctx_loss_count() uses dss device's platform data to find the
get_ctx_loss_count function pointer.

To fix this, dss_get_ctx_loss_count() needs to be changed to get the
platform data from the omapdss device, which is a virtual device and
always has platform data.

Signed-off-by: Tomi Valkeinen tomi.valkei...@ti.com
---
 drivers/video/omap2/dss/dss.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/video/omap2/dss/dss.c b/drivers/video/omap2/dss/dss.c
index 94f66f9..bd01608 100644
--- a/drivers/video/omap2/dss/dss.c
+++ b/drivers/video/omap2/dss/dss.c
@@ -157,7 +157,8 @@ static void dss_restore_context(void)
 
 int dss_get_ctx_loss_count(void)
 {
-   struct omap_dss_board_info *board_data = dss.pdev-dev.platform_data;
+   struct platform_device *core_pdev = dss_get_core_pdev();
+   struct omap_dss_board_info *board_data = core_pdev-dev.platform_data;
int cnt;
 
if (!board_data-get_context_loss_count)
-- 
1.8.1.2

--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 05/32] OMAPDSS: fix dss_get_ctx_loss_count for DT

2013-05-30 Thread Tomi Valkeinen
On 30/05/13 14:09, Jean-Christophe PLAGNIOL-VILLARD wrote:
 On 12:34 Thu 30 May , Tomi Valkeinen wrote:
 When using DT, dss device does not have platform data. However,
 dss_get_ctx_loss_count() uses dss device's platform data to find the
 get_ctx_loss_count function pointer.

 To fix this, dss_get_ctx_loss_count() needs to be changed to get the
 platform data from the omapdss device, which is a virtual device and
 always has platform data.

 Signed-off-by: Tomi Valkeinen tomi.valkei...@ti.com
 ---
  drivers/video/omap2/dss/dss.c | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

 diff --git a/drivers/video/omap2/dss/dss.c b/drivers/video/omap2/dss/dss.c
 index 94f66f9..bd01608 100644
 --- a/drivers/video/omap2/dss/dss.c
 +++ b/drivers/video/omap2/dss/dss.c
 @@ -157,7 +157,8 @@ static void dss_restore_context(void)
  
  int dss_get_ctx_loss_count(void)
  {
 -struct omap_dss_board_info *board_data = dss.pdev-dev.platform_data;
 +struct platform_device *core_pdev = dss_get_core_pdev();
 +struct omap_dss_board_info *board_data = core_pdev-dev.platform_data;
 
   how about store the pdata in the drivers internal struct and if !dt
   you ust do this
 
   dss_dev-pdata = *pdev-dev.platform_data;
 
   to copy it and we do not alloc it for dt

It's not quite that simple. We need some OMAP arch functions (like
get_ctx_loss_count here) that are not currently exposed via any other
method to drivers except passing a function pointer with platform data.
We need that also when booting with DT.

We have a bunch of devices for the display subsystem hardware blocks,
like the dss here. When booting with DT, these blocks are represented
in the DT data, and do not have platform data.

We also have a virtual device, omapdss, which doesn't match any hw
block. It's created in the arch setup stage. It's really a legacy thing,
but with DT it can be used conveniently to pass the platform data.

The problem this patch fixes is that we used to pass the arch functions
for each of those HW block drivers. But with DT, we need to get the arch
functions from the omapdss device, gotten with dss_get_core_pdev().

 Tomi




signature.asc
Description: OpenPGP digital signature


Re: [PATCH 05/32] OMAPDSS: fix dss_get_ctx_loss_count for DT

2013-05-30 Thread Jean-Christophe PLAGNIOL-VILLARD
On 12:34 Thu 30 May , Tomi Valkeinen wrote:
 When using DT, dss device does not have platform data. However,
 dss_get_ctx_loss_count() uses dss device's platform data to find the
 get_ctx_loss_count function pointer.
 
 To fix this, dss_get_ctx_loss_count() needs to be changed to get the
 platform data from the omapdss device, which is a virtual device and
 always has platform data.
 
 Signed-off-by: Tomi Valkeinen tomi.valkei...@ti.com
 ---
  drivers/video/omap2/dss/dss.c | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)
 
 diff --git a/drivers/video/omap2/dss/dss.c b/drivers/video/omap2/dss/dss.c
 index 94f66f9..bd01608 100644
 --- a/drivers/video/omap2/dss/dss.c
 +++ b/drivers/video/omap2/dss/dss.c
 @@ -157,7 +157,8 @@ static void dss_restore_context(void)
  
  int dss_get_ctx_loss_count(void)
  {
 - struct omap_dss_board_info *board_data = dss.pdev-dev.platform_data;
 + struct platform_device *core_pdev = dss_get_core_pdev();
 + struct omap_dss_board_info *board_data = core_pdev-dev.platform_data;

how about store the pdata in the drivers internal struct and if !dt
you ust do this

dss_dev-pdata = *pdev-dev.platform_data;

to copy it and we do not alloc it for dt

Best Regards,
J.
   int cnt;
  
   if (!board_data-get_context_loss_count)
 -- 
 1.8.1.2
 
 --
 To unsubscribe from this list: send the line unsubscribe linux-fbdev in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 05/32] OMAPDSS: fix dss_get_ctx_loss_count for DT

2013-05-30 Thread Jean-Christophe PLAGNIOL-VILLARD
On 14:28 Thu 30 May , Tomi Valkeinen wrote:
 On 30/05/13 14:09, Jean-Christophe PLAGNIOL-VILLARD wrote:
  On 12:34 Thu 30 May , Tomi Valkeinen wrote:
  When using DT, dss device does not have platform data. However,
  dss_get_ctx_loss_count() uses dss device's platform data to find the
  get_ctx_loss_count function pointer.
 
  To fix this, dss_get_ctx_loss_count() needs to be changed to get the
  platform data from the omapdss device, which is a virtual device and
  always has platform data.
 
  Signed-off-by: Tomi Valkeinen tomi.valkei...@ti.com
  ---
   drivers/video/omap2/dss/dss.c | 3 ++-
   1 file changed, 2 insertions(+), 1 deletion(-)
 
  diff --git a/drivers/video/omap2/dss/dss.c b/drivers/video/omap2/dss/dss.c
  index 94f66f9..bd01608 100644
  --- a/drivers/video/omap2/dss/dss.c
  +++ b/drivers/video/omap2/dss/dss.c
  @@ -157,7 +157,8 @@ static void dss_restore_context(void)
   
   int dss_get_ctx_loss_count(void)
   {
  -  struct omap_dss_board_info *board_data = dss.pdev-dev.platform_data;
  +  struct platform_device *core_pdev = dss_get_core_pdev();
  +  struct omap_dss_board_info *board_data = core_pdev-dev.platform_data;
  
  how about store the pdata in the drivers internal struct and if !dt
  you ust do this
  
  dss_dev-pdata = *pdev-dev.platform_data;
  
  to copy it and we do not alloc it for dt
 
 It's not quite that simple. We need some OMAP arch functions (like
 get_ctx_loss_count here) that are not currently exposed via any other
 method to drivers except passing a function pointer with platform data.
 We need that also when booting with DT.
 
 We have a bunch of devices for the display subsystem hardware blocks,
 like the dss here. When booting with DT, these blocks are represented
 in the DT data, and do not have platform data.
 
 We also have a virtual device, omapdss, which doesn't match any hw
 block. It's created in the arch setup stage. It's really a legacy thing,
 but with DT it can be used conveniently to pass the platform data.
 
 The problem this patch fixes is that we used to pass the arch functions
 for each of those HW block drivers. But with DT, we need to get the arch
 functions from the omapdss device, gotten with dss_get_core_pdev().

ok

do not take it bad is it worth the effort those 54 patches?

is not better to work on DRM?
just an open question

Best Regards,
J.
 
  Tomi
 
 


--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 05/32] OMAPDSS: fix dss_get_ctx_loss_count for DT

2013-05-30 Thread Tomi Valkeinen
On 30/05/13 18:43, Jean-Christophe PLAGNIOL-VILLARD wrote:
 On 14:28 Thu 30 May , Tomi Valkeinen wrote:
 On 30/05/13 14:09, Jean-Christophe PLAGNIOL-VILLARD wrote:
 On 12:34 Thu 30 May , Tomi Valkeinen wrote:
 When using DT, dss device does not have platform data. However,
 dss_get_ctx_loss_count() uses dss device's platform data to find the
 get_ctx_loss_count function pointer.

 To fix this, dss_get_ctx_loss_count() needs to be changed to get the
 platform data from the omapdss device, which is a virtual device and
 always has platform data.

 Signed-off-by: Tomi Valkeinen tomi.valkei...@ti.com
 ---
  drivers/video/omap2/dss/dss.c | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

 diff --git a/drivers/video/omap2/dss/dss.c b/drivers/video/omap2/dss/dss.c
 index 94f66f9..bd01608 100644
 --- a/drivers/video/omap2/dss/dss.c
 +++ b/drivers/video/omap2/dss/dss.c
 @@ -157,7 +157,8 @@ static void dss_restore_context(void)
  
  int dss_get_ctx_loss_count(void)
  {
 -  struct omap_dss_board_info *board_data = dss.pdev-dev.platform_data;
 +  struct platform_device *core_pdev = dss_get_core_pdev();
 +  struct omap_dss_board_info *board_data = core_pdev-dev.platform_data;

 how about store the pdata in the drivers internal struct and if !dt
 you ust do this

 dss_dev-pdata = *pdev-dev.platform_data;

 to copy it and we do not alloc it for dt

 It's not quite that simple. We need some OMAP arch functions (like
 get_ctx_loss_count here) that are not currently exposed via any other
 method to drivers except passing a function pointer with platform data.
 We need that also when booting with DT.

 We have a bunch of devices for the display subsystem hardware blocks,
 like the dss here. When booting with DT, these blocks are represented
 in the DT data, and do not have platform data.

 We also have a virtual device, omapdss, which doesn't match any hw
 block. It's created in the arch setup stage. It's really a legacy thing,
 but with DT it can be used conveniently to pass the platform data.

 The problem this patch fixes is that we used to pass the arch functions
 for each of those HW block drivers. But with DT, we need to get the arch
 functions from the omapdss device, gotten with dss_get_core_pdev().
 
 ok
 
 do not take it bad is it worth the effort those 54 patches?
 
 is not better to work on DRM?
 just an open question

Both omapfb and omapdrm use omapdss. omapdss provides the HW level
support, and also panel support. At some point in the future we'll
probably deprecate omapfb, and move wholly to omapdrm. At that point
omapdss and omapdrm can be merged together, simplifying the design.

And regarding the amount of the patches, there has been some bad design
decisions in the history of omapdss, and as it's a big driver (plus the
panel drivers), it takes quite a bit to fix these. There will be more
coming to convert the rest of the panel drivers, and also to implement
DT support. But those all also work for omapdrm, so it's not fbdev-only
stuff.

 Tomi




signature.asc
Description: OpenPGP digital signature


Re: [PATCH 05/32] OMAPDSS: fix dss_get_ctx_loss_count for DT

2013-05-30 Thread Kevin Hilman
Tomi Valkeinen tomi.valkei...@ti.com writes:

 When using DT, dss device does not have platform data. However,
 dss_get_ctx_loss_count() uses dss device's platform data to find the
 get_ctx_loss_count function pointer.

 To fix this, dss_get_ctx_loss_count() needs to be changed to get the
 platform data from the omapdss device, which is a virtual device and
 always has platform data.

Dumb question (since the DSS device model has always been beyond my
grasp): how/why does the virtual device still have platform_data on a DT
boot?

Also, this context_loss_count and DT boot is starting to get cumbersome,
and there's currently no (good) way of handling the context loss in a
generic way without pdata function pointers.

I'm starting to think we should move towards dropping this OMAP-specific
context loss counting, and just assume context is always lost.  If there
are performance problems, runtime PM autosuspend timeouts can be used to
avoid the transitions.

Or, on some devices, I suspect comparing a few registers against their
power-on reset values might be a quicker way of detecting lost context
and would avoid having to call into platform code all together.

Kevin

 Signed-off-by: Tomi Valkeinen tomi.valkei...@ti.com
 ---
  drivers/video/omap2/dss/dss.c | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

 diff --git a/drivers/video/omap2/dss/dss.c b/drivers/video/omap2/dss/dss.c
 index 94f66f9..bd01608 100644
 --- a/drivers/video/omap2/dss/dss.c
 +++ b/drivers/video/omap2/dss/dss.c
 @@ -157,7 +157,8 @@ static void dss_restore_context(void)
  
  int dss_get_ctx_loss_count(void)
  {
 - struct omap_dss_board_info *board_data = dss.pdev-dev.platform_data;
 + struct platform_device *core_pdev = dss_get_core_pdev();
 + struct omap_dss_board_info *board_data = core_pdev-dev.platform_data;
   int cnt;
  
   if (!board_data-get_context_loss_count)
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 05/32] OMAPDSS: fix dss_get_ctx_loss_count for DT

2013-05-30 Thread Tomi Valkeinen
On 30/05/13 19:36, Kevin Hilman wrote:
 Tomi Valkeinen tomi.valkei...@ti.com writes:
 
 When using DT, dss device does not have platform data. However,
 dss_get_ctx_loss_count() uses dss device's platform data to find the
 get_ctx_loss_count function pointer.

 To fix this, dss_get_ctx_loss_count() needs to be changed to get the
 platform data from the omapdss device, which is a virtual device and
 always has platform data.
 
 Dumb question (since the DSS device model has always been beyond my
 grasp): how/why does the virtual device still have platform_data on a DT
 boot?

The virtual device will be created in generic omap arch code for DT boot
also.

The omapdss virtual device is an old relic. Originally we only had the
omapdss device, not the sub-devices for HW block like we have now. After
adding the sub-devices, omapdss device was still left, as it provided
useful functionality. It wasn't strictly needed anymore, but I've just
never had time to start refactoring code to remove it.

Now, with DT, it was just an easy way around to pass the func pointers.
Note that we also pass set_min_bus_tput, and a version identifier for
the DSS HW. The PM funcs could perhaps be handled some other way, but
I'm not sure how I could pass the DSS HW version.

 Also, this context_loss_count and DT boot is starting to get cumbersome,
 and there's currently no (good) way of handling the context loss in a
 generic way without pdata function pointers.
 
 I'm starting to think we should move towards dropping this OMAP-specific
 context loss counting, and just assume context is always lost.  If there
 are performance problems, runtime PM autosuspend timeouts can be used to
 avoid the transitions.
 
 Or, on some devices, I suspect comparing a few registers against their
 power-on reset values might be a quicker way of detecting lost context
 and would avoid having to call into platform code all together.

Hmm, true. I'll look at this. Maybe I won't need get_ctx_loss in omapdss
at all.

How about omap_pm_set_min_bus_tput(), can that be handled in some
generic manner? Although, we currently use that in a bit hacky case.
What we're doing is not really setting min bus tput, but we're just
preventing OPP50.

The DSS clocks have different maximums on OPP50 than on OPP100, and we
need to keep the core in OPP100 to keep DSS working. So what I'd rather
use is prevent_opp50() than setting arbitrarily high min bus tput.

 Tomi




signature.asc
Description: OpenPGP digital signature