All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] rcar-vin: add support for UDS (Up Down Scaler)
@ 2018-09-14  2:13 Niklas Söderlund
  2018-09-14  2:13 ` [PATCH 1/3] rcar-vin: align format width with hardware limits Niklas Söderlund
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Niklas Söderlund @ 2018-09-14  2:13 UTC (permalink / raw)
  To: Laurent Pinchart, linux-media; +Cc: linux-renesas-soc, Niklas Söderlund

Hi,

This series adds support for Renesas R-Car Gen3 VIN Up Down Scaler 
(UDS). Some VIN instances have access to a often shared UDS which can be 
used to scale the captured image up or down. If the scaler is shared it 
can only be used exclusively by one VIN at a time, switching in runtime 
and detection if a UDS are in use is supported in this series. If the 
user tries to start a capture on a VIN which would require the use of a 
scaler but that scaler is in use -EBUSY is returned.

Patch 1/3 fix a format alignment issue found when working with UDS 
support. While patch 2/3 ands the UDS logic and 3/3 defines which VIN on 
which SoC have access to a UDS and how it's shared.

The series is based on top of media-tree/master and is tested on R-Car 
Gen3 H3, M3-W, M3-N and Gen2 Koelsch (checking for regressions as Gen2 
have no UDS).

Niklas Söderlund (3):
  rcar-vin: align format width with hardware limits
  rcar-vin: add support for UDS (Up Down Scaler)
  rcar-vin: declare which VINs can use a Up Down Scaler (UDS)

 drivers/media/platform/rcar-vin/rcar-core.c |  18 +++
 drivers/media/platform/rcar-vin/rcar-dma.c  | 134 +++++++++++++++++++-
 drivers/media/platform/rcar-vin/rcar-v4l2.c |  15 +++
 drivers/media/platform/rcar-vin/rcar-vin.h  |  24 ++++
 4 files changed, 185 insertions(+), 6 deletions(-)

-- 
2.18.0

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 1/3] rcar-vin: align format width with hardware limits
  2018-09-14  2:13 [PATCH 0/3] rcar-vin: add support for UDS (Up Down Scaler) Niklas Söderlund
@ 2018-09-14  2:13 ` Niklas Söderlund
  2018-09-14  8:47   ` Hans Verkuil
  2018-09-14  2:13 ` [PATCH 2/3] rcar-vin: add support for UDS (Up Down Scaler) Niklas Söderlund
  2018-09-14  2:13 ` [PATCH 3/3] rcar-vin: declare which VINs can use a Up Down Scaler (UDS) Niklas Söderlund
  2 siblings, 1 reply; 7+ messages in thread
From: Niklas Söderlund @ 2018-09-14  2:13 UTC (permalink / raw)
  To: Laurent Pinchart, linux-media; +Cc: linux-renesas-soc, Niklas Söderlund

The Gen3 datasheets lists specific alignment restrictions compared to
Gen2. This was overlooked when adding Gen3 support as no problematic
configuration was encountered. However when adding support for Gen3 Up
Down Scaler (UDS) strange issues could be observed for odd widths
without taking this limit into consideration.

Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
---
 drivers/media/platform/rcar-vin/rcar-v4l2.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c b/drivers/media/platform/rcar-vin/rcar-v4l2.c
index dc77682b47857c97..2fc2a05eaeacb134 100644
--- a/drivers/media/platform/rcar-vin/rcar-v4l2.c
+++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c
@@ -673,6 +673,21 @@ static void rvin_mc_try_format(struct rvin_dev *vin,
 	pix->quantization = V4L2_MAP_QUANTIZATION_DEFAULT(true, pix->colorspace,
 							  pix->ycbcr_enc);
 
+	switch (vin->format.pixelformat) {
+	case V4L2_PIX_FMT_NV16:
+		pix->width = ALIGN(pix->width, 0x80);
+		break;
+	case V4L2_PIX_FMT_YUYV:
+	case V4L2_PIX_FMT_UYVY:
+	case V4L2_PIX_FMT_RGB565:
+	case V4L2_PIX_FMT_XRGB555:
+		pix->width = ALIGN(pix->width, 0x40);
+		break;
+	default:
+		pix->width = ALIGN(pix->width, 0x20);
+		break;
+	}
+
 	rvin_format_align(vin, pix);
 }
 
-- 
2.18.0

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 2/3] rcar-vin: add support for UDS (Up Down Scaler)
  2018-09-14  2:13 [PATCH 0/3] rcar-vin: add support for UDS (Up Down Scaler) Niklas Söderlund
  2018-09-14  2:13 ` [PATCH 1/3] rcar-vin: align format width with hardware limits Niklas Söderlund
@ 2018-09-14  2:13 ` Niklas Söderlund
  2018-09-14  2:13 ` [PATCH 3/3] rcar-vin: declare which VINs can use a Up Down Scaler (UDS) Niklas Söderlund
  2 siblings, 0 replies; 7+ messages in thread
From: Niklas Söderlund @ 2018-09-14  2:13 UTC (permalink / raw)
  To: Laurent Pinchart, linux-media; +Cc: linux-renesas-soc, Niklas Söderlund

Some VIN instances have access to a Up Down Scaler (UDS). The UDS are on
most SoCs shared between two VINs, the UDS can of course only be used by
one VIN at a time.

Add support to configure the UDS registers which are mapped to both VINs
sharing the UDS address-space. While validations the format at stream
start make sure the companion VIN is not already using the scaler. If
the scaler already is in use return -EBUSY.

Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
---
 drivers/media/platform/rcar-vin/rcar-dma.c | 134 ++++++++++++++++++++-
 drivers/media/platform/rcar-vin/rcar-vin.h |  24 ++++
 2 files changed, 152 insertions(+), 6 deletions(-)

diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c b/drivers/media/platform/rcar-vin/rcar-dma.c
index 92323310f7352147..673034f0f735dbb0 100644
--- a/drivers/media/platform/rcar-vin/rcar-dma.c
+++ b/drivers/media/platform/rcar-vin/rcar-dma.c
@@ -74,6 +74,10 @@
 
 /* Register offsets specific for Gen3 */
 #define VNCSI_IFMD_REG		0x20 /* Video n CSI2 Interface Mode Register */
+#define VNUDS_CTRL_REG		0x80 /* Video n scaling control register */
+#define VNUDS_SCALE_REG		0x84 /* Video n scaling factor register */
+#define VNUDS_PASS_BWIDTH_REG	0x90 /* Video n passband register */
+#define VNUDS_CLIP_SIZE_REG	0xa4 /* Video n UDS output size clipping reg */
 
 /* Register bit fields for R-Car VIN */
 /* Video n Main Control Register bits */
@@ -129,6 +133,9 @@
 #define VNCSI_IFMD_CSI_CHSEL(n) (((n) & 0xf) << 0)
 #define VNCSI_IFMD_CSI_CHSEL_MASK 0xf
 
+/* Video n scaling control register (Gen3) */
+#define VNUDS_CTRL_AMD		(1 << 30)
+
 struct rvin_buffer {
 	struct vb2_v4l2_buffer vb;
 	struct list_head list;
@@ -572,6 +579,78 @@ static void rvin_crop_scale_comp_gen2(struct rvin_dev *vin)
 		0, 0);
 }
 
+static unsigned int rvin_scale_ratio(unsigned int in, unsigned int out)
+{
+	unsigned int ratio;
+
+	ratio = in * 4096 / out;
+	return ratio >= 0x10000 ? 0xffff : ratio;
+}
+
+static unsigned int rvin_ratio_to_bwidth(unsigned int ratio)
+{
+	unsigned int mant, frac;
+
+	mant = (ratio & 0xF000) >> 12;
+	frac = ratio & 0x0FFF;
+	if (mant)
+		return 64 * 4096 * mant / (4096 * mant + frac);
+
+	return 64;
+}
+
+static bool rvin_gen3_need_scaling(struct rvin_dev *vin)
+{
+	if (vin->info->model != RCAR_GEN3)
+		return false;
+
+	return vin->crop.width != vin->format.width ||
+		vin->crop.height != vin->format.height;
+}
+
+static void rvin_crop_scale_comp_gen3(struct rvin_dev *vin)
+{
+	unsigned int ratio_h, ratio_v;
+	unsigned int bwidth_h, bwidth_v;
+	u32 vnmc, clip_size;
+
+	if (!rvin_gen3_need_scaling(vin))
+		return;
+
+	ratio_h = rvin_scale_ratio(vin->crop.width, vin->format.width);
+	bwidth_h = rvin_ratio_to_bwidth(ratio_h);
+
+	ratio_v = rvin_scale_ratio(vin->crop.height, vin->format.height);
+	bwidth_v = rvin_ratio_to_bwidth(ratio_v);
+
+	clip_size = vin->format.width << 16;
+
+	switch (vin->format.field) {
+	case V4L2_FIELD_INTERLACED_TB:
+	case V4L2_FIELD_INTERLACED_BT:
+	case V4L2_FIELD_INTERLACED:
+	case V4L2_FIELD_SEQ_TB:
+	case V4L2_FIELD_SEQ_BT:
+		clip_size |= vin->format.height / 2;
+		break;
+	default:
+		clip_size |= vin->format.height;
+		break;
+	}
+
+	vnmc = rvin_read(vin, VNMC_REG);
+	rvin_write(vin, (vnmc & ~VNMC_VUP) | VNMC_SCLE, VNMC_REG);
+	rvin_write(vin, VNUDS_CTRL_AMD, VNUDS_CTRL_REG);
+	rvin_write(vin, (ratio_h << 16) | ratio_v, VNUDS_SCALE_REG);
+	rvin_write(vin, (bwidth_h << 16) | bwidth_v, VNUDS_PASS_BWIDTH_REG);
+	rvin_write(vin, clip_size, VNUDS_CLIP_SIZE_REG);
+	rvin_write(vin, vnmc, VNMC_REG);
+
+	vin_dbg(vin, "Pre-Clip: %ux%u@%u:%u Post-Clip: %ux%u@%u:%u\n",
+		vin->crop.width, vin->crop.height, vin->crop.left,
+		vin->crop.top, vin->format.width, vin->format.height, 0, 0);
+}
+
 void rvin_crop_scale_comp(struct rvin_dev *vin)
 {
 	/* Set Start/End Pixel/Line Pre-Clip */
@@ -593,8 +672,9 @@ void rvin_crop_scale_comp(struct rvin_dev *vin)
 		break;
 	}
 
-	/* TODO: Add support for the UDS scaler. */
-	if (vin->info->model != RCAR_GEN3)
+	if (vin->info->model == RCAR_GEN3)
+		rvin_crop_scale_comp_gen3(vin);
+	else
 		rvin_crop_scale_comp_gen2(vin);
 
 	if (vin->format.pixelformat == V4L2_PIX_FMT_NV16)
@@ -751,6 +831,9 @@ static int rvin_setup(struct rvin_dev *vin)
 			vnmc |= VNMC_DPINE;
 	}
 
+	if (rvin_gen3_need_scaling(vin))
+		vnmc |= VNMC_SCLE;
+
 	/* Progressive or interlaced mode */
 	interrupts = progressive ? VNIE_FIE : VNIE_EFE;
 
@@ -1081,10 +1164,42 @@ static int rvin_mc_validate_format(struct rvin_dev *vin, struct v4l2_subdev *sd,
 		return -EPIPE;
 	}
 
-	if (fmt.format.width != vin->format.width ||
-	    fmt.format.height != vin->format.height ||
-	    fmt.format.code != vin->mbus_code)
-		return -EPIPE;
+	vin->crop.width = fmt.format.width;
+	vin->crop.height = fmt.format.height;
+
+	if (rvin_gen3_need_scaling(vin)) {
+		const struct rvin_group_scaler *scaler;
+		struct rvin_dev *companion;
+
+		if (fmt.format.code != vin->mbus_code)
+			return -EPIPE;
+
+		if (!vin->info->scalers)
+			return -EPIPE;
+
+		for (scaler = vin->info->scalers;
+		     scaler->vin || scaler->companion; scaler++)
+			if (scaler->vin == vin->id)
+				break;
+
+		/* No scaler found for VIN. */
+		if (!scaler->vin && !scaler->companion)
+			return -EPIPE;
+
+		/* Make sure companion not using scaler. */
+		if (scaler->companion != -1) {
+			companion = vin->group->vin[scaler->companion];
+			if (companion &&
+			    companion->state != STOPPED &&
+			    rvin_gen3_need_scaling(companion))
+				return -EBUSY;
+		}
+	} else {
+		if (fmt.format.width != vin->format.width ||
+		    fmt.format.height != vin->format.height ||
+		    fmt.format.code != vin->mbus_code)
+			return -EPIPE;
+	}
 
 	return 0;
 }
@@ -1192,6 +1307,7 @@ static void rvin_stop_streaming(struct vb2_queue *vq)
 	struct rvin_dev *vin = vb2_get_drv_priv(vq);
 	unsigned long flags;
 	int retries = 0;
+	u32 vnmc;
 
 	spin_lock_irqsave(&vin->qlock, flags);
 
@@ -1223,6 +1339,12 @@ static void rvin_stop_streaming(struct vb2_queue *vq)
 		vin->state = STOPPED;
 	}
 
+	/* Clear UDS usage after we have stopped */
+	if (vin->info->model == RCAR_GEN3) {
+		vnmc = rvin_read(vin, VNMC_REG) & ~(VNMC_SCLE | VNMC_VUP);
+		rvin_write(vin, vnmc, VNMC_REG);
+	}
+
 	/* Release all active buffers */
 	return_all_buffers(vin, VB2_BUF_STATE_ERROR);
 
diff --git a/drivers/media/platform/rcar-vin/rcar-vin.h b/drivers/media/platform/rcar-vin/rcar-vin.h
index 0b13b34d03e3dce4..5a617a30ba8c9a5a 100644
--- a/drivers/media/platform/rcar-vin/rcar-vin.h
+++ b/drivers/media/platform/rcar-vin/rcar-vin.h
@@ -122,6 +122,28 @@ struct rvin_group_route {
 	unsigned int mask;
 };
 
+/**
+ * struct rvin_group_scaler - describes a scaler attached to a VIN
+ *
+ * @vin:	Numerical VIN id that have access to a UDS.
+ * @companion:  Numerical VIN id that @vin share the UDS with.
+ *
+ * -- note::
+ *	Some R-Car VIN instances have access to a Up Down Scaler (UDS).
+ *	If a VIN have a UDS attached it's almost always shared between
+ *	two VIN instances. The UDS can only be used by one VIN at a time,
+ *	so the companion relationship needs to be described as well.
+ *
+ *	There are at most two VINs sharing a UDS. For each UDS shared
+ *	between two VINs there needs to be two instances of struct
+ *	rvin_group_scaler describing each of the VINs individually. If
+ *	a VIN do not share its UDS set companion to -1.
+ */
+struct rvin_group_scaler {
+	int vin;
+	int companion;
+};
+
 /**
  * struct rvin_info - Information about the particular VIN implementation
  * @model:		VIN model
@@ -130,6 +152,7 @@ struct rvin_group_route {
  * @max_height:		max input height the VIN supports
  * @routes:		list of possible routes from the CSI-2 recivers to
  *			all VINs. The list mush be NULL terminated.
+ * @scalers:		List of available scalers, must be NULL terminated.
  */
 struct rvin_info {
 	enum model_id model;
@@ -138,6 +161,7 @@ struct rvin_info {
 	unsigned int max_width;
 	unsigned int max_height;
 	const struct rvin_group_route *routes;
+	const struct rvin_group_scaler *scalers;
 };
 
 /**
-- 
2.18.0

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 3/3] rcar-vin: declare which VINs can use a Up Down Scaler (UDS)
  2018-09-14  2:13 [PATCH 0/3] rcar-vin: add support for UDS (Up Down Scaler) Niklas Söderlund
  2018-09-14  2:13 ` [PATCH 1/3] rcar-vin: align format width with hardware limits Niklas Söderlund
  2018-09-14  2:13 ` [PATCH 2/3] rcar-vin: add support for UDS (Up Down Scaler) Niklas Söderlund
@ 2018-09-14  2:13 ` Niklas Söderlund
  2 siblings, 0 replies; 7+ messages in thread
From: Niklas Söderlund @ 2018-09-14  2:13 UTC (permalink / raw)
  To: Laurent Pinchart, linux-media; +Cc: linux-renesas-soc, Niklas Söderlund

Add information about which VINs on which SoC have access to a UDS
scaler.

Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
---
 drivers/media/platform/rcar-vin/rcar-core.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/drivers/media/platform/rcar-vin/rcar-core.c b/drivers/media/platform/rcar-vin/rcar-core.c
index 5dd16af3625c333b..64f74dfebbedbf78 100644
--- a/drivers/media/platform/rcar-vin/rcar-core.c
+++ b/drivers/media/platform/rcar-vin/rcar-core.c
@@ -920,12 +920,21 @@ static const struct rvin_group_route rcar_info_r8a7795_routes[] = {
 	{ /* Sentinel */ }
 };
 
+static const struct rvin_group_scaler rcar_info_h3_m3w_m3n_scalers[] = {
+	{ .vin = 0, .companion = 1 },
+	{ .vin = 1, .companion = 0 },
+	{ .vin = 4, .companion = 5 },
+	{ .vin = 5, .companion = 4 },
+	{ /* Sentinel */ }
+};
+
 static const struct rvin_info rcar_info_r8a7795 = {
 	.model = RCAR_GEN3,
 	.use_mc = true,
 	.max_width = 4096,
 	.max_height = 4096,
 	.routes = rcar_info_r8a7795_routes,
+	.scalers = rcar_info_h3_m3w_m3n_scalers,
 };
 
 static const struct rvin_group_route rcar_info_r8a7795es1_routes[] = {
@@ -980,6 +989,7 @@ static const struct rvin_info rcar_info_r8a7795es1 = {
 	.max_width = 4096,
 	.max_height = 4096,
 	.routes = rcar_info_r8a7795es1_routes,
+	.scalers = rcar_info_h3_m3w_m3n_scalers,
 };
 
 static const struct rvin_group_route rcar_info_r8a7796_routes[] = {
@@ -1020,6 +1030,7 @@ static const struct rvin_info rcar_info_r8a7796 = {
 	.max_width = 4096,
 	.max_height = 4096,
 	.routes = rcar_info_r8a7796_routes,
+	.scalers = rcar_info_h3_m3w_m3n_scalers,
 };
 
 static const struct rvin_group_route rcar_info_r8a77965_routes[] = {
@@ -1064,6 +1075,7 @@ static const struct rvin_info rcar_info_r8a77965 = {
 	.max_width = 4096,
 	.max_height = 4096,
 	.routes = rcar_info_r8a77965_routes,
+	.scalers = rcar_info_h3_m3w_m3n_scalers,
 };
 
 static const struct rvin_group_route rcar_info_r8a77970_routes[] = {
@@ -1089,12 +1101,18 @@ static const struct rvin_group_route rcar_info_r8a77995_routes[] = {
 	{ /* Sentinel */ }
 };
 
+static const struct rvin_group_scaler rcar_info_r8a77995_scalers[] = {
+	{ .vin = 4, .companion = -1 },
+	{ /* Sentinel */ }
+};
+
 static const struct rvin_info rcar_info_r8a77995 = {
 	.model = RCAR_GEN3,
 	.use_mc = true,
 	.max_width = 4096,
 	.max_height = 4096,
 	.routes = rcar_info_r8a77995_routes,
+	.scalers = rcar_info_r8a77995_scalers,
 };
 
 static const struct of_device_id rvin_of_id_table[] = {
-- 
2.18.0

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/3] rcar-vin: align format width with hardware limits
  2018-09-14  2:13 ` [PATCH 1/3] rcar-vin: align format width with hardware limits Niklas Söderlund
@ 2018-09-14  8:47   ` Hans Verkuil
  2018-10-04 20:01       ` Niklas Söderlund
  0 siblings, 1 reply; 7+ messages in thread
From: Hans Verkuil @ 2018-09-14  8:47 UTC (permalink / raw)
  To: Niklas Söderlund, Laurent Pinchart, linux-media; +Cc: linux-renesas-soc

On 09/14/2018 04:13 AM, Niklas Söderlund wrote:
> The Gen3 datasheets lists specific alignment restrictions compared to
> Gen2. This was overlooked when adding Gen3 support as no problematic
> configuration was encountered. However when adding support for Gen3 Up
> Down Scaler (UDS) strange issues could be observed for odd widths
> without taking this limit into consideration.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> ---
>  drivers/media/platform/rcar-vin/rcar-v4l2.c | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c b/drivers/media/platform/rcar-vin/rcar-v4l2.c
> index dc77682b47857c97..2fc2a05eaeacb134 100644
> --- a/drivers/media/platform/rcar-vin/rcar-v4l2.c
> +++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c
> @@ -673,6 +673,21 @@ static void rvin_mc_try_format(struct rvin_dev *vin,
>  	pix->quantization = V4L2_MAP_QUANTIZATION_DEFAULT(true, pix->colorspace,
>  							  pix->ycbcr_enc);
>  
> +	switch (vin->format.pixelformat) {
> +	case V4L2_PIX_FMT_NV16:
> +		pix->width = ALIGN(pix->width, 0x80);
> +		break;
> +	case V4L2_PIX_FMT_YUYV:
> +	case V4L2_PIX_FMT_UYVY:
> +	case V4L2_PIX_FMT_RGB565:
> +	case V4L2_PIX_FMT_XRGB555:
> +		pix->width = ALIGN(pix->width, 0x40);
> +		break;
> +	default:
> +		pix->width = ALIGN(pix->width, 0x20);
> +		break;
> +	}
> +
>  	rvin_format_align(vin, pix);
>  }
>  
> 

This looks weird. So for NV16 the width must be a multiple of 128,
do I read that correctly?

Are you sure you don't mean bytesperline?

And if it really is the width, doesn't this place very big constraints
on the vin driver? If you don't want/need the UDS, then I can imagine
that you don't want these alignments.

Regards,

	Hans

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/3] rcar-vin: align format width with hardware limits
  2018-09-14  8:47   ` Hans Verkuil
@ 2018-10-04 20:01       ` Niklas Söderlund
  0 siblings, 0 replies; 7+ messages in thread
From: Niklas Söderlund @ 2018-10-04 20:01 UTC (permalink / raw)
  To: Hans Verkuil; +Cc: Laurent Pinchart, linux-media, linux-renesas-soc

Hi Hans,

Thanks for your feedback.

On 2018-09-14 10:47:41 +0200, Hans Verkuil wrote:
> On 09/14/2018 04:13 AM, Niklas Söderlund wrote:
> > The Gen3 datasheets lists specific alignment restrictions compared to
> > Gen2. This was overlooked when adding Gen3 support as no problematic
> > configuration was encountered. However when adding support for Gen3 Up
> > Down Scaler (UDS) strange issues could be observed for odd widths
> > without taking this limit into consideration.
> > 
> > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> > ---
> >  drivers/media/platform/rcar-vin/rcar-v4l2.c | 15 +++++++++++++++
> >  1 file changed, 15 insertions(+)
> > 
> > diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c b/drivers/media/platform/rcar-vin/rcar-v4l2.c
> > index dc77682b47857c97..2fc2a05eaeacb134 100644
> > --- a/drivers/media/platform/rcar-vin/rcar-v4l2.c
> > +++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c
> > @@ -673,6 +673,21 @@ static void rvin_mc_try_format(struct rvin_dev *vin,
> >  	pix->quantization = V4L2_MAP_QUANTIZATION_DEFAULT(true, pix->colorspace,
> >  							  pix->ycbcr_enc);
> >  
> > +	switch (vin->format.pixelformat) {
> > +	case V4L2_PIX_FMT_NV16:
> > +		pix->width = ALIGN(pix->width, 0x80);
> > +		break;
> > +	case V4L2_PIX_FMT_YUYV:
> > +	case V4L2_PIX_FMT_UYVY:
> > +	case V4L2_PIX_FMT_RGB565:
> > +	case V4L2_PIX_FMT_XRGB555:
> > +		pix->width = ALIGN(pix->width, 0x40);
> > +		break;
> > +	default:
> > +		pix->width = ALIGN(pix->width, 0x20);
> > +		break;
> > +	}
> > +
> >  	rvin_format_align(vin, pix);
> >  }
> >  
> > 
> 
> This looks weird. So for NV16 the width must be a multiple of 128,
> do I read that correctly?
> 
> Are you sure you don't mean bytesperline?
> 
> And if it really is the width, doesn't this place very big constraints
> on the vin driver? If you don't want/need the UDS, then I can imagine
> that you don't want these alignments.

Thanks for brining this up, after more carefully reading the datasheet 
(which is a bit ambiguous in this section) I figured out that this is to 
strict. Will rewrite this patch for v2.

-- 
Regards,
Niklas Söderlund

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/3] rcar-vin: align format width with hardware limits
@ 2018-10-04 20:01       ` Niklas Söderlund
  0 siblings, 0 replies; 7+ messages in thread
From: Niklas Söderlund @ 2018-10-04 20:01 UTC (permalink / raw)
  To: Hans Verkuil; +Cc: Laurent Pinchart, linux-media, linux-renesas-soc

Hi Hans,

Thanks for your feedback.

On 2018-09-14 10:47:41 +0200, Hans Verkuil wrote:
> On 09/14/2018 04:13 AM, Niklas S�derlund wrote:
> > The Gen3 datasheets lists specific alignment restrictions compared to
> > Gen2. This was overlooked when adding Gen3 support as no problematic
> > configuration was encountered. However when adding support for Gen3 Up
> > Down Scaler (UDS) strange issues could be observed for odd widths
> > without taking this limit into consideration.
> > 
> > Signed-off-by: Niklas S�derlund <niklas.soderlund+renesas@ragnatech.se>
> > ---
> >  drivers/media/platform/rcar-vin/rcar-v4l2.c | 15 +++++++++++++++
> >  1 file changed, 15 insertions(+)
> > 
> > diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c b/drivers/media/platform/rcar-vin/rcar-v4l2.c
> > index dc77682b47857c97..2fc2a05eaeacb134 100644
> > --- a/drivers/media/platform/rcar-vin/rcar-v4l2.c
> > +++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c
> > @@ -673,6 +673,21 @@ static void rvin_mc_try_format(struct rvin_dev *vin,
> >  	pix->quantization = V4L2_MAP_QUANTIZATION_DEFAULT(true, pix->colorspace,
> >  							  pix->ycbcr_enc);
> >  
> > +	switch (vin->format.pixelformat) {
> > +	case V4L2_PIX_FMT_NV16:
> > +		pix->width = ALIGN(pix->width, 0x80);
> > +		break;
> > +	case V4L2_PIX_FMT_YUYV:
> > +	case V4L2_PIX_FMT_UYVY:
> > +	case V4L2_PIX_FMT_RGB565:
> > +	case V4L2_PIX_FMT_XRGB555:
> > +		pix->width = ALIGN(pix->width, 0x40);
> > +		break;
> > +	default:
> > +		pix->width = ALIGN(pix->width, 0x20);
> > +		break;
> > +	}
> > +
> >  	rvin_format_align(vin, pix);
> >  }
> >  
> > 
> 
> This looks weird. So for NV16 the width must be a multiple of 128,
> do I read that correctly?
> 
> Are you sure you don't mean bytesperline?
> 
> And if it really is the width, doesn't this place very big constraints
> on the vin driver? If you don't want/need the UDS, then I can imagine
> that you don't want these alignments.

Thanks for brining this up, after more carefully reading the datasheet 
(which is a bit ambiguous in this section) I figured out that this is to 
strict. Will rewrite this patch for v2.

-- 
Regards,
Niklas S�derlund

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2018-10-05  2:56 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-14  2:13 [PATCH 0/3] rcar-vin: add support for UDS (Up Down Scaler) Niklas Söderlund
2018-09-14  2:13 ` [PATCH 1/3] rcar-vin: align format width with hardware limits Niklas Söderlund
2018-09-14  8:47   ` Hans Verkuil
2018-10-04 20:01     ` Niklas Söderlund
2018-10-04 20:01       ` Niklas Söderlund
2018-09-14  2:13 ` [PATCH 2/3] rcar-vin: add support for UDS (Up Down Scaler) Niklas Söderlund
2018-09-14  2:13 ` [PATCH 3/3] rcar-vin: declare which VINs can use a Up Down Scaler (UDS) Niklas Söderlund

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.