Re: [PATCH v2 1/5] media: rcar-vin: Remove emulated SEQ_{TB,BT}

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 

Hi,

On 22/01/2025 18:53, Niklas Söderlund wrote:

When the driver was converted from soc_camera software support for
V4L2_FIELD_SEQ_TB and V4L2_FIELD_SEQ_BT where added. This was done by


s/where/were/


capturing twice to the same VB2 buffer, but at different offsets.

This trend out to be a bad idea and it never really worked properly in


s/trend/turned/


all situations. As the hardware can't support this mode natively remove
trying to emulate it in software. It's still possible to capture TOP or
BOTTOM fields separately or both ALTERNATING. If user-space wants the
same  fields in the same buffer the same hack to capture twice to the
same buffer can be done.

Removing this error prone emulated support pave ways in future work to
simplify the internal buffer handling and making it less fragile, while
enabling adding support for other features the hardware actually
supports.

Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@xxxxxxxxxxxx>
---
* Changes since v1
- Remove 'prev' variable in rvin_fill_hw_slot() which is no longer used,
   but set.



I'm not really familiar with rcar-dma or this feature, but looks fine:

Reviewed-by: Tomi Valkeinen <tomi.valkeinen+renesas@xxxxxxxxxxxxxxxx>

 Tomi


---
  .../platform/renesas/rcar-vin/rcar-dma.c      | 58 +++----------------
  .../platform/renesas/rcar-vin/rcar-v4l2.c     |  7 ---
  .../platform/renesas/rcar-vin/rcar-vin.h      | 18 ------
  3 files changed, 7 insertions(+), 76 deletions(-)

diff --git a/drivers/media/platform/renesas/rcar-vin/rcar-dma.c b/drivers/media/platform/renesas/rcar-vin/rcar-dma.c
index 8773998101ff..a16adc6fd4dc 100644
--- a/drivers/media/platform/renesas/rcar-vin/rcar-dma.c
+++ b/drivers/media/platform/renesas/rcar-vin/rcar-dma.c
@@ -642,8 +642,6 @@ void rvin_scaler_gen3(struct rvin_dev *vin)
  	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->compose.height / 2;
  		break;
  	default:
@@ -727,8 +725,6 @@ static int rvin_setup(struct rvin_dev *vin)
  	case V4L2_FIELD_INTERLACED_BT:
  		vnmc = VNMC_IM_FULL | VNMC_FOC;
  		break;
-	case V4L2_FIELD_SEQ_TB:
-	case V4L2_FIELD_SEQ_BT:
  	case V4L2_FIELD_NONE:
  	case V4L2_FIELD_ALTERNATE:
  		vnmc = VNMC_IM_ODD_EVEN;
@@ -1021,33 +1017,14 @@ static void rvin_fill_hw_slot(struct rvin_dev *vin, int slot)
  	struct rvin_buffer *buf;
  	struct vb2_v4l2_buffer *vbuf;
  	dma_addr_t phys_addr;
-	int prev;
/* A already populated slot shall never be overwritten. */ 
  	if (WARN_ON(vin->buf_hw[slot].buffer))
  		return;
- prev = (slot == 0 ? HW_BUFFER_NUM : slot) - 1; 
-
-	if (vin->buf_hw[prev].type == HALF_TOP) {
-		vbuf = vin->buf_hw[prev].buffer;
-		vin->buf_hw[slot].buffer = vbuf;
-		vin->buf_hw[slot].type = HALF_BOTTOM;
-		switch (vin->format.pixelformat) {
-		case V4L2_PIX_FMT_NV12:
-		case V4L2_PIX_FMT_NV16:
-			phys_addr = vin->buf_hw[prev].phys +
-				vin->format.sizeimage / 4;
-			break;
-		default:
-			phys_addr = vin->buf_hw[prev].phys +
-				vin->format.sizeimage / 2;
-			break;
-		}
-	} else if ((vin->state != STOPPED && vin->state != RUNNING) ||
-		   list_empty(&vin->buf_list)) {
+	if ((vin->state != STOPPED && vin->state != RUNNING) ||
+	    list_empty(&vin->buf_list)) {
  		vin->buf_hw[slot].buffer = NULL;
-		vin->buf_hw[slot].type = FULL;
  		phys_addr = vin->scratch_phys;
  	} else {
  		/* Keep track of buffer we give to HW */
@@ -1056,16 +1033,12 @@ static void rvin_fill_hw_slot(struct rvin_dev *vin, int slot)
  		list_del_init(to_buf_list(vbuf));
  		vin->buf_hw[slot].buffer = vbuf;
- vin->buf_hw[slot].type = 
-			V4L2_FIELD_IS_SEQUENTIAL(vin->format.field) ?
-			HALF_TOP : FULL;
-
  		/* Setup DMA */
  		phys_addr = vb2_dma_contig_plane_dma_addr(&vbuf->vb2_buf, 0);
  	}
- vin_dbg(vin, "Filling HW slot: %d type: %d buffer: %p\n", 
-		slot, vin->buf_hw[slot].type, vin->buf_hw[slot].buffer);
+	vin_dbg(vin, "Filling HW slot: %d buffer: %p\n",
+		slot, vin->buf_hw[slot].buffer);
vin->buf_hw[slot].phys = phys_addr; 
  	rvin_set_slot_addr(vin, slot, phys_addr);
@@ -1073,15 +1046,12 @@ static void rvin_fill_hw_slot(struct rvin_dev *vin, int slot)
static int rvin_capture_start(struct rvin_dev *vin) 
  {
-	int slot, ret;
+	int ret;
- for (slot = 0; slot < HW_BUFFER_NUM; slot++) { 
+	for (unsigned int slot = 0; slot < HW_BUFFER_NUM; slot++) {
  		vin->buf_hw[slot].buffer = NULL;
-		vin->buf_hw[slot].type = FULL;
-	}
-
-	for (slot = 0; slot < HW_BUFFER_NUM; slot++)
  		rvin_fill_hw_slot(vin, slot);
+	}
ret = rvin_setup(vin); 
  	if (ret)
@@ -1162,16 +1132,6 @@ static irqreturn_t rvin_irq(int irq, void *data)
/* Capture frame */ 
  	if (vin->buf_hw[slot].buffer) {
-		/*
-		 * Nothing to do but refill the hardware slot if
-		 * capture only filled first half of vb2 buffer.
-		 */
-		if (vin->buf_hw[slot].type == HALF_TOP) {
-			vin->buf_hw[slot].buffer = NULL;
-			rvin_fill_hw_slot(vin, slot);
-			goto done;
-		}
-
  		vin->buf_hw[slot].buffer->field =
  			rvin_get_active_field(vin, vnms);
  		vin->buf_hw[slot].buffer->sequence = vin->sequence;
@@ -1322,8 +1282,6 @@ static int rvin_mc_validate_format(struct rvin_dev *vin, struct v4l2_subdev *sd,
  	case V4L2_FIELD_INTERLACED_TB:
  	case V4L2_FIELD_INTERLACED_BT:
  	case V4L2_FIELD_INTERLACED:
-	case V4L2_FIELD_SEQ_TB:
-	case V4L2_FIELD_SEQ_BT:
  		/* Supported natively */
  		break;
  	case V4L2_FIELD_ALTERNATE:
@@ -1336,8 +1294,6 @@ static int rvin_mc_validate_format(struct rvin_dev *vin, struct v4l2_subdev *sd,
  		case V4L2_FIELD_INTERLACED_TB:
  		case V4L2_FIELD_INTERLACED_BT:
  		case V4L2_FIELD_INTERLACED:
-		case V4L2_FIELD_SEQ_TB:
-		case V4L2_FIELD_SEQ_BT:
  			/* Use VIN hardware to combine the two fields */
  			fmt.format.height *= 2;
  			break;
diff --git a/drivers/media/platform/renesas/rcar-vin/rcar-v4l2.c b/drivers/media/platform/renesas/rcar-vin/rcar-v4l2.c
index 756fdfdbce61..a5763f1c5784 100644
--- a/drivers/media/platform/renesas/rcar-vin/rcar-v4l2.c
+++ b/drivers/media/platform/renesas/rcar-vin/rcar-v4l2.c
@@ -161,9 +161,6 @@ static u32 rvin_format_bytesperline(struct rvin_dev *vin,
  		break;
  	}
- if (V4L2_FIELD_IS_SEQUENTIAL(pix->field)) 
-		align = 0x80;
-
  	return ALIGN(pix->width, align) * fmt->bpp;
  }
@@ -194,8 +191,6 @@ static void rvin_format_align(struct rvin_dev *vin, struct v4l2_pix_format *pix) 
  	case V4L2_FIELD_INTERLACED_BT:
  	case V4L2_FIELD_INTERLACED:
  	case V4L2_FIELD_ALTERNATE:
-	case V4L2_FIELD_SEQ_TB:
-	case V4L2_FIELD_SEQ_BT:
  		break;
  	default:
  		pix->field = RVIN_DEFAULT_FIELD;
@@ -504,8 +499,6 @@ static int rvin_remote_rectangle(struct rvin_dev *vin, struct v4l2_rect *rect)
  		case V4L2_FIELD_INTERLACED_TB:
  		case V4L2_FIELD_INTERLACED_BT:
  		case V4L2_FIELD_INTERLACED:
-		case V4L2_FIELD_SEQ_TB:
-		case V4L2_FIELD_SEQ_BT:
  			rect->height *= 2;
  			break;
  		}
diff --git a/drivers/media/platform/renesas/rcar-vin/rcar-vin.h b/drivers/media/platform/renesas/rcar-vin/rcar-vin.h
index f87d4bc9e53e..d5763462809a 100644
--- a/drivers/media/platform/renesas/rcar-vin/rcar-vin.h
+++ b/drivers/media/platform/renesas/rcar-vin/rcar-vin.h
@@ -77,23 +77,6 @@ enum rvin_dma_state {
  	SUSPENDED,
  };
-/** 
- * enum rvin_buffer_type
- *
- * Describes how a buffer is given to the hardware. To be able
- * to capture SEQ_TB/BT it's needed to capture to the same vb2
- * buffer twice so the type of buffer needs to be kept.
- *
- * @FULL: One capture fills the whole vb2 buffer
- * @HALF_TOP: One capture fills the top half of the vb2 buffer
- * @HALF_BOTTOM: One capture fills the bottom half of the vb2 buffer
- */
-enum rvin_buffer_type {
-	FULL,
-	HALF_TOP,
-	HALF_BOTTOM,
-};
-
  /**
   * struct rvin_video_format - Data format stored in memory
   * @fourcc:	Pixelformat
@@ -237,7 +220,6 @@ struct rvin_dev {
  	spinlock_t qlock;
  	struct {
  		struct vb2_v4l2_buffer *buffer;
-		enum rvin_buffer_type type;
  		dma_addr_t phys;
  	} buf_hw[HW_BUFFER_NUM];
  	struct list_head buf_list;
[Index of Archives]     [Linux Samsung SOC]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]

  Powered by Linux