Re: [Patch v5 04/12] [media] s5p-mfc: Support MFCv10.10 buffer requirements

2017-06-27 Thread Smitha T Murthy
On Tue, 2017-06-27 at 22:30 +0200, Kamil Debski wrote:
> Hi,
> 
> Please find my comments inline.
> 
> On 19 June 2017 at 07:10, Smitha T Murthy  wrote:
> > Aligning the luma_dpb_size, chroma_dpb_size, mv_size and me_buffer_size
> > for MFCv10.10.
> >
> > Signed-off-by: Smitha T Murthy 
> > Reviewed-by: Andrzej Hajda 
> > ---
> >  drivers/media/platform/s5p-mfc/regs-mfc-v10.h   | 19 +
> >  drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c | 95 
> > +++--
> >  drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.h |  2 +
> >  3 files changed, 95 insertions(+), 21 deletions(-)
> >
> > diff --git a/drivers/media/platform/s5p-mfc/regs-mfc-v10.h 
> > b/drivers/media/platform/s5p-mfc/regs-mfc-v10.h
> > index 1ca09d6..3f0dab3 100644
> > --- a/drivers/media/platform/s5p-mfc/regs-mfc-v10.h
> > +++ b/drivers/media/platform/s5p-mfc/regs-mfc-v10.h
> > @@ -32,5 +32,24 @@
> >  #define MFC_VERSION_V100xA0
> >  #define MFC_NUM_PORTS_V10  1
> >
> > +/* MFCv10 codec defines*/
> > +#define S5P_FIMV_CODEC_HEVC_ENC 26
> > +
> > +/* Encoder buffer size for MFC v10.0 */
> > +#define ENC_V100_BASE_SIZE(x, y) \
> > +   (((x + 3) * (y + 3) * 8) \
> > +   +  ((y * 64) + 1280) * DIV_ROUND_UP(x, 8))
> > +
> > +#define ENC_V100_H264_ME_SIZE(x, y) \
> > +   (ENC_V100_BASE_SIZE(x, y) \
> > +   + (DIV_ROUND_UP(x * y, 64) * 32))
> > +
> > +#define ENC_V100_MPEG4_ME_SIZE(x, y) \
> > +   (ENC_V100_BASE_SIZE(x, y) \
> > +   + (DIV_ROUND_UP(x * y, 128) * 16))
> > +
> > +#define ENC_V100_VP8_ME_SIZE(x, y) \
> > +   ENC_V100_BASE_SIZE(x, y)
> > +
> >  #endif /*_REGS_MFC_V10_H*/
> >
> > diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c 
> > b/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c
> > index f1a8c53..83ea733 100644
> > --- a/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c
> > +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c
> > @@ -64,6 +64,7 @@ static int s5p_mfc_alloc_codec_buffers_v6(struct 
> > s5p_mfc_ctx *ctx)
> >  {
> > struct s5p_mfc_dev *dev = ctx->dev;
> > unsigned int mb_width, mb_height;
> > +   unsigned int lcu_width = 0, lcu_height = 0;
> > int ret;
> >
> > mb_width = MB_WIDTH(ctx->img_width);
> > @@ -74,7 +75,9 @@ static int s5p_mfc_alloc_codec_buffers_v6(struct 
> > s5p_mfc_ctx *ctx)
> >   ctx->luma_size, ctx->chroma_size, ctx->mv_size);
> > mfc_debug(2, "Totals bufs: %d\n", ctx->total_dpb_count);
> > } else if (ctx->type == MFCINST_ENCODER) {
> > -   if (IS_MFCV8_PLUS(dev))
> > +   if (IS_MFCV10(dev)) {IZE_V10  (15 * SZ_1K)
> > +   ctx->tmv_buffer_size = 0;
> 
> It would look much better to surround the above with braces, even
> though it's only a single line.
> 
I will add the braces in the next version.

> > +   } else if (IS_MFCV8_PLUS(dev))
> > ctx->tmv_buffer_size = S5P_FIMV_NUM_TMV_BUFFERS_V6 *
> > ALIGN(S5P_FIMV_TMV_BUFFER_SIZE_V8(mb_width, 
> > mb_height),
> > S5P_FIMV_TMV_BUFFER_ALIGN_V6);
> > @@ -82,13 +85,36 @@ static int s5p_mfc_alloc_codec_buffers_v6(struct 
> > s5p_mfc_ctx *ctx)
> > ctx->tmv_buffer_size = S5P_FIMV_NUM_TMV_BUFFERS_V6 *
> > ALIGN(S5P_FIMV_TMV_BUFFER_SIZE_V6(mb_width, 
> > mb_height),
> > S5P_FIMV_TMV_BUFFER_ALIGN_V6);
> > -
> > -   ctx->luma_dpb_size = ALIGN((mb_width * mb_height) *
> > -   S5P_FIMV_LUMA_MB_TO_PIXEL_V6,
> > -   S5P_FIMV_LUMA_DPB_BUFFER_ALIGN_V6);
> > -   ctx->chroma_dpb_size = ALIGN((mb_width * mb_height) *
> > -   S5P_FIMV_CHROMA_MB_TO_PIXEL_V6,
> > -   S5P_FIMV_CHROMA_DPB_BUFFER_ALIGN_V6);
> > +   if (IS_MFCV10(dev)) {
> > +   lcu_width = enc_lcu_width(ctx->img_width);
> > +   lcu_height = enc_lcu_height(ctx->img_height);
> > +   if (ctx->codec_mode != S5P_FIMV_CODEC_HEVC_ENC) {
> > +   ctx->luma_dpb_size =
> > +   ALIGN((mb_width * 16), 64)
> > +   * ALIGN((mb_height * 16), 32)
> > +   + 64;
> > +   ctx->chroma_dpb_size =
> > +   ALIGN((mb_width * 16), 64)
> > +   * (mb_height * 8)
> > +   + 64;
> > +   } else {
> > +   ctx->luma_dpb_size =
> > +   ALIGN((lcu_width * 32), 64)
> > +   * ALIGN((lcu_height * 32), 32)
> > 

Re: [Patch v5 04/12] [media] s5p-mfc: Support MFCv10.10 buffer requirements

2017-06-27 Thread Kamil Debski
Hi,

Please find my comments inline.

On 19 June 2017 at 07:10, Smitha T Murthy  wrote:
> Aligning the luma_dpb_size, chroma_dpb_size, mv_size and me_buffer_size
> for MFCv10.10.
>
> Signed-off-by: Smitha T Murthy 
> Reviewed-by: Andrzej Hajda 
> ---
>  drivers/media/platform/s5p-mfc/regs-mfc-v10.h   | 19 +
>  drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c | 95 
> +++--
>  drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.h |  2 +
>  3 files changed, 95 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/media/platform/s5p-mfc/regs-mfc-v10.h 
> b/drivers/media/platform/s5p-mfc/regs-mfc-v10.h
> index 1ca09d6..3f0dab3 100644
> --- a/drivers/media/platform/s5p-mfc/regs-mfc-v10.h
> +++ b/drivers/media/platform/s5p-mfc/regs-mfc-v10.h
> @@ -32,5 +32,24 @@
>  #define MFC_VERSION_V100xA0
>  #define MFC_NUM_PORTS_V10  1
>
> +/* MFCv10 codec defines*/
> +#define S5P_FIMV_CODEC_HEVC_ENC 26
> +
> +/* Encoder buffer size for MFC v10.0 */
> +#define ENC_V100_BASE_SIZE(x, y) \
> +   (((x + 3) * (y + 3) * 8) \
> +   +  ((y * 64) + 1280) * DIV_ROUND_UP(x, 8))
> +
> +#define ENC_V100_H264_ME_SIZE(x, y) \
> +   (ENC_V100_BASE_SIZE(x, y) \
> +   + (DIV_ROUND_UP(x * y, 64) * 32))
> +
> +#define ENC_V100_MPEG4_ME_SIZE(x, y) \
> +   (ENC_V100_BASE_SIZE(x, y) \
> +   + (DIV_ROUND_UP(x * y, 128) * 16))
> +
> +#define ENC_V100_VP8_ME_SIZE(x, y) \
> +   ENC_V100_BASE_SIZE(x, y)
> +
>  #endif /*_REGS_MFC_V10_H*/
>
> diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c 
> b/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c
> index f1a8c53..83ea733 100644
> --- a/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c
> +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c
> @@ -64,6 +64,7 @@ static int s5p_mfc_alloc_codec_buffers_v6(struct 
> s5p_mfc_ctx *ctx)
>  {
> struct s5p_mfc_dev *dev = ctx->dev;
> unsigned int mb_width, mb_height;
> +   unsigned int lcu_width = 0, lcu_height = 0;
> int ret;
>
> mb_width = MB_WIDTH(ctx->img_width);
> @@ -74,7 +75,9 @@ static int s5p_mfc_alloc_codec_buffers_v6(struct 
> s5p_mfc_ctx *ctx)
>   ctx->luma_size, ctx->chroma_size, ctx->mv_size);
> mfc_debug(2, "Totals bufs: %d\n", ctx->total_dpb_count);
> } else if (ctx->type == MFCINST_ENCODER) {
> -   if (IS_MFCV8_PLUS(dev))
> +   if (IS_MFCV10(dev)) {IZE_V10  (15 * SZ_1K)
> +   ctx->tmv_buffer_size = 0;

It would look much better to surround the above with braces, even
though it's only a single line.

> +   } else if (IS_MFCV8_PLUS(dev))
> ctx->tmv_buffer_size = S5P_FIMV_NUM_TMV_BUFFERS_V6 *
> ALIGN(S5P_FIMV_TMV_BUFFER_SIZE_V8(mb_width, 
> mb_height),
> S5P_FIMV_TMV_BUFFER_ALIGN_V6);
> @@ -82,13 +85,36 @@ static int s5p_mfc_alloc_codec_buffers_v6(struct 
> s5p_mfc_ctx *ctx)
> ctx->tmv_buffer_size = S5P_FIMV_NUM_TMV_BUFFERS_V6 *
> ALIGN(S5P_FIMV_TMV_BUFFER_SIZE_V6(mb_width, 
> mb_height),
> S5P_FIMV_TMV_BUFFER_ALIGN_V6);
> -
> -   ctx->luma_dpb_size = ALIGN((mb_width * mb_height) *
> -   S5P_FIMV_LUMA_MB_TO_PIXEL_V6,
> -   S5P_FIMV_LUMA_DPB_BUFFER_ALIGN_V6);
> -   ctx->chroma_dpb_size = ALIGN((mb_width * mb_height) *
> -   S5P_FIMV_CHROMA_MB_TO_PIXEL_V6,
> -   S5P_FIMV_CHROMA_DPB_BUFFER_ALIGN_V6);
> +   if (IS_MFCV10(dev)) {
> +   lcu_width = enc_lcu_width(ctx->img_width);
> +   lcu_height = enc_lcu_height(ctx->img_height);
> +   if (ctx->codec_mode != S5P_FIMV_CODEC_HEVC_ENC) {
> +   ctx->luma_dpb_size =
> +   ALIGN((mb_width * 16), 64)
> +   * ALIGN((mb_height * 16), 32)
> +   + 64;
> +   ctx->chroma_dpb_size =
> +   ALIGN((mb_width * 16), 64)
> +   * (mb_height * 8)
> +   + 64;
> +   } else {
> +   ctx->luma_dpb_size =
> +   ALIGN((lcu_width * 32), 64)
> +   * ALIGN((lcu_height * 32), 32)
> +   + 64;
> +   ctx->chroma_dpb_size =
> +   ALIGN((lcu_width * 32), 64)
> +   * (lcu_height * 16)
> + 

[Patch v5 04/12] [media] s5p-mfc: Support MFCv10.10 buffer requirements

2017-06-18 Thread Smitha T Murthy
Aligning the luma_dpb_size, chroma_dpb_size, mv_size and me_buffer_size
for MFCv10.10.

Signed-off-by: Smitha T Murthy 
Reviewed-by: Andrzej Hajda 
---
 drivers/media/platform/s5p-mfc/regs-mfc-v10.h   | 19 +
 drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c | 95 +++--
 drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.h |  2 +
 3 files changed, 95 insertions(+), 21 deletions(-)

diff --git a/drivers/media/platform/s5p-mfc/regs-mfc-v10.h 
b/drivers/media/platform/s5p-mfc/regs-mfc-v10.h
index 1ca09d6..3f0dab3 100644
--- a/drivers/media/platform/s5p-mfc/regs-mfc-v10.h
+++ b/drivers/media/platform/s5p-mfc/regs-mfc-v10.h
@@ -32,5 +32,24 @@
 #define MFC_VERSION_V100xA0
 #define MFC_NUM_PORTS_V10  1
 
+/* MFCv10 codec defines*/
+#define S5P_FIMV_CODEC_HEVC_ENC 26
+
+/* Encoder buffer size for MFC v10.0 */
+#define ENC_V100_BASE_SIZE(x, y) \
+   (((x + 3) * (y + 3) * 8) \
+   +  ((y * 64) + 1280) * DIV_ROUND_UP(x, 8))
+
+#define ENC_V100_H264_ME_SIZE(x, y) \
+   (ENC_V100_BASE_SIZE(x, y) \
+   + (DIV_ROUND_UP(x * y, 64) * 32))
+
+#define ENC_V100_MPEG4_ME_SIZE(x, y) \
+   (ENC_V100_BASE_SIZE(x, y) \
+   + (DIV_ROUND_UP(x * y, 128) * 16))
+
+#define ENC_V100_VP8_ME_SIZE(x, y) \
+   ENC_V100_BASE_SIZE(x, y)
+
 #endif /*_REGS_MFC_V10_H*/
 
diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c 
b/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c
index f1a8c53..83ea733 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c
@@ -64,6 +64,7 @@ static int s5p_mfc_alloc_codec_buffers_v6(struct s5p_mfc_ctx 
*ctx)
 {
struct s5p_mfc_dev *dev = ctx->dev;
unsigned int mb_width, mb_height;
+   unsigned int lcu_width = 0, lcu_height = 0;
int ret;
 
mb_width = MB_WIDTH(ctx->img_width);
@@ -74,7 +75,9 @@ static int s5p_mfc_alloc_codec_buffers_v6(struct s5p_mfc_ctx 
*ctx)
  ctx->luma_size, ctx->chroma_size, ctx->mv_size);
mfc_debug(2, "Totals bufs: %d\n", ctx->total_dpb_count);
} else if (ctx->type == MFCINST_ENCODER) {
-   if (IS_MFCV8_PLUS(dev))
+   if (IS_MFCV10(dev)) {
+   ctx->tmv_buffer_size = 0;
+   } else if (IS_MFCV8_PLUS(dev))
ctx->tmv_buffer_size = S5P_FIMV_NUM_TMV_BUFFERS_V6 *
ALIGN(S5P_FIMV_TMV_BUFFER_SIZE_V8(mb_width, mb_height),
S5P_FIMV_TMV_BUFFER_ALIGN_V6);
@@ -82,13 +85,36 @@ static int s5p_mfc_alloc_codec_buffers_v6(struct 
s5p_mfc_ctx *ctx)
ctx->tmv_buffer_size = S5P_FIMV_NUM_TMV_BUFFERS_V6 *
ALIGN(S5P_FIMV_TMV_BUFFER_SIZE_V6(mb_width, mb_height),
S5P_FIMV_TMV_BUFFER_ALIGN_V6);
-
-   ctx->luma_dpb_size = ALIGN((mb_width * mb_height) *
-   S5P_FIMV_LUMA_MB_TO_PIXEL_V6,
-   S5P_FIMV_LUMA_DPB_BUFFER_ALIGN_V6);
-   ctx->chroma_dpb_size = ALIGN((mb_width * mb_height) *
-   S5P_FIMV_CHROMA_MB_TO_PIXEL_V6,
-   S5P_FIMV_CHROMA_DPB_BUFFER_ALIGN_V6);
+   if (IS_MFCV10(dev)) {
+   lcu_width = enc_lcu_width(ctx->img_width);
+   lcu_height = enc_lcu_height(ctx->img_height);
+   if (ctx->codec_mode != S5P_FIMV_CODEC_HEVC_ENC) {
+   ctx->luma_dpb_size =
+   ALIGN((mb_width * 16), 64)
+   * ALIGN((mb_height * 16), 32)
+   + 64;
+   ctx->chroma_dpb_size =
+   ALIGN((mb_width * 16), 64)
+   * (mb_height * 8)
+   + 64;
+   } else {
+   ctx->luma_dpb_size =
+   ALIGN((lcu_width * 32), 64)
+   * ALIGN((lcu_height * 32), 32)
+   + 64;
+   ctx->chroma_dpb_size =
+   ALIGN((lcu_width * 32), 64)
+   * (lcu_height * 16)
+   + 64;
+   }
+   } else {
+   ctx->luma_dpb_size = ALIGN((mb_width * mb_height) *
+   S5P_FIMV_LUMA_MB_TO_PIXEL_V6,
+   S5P_FIMV_LUMA_DPB_BUFFER_ALIGN_V6);
+   ctx->chroma_dpb_size = ALIGN((mb_width * mb_height) *
+   S5P_FIMV_CHROMA_MB_TO_PIXEL_V6,