Hi Hans
On Sun, 14 Mar 2010, Hans Verkuil wrote:
> Hi Guennadi,
>
> Here is a quick review. It looks good, just a few small points.
Thanks for the review! To most points - right, will update. The ones, that
I have more questions about:
> On Thursday 11 March 2010 11:24:42 Guennadi Liakhovetski wrote:
> > A number of SuperH SoCs, including sh7724, include a Video Output Unit. This
> > patch adds a video (V4L2) output driver for it. The driver uses v4l2-subdev and
> > mediabus APIs to interface to TV encoders.
> >
> > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@xxxxxx>
> > ---
> >
> > Tested on ms7724se
> >
> > drivers/media/video/Kconfig | 7 +
> > drivers/media/video/Makefile | 2 +
> > drivers/media/video/sh_vou.c | 1437 ++++++++++++++++++++++++++++++++++++++++++
> > include/media/sh_vou.h | 35 +
> > 4 files changed, 1481 insertions(+), 0 deletions(-)
> > create mode 100644 drivers/media/video/sh_vou.c
> > create mode 100644 include/media/sh_vou.h
> >
> > diff --git a/drivers/media/video/Kconfig b/drivers/media/video/Kconfig
> > index 64682bf..be6d016 100644
> > --- a/drivers/media/video/Kconfig
> > +++ b/drivers/media/video/Kconfig
[snip]
> > +static int sh_vou_g_fmt_vid_ovrl(struct file *file, void *priv,
> > + struct v4l2_format *fmt)
> > +{
> > + /* This is needed for gstreamer, even if not used... */
> > + return 0;
> > +}
>
> Shouldn't this return -EINVAL if there is no overlay support?
In fact I would just drop this methos altogether, but gstreamer needs it
_and_ it shouldn't return an error. In fact, I think, we can persuade
gstreamer guys to drop this restriction, if we really think this is
irrelevant. For some reason their v4l2sink is somewhat overlay-centric,
but I think we can discuss this with them.
> > +enum sh_vou_bus_fmt {
> > + SH_VOU_BUS_NTSC_16BIT = 0,
> > + SH_VOU_BUS_NTSC_8BIT = 1,
> > + SH_VOU_BUS_NTSC_8BIT_REC656 = 3,
> > + SH_VOU_BUS_PAL_8BIT = 5,
>
> Rather than NTSC and PAL it might be better to talk about 50 vs 60 Hz.
Don't know, this is how these modes are called in the datasheet, so... Do
you think it's better to call them "correctly" or as in the datasheet?
Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
--
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
