[v3,4/4] OMAP_VOUT: Don't trigger updates in omap_vout_probe

Message ID 1317038365-30650-5-git-send-email-archit@ti.com (mailing list archive)
State Superseded, archived
Headers

Commit Message

Archit Taneja Sept. 26, 2011, 11:59 a.m. UTC
  Remove the code in omap_vout_probe() which calls display->driver->update() for
all the displays. This isn't correct because:

- An update in probe doesn't make sense, because we don't have any valid content
  to show at this time.
- Calling update for a panel which isn't enabled is not supported by DSS2. This
  leads to a crash at probe.

Signed-off-by: Archit Taneja <archit@ti.com>
---
 drivers/media/video/omap/omap_vout.c |    8 --------
 1 files changed, 0 insertions(+), 8 deletions(-)
  

Comments

Tomi Valkeinen Sept. 27, 2011, 6:10 a.m. UTC | #1
On Mon, 2011-09-26 at 17:29 +0530, Archit Taneja wrote:
> Remove the code in omap_vout_probe() which calls display->driver->update() for
> all the displays. This isn't correct because:
> 
> - An update in probe doesn't make sense, because we don't have any valid content
>   to show at this time.
> - Calling update for a panel which isn't enabled is not supported by DSS2. This
>   leads to a crash at probe.

Calling update() on a disabled panel should not crash... Where is the
crash coming from?

 Tomi


--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
  
Hiremath, Vaibhav Sept. 27, 2011, 6:26 a.m. UTC | #2
> -----Original Message-----
> From: Taneja, Archit
> Sent: Monday, September 26, 2011 5:29 PM
> To: Hiremath, Vaibhav
> Cc: Valkeinen, Tomi; linux-omap@vger.kernel.org; Semwal, Sumit; linux-
> media@vger.kernel.org; Taneja, Archit
> Subject: [PATCH v3 4/4] OMAP_VOUT: Don't trigger updates in
> omap_vout_probe
> 
> Remove the code in omap_vout_probe() which calls display->driver->update()
> for
> all the displays. This isn't correct because:
> 
> - An update in probe doesn't make sense, because we don't have any valid
> content
>   to show at this time.
> - Calling update for a panel which isn't enabled is not supported by DSS2.
> This
>   leads to a crash at probe.
> 
It should not crash, do you have crash log here?

Thanks,
Vaibhav

> Signed-off-by: Archit Taneja <archit@ti.com>
> ---
>  drivers/media/video/omap/omap_vout.c |    8 --------
>  1 files changed, 0 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/media/video/omap/omap_vout.c
> b/drivers/media/video/omap/omap_vout.c
> index 7b8e87a..3d9c83e 100644
> --- a/drivers/media/video/omap/omap_vout.c
> +++ b/drivers/media/video/omap/omap_vout.c
> @@ -2213,14 +2213,6 @@ static int __init omap_vout_probe(struct
> platform_device *pdev)
>  	if (ret)
>  		goto probe_err2;
> 
> -	for (i = 0; i < vid_dev->num_displays; i++) {
> -		struct omap_dss_device *display = vid_dev->displays[i];
> -
> -		if (display->driver->update)
> -			display->driver->update(display, 0, 0,
> -					display->panel.timings.x_res,
> -					display->panel.timings.y_res);
> -	}
>  	return 0;
> 
>  probe_err2:
> --
> 1.7.1

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
  
Archit Taneja Sept. 27, 2011, 7:02 a.m. UTC | #3
On Tuesday 27 September 2011 11:40 AM, Valkeinen, Tomi wrote:
> On Mon, 2011-09-26 at 17:29 +0530, Archit Taneja wrote:
>> Remove the code in omap_vout_probe() which calls display->driver->update() for
>> all the displays. This isn't correct because:
>>
>> - An update in probe doesn't make sense, because we don't have any valid content
>>    to show at this time.
>> - Calling update for a panel which isn't enabled is not supported by DSS2. This
>>    leads to a crash at probe.
>
> Calling update() on a disabled panel should not crash... Where is the
> crash coming from?

you are right, the crash isn't coming from the updates. I see the crash 
when we have 4 dss devices in our board file. The last display pointer 
is corrupted in that case. I'm trying to figure out why.

Archit

>
>   Tomi
>
>

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
  
Tomi Valkeinen Sept. 27, 2011, 7:07 a.m. UTC | #4
On Tue, 2011-09-27 at 12:32 +0530, Archit Taneja wrote:
> On Tuesday 27 September 2011 11:40 AM, Valkeinen, Tomi wrote:
> > On Mon, 2011-09-26 at 17:29 +0530, Archit Taneja wrote:
> >> Remove the code in omap_vout_probe() which calls display->driver->update() for
> >> all the displays. This isn't correct because:
> >>
> >> - An update in probe doesn't make sense, because we don't have any valid content
> >>    to show at this time.
> >> - Calling update for a panel which isn't enabled is not supported by DSS2. This
> >>    leads to a crash at probe.
> >
> > Calling update() on a disabled panel should not crash... Where is the
> > crash coming from?
> 
> you are right, the crash isn't coming from the updates. I see the crash 
> when we have 4 dss devices in our board file. The last display pointer 
> is corrupted in that case. I'm trying to figure out why.

Could be totally unrelated, but does the V4L2 driver make sure that the
used dss devices have a driver loaded?

OMAPFB previously refused to start if all the devices do not have a
driver, but nowadays it starts fine by skipping the devices without a
driver.

 Tomi


--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
  
Archit Taneja Sept. 27, 2011, 7:15 a.m. UTC | #5
On Tuesday 27 September 2011 12:37 PM, Valkeinen, Tomi wrote:
> On Tue, 2011-09-27 at 12:32 +0530, Archit Taneja wrote:
>> On Tuesday 27 September 2011 11:40 AM, Valkeinen, Tomi wrote:
>>> On Mon, 2011-09-26 at 17:29 +0530, Archit Taneja wrote:
>>>> Remove the code in omap_vout_probe() which calls display->driver->update() for
>>>> all the displays. This isn't correct because:
>>>>
>>>> - An update in probe doesn't make sense, because we don't have any valid content
>>>>     to show at this time.
>>>> - Calling update for a panel which isn't enabled is not supported by DSS2. This
>>>>     leads to a crash at probe.
>>>
>>> Calling update() on a disabled panel should not crash... Where is the
>>> crash coming from?
>>
>> you are right, the crash isn't coming from the updates. I see the crash
>> when we have 4 dss devices in our board file. The last display pointer
>> is corrupted in that case. I'm trying to figure out why.
>
> Could be totally unrelated, but does the V4L2 driver make sure that the
> used dss devices have a driver loaded?
>
> OMAPFB previously refused to start if all the devices do not have a
> driver, but nowadays it starts fine by skipping the devices without a
> driver.

The drivers were loaded in. The issue was something else totally. I 
assumed it was something related to update call.

In drivers/media/video/omap/omap_voutdef.h:

struct omap2video_device {
...
...
struct omap_dss_device *displays[MAX_DISPLAYS];
...
...
};

MAX_DISPLAYS is 3, so the 4th display pointer was getting messed up 
wherever we used it.

I guess we don't need this patch. We may want to set MAX_DISPLAYS to a 
higher number i guess, because we could theoretically register as many 
panels as we want, and set/unset them.

Thanks,
Archit

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

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

Patch

diff --git a/drivers/media/video/omap/omap_vout.c b/drivers/media/video/omap/omap_vout.c
index 7b8e87a..3d9c83e 100644
--- a/drivers/media/video/omap/omap_vout.c
+++ b/drivers/media/video/omap/omap_vout.c
@@ -2213,14 +2213,6 @@  static int __init omap_vout_probe(struct platform_device *pdev)
 	if (ret)
 		goto probe_err2;
 
-	for (i = 0; i < vid_dev->num_displays; i++) {
-		struct omap_dss_device *display = vid_dev->displays[i];
-
-		if (display->driver->update)
-			display->driver->update(display, 0, 0,
-					display->panel.timings.x_res,
-					display->panel.timings.y_res);
-	}
 	return 0;
 
 probe_err2: