[v2,2/3] media: rcar-vin: Add support for MEDIA_BUS_FMT_SRGGB8_1X8 format
diff mbox series

Message ID 1583838364-12932-3-git-send-email-prabhakar.mahadev-lad.rj@bp.renesas.com
State New
Delegated to: Kieran Bingham
Headers show
Series
  • media: rcar-vin: Enable MEDIA_BUS_FMT_SRGGB8_1X8 format and support for matching fwnode against endpoints/nodes
Related show

Commit Message

Lad Prabhakar March 10, 2020, 11:06 a.m. UTC
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.

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
 drivers/media/platform/rcar-vin/rcar-core.c |  1 +
 drivers/media/platform/rcar-vin/rcar-dma.c  |  9 ++++++++-
 drivers/media/platform/rcar-vin/rcar-v4l2.c | 13 ++++++++++++-
 3 files changed, 21 insertions(+), 2 deletions(-)

Comments

Niklas March 10, 2020, 12:46 p.m. UTC | #1
Hi Lad,

Thanks for your work.

On 2020-03-10 11:06:03 +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.
> 
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> ---
>  drivers/media/platform/rcar-vin/rcar-core.c |  1 +
>  drivers/media/platform/rcar-vin/rcar-dma.c  |  9 ++++++++-
>  drivers/media/platform/rcar-vin/rcar-v4l2.c | 13 ++++++++++++-
>  3 files changed, 21 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/platform/rcar-vin/rcar-core.c b/drivers/media/platform/rcar-vin/rcar-core.c
> index 7440c89..76daf2f 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:
>  			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 1a30cd0..1c1fafa 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,7 +588,6 @@ 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);
> @@ -676,6 +676,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;
>  	default:
>  		break;
>  	}
> @@ -737,6 +740,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;
>  	default:
>  		vin_err(vin, "Invalid pixelformat (0x%x)\n",
>  			vin->format.pixelformat);
> @@ -1110,6 +1116,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;
>  		break;
>  	default:
> diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c b/drivers/media/platform/rcar-vin/rcar-v4l2.c
> index 5151a3c..4698099 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			= 2,

This does not look right, is not bytes-per-pixel 1 for a SRGGB8?

> +	},
>  };
>  
>  const struct rvin_video_format *rvin_format_from_pixel(struct rvin_dev *vin,
> @@ -102,6 +106,7 @@ static u32 rvin_format_bytesperline(struct rvin_dev *vin,
>  {
>  	const struct rvin_video_format *fmt;
>  	u32 align;
> +	u8 div;
>  
>  	fmt = rvin_format_from_pixel(vin, pix->pixelformat);
>  
> @@ -112,16 +117,22 @@ static u32 rvin_format_bytesperline(struct rvin_dev *vin,
>  	case V4L2_PIX_FMT_NV12:
>  	case V4L2_PIX_FMT_NV16:
>  		align = 0x20;
> +		div = 1;
> +		break;
> +	case V4L2_PIX_FMT_SRGGB8:
> +		align = 0x10;
> +		div = 2;

Yes this does not look right at all, I think you should set bpp to 1 and 
drop the div handling here.

>  		break;
>  	default:
>  		align = 0x10;
> +		div = 1;
>  		break;
>  	}
>  
>  	if (V4L2_FIELD_IS_SEQUENTIAL(pix->field))
>  		align = 0x80;
>  
> -	return ALIGN(pix->width, align) * fmt->bpp;
> +	return ALIGN(pix->width / div, align) * fmt->bpp;
>  }
>  
>  static u32 rvin_format_sizeimage(struct v4l2_pix_format *pix)
> -- 
> 2.7.4
>
Lad Prabhakar March 10, 2020, 1:42 p.m. UTC | #2
Hi Niklas,

Thank for the review.

> -----Original Message-----
> From: Niklas <niklas.soderlund@ragnatech.se>
> Sent: 10 March 2020 12:46
> To: Prabhakar Mahadev Lad <prabhakar.mahadev-lad.rj@bp.renesas.com>
> Cc: Mauro Carvalho Chehab <mchehab@kernel.org>; linux-
> media@vger.kernel.org; linux-renesas-soc@vger.kernel.org; linux-
> kernel@vger.kernel.org; Lad Prabhakar <prabhakar.csengg@gmail.com>
> Subject: Re: [PATCH v2 2/3] media: rcar-vin: Add support for
> MEDIA_BUS_FMT_SRGGB8_1X8 format
>
> Hi Lad,
>
> Thanks for your work.
>
> On 2020-03-10 11:06:03 +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.
> >
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-
> lad.rj@bp.renesas.com>
> > ---
> >  drivers/media/platform/rcar-vin/rcar-core.c |  1 +
> > drivers/media/platform/rcar-vin/rcar-dma.c  |  9 ++++++++-
> > drivers/media/platform/rcar-vin/rcar-v4l2.c | 13 ++++++++++++-
> >  3 files changed, 21 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/media/platform/rcar-vin/rcar-core.c
> > b/drivers/media/platform/rcar-vin/rcar-core.c
> > index 7440c89..76daf2f 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:
> >  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 1a30cd0..1c1fafa 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,7 +588,6 @@ 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);
> > @@ -676,6 +676,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;
> >  default:
> >  break;
> >  }
> > @@ -737,6 +740,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;
> >  default:
> >  vin_err(vin, "Invalid pixelformat (0x%x)\n",
> >  vin->format.pixelformat);
> > @@ -1110,6 +1116,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;
> >  break;
> >  default:
> > diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c
> > b/drivers/media/platform/rcar-vin/rcar-v4l2.c
> > index 5151a3c..4698099 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= 2,
>
> This does not look right, is not bytes-per-pixel 1 for a SRGGB8?
>
I guessed the bpp's were picked from VnIS table as I result I did the same.

> > +},
> >  };
> >
> >  const struct rvin_video_format *rvin_format_from_pixel(struct
> > rvin_dev *vin, @@ -102,6 +106,7 @@ static u32
> > rvin_format_bytesperline(struct rvin_dev *vin,  {
> >  const struct rvin_video_format *fmt;
> >  u32 align;
> > +u8 div;
> >
> >  fmt = rvin_format_from_pixel(vin, pix->pixelformat);
> >
> > @@ -112,16 +117,22 @@ static u32 rvin_format_bytesperline(struct
> rvin_dev *vin,
> >  case V4L2_PIX_FMT_NV12:
> >  case V4L2_PIX_FMT_NV16:
> >  align = 0x20;
> > +div = 1;
> > +break;
> > +case V4L2_PIX_FMT_SRGGB8:
> > +align = 0x10;
> > +div = 2;
>
> Yes this does not look right at all, I think you should set bpp to 1 and drop the
> div handling here.
>
If I set bpp as 1 and drop the div VNIS_REG will be wrongly configured in
rvin_crop_scale_comp() and the image captured will be wrong.

For example for 640x480:

With the current patch bpp = 2:
bytesperline = 640
image size = 307200
stride = 320

And with bpp = 1 and div removed
bytesperline = 640
image size = 307200
stride = 640

Cheers,
--Prabhakar

> >  break;
> >  default:
> >  align = 0x10;
> > +div = 1;
> >  break;
> >  }
> >
> >  if (V4L2_FIELD_IS_SEQUENTIAL(pix->field))
> >  align = 0x80;
> >
> > -return ALIGN(pix->width, align) * fmt->bpp;
> > +return ALIGN(pix->width / div, align) * fmt->bpp;
> >  }
> >
> >  static u32 rvin_format_sizeimage(struct v4l2_pix_format *pix)
> > --
> > 2.7.4
> >
>
> --
> Regards,
> Niklas Söderlund


Renesas Electronics Europe GmbH, Geschaeftsfuehrer/President: Carsten Jauch, Sitz der Gesellschaft/Registered office: Duesseldorf, Arcadiastrasse 10, 40472 Duesseldorf, Germany, Handelsregister/Commercial Register: Duesseldorf, HRB 3708 USt-IDNr./Tax identification no.: DE 119353406 WEEE-Reg.-Nr./WEEE reg. no.: DE 14978647
Niklas March 10, 2020, 2:06 p.m. UTC | #3
Hi Lad,

On 2020-03-10 13:42:20 +0000, Prabhakar Mahadev Lad wrote:
> Hi Niklas,
> 
> Thank for the review.
> 
> > -----Original Message-----
> > From: Niklas <niklas.soderlund@ragnatech.se>
> > Sent: 10 March 2020 12:46
> > To: Prabhakar Mahadev Lad <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > Cc: Mauro Carvalho Chehab <mchehab@kernel.org>; linux-
> > media@vger.kernel.org; linux-renesas-soc@vger.kernel.org; linux-
> > kernel@vger.kernel.org; Lad Prabhakar <prabhakar.csengg@gmail.com>
> > Subject: Re: [PATCH v2 2/3] media: rcar-vin: Add support for
> > MEDIA_BUS_FMT_SRGGB8_1X8 format
> >
> > Hi Lad,
> >
> > Thanks for your work.
> >
> > On 2020-03-10 11:06:03 +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.
> > >
> > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-
> > lad.rj@bp.renesas.com>
> > > ---
> > >  drivers/media/platform/rcar-vin/rcar-core.c |  1 +
> > > drivers/media/platform/rcar-vin/rcar-dma.c  |  9 ++++++++-
> > > drivers/media/platform/rcar-vin/rcar-v4l2.c | 13 ++++++++++++-
> > >  3 files changed, 21 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/media/platform/rcar-vin/rcar-core.c
> > > b/drivers/media/platform/rcar-vin/rcar-core.c
> > > index 7440c89..76daf2f 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:
> > >  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 1a30cd0..1c1fafa 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,7 +588,6 @@ 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);
> > > @@ -676,6 +676,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;
> > >  default:
> > >  break;
> > >  }
> > > @@ -737,6 +740,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;
> > >  default:
> > >  vin_err(vin, "Invalid pixelformat (0x%x)\n",
> > >  vin->format.pixelformat);
> > > @@ -1110,6 +1116,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;
> > >  break;
> > >  default:
> > > diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c
> > > b/drivers/media/platform/rcar-vin/rcar-v4l2.c
> > > index 5151a3c..4698099 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= 2,
> >
> > This does not look right, is not bytes-per-pixel 1 for a SRGGB8?
> >
> I guessed the bpp's were picked from VnIS table as I result I did the same.
> 
> > > +},
> > >  };
> > >
> > >  const struct rvin_video_format *rvin_format_from_pixel(struct
> > > rvin_dev *vin, @@ -102,6 +106,7 @@ static u32
> > > rvin_format_bytesperline(struct rvin_dev *vin,  {
> > >  const struct rvin_video_format *fmt;
> > >  u32 align;
> > > +u8 div;
> > >
> > >  fmt = rvin_format_from_pixel(vin, pix->pixelformat);
> > >
> > > @@ -112,16 +117,22 @@ static u32 rvin_format_bytesperline(struct
> > rvin_dev *vin,
> > >  case V4L2_PIX_FMT_NV12:
> > >  case V4L2_PIX_FMT_NV16:
> > >  align = 0x20;
> > > +div = 1;
> > > +break;
> > > +case V4L2_PIX_FMT_SRGGB8:
> > > +align = 0x10;
> > > +div = 2;
> >
> > Yes this does not look right at all, I think you should set bpp to 1 and drop the
> > div handling here.
> >
> If I set bpp as 1 and drop the div VNIS_REG will be wrongly configured in
> rvin_crop_scale_comp() and the image captured will be wrong.
> 
> For example for 640x480:
> 
> With the current patch bpp = 2:
> bytesperline = 640

This is wrong, if we have a line of 640 pixels and 2 bytes per pixel 
then bytesperline must be at least 1280 bytes right?

> image size = 307200
> stride = 320

But this is incorrect, the VNIS_REG shall be at least the number of 
pixels in a line (EPPrC - SPPrC -> 640 - 0 = 640). Then we need to align 
it to the pixel unit (16, 32, 64, 128) depending on the output pixel 
format.

This usually result in a stride that is larger then the line length.  
Thus you need a test application that knows the difference between width 
* bpp and bytesperline. I use qv4l2 without opengl support when I do quick 
tests and it does not support this hence I get a incorrect visual view 
of the stream when testing.

How does the image capture fail with bpp = 1?

> 
> And with bpp = 1 and div removed
> bytesperline = 640
> image size = 307200
> stride = 640


> 
> Cheers,
> --Prabhakar
> 
> > >  break;
> > >  default:
> > >  align = 0x10;
> > > +div = 1;
> > >  break;
> > >  }
> > >
> > >  if (V4L2_FIELD_IS_SEQUENTIAL(pix->field))
> > >  align = 0x80;
> > >
> > > -return ALIGN(pix->width, align) * fmt->bpp;
> > > +return ALIGN(pix->width / div, align) * fmt->bpp;
> > >  }
> > >
> > >  static u32 rvin_format_sizeimage(struct v4l2_pix_format *pix)
> > > --
> > > 2.7.4
> > >
> >
> > --
> > Regards,
> > Niklas Söderlund
> 
> 
> Renesas Electronics Europe GmbH, Geschaeftsfuehrer/President: Carsten Jauch, Sitz der Gesellschaft/Registered office: Duesseldorf, Arcadiastrasse 10, 40472 Duesseldorf, Germany, Handelsregister/Commercial Register: Duesseldorf, HRB 3708 USt-IDNr./Tax identification no.: DE 119353406 WEEE-Reg.-Nr./WEEE reg. no.: DE 14978647
Niklas March 30, 2020, 12:07 p.m. UTC | #4
Hi Lad,

Thanks for your reply.

On 2020-03-27 12:59:52 +0000, Lad, Prabhakar wrote:
> Hi Niklas,
> 
> On Thu, Mar 19, 2020 at 3:03 PM Niklas <niklas.soderlund@ragnatech.se> wrote:
> >
> > Hi Prabhakar,
> >
> > Thanks for the sample files, sorry I did not have time before now to
> > look at them. After doing so I believe I know what is wrong, see bellow.
> >
> > On 2020-03-15 19:35:58 +0000, Lad, Prabhakar wrote:
> > > Hi Niklas,
> > >
> > > On Tue, Mar 10, 2020 at 2:06 PM Niklas <niklas.soderlund@ragnatech.se> wrote:
> > > >
> > > > Hi Lad,
> > > >
> > > > On 2020-03-10 13:42:20 +0000, Prabhakar Mahadev Lad wrote:
> > > > > Hi Niklas,
> > > > >
> > > > > Thank for the review.
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: Niklas <niklas.soderlund@ragnatech.se>
> > > > > > Sent: 10 March 2020 12:46
> > > > > > To: Prabhakar Mahadev Lad <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > > > > Cc: Mauro Carvalho Chehab <mchehab@kernel.org>; linux-
> > > > > > media@vger.kernel.org; linux-renesas-soc@vger.kernel.org; linux-
> > > > > > kernel@vger.kernel.org; Lad Prabhakar <prabhakar.csengg@gmail.com>
> > > > > > Subject: Re: [PATCH v2 2/3] media: rcar-vin: Add support for
> > > > > > MEDIA_BUS_FMT_SRGGB8_1X8 format
> > > > > >
> > > > > > Hi Lad,
> > > > > >
> > > > > > Thanks for your work.
> > > > > >
> > > > > > On 2020-03-10 11:06:03 +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.
> > > > > > >
> > > > > > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-
> > > > > > lad.rj@bp.renesas.com>
> > > > > > > ---
> > > > > > >  drivers/media/platform/rcar-vin/rcar-core.c |  1 +
> > > > > > > drivers/media/platform/rcar-vin/rcar-dma.c  |  9 ++++++++-
> > > > > > > drivers/media/platform/rcar-vin/rcar-v4l2.c | 13 ++++++++++++-
> > > > > > >  3 files changed, 21 insertions(+), 2 deletions(-)
> > > > > > >
> > > > > > > diff --git a/drivers/media/platform/rcar-vin/rcar-core.c
> > > > > > > b/drivers/media/platform/rcar-vin/rcar-core.c
> > > > > > > index 7440c89..76daf2f 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:
> > > > > > >  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 1a30cd0..1c1fafa 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,7 +588,6 @@ 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);
> > > > > > > @@ -676,6 +676,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;
> > > > > > >  default:
> > > > > > >  break;
> > > > > > >  }
> > > > > > > @@ -737,6 +740,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;
> > > > > > >  default:
> > > > > > >  vin_err(vin, "Invalid pixelformat (0x%x)\n",
> > > > > > >  vin->format.pixelformat);
> > > > > > > @@ -1110,6 +1116,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;
> > > > > > >  break;
> > > > > > >  default:
> > > > > > > diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c
> > > > > > > b/drivers/media/platform/rcar-vin/rcar-v4l2.c
> > > > > > > index 5151a3c..4698099 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= 2,
> > > > > >
> > > > > > This does not look right, is not bytes-per-pixel 1 for a SRGGB8?
> > > > > >
> > > > > I guessed the bpp's were picked from VnIS table as I result I did the same.
> > > > >
> > > > > > > +},
> > > > > > >  };
> > > > > > >
> > > > > > >  const struct rvin_video_format *rvin_format_from_pixel(struct
> > > > > > > rvin_dev *vin, @@ -102,6 +106,7 @@ static u32
> > > > > > > rvin_format_bytesperline(struct rvin_dev *vin,  {
> > > > > > >  const struct rvin_video_format *fmt;
> > > > > > >  u32 align;
> > > > > > > +u8 div;
> > > > > > >
> > > > > > >  fmt = rvin_format_from_pixel(vin, pix->pixelformat);
> > > > > > >
> > > > > > > @@ -112,16 +117,22 @@ static u32 rvin_format_bytesperline(struct
> > > > > > rvin_dev *vin,
> > > > > > >  case V4L2_PIX_FMT_NV12:
> > > > > > >  case V4L2_PIX_FMT_NV16:
> > > > > > >  align = 0x20;
> > > > > > > +div = 1;
> > > > > > > +break;
> > > > > > > +case V4L2_PIX_FMT_SRGGB8:
> > > > > > > +align = 0x10;
> > > > > > > +div = 2;
> > > > > >
> > > > > > Yes this does not look right at all, I think you should set bpp to 1 and drop the
> > > > > > div handling here.
> > > > > >
> > > > > If I set bpp as 1 and drop the div VNIS_REG will be wrongly configured in
> > > > > rvin_crop_scale_comp() and the image captured will be wrong.
> > > > >
> > > > > For example for 640x480:
> > > > >
> > > > > With the current patch bpp = 2:
> > > > > bytesperline = 640
> > > >
> > > > This is wrong, if we have a line of 640 pixels and 2 bytes per pixel
> > > > then bytesperline must be at least 1280 bytes right?
> > > >
> > > > > image size = 307200
> > > > > stride = 320
> > > >
> > > > But this is incorrect, the VNIS_REG shall be at least the number of
> > > > pixels in a line (EPPrC - SPPrC -> 640 - 0 = 640). Then we need to align
> > > > it to the pixel unit (16, 32, 64, 128) depending on the output pixel
> > > > format.
> > > >
> > > > This usually result in a stride that is larger then the line length.
> > > > Thus you need a test application that knows the difference between width
> > > > * bpp and bytesperline. I use qv4l2 without opengl support when I do quick
> > > > tests and it does not support this hence I get a incorrect visual view
> > > > of the stream when testing.
> > > >
> > > > How does the image capture fail with bpp = 1?
> > > >
> > > Attached is the captured bayer images 640x480 with bpp set to 1, for
> > > file1bppstridediv1.raw
> > > VNIS_REG stride set to 640 and for file file1bppstridediv2.raw
> > > VNIS_REF stride  set to (640 * 1) / 2.
> > > When the file1bppstridediv1.raw image is converted to png colors are incorrect
> > > but whereas file1bppstridediv2.raw converted to png the picture is clear.
> > >
> > > Also while doing a loop-back to fbdevsink with the below pipeline:
> > > gst-launch-1.0 -vvv v4l2src device=/dev/video0 io-mode=dmabuf ! 'video/x-bayer,
> > > format=rggb,width=640,height=480,framerate=30/1' ! queue ! bayer2rgb !
> > > videoconvert
> > > ! fbdevsink sync=false
> > >
> > > width = 640 height = 480 bpp = 1, bytesperline = 640 stride = 320
> > > works correctly
> > > width = 640 height = 480 bpp = 1, bytesperline = 640 stride = 640
> > > image displayed is incorrect
> >
> > It's very unlogical to have a stride that is less then the width, which
> > got me interested why the second one gave you better results. I wrote a
> > small python hack which converts the raw SRGGB8 to PNG without any
> > debyaer, just rows of RGRGRG and BGBGBG.
> >
> Finally I have some information from the hardware team, the VIN process RAW8
> in 2 pixel units as a result the stride for RAW8 needs to be
> configured as bytesperline/2.

Interesting, that is not how I have interpreted the datasheet. But 
rereading it now after our discussion I see how it could be so. I will 
dig into it during the week and see if I get make it all click in my 
head. Thanks for pointing this out.

> 
> The python script which you attached doesn't seem to do the right
> conversion. I discovered
> that Shotwell Viewer on Ubuntu can open raw files. I also confirmed
> this by bayer2rg [1]

Oops, you are right. My bad sorry for sending you down that path.

> 
> # ./bayer2rgb --input=file1bppstridediv1.raw --output=file1.tiff
> --width=640 --height=480 --bpp=8 --first=RGGB --method=BILINEAR --tiff
> # ./bayer2rgb --input=file1bppstridediv2.raw --output=file2.tiff
> --width=640 --height=480 --bpp=8 --first=RGGB --method=BILINEAR --tiff
> 
> # convert file1.tiff file1bppstridediv1.png
> # convert file2.tiff file1bppstridediv2.png
> 
> Attached are the png images for reference.
> 
> > Looking at the output of that seems your sensor is not sending frames of
> > 640x480 but 480x640. Both the raw files you sent have holes in them.
> > The first line is always 480 pixels of data and then there are sections
> > of no data, followed by good data. Some rows are chopped and some have
> > their 480 bytes of good data on the "left" and some on the "right" side
> > of the frame.
> >
> I can confirm the sensor is sending 640x480 as the support for same
> was added recently in IMX219 driver
> and  was  was tested on raspi by the maintainer.
> 
> [1] https://github.com/jdthomas/bayer2rgb
> 
> Cheers,
> --Prabhakar
> 
> > So for rcar-vin I think the following settings are what you want,
> >
> >     width = 480 height = 640 bpp = 1, bytesperline = 480* stride = 480
> >     * = I have not checked if this fits with alignment for VNIS
> >
> > I have attached the python hack and the two generated png files from
> > your raw files so you can play with them yourself.
> >
> > >
> > > Cheers,
> > > --Prabhakar
> > >
> > > > >
> > > > > And with bpp = 1 and div removed
> > > > > bytesperline = 640
> > > > > image size = 307200
> > > > > stride = 640
> > > >
> > > >
> > > > >
> > > > > Cheers,
> > > > > --Prabhakar
> > > > >
> > > > > > >  break;
> > > > > > >  default:
> > > > > > >  align = 0x10;
> > > > > > > +div = 1;
> > > > > > >  break;
> > > > > > >  }
> > > > > > >
> > > > > > >  if (V4L2_FIELD_IS_SEQUENTIAL(pix->field))
> > > > > > >  align = 0x80;
> > > > > > >
> > > > > > > -return ALIGN(pix->width, align) * fmt->bpp;
> > > > > > > +return ALIGN(pix->width / div, align) * fmt->bpp;
> > > > > > >  }
> > > > > > >
> > > > > > >  static u32 rvin_format_sizeimage(struct v4l2_pix_format *pix)
> > > > > > > --
> > > > > > > 2.7.4
> > > > > > >
> > > > > >
> > > > > > --
> > > > > > Regards,
> > > > > > Niklas Söderlund
> > > > >
> > > > >
> > > > > Renesas Electronics Europe GmbH, Geschaeftsfuehrer/President: Carsten Jauch, Sitz der Gesellschaft/Registered office: Duesseldorf, Arcadiastrasse 10, 40472 Duesseldorf, Germany, Handelsregister/Commercial Register: Duesseldorf, HRB 3708 USt-IDNr./Tax identification no.: DE 119353406 WEEE-Reg.-Nr./WEEE reg. no.: DE 14978647
> > > >
> > > > --
> > > > Regards,
> > > > Niklas Söderlund
> >
> >
> >
> >
> > --
> > Regards,
> > Niklas Söderlund
Lad, Prabhakar March 30, 2020, 1:13 p.m. UTC | #5
Hi Niklas,

On Mon, Mar 30, 2020 at 1:07 PM Niklas <niklas.soderlund@ragnatech.se> wrote:
>
> Hi Lad,
>
> Thanks for your reply.
>
> On 2020-03-27 12:59:52 +0000, Lad, Prabhakar wrote:
> > Hi Niklas,
> >
> > On Thu, Mar 19, 2020 at 3:03 PM Niklas <niklas.soderlund@ragnatech.se> wrote:
> > >
> > > Hi Prabhakar,
> > >
> > > Thanks for the sample files, sorry I did not have time before now to
> > > look at them. After doing so I believe I know what is wrong, see bellow.
> > >
> > > On 2020-03-15 19:35:58 +0000, Lad, Prabhakar wrote:
> > > > Hi Niklas,
> > > >
> > > > On Tue, Mar 10, 2020 at 2:06 PM Niklas <niklas.soderlund@ragnatech.se> wrote:
> > > > >
> > > > > Hi Lad,
> > > > >
> > > > > On 2020-03-10 13:42:20 +0000, Prabhakar Mahadev Lad wrote:
> > > > > > Hi Niklas,
> > > > > >
> > > > > > Thank for the review.
> > > > > >
> > > > > > > -----Original Message-----
> > > > > > > From: Niklas <niklas.soderlund@ragnatech.se>
> > > > > > > Sent: 10 March 2020 12:46
> > > > > > > To: Prabhakar Mahadev Lad <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > > > > > Cc: Mauro Carvalho Chehab <mchehab@kernel.org>; linux-
> > > > > > > media@vger.kernel.org; linux-renesas-soc@vger.kernel.org; linux-
> > > > > > > kernel@vger.kernel.org; Lad Prabhakar <prabhakar.csengg@gmail.com>
> > > > > > > Subject: Re: [PATCH v2 2/3] media: rcar-vin: Add support for
> > > > > > > MEDIA_BUS_FMT_SRGGB8_1X8 format
> > > > > > >
> > > > > > > Hi Lad,
> > > > > > >
> > > > > > > Thanks for your work.
> > > > > > >
> > > > > > > On 2020-03-10 11:06:03 +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.
> > > > > > > >
> > > > > > > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-
> > > > > > > lad.rj@bp.renesas.com>
> > > > > > > > ---
> > > > > > > >  drivers/media/platform/rcar-vin/rcar-core.c |  1 +
> > > > > > > > drivers/media/platform/rcar-vin/rcar-dma.c  |  9 ++++++++-
> > > > > > > > drivers/media/platform/rcar-vin/rcar-v4l2.c | 13 ++++++++++++-
> > > > > > > >  3 files changed, 21 insertions(+), 2 deletions(-)
> > > > > > > >
> > > > > > > > diff --git a/drivers/media/platform/rcar-vin/rcar-core.c
> > > > > > > > b/drivers/media/platform/rcar-vin/rcar-core.c
> > > > > > > > index 7440c89..76daf2f 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:
> > > > > > > >  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 1a30cd0..1c1fafa 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,7 +588,6 @@ 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);
> > > > > > > > @@ -676,6 +676,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;
> > > > > > > >  default:
> > > > > > > >  break;
> > > > > > > >  }
> > > > > > > > @@ -737,6 +740,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;
> > > > > > > >  default:
> > > > > > > >  vin_err(vin, "Invalid pixelformat (0x%x)\n",
> > > > > > > >  vin->format.pixelformat);
> > > > > > > > @@ -1110,6 +1116,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;
> > > > > > > >  break;
> > > > > > > >  default:
> > > > > > > > diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c
> > > > > > > > b/drivers/media/platform/rcar-vin/rcar-v4l2.c
> > > > > > > > index 5151a3c..4698099 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= 2,
> > > > > > >
> > > > > > > This does not look right, is not bytes-per-pixel 1 for a SRGGB8?
> > > > > > >
> > > > > > I guessed the bpp's were picked from VnIS table as I result I did the same.
> > > > > >
> > > > > > > > +},
> > > > > > > >  };
> > > > > > > >
> > > > > > > >  const struct rvin_video_format *rvin_format_from_pixel(struct
> > > > > > > > rvin_dev *vin, @@ -102,6 +106,7 @@ static u32
> > > > > > > > rvin_format_bytesperline(struct rvin_dev *vin,  {
> > > > > > > >  const struct rvin_video_format *fmt;
> > > > > > > >  u32 align;
> > > > > > > > +u8 div;
> > > > > > > >
> > > > > > > >  fmt = rvin_format_from_pixel(vin, pix->pixelformat);
> > > > > > > >
> > > > > > > > @@ -112,16 +117,22 @@ static u32 rvin_format_bytesperline(struct
> > > > > > > rvin_dev *vin,
> > > > > > > >  case V4L2_PIX_FMT_NV12:
> > > > > > > >  case V4L2_PIX_FMT_NV16:
> > > > > > > >  align = 0x20;
> > > > > > > > +div = 1;
> > > > > > > > +break;
> > > > > > > > +case V4L2_PIX_FMT_SRGGB8:
> > > > > > > > +align = 0x10;
> > > > > > > > +div = 2;
> > > > > > >
> > > > > > > Yes this does not look right at all, I think you should set bpp to 1 and drop the
> > > > > > > div handling here.
> > > > > > >
> > > > > > If I set bpp as 1 and drop the div VNIS_REG will be wrongly configured in
> > > > > > rvin_crop_scale_comp() and the image captured will be wrong.
> > > > > >
> > > > > > For example for 640x480:
> > > > > >
> > > > > > With the current patch bpp = 2:
> > > > > > bytesperline = 640
> > > > >
> > > > > This is wrong, if we have a line of 640 pixels and 2 bytes per pixel
> > > > > then bytesperline must be at least 1280 bytes right?
> > > > >
> > > > > > image size = 307200
> > > > > > stride = 320
> > > > >
> > > > > But this is incorrect, the VNIS_REG shall be at least the number of
> > > > > pixels in a line (EPPrC - SPPrC -> 640 - 0 = 640). Then we need to align
> > > > > it to the pixel unit (16, 32, 64, 128) depending on the output pixel
> > > > > format.
> > > > >
> > > > > This usually result in a stride that is larger then the line length.
> > > > > Thus you need a test application that knows the difference between width
> > > > > * bpp and bytesperline. I use qv4l2 without opengl support when I do quick
> > > > > tests and it does not support this hence I get a incorrect visual view
> > > > > of the stream when testing.
> > > > >
> > > > > How does the image capture fail with bpp = 1?
> > > > >
> > > > Attached is the captured bayer images 640x480 with bpp set to 1, for
> > > > file1bppstridediv1.raw
> > > > VNIS_REG stride set to 640 and for file file1bppstridediv2.raw
> > > > VNIS_REF stride  set to (640 * 1) / 2.
> > > > When the file1bppstridediv1.raw image is converted to png colors are incorrect
> > > > but whereas file1bppstridediv2.raw converted to png the picture is clear.
> > > >
> > > > Also while doing a loop-back to fbdevsink with the below pipeline:
> > > > gst-launch-1.0 -vvv v4l2src device=/dev/video0 io-mode=dmabuf ! 'video/x-bayer,
> > > > format=rggb,width=640,height=480,framerate=30/1' ! queue ! bayer2rgb !
> > > > videoconvert
> > > > ! fbdevsink sync=false
> > > >
> > > > width = 640 height = 480 bpp = 1, bytesperline = 640 stride = 320
> > > > works correctly
> > > > width = 640 height = 480 bpp = 1, bytesperline = 640 stride = 640
> > > > image displayed is incorrect
> > >
> > > It's very unlogical to have a stride that is less then the width, which
> > > got me interested why the second one gave you better results. I wrote a
> > > small python hack which converts the raw SRGGB8 to PNG without any
> > > debyaer, just rows of RGRGRG and BGBGBG.
> > >
> > Finally I have some information from the hardware team, the VIN process RAW8
> > in 2 pixel units as a result the stride for RAW8 needs to be
> > configured as bytesperline/2.
>
> Interesting, that is not how I have interpreted the datasheet. But
> rereading it now after our discussion I see how it could be so. I will
> dig into it during the week and see if I get make it all click in my
> head. Thanks for pointing this out.
>
Great, In that case Ill wait before I post a V4.

> >
> > The python script which you attached doesn't seem to do the right
> > conversion. I discovered
> > that Shotwell Viewer on Ubuntu can open raw files. I also confirmed
> > this by bayer2rg [1]
>
> Oops, you are right. My bad sorry for sending you down that path.
>
That gave me an opportunity to explore a bit :)

Cheers,
--Prabhakar

> >
> > # ./bayer2rgb --input=file1bppstridediv1.raw --output=file1.tiff
> > --width=640 --height=480 --bpp=8 --first=RGGB --method=BILINEAR --tiff
> > # ./bayer2rgb --input=file1bppstridediv2.raw --output=file2.tiff
> > --width=640 --height=480 --bpp=8 --first=RGGB --method=BILINEAR --tiff
> >
> > # convert file1.tiff file1bppstridediv1.png
> > # convert file2.tiff file1bppstridediv2.png
> >
> > Attached are the png images for reference.
> >
> > > Looking at the output of that seems your sensor is not sending frames of
> > > 640x480 but 480x640. Both the raw files you sent have holes in them.
> > > The first line is always 480 pixels of data and then there are sections
> > > of no data, followed by good data. Some rows are chopped and some have
> > > their 480 bytes of good data on the "left" and some on the "right" side
> > > of the frame.
> > >
> > I can confirm the sensor is sending 640x480 as the support for same
> > was added recently in IMX219 driver
> > and  was  was tested on raspi by the maintainer.
> >
> > [1] https://github.com/jdthomas/bayer2rgb
> >
> > Cheers,
> > --Prabhakar
> >
> > > So for rcar-vin I think the following settings are what you want,
> > >
> > >     width = 480 height = 640 bpp = 1, bytesperline = 480* stride = 480
> > >     * = I have not checked if this fits with alignment for VNIS
> > >
> > > I have attached the python hack and the two generated png files from
> > > your raw files so you can play with them yourself.
> > >
> > > >
> > > > Cheers,
> > > > --Prabhakar
> > > >
> > > > > >
> > > > > > And with bpp = 1 and div removed
> > > > > > bytesperline = 640
> > > > > > image size = 307200
> > > > > > stride = 640
> > > > >
> > > > >
> > > > > >
> > > > > > Cheers,
> > > > > > --Prabhakar
> > > > > >
> > > > > > > >  break;
> > > > > > > >  default:
> > > > > > > >  align = 0x10;
> > > > > > > > +div = 1;
> > > > > > > >  break;
> > > > > > > >  }
> > > > > > > >
> > > > > > > >  if (V4L2_FIELD_IS_SEQUENTIAL(pix->field))
> > > > > > > >  align = 0x80;
> > > > > > > >
> > > > > > > > -return ALIGN(pix->width, align) * fmt->bpp;
> > > > > > > > +return ALIGN(pix->width / div, align) * fmt->bpp;
> > > > > > > >  }
> > > > > > > >
> > > > > > > >  static u32 rvin_format_sizeimage(struct v4l2_pix_format *pix)
> > > > > > > > --
> > > > > > > > 2.7.4
> > > > > > > >
> > > > > > >
> > > > > > > --
> > > > > > > Regards,
> > > > > > > Niklas Söderlund
> > > > > >
> > > > > >
> > > > > > Renesas Electronics Europe GmbH, Geschaeftsfuehrer/President: Carsten Jauch, Sitz der Gesellschaft/Registered office: Duesseldorf, Arcadiastrasse 10, 40472 Duesseldorf, Germany, Handelsregister/Commercial Register: Duesseldorf, HRB 3708 USt-IDNr./Tax identification no.: DE 119353406 WEEE-Reg.-Nr./WEEE reg. no.: DE 14978647
> > > > >
> > > > > --
> > > > > Regards,
> > > > > Niklas Söderlund
> > >
> > >
> > >
> > >
> > > --
> > > Regards,
> > > Niklas Söderlund
>
>
>
>
> --
> Regards,
> Niklas Söderlund
Lad, Prabhakar April 6, 2020, 5:20 p.m. UTC | #6
HI Niklas,

On Mon, Mar 30, 2020 at 1:07 PM Niklas <niklas.soderlund@ragnatech.se> wrote:
>
> Hi Lad,
>
> Thanks for your reply.
>
> On 2020-03-27 12:59:52 +0000, Lad, Prabhakar wrote:
> > Hi Niklas,
> >
> > On Thu, Mar 19, 2020 at 3:03 PM Niklas <niklas.soderlund@ragnatech.se> wrote:
> > >
> > > Hi Prabhakar,
> > >
> > > Thanks for the sample files, sorry I did not have time before now to
> > > look at them. After doing so I believe I know what is wrong, see bellow.
> > >
> > > On 2020-03-15 19:35:58 +0000, Lad, Prabhakar wrote:
> > > > Hi Niklas,
> > > >
> > > > On Tue, Mar 10, 2020 at 2:06 PM Niklas <niklas.soderlund@ragnatech.se> wrote:
> > > > >
> > > > > Hi Lad,
> > > > >
> > > > > On 2020-03-10 13:42:20 +0000, Prabhakar Mahadev Lad wrote:
> > > > > > Hi Niklas,
> > > > > >
> > > > > > Thank for the review.
> > > > > >
> > > > > > > -----Original Message-----
> > > > > > > From: Niklas <niklas.soderlund@ragnatech.se>
> > > > > > > Sent: 10 March 2020 12:46
> > > > > > > To: Prabhakar Mahadev Lad <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > > > > > Cc: Mauro Carvalho Chehab <mchehab@kernel.org>; linux-
> > > > > > > media@vger.kernel.org; linux-renesas-soc@vger.kernel.org; linux-
> > > > > > > kernel@vger.kernel.org; Lad Prabhakar <prabhakar.csengg@gmail.com>
> > > > > > > Subject: Re: [PATCH v2 2/3] media: rcar-vin: Add support for
> > > > > > > MEDIA_BUS_FMT_SRGGB8_1X8 format
> > > > > > >
> > > > > > > Hi Lad,
> > > > > > >
> > > > > > > Thanks for your work.
> > > > > > >
> > > > > > > On 2020-03-10 11:06:03 +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.
> > > > > > > >
> > > > > > > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-
> > > > > > > lad.rj@bp.renesas.com>
> > > > > > > > ---
> > > > > > > >  drivers/media/platform/rcar-vin/rcar-core.c |  1 +
> > > > > > > > drivers/media/platform/rcar-vin/rcar-dma.c  |  9 ++++++++-
> > > > > > > > drivers/media/platform/rcar-vin/rcar-v4l2.c | 13 ++++++++++++-
> > > > > > > >  3 files changed, 21 insertions(+), 2 deletions(-)
> > > > > > > >
> > > > > > > > diff --git a/drivers/media/platform/rcar-vin/rcar-core.c
> > > > > > > > b/drivers/media/platform/rcar-vin/rcar-core.c
> > > > > > > > index 7440c89..76daf2f 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:
> > > > > > > >  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 1a30cd0..1c1fafa 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,7 +588,6 @@ 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);
> > > > > > > > @@ -676,6 +676,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;
> > > > > > > >  default:
> > > > > > > >  break;
> > > > > > > >  }
> > > > > > > > @@ -737,6 +740,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;
> > > > > > > >  default:
> > > > > > > >  vin_err(vin, "Invalid pixelformat (0x%x)\n",
> > > > > > > >  vin->format.pixelformat);
> > > > > > > > @@ -1110,6 +1116,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;
> > > > > > > >  break;
> > > > > > > >  default:
> > > > > > > > diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c
> > > > > > > > b/drivers/media/platform/rcar-vin/rcar-v4l2.c
> > > > > > > > index 5151a3c..4698099 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= 2,
> > > > > > >
> > > > > > > This does not look right, is not bytes-per-pixel 1 for a SRGGB8?
> > > > > > >
> > > > > > I guessed the bpp's were picked from VnIS table as I result I did the same.
> > > > > >
> > > > > > > > +},
> > > > > > > >  };
> > > > > > > >
> > > > > > > >  const struct rvin_video_format *rvin_format_from_pixel(struct
> > > > > > > > rvin_dev *vin, @@ -102,6 +106,7 @@ static u32
> > > > > > > > rvin_format_bytesperline(struct rvin_dev *vin,  {
> > > > > > > >  const struct rvin_video_format *fmt;
> > > > > > > >  u32 align;
> > > > > > > > +u8 div;
> > > > > > > >
> > > > > > > >  fmt = rvin_format_from_pixel(vin, pix->pixelformat);
> > > > > > > >
> > > > > > > > @@ -112,16 +117,22 @@ static u32 rvin_format_bytesperline(struct
> > > > > > > rvin_dev *vin,
> > > > > > > >  case V4L2_PIX_FMT_NV12:
> > > > > > > >  case V4L2_PIX_FMT_NV16:
> > > > > > > >  align = 0x20;
> > > > > > > > +div = 1;
> > > > > > > > +break;
> > > > > > > > +case V4L2_PIX_FMT_SRGGB8:
> > > > > > > > +align = 0x10;
> > > > > > > > +div = 2;
> > > > > > >
> > > > > > > Yes this does not look right at all, I think you should set bpp to 1 and drop the
> > > > > > > div handling here.
> > > > > > >
> > > > > > If I set bpp as 1 and drop the div VNIS_REG will be wrongly configured in
> > > > > > rvin_crop_scale_comp() and the image captured will be wrong.
> > > > > >
> > > > > > For example for 640x480:
> > > > > >
> > > > > > With the current patch bpp = 2:
> > > > > > bytesperline = 640
> > > > >
> > > > > This is wrong, if we have a line of 640 pixels and 2 bytes per pixel
> > > > > then bytesperline must be at least 1280 bytes right?
> > > > >
> > > > > > image size = 307200
> > > > > > stride = 320
> > > > >
> > > > > But this is incorrect, the VNIS_REG shall be at least the number of
> > > > > pixels in a line (EPPrC - SPPrC -> 640 - 0 = 640). Then we need to align
> > > > > it to the pixel unit (16, 32, 64, 128) depending on the output pixel
> > > > > format.
> > > > >
> > > > > This usually result in a stride that is larger then the line length.
> > > > > Thus you need a test application that knows the difference between width
> > > > > * bpp and bytesperline. I use qv4l2 without opengl support when I do quick
> > > > > tests and it does not support this hence I get a incorrect visual view
> > > > > of the stream when testing.
> > > > >
> > > > > How does the image capture fail with bpp = 1?
> > > > >
> > > > Attached is the captured bayer images 640x480 with bpp set to 1, for
> > > > file1bppstridediv1.raw
> > > > VNIS_REG stride set to 640 and for file file1bppstridediv2.raw
> > > > VNIS_REF stride  set to (640 * 1) / 2.
> > > > When the file1bppstridediv1.raw image is converted to png colors are incorrect
> > > > but whereas file1bppstridediv2.raw converted to png the picture is clear.
> > > >
> > > > Also while doing a loop-back to fbdevsink with the below pipeline:
> > > > gst-launch-1.0 -vvv v4l2src device=/dev/video0 io-mode=dmabuf ! 'video/x-bayer,
> > > > format=rggb,width=640,height=480,framerate=30/1' ! queue ! bayer2rgb !
> > > > videoconvert
> > > > ! fbdevsink sync=false
> > > >
> > > > width = 640 height = 480 bpp = 1, bytesperline = 640 stride = 320
> > > > works correctly
> > > > width = 640 height = 480 bpp = 1, bytesperline = 640 stride = 640
> > > > image displayed is incorrect
> > >
> > > It's very unlogical to have a stride that is less then the width, which
> > > got me interested why the second one gave you better results. I wrote a
> > > small python hack which converts the raw SRGGB8 to PNG without any
> > > debyaer, just rows of RGRGRG and BGBGBG.
> > >
> > Finally I have some information from the hardware team, the VIN process RAW8
> > in 2 pixel units as a result the stride for RAW8 needs to be
> > configured as bytesperline/2.
>
> Interesting, that is not how I have interpreted the datasheet. But
> rereading it now after our discussion I see how it could be so. I will
> dig into it during the week and see if I get make it all click in my
> head. Thanks for pointing this out.
>
Did you manage to get the required information on this ?

Cheers,
--Prabhakar
Niklas April 7, 2020, 9:56 a.m. UTC | #7
Hi Lad,

On 2020-04-06 18:20:33 +0100, Lad, Prabhakar wrote:
> Did you manage to get the required information on this ?

I'm still working on it, sorry for not completing it last week. I will 
let you know as soon as I can.
Niklas April 14, 2020, 7:39 p.m. UTC | #8
Hi Lad,

I spent all day playing with different solutions to how to move forward 
with this. My main problem is I have no setup where I can produce RAW 
image formats to test. But reading the datasheet I see the problem you 
are trying to solve.

I think for now the best solution might be to in rvin_crop_scale_comp() 
add a check for if the pixelformat is RAW and cut the value written to 
VNIS_REG in half. The bpp for the format shall still be set to 1.


    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;

    rvin_write(vin, stride, VNIS_REG);

I would also add a nice big comment above the if () that explains why 
the stride is cut in half for raw.

On 2020-04-07 11:56:23 +0200, Niklas wrote:
> Hi Lad,
> 
> On 2020-04-06 18:20:33 +0100, Lad, Prabhakar wrote:
> > Did you manage to get the required information on this ?
> 
> I'm still working on it, sorry for not completing it last week. I will 
> let you know as soon as I can.
> 
> -- 
> Regards,
> Niklas Söderlund
Lad, Prabhakar April 15, 2020, 8:21 a.m. UTC | #9
Hi Niklas,

On Tue, Apr 14, 2020 at 8:39 PM Niklas <niklas.soderlund@ragnatech.se> wrote:
>
> Hi Lad,
>
> I spent all day playing with different solutions to how to move forward
> with this. My main problem is I have no setup where I can produce RAW
> image formats to test. But reading the datasheet I see the problem you
> are trying to solve.
>
Thank you for looking into this.

> I think for now the best solution might be to in rvin_crop_scale_comp()
> add a check for if the pixelformat is RAW and cut the value written to
> VNIS_REG in half. The bpp for the format shall still be set to 1.
>
>
>     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;
>
>     rvin_write(vin, stride, VNIS_REG);
>
> I would also add a nice big comment above the if () that explains why
> the stride is cut in half for raw.
>
Agreed shall do that as above.

Cheers,
--Prabhakar

> On 2020-04-07 11:56:23 +0200, Niklas wrote:
> > Hi Lad,
> >
> > On 2020-04-06 18:20:33 +0100, Lad, Prabhakar wrote:
> > > Did you manage to get the required information on this ?
> >
> > I'm still working on it, sorry for not completing it last week. I will
> > let you know as soon as I can.
> >
> > --
> > Regards,
> > Niklas Söderlund
>
> --
> Regards,
> Niklas Söderlund

Patch
diff mbox series

diff --git a/drivers/media/platform/rcar-vin/rcar-core.c b/drivers/media/platform/rcar-vin/rcar-core.c
index 7440c89..76daf2f 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:
 			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 1a30cd0..1c1fafa 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,7 +588,6 @@  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);
@@ -676,6 +676,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;
 	default:
 		break;
 	}
@@ -737,6 +740,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;
 	default:
 		vin_err(vin, "Invalid pixelformat (0x%x)\n",
 			vin->format.pixelformat);
@@ -1110,6 +1116,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;
 		break;
 	default:
diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c b/drivers/media/platform/rcar-vin/rcar-v4l2.c
index 5151a3c..4698099 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			= 2,
+	},
 };
 
 const struct rvin_video_format *rvin_format_from_pixel(struct rvin_dev *vin,
@@ -102,6 +106,7 @@  static u32 rvin_format_bytesperline(struct rvin_dev *vin,
 {
 	const struct rvin_video_format *fmt;
 	u32 align;
+	u8 div;
 
 	fmt = rvin_format_from_pixel(vin, pix->pixelformat);
 
@@ -112,16 +117,22 @@  static u32 rvin_format_bytesperline(struct rvin_dev *vin,
 	case V4L2_PIX_FMT_NV12:
 	case V4L2_PIX_FMT_NV16:
 		align = 0x20;
+		div = 1;
+		break;
+	case V4L2_PIX_FMT_SRGGB8:
+		align = 0x10;
+		div = 2;
 		break;
 	default:
 		align = 0x10;
+		div = 1;
 		break;
 	}
 
 	if (V4L2_FIELD_IS_SEQUENTIAL(pix->field))
 		align = 0x80;
 
-	return ALIGN(pix->width, align) * fmt->bpp;
+	return ALIGN(pix->width / div, align) * fmt->bpp;
 }
 
 static u32 rvin_format_sizeimage(struct v4l2_pix_format *pix)