On Wednesday 27 June 2012 13:54:30 Laurent Pinchart wrote:
> Hi Sakari,
>
> Thanks for the review.
>
> On Wednesday 27 June 2012 14:07:51 Sakari Ailus wrote:
> > Laurent Pinchart wrote:
> > > The bpp field currently stores the sample width and is aligned to the
> > > next multiple of 8 bits when computing data size in memory. This won't
> > > work anymore for YUYV8_2X8 formats. Split the bpp field into a sample
> > > width and a bits per pixel value.
> > >
> > > Signed-off-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> >
> > ...
> >
> > > diff --git a/drivers/media/video/omap3isp/ispvideo.h
> > > b/drivers/media/video/omap3isp/ispvideo.h index 5acc909..f8092cc 100644
> > > --- a/drivers/media/video/omap3isp/ispvideo.h
> > > +++ b/drivers/media/video/omap3isp/ispvideo.h
> > > @@ -51,7 +51,8 @@ struct v4l2_pix_format;
> > >
> > > * @flavor: V4L2 media bus format code for the same pixel layout but
> > > * shifted to be 8 bits per pixel. =0 if format is not shiftable.
> > > * @pixelformat: V4L2 pixel format FCC identifier
> > >
> > > - * @bpp: Bits per pixel
> > > + * @width: Data bus width
> > > + * @bpp: Bits per pixel (when stored in memory)
> >
> > Would it make sense to use bytes rather than bits?
>
> I'll change that.
>
> > Also width isn't really the width of the data bus on serial busses, is it?
> > How about busses that transfer pixels 8 bits at the time?
>
> I could change the comment to "bits per pixel (when transferred on a bus)",
> would that be better ?
s/pixel/sample/
> > You can also stop using ALIGN() in isp_video_mbus_to_pix() (in ispvideo.c)
> > as the ISP will always write complete bytes.
>
> Indeed.
>
> > I might even switch the meaning between width and bpp but this is up to
> > you.
> I got used to width/bpp as defined by this patch, so that would be confusing
> :-)
> :
> > > */
> > >
> > > struct isp_format_info {
> > >
> > > enum v4l2_mbus_pixelcode code;
> > >
> > > @@ -59,6 +60,7 @@ struct isp_format_info {
> > >
> > > enum v4l2_mbus_pixelcode uncompressed;
> > > enum v4l2_mbus_pixelcode flavor;
> > > u32 pixelformat;
> > >
> > > + unsigned int width;
> > >
> > > unsigned int bpp;
> > >
> > > };
> > >
> > > @@ -106,7 +108,7 @@ struct isp_pipeline {
> > >
> > > struct v4l2_fract max_timeperframe;
> > > struct v4l2_subdev *external;
> > > unsigned int external_rate;
> > >
> > > - unsigned int external_bpp;
> > > + unsigned int external_width;
> > >
> > > };
> > >
> > > #define to_isp_pipeline(__e) \
--
Regards,
Laurent Pinchart
--
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
