From 372faeb46b91be672fb0f03f7b358759bcd4aed8 Mon Sep 17 00:00:00 2001 From: Dafna Hirschfeld Date: Fri, 15 May 2020 16:29:52 +0200 Subject: media: staging: rkisp1: cap: remove support of BGR666 format The rkisp1 supports RGB encoding with 6 bits per color with the following format: - - b5 b4 b3 b2 b1 b0 - - g5 g4 g3 g2 g1 g0 - - r5 r4 r3 r2 r1 r0 - - - - - - - - This is not how V4L2_PIX_FMT_BGR666 is defined, so remove this format from the driver's formats list. Signed-off-by: Dafna Hirschfeld Acked-by: Helen Koike Signed-off-by: Hans Verkuil Signed-off-by: Mauro Carvalho Chehab --- drivers/staging/media/rkisp1/rkisp1-capture.c | 4 ---- 1 file changed, 4 deletions(-) (limited to 'drivers/staging/media/rkisp1/rkisp1-capture.c') diff --git a/drivers/staging/media/rkisp1/rkisp1-capture.c b/drivers/staging/media/rkisp1/rkisp1-capture.c index f69235f82c45..793ec884c894 100644 --- a/drivers/staging/media/rkisp1/rkisp1-capture.c +++ b/drivers/staging/media/rkisp1/rkisp1-capture.c @@ -283,10 +283,6 @@ static const struct rkisp1_capture_fmt_cfg rkisp1_sp_fmts[] = { .fourcc = V4L2_PIX_FMT_RGB565, .write_format = RKISP1_MI_CTRL_SP_WRITE_PLA, .output_format = RKISP1_MI_CTRL_SP_OUTPUT_RGB565, - }, { - .fourcc = V4L2_PIX_FMT_BGR666, - .write_format = RKISP1_MI_CTRL_SP_WRITE_PLA, - .output_format = RKISP1_MI_CTRL_SP_OUTPUT_RGB666, }, }; -- cgit v1.2.3 From aa86e0bbf2ee3fe338798af78f02b5e6088b50c1 Mon Sep 17 00:00:00 2001 From: Dafna Hirschfeld Date: Tue, 14 Jul 2020 14:38:29 +0200 Subject: media: staging: rkisp1: cap: don't set next buffer from rkisp1_vb2_buf_queue The function 'rkisp1_vb2_buf_queue' sets the next buffer directly in case the capture is already streaming but no frame yet arrived from the sensor. This is an optimization that tries to avoid dropping a frame. The call atomic_read(&cap->rkisp1->isp.frame_sequence) is used to check if a frame arrived. Reading the 'frame_sequence' should be avoided outside irq handlers to avoid race conditions. This patch removes this optimization. Dropping of the first frames can be avoided if userspace queues the buffers before start streaming. If userspace starts queueing buffers only after calling 'streamon' he risks frame drops anyway. Signed-off-by: Dafna Hirschfeld Acked-by: Helen Koike Signed-off-by: Hans Verkuil Signed-off-by: Mauro Carvalho Chehab --- drivers/staging/media/rkisp1/rkisp1-capture.c | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-) (limited to 'drivers/staging/media/rkisp1/rkisp1-capture.c') diff --git a/drivers/staging/media/rkisp1/rkisp1-capture.c b/drivers/staging/media/rkisp1/rkisp1-capture.c index 793ec884c894..572b0949c81f 100644 --- a/drivers/staging/media/rkisp1/rkisp1-capture.c +++ b/drivers/staging/media/rkisp1/rkisp1-capture.c @@ -743,18 +743,7 @@ static void rkisp1_vb2_buf_queue(struct vb2_buffer *vb) ispbuf->buff_addr[RKISP1_PLANE_CB]); spin_lock_irqsave(&cap->buf.lock, flags); - - /* - * If there's no next buffer assigned, queue this buffer directly - * as the next buffer, and update the memory interface. - */ - if (cap->is_streaming && !cap->buf.next && - atomic_read(&cap->rkisp1->isp.frame_sequence) == -1) { - cap->buf.next = ispbuf; - rkisp1_set_next_buf(cap); - } else { - list_add_tail(&ispbuf->queue, &cap->buf.queue); - } + list_add_tail(&ispbuf->queue, &cap->buf.queue); spin_unlock_irqrestore(&cap->buf.lock, flags); } -- cgit v1.2.3 From 454748e3b3529218a50920e643dab6f149ab2639 Mon Sep 17 00:00:00 2001 From: Dafna Hirschfeld Date: Tue, 14 Jul 2020 14:38:30 +0200 Subject: media: staging: rkisp1: cap: protect buf.curr and buf.next with buf.lock The spinlock buf.lock should protect access to the buffers. This includes the buffers in buf.queue and also buf.curr and buf.next. The function 'rkisp1_set_next_buf' access buf.next therefore it should also be protected with the lock. Signed-off-by: Dafna Hirschfeld Acked-by: Helen Koike Signed-off-by: Hans Verkuil Signed-off-by: Mauro Carvalho Chehab --- drivers/staging/media/rkisp1/rkisp1-capture.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'drivers/staging/media/rkisp1/rkisp1-capture.c') diff --git a/drivers/staging/media/rkisp1/rkisp1-capture.c b/drivers/staging/media/rkisp1/rkisp1-capture.c index 572b0949c81f..fa3eaeac2a00 100644 --- a/drivers/staging/media/rkisp1/rkisp1-capture.c +++ b/drivers/staging/media/rkisp1/rkisp1-capture.c @@ -617,10 +617,11 @@ static void rkisp1_set_next_buf(struct rkisp1_capture *cap) static void rkisp1_handle_buffer(struct rkisp1_capture *cap) { struct rkisp1_isp *isp = &cap->rkisp1->isp; - struct rkisp1_buffer *curr_buf = cap->buf.curr; + struct rkisp1_buffer *curr_buf; unsigned long flags; spin_lock_irqsave(&cap->buf.lock, flags); + curr_buf = cap->buf.curr; if (curr_buf) { curr_buf->vb.sequence = atomic_read(&isp->frame_sequence); @@ -640,9 +641,9 @@ static void rkisp1_handle_buffer(struct rkisp1_capture *cap) queue); list_del(&cap->buf.next->queue); } - spin_unlock_irqrestore(&cap->buf.lock, flags); rkisp1_set_next_buf(cap); + spin_unlock_irqrestore(&cap->buf.lock, flags); } void rkisp1_capture_isr(struct rkisp1_device *rkisp1) -- cgit v1.2.3 From 23780e14fec91242bb621670c61afa0898407892 Mon Sep 17 00:00:00 2001 From: Dafna Hirschfeld Date: Tue, 14 Jul 2020 14:38:31 +0200 Subject: media: staging: rkisp1: cap: move code that manages the buffers to rkisp1_set_next_buf The function 'rkisp1_set_next_buf' configures the registers according to 'cap->buf.next'. It is called after updating 'cap->buf.next' and 'cap->buf.curr'. This patch moves the code that updates those fields to rkisp1_set_next_buf. This is a preparation for later patch that change a call to 'rkisp1_handle_buffer' with a call to 'rkisp1_set_next_buf'. Signed-off-by: Dafna Hirschfeld Acked-by: Helen Koike Signed-off-by: Hans Verkuil Signed-off-by: Mauro Carvalho Chehab --- drivers/staging/media/rkisp1/rkisp1-capture.c | 30 +++++++++++++-------------- 1 file changed, 14 insertions(+), 16 deletions(-) (limited to 'drivers/staging/media/rkisp1/rkisp1-capture.c') diff --git a/drivers/staging/media/rkisp1/rkisp1-capture.c b/drivers/staging/media/rkisp1/rkisp1-capture.c index fa3eaeac2a00..7f400aefe550 100644 --- a/drivers/staging/media/rkisp1/rkisp1-capture.c +++ b/drivers/staging/media/rkisp1/rkisp1-capture.c @@ -575,12 +575,16 @@ static void rkisp1_dummy_buf_destroy(struct rkisp1_capture *cap) static void rkisp1_set_next_buf(struct rkisp1_capture *cap) { - /* - * Use the dummy space allocated by dma_alloc_coherent to - * throw data if there is no available buffer. - */ - if (cap->buf.next) { - u32 *buff_addr = cap->buf.next->buff_addr; + cap->buf.curr = cap->buf.next; + cap->buf.next = NULL; + + if (!list_empty(&cap->buf.queue)) { + u32 *buff_addr; + + cap->buf.next = list_first_entry(&cap->buf.queue, struct rkisp1_buffer, queue); + list_del(&cap->buf.next->queue); + + buff_addr = cap->buf.next->buff_addr; rkisp1_write(cap->rkisp1, buff_addr[RKISP1_PLANE_Y], @@ -592,6 +596,10 @@ static void rkisp1_set_next_buf(struct rkisp1_capture *cap) buff_addr[RKISP1_PLANE_CR], cap->config->mi.cr_base_ad_init); } else { + /* + * Use the dummy space allocated by dma_alloc_coherent to + * throw data if there is no available buffer. + */ rkisp1_write(cap->rkisp1, cap->buf.dummy.dma_addr, cap->config->mi.y_base_ad_init); @@ -632,16 +640,6 @@ static void rkisp1_handle_buffer(struct rkisp1_capture *cap) cap->rkisp1->debug.frame_drop[cap->id]++; } - cap->buf.curr = cap->buf.next; - cap->buf.next = NULL; - - if (!list_empty(&cap->buf.queue)) { - cap->buf.next = list_first_entry(&cap->buf.queue, - struct rkisp1_buffer, - queue); - list_del(&cap->buf.next->queue); - } - rkisp1_set_next_buf(cap); spin_unlock_irqrestore(&cap->buf.lock, flags); } -- cgit v1.2.3 From 20698ed90f018607e7a24c5258b7ed2dffafa7ae Mon Sep 17 00:00:00 2001 From: Dafna Hirschfeld Date: Tue, 14 Jul 2020 14:38:32 +0200 Subject: media: staging: rkisp1: cap: in stream start, replace calls to rkisp1_handle_buffer with rkisp1_set_next_buf The function 'rkisp1_stream_start' calls 'rkisp1_handle_buffer' in order to update the 'buf.curr' and 'buf.next' fields and configure the device before streaming starts. This cause a wrong increment of the debugs field 'frame_drop'. This patch replaces the call to 'rkisp1_handle_buffer' with a call to 'rkisp1_set_next_buf'. Signed-off-by: Dafna Hirschfeld Acked-by: Helen Koike Signed-off-by: Hans Verkuil Signed-off-by: Mauro Carvalho Chehab --- drivers/staging/media/rkisp1/rkisp1-capture.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'drivers/staging/media/rkisp1/rkisp1-capture.c') diff --git a/drivers/staging/media/rkisp1/rkisp1-capture.c b/drivers/staging/media/rkisp1/rkisp1-capture.c index 7f400aefe550..c05280950ea0 100644 --- a/drivers/staging/media/rkisp1/rkisp1-capture.c +++ b/drivers/staging/media/rkisp1/rkisp1-capture.c @@ -916,7 +916,7 @@ static void rkisp1_stream_start(struct rkisp1_capture *cap) cap->ops->config(cap); /* Setup a buffer for the next frame */ - rkisp1_handle_buffer(cap); + rkisp1_set_next_buf(cap); cap->ops->enable(cap); /* It's safe to config ACTIVE and SHADOW regs for the * first stream. While when the second is starting, do NOT @@ -931,7 +931,7 @@ static void rkisp1_stream_start(struct rkisp1_capture *cap) /* force cfg update */ rkisp1_write(rkisp1, RKISP1_CIF_MI_INIT_SOFT_UPD, RKISP1_CIF_MI_INIT); - rkisp1_handle_buffer(cap); + rkisp1_set_next_buf(cap); } cap->is_streaming = true; } -- cgit v1.2.3