Hi Niklas, On Thu, Mar 19, 2020 at 1:13 PM Niklas <niklas.soderlund@xxxxxxxxxxxx> wrote: > > Hi Prabhakar, > > Thanks for your work. > > On 2020-03-18 17:06:37 +0000, Lad Prabhakar wrote: > > Add support for MEDIA_BUS_FMT_SRGGB8_1X8 format in rcar-vin by setting > > format type to RAW8 in VNMC register and appropriately setting the > > bpp, bytesperline to enable V4L2_PIX_FMT_SRGGB8. > > > For RAW8 format data is transferred by 4-Byte unit so VnIS register is > > configured accordingly. > > > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx> > > --- > > drivers/media/platform/rcar-vin/rcar-core.c | 1 + > > drivers/media/platform/rcar-vin/rcar-dma.c | 11 ++++++++++- > > drivers/media/platform/rcar-vin/rcar-v4l2.c | 4 ++++ > > 3 files changed, 15 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/media/platform/rcar-vin/rcar-core.c b/drivers/media/platform/rcar-vin/rcar-core.c > > index 7440c8965d27..76daf2fe5bcd 100644 > > --- a/drivers/media/platform/rcar-vin/rcar-core.c > > +++ b/drivers/media/platform/rcar-vin/rcar-core.c > > @@ -469,6 +469,7 @@ static int rvin_parallel_subdevice_attach(struct rvin_dev *vin, > > case MEDIA_BUS_FMT_UYVY8_2X8: > > case MEDIA_BUS_FMT_UYVY10_2X10: > > case MEDIA_BUS_FMT_RGB888_1X24: > > + case MEDIA_BUS_FMT_SRGGB8_1X8: > > This is wrong RAW formats are only supported on the CSI-2 interface and > not the parallel one. This line shall be dropped. > sure will drop this. > > vin->mbus_code = code.code; > > vin_dbg(vin, "Found media bus format for %s: %d\n", > > subdev->name, vin->mbus_code); > > diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c b/drivers/media/platform/rcar-vin/rcar-dma.c > > index 1a30cd036371..ec7b49c0b281 100644 > > --- a/drivers/media/platform/rcar-vin/rcar-dma.c > > +++ b/drivers/media/platform/rcar-vin/rcar-dma.c > > @@ -85,6 +85,7 @@ > > #define VNMC_INF_YUV8_BT601 (1 << 16) > > #define VNMC_INF_YUV10_BT656 (2 << 16) > > #define VNMC_INF_YUV10_BT601 (3 << 16) > > +#define VNMC_INF_RAW8 (4 << 16) > > #define VNMC_INF_YUV16 (5 << 16) > > #define VNMC_INF_RGB888 (6 << 16) > > #define VNMC_VUP (1 << 10) > > @@ -587,13 +588,14 @@ void rvin_crop_scale_comp(struct rvin_dev *vin) > > rvin_write(vin, vin->crop.top, VNSLPRC_REG); > > rvin_write(vin, vin->crop.top + vin->crop.height - 1, VNELPRC_REG); > > > > - > > /* TODO: Add support for the UDS scaler. */ > > if (vin->info->model != RCAR_GEN3) > > rvin_crop_scale_comp_gen2(vin); > > > > fmt = rvin_format_from_pixel(vin, vin->format.pixelformat); > > stride = vin->format.bytesperline / fmt->bpp; > > + if (vin->format.pixelformat == V4L2_PIX_FMT_SRGGB8) > > + stride /= 2; > > I'm sorry this makes no sens to me. > > - The width of the image is number of pixels in the raw format. > - In memory each row is either is either RGRGRG... or GBGBGB... > - The pixel size is 1 byte per pixel. > - We calculate bytesperline as ALIGN(width, align) * bpp, where align is > how much we need to "adjust" the width to match the VNIS_REG reg > value. We do this in rvin_format_bytesperline(). > - We then remove bpp from bytesperline and we have a unit in pixels > which is our stride. > > I can't see why you need to cut the stride in half. In my view you > should add a check for V4L2_PIX_FMT_SRGGB8 in rvin_format_bytesperline() > and pick an alignment value that matches the restrictions. > > I might miss something, but then I wish to learn. > I have replied for this issue on v2 (https://lkml.org/lkml/2020/3/27/384) as the VIN processes RAW8 as 2 byte per pixel. > > rvin_write(vin, stride, VNIS_REG); > > } > > > > @@ -676,6 +678,9 @@ static int rvin_setup(struct rvin_dev *vin) > > > > input_is_yuv = true; > > break; > > + case MEDIA_BUS_FMT_SRGGB8_1X8: > > + vnmc |= VNMC_INF_RAW8; > > + break; > > Here and ... > > > default: > > break; > > } > > @@ -737,6 +742,9 @@ static int rvin_setup(struct rvin_dev *vin) > > case V4L2_PIX_FMT_ABGR32: > > dmr = VNDMR_A8BIT(vin->alpha) | VNDMR_EXRGB | VNDMR_DTMD_ARGB; > > break; > > + case V4L2_PIX_FMT_SRGGB8: > > + dmr = 0; > > + break; > > ... here we have a new problem, sorry for not thinking of it before. > > Up until now the VIN was capable to convert any of its supported input > mbus formats to any of it's supported output pixel formats. With the > addition of RAW formats this is no longer true. This new restriction > needs to be added to the driver. > > Luck has it we can fix ... > > > default: > > vin_err(vin, "Invalid pixelformat (0x%x)\n", > > vin->format.pixelformat); > > @@ -1110,6 +1118,7 @@ static int rvin_mc_validate_format(struct rvin_dev *vin, struct v4l2_subdev *sd, > > case MEDIA_BUS_FMT_UYVY8_2X8: > > case MEDIA_BUS_FMT_UYVY10_2X10: > > case MEDIA_BUS_FMT_RGB888_1X24: > > + case MEDIA_BUS_FMT_SRGGB8_1X8: > > vin->mbus_code = fmt.format.code; > > ... this here by changes this to > > switch (fmt.format.code) { > case MEDIA_BUS_FMT_YUYV8_1X16: > case MEDIA_BUS_FMT_UYVY8_1X16: > case MEDIA_BUS_FMT_UYVY8_2X8: > case MEDIA_BUS_FMT_UYVY10_2X10: > break; > case MEDIA_BUS_FMT_RGB888_1X24: > if (vin->format.pixelformat != V4L2_PIX_FMT_SRGGB8) > return -EPIPE: > break; > default: > return -EPIPE; > } > > vin->mbus_code = fmt.format.code; > Will fix it as above. Cheers, --Prabhakar > > break; > > default: > > diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c b/drivers/media/platform/rcar-vin/rcar-v4l2.c > > index 5151a3cd8a6e..ca542219e8ae 100644 > > --- a/drivers/media/platform/rcar-vin/rcar-v4l2.c > > +++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c > > @@ -66,6 +66,10 @@ static const struct rvin_video_format rvin_formats[] = { > > .fourcc = V4L2_PIX_FMT_ABGR32, > > .bpp = 4, > > }, > > + { > > + .fourcc = V4L2_PIX_FMT_SRGGB8, > > + .bpp = 1, > > + }, > > }; > > > > const struct rvin_video_format *rvin_format_from_pixel(struct rvin_dev *vin, > > -- > > 2.20.1 > > > > -- > Regards, > Niklas Söderlund
