RE: [PATCH 3/3] OMAP2/3 V4L2 Display Driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Inki,
I was on vacation just came, 
Below are my comments.

> -----Original Message-----
> From: InKi Dae [mailto:daeinki@xxxxxxxxx]
> Sent: Thursday, April 30, 2009 6:19 PM
> To: Shah, Hardik
> Cc: tomi.valkeinen@xxxxxxxxx; linux-media@xxxxxxxxxxxxxxx; linux-
> omap@xxxxxxxxxxxxxxx; Jadav, Brijesh R; Hiremath, Vaibhav
> Subject: Re: [PATCH 3/3] OMAP2/3 V4L2 Display Driver
> 
> Thank you Shah, Hardik.
> I have patched your driver through that git.
> 
> but your patch recognizes video2 layer as video1 device node in case
> that DSS2 has fb0 and fb1.
[Shah, Hardik] There is no need to change the nodes.  If there is only one video device present than it should be /dev/video1 and not /dev/video2.  Its upto the driver to use which video pipeline 1 or 2. So let the node nomenclature be same.

Regards,
Hardik 
> 
> you said my patch will give rise to couple of more bugs related to
> global_alpha and pixel formats
> maybe that would be because of vout->vid_info.id = k + vout_count;
> 
> so I have applied your patch to my way.
> this patch is more flexible and recognizes video2 layer correctly as
> video2 device node.
> ------------------------------------------------------------------------------
> ------------------------------------------------------------------------
> --- a/drivers/media/video/omap/omap_vout.c
> +++ b/drivers/media/video/omap/omap_vout.c
> @@ -2204,7 +2204,7 @@ free_buffers:
>  /* Create video out devices */
>  static int __init omap_vout_create_video_devices(struct platform_device
> *pdev)
>  {
> -       int r = 0, k;
> +       int r = 0, k, vout_count;
>         struct omap_vout_device *vout;
>         struct video_device *vfd = NULL;
>         struct v4l2_device *v4l2_dev = platform_get_drvdata(pdev);
> @@ -2212,6 +2212,8 @@ static int __init
> omap_vout_create_video_devices(struct platform_device *pdev)
>         struct omap2video_device *vid_dev = container_of(v4l2_dev, struct
>                         omap2video_device, v4l2_dev);
> 
> +       vout_count = 3 - pdev->num_resources;
> +
>         for (k = 0; k < pdev->num_resources; k++) {
> 
>                 vout = kmalloc(sizeof(struct omap_vout_device), GFP_KERNEL);
> @@ -2225,12 +2227,7 @@ static int __init
> omap_vout_create_video_devices(struct platform_device *pdev)
>                 vout->vid = k;
>                 vid_dev->vouts[k] = vout;
>                 vout->vid_dev = vid_dev;
> -               /* Select video2 if only 1 overlay is controlled by V4L2 */
> -               if (pdev->num_resources == 1)
> -                       vout->vid_info.overlays[0] = vid_dev->overlays[k + 2];
> -               else
> -                       /* Else select video1 and video2 one by one. */
> -                       vout->vid_info.overlays[0] = vid_dev->overlays[k + 1];
> +               vout->vid_info.overlays[0] = vid_dev->overlays[k +
> vout_count];
>                 vout->vid_info.num_overlays = 1;
>                 vout->vid_info.id = k + 1;
>                 vid_dev->num_videos++;
> @@ -2253,7 +2250,7 @@ static int __init
> omap_vout_create_video_devices(struct platform_device *pdev)
>                 /* Register the Video device with V4L2
>                 */
>                 vfd = vout->vfd;
> -               if (video_register_device(vfd, VFL_TYPE_GRABBER, k + 1) < 0) {
> +               if (video_register_device(vfd, VFL_TYPE_GRABBER, k +
> vout_count) < 0) {
>                         printk(KERN_ERR VOUT_NAME ": could not register "
>                                         "Video for Linux device\n");
>                         vfd->minor = -1
> ------------------------------------------------------------------------------
> ------------------------------------------------------------------------------
> ---------
> 
> 2009/4/30 Shah, Hardik <hardik.shah@xxxxxx>:
> 
> >
> >> -----Original Message-----
> >> From: linux-media-owner@xxxxxxxxxxxxxxx [mailto:linux-media-
> >> owner@xxxxxxxxxxxxxxx] On Behalf Of Shah, Hardik
> >> Sent: Thursday, April 30, 2009 11:51 AM
> >> To: InKi Dae
> >> Cc: tomi.valkeinen@xxxxxxxxx; linux-media@xxxxxxxxxxxxxxx; linux-
> >> omap@xxxxxxxxxxxxxxx; Jadav, Brijesh R; Hiremath, Vaibhav
> >> Subject: RE: [PATCH 3/3] OMAP2/3 V4L2 Display Driver
> >>
> >>
> >>
> >> > -----Original Message-----
> >> > From: InKi Dae [mailto:daeinki@xxxxxxxxx]
> >> > Sent: Thursday, April 30, 2009 11:48 AM
> >> > To: Shah, Hardik
> >> > Cc: tomi.valkeinen@xxxxxxxxx; linux-media@xxxxxxxxxxxxxxx; linux-
> >> > omap@xxxxxxxxxxxxxxx; Jadav, Brijesh R; Hiremath, Vaibhav
> >> > Subject: Re: [PATCH 3/3] OMAP2/3 V4L2 Display Driver
> >> >
> >> > hello Shah, Hardik..
> >> >
> >> > your omap_vout.c has the problem that it disables video1 or fb1.
> >> > so I have modified your code.
> >> >
> >> > I defined and set platform_data for DSS2 in machine code.(or board file)
> >> >
> >> > static struct omapfb_platform_data xxx_dss_platform_data = {
> >> >     .mem_desc.region[0].format = OMAPFB_COLOR_ARGB32,
> >> >     .mem_desc.region[0].format_used=1,
> >> >
> >> >     .mem_desc.region[1].format = OMAPFB_COLOR_RGB24U,
> >> >     .mem_desc.region[1].format_used=1,
> >> >
> >> >     .mem_desc.region[2].format = OMAPFB_COLOR_ARGB32,
> >> >     .mem_desc.region[2].format_used=1,
> >> > };
> >> >
> >> > omapfb_set_platform_data(&xxx_dss_platform_data);
> >> >
> >> > after that, omap_vout has resource count got referring to framebuffer
> count,
> >> > registers overlay as vout's one and would decide to use which overlay.
> >> >
> >> > at that time, your code would face with impact on some overlay(fb or
> video).
> >> >
> >> > this patch would solve that problem.
> >> > when it sets overlay to vout, vout would get overlay array index to
> >> > avoid overlapping with other overlay.
> >> >
> >> >
> >> > sighed-off-by: InKi Dae. <inki.dae@xxxxxxxxxxx>
> >> > ---
> >> > diff --git a/drivers/media/video/omap/omap_vout.c
> >> > b/drivers/media/video/omap/omap_vout.c
> >> > index 9b4a0d7..051298a 100644
> >> > --- a/drivers/media/video/omap/omap_vout.c
> >> > +++ b/drivers/media/video/omap/omap_vout.c
> >> > @@ -2246,11 +2246,13 @@ free_buffers:
> >> >  /* Create video out devices */
> >> >  static int __init omap_vout_create_video_devices(struct platform_device
> >> > *pdev)
> >> >  {
> >> > -   int r = 0, k;
> >> > +   int r = 0, k, vout_count;
> >> >     struct omap_vout_device *vout;
> >> >     struct video_device *vfd = NULL;
> >> >     struct omap2video_device *vid_dev = platform_get_drvdata(pdev);
> >> >
> >> > +   vout_count = 3 - pdev->num_resources;
> >> > +
> >> >     for (k = 0; k < pdev->num_resources; k++) {
> >> >
> >> >             vout = kmalloc(sizeof(struct omap_vout_device), GFP_KERNEL);
> >> > @@ -2266,9 +2268,9 @@ static int __init
> >> > omap_vout_create_video_devices(struct platform_device *pdev)
> >> >             vout->vid = k;
> >> >             vid_dev->vouts[k] = vout;
> >> >             vout->vid_info.vid_dev = vid_dev;
> >> > -           vout->vid_info.overlays[0] = vid_dev->overlays[k + 1];
> >> > +           vout->vid_info.overlays[0] = vid_dev->overlays[k +
> vout_count];
> >> >             vout->vid_info.num_overlays = 1;
> >> > -           vout->vid_info.id = k + 1;
> >> > +           vout->vid_info.id = k + vout_count;
> >> >             vid_dev->num_videos++;
> >> >
> >> >             /* Setup the default configuration for the video devices
> >> > @@ -2289,7 +2291,7 @@ static int __init
> >> > omap_vout_create_video_devices(struct platform_device *pdev)
> >> >             /* Register the Video device with V4L2
> >> >              */
> >> >             vfd = vout->vfd;
> >> > -           if (video_register_device(vfd, VFL_TYPE_GRABBER, k + 1) < 0)
> {
> >> > +           if (video_register_device(vfd, VFL_TYPE_GRABBER, k +
> vout_count) <
> >> > 0) {
> >> >                     printk(KERN_ERR VOUT_NAME ": could not register \
> >> >                                     Video for Linux device\n");
> >> >                     vfd->minor = -1;
> >> >
> >> >
> >> > 2009/4/22 Shah, Hardik <hardik.shah@xxxxxx>:
> >> [Shah, Hardik] Yes this is correct,
> >> I will apply this patch.  I already found it and fixed it in different way
> but
> >> any way I will apply your patch.
> > [Shah, Hardik] Further on this inki.  Solving this bug will give rise to
> couple of more bugs related to global_alpha and pixel formats. That also is
> fixed. You can refer http://arago-project.org/git/people/vaibhav/ti-psp-omap-
> video.git?p=people/vaibhav/ti-psp-omap-video.git;a=summary
> >> > >
> >> > >
> >> > >> -----Original Message-----
> >> > >> From: Tomi Valkeinen [mailto:tomi.valkeinen@xxxxxxxxx]
> >> > >> Sent: Wednesday, April 22, 2009 1:53 PM
> >> > >> To: Shah, Hardik
> >> > >> Cc: linux-media@xxxxxxxxxxxxxxx; linux-omap@xxxxxxxxxxxxxxx; Jadav,
> >> Brijesh
> >> > R;
> >> > >> Hiremath, Vaibhav
> >> > >> Subject: Re: [PATCH 3/3] OMAP2/3 V4L2 Display Driver
> >> > >>
> >> > >> Hi,
> >> > >>
> >> > >> On Wed, 2009-04-22 at 08:25 +0200, ext Hardik Shah wrote:
> >> > >> > This is the version 5th of the Driver.
> >> > >> >
> >> > >> > Following are the features supported.
> >> > >> > 1. Provides V4L2 user interface for the video pipelines of DSS
> >> > >> > 2. Basic streaming working on LCD and TV.
> >> > >> > 3. Support for various pixel formats like YUV, UYVY, RGB32, RGB24,
> >> RGB565
> >> > >> > 4. Supports Alpha blending.
> >> > >> > 5. Supports Color keying both source and destination.
> >> > >> > 6. Supports rotation with RGB565 and RGB32 pixels formats.
> >> > >> > 7. Supports cropping.
> >> > >> > 8. Supports Background color setting.
> >> > >> > 9. Works on latest DSS2 library from Tomi
> >> > >> > http://www.bat.org/~tomba/git/linux-omap-dss.git/
> >> > >> > 10. 1/4x scaling added.  Detail testing left
> >> > >> >
> >> > >> > TODOS
> >> > >> > 1. Ioctls needs to be added for color space conversion matrix
> >> > >> > coefficient programming.
> >> > >> > 2. To be tested on DVI resolutions.
> >> > >> >
> >> > >> > Comments fixed from community.
> >> > >> > 1. V4L2 Driver for OMAP3/3 DSS.
> >> > >> > 2.  Conversion of the custom ioctls to standard V4L2 ioctls like
> alpha
> >> > >> blending,
> >> > >> > color keying, rotation and back ground color setting
> >> > >> > 3.  Re-organised the code as per community comments.
> >> > >> > 4.  Added proper copyright year.
> >> > >> > 5.  Added module name in printk
> >> > >> > 6.  Kconfig option copy/paste error
> >> > >> > 7.  Module param desc addded.
> >> > >> > 8.  Query control implemented using standard query_fill
> >> > >> > 9.  Re-arranged if-else constructs.
> >> > >> > 10. Changed to use mutex instead of semaphore.
> >> > >> > 11. Removed dual usage of rotation angles.
> >> > >> > 12. Implemented function to convert the V4L2 angle to DSS angle.
> >> > >> > 13. Y-position was set half by video driver for TV output
> >> > >> > Now its done by DSS so removed that.
> >> > >> > 14. Minor cleanup
> >> > >> > 15. Added support to pass the page offset to application.
> >> > >> > 14. Minor cleanup
> >> > >> > 15. Added support to pass the page offset to application.
> >> > >> > 16. Renamed V4L2_CID_ROTATION to V4L2_CID_ROTATE
> >> > >> > 17. Major comment from Hans fixed.
> >> > >> > 18. Copy right year changed.
> >> > >> > 19. Added module name for each error/warning print message.
> >> > >> >
> >> > >> > Changes from Previous Version.
> >> > >> > 1. Supported YUV rotation.
> >> > >> > 2. Supported Flipping.
> >> > >> > 3. Rebased line with Tomi's latest DSS2 master branch with commit
>  id
> >> > >> > f575a02edf2218a18d6f2ced308b4f3e26b44ce2.
> >> > >> > 4. Kconfig option removed to select between the TV and LCD.
> >> > >> > Now supported dynamically by DSS2 library.
> >> > >> > 5. Kconfig option for the NTSC_M and PAL_BDGHI mode but not
> >> > >> > supported by DSS2.  so it will not work now.
> >> > >>
> >> > >> There is basic support for this. See the DSS doc:
> >> > >>
> >> > >> /sys/devices/platform/omapdss/display? directory:
> >> > >> ...
> >> > >> timings         Display timings
> >> > (pixclock,xres/hfp/hbp/hsw,yres/vfp/vbp/vsw)
> >> > >>                 When writing, two special timings are accepted for tv-
> >> out:
> >> > >>                 "pal" and "ntsc"
> >> > > [Shah, Hardik] I was not aware of it will remove the compile time
> option
> >> and
> >> > for now let the sysfs entry change the standard.  In future I will try to
> do
> >> > it with the S_STD and G_STD ioctls of the V4L2 framework.
> >> > >>
> >> > >>  Tomi
> >> > >>
> >> > >>
> >> > >
> >> > > --
> >> > > To unsubscribe from this list: send the line "unsubscribe linux-media"
> in
> >> > > the body of a message to majordomo@xxxxxxxxxxxxxxx
> >> > > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> >> > >
> >>
> >> --
> >> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> >> the body of a message to majordomo@xxxxxxxxxxxxxxx
> >> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> >
> >

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

[Index of Archives]     [Linux Arm (vger)]     [ARM Kernel]     [ARM MSM]     [Linux Tegra]     [Linux WPAN Networking]     [Linux Wireless Networking]     [Maemo Users]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux