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