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@xxxxxxxxxxxxx>
> Acked-by: Helen Koike <helen.koike@xxxxxxxxxxxxx>
> ---
> 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
