diff mbox series

[v3] media: mtk-vcodec: Align width and height to 64 bytes

Message ID 20211104122426.9597-1-yunfei.dong@mediatek.com (mailing list archive)
State New, archived
Headers show
Series [v3] media: mtk-vcodec: Align width and height to 64 bytes | expand

Commit Message

Yunfei Dong Nov. 4, 2021, 12:24 p.m. UTC 
Width and height need to 64 bytes aligned when setting the format.
Need to make sure all is 64 bytes align when use width and height to
calculate buffer size.

Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>
Acked-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
Tested-by: Steve Cho <stevecho@chromium.org>
---
 drivers/media/platform/mtk-vcodec/mtk_vcodec_dec.h        | 1 +
 drivers/media/platform/mtk-vcodec/vdec/vdec_h264_req_if.c | 4 ++--
 2 files changed, 3 insertions(+), 2 deletions(-)

Comments

Dafna Hirschfeld Nov. 22, 2021, 7:11 a.m. UTC | #1 
On 04.11.21 14:24, Yunfei Dong wrote:
> Width and height need to 64 bytes aligned when setting the format.
> Need to make sure all is 64 bytes align when use width and height to
> calculate buffer size.
> 
> Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>
> Acked-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
> Tested-by: Steve Cho <stevecho@chromium.org>
> ---
>   drivers/media/platform/mtk-vcodec/mtk_vcodec_dec.h        | 1 +
>   drivers/media/platform/mtk-vcodec/vdec/vdec_h264_req_if.c | 4 ++--
>   2 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec.h b/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec.h
> index e30806c1faea..66cd6d2242c3 100644
> --- a/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec.h
> +++ b/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec.h
> @@ -11,6 +11,7 @@
>   #include <media/videobuf2-core.h>
>   #include <media/v4l2-mem2mem.h>
>   
> +#define VCODEC_DEC_ALIGNED_64 64

This define is a bit useless, it doesn't improve readability,
if you do ALIGN(x, 64) it is already clear enough.

Thank,
Dafna

>   #define VCODEC_CAPABILITY_4K_DISABLED	0x10
>   #define VCODEC_DEC_4K_CODED_WIDTH	4096U
>   #define VCODEC_DEC_4K_CODED_HEIGHT	2304U
> diff --git a/drivers/media/platform/mtk-vcodec/vdec/vdec_h264_req_if.c b/drivers/media/platform/mtk-vcodec/vdec/vdec_h264_req_if.c
> index d402fc4bda69..e1a3011772a9 100644
> --- a/drivers/media/platform/mtk-vcodec/vdec/vdec_h264_req_if.c
> +++ b/drivers/media/platform/mtk-vcodec/vdec/vdec_h264_req_if.c
> @@ -562,8 +562,8 @@ static void get_pic_info(struct vdec_h264_slice_inst *inst,
>   {
>   	struct mtk_vcodec_ctx *ctx = inst->ctx;
>   
> -	ctx->picinfo.buf_w = (ctx->picinfo.pic_w + 15) & 0xFFFFFFF0;
> -	ctx->picinfo.buf_h = (ctx->picinfo.pic_h + 31) & 0xFFFFFFE0;
> +	ctx->picinfo.buf_w = ALIGN(ctx->picinfo.pic_w, VCODEC_DEC_ALIGNED_64);
> +	ctx->picinfo.buf_h = ALIGN(ctx->picinfo.pic_h, VCODEC_DEC_ALIGNED_64);
>   	ctx->picinfo.fb_sz[0] = ctx->picinfo.buf_w * ctx->picinfo.buf_h;
>   	ctx->picinfo.fb_sz[1] = ctx->picinfo.fb_sz[0] >> 1;
>   	inst->vsi_ctx.dec.cap_num_planes =
>
Steve Cho Nov. 22, 2021, 6:25 p.m. UTC | #2 
Thank you Dafna for providing the feedback.

Yes, I agree that ALIGN(x, 64) is clear enough.
I forgot the history, but I think it was probably one of my comment before.
For this case, it is also only used in one place, so not much benefit in
that sense as well.
(Unless it is planned to be used in many other places.)

Yunfei, either way is fine with me.
Wasn't this change was already merged in media stage though?

Thanks,
Steve

On Sun, Nov 21, 2021 at 11:11 PM Dafna Hirschfeld <
dafna.hirschfeld@collabora.com> wrote:

>
>
> On 04.11.21 14:24, Yunfei Dong wrote:
> > Width and height need to 64 bytes aligned when setting the format.
> > Need to make sure all is 64 bytes align when use width and height to
> > calculate buffer size.
> >
> > Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>
> > Acked-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
> > Tested-by: Steve Cho <stevecho@chromium.org>
> > ---
> >   drivers/media/platform/mtk-vcodec/mtk_vcodec_dec.h        | 1 +
> >   drivers/media/platform/mtk-vcodec/vdec/vdec_h264_req_if.c | 4 ++--
> >   2 files changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec.h
> b/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec.h
> > index e30806c1faea..66cd6d2242c3 100644
> > --- a/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec.h
> > +++ b/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec.h
> > @@ -11,6 +11,7 @@
> >   #include <media/videobuf2-core.h>
> >   #include <media/v4l2-mem2mem.h>
> >
> > +#define VCODEC_DEC_ALIGNED_64 64
>
> This define is a bit useless, it doesn't improve readability,
> if you do ALIGN(x, 64) it is already clear enough.
>
> Thank,
> Dafna
>
> >   #define VCODEC_CAPABILITY_4K_DISABLED       0x10
> >   #define VCODEC_DEC_4K_CODED_WIDTH   4096U
> >   #define VCODEC_DEC_4K_CODED_HEIGHT  2304U
> > diff --git a/drivers/media/platform/mtk-vcodec/vdec/vdec_h264_req_if.c
> b/drivers/media/platform/mtk-vcodec/vdec/vdec_h264_req_if.c
> > index d402fc4bda69..e1a3011772a9 100644
> > --- a/drivers/media/platform/mtk-vcodec/vdec/vdec_h264_req_if.c
> > +++ b/drivers/media/platform/mtk-vcodec/vdec/vdec_h264_req_if.c
> > @@ -562,8 +562,8 @@ static void get_pic_info(struct vdec_h264_slice_inst
> *inst,
> >   {
> >       struct mtk_vcodec_ctx *ctx = inst->ctx;
> >
> > -     ctx->picinfo.buf_w = (ctx->picinfo.pic_w + 15) & 0xFFFFFFF0;
> > -     ctx->picinfo.buf_h = (ctx->picinfo.pic_h + 31) & 0xFFFFFFE0;
> > +     ctx->picinfo.buf_w = ALIGN(ctx->picinfo.pic_w,
> VCODEC_DEC_ALIGNED_64);
> > +     ctx->picinfo.buf_h = ALIGN(ctx->picinfo.pic_h,
> VCODEC_DEC_ALIGNED_64);
> >       ctx->picinfo.fb_sz[0] = ctx->picinfo.buf_w * ctx->picinfo.buf_h;
> >       ctx->picinfo.fb_sz[1] = ctx->picinfo.fb_sz[0] >> 1;
> >       inst->vsi_ctx.dec.cap_num_planes =
> >
>
diff mbox series

Patch

diff --git a/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec.h b/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec.h
index e30806c1faea..66cd6d2242c3 100644
--- a/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec.h
+++ b/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec.h
@@ -11,6 +11,7 @@ 
 #include <media/videobuf2-core.h>
 #include <media/v4l2-mem2mem.h>
 
+#define VCODEC_DEC_ALIGNED_64 64
 #define VCODEC_CAPABILITY_4K_DISABLED	0x10
 #define VCODEC_DEC_4K_CODED_WIDTH	4096U
 #define VCODEC_DEC_4K_CODED_HEIGHT	2304U
diff --git a/drivers/media/platform/mtk-vcodec/vdec/vdec_h264_req_if.c b/drivers/media/platform/mtk-vcodec/vdec/vdec_h264_req_if.c
index d402fc4bda69..e1a3011772a9 100644
--- a/drivers/media/platform/mtk-vcodec/vdec/vdec_h264_req_if.c
+++ b/drivers/media/platform/mtk-vcodec/vdec/vdec_h264_req_if.c
@@ -562,8 +562,8 @@  static void get_pic_info(struct vdec_h264_slice_inst *inst,
 {
 	struct mtk_vcodec_ctx *ctx = inst->ctx;
 
-	ctx->picinfo.buf_w = (ctx->picinfo.pic_w + 15) & 0xFFFFFFF0;
-	ctx->picinfo.buf_h = (ctx->picinfo.pic_h + 31) & 0xFFFFFFE0;
+	ctx->picinfo.buf_w = ALIGN(ctx->picinfo.pic_w, VCODEC_DEC_ALIGNED_64);
+	ctx->picinfo.buf_h = ALIGN(ctx->picinfo.pic_h, VCODEC_DEC_ALIGNED_64);
 	ctx->picinfo.fb_sz[0] = ctx->picinfo.buf_w * ctx->picinfo.buf_h;
 	ctx->picinfo.fb_sz[1] = ctx->picinfo.fb_sz[0] >> 1;
 	inst->vsi_ctx.dec.cap_num_planes =