Re: [PATCH v2 20/23] OMAPDSS: MANAGER: Update display sysfs store

2012-08-31 Thread Tomi Valkeinen
On Thu, 2012-08-30 at 17:10 +0530, Archit Taneja wrote:
 The display sysfs attribute's store function needs to be changed with the
 introduction of outputs.
 
 Providing a manager to the display isn't enough to create a link now, the
 manager needs and output to connect to. A manager's display store file only
 has the information of the manager and the desired display, it is not aware
 of which output should the manager connect to.
 
 Because of this, a new constraint needs to be set up when setting a display 
 via
 this sysfs file. The constraint is that the desired display should already be
 connected to an output before calling this sysfs function.
 
 This might break some existing user space stuff which uses sysfs directly. But
 in most cases dss_recheck_connections will connect displays to floating 
 outputs.
 DSS sysfs files are being planned to be remove anyway, so it's not much of a
 harm.
 
 Signed-off-by: Archit Taneja arc...@ti.com
 ---
  drivers/video/omap2/dss/manager.c |   25 -
  1 file changed, 20 insertions(+), 5 deletions(-)
 
 diff --git a/drivers/video/omap2/dss/manager.c 
 b/drivers/video/omap2/dss/manager.c
 index fd39f66..d808ce2 100644
 --- a/drivers/video/omap2/dss/manager.c
 +++ b/drivers/video/omap2/dss/manager.c
 @@ -74,18 +74,33 @@ static ssize_t manager_display_store(struct 
 omap_overlay_manager *mgr,
   if (dssdev)
   DSSDBG(display %s found\n, dssdev-name);
  
 - if (mgr-get_device(mgr)) {
 - r = mgr-unset_device(mgr);
 + if (mgr-output) {
 + if (mgr-output-device) {
 + r = mgr-output-unset_device(mgr-output);
 + if (r) {
 + goto put_device;
 + DSSERR(failed to unset device from output\n);
 + }
 + }
 +
 + r = mgr-unset_output(mgr);
   if (r) {
 - DSSERR(failed to unset display\n);
 + DSSERR(failed to unset current output\n);
   goto put_device;
   }
   }
  
   if (dssdev) {
 - r = mgr-set_device(mgr, dssdev);
 + struct omap_dss_output *out = dssdev-output;
 +
 + if (!out) {
 + DSSERR(no output connected to device\n);
 + goto put_device;
 + }
 +
 + r = mgr-set_output(mgr, out);
   if (r) {
 - DSSERR(failed to set manager\n);
 + DSSERR(failed to set manager output\n);
   goto put_device;
   }
  

Hmm, I think this is a bit broken. If I read this right, the unlinking
removes both mgr-output link and the output-dssdev link. But the linking
part only sets up the mgr-output link.

So if there's a very simple configuration with one display, if you
unlink it via sysfs you can't link it back again.

The store function gets the mgr and the dssdev as arguments. I think
this could be changed so that the function would guess the needed
output. Well, not so much a guess, because a dssdev can only be
connected to one output, defined by the HW design. That is basically
what the recheck_connections does, we could use the same method here.

Then this sysfs file should work just like before.

 Tomi



signature.asc
Description: This is a digitally signed message part


Re: [PATCH v2 20/23] OMAPDSS: MANAGER: Update display sysfs store

2012-08-31 Thread Archit Taneja

On Friday 31 August 2012 08:00 PM, Tomi Valkeinen wrote:

On Thu, 2012-08-30 at 17:10 +0530, Archit Taneja wrote:

The display sysfs attribute's store function needs to be changed with the
introduction of outputs.

Providing a manager to the display isn't enough to create a link now, the
manager needs and output to connect to. A manager's display store file only
has the information of the manager and the desired display, it is not aware
of which output should the manager connect to.

Because of this, a new constraint needs to be set up when setting a display via
this sysfs file. The constraint is that the desired display should already be
connected to an output before calling this sysfs function.

This might break some existing user space stuff which uses sysfs directly. But
in most cases dss_recheck_connections will connect displays to floating outputs.
DSS sysfs files are being planned to be remove anyway, so it's not much of a
harm.

Signed-off-by: Archit Taneja arc...@ti.com
---
  drivers/video/omap2/dss/manager.c |   25 -
  1 file changed, 20 insertions(+), 5 deletions(-)

diff --git a/drivers/video/omap2/dss/manager.c 
b/drivers/video/omap2/dss/manager.c
index fd39f66..d808ce2 100644
--- a/drivers/video/omap2/dss/manager.c
+++ b/drivers/video/omap2/dss/manager.c
@@ -74,18 +74,33 @@ static ssize_t manager_display_store(struct 
omap_overlay_manager *mgr,
if (dssdev)
DSSDBG(display %s found\n, dssdev-name);

-   if (mgr-get_device(mgr)) {
-   r = mgr-unset_device(mgr);
+   if (mgr-output) {
+   if (mgr-output-device) {
+   r = mgr-output-unset_device(mgr-output);
+   if (r) {
+   goto put_device;
+   DSSERR(failed to unset device from output\n);
+   }
+   }
+
+   r = mgr-unset_output(mgr);
if (r) {
-   DSSERR(failed to unset display\n);
+   DSSERR(failed to unset current output\n);
goto put_device;
}
}

if (dssdev) {
-   r = mgr-set_device(mgr, dssdev);
+   struct omap_dss_output *out = dssdev-output;
+
+   if (!out) {
+   DSSERR(no output connected to device\n);
+   goto put_device;
+   }
+
+   r = mgr-set_output(mgr, out);
if (r) {
-   DSSERR(failed to set manager\n);
+   DSSERR(failed to set manager output\n);
goto put_device;
}



Hmm, I think this is a bit broken. If I read this right, the unlinking
removes both mgr-output link and the output-dssdev link. But the linking
part only sets up the mgr-output link.

So if there's a very simple configuration with one display, if you
unlink it via sysfs you can't link it back again.


Ah, right. You might need to reinsert the display module again to get 
the output-display link again. Which is bad. Or have a sysfs file for 
setting manager output. Which is something we want to stay away from anyway.




The store function gets the mgr and the dssdev as arguments. I think
this could be changed so that the function would guess the needed
output. Well, not so much a guess, because a dssdev can only be
connected to one output, defined by the HW design. That is basically
what the recheck_connections does, we could use the same method here.

Then this sysfs file should work just like before.


That's a good idea, we could reuse the code in recheck_connections a 
bit. I wanted to stay away from the guessing. But I think we need to do 
it if a simple unlink-link of a display itself fails.


Archit

--
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 v2 20/23] OMAPDSS: MANAGER: Update display sysfs store

2012-08-30 Thread Archit Taneja
The display sysfs attribute's store function needs to be changed with the
introduction of outputs.

Providing a manager to the display isn't enough to create a link now, the
manager needs and output to connect to. A manager's display store file only
has the information of the manager and the desired display, it is not aware
of which output should the manager connect to.

Because of this, a new constraint needs to be set up when setting a display via
this sysfs file. The constraint is that the desired display should already be
connected to an output before calling this sysfs function.

This might break some existing user space stuff which uses sysfs directly. But
in most cases dss_recheck_connections will connect displays to floating outputs.
DSS sysfs files are being planned to be remove anyway, so it's not much of a
harm.

Signed-off-by: Archit Taneja arc...@ti.com
---
 drivers/video/omap2/dss/manager.c |   25 -
 1 file changed, 20 insertions(+), 5 deletions(-)

diff --git a/drivers/video/omap2/dss/manager.c 
b/drivers/video/omap2/dss/manager.c
index fd39f66..d808ce2 100644
--- a/drivers/video/omap2/dss/manager.c
+++ b/drivers/video/omap2/dss/manager.c
@@ -74,18 +74,33 @@ static ssize_t manager_display_store(struct 
omap_overlay_manager *mgr,
if (dssdev)
DSSDBG(display %s found\n, dssdev-name);
 
-   if (mgr-get_device(mgr)) {
-   r = mgr-unset_device(mgr);
+   if (mgr-output) {
+   if (mgr-output-device) {
+   r = mgr-output-unset_device(mgr-output);
+   if (r) {
+   goto put_device;
+   DSSERR(failed to unset device from output\n);
+   }
+   }
+
+   r = mgr-unset_output(mgr);
if (r) {
-   DSSERR(failed to unset display\n);
+   DSSERR(failed to unset current output\n);
goto put_device;
}
}
 
if (dssdev) {
-   r = mgr-set_device(mgr, dssdev);
+   struct omap_dss_output *out = dssdev-output;
+
+   if (!out) {
+   DSSERR(no output connected to device\n);
+   goto put_device;
+   }
+
+   r = mgr-set_output(mgr, out);
if (r) {
-   DSSERR(failed to set manager\n);
+   DSSERR(failed to set manager output\n);
goto put_device;
}
 
-- 
1.7.9.5

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