* [PATCH v2 0/5] media: staging: rkisp1: cap: various fixes for capture formats
@ 2020-04-02 19:04 Dafna Hirschfeld
2020-04-02 19:04 ` [PATCH v2 1/5] media: staging: rkisp1: cap: cleanup in mainpath config for uv swap format Dafna Hirschfeld
` (4 more replies)
0 siblings, 5 replies; 19+ messages in thread
From: Dafna Hirschfeld @ 2020-04-02 19:04 UTC (permalink / raw)
To: linux-media, dafna.hirschfeld, helen.koike, ezequiel, hverkuil,
kernel, dafna3, laurent.pinchart, linux-rockchip
This patchset fixes various issues related to
the supported formats in the rkisp1 capture.
Patches summary:
patches 1,2 - fixes a wrong assignments to the register that swaps the
'u', 'v' planes in YUV semiplanar formats.
patch 3 - sets the register from patch 1 only if the format is semiplanar.
patch 4 - adds support to planar YUV formats with swapped u,v planes
by swapping the addresses of the planes
patch 5 - removes some packed YUV formats that are not supported
by the driver.
changes from v1:
- split the first patch from v1 into two separate patches, the first is a cleanup patch
the second fixes a bug.
Dafna Hirschfeld (5):
media: staging: rkisp1: cap: cleanup in mainpath config for uv swap
format
media: staging: rkisp1: cap: fix value written to uv swap register in
selfpath
media: staging: rkisp1: cap: support uv swap only for semiplanar
formats
media: staging: rkisp1: cap: support uv swapped plane formats
media: staging: rkisp1: cap: remove unsupported formats
drivers/staging/media/rkisp1/rkisp1-capture.c | 55 ++++++++++---------
1 file changed, 28 insertions(+), 27 deletions(-)
--
2.17.1
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v2 1/5] media: staging: rkisp1: cap: cleanup in mainpath config for uv swap format
2020-04-02 19:04 [PATCH v2 0/5] media: staging: rkisp1: cap: various fixes for capture formats Dafna Hirschfeld
@ 2020-04-02 19:04 ` Dafna Hirschfeld
2020-04-02 19:53 ` Helen Koike
2020-04-05 17:57 ` Laurent Pinchart
2020-04-02 19:04 ` [PATCH v2 2/5] media: staging: rkisp1: cap: fix value written to uv swap register in selfpath Dafna Hirschfeld
` (3 subsequent siblings)
4 siblings, 2 replies; 19+ messages in thread
From: Dafna Hirschfeld @ 2020-04-02 19:04 UTC (permalink / raw)
To: linux-media, dafna.hirschfeld, helen.koike, ezequiel, hverkuil,
kernel, dafna3, laurent.pinchart, linux-rockchip
The value RKISP1_CIF_MI_XTD_FMT_CTRL_MP_CB_CR_SWAP equals BIT(0),
Therefore when writing it to the register there is no need to mask
it first with ~BIT(0).
Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
---
drivers/staging/media/rkisp1/rkisp1-capture.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/staging/media/rkisp1/rkisp1-capture.c b/drivers/staging/media/rkisp1/rkisp1-capture.c
index 45d237a77ca4..5700d7be2819 100644
--- a/drivers/staging/media/rkisp1/rkisp1-capture.c
+++ b/drivers/staging/media/rkisp1/rkisp1-capture.c
@@ -432,8 +432,7 @@ static void rkisp1_mp_config(struct rkisp1_capture *cap)
if (cap->pix.cfg->uv_swap) {
reg = rkisp1_read(rkisp1, RKISP1_CIF_MI_XTD_FORMAT_CTRL);
- reg = (reg & ~BIT(0)) |
- RKISP1_CIF_MI_XTD_FMT_CTRL_MP_CB_CR_SWAP;
+ reg = reg | RKISP1_CIF_MI_XTD_FMT_CTRL_MP_CB_CR_SWAP;
rkisp1_write(rkisp1, reg, RKISP1_CIF_MI_XTD_FORMAT_CTRL);
}
--
2.17.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v2 2/5] media: staging: rkisp1: cap: fix value written to uv swap register in selfpath
2020-04-02 19:04 [PATCH v2 0/5] media: staging: rkisp1: cap: various fixes for capture formats Dafna Hirschfeld
2020-04-02 19:04 ` [PATCH v2 1/5] media: staging: rkisp1: cap: cleanup in mainpath config for uv swap format Dafna Hirschfeld
@ 2020-04-02 19:04 ` Dafna Hirschfeld
2020-04-02 19:53 ` Helen Koike
2020-04-05 18:10 ` Laurent Pinchart
2020-04-02 19:04 ` [PATCH v2 3/5] media: staging: rkisp1: cap: support uv swap only for semiplanar formats Dafna Hirschfeld
` (2 subsequent siblings)
4 siblings, 2 replies; 19+ messages in thread
From: Dafna Hirschfeld @ 2020-04-02 19:04 UTC (permalink / raw)
To: linux-media, dafna.hirschfeld, helen.koike, ezequiel, hverkuil,
kernel, dafna3, laurent.pinchart, linux-rockchip
The value RKISP1_CIF_MI_XTD_FMT_CTRL_SP_CB_CR_SWAP should be
set to the register instead of masking with ~BIT(1)
Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
---
drivers/staging/media/rkisp1/rkisp1-capture.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/media/rkisp1/rkisp1-capture.c b/drivers/staging/media/rkisp1/rkisp1-capture.c
index 5700d7be2819..84a3cf565106 100644
--- a/drivers/staging/media/rkisp1/rkisp1-capture.c
+++ b/drivers/staging/media/rkisp1/rkisp1-capture.c
@@ -469,8 +469,8 @@ static void rkisp1_sp_config(struct rkisp1_capture *cap)
if (cap->pix.cfg->uv_swap) {
u32 reg = rkisp1_read(rkisp1, RKISP1_CIF_MI_XTD_FORMAT_CTRL);
- rkisp1_write(rkisp1, reg & ~BIT(1),
- RKISP1_CIF_MI_XTD_FORMAT_CTRL);
+ reg = reg | RKISP1_CIF_MI_XTD_FMT_CTRL_SP_CB_CR_SWAP;
+ rkisp1_write(rkisp1, reg, RKISP1_CIF_MI_XTD_FORMAT_CTRL);
}
rkisp1_mi_config_ctrl(cap);
--
2.17.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v2 3/5] media: staging: rkisp1: cap: support uv swap only for semiplanar formats
2020-04-02 19:04 [PATCH v2 0/5] media: staging: rkisp1: cap: various fixes for capture formats Dafna Hirschfeld
2020-04-02 19:04 ` [PATCH v2 1/5] media: staging: rkisp1: cap: cleanup in mainpath config for uv swap format Dafna Hirschfeld
2020-04-02 19:04 ` [PATCH v2 2/5] media: staging: rkisp1: cap: fix value written to uv swap register in selfpath Dafna Hirschfeld
@ 2020-04-02 19:04 ` Dafna Hirschfeld
2020-04-05 18:14 ` Laurent Pinchart
2020-04-02 19:04 ` [PATCH v2 4/5] media: staging: rkisp1: cap: support uv swapped plane formats Dafna Hirschfeld
2020-04-02 19:04 ` [PATCH v2 5/5] media: staging: rkisp1: cap: remove unsupported formats Dafna Hirschfeld
4 siblings, 1 reply; 19+ messages in thread
From: Dafna Hirschfeld @ 2020-04-02 19:04 UTC (permalink / raw)
To: linux-media, dafna.hirschfeld, helen.koike, ezequiel, hverkuil,
kernel, dafna3, laurent.pinchart, linux-rockchip
The register RKISP1_CIF_MI_XTD_FORMAT_CTRL is relevant only
for semiplanar formats, therefore the uv swap can be supported
through this register only for semiplanar formats.
Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
Acked-by: Helen Koike <helen.koike@collabora.com>
---
drivers/staging/media/rkisp1/rkisp1-capture.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/media/rkisp1/rkisp1-capture.c b/drivers/staging/media/rkisp1/rkisp1-capture.c
index 84a3cf565106..fa2849209433 100644
--- a/drivers/staging/media/rkisp1/rkisp1-capture.c
+++ b/drivers/staging/media/rkisp1/rkisp1-capture.c
@@ -37,6 +37,10 @@
#define RKISP1_MIN_BUFFERS_NEEDED 3
+#define RKISP1_IS_SEMI_PLANAR(write_format) \
+ (((write_format) == RKISP1_MI_CTRL_MP_WRITE_YUV_SPLA) || \
+ ((write_format) == RKISP1_MI_CTRL_SP_WRITE_SPLA))
+
enum rkisp1_plane {
RKISP1_PLANE_Y = 0,
RKISP1_PLANE_CB = 1,
@@ -429,7 +433,8 @@ static void rkisp1_mp_config(struct rkisp1_capture *cap)
cap->config->mi.cr_size_init);
rkisp1_irq_frame_end_enable(cap);
- if (cap->pix.cfg->uv_swap) {
+ if (RKISP1_IS_SEMI_PLANAR(cap->pix.cfg->write_format) &&
+ cap->pix.cfg->uv_swap) {
reg = rkisp1_read(rkisp1, RKISP1_CIF_MI_XTD_FORMAT_CTRL);
reg = reg | RKISP1_CIF_MI_XTD_FMT_CTRL_MP_CB_CR_SWAP;
@@ -466,7 +471,8 @@ static void rkisp1_sp_config(struct rkisp1_capture *cap)
rkisp1_write(rkisp1, cap->sp_y_stride, RKISP1_CIF_MI_SP_Y_LLENGTH);
rkisp1_irq_frame_end_enable(cap);
- if (cap->pix.cfg->uv_swap) {
+ if (RKISP1_IS_SEMI_PLANAR(cap->pix.cfg->write_format) &&
+ cap->pix.cfg->uv_swap) {
u32 reg = rkisp1_read(rkisp1, RKISP1_CIF_MI_XTD_FORMAT_CTRL);
reg = reg | RKISP1_CIF_MI_XTD_FMT_CTRL_SP_CB_CR_SWAP;
--
2.17.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v2 4/5] media: staging: rkisp1: cap: support uv swapped plane formats
2020-04-02 19:04 [PATCH v2 0/5] media: staging: rkisp1: cap: various fixes for capture formats Dafna Hirschfeld
` (2 preceding siblings ...)
2020-04-02 19:04 ` [PATCH v2 3/5] media: staging: rkisp1: cap: support uv swap only for semiplanar formats Dafna Hirschfeld
@ 2020-04-02 19:04 ` Dafna Hirschfeld
2020-04-05 18:16 ` Laurent Pinchart
2020-04-02 19:04 ` [PATCH v2 5/5] media: staging: rkisp1: cap: remove unsupported formats Dafna Hirschfeld
4 siblings, 1 reply; 19+ messages in thread
From: Dafna Hirschfeld @ 2020-04-02 19:04 UTC (permalink / raw)
To: linux-media, dafna.hirschfeld, helen.koike, ezequiel, hverkuil,
kernel, dafna3, laurent.pinchart, linux-rockchip
Plane formats with the u and v planes swapped can be
supported by changing the address of the cb and cr in
the buffer.
Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
Acked-by: Helen Koike <helen.koike@collabora.com>
---
drivers/staging/media/rkisp1/rkisp1-capture.c | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/drivers/staging/media/rkisp1/rkisp1-capture.c b/drivers/staging/media/rkisp1/rkisp1-capture.c
index fa2849209433..2d274e8f565b 100644
--- a/drivers/staging/media/rkisp1/rkisp1-capture.c
+++ b/drivers/staging/media/rkisp1/rkisp1-capture.c
@@ -41,6 +41,10 @@
(((write_format) == RKISP1_MI_CTRL_MP_WRITE_YUV_SPLA) || \
((write_format) == RKISP1_MI_CTRL_SP_WRITE_SPLA))
+#define RKISP1_IS_PLANAR(write_format) \
+ (((write_format) == RKISP1_MI_CTRL_SP_WRITE_PLA) || \
+ ((write_format) == RKISP1_MI_CTRL_MP_WRITE_YUV_PLA_OR_RAW8))
+
enum rkisp1_plane {
RKISP1_PLANE_Y = 0,
RKISP1_PLANE_CB = 1,
@@ -788,6 +792,19 @@ static void rkisp1_vb2_buf_queue(struct vb2_buffer *vb)
rkisp1_pixfmt_comp_size(pixm, RKISP1_PLANE_CB);
}
+ /*
+ * uv swap can be supported for plane formats by switching
+ * the address of cb and cr
+ */
+ if (RKISP1_IS_PLANAR(cap->pix.cfg->write_format) &&
+ cap->pix.cfg->uv_swap) {
+ ispbuf->buff_addr[RKISP1_PLANE_CR] =
+ ispbuf->buff_addr[RKISP1_PLANE_CB];
+ ispbuf->buff_addr[RKISP1_PLANE_CB] =
+ ispbuf->buff_addr[RKISP1_PLANE_CR] +
+ rkisp1_pixfmt_comp_size(pixm, RKISP1_PLANE_CR);
+ }
+
spin_lock_irqsave(&cap->buf.lock, flags);
/*
--
2.17.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v2 5/5] media: staging: rkisp1: cap: remove unsupported formats
2020-04-02 19:04 [PATCH v2 0/5] media: staging: rkisp1: cap: various fixes for capture formats Dafna Hirschfeld
` (3 preceding siblings ...)
2020-04-02 19:04 ` [PATCH v2 4/5] media: staging: rkisp1: cap: support uv swapped plane formats Dafna Hirschfeld
@ 2020-04-02 19:04 ` Dafna Hirschfeld
2020-04-05 22:43 ` Laurent Pinchart
4 siblings, 1 reply; 19+ messages in thread
From: Dafna Hirschfeld @ 2020-04-02 19:04 UTC (permalink / raw)
To: linux-media, dafna.hirschfeld, helen.koike, ezequiel, hverkuil,
kernel, dafna3, laurent.pinchart, linux-rockchip
For Ycbcr packed formats only YUYV can be supported by
the driver. This patch removes the other formats.
Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
Acked-by: Helen Koike <helen.koike@collabora.com>
---
drivers/staging/media/rkisp1/rkisp1-capture.c | 21 -------------------
1 file changed, 21 deletions(-)
diff --git a/drivers/staging/media/rkisp1/rkisp1-capture.c b/drivers/staging/media/rkisp1/rkisp1-capture.c
index 2d274e8f565b..076335193f40 100644
--- a/drivers/staging/media/rkisp1/rkisp1-capture.c
+++ b/drivers/staging/media/rkisp1/rkisp1-capture.c
@@ -98,15 +98,6 @@ static const struct rkisp1_capture_fmt_cfg rkisp1_mp_fmts[] = {
.fmt_type = RKISP1_FMT_YUV,
.uv_swap = 0,
.write_format = RKISP1_MI_CTRL_MP_WRITE_YUVINT,
- }, {
- .fourcc = V4L2_PIX_FMT_YVYU,
- .fmt_type = RKISP1_FMT_YUV,
- .uv_swap = 1,
- .write_format = RKISP1_MI_CTRL_MP_WRITE_YUVINT,
- }, {
- .fourcc = V4L2_PIX_FMT_VYUY,
- .fmt_type = RKISP1_FMT_YUV,
- .write_format = RKISP1_MI_CTRL_MP_WRITE_YUVINT,
}, {
.fourcc = V4L2_PIX_FMT_YUV422P,
.fmt_type = RKISP1_FMT_YUV,
@@ -234,18 +225,6 @@ static const struct rkisp1_capture_fmt_cfg rkisp1_sp_fmts[] = {
.uv_swap = 0,
.write_format = RKISP1_MI_CTRL_SP_WRITE_INT,
.output_format = RKISP1_MI_CTRL_SP_OUTPUT_YUV422,
- }, {
- .fourcc = V4L2_PIX_FMT_YVYU,
- .fmt_type = RKISP1_FMT_YUV,
- .uv_swap = 1,
- .write_format = RKISP1_MI_CTRL_SP_WRITE_INT,
- .output_format = RKISP1_MI_CTRL_SP_OUTPUT_YUV422,
- }, {
- .fourcc = V4L2_PIX_FMT_VYUY,
- .fmt_type = RKISP1_FMT_YUV,
- .uv_swap = 1,
- .write_format = RKISP1_MI_CTRL_SP_WRITE_INT,
- .output_format = RKISP1_MI_CTRL_SP_OUTPUT_YUV422,
}, {
.fourcc = V4L2_PIX_FMT_YUV422P,
.fmt_type = RKISP1_FMT_YUV,
--
2.17.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH v2 1/5] media: staging: rkisp1: cap: cleanup in mainpath config for uv swap format
2020-04-02 19:04 ` [PATCH v2 1/5] media: staging: rkisp1: cap: cleanup in mainpath config for uv swap format Dafna Hirschfeld
@ 2020-04-02 19:53 ` Helen Koike
2020-04-05 17:57 ` Laurent Pinchart
1 sibling, 0 replies; 19+ messages in thread
From: Helen Koike @ 2020-04-02 19:53 UTC (permalink / raw)
To: Dafna Hirschfeld, linux-media, ezequiel, hverkuil, kernel,
dafna3, laurent.pinchart, linux-rockchip
On 4/2/20 4:04 PM, Dafna Hirschfeld wrote:
> The value RKISP1_CIF_MI_XTD_FMT_CTRL_MP_CB_CR_SWAP equals BIT(0),
> Therefore when writing it to the register there is no need to mask
> it first with ~BIT(0).
>
> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
Acked-by: Helen Koike <helen.koike@collabora.com>
> ---
> drivers/staging/media/rkisp1/rkisp1-capture.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/staging/media/rkisp1/rkisp1-capture.c b/drivers/staging/media/rkisp1/rkisp1-capture.c
> index 45d237a77ca4..5700d7be2819 100644
> --- a/drivers/staging/media/rkisp1/rkisp1-capture.c
> +++ b/drivers/staging/media/rkisp1/rkisp1-capture.c
> @@ -432,8 +432,7 @@ static void rkisp1_mp_config(struct rkisp1_capture *cap)
> if (cap->pix.cfg->uv_swap) {
> reg = rkisp1_read(rkisp1, RKISP1_CIF_MI_XTD_FORMAT_CTRL);
>
> - reg = (reg & ~BIT(0)) |
> - RKISP1_CIF_MI_XTD_FMT_CTRL_MP_CB_CR_SWAP;
> + reg = reg | RKISP1_CIF_MI_XTD_FMT_CTRL_MP_CB_CR_SWAP;
> rkisp1_write(rkisp1, reg, RKISP1_CIF_MI_XTD_FORMAT_CTRL);
> }
>
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 2/5] media: staging: rkisp1: cap: fix value written to uv swap register in selfpath
2020-04-02 19:04 ` [PATCH v2 2/5] media: staging: rkisp1: cap: fix value written to uv swap register in selfpath Dafna Hirschfeld
@ 2020-04-02 19:53 ` Helen Koike
2020-04-05 18:10 ` Laurent Pinchart
1 sibling, 0 replies; 19+ messages in thread
From: Helen Koike @ 2020-04-02 19:53 UTC (permalink / raw)
To: Dafna Hirschfeld, linux-media, ezequiel, hverkuil, kernel,
dafna3, laurent.pinchart, linux-rockchip
On 4/2/20 4:04 PM, Dafna Hirschfeld wrote:
> The value RKISP1_CIF_MI_XTD_FMT_CTRL_SP_CB_CR_SWAP should be
> set to the register instead of masking with ~BIT(1)
>
> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
Acked-by: Helen Koike <helen.koike@collabora.com>
> ---
> drivers/staging/media/rkisp1/rkisp1-capture.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/media/rkisp1/rkisp1-capture.c b/drivers/staging/media/rkisp1/rkisp1-capture.c
> index 5700d7be2819..84a3cf565106 100644
> --- a/drivers/staging/media/rkisp1/rkisp1-capture.c
> +++ b/drivers/staging/media/rkisp1/rkisp1-capture.c
> @@ -469,8 +469,8 @@ static void rkisp1_sp_config(struct rkisp1_capture *cap)
> if (cap->pix.cfg->uv_swap) {
> u32 reg = rkisp1_read(rkisp1, RKISP1_CIF_MI_XTD_FORMAT_CTRL);
>
> - rkisp1_write(rkisp1, reg & ~BIT(1),
> - RKISP1_CIF_MI_XTD_FORMAT_CTRL);
> + reg = reg | RKISP1_CIF_MI_XTD_FMT_CTRL_SP_CB_CR_SWAP;
> + rkisp1_write(rkisp1, reg, RKISP1_CIF_MI_XTD_FORMAT_CTRL);
> }
>
> rkisp1_mi_config_ctrl(cap);
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 1/5] media: staging: rkisp1: cap: cleanup in mainpath config for uv swap format
2020-04-02 19:04 ` [PATCH v2 1/5] media: staging: rkisp1: cap: cleanup in mainpath config for uv swap format Dafna Hirschfeld
2020-04-02 19:53 ` Helen Koike
@ 2020-04-05 17:57 ` Laurent Pinchart
1 sibling, 0 replies; 19+ messages in thread
From: Laurent Pinchart @ 2020-04-05 17:57 UTC (permalink / raw)
To: Dafna Hirschfeld
Cc: linux-media, helen.koike, ezequiel, hverkuil, kernel, dafna3,
linux-rockchip
Hi Dafna,
Thank you for the patch.
On Thu, Apr 02, 2020 at 09:04:15PM +0200, Dafna Hirschfeld wrote:
> The value RKISP1_CIF_MI_XTD_FMT_CTRL_MP_CB_CR_SWAP equals BIT(0),
> Therefore when writing it to the register there is no need to mask
> it first with ~BIT(0).
>
> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
> ---
> drivers/staging/media/rkisp1/rkisp1-capture.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/staging/media/rkisp1/rkisp1-capture.c b/drivers/staging/media/rkisp1/rkisp1-capture.c
> index 45d237a77ca4..5700d7be2819 100644
> --- a/drivers/staging/media/rkisp1/rkisp1-capture.c
> +++ b/drivers/staging/media/rkisp1/rkisp1-capture.c
> @@ -432,8 +432,7 @@ static void rkisp1_mp_config(struct rkisp1_capture *cap)
> if (cap->pix.cfg->uv_swap) {
> reg = rkisp1_read(rkisp1, RKISP1_CIF_MI_XTD_FORMAT_CTRL);
>
> - reg = (reg & ~BIT(0)) |
> - RKISP1_CIF_MI_XTD_FMT_CTRL_MP_CB_CR_SWAP;
> + reg = reg | RKISP1_CIF_MI_XTD_FMT_CTRL_MP_CB_CR_SWAP;
I'd write this
reg |= RKISP1_CIF_MI_XTD_FMT_CTRL_MP_CB_CR_SWAP;
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> rkisp1_write(rkisp1, reg, RKISP1_CIF_MI_XTD_FORMAT_CTRL);
> }
>
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 2/5] media: staging: rkisp1: cap: fix value written to uv swap register in selfpath
2020-04-02 19:04 ` [PATCH v2 2/5] media: staging: rkisp1: cap: fix value written to uv swap register in selfpath Dafna Hirschfeld
2020-04-02 19:53 ` Helen Koike
@ 2020-04-05 18:10 ` Laurent Pinchart
2020-04-06 11:20 ` Dafna Hirschfeld
1 sibling, 1 reply; 19+ messages in thread
From: Laurent Pinchart @ 2020-04-05 18:10 UTC (permalink / raw)
To: Dafna Hirschfeld
Cc: linux-media, helen.koike, ezequiel, hverkuil, kernel, dafna3,
linux-rockchip
Hi Dafna,
Thank you for the patch.
On Thu, Apr 02, 2020 at 09:04:16PM +0200, Dafna Hirschfeld wrote:
> The value RKISP1_CIF_MI_XTD_FMT_CTRL_SP_CB_CR_SWAP should be
> set to the register instead of masking with ~BIT(1)
>
> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
> ---
> drivers/staging/media/rkisp1/rkisp1-capture.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/media/rkisp1/rkisp1-capture.c b/drivers/staging/media/rkisp1/rkisp1-capture.c
> index 5700d7be2819..84a3cf565106 100644
> --- a/drivers/staging/media/rkisp1/rkisp1-capture.c
> +++ b/drivers/staging/media/rkisp1/rkisp1-capture.c
> @@ -469,8 +469,8 @@ static void rkisp1_sp_config(struct rkisp1_capture *cap)
> if (cap->pix.cfg->uv_swap) {
> u32 reg = rkisp1_read(rkisp1, RKISP1_CIF_MI_XTD_FORMAT_CTRL);
>
> - rkisp1_write(rkisp1, reg & ~BIT(1),
> - RKISP1_CIF_MI_XTD_FORMAT_CTRL);
> + reg = reg | RKISP1_CIF_MI_XTD_FMT_CTRL_SP_CB_CR_SWAP;
> + rkisp1_write(rkisp1, reg, RKISP1_CIF_MI_XTD_FORMAT_CTRL);
This indeed simplifies the code, but I think the logic is wrong in the
first place. Shouldn't this be
reg = rkisp1_read(rkisp1, RKISP1_CIF_MI_XTD_FORMAT_CTRL);
if (cap->pix.cfg->uv_swap)
reg |= RKISP1_CIF_MI_XTD_FMT_CTRL_SP_CB_CR_SWAP;
else
reg &= ~RKISP1_CIF_MI_XTD_FMT_CTRL_SP_CB_CR_SWAP;
rkisp1_write(rkisp1, reg, RKISP1_CIF_MI_XTD_FORMAT_CTRL);
? Same for patch 1/5.
> }
>
> rkisp1_mi_config_ctrl(cap);
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 3/5] media: staging: rkisp1: cap: support uv swap only for semiplanar formats
2020-04-02 19:04 ` [PATCH v2 3/5] media: staging: rkisp1: cap: support uv swap only for semiplanar formats Dafna Hirschfeld
@ 2020-04-05 18:14 ` Laurent Pinchart
0 siblings, 0 replies; 19+ messages in thread
From: Laurent Pinchart @ 2020-04-05 18:14 UTC (permalink / raw)
To: Dafna Hirschfeld
Cc: linux-media, helen.koike, ezequiel, hverkuil, kernel, dafna3,
linux-rockchip
Hi Dafna,
Thank you for the patch.
On Thu, Apr 02, 2020 at 09:04:17PM +0200, Dafna Hirschfeld wrote:
> The register RKISP1_CIF_MI_XTD_FORMAT_CTRL is relevant only
> for semiplanar formats, therefore the uv swap can be supported
> through this register only for semiplanar formats.
>
> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
> Acked-by: Helen Koike <helen.koike@collabora.com>
> ---
> drivers/staging/media/rkisp1/rkisp1-capture.c | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/media/rkisp1/rkisp1-capture.c b/drivers/staging/media/rkisp1/rkisp1-capture.c
> index 84a3cf565106..fa2849209433 100644
> --- a/drivers/staging/media/rkisp1/rkisp1-capture.c
> +++ b/drivers/staging/media/rkisp1/rkisp1-capture.c
> @@ -37,6 +37,10 @@
>
> #define RKISP1_MIN_BUFFERS_NEEDED 3
>
> +#define RKISP1_IS_SEMI_PLANAR(write_format) \
> + (((write_format) == RKISP1_MI_CTRL_MP_WRITE_YUV_SPLA) || \
> + ((write_format) == RKISP1_MI_CTRL_SP_WRITE_SPLA))
> +
> enum rkisp1_plane {
> RKISP1_PLANE_Y = 0,
> RKISP1_PLANE_CB = 1,
> @@ -429,7 +433,8 @@ static void rkisp1_mp_config(struct rkisp1_capture *cap)
> cap->config->mi.cr_size_init);
>
> rkisp1_irq_frame_end_enable(cap);
> - if (cap->pix.cfg->uv_swap) {
> + if (RKISP1_IS_SEMI_PLANAR(cap->pix.cfg->write_format) &&
As this is the mp config path, I suppose write_format can only be
RKISP1_MI_CTRL_MP_WRITE_YUV_SPLA, right ? You could then write
if (cap->pix.cfg->write_format == RKISP1_MI_CTRL_MP_WRITE_YUV_SPLA &&
> + cap->pix.cfg->uv_swap) {
> reg = rkisp1_read(rkisp1, RKISP1_CIF_MI_XTD_FORMAT_CTRL);
>
> reg = reg | RKISP1_CIF_MI_XTD_FMT_CTRL_MP_CB_CR_SWAP;
> @@ -466,7 +471,8 @@ static void rkisp1_sp_config(struct rkisp1_capture *cap)
> rkisp1_write(rkisp1, cap->sp_y_stride, RKISP1_CIF_MI_SP_Y_LLENGTH);
>
> rkisp1_irq_frame_end_enable(cap);
> - if (cap->pix.cfg->uv_swap) {
> + if (RKISP1_IS_SEMI_PLANAR(cap->pix.cfg->write_format) &&
Same here with RKISP1_MI_CTRL_SP_WRITE_SPLA.
Another option would be to check the number of planes from
v4l2_format_info, which could lead to simpler to read code.
> + cap->pix.cfg->uv_swap) {
> u32 reg = rkisp1_read(rkisp1, RKISP1_CIF_MI_XTD_FORMAT_CTRL);
>
> reg = reg | RKISP1_CIF_MI_XTD_FMT_CTRL_SP_CB_CR_SWAP;
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 4/5] media: staging: rkisp1: cap: support uv swapped plane formats
2020-04-02 19:04 ` [PATCH v2 4/5] media: staging: rkisp1: cap: support uv swapped plane formats Dafna Hirschfeld
@ 2020-04-05 18:16 ` Laurent Pinchart
2020-04-06 11:56 ` Dafna Hirschfeld
0 siblings, 1 reply; 19+ messages in thread
From: Laurent Pinchart @ 2020-04-05 18:16 UTC (permalink / raw)
To: Dafna Hirschfeld
Cc: linux-media, helen.koike, ezequiel, hverkuil, kernel, dafna3,
linux-rockchip
Hi Dafna,
Thank you for the patch.
On Thu, Apr 02, 2020 at 09:04:18PM +0200, Dafna Hirschfeld wrote:
> Plane formats with the u and v planes swapped can be
> supported by changing the address of the cb and cr in
> the buffer.
>
> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
> Acked-by: Helen Koike <helen.koike@collabora.com>
> ---
> drivers/staging/media/rkisp1/rkisp1-capture.c | 17 +++++++++++++++++
> 1 file changed, 17 insertions(+)
>
> diff --git a/drivers/staging/media/rkisp1/rkisp1-capture.c b/drivers/staging/media/rkisp1/rkisp1-capture.c
> index fa2849209433..2d274e8f565b 100644
> --- a/drivers/staging/media/rkisp1/rkisp1-capture.c
> +++ b/drivers/staging/media/rkisp1/rkisp1-capture.c
> @@ -41,6 +41,10 @@
> (((write_format) == RKISP1_MI_CTRL_MP_WRITE_YUV_SPLA) || \
> ((write_format) == RKISP1_MI_CTRL_SP_WRITE_SPLA))
>
> +#define RKISP1_IS_PLANAR(write_format) \
> + (((write_format) == RKISP1_MI_CTRL_SP_WRITE_PLA) || \
> + ((write_format) == RKISP1_MI_CTRL_MP_WRITE_YUV_PLA_OR_RAW8))
> +
> enum rkisp1_plane {
> RKISP1_PLANE_Y = 0,
> RKISP1_PLANE_CB = 1,
> @@ -788,6 +792,19 @@ static void rkisp1_vb2_buf_queue(struct vb2_buffer *vb)
> rkisp1_pixfmt_comp_size(pixm, RKISP1_PLANE_CB);
> }
>
> + /*
> + * uv swap can be supported for plane formats by switching
> + * the address of cb and cr
> + */
> + if (RKISP1_IS_PLANAR(cap->pix.cfg->write_format) &&
As commented on patch 3/5, could this be checked from the data in
v4l2_format_info ?
> + cap->pix.cfg->uv_swap) {
> + ispbuf->buff_addr[RKISP1_PLANE_CR] =
> + ispbuf->buff_addr[RKISP1_PLANE_CB];
> + ispbuf->buff_addr[RKISP1_PLANE_CB] =
> + ispbuf->buff_addr[RKISP1_PLANE_CR] +
> + rkisp1_pixfmt_comp_size(pixm, RKISP1_PLANE_CR);
How about
swap(ispbuf->buff_addr[RKISP1_PLANE_CR],
ispbuf->buff_addr[RKISP1_PLANE_CB]);
?
> + }
> +
> spin_lock_irqsave(&cap->buf.lock, flags);
>
> /*
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 5/5] media: staging: rkisp1: cap: remove unsupported formats
2020-04-02 19:04 ` [PATCH v2 5/5] media: staging: rkisp1: cap: remove unsupported formats Dafna Hirschfeld
@ 2020-04-05 22:43 ` Laurent Pinchart
2020-04-06 12:42 ` Dafna Hirschfeld
0 siblings, 1 reply; 19+ messages in thread
From: Laurent Pinchart @ 2020-04-05 22:43 UTC (permalink / raw)
To: Dafna Hirschfeld
Cc: linux-media, helen.koike, ezequiel, hverkuil, kernel, dafna3,
linux-rockchip
Hi Dafna,
Thank you for the patch.
On Thu, Apr 02, 2020 at 09:04:19PM +0200, Dafna Hirschfeld wrote:
> For Ycbcr packed formats only YUYV can be supported by
> the driver. This patch removes the other formats.
The RKISP1_CIF_MI_BYTE_SWAP bit could possibly help achieving other YUV
orders, but as far as I can tell, it would affect both the main path and
the self path, so it wouldn't be very convenient. At a quick glance I
haven't found a way to support those formats, but just to make sure,
have you double-checked that the nv21_self and nv21_main bits of
MI_XTD_FORMAT_CTRL don't also affect packed mode ? If they don't,
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
> Acked-by: Helen Koike <helen.koike@collabora.com>
> ---
> drivers/staging/media/rkisp1/rkisp1-capture.c | 21 -------------------
> 1 file changed, 21 deletions(-)
>
> diff --git a/drivers/staging/media/rkisp1/rkisp1-capture.c b/drivers/staging/media/rkisp1/rkisp1-capture.c
> index 2d274e8f565b..076335193f40 100644
> --- a/drivers/staging/media/rkisp1/rkisp1-capture.c
> +++ b/drivers/staging/media/rkisp1/rkisp1-capture.c
> @@ -98,15 +98,6 @@ static const struct rkisp1_capture_fmt_cfg rkisp1_mp_fmts[] = {
> .fmt_type = RKISP1_FMT_YUV,
> .uv_swap = 0,
> .write_format = RKISP1_MI_CTRL_MP_WRITE_YUVINT,
> - }, {
> - .fourcc = V4L2_PIX_FMT_YVYU,
> - .fmt_type = RKISP1_FMT_YUV,
> - .uv_swap = 1,
> - .write_format = RKISP1_MI_CTRL_MP_WRITE_YUVINT,
> - }, {
> - .fourcc = V4L2_PIX_FMT_VYUY,
> - .fmt_type = RKISP1_FMT_YUV,
> - .write_format = RKISP1_MI_CTRL_MP_WRITE_YUVINT,
> }, {
> .fourcc = V4L2_PIX_FMT_YUV422P,
> .fmt_type = RKISP1_FMT_YUV,
> @@ -234,18 +225,6 @@ static const struct rkisp1_capture_fmt_cfg rkisp1_sp_fmts[] = {
> .uv_swap = 0,
> .write_format = RKISP1_MI_CTRL_SP_WRITE_INT,
> .output_format = RKISP1_MI_CTRL_SP_OUTPUT_YUV422,
> - }, {
> - .fourcc = V4L2_PIX_FMT_YVYU,
> - .fmt_type = RKISP1_FMT_YUV,
> - .uv_swap = 1,
> - .write_format = RKISP1_MI_CTRL_SP_WRITE_INT,
> - .output_format = RKISP1_MI_CTRL_SP_OUTPUT_YUV422,
> - }, {
> - .fourcc = V4L2_PIX_FMT_VYUY,
> - .fmt_type = RKISP1_FMT_YUV,
> - .uv_swap = 1,
> - .write_format = RKISP1_MI_CTRL_SP_WRITE_INT,
> - .output_format = RKISP1_MI_CTRL_SP_OUTPUT_YUV422,
> }, {
> .fourcc = V4L2_PIX_FMT_YUV422P,
> .fmt_type = RKISP1_FMT_YUV,
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 2/5] media: staging: rkisp1: cap: fix value written to uv swap register in selfpath
2020-04-05 18:10 ` Laurent Pinchart
@ 2020-04-06 11:20 ` Dafna Hirschfeld
0 siblings, 0 replies; 19+ messages in thread
From: Dafna Hirschfeld @ 2020-04-06 11:20 UTC (permalink / raw)
To: Laurent Pinchart
Cc: linux-media, helen.koike, ezequiel, hverkuil, kernel, dafna3,
linux-rockchip
On 4/5/20 8:10 PM, Laurent Pinchart wrote:
> Hi Dafna,
>
> Thank you for the patch.
>
> On Thu, Apr 02, 2020 at 09:04:16PM +0200, Dafna Hirschfeld wrote:
>> The value RKISP1_CIF_MI_XTD_FMT_CTRL_SP_CB_CR_SWAP should be
>> set to the register instead of masking with ~BIT(1)
>>
>> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
>> ---
>> drivers/staging/media/rkisp1/rkisp1-capture.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/staging/media/rkisp1/rkisp1-capture.c b/drivers/staging/media/rkisp1/rkisp1-capture.c
>> index 5700d7be2819..84a3cf565106 100644
>> --- a/drivers/staging/media/rkisp1/rkisp1-capture.c
>> +++ b/drivers/staging/media/rkisp1/rkisp1-capture.c
>> @@ -469,8 +469,8 @@ static void rkisp1_sp_config(struct rkisp1_capture *cap)
>> if (cap->pix.cfg->uv_swap) {
>> u32 reg = rkisp1_read(rkisp1, RKISP1_CIF_MI_XTD_FORMAT_CTRL);
>>
>> - rkisp1_write(rkisp1, reg & ~BIT(1),
>> - RKISP1_CIF_MI_XTD_FORMAT_CTRL);
>> + reg = reg | RKISP1_CIF_MI_XTD_FMT_CTRL_SP_CB_CR_SWAP;
>> + rkisp1_write(rkisp1, reg, RKISP1_CIF_MI_XTD_FORMAT_CTRL);
>
> This indeed simplifies the code, but I think the logic is wrong in the
> first place. Shouldn't this be
>
> reg = rkisp1_read(rkisp1, RKISP1_CIF_MI_XTD_FORMAT_CTRL);
> if (cap->pix.cfg->uv_swap)
> reg |= RKISP1_CIF_MI_XTD_FMT_CTRL_SP_CB_CR_SWAP;
> else
> reg &= ~RKISP1_CIF_MI_XTD_FMT_CTRL_SP_CB_CR_SWAP;
> rkisp1_write(rkisp1, reg, RKISP1_CIF_MI_XTD_FORMAT_CTRL);
>
> ? Same for patch 1/5.
Right, I'll send v3.
Thanks,
Dafna
>
>> }
>>
>> rkisp1_mi_config_ctrl(cap);
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 4/5] media: staging: rkisp1: cap: support uv swapped plane formats
2020-04-05 18:16 ` Laurent Pinchart
@ 2020-04-06 11:56 ` Dafna Hirschfeld
2020-04-06 12:27 ` Helen Koike
0 siblings, 1 reply; 19+ messages in thread
From: Dafna Hirschfeld @ 2020-04-06 11:56 UTC (permalink / raw)
To: Laurent Pinchart
Cc: linux-media, helen.koike, ezequiel, hverkuil, kernel, dafna3,
linux-rockchip
On 4/5/20 8:16 PM, Laurent Pinchart wrote:
> Hi Dafna,
>
> Thank you for the patch.
>
> On Thu, Apr 02, 2020 at 09:04:18PM +0200, Dafna Hirschfeld wrote:
>> Plane formats with the u and v planes swapped can be
>> supported by changing the address of the cb and cr in
>> the buffer.
>>
>> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
>> Acked-by: Helen Koike <helen.koike@collabora.com>
>> ---
>> drivers/staging/media/rkisp1/rkisp1-capture.c | 17 +++++++++++++++++
>> 1 file changed, 17 insertions(+)
>>
>> diff --git a/drivers/staging/media/rkisp1/rkisp1-capture.c b/drivers/staging/media/rkisp1/rkisp1-capture.c
>> index fa2849209433..2d274e8f565b 100644
>> --- a/drivers/staging/media/rkisp1/rkisp1-capture.c
>> +++ b/drivers/staging/media/rkisp1/rkisp1-capture.c
>> @@ -41,6 +41,10 @@
>> (((write_format) == RKISP1_MI_CTRL_MP_WRITE_YUV_SPLA) || \
>> ((write_format) == RKISP1_MI_CTRL_SP_WRITE_SPLA))
>>
>> +#define RKISP1_IS_PLANAR(write_format) \
>> + (((write_format) == RKISP1_MI_CTRL_SP_WRITE_PLA) || \
>> + ((write_format) == RKISP1_MI_CTRL_MP_WRITE_YUV_PLA_OR_RAW8))
>> +
>> enum rkisp1_plane {
>> RKISP1_PLANE_Y = 0,
>> RKISP1_PLANE_CB = 1,
>> @@ -788,6 +792,19 @@ static void rkisp1_vb2_buf_queue(struct vb2_buffer *vb)
>> rkisp1_pixfmt_comp_size(pixm, RKISP1_PLANE_CB);
>> }
>>
>> + /*
>> + * uv swap can be supported for plane formats by switching
>> + * the address of cb and cr
>> + */
>> + if (RKISP1_IS_PLANAR(cap->pix.cfg->write_format) &&
>
> As commented on patch 3/5, could this be checked from the data in
> v4l2_format_info ?
yes
>
>> + cap->pix.cfg->uv_swap) {
>> + ispbuf->buff_addr[RKISP1_PLANE_CR] =
>> + ispbuf->buff_addr[RKISP1_PLANE_CB];
>> + ispbuf->buff_addr[RKISP1_PLANE_CB] =
>> + ispbuf->buff_addr[RKISP1_PLANE_CR] +
>> + rkisp1_pixfmt_comp_size(pixm, RKISP1_PLANE_CR);
>
> How about
>
> swap(ispbuf->buff_addr[RKISP1_PLANE_CR],
> ispbuf->buff_addr[RKISP1_PLANE_CB]);
>
> ?
This also works, theoretically if there was a format where the Cb, Cr planes
are not equal size then a swap will not work.
Thanks,
Dafna
>
>> + }
>> +
>> spin_lock_irqsave(&cap->buf.lock, flags);
>>
>> /*
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 4/5] media: staging: rkisp1: cap: support uv swapped plane formats
2020-04-06 11:56 ` Dafna Hirschfeld
@ 2020-04-06 12:27 ` Helen Koike
2020-04-06 12:40 ` Dafna Hirschfeld
0 siblings, 1 reply; 19+ messages in thread
From: Helen Koike @ 2020-04-06 12:27 UTC (permalink / raw)
To: Dafna Hirschfeld, Laurent Pinchart
Cc: linux-media, ezequiel, hverkuil, kernel, dafna3, linux-rockchip
Hi,
On 4/6/20 8:56 AM, Dafna Hirschfeld wrote:
>
>
> On 4/5/20 8:16 PM, Laurent Pinchart wrote:
>> Hi Dafna,
>>
>> Thank you for the patch.
>>
>> On Thu, Apr 02, 2020 at 09:04:18PM +0200, Dafna Hirschfeld wrote:
>>> Plane formats with the u and v planes swapped can be
>>> supported by changing the address of the cb and cr in
>>> the buffer.
>>>
>>> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
>>> Acked-by: Helen Koike <helen.koike@collabora.com>
>>> ---
>>> drivers/staging/media/rkisp1/rkisp1-capture.c | 17 +++++++++++++++++
>>> 1 file changed, 17 insertions(+)
>>>
>>> diff --git a/drivers/staging/media/rkisp1/rkisp1-capture.c b/drivers/staging/media/rkisp1/rkisp1-capture.c
>>> index fa2849209433..2d274e8f565b 100644
>>> --- a/drivers/staging/media/rkisp1/rkisp1-capture.c
>>> +++ b/drivers/staging/media/rkisp1/rkisp1-capture.c
>>> @@ -41,6 +41,10 @@
>>> (((write_format) == RKISP1_MI_CTRL_MP_WRITE_YUV_SPLA) || \
>>> ((write_format) == RKISP1_MI_CTRL_SP_WRITE_SPLA))
>>> +#define RKISP1_IS_PLANAR(write_format) \
>>> + (((write_format) == RKISP1_MI_CTRL_SP_WRITE_PLA) || \
>>> + ((write_format) == RKISP1_MI_CTRL_MP_WRITE_YUV_PLA_OR_RAW8))
>>> +
>>> enum rkisp1_plane {
>>> RKISP1_PLANE_Y = 0,
>>> RKISP1_PLANE_CB = 1,
>>> @@ -788,6 +792,19 @@ static void rkisp1_vb2_buf_queue(struct vb2_buffer *vb)
>>> rkisp1_pixfmt_comp_size(pixm, RKISP1_PLANE_CB);
>>> }
>>> + /*
>>> + * uv swap can be supported for plane formats by switching
>>> + * the address of cb and cr
>>> + */
>>> + if (RKISP1_IS_PLANAR(cap->pix.cfg->write_format) &&
>>
>> As commented on patch 3/5, could this be checked from the data in
>> v4l2_format_info ?
> yes
>>
>>> + cap->pix.cfg->uv_swap) {
>>> + ispbuf->buff_addr[RKISP1_PLANE_CR] =
>>> + ispbuf->buff_addr[RKISP1_PLANE_CB];
>>> + ispbuf->buff_addr[RKISP1_PLANE_CB] =
>>> + ispbuf->buff_addr[RKISP1_PLANE_CR] +
>>> + rkisp1_pixfmt_comp_size(pixm, RKISP1_PLANE_CR);
Actually this is wrong if pixm->num_planes != 1, since they are different buffers.
>>
>> How about
>>
>> swap(ispbuf->buff_addr[RKISP1_PLANE_CR],
>> ispbuf->buff_addr[RKISP1_PLANE_CB]);
>>
>> ?
> This also works, theoretically if there was a format where the Cb, Cr planes
> are not equal size then a swap will not work.
If you check rkisp1_fill_pixfmt(), you'll see that they are equal size.
hdiv and vdiv applies to both.
Thank you Laurent for the review and thank you Dafna for working on this.
Regards,
Helen
>
> Thanks,
> Dafna
>>
>>> + }
>>> +
>>> spin_lock_irqsave(&cap->buf.lock, flags);
>>> /*
>>
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 4/5] media: staging: rkisp1: cap: support uv swapped plane formats
2020-04-06 12:27 ` Helen Koike
@ 2020-04-06 12:40 ` Dafna Hirschfeld
0 siblings, 0 replies; 19+ messages in thread
From: Dafna Hirschfeld @ 2020-04-06 12:40 UTC (permalink / raw)
To: Helen Koike, Laurent Pinchart
Cc: linux-media, ezequiel, hverkuil, kernel, dafna3, linux-rockchip
On 4/6/20 2:27 PM, Helen Koike wrote:
> Hi,
>
> On 4/6/20 8:56 AM, Dafna Hirschfeld wrote:
>>
>>
>> On 4/5/20 8:16 PM, Laurent Pinchart wrote:
>>> Hi Dafna,
>>>
>>> Thank you for the patch.
>>>
>>> On Thu, Apr 02, 2020 at 09:04:18PM +0200, Dafna Hirschfeld wrote:
>>>> Plane formats with the u and v planes swapped can be
>>>> supported by changing the address of the cb and cr in
>>>> the buffer.
>>>>
>>>> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
>>>> Acked-by: Helen Koike <helen.koike@collabora.com>
>>>> ---
>>>> drivers/staging/media/rkisp1/rkisp1-capture.c | 17 +++++++++++++++++
>>>> 1 file changed, 17 insertions(+)
>>>>
>>>> diff --git a/drivers/staging/media/rkisp1/rkisp1-capture.c b/drivers/staging/media/rkisp1/rkisp1-capture.c
>>>> index fa2849209433..2d274e8f565b 100644
>>>> --- a/drivers/staging/media/rkisp1/rkisp1-capture.c
>>>> +++ b/drivers/staging/media/rkisp1/rkisp1-capture.c
>>>> @@ -41,6 +41,10 @@
>>>> (((write_format) == RKISP1_MI_CTRL_MP_WRITE_YUV_SPLA) || \
>>>> ((write_format) == RKISP1_MI_CTRL_SP_WRITE_SPLA))
>>>> +#define RKISP1_IS_PLANAR(write_format) \
>>>> + (((write_format) == RKISP1_MI_CTRL_SP_WRITE_PLA) || \
>>>> + ((write_format) == RKISP1_MI_CTRL_MP_WRITE_YUV_PLA_OR_RAW8))
>>>> +
>>>> enum rkisp1_plane {
>>>> RKISP1_PLANE_Y = 0,
>>>> RKISP1_PLANE_CB = 1,
>>>> @@ -788,6 +792,19 @@ static void rkisp1_vb2_buf_queue(struct vb2_buffer *vb)
>>>> rkisp1_pixfmt_comp_size(pixm, RKISP1_PLANE_CB);
>>>> }
>>>> + /*
>>>> + * uv swap can be supported for plane formats by switching
>>>> + * the address of cb and cr
>>>> + */
>>>> + if (RKISP1_IS_PLANAR(cap->pix.cfg->write_format) &&
>>>
>>> As commented on patch 3/5, could this be checked from the data in
>>> v4l2_format_info ?
>> yes
>>>
>>>> + cap->pix.cfg->uv_swap) {
>>>> + ispbuf->buff_addr[RKISP1_PLANE_CR] =
>>>> + ispbuf->buff_addr[RKISP1_PLANE_CB];
>>>> + ispbuf->buff_addr[RKISP1_PLANE_CB] =
>>>> + ispbuf->buff_addr[RKISP1_PLANE_CR] +
>>>> + rkisp1_pixfmt_comp_size(pixm, RKISP1_PLANE_CR);
>
> Actually this is wrong if pixm->num_planes != 1, since they are different buffers.
Hi, right, I will change to swap
Thanks,
Dafna
>
>>>
>>> How about
>>>
>>> swap(ispbuf->buff_addr[RKISP1_PLANE_CR],
>>> ispbuf->buff_addr[RKISP1_PLANE_CB]);
>>>
>>> ?
>> This also works, theoretically if there was a format where the Cb, Cr planes
>> are not equal size then a swap will not work.
>
> If you check rkisp1_fill_pixfmt(), you'll see that they are equal size.
> hdiv and vdiv applies to both.
>
> Thank you Laurent for the review and thank you Dafna for working on this.
>
> Regards,
> Helen
>
>>
>> Thanks,
>> Dafna
>>>
>>>> + }
>>>> +
>>>> spin_lock_irqsave(&cap->buf.lock, flags);
>>>> /*
>>>
>>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 5/5] media: staging: rkisp1: cap: remove unsupported formats
2020-04-05 22:43 ` Laurent Pinchart
@ 2020-04-06 12:42 ` Dafna Hirschfeld
2020-04-08 10:24 ` Dafna Hirschfeld
0 siblings, 1 reply; 19+ messages in thread
From: Dafna Hirschfeld @ 2020-04-06 12:42 UTC (permalink / raw)
To: Laurent Pinchart
Cc: linux-media, helen.koike, ezequiel, hverkuil, kernel, dafna3,
linux-rockchip
On 4/6/20 12:43 AM, Laurent Pinchart wrote:
> Hi Dafna,
>
> Thank you for the patch.
>
> On Thu, Apr 02, 2020 at 09:04:19PM +0200, Dafna Hirschfeld wrote:
>> For Ycbcr packed formats only YUYV can be supported by
>> the driver. This patch removes the other formats.
>
> The RKISP1_CIF_MI_BYTE_SWAP bit could possibly help achieving other YUV
> orders, but as far as I can tell, it would affect both the main path and
> the self path, so it wouldn't be very convenient. At a quick glance I
> haven't found a way to support those formats, but just to make sure,
> have you double-checked that the nv21_self and nv21_main bits of
> MI_XTD_FORMAT_CTRL don't also affect packed mode ? If they don't,
Hi, thanks a lot for the reviews, I'll check that.
Dafna
>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>
>> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
>> Acked-by: Helen Koike <helen.koike@collabora.com>
>> ---
>> drivers/staging/media/rkisp1/rkisp1-capture.c | 21 -------------------
>> 1 file changed, 21 deletions(-)
>>
>> diff --git a/drivers/staging/media/rkisp1/rkisp1-capture.c b/drivers/staging/media/rkisp1/rkisp1-capture.c
>> index 2d274e8f565b..076335193f40 100644
>> --- a/drivers/staging/media/rkisp1/rkisp1-capture.c
>> +++ b/drivers/staging/media/rkisp1/rkisp1-capture.c
>> @@ -98,15 +98,6 @@ static const struct rkisp1_capture_fmt_cfg rkisp1_mp_fmts[] = {
>> .fmt_type = RKISP1_FMT_YUV,
>> .uv_swap = 0,
>> .write_format = RKISP1_MI_CTRL_MP_WRITE_YUVINT,
>> - }, {
>> - .fourcc = V4L2_PIX_FMT_YVYU,
>> - .fmt_type = RKISP1_FMT_YUV,
>> - .uv_swap = 1,
>> - .write_format = RKISP1_MI_CTRL_MP_WRITE_YUVINT,
>> - }, {
>> - .fourcc = V4L2_PIX_FMT_VYUY,
>> - .fmt_type = RKISP1_FMT_YUV,
>> - .write_format = RKISP1_MI_CTRL_MP_WRITE_YUVINT,
>> }, {
>> .fourcc = V4L2_PIX_FMT_YUV422P,
>> .fmt_type = RKISP1_FMT_YUV,
>> @@ -234,18 +225,6 @@ static const struct rkisp1_capture_fmt_cfg rkisp1_sp_fmts[] = {
>> .uv_swap = 0,
>> .write_format = RKISP1_MI_CTRL_SP_WRITE_INT,
>> .output_format = RKISP1_MI_CTRL_SP_OUTPUT_YUV422,
>> - }, {
>> - .fourcc = V4L2_PIX_FMT_YVYU,
>> - .fmt_type = RKISP1_FMT_YUV,
>> - .uv_swap = 1,
>> - .write_format = RKISP1_MI_CTRL_SP_WRITE_INT,
>> - .output_format = RKISP1_MI_CTRL_SP_OUTPUT_YUV422,
>> - }, {
>> - .fourcc = V4L2_PIX_FMT_VYUY,
>> - .fmt_type = RKISP1_FMT_YUV,
>> - .uv_swap = 1,
>> - .write_format = RKISP1_MI_CTRL_SP_WRITE_INT,
>> - .output_format = RKISP1_MI_CTRL_SP_OUTPUT_YUV422,
>> }, {
>> .fourcc = V4L2_PIX_FMT_YUV422P,
>> .fmt_type = RKISP1_FMT_YUV,
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 5/5] media: staging: rkisp1: cap: remove unsupported formats
2020-04-06 12:42 ` Dafna Hirschfeld
@ 2020-04-08 10:24 ` Dafna Hirschfeld
0 siblings, 0 replies; 19+ messages in thread
From: Dafna Hirschfeld @ 2020-04-08 10:24 UTC (permalink / raw)
To: Laurent Pinchart
Cc: linux-media, helen.koike, ezequiel, hverkuil, kernel, dafna3,
linux-rockchip
On 4/6/20 2:42 PM, Dafna Hirschfeld wrote:
>
>
> On 4/6/20 12:43 AM, Laurent Pinchart wrote:
>> Hi Dafna,
>>
>> Thank you for the patch.
>>
>> On Thu, Apr 02, 2020 at 09:04:19PM +0200, Dafna Hirschfeld wrote:
>>> For Ycbcr packed formats only YUYV can be supported by
>>> the driver. This patch removes the other formats.
>>
>> The RKISP1_CIF_MI_BYTE_SWAP bit could possibly help achieving other YUV
>> orders, but as far as I can tell, it would affect both the main path and
>> the self path, so it wouldn't be very convenient. At a quick glance I
>> haven't found a way to support those formats, but just to make sure,
>> have you double-checked that the nv21_self and nv21_main bits of
>> MI_XTD_FORMAT_CTRL don't also affect packed mode ? If they don't,
> Hi, thanks a lot for the reviews, I'll check that.
Hi, unfortunately MI_XTD_FORMAT_CTRL doesn't affect the the packed formats.
Dafna
>
> Dafna
>>
>> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>>
>>> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
>>> Acked-by: Helen Koike <helen.koike@collabora.com>
>>> ---
>>> drivers/staging/media/rkisp1/rkisp1-capture.c | 21 -------------------
>>> 1 file changed, 21 deletions(-)
>>>
>>> diff --git a/drivers/staging/media/rkisp1/rkisp1-capture.c b/drivers/staging/media/rkisp1/rkisp1-capture.c
>>> index 2d274e8f565b..076335193f40 100644
>>> --- a/drivers/staging/media/rkisp1/rkisp1-capture.c
>>> +++ b/drivers/staging/media/rkisp1/rkisp1-capture.c
>>> @@ -98,15 +98,6 @@ static const struct rkisp1_capture_fmt_cfg rkisp1_mp_fmts[] = {
>>> .fmt_type = RKISP1_FMT_YUV,
>>> .uv_swap = 0,
>>> .write_format = RKISP1_MI_CTRL_MP_WRITE_YUVINT,
>>> - }, {
>>> - .fourcc = V4L2_PIX_FMT_YVYU,
>>> - .fmt_type = RKISP1_FMT_YUV,
>>> - .uv_swap = 1,
>>> - .write_format = RKISP1_MI_CTRL_MP_WRITE_YUVINT,
>>> - }, {
>>> - .fourcc = V4L2_PIX_FMT_VYUY,
>>> - .fmt_type = RKISP1_FMT_YUV,
>>> - .write_format = RKISP1_MI_CTRL_MP_WRITE_YUVINT,
>>> }, {
>>> .fourcc = V4L2_PIX_FMT_YUV422P,
>>> .fmt_type = RKISP1_FMT_YUV,
>>> @@ -234,18 +225,6 @@ static const struct rkisp1_capture_fmt_cfg rkisp1_sp_fmts[] = {
>>> .uv_swap = 0,
>>> .write_format = RKISP1_MI_CTRL_SP_WRITE_INT,
>>> .output_format = RKISP1_MI_CTRL_SP_OUTPUT_YUV422,
>>> - }, {
>>> - .fourcc = V4L2_PIX_FMT_YVYU,
>>> - .fmt_type = RKISP1_FMT_YUV,
>>> - .uv_swap = 1,
>>> - .write_format = RKISP1_MI_CTRL_SP_WRITE_INT,
>>> - .output_format = RKISP1_MI_CTRL_SP_OUTPUT_YUV422,
>>> - }, {
>>> - .fourcc = V4L2_PIX_FMT_VYUY,
>>> - .fmt_type = RKISP1_FMT_YUV,
>>> - .uv_swap = 1,
>>> - .write_format = RKISP1_MI_CTRL_SP_WRITE_INT,
>>> - .output_format = RKISP1_MI_CTRL_SP_OUTPUT_YUV422,
>>> }, {
>>> .fourcc = V4L2_PIX_FMT_YUV422P,
>>> .fmt_type = RKISP1_FMT_YUV,
>>
^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2020-04-08 10:24 UTC | newest]
Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-02 19:04 [PATCH v2 0/5] media: staging: rkisp1: cap: various fixes for capture formats Dafna Hirschfeld
2020-04-02 19:04 ` [PATCH v2 1/5] media: staging: rkisp1: cap: cleanup in mainpath config for uv swap format Dafna Hirschfeld
2020-04-02 19:53 ` Helen Koike
2020-04-05 17:57 ` Laurent Pinchart
2020-04-02 19:04 ` [PATCH v2 2/5] media: staging: rkisp1: cap: fix value written to uv swap register in selfpath Dafna Hirschfeld
2020-04-02 19:53 ` Helen Koike
2020-04-05 18:10 ` Laurent Pinchart
2020-04-06 11:20 ` Dafna Hirschfeld
2020-04-02 19:04 ` [PATCH v2 3/5] media: staging: rkisp1: cap: support uv swap only for semiplanar formats Dafna Hirschfeld
2020-04-05 18:14 ` Laurent Pinchart
2020-04-02 19:04 ` [PATCH v2 4/5] media: staging: rkisp1: cap: support uv swapped plane formats Dafna Hirschfeld
2020-04-05 18:16 ` Laurent Pinchart
2020-04-06 11:56 ` Dafna Hirschfeld
2020-04-06 12:27 ` Helen Koike
2020-04-06 12:40 ` Dafna Hirschfeld
2020-04-02 19:04 ` [PATCH v2 5/5] media: staging: rkisp1: cap: remove unsupported formats Dafna Hirschfeld
2020-04-05 22:43 ` Laurent Pinchart
2020-04-06 12:42 ` Dafna Hirschfeld
2020-04-08 10:24 ` Dafna Hirschfeld
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).