On Mon, 3 Aug 2009, Eric Miao wrote:

> Marek Vasut wrote:
> > Hi!
> > 
> > Eric, would you mind applying ?
> > 
> > From 4dcbff010e996f4c6e5761b3c19f5d863ab51b39 Mon Sep 17 00:00:00 2001
> > From: Marek Vasut <marek.va...@gmail.com>
> > Date: Mon, 3 Aug 2009 10:27:57 +0200
> > Subject: [PATCH] Add RGB555X and RGB565X formats to pxa-camera
> > 
> > Those formats are requiered on widely used OmniVision OV96xx cameras.
> > Those formats are nothing more then endian-swapped RGB555 and RGB565.
> > 
> > Signed-off-by: Marek Vasut <marek.va...@gmail.com>
> 
> Acked-by: Eric Miao <eric.y.m...@gmail.com>
> 
> Guennadi,
> 
> Would be better if this gets merged by you, thanks.

Indeed it would, and I do have a couple of questions to this and related 
patches:

1. Marek, you're saying, you need these formats for the OV96xx camera. Yre 
you using the patch from Stefan Herbrechtsmeier to support ov96xx or some 
other?

2. Mike, while reviewing this patch I came across code in 
pxa_camera_setup_cicr(), introduced by your earlier patch:

        case V4L2_PIX_FMT_RGB555:
                cicr1 |= CICR1_RGB_BPP_VAL(1) | CICR1_RGBT_CONV_VAL(2) |
                        CICR1_TBIT | CICR1_COLOR_SP_VAL(1);
                break;

Why are you enabling the RGB to RGBT conversion here unconditionally? 
Generally, what are the advantages of configuring CICR1 for a specific RGB 
format compared to using just a raw capture? Do I understand it right, 
that ATM we are not using any of those features?

Thanks
Guennadi

> 
> > ---
> >  drivers/media/video/pxa_camera.c |    4 ++++
> >  1 files changed, 4 insertions(+), 0 deletions(-)
> > 
> > diff --git a/drivers/media/video/pxa_camera.c 
> > b/drivers/media/video/pxa_camera.c
> > index 46e0d8a..de0fc8a 100644
> > --- a/drivers/media/video/pxa_camera.c
> > +++ b/drivers/media/video/pxa_camera.c
> > @@ -1222,6 +1222,8 @@ static int required_buswidth(const struct 
> > soc_camera_data_format *fmt)
> >     case V4L2_PIX_FMT_YVYU:
> >     case V4L2_PIX_FMT_RGB565:
> >     case V4L2_PIX_FMT_RGB555:
> > +   case V4L2_PIX_FMT_RGB565X:
> > +   case V4L2_PIX_FMT_RGB555X:
> >             return 8;
> >     default:
> >             return fmt->depth;
> > @@ -1260,6 +1262,8 @@ static int pxa_camera_get_formats(struct 
> > soc_camera_device *icd, int idx,
> >     case V4L2_PIX_FMT_YVYU:
> >     case V4L2_PIX_FMT_RGB565:
> >     case V4L2_PIX_FMT_RGB555:
> > +   case V4L2_PIX_FMT_RGB565X:
> > +   case V4L2_PIX_FMT_RGB555X:
> >             formats++;
> >             if (xlate) {
> >                     xlate->host_fmt = icd->formats + idx;
> 

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

Reply via email to