| Message ID | 20250122165353.1273739-5-niklas.soderlund+renesas@ragnatech.se (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | media: rcar-vin: Remove emulated SEQ_{TB,BT} | expand |
Hi, On 22/01/2025 18:53, Niklas Söderlund wrote: > When shutting down capture extra care was needed to try and complete a > buffer that was involved in the emulated support for SEQ_{TB,BT}. This > was needed as a buffer might be queued once to the driver, but two times > to the hardware using different offsets. > > As support for SEQ_{TB,BT} is now removed this shutdown process can be > greatly simplified. And in addition the state keeping of the VIN device > can be reduced to a single boolean value instead of keeping track of > this SEQ_{TB,BT} stopping dance. > > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se> > --- > .../platform/renesas/rcar-vin/rcar-core.c | 4 +- > .../platform/renesas/rcar-vin/rcar-dma.c | 75 ++++++------------- > .../platform/renesas/rcar-vin/rcar-vin.h | 18 +---- > 3 files changed, 26 insertions(+), 71 deletions(-) > Reviewed-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com> Tomi > diff --git a/drivers/media/platform/renesas/rcar-vin/rcar-core.c b/drivers/media/platform/renesas/rcar-vin/rcar-core.c > index b8e35ef4d9d8..cfbc9ec27706 100644 > --- a/drivers/media/platform/renesas/rcar-vin/rcar-core.c > +++ b/drivers/media/platform/renesas/rcar-vin/rcar-core.c > @@ -1080,7 +1080,7 @@ static int __maybe_unused rvin_suspend(struct device *dev) > { > struct rvin_dev *vin = dev_get_drvdata(dev); > > - if (vin->state != RUNNING) > + if (!vin->running) > return 0; > > rvin_stop_streaming(vin); > @@ -1092,7 +1092,7 @@ static int __maybe_unused rvin_resume(struct device *dev) > { > struct rvin_dev *vin = dev_get_drvdata(dev); > > - if (vin->state != RUNNING) > + if (!vin->running) > return 0; > > /* > diff --git a/drivers/media/platform/renesas/rcar-vin/rcar-dma.c b/drivers/media/platform/renesas/rcar-vin/rcar-dma.c > index ba55ccf648de..3eb6b5fcac89 100644 > --- a/drivers/media/platform/renesas/rcar-vin/rcar-dma.c > +++ b/drivers/media/platform/renesas/rcar-vin/rcar-dma.c > @@ -1022,8 +1022,7 @@ static void rvin_fill_hw_slot(struct rvin_dev *vin, int slot) > if (WARN_ON(vin->buf_hw[slot].buffer)) > return; > > - if ((vin->state != STOPPED && vin->state != RUNNING) || > - list_empty(&vin->buf_list)) { > + if (list_empty(&vin->buf_list)) { > vin->buf_hw[slot].buffer = NULL; > phys_addr = vin->scratch_phys; > } else { > @@ -1064,8 +1063,6 @@ static int rvin_capture_start(struct rvin_dev *vin) > /* Continuous Frame Capture Mode */ > rvin_write(vin, VNFC_C_FRAME, VNFC_REG); > > - vin->state = RUNNING; > - > return 0; > } > > @@ -1106,9 +1103,9 @@ static irqreturn_t rvin_irq(int irq, void *data) > if (!(int_status & VNINTS_FIS)) > goto done; > > - /* Nothing to do if capture status is 'STOPPED' */ > - if (vin->state == STOPPED) { > - vin_dbg(vin, "IRQ while state stopped\n"); > + /* Nothing to do if not running. */ > + if (!vin->running) { > + vin_dbg(vin, "IRQ while not running, ignoring\n"); > goto done; > } > > @@ -1389,6 +1386,8 @@ int rvin_start_streaming(struct rvin_dev *vin) > if (ret) > rvin_set_stream(vin, 0); > > + vin->running = true; > + > spin_unlock_irqrestore(&vin->qlock, flags); > > return ret; > @@ -1421,44 +1420,21 @@ static int rvin_start_streaming_vq(struct vb2_queue *vq, unsigned int count) > > void rvin_stop_streaming(struct rvin_dev *vin) > { > - unsigned int i, retries; > unsigned long flags; > - bool buffersFreed; > > spin_lock_irqsave(&vin->qlock, flags); > > - if (vin->state == STOPPED) { > + if (!vin->running) { > spin_unlock_irqrestore(&vin->qlock, flags); > return; > } > > - vin->state = STOPPING; > - > - /* Wait until only scratch buffer is used, max 3 interrupts. */ > - retries = 0; > - while (retries++ < RVIN_RETRIES) { > - buffersFreed = true; > - for (i = 0; i < HW_BUFFER_NUM; i++) > - if (vin->buf_hw[i].buffer) > - buffersFreed = false; > - > - if (buffersFreed) > - break; > - > - spin_unlock_irqrestore(&vin->qlock, flags); > - msleep(RVIN_TIMEOUT_MS); > - spin_lock_irqsave(&vin->qlock, flags); > - } > - > /* Wait for streaming to stop */ > - retries = 0; > - while (retries++ < RVIN_RETRIES) { > - > + for (unsigned int i = 0; i < RVIN_RETRIES; i++) { > rvin_capture_stop(vin); > > /* Check if HW is stopped */ > if (!rvin_capture_active(vin)) { > - vin->state = STOPPED; > break; > } > > @@ -1467,32 +1443,25 @@ void rvin_stop_streaming(struct rvin_dev *vin) > spin_lock_irqsave(&vin->qlock, flags); > } > > - if (!buffersFreed || vin->state != STOPPED) { > - /* > - * If this happens something have gone horribly wrong. > - * Set state to stopped to prevent the interrupt handler > - * to make things worse... > - */ > - vin_err(vin, "Failed stop HW, something is seriously broken\n"); > - vin->state = STOPPED; > - } > + if (rvin_capture_active(vin)) > + vin_err(vin, "Hardware did not stop\n"); > + > + vin->running = false; > > spin_unlock_irqrestore(&vin->qlock, flags); > > - /* If something went wrong, free buffers with an error. */ > - if (!buffersFreed) { > - return_unused_buffers(vin, VB2_BUF_STATE_ERROR); > - for (i = 0; i < HW_BUFFER_NUM; i++) { > - if (vin->buf_hw[i].buffer) > - vb2_buffer_done(&vin->buf_hw[i].buffer->vb2_buf, > - VB2_BUF_STATE_ERROR); > - } > - } > - > rvin_set_stream(vin, 0); > > /* disable interrupts */ > rvin_disable_interrupts(vin); > + > + /* Return unprocessed buffers from hardware. */ > + for (unsigned int i = 0; i < HW_BUFFER_NUM; i++) { > + if (vin->buf_hw[i].buffer) > + vb2_buffer_done(&vin->buf_hw[i].buffer->vb2_buf, > + VB2_BUF_STATE_ERROR); > + } > + > } > > static void rvin_stop_streaming_vq(struct vb2_queue *vq) > @@ -1538,8 +1507,6 @@ int rvin_dma_register(struct rvin_dev *vin, int irq) > > spin_lock_init(&vin->qlock); > > - vin->state = STOPPED; > - > for (i = 0; i < HW_BUFFER_NUM; i++) > vin->buf_hw[i].buffer = NULL; > > @@ -1642,7 +1609,7 @@ void rvin_set_alpha(struct rvin_dev *vin, unsigned int alpha) > > vin->alpha = alpha; > > - if (vin->state == STOPPED) > + if (!vin->running) > goto out; > > switch (vin->format.pixelformat) { > diff --git a/drivers/media/platform/renesas/rcar-vin/rcar-vin.h b/drivers/media/platform/renesas/rcar-vin/rcar-vin.h > index f13ef379d095..934474d2334a 100644 > --- a/drivers/media/platform/renesas/rcar-vin/rcar-vin.h > +++ b/drivers/media/platform/renesas/rcar-vin/rcar-vin.h > @@ -61,18 +61,6 @@ enum rvin_isp_id { > (((unsigned int)RVIN_CSI_MAX) > ((unsigned int)RVIN_ISP_MAX) ? \ > (unsigned int)RVIN_CSI_MAX : (unsigned int)RVIN_ISP_MAX) > > -/** > - * enum rvin_dma_state - DMA states > - * @STOPPED: No operation in progress > - * @RUNNING: Operation in progress have buffers > - * @STOPPING: Stopping operation > - */ > -enum rvin_dma_state { > - STOPPED = 0, > - RUNNING, > - STOPPING, > -}; > - > /** > * struct rvin_video_format - Data format stored in memory > * @fourcc: Pixelformat > @@ -173,11 +161,11 @@ struct rvin_info { > * @scratch: cpu address for scratch buffer > * @scratch_phys: physical address of the scratch buffer > * > - * @qlock: protects @buf_hw, @buf_list, @sequence and @state > + * @qlock: Protects @buf_hw, @buf_list, @sequence and @running > * @buf_hw: Keeps track of buffers given to HW slot > * @buf_list: list of queued buffers > * @sequence: V4L2 buffers sequence number > - * @state: keeps track of operation state > + * @running: Keeps track of if the VIN is running > * > * @is_csi: flag to mark the VIN as using a CSI-2 subdevice > * @chsel: Cached value of the current CSI-2 channel selection > @@ -220,7 +208,7 @@ struct rvin_dev { > } buf_hw[HW_BUFFER_NUM]; > struct list_head buf_list; > unsigned int sequence; > - enum rvin_dma_state state; > + bool running; > > bool is_csi; > unsigned int chsel;
diff --git a/drivers/media/platform/renesas/rcar-vin/rcar-core.c b/drivers/media/platform/renesas/rcar-vin/rcar-core.c index b8e35ef4d9d8..cfbc9ec27706 100644 --- a/drivers/media/platform/renesas/rcar-vin/rcar-core.c +++ b/drivers/media/platform/renesas/rcar-vin/rcar-core.c @@ -1080,7 +1080,7 @@ static int __maybe_unused rvin_suspend(struct device *dev) { struct rvin_dev *vin = dev_get_drvdata(dev); - if (vin->state != RUNNING) + if (!vin->running) return 0; rvin_stop_streaming(vin); @@ -1092,7 +1092,7 @@ static int __maybe_unused rvin_resume(struct device *dev) { struct rvin_dev *vin = dev_get_drvdata(dev); - if (vin->state != RUNNING) + if (!vin->running) return 0; /* diff --git a/drivers/media/platform/renesas/rcar-vin/rcar-dma.c b/drivers/media/platform/renesas/rcar-vin/rcar-dma.c index ba55ccf648de..3eb6b5fcac89 100644 --- a/drivers/media/platform/renesas/rcar-vin/rcar-dma.c +++ b/drivers/media/platform/renesas/rcar-vin/rcar-dma.c @@ -1022,8 +1022,7 @@ static void rvin_fill_hw_slot(struct rvin_dev *vin, int slot) if (WARN_ON(vin->buf_hw[slot].buffer)) return; - if ((vin->state != STOPPED && vin->state != RUNNING) || - list_empty(&vin->buf_list)) { + if (list_empty(&vin->buf_list)) { vin->buf_hw[slot].buffer = NULL; phys_addr = vin->scratch_phys; } else { @@ -1064,8 +1063,6 @@ static int rvin_capture_start(struct rvin_dev *vin) /* Continuous Frame Capture Mode */ rvin_write(vin, VNFC_C_FRAME, VNFC_REG); - vin->state = RUNNING; - return 0; } @@ -1106,9 +1103,9 @@ static irqreturn_t rvin_irq(int irq, void *data) if (!(int_status & VNINTS_FIS)) goto done; - /* Nothing to do if capture status is 'STOPPED' */ - if (vin->state == STOPPED) { - vin_dbg(vin, "IRQ while state stopped\n"); + /* Nothing to do if not running. */ + if (!vin->running) { + vin_dbg(vin, "IRQ while not running, ignoring\n"); goto done; } @@ -1389,6 +1386,8 @@ int rvin_start_streaming(struct rvin_dev *vin) if (ret) rvin_set_stream(vin, 0); + vin->running = true; + spin_unlock_irqrestore(&vin->qlock, flags); return ret; @@ -1421,44 +1420,21 @@ static int rvin_start_streaming_vq(struct vb2_queue *vq, unsigned int count) void rvin_stop_streaming(struct rvin_dev *vin) { - unsigned int i, retries; unsigned long flags; - bool buffersFreed; spin_lock_irqsave(&vin->qlock, flags); - if (vin->state == STOPPED) { + if (!vin->running) { spin_unlock_irqrestore(&vin->qlock, flags); return; } - vin->state = STOPPING; - - /* Wait until only scratch buffer is used, max 3 interrupts. */ - retries = 0; - while (retries++ < RVIN_RETRIES) { - buffersFreed = true; - for (i = 0; i < HW_BUFFER_NUM; i++) - if (vin->buf_hw[i].buffer) - buffersFreed = false; - - if (buffersFreed) - break; - - spin_unlock_irqrestore(&vin->qlock, flags); - msleep(RVIN_TIMEOUT_MS); - spin_lock_irqsave(&vin->qlock, flags); - } - /* Wait for streaming to stop */ - retries = 0; - while (retries++ < RVIN_RETRIES) { - + for (unsigned int i = 0; i < RVIN_RETRIES; i++) { rvin_capture_stop(vin); /* Check if HW is stopped */ if (!rvin_capture_active(vin)) { - vin->state = STOPPED; break; } @@ -1467,32 +1443,25 @@ void rvin_stop_streaming(struct rvin_dev *vin) spin_lock_irqsave(&vin->qlock, flags); } - if (!buffersFreed || vin->state != STOPPED) { - /* - * If this happens something have gone horribly wrong. - * Set state to stopped to prevent the interrupt handler - * to make things worse... - */ - vin_err(vin, "Failed stop HW, something is seriously broken\n"); - vin->state = STOPPED; - } + if (rvin_capture_active(vin)) + vin_err(vin, "Hardware did not stop\n"); + + vin->running = false; spin_unlock_irqrestore(&vin->qlock, flags); - /* If something went wrong, free buffers with an error. */ - if (!buffersFreed) { - return_unused_buffers(vin, VB2_BUF_STATE_ERROR); - for (i = 0; i < HW_BUFFER_NUM; i++) { - if (vin->buf_hw[i].buffer) - vb2_buffer_done(&vin->buf_hw[i].buffer->vb2_buf, - VB2_BUF_STATE_ERROR); - } - } - rvin_set_stream(vin, 0); /* disable interrupts */ rvin_disable_interrupts(vin); + + /* Return unprocessed buffers from hardware. */ + for (unsigned int i = 0; i < HW_BUFFER_NUM; i++) { + if (vin->buf_hw[i].buffer) + vb2_buffer_done(&vin->buf_hw[i].buffer->vb2_buf, + VB2_BUF_STATE_ERROR); + } + } static void rvin_stop_streaming_vq(struct vb2_queue *vq) @@ -1538,8 +1507,6 @@ int rvin_dma_register(struct rvin_dev *vin, int irq) spin_lock_init(&vin->qlock); - vin->state = STOPPED; - for (i = 0; i < HW_BUFFER_NUM; i++) vin->buf_hw[i].buffer = NULL; @@ -1642,7 +1609,7 @@ void rvin_set_alpha(struct rvin_dev *vin, unsigned int alpha) vin->alpha = alpha; - if (vin->state == STOPPED) + if (!vin->running) goto out; switch (vin->format.pixelformat) { diff --git a/drivers/media/platform/renesas/rcar-vin/rcar-vin.h b/drivers/media/platform/renesas/rcar-vin/rcar-vin.h index f13ef379d095..934474d2334a 100644 --- a/drivers/media/platform/renesas/rcar-vin/rcar-vin.h +++ b/drivers/media/platform/renesas/rcar-vin/rcar-vin.h @@ -61,18 +61,6 @@ enum rvin_isp_id { (((unsigned int)RVIN_CSI_MAX) > ((unsigned int)RVIN_ISP_MAX) ? \ (unsigned int)RVIN_CSI_MAX : (unsigned int)RVIN_ISP_MAX) -/** - * enum rvin_dma_state - DMA states - * @STOPPED: No operation in progress - * @RUNNING: Operation in progress have buffers - * @STOPPING: Stopping operation - */ -enum rvin_dma_state { - STOPPED = 0, - RUNNING, - STOPPING, -}; - /** * struct rvin_video_format - Data format stored in memory * @fourcc: Pixelformat @@ -173,11 +161,11 @@ struct rvin_info { * @scratch: cpu address for scratch buffer * @scratch_phys: physical address of the scratch buffer * - * @qlock: protects @buf_hw, @buf_list, @sequence and @state + * @qlock: Protects @buf_hw, @buf_list, @sequence and @running * @buf_hw: Keeps track of buffers given to HW slot * @buf_list: list of queued buffers * @sequence: V4L2 buffers sequence number - * @state: keeps track of operation state + * @running: Keeps track of if the VIN is running * * @is_csi: flag to mark the VIN as using a CSI-2 subdevice * @chsel: Cached value of the current CSI-2 channel selection @@ -220,7 +208,7 @@ struct rvin_dev { } buf_hw[HW_BUFFER_NUM]; struct list_head buf_list; unsigned int sequence; - enum rvin_dma_state state; + bool running; bool is_csi; unsigned int chsel;
When shutting down capture extra care was needed to try and complete a buffer that was involved in the emulated support for SEQ_{TB,BT}. This was needed as a buffer might be queued once to the driver, but two times to the hardware using different offsets. As support for SEQ_{TB,BT} is now removed this shutdown process can be greatly simplified. And in addition the state keeping of the VIN device can be reduced to a single boolean value instead of keeping track of this SEQ_{TB,BT} stopping dance. Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se> --- .../platform/renesas/rcar-vin/rcar-core.c | 4 +- .../platform/renesas/rcar-vin/rcar-dma.c | 75 ++++++------------- .../platform/renesas/rcar-vin/rcar-vin.h | 18 +---- 3 files changed, 26 insertions(+), 71 deletions(-)