* [PATCH] omap_vout: Set DSS overlay_info only if paddr is non zero @ 2012-03-07 9:01 Archit Taneja 2012-03-08 23:47 ` Laurent Pinchart 2012-03-12 10:04 ` Hiremath, Vaibhav 0 siblings, 2 replies; 13+ messages in thread From: Archit Taneja @ 2012-03-07 9:01 UTC (permalink / raw) To: hvaibhav; +Cc: tomi.valkeinen, linux-omap, linux-media, 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 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH] omap_vout: Set DSS overlay_info only if paddr is non zero 2012-03-07 9:01 [PATCH] omap_vout: Set DSS overlay_info only if paddr is non zero Archit Taneja @ 2012-03-08 23:47 ` Laurent Pinchart 2012-03-09 8:03 ` Hiremath, Vaibhav 2012-03-12 10:04 ` Hiremath, Vaibhav 1 sibling, 1 reply; 13+ messages in thread From: Laurent Pinchart @ 2012-03-08 23:47 UTC (permalink / raw) To: Archit Taneja; +Cc: hvaibhav, tomi.valkeinen, linux-omap, linux-media 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* RE: [PATCH] omap_vout: Set DSS overlay_info only if paddr is non zero 2012-03-08 23:47 ` Laurent Pinchart @ 2012-03-09 8:03 ` Hiremath, Vaibhav 2012-10-25 14:00 ` Tomi Valkeinen 0 siblings, 1 reply; 13+ messages in thread From: Hiremath, Vaibhav @ 2012-03-09 8:03 UTC (permalink / raw) To: Laurent Pinchart, Taneja, Archit Cc: Valkeinen, Tomi, linux-omap@vger.kernel.org, linux-media@vger.kernel.org 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 > > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] omap_vout: Set DSS overlay_info only if paddr is non zero 2012-03-09 8:03 ` Hiremath, Vaibhav @ 2012-10-25 14:00 ` Tomi Valkeinen 2012-10-26 9:13 ` Hiremath, Vaibhav 0 siblings, 1 reply; 13+ messages in thread From: Tomi Valkeinen @ 2012-10-25 14:00 UTC (permalink / raw) To: Hiremath, Vaibhav Cc: Laurent Pinchart, Taneja, Archit, linux-omap@vger.kernel.org, linux-media@vger.kernel.org [-- 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 --] ^ permalink raw reply [flat|nested] 13+ messages in thread
* RE: [PATCH] omap_vout: Set DSS overlay_info only if paddr is non zero 2012-10-25 14:00 ` Tomi Valkeinen @ 2012-10-26 9:13 ` Hiremath, Vaibhav 2012-10-27 11:31 ` Laurent Pinchart 0 siblings, 1 reply; 13+ messages in thread From: Hiremath, Vaibhav @ 2012-10-26 9:13 UTC (permalink / raw) To: Valkeinen, Tomi Cc: Laurent Pinchart, Taneja, Archit, linux-omap@vger.kernel.org, linux-media@vger.kernel.org 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] omap_vout: Set DSS overlay_info only if paddr is non zero 2012-10-26 9:13 ` Hiremath, Vaibhav @ 2012-10-27 11:31 ` Laurent Pinchart 0 siblings, 0 replies; 13+ messages in thread From: Laurent Pinchart @ 2012-10-27 11:31 UTC (permalink / raw) To: Hiremath, Vaibhav Cc: Valkeinen, Tomi, Taneja, Archit, linux-omap@vger.kernel.org, linux-media@vger.kernel.org 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* RE: [PATCH] omap_vout: Set DSS overlay_info only if paddr is non zero 2012-03-07 9:01 [PATCH] omap_vout: Set DSS overlay_info only if paddr is non zero Archit Taneja 2012-03-08 23:47 ` Laurent Pinchart @ 2012-03-12 10:04 ` Hiremath, Vaibhav 2012-03-16 10:16 ` Archit Taneja 1 sibling, 1 reply; 13+ messages in thread From: Hiremath, Vaibhav @ 2012-03-12 10:04 UTC (permalink / raw) To: Taneja, Archit Cc: Valkeinen, Tomi, linux-omap@vger.kernel.org, linux-media@vger.kernel.org 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 > > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] omap_vout: Set DSS overlay_info only if paddr is non zero 2012-03-12 10:04 ` Hiremath, Vaibhav @ 2012-03-16 10:16 ` Archit Taneja 2012-03-16 11:11 ` Archit Taneja 0 siblings, 1 reply; 13+ messages in thread From: Archit Taneja @ 2012-03-16 10:16 UTC (permalink / raw) To: Hiremath, Vaibhav Cc: Taneja, Archit, Valkeinen, Tomi, linux-omap@vger.kernel.org, linux-media@vger.kernel.org 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 >> >> > > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] omap_vout: Set DSS overlay_info only if paddr is non zero 2012-03-16 10:16 ` Archit Taneja @ 2012-03-16 11:11 ` Archit Taneja 2012-03-19 8:45 ` Hiremath, Vaibhav 0 siblings, 1 reply; 13+ messages in thread From: Archit Taneja @ 2012-03-16 11:11 UTC (permalink / raw) Cc: Hiremath, Vaibhav, Valkeinen, Tomi, linux-omap@vger.kernel.org, linux-media@vger.kernel.org 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 >>> >>> >> >> > > ^ permalink raw reply [flat|nested] 13+ messages in thread
* RE: [PATCH] omap_vout: Set DSS overlay_info only if paddr is non zero 2012-03-16 11:11 ` Archit Taneja @ 2012-03-19 8:45 ` Hiremath, Vaibhav 2012-03-19 11:46 ` Archit Taneja 0 siblings, 1 reply; 13+ messages in thread From: Hiremath, Vaibhav @ 2012-03-19 8:45 UTC (permalink / raw) To: Taneja, Archit Cc: Valkeinen, Tomi, linux-omap@vger.kernel.org, linux-media@vger.kernel.org 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 > >>> > >>> > >> > >> > > > > > > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] omap_vout: Set DSS overlay_info only if paddr is non zero 2012-03-19 8:45 ` Hiremath, Vaibhav @ 2012-03-19 11:46 ` Archit Taneja 2012-06-28 6:06 ` Semwal, Sumit 0 siblings, 1 reply; 13+ messages in thread From: Archit Taneja @ 2012-03-19 11:46 UTC (permalink / raw) To: Hiremath, Vaibhav Cc: Taneja, Archit, Valkeinen, Tomi, linux-omap@vger.kernel.org, linux-media@vger.kernel.org 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 >>>>> >>>>> >>>> >>>> >>> >>> >> >> > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] omap_vout: Set DSS overlay_info only if paddr is non zero 2012-03-19 11:46 ` Archit Taneja @ 2012-06-28 6:06 ` Semwal, Sumit 2012-08-07 11:04 ` Laurent Pinchart 0 siblings, 1 reply; 13+ messages in thread From: Semwal, Sumit @ 2012-06-28 6:06 UTC (permalink / raw) To: Archit Taneja Cc: Hiremath, Vaibhav, Taneja, Archit, Valkeinen, Tomi, linux-omap@vger.kernel.org, linux-media@vger.kernel.org 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] omap_vout: Set DSS overlay_info only if paddr is non zero 2012-06-28 6:06 ` Semwal, Sumit @ 2012-08-07 11:04 ` Laurent Pinchart 0 siblings, 0 replies; 13+ messages in thread From: Laurent Pinchart @ 2012-08-07 11:04 UTC (permalink / raw) To: Semwal, Sumit Cc: Archit Taneja, Hiremath, Vaibhav, Taneja, Archit, Valkeinen, Tomi, linux-omap@vger.kernel.org, linux-media@vger.kernel.org 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2012-10-27 11:30 UTC | newest] Thread overview: 13+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-03-07 9:01 [PATCH] omap_vout: Set DSS overlay_info only if paddr is non zero Archit Taneja 2012-03-08 23:47 ` Laurent Pinchart 2012-03-09 8:03 ` Hiremath, Vaibhav 2012-10-25 14:00 ` Tomi Valkeinen 2012-10-26 9:13 ` Hiremath, Vaibhav 2012-10-27 11:31 ` Laurent Pinchart 2012-03-12 10:04 ` Hiremath, Vaibhav 2012-03-16 10:16 ` Archit Taneja 2012-03-16 11:11 ` Archit Taneja 2012-03-19 8:45 ` Hiremath, Vaibhav 2012-03-19 11:46 ` Archit Taneja 2012-06-28 6:06 ` Semwal, Sumit 2012-08-07 11:04 ` Laurent Pinchart
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).