Re: [PATCH] soc_camera: Add RGB666 & RGB888 formats

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

 



Hi Guennadi,

> From: Guennadi Liakhovetski <g.liakhovetski@xxxxxx>
> To: Phil Edworthy <phil.edworthy@xxxxxxxxxxx>, 
> Cc: Mauro Carvalho Chehab <mchehab@xxxxxxxxxx>, 
linux-media@xxxxxxxxxxxxxxx
> Date: 08/03/2013 13:30
> Subject: Re: [PATCH] soc_camera: Add RGB666 & RGB888 formats
> 
> Hi Phil
> 
> On Thu, 14 Feb 2013, Phil Edworthy wrote:
> 
> > Based on work done by Katsuya Matsubara.
> > 
> > Signed-off-by: Phil Edworthy <phil.edworthy@xxxxxxxxxxx>
> 
> Looks mostly good to me, but please also provide format descriptions for 

> Documentation/DocBook/media/v4l/pixfmt-packed-rgb.xml, also see a couple 

> of minor notes below

I had a look at the doc you pointed to, but it appears to only describe 
formats that can be accessed by userspace. I take it you meant 
Documentation/DocBook/media/v4l/subdev-formats.xml?

> > ---
> >  drivers/media/platform/soc_camera/soc_mediabus.c |   42 +++++++++
> +++++++++++++
> >  include/media/soc_camera.h                       |    6 +++-
> >  include/media/soc_mediabus.h                     |    3 ++
> >  include/uapi/linux/v4l2-mediabus.h               |    6 +++-
> >  4 files changed, 55 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/media/platform/soc_camera/soc_mediabus.c b/
> drivers/media/platform/soc_camera/soc_mediabus.c
> > index a397812..d8acfd3 100644
> > --- a/drivers/media/platform/soc_camera/soc_mediabus.c
> > +++ b/drivers/media/platform/soc_camera/soc_mediabus.c
> > @@ -97,6 +97,42 @@ static const struct soc_mbus_lookup mbus_fmt[] = {
> >        .layout         = SOC_MBUS_LAYOUT_PACKED,
> >     },
> >  }, {
> > +   .code = V4L2_MBUS_FMT_RGB666_1X18,
> > +   .fmt = {
> > +      .fourcc         = V4L2_PIX_FMT_RGB32,
> > +      .name         = "RGB666/32bpp",
> > +      .bits_per_sample   = 18,
> > +      .packing      = SOC_MBUS_PACKING_EXTEND32,
> > +      .order         = SOC_MBUS_ORDER_LE,
> > +   },
> > +}, {
> > +   .code = V4L2_MBUS_FMT_RGB888_1X24,
> > +   .fmt = {
> > +      .fourcc         = V4L2_PIX_FMT_RGB32,
> > +      .name         = "RGB888/32bpp",
> > +      .bits_per_sample   = 24,
> > +      .packing      = SOC_MBUS_PACKING_EXTEND32,
> > +      .order         = SOC_MBUS_ORDER_LE,
> > +   },
> > +}, {
> > +   .code = V4L2_MBUS_FMT_RGB888_2X12_BE,
> > +   .fmt = {
> > +      .fourcc         = V4L2_PIX_FMT_RGB32,
> > +      .name         = "RGB888/32bpp",
> > +      .bits_per_sample   = 12,
> > +      .packing      = SOC_MBUS_PACKING_EXTEND32,
> > +      .order         = SOC_MBUS_ORDER_BE,
> > +   },
> > +}, {
> > +   .code = V4L2_MBUS_FMT_RGB888_2X12_LE,
> > +   .fmt = {
> > +      .fourcc         = V4L2_PIX_FMT_RGB32,
> > +      .name         = "RGB888/32bpp",
> > +      .bits_per_sample   = 12,
> > +      .packing      = SOC_MBUS_PACKING_EXTEND32,
> > +      .order         = SOC_MBUS_ORDER_LE,
> > +   },
> > +}, {
> >     .code = V4L2_MBUS_FMT_SBGGR8_1X8,
> >     .fmt = {
> >        .fourcc         = V4L2_PIX_FMT_SBGGR8,
> > @@ -358,6 +394,10 @@ int soc_mbus_samples_per_pixel(const struct 
> soc_mbus_pixelfmt *mf,
> >        *numerator = 1;
> >        *denominator = 1;
> >        return 0;
> > +   case SOC_MBUS_PACKING_EXTEND32:
> > +      *numerator = 1;
> > +      *denominator = 1;
> > +      return 0;
> >     case SOC_MBUS_PACKING_2X8_PADHI:
> >     case SOC_MBUS_PACKING_2X8_PADLO:
> >        *numerator = 2;
> > @@ -395,6 +435,8 @@ s32 soc_mbus_bytes_per_line(u32 width, const 
> struct soc_mbus_pixelfmt *mf)
> >        return width * 3 / 2;
> >     case SOC_MBUS_PACKING_VARIABLE:
> >        return 0;
> > +   case SOC_MBUS_PACKING_EXTEND32:
> > +      return width * 4;
> >     }
> >     return -EINVAL;
> >  }
> > diff --git a/include/media/soc_camera.h b/include/media/soc_camera.h
> > index 6442edc..c820be2 100644
> > --- a/include/media/soc_camera.h
> > +++ b/include/media/soc_camera.h
> > @@ -231,10 +231,14 @@ struct soc_camera_sense {
> >  #define SOCAM_DATAWIDTH_10   SOCAM_DATAWIDTH(10)
> 
> Didn't you forget to define SOCAM_DATAWIDTH_12 here?
Yes, I forgot this.

> >  #define SOCAM_DATAWIDTH_15   SOCAM_DATAWIDTH(15)
> >  #define SOCAM_DATAWIDTH_16   SOCAM_DATAWIDTH(16)
> > +#define SOCAM_DATAWIDTH_18   SOCAM_DATAWIDTH(18)
> > +#define SOCAM_DATAWIDTH_24   SOCAM_DATAWIDTH(24)
> > 
> >  #define SOCAM_DATAWIDTH_MASK (SOCAM_DATAWIDTH_4 | SOCAM_DATAWIDTH_8 | 
\
> >                 SOCAM_DATAWIDTH_9 | SOCAM_DATAWIDTH_10 | \
> > -               SOCAM_DATAWIDTH_15 | SOCAM_DATAWIDTH_16)
> > +               SOCAM_DATAWIDTH_12 | SOCAM_DATAWIDTH_15 | \
> > +               SOCAM_DATAWIDTH_16 | SOCAM_DATAWIDTH_18 | \
> > +               SOCAM_DATAWIDTH_24)
> > 
> >  static inline void soc_camera_limit_side(int *start, int *length,
> >        unsigned int start_min,
> > diff --git a/include/media/soc_mediabus.h 
b/include/media/soc_mediabus.h
> > index 0dc6f46..eea98d1 100644
> > --- a/include/media/soc_mediabus.h
> > +++ b/include/media/soc_mediabus.h
> > @@ -26,6 +26,8 @@
> >   * @SOC_MBUS_PACKING_VARIABLE:   compressed formats with variable 
packing
> >   * @SOC_MBUS_PACKING_1_5X8:   used for packed YUV 4:2:0 formats, 
where 4
> >   *            pixels occupy 6 bytes in RAM
> > + * @SOC_MBUS_PACKING_EXTEND32:  sample width (e.g., 24 bits) has 
> to be extended
> 
> Please, use a TAB above
Of course.

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


[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux