Hi Dafna,
On 8/15/20 7:37 AM, Dafna Hirschfeld wrote:
> The signal RKISP1_CIF_ISP_FRAME is set when the ISP completes
> outputting the frame to the next block in the pipeline.
> In order to keep buffer synchronization we assume that the
> RKISP1_CIF_ISP_V_START signal never arrives together with the
> RKISP1_CIF_ISP_FRAME signal.
> In case those signals arrive together then the code is not able to
> tell if the RKISP1_CIF_ISP_FRAME signal relates to the frame of
> the current v-start or the previous. This patch adds a WARN_ONCE
> and a debugfs var to catch it.
>
> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@xxxxxxxxxxxxx>
> ---
> drivers/staging/media/rkisp1/rkisp1-common.h | 1 +
> drivers/staging/media/rkisp1/rkisp1-dev.c | 2 ++
> drivers/staging/media/rkisp1/rkisp1-isp.c | 5 +++++
> 3 files changed, 8 insertions(+)
>
> diff --git a/drivers/staging/media/rkisp1/rkisp1-common.h b/drivers/staging/media/rkisp1/rkisp1-common.h
> index 79edece6ee77..5aed0e37850a 100644
> --- a/drivers/staging/media/rkisp1/rkisp1-common.h
> +++ b/drivers/staging/media/rkisp1/rkisp1-common.h
> @@ -228,6 +228,7 @@ struct rkisp1_debug {
> unsigned long outform_size_error;
> unsigned long img_stabilization_size_error;
> unsigned long inform_size_error;
> + unsigned long irq_delay;
> unsigned long mipi_error;
> unsigned long stats_error;
> unsigned long stop_timeout[2];
> diff --git a/drivers/staging/media/rkisp1/rkisp1-dev.c b/drivers/staging/media/rkisp1/rkisp1-dev.c
> index a0eb8f08708b..557de940286b 100644
> --- a/drivers/staging/media/rkisp1/rkisp1-dev.c
> +++ b/drivers/staging/media/rkisp1/rkisp1-dev.c
> @@ -445,6 +445,8 @@ static void rkisp1_debug_init(struct rkisp1_device *rkisp1)
> &debug->img_stabilization_size_error);
> debugfs_create_ulong("inform_size_error", 0444, debug->debugfs_dir,
> &debug->inform_size_error);
> + debugfs_create_ulong("irq_delay", 0444, debug->debugfs_dir,
> + &debug->irq_delay);
> debugfs_create_ulong("mipi_error", 0444, debug->debugfs_dir,
> &debug->mipi_error);
> debugfs_create_ulong("stats_error", 0444, debug->debugfs_dir,
> diff --git a/drivers/staging/media/rkisp1/rkisp1-isp.c b/drivers/staging/media/rkisp1/rkisp1-isp.c
> index 1ffe7cc7bb12..33cfad19dde2 100644
> --- a/drivers/staging/media/rkisp1/rkisp1-isp.c
> +++ b/drivers/staging/media/rkisp1/rkisp1-isp.c
> @@ -1118,6 +1118,11 @@ void rkisp1_isp_isr(struct rkisp1_device *rkisp1)
> */
> rkisp1->isp.frame_sequence++;
> rkisp1_isp_queue_event_sof(&rkisp1->isp);
> + if (status & RKISP1_CIF_ISP_FRAME) {
> + WARN_ONCE(1, "irq delay is too long, buffers might not be in sync\n");
> + rkisp1->debug.irq_delay++;
> + }
> + }
I was wondering, if both interrupts arrive at the same time, shouldn't we handle the
frame before incrementing the frame_sequence?
I mean, move the code block
if (status & RKISP1_CIF_ISP_V_START) {...}
after this other code block
if (status & RKISP1_CIF_ISP_FRAME) {...}
Thanks,
Helen
>
> if (status & RKISP1_CIF_ISP_PIC_SIZE_ERROR) {
> /* Clear pic_size_error */
>
