From 9132979f840e8ec499e0e4d9d8d042e694c8d5b5 Mon Sep 17 00:00:00 2001 From: Nicolas Dufresne Date: Fri, 21 Mar 2014 17:15:17 -0400 Subject: [PATCH 1/6] s5p-fimc: Align imagesize to row size for tiled formats For tiled format, we need to allocated a multiple of the row size. A good example is for 1280x720, wich get adjusted to 1280x736. In tiles this mean Y plane is 20x23 and UV plane 20x12. Because of the rounding, the previous code would only have enough space to fit half of the last row. --- drivers/media/platform/s5p-fimc/fimc-core.c | 22 ++++++++++++--------- drivers/media/platform/s5p-fimc/fimc-m2m.c | 4 ++-- 2 files changed, 15 insertions(+), 11 deletions(-) diff --git a/drivers/media/platform/s5p-fimc/fimc-core.c b/drivers/media/platform/s5p-fimc/fimc-core.c index 8d6ba2ffc1432d..6e3a61f7d84645 100644 --- a/drivers/media/platform/s5p-fimc/fimc-core.c +++ b/drivers/media/platform/s5p-fimc/fimc-core.c @@ -712,13 +712,8 @@ int fimc_fill_format(struct fimc_frame *frame, struct v4l2_format *f) bpl = (bpl * frame->fmt->depth[0]) / 8; pixm->plane_fmt[i].bytesperline = bpl; - if (frame->fmt->flags & FMT_FLAGS_COMPRESSED) { - pixm->plane_fmt[i].sizeimage = frame->payload[i]; - continue; - } - pixm->plane_fmt[i].sizeimage = (frame->o_width * - frame->o_height * frame->fmt->depth[i]) / 8; - } + pixm->plane_fmt[i].sizeimage = frame->payload[i]; + } return 0; } @@ -761,6 +756,7 @@ void fimc_adjust_mplane_format(struct fimc_fmt *fmt, u32 width, u32 height, for (i = 0; i < pix->num_planes; ++i) { struct v4l2_plane_pix_format *plane_fmt = &pix->plane_fmt[i]; u32 bpl = plane_fmt->bytesperline; + u32 sizeimage; if (fmt->colplanes > 1 && (bpl == 0 || bpl < pix->width)) bpl = pix->width; /* Planar */ @@ -773,8 +769,16 @@ void fimc_adjust_mplane_format(struct fimc_fmt *fmt, u32 width, u32 height, bytesperline = bpl; plane_fmt->bytesperline = bytesperline; - plane_fmt->sizeimage = max((pix->width * pix->height * - fmt->depth[i]) / 8, plane_fmt->sizeimage); + sizeimage = pix->width * pix->height * fmt->depth[i] / 8; + + /* Ensure full row for tiled formats */ + if (tiled_fmt(fmt)) { + /* 64 * 32 * plane_fmt->bytesperline / 64 */ + u32 row_size = plane_fmt->bytesperline * 32; + sizeimage = ALIGN(sizeimage, row_size); + } + + plane_fmt->sizeimage = max(sizeimage, plane_fmt->sizeimage); } } diff --git a/drivers/media/platform/s5p-fimc/fimc-m2m.c b/drivers/media/platform/s5p-fimc/fimc-m2m.c index cef068874f5960..b9b7b1faecca1a 100644 --- a/drivers/media/platform/s5p-fimc/fimc-m2m.c +++ b/drivers/media/platform/s5p-fimc/fimc-m2m.c @@ -389,8 +389,8 @@ static int fimc_m2m_s_fmt_mplane(struct file *file, void *fh, fimc_alpha_ctrl_update(ctx); for (i = 0; i < frame->fmt->colplanes; i++) { - frame->payload[i] = - (pix->width * pix->height * frame->fmt->depth[i]) / 8; + struct v4l2_plane_pix_format *plane_fmt = &pix->plane_fmt[i]; + frame->payload[i] = plane_fmt->sizeimage; } fimc_fill_frame(frame, f); From b837b172cb81302e9fc9aa263362b085e5c7dab9 Mon Sep 17 00:00:00 2001 From: Nicolas Dufresne Date: Fri, 21 Mar 2014 19:49:31 -0400 Subject: [PATCH 2/6] s5p-fimc: Backport bytesperline calculation fix The supported planar YUV format (YUV420P and YUV422P) has 3 planes, where the bytesperline for each should be width, widht/2, width/2. It is expected that width has been aligned to a multiple of 4 to stay word aligned. --- drivers/media/platform/s5p-fimc/fimc-core.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/drivers/media/platform/s5p-fimc/fimc-core.c b/drivers/media/platform/s5p-fimc/fimc-core.c index 6e3a61f7d84645..f8da78ca665060 100644 --- a/drivers/media/platform/s5p-fimc/fimc-core.c +++ b/drivers/media/platform/s5p-fimc/fimc-core.c @@ -764,9 +764,16 @@ void fimc_adjust_mplane_format(struct fimc_fmt *fmt, u32 width, u32 height, if (fmt->colplanes == 1 && /* Packed */ (bpl == 0 || ((bpl * 8) / fmt->depth[i]) < pix->width)) bpl = (pix->width * fmt->depth[0]) / 8; - - if (i == 0) /* Same bytesperline for each plane. */ + /* + * Currently bytesperline for each plane is same, except + * V4L2_PIX_FMT_YUV420M format. This calculation may need + * to be changed when other multi-planar formats are added + * to the fimc_formats[] array. + */ + if (i == 0) bytesperline = bpl; + else if (i == 1 && fmt->memplanes == 3) + bytesperline /= 2; plane_fmt->bytesperline = bytesperline; sizeimage = pix->width * pix->height * fmt->depth[i] / 8; From e6ebb310063788fd420c329eb8037cd7f58276fe Mon Sep 17 00:00:00 2001 From: Nicolas Dufresne Date: Tue, 25 Mar 2014 11:08:19 -0400 Subject: [PATCH 3/6] s5p-fimc: Iterate for each memory plane Depth and payload is defined per memory plane. It's better to iterate using number of memory planes. this was not causing much issue since the rest of the arrays involved where intialized to zero. --- drivers/media/platform/s5p-fimc/fimc-core.c | 2 +- drivers/media/platform/s5p-fimc/fimc-m2m.c | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/media/platform/s5p-fimc/fimc-core.c b/drivers/media/platform/s5p-fimc/fimc-core.c index f8da78ca665060..67e1217fa61b51 100644 --- a/drivers/media/platform/s5p-fimc/fimc-core.c +++ b/drivers/media/platform/s5p-fimc/fimc-core.c @@ -446,7 +446,7 @@ void fimc_prepare_dma_offset(struct fimc_ctx *ctx, struct fimc_frame *f) struct fimc_variant *variant = ctx->fimc_dev->variant; u32 i, depth = 0; - for (i = 0; i < f->fmt->colplanes; i++) + for (i = 0; i < f->fmt->memplanes; i++) depth += f->fmt->depth[i]; f->dma_offset.y_h = f->offs_h; diff --git a/drivers/media/platform/s5p-fimc/fimc-m2m.c b/drivers/media/platform/s5p-fimc/fimc-m2m.c index b9b7b1faecca1a..7ab0a4c291171c 100644 --- a/drivers/media/platform/s5p-fimc/fimc-m2m.c +++ b/drivers/media/platform/s5p-fimc/fimc-m2m.c @@ -388,7 +388,7 @@ static int fimc_m2m_s_fmt_mplane(struct file *file, void *fh, /* Update RGB Alpha control state and value range */ fimc_alpha_ctrl_update(ctx); - for (i = 0; i < frame->fmt->colplanes; i++) { + for (i = 0; i < frame->fmt->memplanes; i++) { struct v4l2_plane_pix_format *plane_fmt = &pix->plane_fmt[i]; frame->payload[i] = plane_fmt->sizeimage; } @@ -536,7 +536,7 @@ static int fimc_m2m_try_crop(struct fimc_ctx *ctx, struct v4l2_crop *cr) else halign = ffs(fimc->variant->min_vsize_align) - 1; - for (i = 0; i < f->fmt->colplanes; i++) + for (i = 0; i < f->fmt->memplanes; i++) depth += f->fmt->depth[i]; v4l_bound_align_image(&cr->c.width, min_size, f->o_width, From d74e34f41ca9b4f12ea3c2baaaad318e119eac6f Mon Sep 17 00:00:00 2001 From: Nicolas Dufresne Date: Tue, 25 Mar 2014 11:10:27 -0400 Subject: [PATCH 4/6] s5p-fimc: Fix YUV422P depth All YUV 422 has 16bit per pixels. --- drivers/media/platform/s5p-fimc/fimc-core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/platform/s5p-fimc/fimc-core.c b/drivers/media/platform/s5p-fimc/fimc-core.c index 67e1217fa61b51..e19153f16722ed 100644 --- a/drivers/media/platform/s5p-fimc/fimc-core.c +++ b/drivers/media/platform/s5p-fimc/fimc-core.c @@ -118,7 +118,7 @@ static struct fimc_fmt fimc_formats[] = { }, { .name = "YUV 4:2:2 planar, Y/Cb/Cr", .fourcc = V4L2_PIX_FMT_YUV422P, - .depth = { 12 }, + .depth = { 16 }, .color = FIMC_FMT_YCBYCR422, .memplanes = 1, .colplanes = 3, From 74bfadd9b438f417c8862569d6bb55db6a38b4cc Mon Sep 17 00:00:00 2001 From: Nicolas Dufresne Date: Tue, 25 Mar 2014 11:12:47 -0400 Subject: [PATCH 5/6] s5p-fimc: Reuse calculated sizes This formula did not take into account the required tiled alignement for NV12MT format. As this was already computed an store in payload array initially, simply reuse that value. --- drivers/media/platform/s5p-fimc/fimc-m2m.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/platform/s5p-fimc/fimc-m2m.c b/drivers/media/platform/s5p-fimc/fimc-m2m.c index 7ab0a4c291171c..a39eda38420063 100644 --- a/drivers/media/platform/s5p-fimc/fimc-m2m.c +++ b/drivers/media/platform/s5p-fimc/fimc-m2m.c @@ -193,7 +193,7 @@ static int fimc_queue_setup(struct vb2_queue *vq, const struct v4l2_format *fmt, *num_planes = f->fmt->memplanes; for (i = 0; i < f->fmt->memplanes; i++) { - sizes[i] = (f->f_width * f->f_height * f->fmt->depth[i]) / 8; + sizes[i] = f->payload[i]; allocators[i] = ctx->fimc_dev->alloc_ctx; } return 0; From c4c988939370b677a18c2346ed9ae25cc361e02d Mon Sep 17 00:00:00 2001 From: Nicolas Dufresne Date: Tue, 25 Mar 2014 11:14:08 -0400 Subject: [PATCH 6/6] s5p-fimc: Don't allow application to choose the buffer size This is not allowed by the spec, and only served the purpose of hiding bugs in size calculation so far. --- drivers/media/platform/s5p-fimc/fimc-core.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/media/platform/s5p-fimc/fimc-core.c b/drivers/media/platform/s5p-fimc/fimc-core.c index e19153f16722ed..8cdb2a723199f4 100644 --- a/drivers/media/platform/s5p-fimc/fimc-core.c +++ b/drivers/media/platform/s5p-fimc/fimc-core.c @@ -785,7 +785,8 @@ void fimc_adjust_mplane_format(struct fimc_fmt *fmt, u32 width, u32 height, sizeimage = ALIGN(sizeimage, row_size); } - plane_fmt->sizeimage = max(sizeimage, plane_fmt->sizeimage); + /* Spec does not allow to using app proposed size */ + plane_fmt->sizeimage = plane_fmt->sizeimage; } }