RE: [PATCH] OMAP 2/3 V4L2 display driver on video planes

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

 




> -----Original Message-----
> From: Hans Verkuil [mailto:hverkuil@xxxxxxxxx]
> Sent: Monday, October 06, 2008 11:59 AM
> To: Shah, Hardik
> Cc: Mauro Carvalho Chehab; video4linux-list@xxxxxxxxxx; linux-
> omap@xxxxxxxxxxxxxxx; linux-fbdev-devel@xxxxxxxxxxxxxxxxxxxxx; Hadli,
> Manjunath
> Subject: Re: [PATCH] OMAP 2/3 V4L2 display driver on video planes
> 
> On Monday 06 October 2008 08:06:30 Shah, Hardik wrote:
> > Hi,
> >
> > > -----Original Message-----
> > > From: Mauro Carvalho Chehab [mailto:mchehab@xxxxxxxxxxxxx]
> > > Sent: Sunday, October 05, 2008 4:50 PM
> > > To: Shah, Hardik
> > > Cc: Hans Verkuil; video4linux-list@xxxxxxxxxx;
> > > linux-omap@xxxxxxxxxxxxxxx; linux-fbdev-
> > > devel@xxxxxxxxxxxxxxxxxxxxx
> > > Subject: Re: [PATCH] OMAP 2/3 V4L2 display driver on video planes
> > >
> > > On Fri, 3 Oct 2008 20:10:36 +0530
> > > "Shah, Hardik" <hardik.shah@xxxxxx> wrote:
> > >
> > >
> > >
> > > I don't like the idea of having private ioctls. This generally
> > > means that only a very restricted subset of userspace apps that are
> > > aware of the that API will work. This is really bad.
> > >
> > > So, I prefer to discuss the need for newer ioctls and add it into
> > > the standard whenever make some sense (ok, maybe you might have
> > > some ioctls that are really very specific for your app and that
> > > won't break userspace apps - I've acked with 2 private ioctls on
> > > uvc driver in the past due to that).
> >
> > [Shah, Hardik] Following are the custom IOCTLs used in the V4L2
> > display driver of DSS.
> >
> > 1.  VIDIOC_S/G_OMAP2_MIRROR: This ioctl is used to enable the
> > mirroring of the image. Hardware supports mirroring. As pointed out
> > by Hans it will be better to move it to VIDIOC_S_CTRL ioctl. we can
> > add the new control id for the mirroring.
> 
> HFLIP and VFLIP user controls already exist.
> 
> > 2.  VIDIOC_S/G_OMAP2_ROTATION: Rotation is used to enable the
> > rotation of the image. This also can be moved to the VIDIOC_S_CTRL
> > ioctl.  Need to add new control id for the rotation also.
> 
> A new standard user control can be added for this.
> 
> > 3.  VIDIOC_S/G_OMAP2_LINK: This feature is software provided. OMAP
> > DSS is having two video pipelines.  Using this feature user can link
> > the two video pipelines. This means the streaming of the video on one
> > pipeline will be linked to the other pipeline with the same
> > parameters as the original pipeline.  Same image can be streamed on
> > both the pipelines, one of the pipeline's output going to TV and
> > other one to LCD.  I believe this feature is very specific to OMAP,
> > and should remain as the custom ioctl.
> 
> I agree.
> 
> > 4.  VIDIOC_S/G_OMAP2_COLORKEY:  Color keying allows the pixels with
> > the defined color on the video pipelines to be replaced with the
> > pixels on the graphics pipelines.  I believe similar feature must be
> > available on almost all next generation of video hardware. We can add
> > new ioctl for this feature in V4L2 framework. I think VIDIOC_S_FBUF
> > ioctl is used for setting up the buffer parameters on per buffer
> > basis.  So IMHO this ioctl is not a natural fit for the above
> > functionality. Please provide your comments on same.
> 
> Do I understand correctly that if the color in the *video* streams
> matches the colorkey, then it is replaced by the color in the
> *framebuffer* (aka menu/overlay)? Usually it is the other way around:
> if the framebuffer (menu) has chromakey pixels, then those pixels are
> replaced by pixels from the video stream. That's what the current API
> does.
> 
> > 5. VIDIOC_S/G_OMAP2_BGCOLOR: This ioctl is used to set the Background
> > color on either TV or LCD. It takes two inputs, first is the output
> > device second is the color to be set. I think this can be added to
> > the standard ioctl list but is it ok to have the output device as one
> > of the parameters in the input structure? Instead we can set the
> > background color for the current output.
> 
> Setting the background color for the current output is the more logical
> choice. It would also be a nice addition for e.g. the ivtv driver where
> a similar functionality exists (currently unused).
> 
> I assume that background color refers to the part of the display not
> covered by a menu or video?
> 
> > 6. VIDIOC_OMAP2_COLORKEY_ENABLE/DISABLE.  This ioctl is used to
> > enable or the disable the color keying feature described above. This
> > can be added as the one of the control using VIDIOC_S_CTRL ioctl.
> 
> Depends on the answer to 4).
> 
> > 7.  VIDIOC_S/G_OMAP2_COLORCONV:  This is a hardware feature.  Video
> > pipelines of the DSS are capable of getting the buffer in the
> > YUV/UYVY format. But internally DSS operates on RGB format.  So
> > hardware has a capability to convert the YUV format to RGB format.
> > This is done using the color conversion matrix in the hardware.  It
> > accepts the structure as input which has 9 unsigned short variables
> > representing the coefficients for color conversion.  I think this
> > feature will also be present in many new devices. So we can have the
> > standard ioctl for this.
> 
> I think so too, it's pretty much a standard operation.
> 
> > 8.  VIDIOC_S_OMAP2_DEFCOLORCONV:  This ioctl just programs the
> > default color conversion matrix defined by the driver.  This we can
> > have as one of the controls using VIDIOC_S_CTRL ioctl.
> 
> I don't understand the need for this one. In what way does it differ
> from OMAP2_COLORCONV?
> 
> > Please let me know your view/thoughts on above custom ioctls added in
> > the driver.
> 
> My pleasure,
> 
> 	Hans
> 
> >
> > > So, if you are having several points where you're violating the
> > > rule, probably your code is very complex or you are using long
> > > names instead of short ones. On the fisrt case, try to break the
> > > complex stuff  into smaller and simpler static functions. The
> > > compiler will deal with those functions like inline, so this won't
> > > cost cpu cycles, but it will make easier for people to understand
> > > what you're doing.
> >
> > [Shah, Hardik] I will revisit the code and structure it properly.
> >
> > > Perhaps the better would be for you to have one public machine
> > > where you all can work and merge your work. I'm OK on pulling and
> > > seeing patches outside LinuxTV.
> > >
> > > > [Shah, Hardik] we are in process of coordinating this.
> > >
> > > One option in the future is to base your work on a git tree. I've
> > > changed a lot the proccess of submitting patches upstream, to avoid
> > > having to rebase my tree (Yet, I had to do two rebases during
> > > 2.6.27 cycle). If I can keep my tree without rebase, the developers
> > > may rely on it and start sending me git pull requests also. Let's
> > > see if I can do this for 2.6.28.
> > >
> > > I think we should start discussing using git trees as the reference
> > > for v4l/dvb development, and start moving developers tree to git.
> > > This would solve the issues with complex projects like OMAP, where
> > > you need to touch not only on V4L/DVB subsystem.
> > >
> > > This topic deserves some more discussion,
> >
Hi All,
I will be posting the version 2 of the DSS library and V4L2 display driver 
with almost all the comments from the community taken care of.
It will be series of 4 patches containing

OMAP 2/3 DSS Library
OMAP3 EVM TV encoder Driver.
New IOCTLS added to V4L2 Framework (Already posted on V4L2 mailing list)
OMAP 2/3 V4L2 Display driver on the Video planes of DSS hardware.

We are enhancing the DSS library.  This is the first post containing the
features like power management, video pipeline, Digital overlay manager,
clock management support.

Further plan is to add graphics pipeline, LCD overlay manager, RFBI, DSI,
support and frame buffer interface for graphics pipeline

Please let us know your comments on further enhancements of the DSS Library
and V4L2 display driver.

Thanks and Regards,
Hardik Shah
> >
> > > Cheers,
> > > Mauro.
> 
> 

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