Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1572181 > unrolled thread

Re: [PATCH 05/11] [media] s5p-mfc: Add support for HEVC decoder

Started byAndrzej Hajda <a.hajda@samsung.com>
First post2017-02-02 09:00 +0100
Last post2017-02-06 10:00 +0100
Articles 4 — 2 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH 05/11] [media] s5p-mfc: Add support for HEVC decoder Andrzej Hajda <a.hajda@samsung.com> - 2017-02-02 09:00 +0100
    Re: [PATCH 05/11] [media] s5p-mfc: Add support for HEVC decoder Andrzej Hajda <a.hajda@samsung.com> - 2017-02-02 09:30 +0100
      Re: [PATCH 05/11] [media] s5p-mfc: Add support for HEVC decoder Smitha T Murthy <smitha.t@samsung.com> - 2017-02-06 10:00 +0100
    Re: [PATCH 05/11] [media] s5p-mfc: Add support for HEVC decoder Smitha T Murthy <smitha.t@samsung.com> - 2017-02-06 10:00 +0100

#1572181 — Re: [PATCH 05/11] [media] s5p-mfc: Add support for HEVC decoder

FromAndrzej Hajda <a.hajda@samsung.com>
Date2017-02-02 09:00 +0100
SubjectRe: [PATCH 05/11] [media] s5p-mfc: Add support for HEVC decoder
Message-ID<t6ktb-VZ-1@gated-at.bofh.it>
On 18.01.2017 11:02, Smitha T Murthy wrote:
> Add support for codec definition and corresponding buffer
> requirements for HEVC decoder.
>
> Signed-off-by: Smitha T Murthy <smitha.t@samsung.com>
> ---
>  drivers/media/platform/s5p-mfc/regs-mfc-v10.h   |    3 +++
>  drivers/media/platform/s5p-mfc/s5p_mfc_cmd_v6.c |    3 +++
>  drivers/media/platform/s5p-mfc/s5p_mfc_common.h |    1 +
>  drivers/media/platform/s5p-mfc/s5p_mfc_dec.c    |    8 ++++++++
>  drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c |   18 ++++++++++++++++--
>  drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.h |    5 +++++
>  6 files changed, 36 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/media/platform/s5p-mfc/regs-mfc-v10.h b/drivers/media/platform/s5p-mfc/regs-mfc-v10.h
> index 153ee68..a57009a 100644
> --- a/drivers/media/platform/s5p-mfc/regs-mfc-v10.h
> +++ b/drivers/media/platform/s5p-mfc/regs-mfc-v10.h
> @@ -32,6 +32,9 @@
>  #define MFC_VERSION_V10		0xA0
>  #define MFC_NUM_PORTS_V10	1
>  
> +/* MFCv10 codec defines*/
> +#define S5P_FIMV_CODEC_HEVC_DEC		17
> +
>  /* Encoder buffer size for MFC v10.0 */
>  #define ENC_V100_H264_ME_SIZE(x, y)	\
>  	(((x + 3) * (y + 3) * 8)	\
> diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_cmd_v6.c b/drivers/media/platform/s5p-mfc/s5p_mfc_cmd_v6.c
> index b1b1491..76eca67 100644
> --- a/drivers/media/platform/s5p-mfc/s5p_mfc_cmd_v6.c
> +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_cmd_v6.c
> @@ -101,6 +101,9 @@ static int s5p_mfc_open_inst_cmd_v6(struct s5p_mfc_ctx *ctx)
>  	case S5P_MFC_CODEC_VP8_DEC:
>  		codec_type = S5P_FIMV_CODEC_VP8_DEC_V6;
>  		break;
> +	case S5P_MFC_CODEC_HEVC_DEC:
> +		codec_type = S5P_FIMV_CODEC_HEVC_DEC;
> +		break;
>  	case S5P_MFC_CODEC_H264_ENC:
>  		codec_type = S5P_FIMV_CODEC_H264_ENC_V6;
>  		break;
> diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_common.h b/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
> index 998e24b..5c46060 100644
> --- a/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
> +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
> @@ -79,6 +79,7 @@ static inline dma_addr_t s5p_mfc_mem_cookie(void *a, void *b)
>  #define S5P_MFC_CODEC_H263_DEC		5
>  #define S5P_MFC_CODEC_VC1RCV_DEC	6
>  #define S5P_MFC_CODEC_VP8_DEC		7
> +#define S5P_MFC_CODEC_HEVC_DEC		17
>  
>  #define S5P_MFC_CODEC_H264_ENC		20
>  #define S5P_MFC_CODEC_H264_MVC_ENC	21
> diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c b/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c
> index 784b28e..9f459b3 100644
> --- a/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c
> +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c
> @@ -156,6 +156,14 @@
>  		.versions	= MFC_V6_BIT | MFC_V7_BIT | MFC_V8_BIT |
>  								MFC_V10_BIT,
>  	},
> +	{
> +		.name		= "HEVC Encoded Stream",
> +		.fourcc		= V4L2_PIX_FMT_HEVC,
> +		.codec_mode	= S5P_FIMV_CODEC_HEVC_DEC,
> +		.type		= MFC_FMT_DEC,
> +		.num_planes	= 1,
> +		.versions	= MFC_V10_BIT,
> +	},
>  };
>  
>  #define NUM_FORMATS ARRAY_SIZE(formats)
> 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 369210a..b6cb280 100644
> --- a/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c
> +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c
> @@ -220,6 +220,13 @@ static int s5p_mfc_alloc_codec_buffers_v6(struct s5p_mfc_ctx *ctx)
>  				S5P_FIMV_SCRATCH_BUFFER_ALIGN_V6);
>  		ctx->bank1.size = ctx->scratch_buf_size;
>  		break;
> +	case S5P_MFC_CODEC_HEVC_DEC:
> +		mfc_debug(2, "Use min scratch buffer size\n");
> +		ctx->scratch_buf_size = ALIGN(ctx->scratch_buf_size, 256);

Again alignment of something which should be already aligned, and magic
number instead of S5P_FIMV_SCRATCH_BUFFER_ALIGN_V6.

> +		ctx->bank1.size =
> +			ctx->scratch_buf_size +
> +			(ctx->mv_count * ctx->mv_size);
> +		break;
>  	case S5P_MFC_CODEC_H264_ENC:
>  		if (IS_MFCV10(dev)) {
>  			mfc_debug(2, "Use min scratch buffer size\n");
> @@ -322,6 +329,7 @@ static int s5p_mfc_alloc_instance_buffer_v6(struct s5p_mfc_ctx *ctx)
>  	switch (ctx->codec_mode) {
>  	case S5P_MFC_CODEC_H264_DEC:
>  	case S5P_MFC_CODEC_H264_MVC_DEC:
> +	case S5P_MFC_CODEC_HEVC_DEC:
>  		ctx->ctx.size = buf_size->h264_dec_ctx;
>  		break;
>  	case S5P_MFC_CODEC_MPEG4_DEC:
> @@ -438,6 +446,10 @@ static void s5p_mfc_dec_calc_dpb_size_v6(struct s5p_mfc_ctx *ctx)
>  					ctx->img_height);
>  			ctx->mv_size = ALIGN(ctx->mv_size, 16);
>  		}
> +	} else if (ctx->codec_mode == S5P_MFC_CODEC_HEVC_DEC) {
> +		ctx->mv_size = s5p_mfc_dec_hevc_mv_size(ctx->img_width,
> +				ctx->img_height);
> +		ctx->mv_size = ALIGN(ctx->mv_size, 32);

Again, unnecessary alignment, result of s5p_mfc_dec_hevc_mv_size is
already aligned to 256.

>  	} else {
>  		ctx->mv_size = 0;
>  	}
> @@ -526,7 +538,8 @@ static int s5p_mfc_set_dec_frame_buffer_v6(struct s5p_mfc_ctx *ctx)
>  	buf_size1 -= ctx->scratch_buf_size;
>  
>  	if (ctx->codec_mode == S5P_FIMV_CODEC_H264_DEC ||
> -			ctx->codec_mode == S5P_FIMV_CODEC_H264_MVC_DEC){
> +			ctx->codec_mode == S5P_FIMV_CODEC_H264_MVC_DEC ||
> +			ctx->codec_mode == S5P_FIMV_CODEC_HEVC_DEC) {
>  		writel(ctx->mv_size, mfc_regs->d_mv_buffer_size);
>  		writel(ctx->mv_count, mfc_regs->d_num_mv);
>  	}
> @@ -549,7 +562,8 @@ static int s5p_mfc_set_dec_frame_buffer_v6(struct s5p_mfc_ctx *ctx)
>  				mfc_regs->d_second_plane_dpb + i * 4);
>  	}
>  	if (ctx->codec_mode == S5P_MFC_CODEC_H264_DEC ||
> -			ctx->codec_mode == S5P_MFC_CODEC_H264_MVC_DEC) {
> +			ctx->codec_mode == S5P_MFC_CODEC_H264_MVC_DEC ||
> +			ctx->codec_mode == S5P_MFC_CODEC_HEVC_DEC) {
>  		for (i = 0; i < ctx->mv_count; i++) {
>  			/* To test alignment */
>  			align_gap = buf_addr1;
> diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.h b/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.h
> index 2b5a9f4..2e404d8 100644
> --- a/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.h
> +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.h
> @@ -29,6 +29,11 @@
>  #define enc_lcu_width(x_size)          ((x_size + 31) / 32)
>  #define enc_lcu_height(y_size)         ((y_size + 31) / 32)
>  
> +#define dec_lcu_width(x_size)		((x_size + 63) / 64)

It could be replaced with:

+#define dec_lcu_width(x_size) ALIGN(x_size, 64)


> +#define dec_lcu_height(y_size)		((y_size + 63) / 64)

The same here.
> +#define s5p_mfc_dec_hevc_mv_size(x, y) \
> +	(dec_lcu_width(x) * dec_lcu_height(y) * 256 + 512)
> +

If dec_lcu_(width|height) are not used anywhere else, you can just
squash them here:

#define s5p_mfc_dec_hevc_mv_size(x, y) (ALIGN(x, 64) * ALIGN(y, 64) * 256 + 512)

And if there is only one use of this macro, maybe it would be good to
just hardcode it there.

Regards
Andrzej

>  /* Definition */
>  #define ENC_MULTI_SLICE_MB_MAX		((1 << 30) - 1)
>  #define ENC_MULTI_SLICE_BIT_MIN		2800

[toc] | [next] | [standalone]

#1572188

FromAndrzej Hajda <a.hajda@samsung.com>
Date2017-02-02 09:30 +0100
Message-ID<t6kWd-1ll-5@gated-at.bofh.it>
In reply to#1572181
On 02.02.2017 08:58, Andrzej Hajda wrote:
> On 18.01.2017 11:02, Smitha T Murthy wrote:
>> Add support for codec definition and corresponding buffer
>> requirements for HEVC decoder.
>>
>> Signed-off-by: Smitha T Murthy <smitha.t@samsung.com>
>> ---
>>  drivers/media/platform/s5p-mfc/regs-mfc-v10.h   |    3 +++
>>  drivers/media/platform/s5p-mfc/s5p_mfc_cmd_v6.c |    3 +++
>>  drivers/media/platform/s5p-mfc/s5p_mfc_common.h |    1 +
>>  drivers/media/platform/s5p-mfc/s5p_mfc_dec.c    |    8 ++++++++
>>  drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c |   18 ++++++++++++++++--
>>  drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.h |    5 +++++
>>  6 files changed, 36 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/media/platform/s5p-mfc/regs-mfc-v10.h b/drivers/media/platform/s5p-mfc/regs-mfc-v10.h
>> index 153ee68..a57009a 100644
>> --- a/drivers/media/platform/s5p-mfc/regs-mfc-v10.h
>> +++ b/drivers/media/platform/s5p-mfc/regs-mfc-v10.h
>> @@ -32,6 +32,9 @@
>>  #define MFC_VERSION_V10		0xA0
>>  #define MFC_NUM_PORTS_V10	1
>>  
>> +/* MFCv10 codec defines*/
>> +#define S5P_FIMV_CODEC_HEVC_DEC		17
>> +
>>  /* Encoder buffer size for MFC v10.0 */
>>  #define ENC_V100_H264_ME_SIZE(x, y)	\
>>  	(((x + 3) * (y + 3) * 8)	\
>> diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_cmd_v6.c b/drivers/media/platform/s5p-mfc/s5p_mfc_cmd_v6.c
>> index b1b1491..76eca67 100644
>> --- a/drivers/media/platform/s5p-mfc/s5p_mfc_cmd_v6.c
>> +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_cmd_v6.c
>> @@ -101,6 +101,9 @@ static int s5p_mfc_open_inst_cmd_v6(struct s5p_mfc_ctx *ctx)
>>  	case S5P_MFC_CODEC_VP8_DEC:
>>  		codec_type = S5P_FIMV_CODEC_VP8_DEC_V6;
>>  		break;
>> +	case S5P_MFC_CODEC_HEVC_DEC:
>> +		codec_type = S5P_FIMV_CODEC_HEVC_DEC;
>> +		break;
>>  	case S5P_MFC_CODEC_H264_ENC:
>>  		codec_type = S5P_FIMV_CODEC_H264_ENC_V6;
>>  		break;
>> diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_common.h b/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
>> index 998e24b..5c46060 100644
>> --- a/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
>> +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
>> @@ -79,6 +79,7 @@ static inline dma_addr_t s5p_mfc_mem_cookie(void *a, void *b)
>>  #define S5P_MFC_CODEC_H263_DEC		5
>>  #define S5P_MFC_CODEC_VC1RCV_DEC	6
>>  #define S5P_MFC_CODEC_VP8_DEC		7
>> +#define S5P_MFC_CODEC_HEVC_DEC		17
>>  
>>  #define S5P_MFC_CODEC_H264_ENC		20
>>  #define S5P_MFC_CODEC_H264_MVC_ENC	21
>> diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c b/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c
>> index 784b28e..9f459b3 100644
>> --- a/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c
>> +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c
>> @@ -156,6 +156,14 @@
>>  		.versions	= MFC_V6_BIT | MFC_V7_BIT | MFC_V8_BIT |
>>  								MFC_V10_BIT,
>>  	},
>> +	{
>> +		.name		= "HEVC Encoded Stream",
>> +		.fourcc		= V4L2_PIX_FMT_HEVC,
>> +		.codec_mode	= S5P_FIMV_CODEC_HEVC_DEC,
>> +		.type		= MFC_FMT_DEC,
>> +		.num_planes	= 1,
>> +		.versions	= MFC_V10_BIT,
>> +	},
>>  };
>>  
>>  #define NUM_FORMATS ARRAY_SIZE(formats)
>> 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 369210a..b6cb280 100644
>> --- a/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c
>> +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c
>> @@ -220,6 +220,13 @@ static int s5p_mfc_alloc_codec_buffers_v6(struct s5p_mfc_ctx *ctx)
>>  				S5P_FIMV_SCRATCH_BUFFER_ALIGN_V6);
>>  		ctx->bank1.size = ctx->scratch_buf_size;
>>  		break;
>> +	case S5P_MFC_CODEC_HEVC_DEC:
>> +		mfc_debug(2, "Use min scratch buffer size\n");
>> +		ctx->scratch_buf_size = ALIGN(ctx->scratch_buf_size, 256);
> Again alignment of something which should be already aligned, and magic
> number instead of S5P_FIMV_SCRATCH_BUFFER_ALIGN_V6.
>
>> +		ctx->bank1.size =
>> +			ctx->scratch_buf_size +
>> +			(ctx->mv_count * ctx->mv_size);
>> +		break;
>>  	case S5P_MFC_CODEC_H264_ENC:
>>  		if (IS_MFCV10(dev)) {
>>  			mfc_debug(2, "Use min scratch buffer size\n");
>> @@ -322,6 +329,7 @@ static int s5p_mfc_alloc_instance_buffer_v6(struct s5p_mfc_ctx *ctx)
>>  	switch (ctx->codec_mode) {
>>  	case S5P_MFC_CODEC_H264_DEC:
>>  	case S5P_MFC_CODEC_H264_MVC_DEC:
>> +	case S5P_MFC_CODEC_HEVC_DEC:
>>  		ctx->ctx.size = buf_size->h264_dec_ctx;
>>  		break;
>>  	case S5P_MFC_CODEC_MPEG4_DEC:
>> @@ -438,6 +446,10 @@ static void s5p_mfc_dec_calc_dpb_size_v6(struct s5p_mfc_ctx *ctx)
>>  					ctx->img_height);
>>  			ctx->mv_size = ALIGN(ctx->mv_size, 16);
>>  		}
>> +	} else if (ctx->codec_mode == S5P_MFC_CODEC_HEVC_DEC) {
>> +		ctx->mv_size = s5p_mfc_dec_hevc_mv_size(ctx->img_width,
>> +				ctx->img_height);
>> +		ctx->mv_size = ALIGN(ctx->mv_size, 32);
> Again, unnecessary alignment, result of s5p_mfc_dec_hevc_mv_size is
> already aligned to 256.
>
>>  	} else {
>>  		ctx->mv_size = 0;
>>  	}
>> @@ -526,7 +538,8 @@ static int s5p_mfc_set_dec_frame_buffer_v6(struct s5p_mfc_ctx *ctx)
>>  	buf_size1 -= ctx->scratch_buf_size;
>>  
>>  	if (ctx->codec_mode == S5P_FIMV_CODEC_H264_DEC ||
>> -			ctx->codec_mode == S5P_FIMV_CODEC_H264_MVC_DEC){
>> +			ctx->codec_mode == S5P_FIMV_CODEC_H264_MVC_DEC ||
>> +			ctx->codec_mode == S5P_FIMV_CODEC_HEVC_DEC) {
>>  		writel(ctx->mv_size, mfc_regs->d_mv_buffer_size);
>>  		writel(ctx->mv_count, mfc_regs->d_num_mv);
>>  	}
>> @@ -549,7 +562,8 @@ static int s5p_mfc_set_dec_frame_buffer_v6(struct s5p_mfc_ctx *ctx)
>>  				mfc_regs->d_second_plane_dpb + i * 4);
>>  	}
>>  	if (ctx->codec_mode == S5P_MFC_CODEC_H264_DEC ||
>> -			ctx->codec_mode == S5P_MFC_CODEC_H264_MVC_DEC) {
>> +			ctx->codec_mode == S5P_MFC_CODEC_H264_MVC_DEC ||
>> +			ctx->codec_mode == S5P_MFC_CODEC_HEVC_DEC) {
>>  		for (i = 0; i < ctx->mv_count; i++) {
>>  			/* To test alignment */
>>  			align_gap = buf_addr1;
>> diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.h b/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.h
>> index 2b5a9f4..2e404d8 100644
>> --- a/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.h
>> +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.h
>> @@ -29,6 +29,11 @@
>>  #define enc_lcu_width(x_size)          ((x_size + 31) / 32)
>>  #define enc_lcu_height(y_size)         ((y_size + 31) / 32)
>>  
>> +#define dec_lcu_width(x_size)		((x_size + 63) / 64)
> It could be replaced with:
>
> +#define dec_lcu_width(x_size) ALIGN(x_size, 64)

Ups, DIV_ROUND_UP should be used instead of ALIGN, here and below.

>
>
>> +#define dec_lcu_height(y_size)		((y_size + 63) / 64)
> The same here.
>> +#define s5p_mfc_dec_hevc_mv_size(x, y) \
>> +	(dec_lcu_width(x) * dec_lcu_height(y) * 256 + 512)
>> +
> If dec_lcu_(width|height) are not used anywhere else, you can just
> squash them here:
>
> #define s5p_mfc_dec_hevc_mv_size(x, y) (ALIGN(x, 64) * ALIGN(y, 64) * 256 + 512)
>
> And if there is only one use of this macro, maybe it would be good to
> just hardcode it there.
>
> Regards
> Andrzej
>
>>  /* Definition */
>>  #define ENC_MULTI_SLICE_MB_MAX		((1 << 30) - 1)
>>  #define ENC_MULTI_SLICE_BIT_MIN		2800
>

[toc] | [prev] | [next] | [standalone]

#1574464

FromSmitha T Murthy <smitha.t@samsung.com>
Date2017-02-06 10:00 +0100
Message-ID<t7Njs-2A1-19@gated-at.bofh.it>
In reply to#1572188
On Thu, 2017-02-02 at 09:20 +0100, Andrzej Hajda wrote: 
> On 02.02.2017 08:58, Andrzej Hajda wrote:
> > On 18.01.2017 11:02, Smitha T Murthy wrote:
> >> Add support for codec definition and corresponding buffer
> >> requirements for HEVC decoder.
> >>
> >> Signed-off-by: Smitha T Murthy <smitha.t@samsung.com>
> >> ---
> >>  drivers/media/platform/s5p-mfc/regs-mfc-v10.h   |    3 +++
> >>  drivers/media/platform/s5p-mfc/s5p_mfc_cmd_v6.c |    3 +++
> >>  drivers/media/platform/s5p-mfc/s5p_mfc_common.h |    1 +
> >>  drivers/media/platform/s5p-mfc/s5p_mfc_dec.c    |    8 ++++++++
> >>  drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c |   18 ++++++++++++++++--
> >>  drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.h |    5 +++++
> >>  6 files changed, 36 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/media/platform/s5p-mfc/regs-mfc-v10.h b/drivers/media/platform/s5p-mfc/regs-mfc-v10.h
> >> index 153ee68..a57009a 100644
> >> --- a/drivers/media/platform/s5p-mfc/regs-mfc-v10.h
> >> +++ b/drivers/media/platform/s5p-mfc/regs-mfc-v10.h
> >> @@ -32,6 +32,9 @@
> >>  #define MFC_VERSION_V10		0xA0
> >>  #define MFC_NUM_PORTS_V10	1
> >>  
> >> +/* MFCv10 codec defines*/
> >> +#define S5P_FIMV_CODEC_HEVC_DEC		17
> >> +
> >>  /* Encoder buffer size for MFC v10.0 */
> >>  #define ENC_V100_H264_ME_SIZE(x, y)	\
> >>  	(((x + 3) * (y + 3) * 8)	\
> >> diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_cmd_v6.c b/drivers/media/platform/s5p-mfc/s5p_mfc_cmd_v6.c
> >> index b1b1491..76eca67 100644
> >> --- a/drivers/media/platform/s5p-mfc/s5p_mfc_cmd_v6.c
> >> +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_cmd_v6.c
> >> @@ -101,6 +101,9 @@ static int s5p_mfc_open_inst_cmd_v6(struct s5p_mfc_ctx *ctx)
> >>  	case S5P_MFC_CODEC_VP8_DEC:
> >>  		codec_type = S5P_FIMV_CODEC_VP8_DEC_V6;
> >>  		break;
> >> +	case S5P_MFC_CODEC_HEVC_DEC:
> >> +		codec_type = S5P_FIMV_CODEC_HEVC_DEC;
> >> +		break;
> >>  	case S5P_MFC_CODEC_H264_ENC:
> >>  		codec_type = S5P_FIMV_CODEC_H264_ENC_V6;
> >>  		break;
> >> diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_common.h b/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
> >> index 998e24b..5c46060 100644
> >> --- a/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
> >> +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
> >> @@ -79,6 +79,7 @@ static inline dma_addr_t s5p_mfc_mem_cookie(void *a, void *b)
> >>  #define S5P_MFC_CODEC_H263_DEC		5
> >>  #define S5P_MFC_CODEC_VC1RCV_DEC	6
> >>  #define S5P_MFC_CODEC_VP8_DEC		7
> >> +#define S5P_MFC_CODEC_HEVC_DEC		17
> >>  
> >>  #define S5P_MFC_CODEC_H264_ENC		20
> >>  #define S5P_MFC_CODEC_H264_MVC_ENC	21
> >> diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c b/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c
> >> index 784b28e..9f459b3 100644
> >> --- a/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c
> >> +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c
> >> @@ -156,6 +156,14 @@
> >>  		.versions	= MFC_V6_BIT | MFC_V7_BIT | MFC_V8_BIT |
> >>  								MFC_V10_BIT,
> >>  	},
> >> +	{
> >> +		.name		= "HEVC Encoded Stream",
> >> +		.fourcc		= V4L2_PIX_FMT_HEVC,
> >> +		.codec_mode	= S5P_FIMV_CODEC_HEVC_DEC,
> >> +		.type		= MFC_FMT_DEC,
> >> +		.num_planes	= 1,
> >> +		.versions	= MFC_V10_BIT,
> >> +	},
> >>  };
> >>  
> >>  #define NUM_FORMATS ARRAY_SIZE(formats)
> >> 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 369210a..b6cb280 100644
> >> --- a/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c
> >> +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c
> >> @@ -220,6 +220,13 @@ static int s5p_mfc_alloc_codec_buffers_v6(struct s5p_mfc_ctx *ctx)
> >>  				S5P_FIMV_SCRATCH_BUFFER_ALIGN_V6);
> >>  		ctx->bank1.size = ctx->scratch_buf_size;
> >>  		break;
> >> +	case S5P_MFC_CODEC_HEVC_DEC:
> >> +		mfc_debug(2, "Use min scratch buffer size\n");
> >> +		ctx->scratch_buf_size = ALIGN(ctx->scratch_buf_size, 256);
> > Again alignment of something which should be already aligned, and magic
> > number instead of S5P_FIMV_SCRATCH_BUFFER_ALIGN_V6.
> >
> >> +		ctx->bank1.size =
> >> +			ctx->scratch_buf_size +
> >> +			(ctx->mv_count * ctx->mv_size);
> >> +		break;
> >>  	case S5P_MFC_CODEC_H264_ENC:
> >>  		if (IS_MFCV10(dev)) {
> >>  			mfc_debug(2, "Use min scratch buffer size\n");
> >> @@ -322,6 +329,7 @@ static int s5p_mfc_alloc_instance_buffer_v6(struct s5p_mfc_ctx *ctx)
> >>  	switch (ctx->codec_mode) {
> >>  	case S5P_MFC_CODEC_H264_DEC:
> >>  	case S5P_MFC_CODEC_H264_MVC_DEC:
> >> +	case S5P_MFC_CODEC_HEVC_DEC:
> >>  		ctx->ctx.size = buf_size->h264_dec_ctx;
> >>  		break;
> >>  	case S5P_MFC_CODEC_MPEG4_DEC:
> >> @@ -438,6 +446,10 @@ static void s5p_mfc_dec_calc_dpb_size_v6(struct s5p_mfc_ctx *ctx)
> >>  					ctx->img_height);
> >>  			ctx->mv_size = ALIGN(ctx->mv_size, 16);
> >>  		}
> >> +	} else if (ctx->codec_mode == S5P_MFC_CODEC_HEVC_DEC) {
> >> +		ctx->mv_size = s5p_mfc_dec_hevc_mv_size(ctx->img_width,
> >> +				ctx->img_height);
> >> +		ctx->mv_size = ALIGN(ctx->mv_size, 32);
> > Again, unnecessary alignment, result of s5p_mfc_dec_hevc_mv_size is
> > already aligned to 256.
> >
> >>  	} else {
> >>  		ctx->mv_size = 0;
> >>  	}
> >> @@ -526,7 +538,8 @@ static int s5p_mfc_set_dec_frame_buffer_v6(struct s5p_mfc_ctx *ctx)
> >>  	buf_size1 -= ctx->scratch_buf_size;
> >>  
> >>  	if (ctx->codec_mode == S5P_FIMV_CODEC_H264_DEC ||
> >> -			ctx->codec_mode == S5P_FIMV_CODEC_H264_MVC_DEC){
> >> +			ctx->codec_mode == S5P_FIMV_CODEC_H264_MVC_DEC ||
> >> +			ctx->codec_mode == S5P_FIMV_CODEC_HEVC_DEC) {
> >>  		writel(ctx->mv_size, mfc_regs->d_mv_buffer_size);
> >>  		writel(ctx->mv_count, mfc_regs->d_num_mv);
> >>  	}
> >> @@ -549,7 +562,8 @@ static int s5p_mfc_set_dec_frame_buffer_v6(struct s5p_mfc_ctx *ctx)
> >>  				mfc_regs->d_second_plane_dpb + i * 4);
> >>  	}
> >>  	if (ctx->codec_mode == S5P_MFC_CODEC_H264_DEC ||
> >> -			ctx->codec_mode == S5P_MFC_CODEC_H264_MVC_DEC) {
> >> +			ctx->codec_mode == S5P_MFC_CODEC_H264_MVC_DEC ||
> >> +			ctx->codec_mode == S5P_MFC_CODEC_HEVC_DEC) {
> >>  		for (i = 0; i < ctx->mv_count; i++) {
> >>  			/* To test alignment */
> >>  			align_gap = buf_addr1;
> >> diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.h b/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.h
> >> index 2b5a9f4..2e404d8 100644
> >> --- a/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.h
> >> +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.h
> >> @@ -29,6 +29,11 @@
> >>  #define enc_lcu_width(x_size)          ((x_size + 31) / 32)
> >>  #define enc_lcu_height(y_size)         ((y_size + 31) / 32)
> >>  
> >> +#define dec_lcu_width(x_size)		((x_size + 63) / 64)
> > It could be replaced with:
> >
> > +#define dec_lcu_width(x_size) ALIGN(x_size, 64)
> 
> Ups, DIV_ROUND_UP should be used instead of ALIGN, here and below.
> 
Yes will add this change in the next version.

Thank you for the review.
Regards,
Smitha 
> >
> >
> >> +#define dec_lcu_height(y_size)		((y_size + 63) / 64)
> > The same here.
> >> +#define s5p_mfc_dec_hevc_mv_size(x, y) \
> >> +	(dec_lcu_width(x) * dec_lcu_height(y) * 256 + 512)
> >> +
> > If dec_lcu_(width|height) are not used anywhere else, you can just
> > squash them here:
> >
> > #define s5p_mfc_dec_hevc_mv_size(x, y) (ALIGN(x, 64) * ALIGN(y, 64) * 256 + 512)
> >
> > And if there is only one use of this macro, maybe it would be good to
> > just hardcode it there.
> >
> > Regards
> > Andrzej
> >
> >>  /* Definition */
> >>  #define ENC_MULTI_SLICE_MB_MAX		((1 << 30) - 1)
> >>  #define ENC_MULTI_SLICE_BIT_MIN		2800
> >
> 
>

[toc] | [prev] | [next] | [standalone]

#1574461

FromSmitha T Murthy <smitha.t@samsung.com>
Date2017-02-06 10:00 +0100
Message-ID<t7Njs-2A1-3@gated-at.bofh.it>
In reply to#1572181
On Thu, 2017-02-02 at 08:58 +0100, Andrzej Hajda wrote: 
> On 18.01.2017 11:02, Smitha T Murthy wrote:
> > Add support for codec definition and corresponding buffer
> > requirements for HEVC decoder.
> >
> > Signed-off-by: Smitha T Murthy <smitha.t@samsung.com>
> > ---
> >  drivers/media/platform/s5p-mfc/regs-mfc-v10.h   |    3 +++
> >  drivers/media/platform/s5p-mfc/s5p_mfc_cmd_v6.c |    3 +++
> >  drivers/media/platform/s5p-mfc/s5p_mfc_common.h |    1 +
> >  drivers/media/platform/s5p-mfc/s5p_mfc_dec.c    |    8 ++++++++
> >  drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c |   18 ++++++++++++++++--
> >  drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.h |    5 +++++
> >  6 files changed, 36 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/media/platform/s5p-mfc/regs-mfc-v10.h b/drivers/media/platform/s5p-mfc/regs-mfc-v10.h
> > index 153ee68..a57009a 100644
> > --- a/drivers/media/platform/s5p-mfc/regs-mfc-v10.h
> > +++ b/drivers/media/platform/s5p-mfc/regs-mfc-v10.h
> > @@ -32,6 +32,9 @@
> >  #define MFC_VERSION_V10		0xA0
> >  #define MFC_NUM_PORTS_V10	1
> >  
> > +/* MFCv10 codec defines*/
> > +#define S5P_FIMV_CODEC_HEVC_DEC		17
> > +
> >  /* Encoder buffer size for MFC v10.0 */
> >  #define ENC_V100_H264_ME_SIZE(x, y)	\
> >  	(((x + 3) * (y + 3) * 8)	\
> > diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_cmd_v6.c b/drivers/media/platform/s5p-mfc/s5p_mfc_cmd_v6.c
> > index b1b1491..76eca67 100644
> > --- a/drivers/media/platform/s5p-mfc/s5p_mfc_cmd_v6.c
> > +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_cmd_v6.c
> > @@ -101,6 +101,9 @@ static int s5p_mfc_open_inst_cmd_v6(struct s5p_mfc_ctx *ctx)
> >  	case S5P_MFC_CODEC_VP8_DEC:
> >  		codec_type = S5P_FIMV_CODEC_VP8_DEC_V6;
> >  		break;
> > +	case S5P_MFC_CODEC_HEVC_DEC:
> > +		codec_type = S5P_FIMV_CODEC_HEVC_DEC;
> > +		break;
> >  	case S5P_MFC_CODEC_H264_ENC:
> >  		codec_type = S5P_FIMV_CODEC_H264_ENC_V6;
> >  		break;
> > diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_common.h b/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
> > index 998e24b..5c46060 100644
> > --- a/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
> > +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
> > @@ -79,6 +79,7 @@ static inline dma_addr_t s5p_mfc_mem_cookie(void *a, void *b)
> >  #define S5P_MFC_CODEC_H263_DEC		5
> >  #define S5P_MFC_CODEC_VC1RCV_DEC	6
> >  #define S5P_MFC_CODEC_VP8_DEC		7
> > +#define S5P_MFC_CODEC_HEVC_DEC		17
> >  
> >  #define S5P_MFC_CODEC_H264_ENC		20
> >  #define S5P_MFC_CODEC_H264_MVC_ENC	21
> > diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c b/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c
> > index 784b28e..9f459b3 100644
> > --- a/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c
> > +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c
> > @@ -156,6 +156,14 @@
> >  		.versions	= MFC_V6_BIT | MFC_V7_BIT | MFC_V8_BIT |
> >  								MFC_V10_BIT,
> >  	},
> > +	{
> > +		.name		= "HEVC Encoded Stream",
> > +		.fourcc		= V4L2_PIX_FMT_HEVC,
> > +		.codec_mode	= S5P_FIMV_CODEC_HEVC_DEC,
> > +		.type		= MFC_FMT_DEC,
> > +		.num_planes	= 1,
> > +		.versions	= MFC_V10_BIT,
> > +	},
> >  };
> >  
> >  #define NUM_FORMATS ARRAY_SIZE(formats)
> > 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 369210a..b6cb280 100644
> > --- a/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c
> > +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c
> > @@ -220,6 +220,13 @@ static int s5p_mfc_alloc_codec_buffers_v6(struct s5p_mfc_ctx *ctx)
> >  				S5P_FIMV_SCRATCH_BUFFER_ALIGN_V6);
> >  		ctx->bank1.size = ctx->scratch_buf_size;
> >  		break;
> > +	case S5P_MFC_CODEC_HEVC_DEC:
> > +		mfc_debug(2, "Use min scratch buffer size\n");
> > +		ctx->scratch_buf_size = ALIGN(ctx->scratch_buf_size, 256);
> 
> Again alignment of something which should be already aligned, and magic
> number instead of S5P_FIMV_SCRATCH_BUFFER_ALIGN_V6.
> 
Yes if we are using the scratch buffer given by the firmware we need not
align. I will change it in the next version.

> > +		ctx->bank1.size =
> > +			ctx->scratch_buf_size +
> > +			(ctx->mv_count * ctx->mv_size);
> > +		break;
> >  	case S5P_MFC_CODEC_H264_ENC:
> >  		if (IS_MFCV10(dev)) {
> >  			mfc_debug(2, "Use min scratch buffer size\n");
> > @@ -322,6 +329,7 @@ static int s5p_mfc_alloc_instance_buffer_v6(struct s5p_mfc_ctx *ctx)
> >  	switch (ctx->codec_mode) {
> >  	case S5P_MFC_CODEC_H264_DEC:
> >  	case S5P_MFC_CODEC_H264_MVC_DEC:
> > +	case S5P_MFC_CODEC_HEVC_DEC:
> >  		ctx->ctx.size = buf_size->h264_dec_ctx;
> >  		break;
> >  	case S5P_MFC_CODEC_MPEG4_DEC:
> > @@ -438,6 +446,10 @@ static void s5p_mfc_dec_calc_dpb_size_v6(struct s5p_mfc_ctx *ctx)
> >  					ctx->img_height);
> >  			ctx->mv_size = ALIGN(ctx->mv_size, 16);
> >  		}
> > +	} else if (ctx->codec_mode == S5P_MFC_CODEC_HEVC_DEC) {
> > +		ctx->mv_size = s5p_mfc_dec_hevc_mv_size(ctx->img_width,
> > +				ctx->img_height);
> > +		ctx->mv_size = ALIGN(ctx->mv_size, 32);
> 
> Again, unnecessary alignment, result of s5p_mfc_dec_hevc_mv_size is
> already aligned to 256.

#define s5p_mfc_dec_hevc_mv_size(x, y)   (dec_lcu_width(x) *
dec_lcu_height(y) * 256 + 512) is for calculation of the size of mv
buffer as given in the user manual. User Manual also mentions it needs
multiple of 32 alignment so added ctx->mv_size = ALIGN(ctx->mv_size,
32). Since it was already done for H264 codec in the same way,
ctx->mv_size = ALIGN(ctx->mv_size, 16) after size calculation through
#define S5P_MFC_DEC_MV_SIZE_V6(x, y)    (MB_WIDTH(x) * \
(((MB_HEIGHT(y)+1)/2)*2) * 64 + 128) I followed the same for HEVC mv
buffers too.

> 
> >  	} else {
> >  		ctx->mv_size = 0;
> >  	}
> > @@ -526,7 +538,8 @@ static int s5p_mfc_set_dec_frame_buffer_v6(struct s5p_mfc_ctx *ctx)
> >  	buf_size1 -= ctx->scratch_buf_size;
> >  
> >  	if (ctx->codec_mode == S5P_FIMV_CODEC_H264_DEC ||
> > -			ctx->codec_mode == S5P_FIMV_CODEC_H264_MVC_DEC){
> > +			ctx->codec_mode == S5P_FIMV_CODEC_H264_MVC_DEC ||
> > +			ctx->codec_mode == S5P_FIMV_CODEC_HEVC_DEC) {
> >  		writel(ctx->mv_size, mfc_regs->d_mv_buffer_size);
> >  		writel(ctx->mv_count, mfc_regs->d_num_mv);
> >  	}
> > @@ -549,7 +562,8 @@ static int s5p_mfc_set_dec_frame_buffer_v6(struct s5p_mfc_ctx *ctx)
> >  				mfc_regs->d_second_plane_dpb + i * 4);
> >  	}
> >  	if (ctx->codec_mode == S5P_MFC_CODEC_H264_DEC ||
> > -			ctx->codec_mode == S5P_MFC_CODEC_H264_MVC_DEC) {
> > +			ctx->codec_mode == S5P_MFC_CODEC_H264_MVC_DEC ||
> > +			ctx->codec_mode == S5P_MFC_CODEC_HEVC_DEC) {
> >  		for (i = 0; i < ctx->mv_count; i++) {
> >  			/* To test alignment */
> >  			align_gap = buf_addr1;
> > diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.h b/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.h
> > index 2b5a9f4..2e404d8 100644
> > --- a/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.h
> > +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.h
> > @@ -29,6 +29,11 @@
> >  #define enc_lcu_width(x_size)          ((x_size + 31) / 32)
> >  #define enc_lcu_height(y_size)         ((y_size + 31) / 32)
> >  
> > +#define dec_lcu_width(x_size)		((x_size + 63) / 64)
> 
> It could be replaced with:
> 
> +#define dec_lcu_width(x_size) ALIGN(x_size, 64)
> 
> 
> > +#define dec_lcu_height(y_size)		((y_size + 63) / 64)
> 
> The same here.
> > +#define s5p_mfc_dec_hevc_mv_size(x, y) \
> > +	(dec_lcu_width(x) * dec_lcu_height(y) * 256 + 512)
> > +
> 
> If dec_lcu_(width|height) are not used anywhere else, you can just
> squash them here:
> 
Yes dec_lcu_(width|height) is not used anywhere else, will squash it.

> #define s5p_mfc_dec_hevc_mv_size(x, y) (ALIGN(x, 64) * ALIGN(y, 64) * 256 + 512)
> 
> And if there is only one use of this macro, maybe it would be good to
> just hardcode it there.
> 
> Regards
> Andrzej
> 
I defined the macro s5p_mfc_dec_hevc_mv_size(x, y) in this file because
the same was done for another macro S5P_MFC_DEC_MV_SIZE_V6(x, y) which
is also used only once.

Thank you for the review.
Regards,
Smitha 
> 
> >  /* Definition */
> >  #define ENC_MULTI_SLICE_MB_MAX		((1 << 30) - 1)
> >  #define ENC_MULTI_SLICE_BIT_MIN		2800
> 
> 
>

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web