[PATCH] omap_vout: Set DSS overlay_info only if paddr is non zero

[PATCH] omap_vout: Set DSS overlay_info only if paddr is non zero

[Archit Taneja]

The omap_vout driver tries to set the DSS overlay_info using set_overlay_info()
when the physical address for the overlay is still not configured. This happens
in omap_vout_probe() and vidioc_s_fmt_vid_out().

The calls to omapvid_init(which internally calls set_overlay_info()) are removed
from these functions. They don't need to be called as the omap_vout_device
struct anyway maintains the overlay related changes made. Also, remove the
explicit call to set_overlay_info() in vidioc_streamon(), this was used to set
the paddr, this isn't needed as omapvid_init() does the same thing later.

These changes are required as the DSS2 driver since 3.3 kernel doesn't let you
set the overlay info with paddr as 0.

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

diff --git a/drivers/media/video/omap/omap_vout.c b/drivers/media/video/omap/omap_vout.c
index 1fb7d5b..dffcf66 100644
--- a/drivers/media/video/omap/omap_vout.c
+++ b/drivers/media/video/omap/omap_vout.c
@@ -1157,13 +1157,6 @@ static int vidioc_s_fmt_vid_out(struct file *file, void *fh,
 	/* set default crop and win */
 	omap_vout_new_format(&vout->pix, &vout->fbuf, &vout->crop, &vout->win);
 
-	/* Save the changes in the overlay strcuture */
-	ret = omapvid_init(vout, 0);
-	if (ret) {
-		v4l2_err(&vout->vid_dev->v4l2_dev, "failed to change mode\n");
-		goto s_fmt_vid_out_exit;
-	}
-
 	ret = 0;
 
 s_fmt_vid_out_exit:
@@ -1664,20 +1657,6 @@ static int vidioc_streamon(struct file *file, void *fh, enum v4l2_buf_type i)
 
 	omap_dispc_register_isr(omap_vout_isr, vout, mask);
 
-	for (j = 0; j < ovid->num_overlays; j++) {
-		struct omap_overlay *ovl = ovid->overlays[j];
-
-		if (ovl->manager && ovl->manager->device) {
-			struct omap_overlay_info info;
-			ovl->get_overlay_info(ovl, &info);
-			info.paddr = addr;
-			if (ovl->set_overlay_info(ovl, &info)) {
-				ret = -EINVAL;
-				goto streamon_err1;
-			}
-		}
-	}
-
 	/* First save the configuration in ovelray structure */
 	ret = omapvid_init(vout, addr);
 	if (ret)
@@ -2071,11 +2050,12 @@ static int __init omap_vout_create_video_devices(struct platform_device *pdev)
 		}
 		video_set_drvdata(vfd, vout);
 
-		/* Configure the overlay structure */
-		ret = omapvid_init(vid_dev->vouts[k], 0);
-		if (!ret)
-			goto success;
+		dev_info(&pdev->dev, ": registered and initialized"
+				" video device %d\n", vfd->minor);
+		if (k == (pdev->num_resources - 1))
+			return 0;
 
+		continue;
 error2:
 		if (vout->vid_info.rotation_type == VOUT_ROT_VRFB)
 			omap_vout_release_vrfb(vout);
@@ -2085,12 +2065,6 @@ error1:
 error:
 		kfree(vout);
 		return ret;
-
-success:
-		dev_info(&pdev->dev, ": registered and initialized"
-				" video device %d\n", vfd->minor);
-		if (k == (pdev->num_resources - 1))
-			return 0;
 	}
 
 	return -ENODEV;
-- 
1.7.5.4

[PATCH] omap_vout: Set DSS overlay_info only if paddr is non zero

[Archit Taneja]

The omap_vout driver tries to set the DSS overlay_info using set_overlay_info()
when the physical address for the overlay is still not configured. This happens
in omap_vout_probe() and vidioc_s_fmt_vid_out().

The calls to omapvid_init(which internally calls set_overlay_info()) are removed
from these functions. They don't need to be called as the omap_vout_device
struct anyway maintains the overlay related changes made. Also, remove the
explicit call to set_overlay_info() in vidioc_streamon(), this was used to set
the paddr, this isn't needed as omapvid_init() does the same thing later.

These changes are required as the DSS2 driver since 3.3 kernel doesn't let you
set the overlay info with paddr as 0.

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

diff --git a/drivers/media/video/omap/omap_vout.c b/drivers/media/video/omap/omap_vout.c
index 1fb7d5b..dffcf66 100644
--- a/drivers/media/video/omap/omap_vout.c
+++ b/drivers/media/video/omap/omap_vout.c
@@ -1157,13 +1157,6 @@ static int vidioc_s_fmt_vid_out(struct file *file, void *fh,
 	/* set default crop and win */
 	omap_vout_new_format(&vout->pix, &vout->fbuf, &vout->crop, &vout->win);
 
-	/* Save the changes in the overlay strcuture */
-	ret = omapvid_init(vout, 0);
-	if (ret) {
-		v4l2_err(&vout->vid_dev->v4l2_dev, "failed to change mode\n");
-		goto s_fmt_vid_out_exit;
-	}
-
 	ret = 0;
 
 s_fmt_vid_out_exit:
@@ -1664,20 +1657,6 @@ static int vidioc_streamon(struct file *file, void *fh, enum v4l2_buf_type i)
 
 	omap_dispc_register_isr(omap_vout_isr, vout, mask);
 
-	for (j = 0; j < ovid->num_overlays; j++) {
-		struct omap_overlay *ovl = ovid->overlays[j];
-
-		if (ovl->manager && ovl->manager->device) {
-			struct omap_overlay_info info;
-			ovl->get_overlay_info(ovl, &info);
-			info.paddr = addr;
-			if (ovl->set_overlay_info(ovl, &info)) {
-				ret = -EINVAL;
-				goto streamon_err1;
-			}
-		}
-	}
-
 	/* First save the configuration in ovelray structure */
 	ret = omapvid_init(vout, addr);
 	if (ret)
@@ -2071,11 +2050,12 @@ static int __init omap_vout_create_video_devices(struct platform_device *pdev)
 		}
 		video_set_drvdata(vfd, vout);
 
-		/* Configure the overlay structure */
-		ret = omapvid_init(vid_dev->vouts[k], 0);
-		if (!ret)
-			goto success;
+		dev_info(&pdev->dev, ": registered and initialized"
+				" video device %d\n", vfd->minor);
+		if (k == (pdev->num_resources - 1))
+			return 0;
 
+		continue;
 error2:
 		if (vout->vid_info.rotation_type == VOUT_ROT_VRFB)
 			omap_vout_release_vrfb(vout);
@@ -2085,12 +2065,6 @@ error1:
 error:
 		kfree(vout);
 		return ret;
-
-success:
-		dev_info(&pdev->dev, ": registered and initialized"
-				" video device %d\n", vfd->minor);
-		if (k == (pdev->num_resources - 1))
-			return 0;
 	}
 
 	return -ENODEV;
-- 
1.7.5.4

Re: [PATCH] omap_vout: Set DSS overlay_info only if paddr is non zero

[Laurent Pinchart]

Hi Archit,

On Wednesday 07 March 2012 14:31:16 Archit Taneja wrote:
> The omap_vout driver tries to set the DSS overlay_info using
> set_overlay_info() when the physical address for the overlay is still not
> configured. This happens in omap_vout_probe() and vidioc_s_fmt_vid_out().
> 
> The calls to omapvid_init(which internally calls set_overlay_info()) are
> removed from these functions. They don't need to be called as the
> omap_vout_device struct anyway maintains the overlay related changes made.
> Also, remove the explicit call to set_overlay_info() in vidioc_streamon(),
> this was used to set the paddr, this isn't needed as omapvid_init() does
> the same thing later.
> 
> These changes are required as the DSS2 driver since 3.3 kernel doesn't let
> you set the overlay info with paddr as 0.
> 
> Signed-off-by: Archit Taneja <archit@ti.com>

Thanks for the patch. This seems to fix memory corruption that would result
in sysfs-related crashes such as

[   31.279541] ------------[ cut here ]------------
[   31.284423] WARNING: at fs/sysfs/file.c:343 sysfs_open_file+0x70/0x1f8()
[   31.291503] missing sysfs attribute operations for kobject: (null)
[   31.298004] Modules linked in: mt9p031 aptina_pll omap3_isp
[   31.303924] [<c0018260>] (unwind_backtrace+0x0/0xec) from [<c0034488>] (warn_slowpath_common+0x4c/0x64)
[   31.313812] [<c0034488>] (warn_slowpath_common+0x4c/0x64) from [<c0034520>] (warn_slowpath_fmt+0x2c/0x3c)
[   31.323913] [<c0034520>] (warn_slowpath_fmt+0x2c/0x3c) from [<c01219bc>] (sysfs_open_file+0x70/0x1f8)
[   31.333618] [<c01219bc>] (sysfs_open_file+0x70/0x1f8) from [<c00ccc94>] (__dentry_open+0x1f8/0x30c)
[   31.343139] [<c00ccc94>] (__dentry_open+0x1f8/0x30c) from [<c00cce58>] (nameidata_to_filp+0x50/0x5c)
[   31.352752] [<c00cce58>] (nameidata_to_filp+0x50/0x5c) from [<c00db4c0>] (do_last+0x55c/0x6a0)
[   31.361999] [<c00db4c0>] (do_last+0x55c/0x6a0) from [<c00db6bc>] (path_openat+0xb8/0x37c)
[   31.370605] [<c00db6bc>] (path_openat+0xb8/0x37c) from [<c00dba60>] (do_filp_open+0x30/0x7c)
[   31.379486] [<c00dba60>] (do_filp_open+0x30/0x7c) from [<c00cc904>] (do_sys_open+0xd8/0x170)
[   31.388366] [<c00cc904>] (do_sys_open+0xd8/0x170) from [<c0012760>] (ret_fast_syscall+0x0/0x3c)
[   31.397552] ---[ end trace 13639ab74f345d7e ]---

Tested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Please push it to v3.3 :-)

-- 
Regards,

Laurent Pinchart

RE: [PATCH] omap_vout: Set DSS overlay_info only if paddr is non zero

[Hiremath, Vaibhav]

On Fri, Mar 09, 2012 at 05:17:41, Laurent Pinchart wrote:
> Hi Archit,
> 
> On Wednesday 07 March 2012 14:31:16 Archit Taneja wrote:
> > The omap_vout driver tries to set the DSS overlay_info using
> > set_overlay_info() when the physical address for the overlay is still not
> > configured. This happens in omap_vout_probe() and vidioc_s_fmt_vid_out().
> > 
> > The calls to omapvid_init(which internally calls set_overlay_info()) are
> > removed from these functions. They don't need to be called as the
> > omap_vout_device struct anyway maintains the overlay related changes made.
> > Also, remove the explicit call to set_overlay_info() in vidioc_streamon(),
> > this was used to set the paddr, this isn't needed as omapvid_init() does
> > the same thing later.
> > 
> > These changes are required as the DSS2 driver since 3.3 kernel doesn't let
> > you set the overlay info with paddr as 0.
> > 
> > Signed-off-by: Archit Taneja <archit@ti.com>
> 
> Thanks for the patch. This seems to fix memory corruption that would result
> in sysfs-related crashes such as
> 
> [   31.279541] ------------[ cut here ]------------
> [   31.284423] WARNING: at fs/sysfs/file.c:343 sysfs_open_file+0x70/0x1f8()
> [   31.291503] missing sysfs attribute operations for kobject: (null)
> [   31.298004] Modules linked in: mt9p031 aptina_pll omap3_isp
> [   31.303924] [<c0018260>] (unwind_backtrace+0x0/0xec) from [<c0034488>] (warn_slowpath_common+0x4c/0x64)
> [   31.313812] [<c0034488>] (warn_slowpath_common+0x4c/0x64) from [<c0034520>] (warn_slowpath_fmt+0x2c/0x3c)
> [   31.323913] [<c0034520>] (warn_slowpath_fmt+0x2c/0x3c) from [<c01219bc>] (sysfs_open_file+0x70/0x1f8)
> [   31.333618] [<c01219bc>] (sysfs_open_file+0x70/0x1f8) from [<c00ccc94>] (__dentry_open+0x1f8/0x30c)
> [   31.343139] [<c00ccc94>] (__dentry_open+0x1f8/0x30c) from [<c00cce58>] (nameidata_to_filp+0x50/0x5c)
> [   31.352752] [<c00cce58>] (nameidata_to_filp+0x50/0x5c) from [<c00db4c0>] (do_last+0x55c/0x6a0)
> [   31.361999] [<c00db4c0>] (do_last+0x55c/0x6a0) from [<c00db6bc>] (path_openat+0xb8/0x37c)
> [   31.370605] [<c00db6bc>] (path_openat+0xb8/0x37c) from [<c00dba60>] (do_filp_open+0x30/0x7c)
> [   31.379486] [<c00dba60>] (do_filp_open+0x30/0x7c) from [<c00cc904>] (do_sys_open+0xd8/0x170)
> [   31.388366] [<c00cc904>] (do_sys_open+0xd8/0x170) from [<c0012760>] (ret_fast_syscall+0x0/0x3c)
> [   31.397552] ---[ end trace 13639ab74f345d7e ]---
> 
> Tested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 

Thanks Laurent for testing this patch.


> Please push it to v3.3 :-)
> 

Will send a pull request today itself.

Thanks,
Vaibhav

> -- 
> Regards,
> 
> Laurent Pinchart
> 
> 

RE: [PATCH] omap_vout: Set DSS overlay_info only if paddr is non zero

[Hiremath, Vaibhav]

On Wed, Mar 07, 2012 at 14:31:16, Taneja, Archit wrote:
> The omap_vout driver tries to set the DSS overlay_info using set_overlay_info()
> when the physical address for the overlay is still not configured. This happens
> in omap_vout_probe() and vidioc_s_fmt_vid_out().
> 
> The calls to omapvid_init(which internally calls set_overlay_info()) are removed
> from these functions. They don't need to be called as the omap_vout_device
> struct anyway maintains the overlay related changes made. Also, remove the
> explicit call to set_overlay_info() in vidioc_streamon(), this was used to set
> the paddr, this isn't needed as omapvid_init() does the same thing later.
> 
> These changes are required as the DSS2 driver since 3.3 kernel doesn't let you
> set the overlay info with paddr as 0.
> 
> Signed-off-by: Archit Taneja <archit@ti.com>
> ---
>  drivers/media/video/omap/omap_vout.c |   36 ++++-----------------------------
>  1 files changed, 5 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/media/video/omap/omap_vout.c b/drivers/media/video/omap/omap_vout.c
> index 1fb7d5b..dffcf66 100644
> --- a/drivers/media/video/omap/omap_vout.c
> +++ b/drivers/media/video/omap/omap_vout.c
> @@ -1157,13 +1157,6 @@ static int vidioc_s_fmt_vid_out(struct file *file, void *fh,
>  	/* set default crop and win */
>  	omap_vout_new_format(&vout->pix, &vout->fbuf, &vout->crop, &vout->win);
>  
> -	/* Save the changes in the overlay strcuture */
> -	ret = omapvid_init(vout, 0);
> -	if (ret) {
> -		v4l2_err(&vout->vid_dev->v4l2_dev, "failed to change mode\n");
> -		goto s_fmt_vid_out_exit;
> -	}
> -
>  	ret = 0;
>  
>  s_fmt_vid_out_exit:
> @@ -1664,20 +1657,6 @@ static int vidioc_streamon(struct file *file, void *fh, enum v4l2_buf_type i)
>  
>  	omap_dispc_register_isr(omap_vout_isr, vout, mask);
>  
> -	for (j = 0; j < ovid->num_overlays; j++) {
> -		struct omap_overlay *ovl = ovid->overlays[j];
> -
> -		if (ovl->manager && ovl->manager->device) {
> -			struct omap_overlay_info info;
> -			ovl->get_overlay_info(ovl, &info);
> -			info.paddr = addr;
> -			if (ovl->set_overlay_info(ovl, &info)) {
> -				ret = -EINVAL;
> -				goto streamon_err1;
> -			}
> -		}
> -	}
> -

Have you checked for build warnings? I am getting build warnings 

  CC      drivers/media/video/omap/omap_vout.o
  CC      drivers/media/video/omap/omap_voutlib.o
  CC      drivers/media/video/omap/omap_vout_vrfb.o
drivers/media/video/omap/omap_vout.c: In function 'vidioc_streamon':
drivers/media/video/omap/omap_vout.c:1619:25: warning: unused variable 'ovid'
drivers/media/video/omap/omap_vout.c:1615:15: warning: unused variable 'j'
  LD      drivers/media/video/omap/omap-vout.o
  LD      drivers/media/video/omap/built-in.o

Can you fix this and submit the next version?

Thanks,
Vaibhav

>  	/* First save the configuration in ovelray structure */
>  	ret = omapvid_init(vout, addr);
>  	if (ret)
> @@ -2071,11 +2050,12 @@ static int __init omap_vout_create_video_devices(struct platform_device *pdev)
>  		}
>  		video_set_drvdata(vfd, vout);
>  
> -		/* Configure the overlay structure */
> -		ret = omapvid_init(vid_dev->vouts[k], 0);
> -		if (!ret)
> -			goto success;
> +		dev_info(&pdev->dev, ": registered and initialized"
> +				" video device %d\n", vfd->minor);
> +		if (k == (pdev->num_resources - 1))
> +			return 0;
>  
> +		continue;
>  error2:
>  		if (vout->vid_info.rotation_type == VOUT_ROT_VRFB)
>  			omap_vout_release_vrfb(vout);
> @@ -2085,12 +2065,6 @@ error1:
>  error:
>  		kfree(vout);
>  		return ret;
> -
> -success:
> -		dev_info(&pdev->dev, ": registered and initialized"
> -				" video device %d\n", vfd->minor);
> -		if (k == (pdev->num_resources - 1))
> -			return 0;
>  	}
>  
>  	return -ENODEV;
> -- 
> 1.7.5.4
> 
> 

Re: [PATCH] omap_vout: Set DSS overlay_info only if paddr is non zero

[Archit Taneja]

On Monday 12 March 2012 03:34 PM, Hiremath, Vaibhav wrote:
> On Wed, Mar 07, 2012 at 14:31:16, Taneja, Archit wrote:
>> The omap_vout driver tries to set the DSS overlay_info using set_overlay_info()
>> when the physical address for the overlay is still not configured. This happens
>> in omap_vout_probe() and vidioc_s_fmt_vid_out().
>>
>> The calls to omapvid_init(which internally calls set_overlay_info()) are removed
>> from these functions. They don't need to be called as the omap_vout_device
>> struct anyway maintains the overlay related changes made. Also, remove the
>> explicit call to set_overlay_info() in vidioc_streamon(), this was used to set
>> the paddr, this isn't needed as omapvid_init() does the same thing later.
>>
>> These changes are required as the DSS2 driver since 3.3 kernel doesn't let you
>> set the overlay info with paddr as 0.
>>
>> Signed-off-by: Archit Taneja<archit@ti.com>
>> ---
>>   drivers/media/video/omap/omap_vout.c |   36 ++++-----------------------------
>>   1 files changed, 5 insertions(+), 31 deletions(-)
>>
>> diff --git a/drivers/media/video/omap/omap_vout.c b/drivers/media/video/omap/omap_vout.c
>> index 1fb7d5b..dffcf66 100644
>> --- a/drivers/media/video/omap/omap_vout.c
>> +++ b/drivers/media/video/omap/omap_vout.c
>> @@ -1157,13 +1157,6 @@ static int vidioc_s_fmt_vid_out(struct file *file, void *fh,
>>   	/* set default crop and win */
>>   	omap_vout_new_format(&vout->pix,&vout->fbuf,&vout->crop,&vout->win);
>>
>> -	/* Save the changes in the overlay strcuture */
>> -	ret = omapvid_init(vout, 0);
>> -	if (ret) {
>> -		v4l2_err(&vout->vid_dev->v4l2_dev, "failed to change mode\n");
>> -		goto s_fmt_vid_out_exit;
>> -	}
>> -
>>   	ret = 0;
>>
>>   s_fmt_vid_out_exit:
>> @@ -1664,20 +1657,6 @@ static int vidioc_streamon(struct file *file, void *fh, enum v4l2_buf_type i)
>>
>>   	omap_dispc_register_isr(omap_vout_isr, vout, mask);
>>
>> -	for (j = 0; j<  ovid->num_overlays; j++) {
>> -		struct omap_overlay *ovl = ovid->overlays[j];
>> -
>> -		if (ovl->manager&&  ovl->manager->device) {
>> -			struct omap_overlay_info info;
>> -			ovl->get_overlay_info(ovl,&info);
>> -			info.paddr = addr;
>> -			if (ovl->set_overlay_info(ovl,&info)) {
>> -				ret = -EINVAL;
>> -				goto streamon_err1;
>> -			}
>> -		}
>> -	}
>> -
>
> Have you checked for build warnings? I am getting build warnings
>
>    CC      drivers/media/video/omap/omap_vout.o
>    CC      drivers/media/video/omap/omap_voutlib.o
>    CC      drivers/media/video/omap/omap_vout_vrfb.o
> drivers/media/video/omap/omap_vout.c: In function 'vidioc_streamon':
> drivers/media/video/omap/omap_vout.c:1619:25: warning: unused variable 'ovid'
> drivers/media/video/omap/omap_vout.c:1615:15: warning: unused variable 'j'
>    LD      drivers/media/video/omap/omap-vout.o
>    LD      drivers/media/video/omap/built-in.o
>
> Can you fix this and submit the next version?

Will fix this and submit.

Archit

>
> Thanks,
> Vaibhav
>
>>   	/* First save the configuration in ovelray structure */
>>   	ret = omapvid_init(vout, addr);
>>   	if (ret)
>> @@ -2071,11 +2050,12 @@ static int __init omap_vout_create_video_devices(struct platform_device *pdev)
>>   		}
>>   		video_set_drvdata(vfd, vout);
>>
>> -		/* Configure the overlay structure */
>> -		ret = omapvid_init(vid_dev->vouts[k], 0);
>> -		if (!ret)
>> -			goto success;
>> +		dev_info(&pdev->dev, ": registered and initialized"
>> +				" video device %d\n", vfd->minor);
>> +		if (k == (pdev->num_resources - 1))
>> +			return 0;
>>
>> +		continue;
>>   error2:
>>   		if (vout->vid_info.rotation_type == VOUT_ROT_VRFB)
>>   			omap_vout_release_vrfb(vout);
>> @@ -2085,12 +2065,6 @@ error1:
>>   error:
>>   		kfree(vout);
>>   		return ret;
>> -
>> -success:
>> -		dev_info(&pdev->dev, ": registered and initialized"
>> -				" video device %d\n", vfd->minor);
>> -		if (k == (pdev->num_resources - 1))
>> -			return 0;
>>   	}
>>
>>   	return -ENODEV;
>> --
>> 1.7.5.4
>>
>>
>
>

Re: [PATCH] omap_vout: Set DSS overlay_info only if paddr is non zero

[Archit Taneja]

Hi,

On Friday 16 March 2012 03:46 PM, Archit Taneja wrote:
> On Monday 12 March 2012 03:34 PM, Hiremath, Vaibhav wrote:
>> On Wed, Mar 07, 2012 at 14:31:16, Taneja, Archit wrote:
>>> The omap_vout driver tries to set the DSS overlay_info using
>>> set_overlay_info()
>>> when the physical address for the overlay is still not configured.
>>> This happens
>>> in omap_vout_probe() and vidioc_s_fmt_vid_out().
>>>
>>> The calls to omapvid_init(which internally calls set_overlay_info())
>>> are removed
>>> from these functions. They don't need to be called as the
>>> omap_vout_device
>>> struct anyway maintains the overlay related changes made. Also,
>>> remove the
>>> explicit call to set_overlay_info() in vidioc_streamon(), this was
>>> used to set
>>> the paddr, this isn't needed as omapvid_init() does the same thing
>>> later.
>>>
>>> These changes are required as the DSS2 driver since 3.3 kernel
>>> doesn't let you
>>> set the overlay info with paddr as 0.
>>>
>>> Signed-off-by: Archit Taneja<archit@ti.com>
>>> ---
>>> drivers/media/video/omap/omap_vout.c | 36
>>> ++++-----------------------------
>>> 1 files changed, 5 insertions(+), 31 deletions(-)
>>>
>>> diff --git a/drivers/media/video/omap/omap_vout.c
>>> b/drivers/media/video/omap/omap_vout.c
>>> index 1fb7d5b..dffcf66 100644
>>> --- a/drivers/media/video/omap/omap_vout.c
>>> +++ b/drivers/media/video/omap/omap_vout.c
>>> @@ -1157,13 +1157,6 @@ static int vidioc_s_fmt_vid_out(struct file
>>> *file, void *fh,
>>> /* set default crop and win */
>>> omap_vout_new_format(&vout->pix,&vout->fbuf,&vout->crop,&vout->win);
>>>
>>> - /* Save the changes in the overlay strcuture */
>>> - ret = omapvid_init(vout, 0);
>>> - if (ret) {
>>> - v4l2_err(&vout->vid_dev->v4l2_dev, "failed to change mode\n");
>>> - goto s_fmt_vid_out_exit;
>>> - }
>>> -
>>> ret = 0;
>>>
>>> s_fmt_vid_out_exit:
>>> @@ -1664,20 +1657,6 @@ static int vidioc_streamon(struct file *file,
>>> void *fh, enum v4l2_buf_type i)
>>>
>>> omap_dispc_register_isr(omap_vout_isr, vout, mask);
>>>
>>> - for (j = 0; j< ovid->num_overlays; j++) {
>>> - struct omap_overlay *ovl = ovid->overlays[j];
>>> -
>>> - if (ovl->manager&& ovl->manager->device) {
>>> - struct omap_overlay_info info;
>>> - ovl->get_overlay_info(ovl,&info);
>>> - info.paddr = addr;
>>> - if (ovl->set_overlay_info(ovl,&info)) {
>>> - ret = -EINVAL;
>>> - goto streamon_err1;
>>> - }
>>> - }
>>> - }
>>> -
>>
>> Have you checked for build warnings? I am getting build warnings
>>
>> CC drivers/media/video/omap/omap_vout.o
>> CC drivers/media/video/omap/omap_voutlib.o
>> CC drivers/media/video/omap/omap_vout_vrfb.o
>> drivers/media/video/omap/omap_vout.c: In function 'vidioc_streamon':
>> drivers/media/video/omap/omap_vout.c:1619:25: warning: unused variable
>> 'ovid'
>> drivers/media/video/omap/omap_vout.c:1615:15: warning: unused variable
>> 'j'
>> LD drivers/media/video/omap/omap-vout.o
>> LD drivers/media/video/omap/built-in.o
>>
>> Can you fix this and submit the next version?

I applied the patch on the current mainline kernel, it doesn't give any 
build warnings. Even after applying the patch, 'j and ovid' are still 
used in vidioc_streamon().

Can you check if it was applied correctly?

Regards,
Archit

>
> Will fix this and submit.
>
> Archit
>
>>
>> Thanks,
>> Vaibhav
>>
>>> /* First save the configuration in ovelray structure */
>>> ret = omapvid_init(vout, addr);
>>> if (ret)
>>> @@ -2071,11 +2050,12 @@ static int __init
>>> omap_vout_create_video_devices(struct platform_device *pdev)
>>> }
>>> video_set_drvdata(vfd, vout);
>>>
>>> - /* Configure the overlay structure */
>>> - ret = omapvid_init(vid_dev->vouts[k], 0);
>>> - if (!ret)
>>> - goto success;
>>> + dev_info(&pdev->dev, ": registered and initialized"
>>> + " video device %d\n", vfd->minor);
>>> + if (k == (pdev->num_resources - 1))
>>> + return 0;
>>>
>>> + continue;
>>> error2:
>>> if (vout->vid_info.rotation_type == VOUT_ROT_VRFB)
>>> omap_vout_release_vrfb(vout);
>>> @@ -2085,12 +2065,6 @@ error1:
>>> error:
>>> kfree(vout);
>>> return ret;
>>> -
>>> -success:
>>> - dev_info(&pdev->dev, ": registered and initialized"
>>> - " video device %d\n", vfd->minor);
>>> - if (k == (pdev->num_resources - 1))
>>> - return 0;
>>> }
>>>
>>> return -ENODEV;
>>> --
>>> 1.7.5.4
>>>
>>>
>>
>>
>
>

Re: [PATCH] omap_vout: Set DSS overlay_info only if paddr is non zero

[Archit Taneja]

Hi,

On Friday 16 March 2012 03:46 PM, Archit Taneja wrote:
> On Monday 12 March 2012 03:34 PM, Hiremath, Vaibhav wrote:
>> On Wed, Mar 07, 2012 at 14:31:16, Taneja, Archit wrote:
>>> The omap_vout driver tries to set the DSS overlay_info using
>>> set_overlay_info()
>>> when the physical address for the overlay is still not configured.
>>> This happens
>>> in omap_vout_probe() and vidioc_s_fmt_vid_out().
>>>
>>> The calls to omapvid_init(which internally calls set_overlay_info())
>>> are removed
>>> from these functions. They don't need to be called as the
>>> omap_vout_device
>>> struct anyway maintains the overlay related changes made. Also,
>>> remove the
>>> explicit call to set_overlay_info() in vidioc_streamon(), this was
>>> used to set
>>> the paddr, this isn't needed as omapvid_init() does the same thing
>>> later.
>>>
>>> These changes are required as the DSS2 driver since 3.3 kernel
>>> doesn't let you
>>> set the overlay info with paddr as 0.
>>>
>>> Signed-off-by: Archit Taneja<archit@ti.com>
>>> ---
>>> drivers/media/video/omap/omap_vout.c | 36
>>> ++++-----------------------------
>>> 1 files changed, 5 insertions(+), 31 deletions(-)
>>>
>>> diff --git a/drivers/media/video/omap/omap_vout.c
>>> b/drivers/media/video/omap/omap_vout.c
>>> index 1fb7d5b..dffcf66 100644
>>> --- a/drivers/media/video/omap/omap_vout.c
>>> +++ b/drivers/media/video/omap/omap_vout.c
>>> @@ -1157,13 +1157,6 @@ static int vidioc_s_fmt_vid_out(struct file
>>> *file, void *fh,
>>> /* set default crop and win */
>>> omap_vout_new_format(&vout->pix,&vout->fbuf,&vout->crop,&vout->win);
>>>
>>> - /* Save the changes in the overlay strcuture */
>>> - ret = omapvid_init(vout, 0);
>>> - if (ret) {
>>> - v4l2_err(&vout->vid_dev->v4l2_dev, "failed to change mode\n");
>>> - goto s_fmt_vid_out_exit;
>>> - }
>>> -
>>> ret = 0;
>>>
>>> s_fmt_vid_out_exit:
>>> @@ -1664,20 +1657,6 @@ static int vidioc_streamon(struct file *file,
>>> void *fh, enum v4l2_buf_type i)
>>>
>>> omap_dispc_register_isr(omap_vout_isr, vout, mask);
>>>
>>> - for (j = 0; j< ovid->num_overlays; j++) {
>>> - struct omap_overlay *ovl = ovid->overlays[j];
>>> -
>>> - if (ovl->manager&& ovl->manager->device) {
>>> - struct omap_overlay_info info;
>>> - ovl->get_overlay_info(ovl,&info);
>>> - info.paddr = addr;
>>> - if (ovl->set_overlay_info(ovl,&info)) {
>>> - ret = -EINVAL;
>>> - goto streamon_err1;
>>> - }
>>> - }
>>> - }
>>> -
>>
>> Have you checked for build warnings? I am getting build warnings
>>
>> CC drivers/media/video/omap/omap_vout.o
>> CC drivers/media/video/omap/omap_voutlib.o
>> CC drivers/media/video/omap/omap_vout_vrfb.o
>> drivers/media/video/omap/omap_vout.c: In function 'vidioc_streamon':
>> drivers/media/video/omap/omap_vout.c:1619:25: warning: unused variable
>> 'ovid'
>> drivers/media/video/omap/omap_vout.c:1615:15: warning: unused variable
>> 'j'
>> LD drivers/media/video/omap/omap-vout.o
>> LD drivers/media/video/omap/built-in.o
>>
>> Can you fix this and submit the next version?

I applied the patch on the current mainline kernel, it doesn't give any 
build warnings. Even after applying the patch, 'j and ovid' are still 
used in vidioc_streamon().

Can you check if it was applied correctly?

Regards,
Archit

>
> Will fix this and submit.
>
> Archit
>
>>
>> Thanks,
>> Vaibhav
>>
>>> /* First save the configuration in ovelray structure */
>>> ret = omapvid_init(vout, addr);
>>> if (ret)
>>> @@ -2071,11 +2050,12 @@ static int __init
>>> omap_vout_create_video_devices(struct platform_device *pdev)
>>> }
>>> video_set_drvdata(vfd, vout);
>>>
>>> - /* Configure the overlay structure */
>>> - ret = omapvid_init(vid_dev->vouts[k], 0);
>>> - if (!ret)
>>> - goto success;
>>> + dev_info(&pdev->dev, ": registered and initialized"
>>> + " video device %d\n", vfd->minor);
>>> + if (k == (pdev->num_resources - 1))
>>> + return 0;
>>>
>>> + continue;
>>> error2:
>>> if (vout->vid_info.rotation_type == VOUT_ROT_VRFB)
>>> omap_vout_release_vrfb(vout);
>>> @@ -2085,12 +2065,6 @@ error1:
>>> error:
>>> kfree(vout);
>>> return ret;
>>> -
>>> -success:
>>> - dev_info(&pdev->dev, ": registered and initialized"
>>> - " video device %d\n", vfd->minor);
>>> - if (k == (pdev->num_resources - 1))
>>> - return 0;
>>> }
>>>
>>> return -ENODEV;
>>> --
>>> 1.7.5.4
>>>
>>>
>>
>>
>
>

RE: [PATCH] omap_vout: Set DSS overlay_info only if paddr is non zero

[Hiremath, Vaibhav]

On Fri, Mar 16, 2012 at 16:41:27, Taneja, Archit wrote:
> Hi,
> 
> On Friday 16 March 2012 03:46 PM, Archit Taneja wrote:
> > On Monday 12 March 2012 03:34 PM, Hiremath, Vaibhav wrote:
> >> On Wed, Mar 07, 2012 at 14:31:16, Taneja, Archit wrote:
> >>> The omap_vout driver tries to set the DSS overlay_info using
> >>> set_overlay_info()
> >>> when the physical address for the overlay is still not configured.
> >>> This happens
> >>> in omap_vout_probe() and vidioc_s_fmt_vid_out().
> >>>
> >>> The calls to omapvid_init(which internally calls set_overlay_info())
> >>> are removed
> >>> from these functions. They don't need to be called as the
> >>> omap_vout_device
> >>> struct anyway maintains the overlay related changes made. Also,
> >>> remove the
> >>> explicit call to set_overlay_info() in vidioc_streamon(), this was
> >>> used to set
> >>> the paddr, this isn't needed as omapvid_init() does the same thing
> >>> later.
> >>>
> >>> These changes are required as the DSS2 driver since 3.3 kernel
> >>> doesn't let you
> >>> set the overlay info with paddr as 0.
> >>>
> >>> Signed-off-by: Archit Taneja<archit@ti.com>
> >>> ---
> >>> drivers/media/video/omap/omap_vout.c | 36
> >>> ++++-----------------------------
> >>> 1 files changed, 5 insertions(+), 31 deletions(-)
> >>>
> >>> diff --git a/drivers/media/video/omap/omap_vout.c
> >>> b/drivers/media/video/omap/omap_vout.c
> >>> index 1fb7d5b..dffcf66 100644
> >>> --- a/drivers/media/video/omap/omap_vout.c
> >>> +++ b/drivers/media/video/omap/omap_vout.c
> >>> @@ -1157,13 +1157,6 @@ static int vidioc_s_fmt_vid_out(struct file
> >>> *file, void *fh,
> >>> /* set default crop and win */
> >>> omap_vout_new_format(&vout->pix,&vout->fbuf,&vout->crop,&vout->win);
> >>>
> >>> - /* Save the changes in the overlay strcuture */
> >>> - ret = omapvid_init(vout, 0);
> >>> - if (ret) {
> >>> - v4l2_err(&vout->vid_dev->v4l2_dev, "failed to change mode\n");
> >>> - goto s_fmt_vid_out_exit;
> >>> - }
> >>> -
> >>> ret = 0;
> >>>
> >>> s_fmt_vid_out_exit:
> >>> @@ -1664,20 +1657,6 @@ static int vidioc_streamon(struct file *file,
> >>> void *fh, enum v4l2_buf_type i)
> >>>
> >>> omap_dispc_register_isr(omap_vout_isr, vout, mask);
> >>>
> >>> - for (j = 0; j< ovid->num_overlays; j++) {
> >>> - struct omap_overlay *ovl = ovid->overlays[j];
> >>> -
> >>> - if (ovl->manager&& ovl->manager->device) {
> >>> - struct omap_overlay_info info;
> >>> - ovl->get_overlay_info(ovl,&info);
> >>> - info.paddr = addr;
> >>> - if (ovl->set_overlay_info(ovl,&info)) {
> >>> - ret = -EINVAL;
> >>> - goto streamon_err1;
> >>> - }
> >>> - }
> >>> - }
> >>> -
> >>
> >> Have you checked for build warnings? I am getting build warnings
> >>
> >> CC drivers/media/video/omap/omap_vout.o
> >> CC drivers/media/video/omap/omap_voutlib.o
> >> CC drivers/media/video/omap/omap_vout_vrfb.o
> >> drivers/media/video/omap/omap_vout.c: In function 'vidioc_streamon':
> >> drivers/media/video/omap/omap_vout.c:1619:25: warning: unused variable
> >> 'ovid'
> >> drivers/media/video/omap/omap_vout.c:1615:15: warning: unused variable
> >> 'j'
> >> LD drivers/media/video/omap/omap-vout.o
> >> LD drivers/media/video/omap/built-in.o
> >>
> >> Can you fix this and submit the next version?
> 
> I applied the patch on the current mainline kernel, it doesn't give any 
> build warnings. Even after applying the patch, 'j and ovid' are still 
> used in vidioc_streamon().
> 
> Can you check if it was applied correctly?
> 

Archit,

I could able to trace what's going on here,

I am using "v4l_for_linus" branch, which has one missing patch,

commit aaa874a985158383c4b394c687c716ef26288741
Author: Tomi Valkeinen <tomi.valkeinen@ti.com>
Date:   Tue Nov 15 16:37:53 2011 +0200

    OMAPDSS: APPLY: rewrite overlay enable/disable


So, I do not have below changes,

@@ -1686,6 +1681,16 @@ static int vidioc_streamon(struct file *file, void *fh, enum v4l2_buf_type i)
        if (ret)
                v4l2_err(&vout->vid_dev->v4l2_dev, "failed to change mode\n");

+       for (j = 0; j < ovid->num_overlays; j++) {
+               struct omap_overlay *ovl = ovid->overlays[j];
+
+               if (ovl->manager && ovl->manager->device) {
+                       ret = ovl->enable(ovl);
+                       if (ret)
+                               goto streamon_err1;
+               }
+       }
+

This explains why I am seeing these warnings. Let me give pull request based on master branch.


Thanks,
Vaibhav

> Regards,
> Archit
> 
> >
> > Will fix this and submit.
> >
> > Archit
> >
> >>
> >> Thanks,
> >> Vaibhav
> >>
> >>> /* First save the configuration in ovelray structure */
> >>> ret = omapvid_init(vout, addr);
> >>> if (ret)
> >>> @@ -2071,11 +2050,12 @@ static int __init
> >>> omap_vout_create_video_devices(struct platform_device *pdev)
> >>> }
> >>> video_set_drvdata(vfd, vout);
> >>>
> >>> - /* Configure the overlay structure */
> >>> - ret = omapvid_init(vid_dev->vouts[k], 0);
> >>> - if (!ret)
> >>> - goto success;
> >>> + dev_info(&pdev->dev, ": registered and initialized"
> >>> + " video device %d\n", vfd->minor);
> >>> + if (k == (pdev->num_resources - 1))
> >>> + return 0;
> >>>
> >>> + continue;
> >>> error2:
> >>> if (vout->vid_info.rotation_type == VOUT_ROT_VRFB)
> >>> omap_vout_release_vrfb(vout);
> >>> @@ -2085,12 +2065,6 @@ error1:
> >>> error:
> >>> kfree(vout);
> >>> return ret;
> >>> -
> >>> -success:
> >>> - dev_info(&pdev->dev, ": registered and initialized"
> >>> - " video device %d\n", vfd->minor);
> >>> - if (k == (pdev->num_resources - 1))
> >>> - return 0;
> >>> }
> >>>
> >>> return -ENODEV;
> >>> --
> >>> 1.7.5.4
> >>>
> >>>
> >>
> >>
> >
> >
> 
> 

Re: [PATCH] omap_vout: Set DSS overlay_info only if paddr is non zero

[Archit Taneja]

On Monday 19 March 2012 02:15 PM, Hiremath, Vaibhav wrote:
> On Fri, Mar 16, 2012 at 16:41:27, Taneja, Archit wrote:
>> Hi,
>>
>> On Friday 16 March 2012 03:46 PM, Archit Taneja wrote:
>>> On Monday 12 March 2012 03:34 PM, Hiremath, Vaibhav wrote:
>>>> On Wed, Mar 07, 2012 at 14:31:16, Taneja, Archit wrote:
>>>>> The omap_vout driver tries to set the DSS overlay_info using
>>>>> set_overlay_info()
>>>>> when the physical address for the overlay is still not configured.
>>>>> This happens
>>>>> in omap_vout_probe() and vidioc_s_fmt_vid_out().
>>>>>
>>>>> The calls to omapvid_init(which internally calls set_overlay_info())
>>>>> are removed
>>>>> from these functions. They don't need to be called as the
>>>>> omap_vout_device
>>>>> struct anyway maintains the overlay related changes made. Also,
>>>>> remove the
>>>>> explicit call to set_overlay_info() in vidioc_streamon(), this was
>>>>> used to set
>>>>> the paddr, this isn't needed as omapvid_init() does the same thing
>>>>> later.
>>>>>
>>>>> These changes are required as the DSS2 driver since 3.3 kernel
>>>>> doesn't let you
>>>>> set the overlay info with paddr as 0.
>>>>>
>>>>> Signed-off-by: Archit Taneja<archit@ti.com>
>>>>> ---
>>>>> drivers/media/video/omap/omap_vout.c | 36
>>>>> ++++-----------------------------
>>>>> 1 files changed, 5 insertions(+), 31 deletions(-)
>>>>>
>>>>> diff --git a/drivers/media/video/omap/omap_vout.c
>>>>> b/drivers/media/video/omap/omap_vout.c
>>>>> index 1fb7d5b..dffcf66 100644
>>>>> --- a/drivers/media/video/omap/omap_vout.c
>>>>> +++ b/drivers/media/video/omap/omap_vout.c
>>>>> @@ -1157,13 +1157,6 @@ static int vidioc_s_fmt_vid_out(struct file
>>>>> *file, void *fh,
>>>>> /* set default crop and win */
>>>>> omap_vout_new_format(&vout->pix,&vout->fbuf,&vout->crop,&vout->win);
>>>>>
>>>>> - /* Save the changes in the overlay strcuture */
>>>>> - ret = omapvid_init(vout, 0);
>>>>> - if (ret) {
>>>>> - v4l2_err(&vout->vid_dev->v4l2_dev, "failed to change mode\n");
>>>>> - goto s_fmt_vid_out_exit;
>>>>> - }
>>>>> -
>>>>> ret = 0;
>>>>>
>>>>> s_fmt_vid_out_exit:
>>>>> @@ -1664,20 +1657,6 @@ static int vidioc_streamon(struct file *file,
>>>>> void *fh, enum v4l2_buf_type i)
>>>>>
>>>>> omap_dispc_register_isr(omap_vout_isr, vout, mask);
>>>>>
>>>>> - for (j = 0; j<  ovid->num_overlays; j++) {
>>>>> - struct omap_overlay *ovl = ovid->overlays[j];
>>>>> -
>>>>> - if (ovl->manager&&  ovl->manager->device) {
>>>>> - struct omap_overlay_info info;
>>>>> - ovl->get_overlay_info(ovl,&info);
>>>>> - info.paddr = addr;
>>>>> - if (ovl->set_overlay_info(ovl,&info)) {
>>>>> - ret = -EINVAL;
>>>>> - goto streamon_err1;
>>>>> - }
>>>>> - }
>>>>> - }
>>>>> -
>>>>
>>>> Have you checked for build warnings? I am getting build warnings
>>>>
>>>> CC drivers/media/video/omap/omap_vout.o
>>>> CC drivers/media/video/omap/omap_voutlib.o
>>>> CC drivers/media/video/omap/omap_vout_vrfb.o
>>>> drivers/media/video/omap/omap_vout.c: In function 'vidioc_streamon':
>>>> drivers/media/video/omap/omap_vout.c:1619:25: warning: unused variable
>>>> 'ovid'
>>>> drivers/media/video/omap/omap_vout.c:1615:15: warning: unused variable
>>>> 'j'
>>>> LD drivers/media/video/omap/omap-vout.o
>>>> LD drivers/media/video/omap/built-in.o
>>>>
>>>> Can you fix this and submit the next version?
>>
>> I applied the patch on the current mainline kernel, it doesn't give any
>> build warnings. Even after applying the patch, 'j and ovid' are still
>> used in vidioc_streamon().
>>
>> Can you check if it was applied correctly?
>>
>
> Archit,
>
> I could able to trace what's going on here,
>
> I am using "v4l_for_linus" branch, which has one missing patch,
>
> commit aaa874a985158383c4b394c687c716ef26288741
> Author: Tomi Valkeinen<tomi.valkeinen@ti.com>
> Date:   Tue Nov 15 16:37:53 2011 +0200
>
>      OMAPDSS: APPLY: rewrite overlay enable/disable
>
>
> So, I do not have below changes,
>
> @@ -1686,6 +1681,16 @@ static int vidioc_streamon(struct file *file, void *fh, enum v4l2_buf_type i)
>          if (ret)
>                  v4l2_err(&vout->vid_dev->v4l2_dev, "failed to change mode\n");
>
> +       for (j = 0; j<  ovid->num_overlays; j++) {
> +               struct omap_overlay *ovl = ovid->overlays[j];
> +
> +               if (ovl->manager&&  ovl->manager->device) {
> +                       ret = ovl->enable(ovl);
> +                       if (ret)
> +                               goto streamon_err1;
> +               }
> +       }
> +
>
> This explains why I am seeing these warnings. Let me give pull request based on master branch.

Okay. Thanks for looking into this.

Archit

>
>
> Thanks,
> Vaibhav
>
>> Regards,
>> Archit
>>
>>>
>>> Will fix this and submit.
>>>
>>> Archit
>>>
>>>>
>>>> Thanks,
>>>> Vaibhav
>>>>
>>>>> /* First save the configuration in ovelray structure */
>>>>> ret = omapvid_init(vout, addr);
>>>>> if (ret)
>>>>> @@ -2071,11 +2050,12 @@ static int __init
>>>>> omap_vout_create_video_devices(struct platform_device *pdev)
>>>>> }
>>>>> video_set_drvdata(vfd, vout);
>>>>>
>>>>> - /* Configure the overlay structure */
>>>>> - ret = omapvid_init(vid_dev->vouts[k], 0);
>>>>> - if (!ret)
>>>>> - goto success;
>>>>> + dev_info(&pdev->dev, ": registered and initialized"
>>>>> + " video device %d\n", vfd->minor);
>>>>> + if (k == (pdev->num_resources - 1))
>>>>> + return 0;
>>>>>
>>>>> + continue;
>>>>> error2:
>>>>> if (vout->vid_info.rotation_type == VOUT_ROT_VRFB)
>>>>> omap_vout_release_vrfb(vout);
>>>>> @@ -2085,12 +2065,6 @@ error1:
>>>>> error:
>>>>> kfree(vout);
>>>>> return ret;
>>>>> -
>>>>> -success:
>>>>> - dev_info(&pdev->dev, ": registered and initialized"
>>>>> - " video device %d\n", vfd->minor);
>>>>> - if (k == (pdev->num_resources - 1))
>>>>> - return 0;
>>>>> }
>>>>>
>>>>> return -ENODEV;
>>>>> --
>>>>> 1.7.5.4
>>>>>
>>>>>
>>>>
>>>>
>>>
>>>
>>
>>
>

Re: [PATCH] omap_vout: Set DSS overlay_info only if paddr is non zero

[Semwal, Sumit]

On Mon, Mar 19, 2012 at 5:16 PM, Archit Taneja <a0393947@ti.com> wrote:
> On Monday 19 March 2012 02:15 PM, Hiremath, Vaibhav wrote:
>>
>> On Fri, Mar 16, 2012 at 16:41:27, Taneja, Archit wrote:
>>>
>>> Hi,
>>>
>>> On Friday 16 March 2012 03:46 PM, Archit Taneja wrote:
>>>>
>>>> On Monday 12 March 2012 03:34 PM, Hiremath, Vaibhav wrote:
>>>>>
>>>>> On Wed, Mar 07, 2012 at 14:31:16, Taneja, Archit wrote:
>>>>>>
>>>>>> The omap_vout driver tries to set the DSS overlay_info using
>>>>>> set_overlay_info()
>>>>>> when the physical address for the overlay is still not configured.
>>>>>> This happens
>>>>>> in omap_vout_probe() and vidioc_s_fmt_vid_out().
>>>>>>
>>>>>> The calls to omapvid_init(which internally calls set_overlay_info())
>>>>>> are removed
>>>>>> from these functions. They don't need to be called as the
>>>>>> omap_vout_device
>>>>>> struct anyway maintains the overlay related changes made. Also,
>>>>>> remove the
>>>>>> explicit call to set_overlay_info() in vidioc_streamon(), this was
>>>>>> used to set
>>>>>> the paddr, this isn't needed as omapvid_init() does the same thing
>>>>>> later.
>>>>>>
>>>>>> These changes are required as the DSS2 driver since 3.3 kernel
>>>>>> doesn't let you
>>>>>> set the overlay info with paddr as 0.
>>>>>>
>>>>>> Signed-off-by: Archit Taneja<archit@ti.com>
>>>>>> ---
>>>>>> drivers/media/video/omap/omap_vout.c | 36
>>>>>> ++++-----------------------------
>>>>>> 1 files changed, 5 insertions(+), 31 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/media/video/omap/omap_vout.c
>>>>>> b/drivers/media/video/omap/omap_vout.c
>>>>>> index 1fb7d5b..dffcf66 100644
>>>>>> --- a/drivers/media/video/omap/omap_vout.c
>>>>>> +++ b/drivers/media/video/omap/omap_vout.c
>>>>>> @@ -1157,13 +1157,6 @@ static int vidioc_s_fmt_vid_out(struct file
>>>>>> *file, void *fh,
>>>>>> /* set default crop and win */
>>>>>> omap_vout_new_format(&vout->pix,&vout->fbuf,&vout->crop,&vout->win);
>>>>>>
>>>>>> - /* Save the changes in the overlay strcuture */
>>>>>> - ret = omapvid_init(vout, 0);
>>>>>> - if (ret) {
>>>>>> - v4l2_err(&vout->vid_dev->v4l2_dev, "failed to change mode\n");
>>>>>> - goto s_fmt_vid_out_exit;
>>>>>> - }
>>>>>> -
>>>>>> ret = 0;
>>>>>>
>>>>>> s_fmt_vid_out_exit:
>>>>>> @@ -1664,20 +1657,6 @@ static int vidioc_streamon(struct file *file,
>>>>>> void *fh, enum v4l2_buf_type i)
>>>>>>
>>>>>> omap_dispc_register_isr(omap_vout_isr, vout, mask);
>>>>>>
>>>>>> - for (j = 0; j<  ovid->num_overlays; j++) {
>>>>>> - struct omap_overlay *ovl = ovid->overlays[j];
>>>>>> -
>>>>>> - if (ovl->manager&&  ovl->manager->device) {
>>>>>> - struct omap_overlay_info info;
>>>>>> - ovl->get_overlay_info(ovl,&info);
>>>>>> - info.paddr = addr;
>>>>>> - if (ovl->set_overlay_info(ovl,&info)) {
>>>>>> - ret = -EINVAL;
>>>>>> - goto streamon_err1;
>>>>>> - }
>>>>>> - }
>>>>>> - }
>>>>>> -
>>>>>
>>>>>
>>>>> Have you checked for build warnings? I am getting build warnings
>>>>>
>>>>> CC drivers/media/video/omap/omap_vout.o
>>>>> CC drivers/media/video/omap/omap_voutlib.o
>>>>> CC drivers/media/video/omap/omap_vout_vrfb.o
>>>>> drivers/media/video/omap/omap_vout.c: In function 'vidioc_streamon':
>>>>> drivers/media/video/omap/omap_vout.c:1619:25: warning: unused variable
>>>>> 'ovid'
>>>>> drivers/media/video/omap/omap_vout.c:1615:15: warning: unused variable
>>>>> 'j'
>>>>> LD drivers/media/video/omap/omap-vout.o
>>>>> LD drivers/media/video/omap/built-in.o
>>>>>
>>>>> Can you fix this and submit the next version?
>>>
>>>
>>> I applied the patch on the current mainline kernel, it doesn't give any
>>> build warnings. Even after applying the patch, 'j and ovid' are still
>>> used in vidioc_streamon().
>>>
>>> Can you check if it was applied correctly?
>>>
>>
>> Archit,
>>
>> I could able to trace what's going on here,
>>
>> I am using "v4l_for_linus" branch, which has one missing patch,
>>
>> commit aaa874a985158383c4b394c687c716ef26288741
>> Author: Tomi Valkeinen<tomi.valkeinen@ti.com>
>> Date:   Tue Nov 15 16:37:53 2011 +0200
>>
>>     OMAPDSS: APPLY: rewrite overlay enable/disable
>>
>>
>> So, I do not have below changes,
>>
>> @@ -1686,6 +1681,16 @@ static int vidioc_streamon(struct file *file, void
>> *fh, enum v4l2_buf_type i)
>>         if (ret)
>>                 v4l2_err(&vout->vid_dev->v4l2_dev, "failed to change
>> mode\n");
>>
>> +       for (j = 0; j<  ovid->num_overlays; j++) {
>> +               struct omap_overlay *ovl = ovid->overlays[j];
>> +
>> +               if (ovl->manager&&  ovl->manager->device) {
>>
>> +                       ret = ovl->enable(ovl);
>> +                       if (ret)
>> +                               goto streamon_err1;
>> +               }
>> +       }
>> +
>>
>> This explains why I am seeing these warnings. Let me give pull request
>> based on master branch.
>
>
> Okay. Thanks for looking into this.
>
> Archit

Hi Vaibhav,

This patch has been outstanding since March, and we have received
reports from various users. Could you please push it ASAP to Mauro for
the upcoming -rc?

Or Did I miss a pull request from you containing this?

Thanks and best regards,
~Sumit.

>
>>
>>
>> Thanks,
>> Vaibhav
>>
>>> Regards,
>>> Archit
>>>
>>>>
>>>> Will fix this and submit.
>>>>
>>>> Archit
>>>>
>>>>>
>>>>> Thanks,
>>>>> Vaibhav
>>>>>
>>>>>> /* First save the configuration in ovelray structure */
>>>>>> ret = omapvid_init(vout, addr);
>>>>>> if (ret)
>>>>>> @@ -2071,11 +2050,12 @@ static int __init
>>>>>> omap_vout_create_video_devices(struct platform_device *pdev)
>>>>>> }
>>>>>> video_set_drvdata(vfd, vout);
>>>>>>
>>>>>> - /* Configure the overlay structure */
>>>>>> - ret = omapvid_init(vid_dev->vouts[k], 0);
>>>>>> - if (!ret)
>>>>>> - goto success;
>>>>>> + dev_info(&pdev->dev, ": registered and initialized"
>>>>>> + " video device %d\n", vfd->minor);
>>>>>> + if (k == (pdev->num_resources - 1))
>>>>>> + return 0;
>>>>>>
>>>>>> + continue;
>>>>>> error2:
>>>>>> if (vout->vid_info.rotation_type == VOUT_ROT_VRFB)
>>>>>> omap_vout_release_vrfb(vout);
>>>>>> @@ -2085,12 +2065,6 @@ error1:
>>>>>> error:
>>>>>> kfree(vout);
>>>>>> return ret;
>>>>>> -
>>>>>> -success:
>>>>>> - dev_info(&pdev->dev, ": registered and initialized"
>>>>>> - " video device %d\n", vfd->minor);
>>>>>> - if (k == (pdev->num_resources - 1))
>>>>>> - return 0;
>>>>>> }
>>>>>>
>>>>>> return -ENODEV;
>>>>>> --
>>>>>> 1.7.5.4
>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>
>>>>
>>>
>>>
>>
>
> --
> 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

Re: [PATCH] omap_vout: Set DSS overlay_info only if paddr is non zero

[Laurent Pinchart]

On Thursday 28 June 2012 11:36:48 Semwal, Sumit wrote:
> On Mon, Mar 19, 2012 at 5:16 PM, Archit Taneja <a0393947@ti.com> wrote:
> > On Monday 19 March 2012 02:15 PM, Hiremath, Vaibhav wrote:
> >> On Fri, Mar 16, 2012 at 16:41:27, Taneja, Archit wrote:
> >>> On Friday 16 March 2012 03:46 PM, Archit Taneja wrote:
> >>>> On Monday 12 March 2012 03:34 PM, Hiremath, Vaibhav wrote:
> >>>>> On Wed, Mar 07, 2012 at 14:31:16, Taneja, Archit wrote:
> >>>>>> The omap_vout driver tries to set the DSS overlay_info using
> >>>>>> set_overlay_info() when the physical address for the overlay is still
> >>>>>> not configured. This happens in omap_vout_probe() and
> >>>>>> vidioc_s_fmt_vid_out().
> >>>>>> 
> >>>>>> The calls to omapvid_init(which internally calls set_overlay_info())
> >>>>>> are removed from these functions. They don't need to be called as the
> >>>>>> omap_vout_device struct anyway maintains the overlay related changes
> >>>>>> made. Also, remove the explicit call to set_overlay_info() in
> >>>>>> vidioc_streamon(), this was used to set the paddr, this isn't needed
> >>>>>> as omapvid_init() does the same thing later.
> >>>>>> 
> >>>>>> These changes are required as the DSS2 driver since 3.3 kernel
> >>>>>> doesn't let you set the overlay info with paddr as 0.
> >>>>>> 
> >>>>>> Signed-off-by: Archit Taneja<archit@ti.com>
> >>>>>> ---
> >>>>>> drivers/media/video/omap/omap_vout.c | 36 ++++-----------------------
> >>>>>> 1 files changed, 5 insertions(+), 31 deletions(-)
> >>>>>> 
> >>>>>> diff --git a/drivers/media/video/omap/omap_vout.c
> >>>>>> b/drivers/media/video/omap/omap_vout.c
> >>>>>> index 1fb7d5b..dffcf66 100644
> >>>>>> --- a/drivers/media/video/omap/omap_vout.c
> >>>>>> +++ b/drivers/media/video/omap/omap_vout.c
> >>>>>> @@ -1157,13 +1157,6 @@ static int vidioc_s_fmt_vid_out(struct file
> >>>>>> *file, void *fh,
> >>>>>> /* set default crop and win */
> >>>>>> omap_vout_new_format(&vout->pix,&vout->fbuf,&vout->crop,&vout->win);
> >>>>>> 
> >>>>>> - /* Save the changes in the overlay strcuture */
> >>>>>> - ret = omapvid_init(vout, 0);
> >>>>>> - if (ret) {
> >>>>>> - v4l2_err(&vout->vid_dev->v4l2_dev, "failed to change mode\n");
> >>>>>> - goto s_fmt_vid_out_exit;
> >>>>>> - }
> >>>>>> -
> >>>>>> ret = 0;
> >>>>>> 
> >>>>>> s_fmt_vid_out_exit:
> >>>>>> @@ -1664,20 +1657,6 @@ static int vidioc_streamon(struct file *file,
> >>>>>> void *fh, enum v4l2_buf_type i)
> >>>>>> 
> >>>>>> omap_dispc_register_isr(omap_vout_isr, vout, mask);
> >>>>>> 
> >>>>>> - for (j = 0; j<  ovid->num_overlays; j++) {
> >>>>>> - struct omap_overlay *ovl = ovid->overlays[j];
> >>>>>> -
> >>>>>> - if (ovl->manager&&  ovl->manager->device) {
> >>>>>> - struct omap_overlay_info info;
> >>>>>> - ovl->get_overlay_info(ovl,&info);
> >>>>>> - info.paddr = addr;
> >>>>>> - if (ovl->set_overlay_info(ovl,&info)) {
> >>>>>> - ret = -EINVAL;
> >>>>>> - goto streamon_err1;
> >>>>>> - }
> >>>>>> - }
> >>>>>> - }
> >>>>>> -
> >>>>> 
> >>>>> Have you checked for build warnings? I am getting build warnings
> >>>>> 
> >>>>> CC drivers/media/video/omap/omap_vout.o
> >>>>> CC drivers/media/video/omap/omap_voutlib.o
> >>>>> CC drivers/media/video/omap/omap_vout_vrfb.o
> >>>>> drivers/media/video/omap/omap_vout.c: In function 'vidioc_streamon':
> >>>>> drivers/media/video/omap/omap_vout.c:1619:25: warning: unused variable
> >>>>> 'ovid'
> >>>>> drivers/media/video/omap/omap_vout.c:1615:15: warning: unused variable
> >>>>> 'j'
> >>>>> LD drivers/media/video/omap/omap-vout.o
> >>>>> LD drivers/media/video/omap/built-in.o
> >>>>> 
> >>>>> Can you fix this and submit the next version?
> >>> 
> >>> I applied the patch on the current mainline kernel, it doesn't give any
> >>> build warnings. Even after applying the patch, 'j and ovid' are still
> >>> used in vidioc_streamon().
> >>> 
> >>> Can you check if it was applied correctly?
> >> 
> >> Archit,
> >> 
> >> I could able to trace what's going on here,
> >> 
> >> I am using "v4l_for_linus" branch, which has one missing patch,
> >> 
> >> commit aaa874a985158383c4b394c687c716ef26288741
> >> Author: Tomi Valkeinen<tomi.valkeinen@ti.com>
> >> Date:   Tue Nov 15 16:37:53 2011 +0200
> >> 
> >>     OMAPDSS: APPLY: rewrite overlay enable/disable
> >> 
> >> 
> >> So, I do not have below changes,
> >> 
> >> @@ -1686,6 +1681,16 @@ static int vidioc_streamon(struct file *file, void
> >> *fh, enum v4l2_buf_type i)
> >>         if (ret)
> >>                 v4l2_err(&vout->vid_dev->v4l2_dev, "failed to change
> >> mode\n");
> >> 
> >> +       for (j = 0; j<  ovid->num_overlays; j++) {
> >> +               struct omap_overlay *ovl = ovid->overlays[j];
> >> +
> >> +               if (ovl->manager&&  ovl->manager->device) {
> >> 
> >> +                       ret = ovl->enable(ovl);
> >> +                       if (ret)
> >> +                               goto streamon_err1;
> >> +               }
> >> +       }
> >> +
> >> 
> >> This explains why I am seeing these warnings. Let me give pull request
> >> based on master branch.
> > 
> > Okay. Thanks for looking into this.
> > 
> > Archit
> 
> Hi Vaibhav,
> 
> This patch has been outstanding since March, and we have received
> reports from various users. Could you please push it ASAP to Mauro for
> the upcoming -rc?

I second this request. Vaibhav, could you please take care of this ?

> Or Did I miss a pull request from you containing this?

-- 
Regards,

Laurent Pinchart

Re: [PATCH] omap_vout: Set DSS overlay_info only if paddr is non zero

[Tomi Valkeinen]

[-- Attachment #1: Type: text/plain, Size: 2907 bytes --]

On 2012-03-09 10:03, Hiremath, Vaibhav wrote:
> On Fri, Mar 09, 2012 at 05:17:41, Laurent Pinchart wrote:
>> Hi Archit,
>>
>> On Wednesday 07 March 2012 14:31:16 Archit Taneja wrote:
>>> The omap_vout driver tries to set the DSS overlay_info using
>>> set_overlay_info() when the physical address for the overlay is still not
>>> configured. This happens in omap_vout_probe() and vidioc_s_fmt_vid_out().
>>>
>>> The calls to omapvid_init(which internally calls set_overlay_info()) are
>>> removed from these functions. They don't need to be called as the
>>> omap_vout_device struct anyway maintains the overlay related changes made.
>>> Also, remove the explicit call to set_overlay_info() in vidioc_streamon(),
>>> this was used to set the paddr, this isn't needed as omapvid_init() does
>>> the same thing later.
>>>
>>> These changes are required as the DSS2 driver since 3.3 kernel doesn't let
>>> you set the overlay info with paddr as 0.
>>>
>>> Signed-off-by: Archit Taneja <archit@ti.com>
>>
>> Thanks for the patch. This seems to fix memory corruption that would result
>> in sysfs-related crashes such as
>>
>> [   31.279541] ------------[ cut here ]------------
>> [   31.284423] WARNING: at fs/sysfs/file.c:343 sysfs_open_file+0x70/0x1f8()
>> [   31.291503] missing sysfs attribute operations for kobject: (null)
>> [   31.298004] Modules linked in: mt9p031 aptina_pll omap3_isp
>> [   31.303924] [<c0018260>] (unwind_backtrace+0x0/0xec) from [<c0034488>] (warn_slowpath_common+0x4c/0x64)
>> [   31.313812] [<c0034488>] (warn_slowpath_common+0x4c/0x64) from [<c0034520>] (warn_slowpath_fmt+0x2c/0x3c)
>> [   31.323913] [<c0034520>] (warn_slowpath_fmt+0x2c/0x3c) from [<c01219bc>] (sysfs_open_file+0x70/0x1f8)
>> [   31.333618] [<c01219bc>] (sysfs_open_file+0x70/0x1f8) from [<c00ccc94>] (__dentry_open+0x1f8/0x30c)
>> [   31.343139] [<c00ccc94>] (__dentry_open+0x1f8/0x30c) from [<c00cce58>] (nameidata_to_filp+0x50/0x5c)
>> [   31.352752] [<c00cce58>] (nameidata_to_filp+0x50/0x5c) from [<c00db4c0>] (do_last+0x55c/0x6a0)
>> [   31.361999] [<c00db4c0>] (do_last+0x55c/0x6a0) from [<c00db6bc>] (path_openat+0xb8/0x37c)
>> [   31.370605] [<c00db6bc>] (path_openat+0xb8/0x37c) from [<c00dba60>] (do_filp_open+0x30/0x7c)
>> [   31.379486] [<c00dba60>] (do_filp_open+0x30/0x7c) from [<c00cc904>] (do_sys_open+0xd8/0x170)
>> [   31.388366] [<c00cc904>] (do_sys_open+0xd8/0x170) from [<c0012760>] (ret_fast_syscall+0x0/0x3c)
>> [   31.397552] ---[ end trace 13639ab74f345d7e ]---
>>
>> Tested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>>
> 
> Thanks Laurent for testing this patch.
> 
> 
>> Please push it to v3.3 :-)
>>
> 
> Will send a pull request today itself.

Vaibhav, I don't see this crash fix in 3.3, 3.4, 3.5, 3.6 nor in 3.7-rc.
Are you still maintaining the omap v4l2 driver? Can you finally push
this fix?

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 897 bytes --]

RE: [PATCH] omap_vout: Set DSS overlay_info only if paddr is non zero

[Hiremath, Vaibhav]

On Thu, Oct 25, 2012 at 19:30:58, Valkeinen, Tomi wrote:
> On 2012-03-09 10:03, Hiremath, Vaibhav wrote:
> > On Fri, Mar 09, 2012 at 05:17:41, Laurent Pinchart wrote:
> >> Hi Archit,
> >>
> >> On Wednesday 07 March 2012 14:31:16 Archit Taneja wrote:
> >>> The omap_vout driver tries to set the DSS overlay_info using
> >>> set_overlay_info() when the physical address for the overlay is still not
> >>> configured. This happens in omap_vout_probe() and vidioc_s_fmt_vid_out().
> >>>
> >>> The calls to omapvid_init(which internally calls set_overlay_info()) are
> >>> removed from these functions. They don't need to be called as the
> >>> omap_vout_device struct anyway maintains the overlay related changes made.
> >>> Also, remove the explicit call to set_overlay_info() in vidioc_streamon(),
> >>> this was used to set the paddr, this isn't needed as omapvid_init() does
> >>> the same thing later.
> >>>
> >>> These changes are required as the DSS2 driver since 3.3 kernel doesn't let
> >>> you set the overlay info with paddr as 0.
> >>>
> >>> Signed-off-by: Archit Taneja <archit@ti.com>
> >>
> >> Thanks for the patch. This seems to fix memory corruption that would result
> >> in sysfs-related crashes such as
> >>
> >> [   31.279541] ------------[ cut here ]------------
> >> [   31.284423] WARNING: at fs/sysfs/file.c:343 sysfs_open_file+0x70/0x1f8()
> >> [   31.291503] missing sysfs attribute operations for kobject: (null)
> >> [   31.298004] Modules linked in: mt9p031 aptina_pll omap3_isp
> >> [   31.303924] [<c0018260>] (unwind_backtrace+0x0/0xec) from [<c0034488>] (warn_slowpath_common+0x4c/0x64)
> >> [   31.313812] [<c0034488>] (warn_slowpath_common+0x4c/0x64) from [<c0034520>] (warn_slowpath_fmt+0x2c/0x3c)
> >> [   31.323913] [<c0034520>] (warn_slowpath_fmt+0x2c/0x3c) from [<c01219bc>] (sysfs_open_file+0x70/0x1f8)
> >> [   31.333618] [<c01219bc>] (sysfs_open_file+0x70/0x1f8) from [<c00ccc94>] (__dentry_open+0x1f8/0x30c)
> >> [   31.343139] [<c00ccc94>] (__dentry_open+0x1f8/0x30c) from [<c00cce58>] (nameidata_to_filp+0x50/0x5c)
> >> [   31.352752] [<c00cce58>] (nameidata_to_filp+0x50/0x5c) from [<c00db4c0>] (do_last+0x55c/0x6a0)
> >> [   31.361999] [<c00db4c0>] (do_last+0x55c/0x6a0) from [<c00db6bc>] (path_openat+0xb8/0x37c)
> >> [   31.370605] [<c00db6bc>] (path_openat+0xb8/0x37c) from [<c00dba60>] (do_filp_open+0x30/0x7c)
> >> [   31.379486] [<c00dba60>] (do_filp_open+0x30/0x7c) from [<c00cc904>] (do_sys_open+0xd8/0x170)
> >> [   31.388366] [<c00cc904>] (do_sys_open+0xd8/0x170) from [<c0012760>] (ret_fast_syscall+0x0/0x3c)
> >> [   31.397552] ---[ end trace 13639ab74f345d7e ]---
> >>
> >> Tested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >>
> > 
> > Thanks Laurent for testing this patch.
> > 
> > 
> >> Please push it to v3.3 :-)
> >>
> > 
> > Will send a pull request today itself.
> 
> Vaibhav, I don't see this crash fix in 3.3, 3.4, 3.5, 3.6 nor in 3.7-rc.
> Are you still maintaining the omap v4l2 driver? Can you finally push
> this fix?
> 

Tomi, Sorry for delayed response.

Laurent,
Can you pull up this patch into your next pull request?

Thanks,
Vaibhav

Re: [PATCH] omap_vout: Set DSS overlay_info only if paddr is non zero

[Laurent Pinchart]

Hi Vaibhav,

On Friday 26 October 2012 09:13:20 Hiremath, Vaibhav wrote:
> On Thu, Oct 25, 2012 at 19:30:58, Valkeinen, Tomi wrote:
> > On 2012-03-09 10:03, Hiremath, Vaibhav wrote:
> > > On Fri, Mar 09, 2012 at 05:17:41, Laurent Pinchart wrote:
> > >> On Wednesday 07 March 2012 14:31:16 Archit Taneja wrote:
> > >>> The omap_vout driver tries to set the DSS overlay_info using
> > >>> set_overlay_info() when the physical address for the overlay is still
> > >>> not configured. This happens in omap_vout_probe() and
> > >>> vidioc_s_fmt_vid_out().
> > >>> 
> > >>> The calls to omapvid_init(which internally calls set_overlay_info())
> > >>> are removed from these functions. They don't need to be called as the
> > >>> omap_vout_device struct anyway maintains the overlay related changes
> > >>> made. Also, remove the explicit call to set_overlay_info() in
> > >>> vidioc_streamon(), this was used to set the paddr, this isn't needed
> > >>> as omapvid_init() does the same thing later.
> > >>> 
> > >>> These changes are required as the DSS2 driver since 3.3 kernel doesn't
> > >>> let you set the overlay info with paddr as 0.
> > >>> 
> > >>> Signed-off-by: Archit Taneja <archit@ti.com>
> > >> 
> > >> Thanks for the patch. This seems to fix memory corruption that would
> > >> result in sysfs-related crashes such as
> > >> 
> > >> [   31.279541] ------------[ cut here ]------------
> > >> [   31.284423] WARNING: at fs/sysfs/file.c:343
> > >> sysfs_open_file+0x70/0x1f8()
> > >> [   31.291503] missing sysfs attribute operations for kobject: (null)
> > >> [   31.298004] Modules linked in: mt9p031 aptina_pll omap3_isp
> > >> [   31.303924] [<c0018260>] (unwind_backtrace+0x0/0xec) from
> > >> [<c0034488>] (warn_slowpath_common+0x4c/0x64) [   31.313812]
> > >> [<c0034488>] (warn_slowpath_common+0x4c/0x64) from [<c0034520>]
> > >> (warn_slowpath_fmt+0x2c/0x3c) [   31.323913] [<c0034520>]
> > >> (warn_slowpath_fmt+0x2c/0x3c) from [<c01219bc>]
> > >> (sysfs_open_file+0x70/0x1f8) [   31.333618] [<c01219bc>]
> > >> (sysfs_open_file+0x70/0x1f8) from [<c00ccc94>]
> > >> (__dentry_open+0x1f8/0x30c) [   31.343139] [<c00ccc94>]
> > >> (__dentry_open+0x1f8/0x30c) from [<c00cce58>]
> > >> (nameidata_to_filp+0x50/0x5c) [   31.352752] [<c00cce58>]
> > >> (nameidata_to_filp+0x50/0x5c) from [<c00db4c0>] (do_last+0x55c/0x6a0)
> > >> [   31.361999] [<c00db4c0>] (do_last+0x55c/0x6a0) from [<c00db6bc>]
> > >> (path_openat+0xb8/0x37c) [   31.370605] [<c00db6bc>]
> > >> (path_openat+0xb8/0x37c) from [<c00dba60>] (do_filp_open+0x30/0x7c) [ 
> > >>  31.379486] [<c00dba60>] (do_filp_open+0x30/0x7c) from [<c00cc904>]
> > >> (do_sys_open+0xd8/0x170) [   31.388366] [<c00cc904>]
> > >> (do_sys_open+0xd8/0x170) from [<c0012760>] (ret_fast_syscall+0x0/0x3c)
> > >> [   31.397552] ---[ end trace 13639ab74f345d7e ]---
> > >> 
> > >> Tested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > 
> > > Thanks Laurent for testing this patch.
> > > 
> > >> Please push it to v3.3 :-)
> > > 
> > > Will send a pull request today itself.
> > 
> > Vaibhav, I don't see this crash fix in 3.3, 3.4, 3.5, 3.6 nor in 3.7-rc.
> > Are you still maintaining the omap v4l2 driver? Can you finally push
> > this fix?
> 
> Tomi, Sorry for delayed response.
> 
> Laurent,
> Can you pull up this patch into your next pull request?

I've asked Mauro to pull the patch for v3.8.

-- 
Regards,

Laurent Pinchart

RE: [PATCH] omap_vout: Set DSS overlay_info only if paddr is non zero

[Hiremath, Vaibhav]

On Sat, Oct 27, 2012 at 17:01:09, Laurent Pinchart wrote:
> Hi Vaibhav,
> 
> On Friday 26 October 2012 09:13:20 Hiremath, Vaibhav wrote:
> > On Thu, Oct 25, 2012 at 19:30:58, Valkeinen, Tomi wrote:
> > > On 2012-03-09 10:03, Hiremath, Vaibhav wrote:
> > > > On Fri, Mar 09, 2012 at 05:17:41, Laurent Pinchart wrote:
> > > >> On Wednesday 07 March 2012 14:31:16 Archit Taneja wrote:
> > > >>> The omap_vout driver tries to set the DSS overlay_info using
> > > >>> set_overlay_info() when the physical address for the overlay is still
> > > >>> not configured. This happens in omap_vout_probe() and
> > > >>> vidioc_s_fmt_vid_out().
> > > >>> 
> > > >>> The calls to omapvid_init(which internally calls set_overlay_info())
> > > >>> are removed from these functions. They don't need to be called as the
> > > >>> omap_vout_device struct anyway maintains the overlay related changes
> > > >>> made. Also, remove the explicit call to set_overlay_info() in
> > > >>> vidioc_streamon(), this was used to set the paddr, this isn't needed
> > > >>> as omapvid_init() does the same thing later.
> > > >>> 
> > > >>> These changes are required as the DSS2 driver since 3.3 kernel doesn't
> > > >>> let you set the overlay info with paddr as 0.
> > > >>> 
> > > >>> Signed-off-by: Archit Taneja <archit@ti.com>
> > > >> 
> > > >> Thanks for the patch. This seems to fix memory corruption that would
> > > >> result in sysfs-related crashes such as
> > > >> 
> > > >> [   31.279541] ------------[ cut here ]------------
> > > >> [   31.284423] WARNING: at fs/sysfs/file.c:343
> > > >> sysfs_open_file+0x70/0x1f8()
> > > >> [   31.291503] missing sysfs attribute operations for kobject: (null)
> > > >> [   31.298004] Modules linked in: mt9p031 aptina_pll omap3_isp
> > > >> [   31.303924] [<c0018260>] (unwind_backtrace+0x0/0xec) from
> > > >> [<c0034488>] (warn_slowpath_common+0x4c/0x64) [   31.313812]
> > > >> [<c0034488>] (warn_slowpath_common+0x4c/0x64) from [<c0034520>]
> > > >> (warn_slowpath_fmt+0x2c/0x3c) [   31.323913] [<c0034520>]
> > > >> (warn_slowpath_fmt+0x2c/0x3c) from [<c01219bc>]
> > > >> (sysfs_open_file+0x70/0x1f8) [   31.333618] [<c01219bc>]
> > > >> (sysfs_open_file+0x70/0x1f8) from [<c00ccc94>]
> > > >> (__dentry_open+0x1f8/0x30c) [   31.343139] [<c00ccc94>]
> > > >> (__dentry_open+0x1f8/0x30c) from [<c00cce58>]
> > > >> (nameidata_to_filp+0x50/0x5c) [   31.352752] [<c00cce58>]
> > > >> (nameidata_to_filp+0x50/0x5c) from [<c00db4c0>] (do_last+0x55c/0x6a0)
> > > >> [   31.361999] [<c00db4c0>] (do_last+0x55c/0x6a0) from [<c00db6bc>]
> > > >> (path_openat+0xb8/0x37c) [   31.370605] [<c00db6bc>]
> > > >> (path_openat+0xb8/0x37c) from [<c00dba60>] (do_filp_open+0x30/0x7c) [ 
> > > >>  31.379486] [<c00dba60>] (do_filp_open+0x30/0x7c) from [<c00cc904>]
> > > >> (do_sys_open+0xd8/0x170) [   31.388366] [<c00cc904>]
> > > >> (do_sys_open+0xd8/0x170) from [<c0012760>] (ret_fast_syscall+0x0/0x3c)
> > > >> [   31.397552] ---[ end trace 13639ab74f345d7e ]---
> > > >> 
> > > >> Tested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > > 
> > > > Thanks Laurent for testing this patch.
> > > > 
> > > >> Please push it to v3.3 :-)
> > > > 
> > > > Will send a pull request today itself.
> > > 
> > > Vaibhav, I don't see this crash fix in 3.3, 3.4, 3.5, 3.6 nor in 3.7-rc.
> > > Are you still maintaining the omap v4l2 driver? Can you finally push
> > > this fix?
> > 
> > Tomi, Sorry for delayed response.
> > 
> > Laurent,
> > Can you pull up this patch into your next pull request?
> 
> I've asked Mauro to pull the patch for v3.8.
> 

Thanks Laurent.

Thanks,
Vaibhav