* Re: [PATCH v7 5/6] [media] s5p-mfc: MFCv6 register definitions
[not found] ` <1348851868-7698-6-git-send-email-arun.kk@samsung.com>
@ 2012-09-29 15:35 ` Sylwester Nawrocki
0 siblings, 0 replies; 5+ messages in thread
From: Sylwester Nawrocki @ 2012-09-29 15:35 UTC (permalink / raw)
To: Arun Kumar K
Cc: linux-media, k.debski, jtp.park, janghyuck.kim, jaeryul.oh,
ch.naveen, m.szyprowski, s.nawrocki, kmpark, joshi
Hi Arun,
I have a few minor comments.
On 09/28/2012 07:04 PM, Arun Kumar K wrote:
> From: Jeongtae Park<jtp.park@samsung.com>
>
> Adds register definitions for MFC v6.x firmware
>
> Signed-off-by: Jeongtae Park<jtp.park@samsung.com>
> Signed-off-by: Janghyuck Kim<janghyuck.kim@samsung.com>
> Signed-off-by: Jaeryul Oh<jaeryul.oh@samsung.com>
> Signed-off-by: Naveen Krishna Chatradhi<ch.naveen@samsung.com>
> Signed-off-by: Arun Kumar K<arun.kk@samsung.com>
> ---
> drivers/media/platform/s5p-mfc/regs-mfc-v6.h | 408 ++++++++++++++++++++++++++
> 1 files changed, 408 insertions(+), 0 deletions(-)
> create mode 100644 drivers/media/platform/s5p-mfc/regs-mfc-v6.h
>
> diff --git a/drivers/media/platform/s5p-mfc/regs-mfc-v6.h b/drivers/media/platform/s5p-mfc/regs-mfc-v6.h
> new file mode 100644
> index 0000000..cce1841
> --- /dev/null
> +++ b/drivers/media/platform/s5p-mfc/regs-mfc-v6.h
> @@ -0,0 +1,408 @@
> +/*
> + * Register definition file for Samsung MFC V6.x Interface (FIMV) driver
> + *
> + * Copyright (c) 2012 Samsung Electronics
I believe the proper notation is
Copyright (c) 2012 Samsung Electronics Co., Ltd.
Please make sure it's correct in all files added in this series.
> + * http://www.samsung.com/
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#ifndef _REGS_FIMV_V6_H
> +#define _REGS_FIMV_V6_H
Please add
#include <linux/kernel.h>
#include <linux/sizes.h>
> +#define S5P_FIMV_REG_SIZE_V6 (S5P_FIMV_END_ADDR - S5P_FIMV_START_ADDR)
> +#define S5P_FIMV_REG_COUNT_V6 ((S5P_FIMV_END_ADDR - S5P_FIMV_START_ADDR) / 4)
> +
> +/* Number of bits that the buffer address should be shifted for particular
> + * MFC buffers. */
> +#define S5P_FIMV_MEM_OFFSET_V6 0
> +
> +#define S5P_FIMV_START_ADDR_V6 0x0000
> +#define S5P_FIMV_END_ADDR_V6 0xfd80
> +
> +#define S5P_FIMV_REG_CLEAR_BEGIN_V6 0xf000
> +#define S5P_FIMV_REG_CLEAR_COUNT_V6 1024
> +
> +/* Codec Common Registers */
> +#define S5P_FIMV_RISC_ON_V6 0x0000
> +#define S5P_FIMV_RISC2HOST_INT_V6 0x003C
Could you make sure all hex numbers are in lower case in this file ?
...
> +#define S5P_FIMV_NUM_TMV_BUFFERS_V6 2
> +
> +#define S5P_FIMV_MAX_FRAME_SIZE_V6 (2048 * 1024)
(2 * SZ_1M)
> +#define S5P_FIMV_NUM_PIXELS_IN_MB_ROW_V6 16
> +#define S5P_FIMV_NUM_PIXELS_IN_MB_COL_V6 16
> +
> +/* Buffer size requirements defined by hardware */
> +#define S5P_FIMV_TMV_BUFFER_SIZE_V6(w, h) ((w + 1) * (h + 1) * 8)
The arguments should be in parentheses in the expressions, i.e.
#define S5P_FIMV_TMV_BUFFER_SIZE_V6(w, h) (((w) + 1) * ((h) + 1) * 8)
> +#define S5P_FIMV_ME_BUFFER_SIZE_V6(imw, imh, mbw, mbh) \
> + (((((imw+63)/64) * 16) * \
> + (((imh+63)/64) * 16)) + \
> + ((((mbw*mbh)+31)/32) * 16))
Could be rewritten as:
#define S5P_FIMV_ME_BUFFER_SIZE_V6(imw, imh, mbw, mbh) \
((ALIGN(imw, 64) * ALIGN(imh, 64) * 256) + (ALIGN((mbw) * (mbh), 32) * 16))
> +#define S5P_FIMV_SCRATCH_BUF_SIZE_H264_DEC_V6(w, h) ((w * 192) + 64)
> +#define S5P_FIMV_SCRATCH_BUF_SIZE_MPEG4_DEC_V6(w, h) \
> + (w * (h * 64 + 144) + \
> + ((2048 + 15)/16 * h * 64) + \
> + ((2048 + 15)/16 * 256 + 8320))
(w * (h * 64 + 144) + (2048/16 * h * 64) + (2048/16 * 256 + 8320))
> +#define S5P_FIMV_SCRATCH_BUF_SIZE_VC1_DEC_V6(w, h) (2096 * (w + h + 1))
> +#define S5P_FIMV_SCRATCH_BUF_SIZE_H263_DEC_V6(w, h) (w * 400)
> +#define S5P_FIMV_SCRATCH_BUF_SIZE_VP8_DEC_V6(w, h) \
> + (w * 32 + h * 128 + \
> + ((w + 1) / 2) * 64 + 2112)
Unnecessarily broken into two lines.
> +#define S5P_FIMV_SCRATCH_BUF_SIZE_H264_ENC_V6(w, h) \
> + ((w * 64) + ((w + 1) * 16) + \
> + (4096 * 16))
Ditto.
> +#define S5P_FIMV_SCRATCH_BUF_SIZE_MPEG4_ENC_V6(w, h) \
> + ((w * 16) + ((w + 1) * 16))
> +
> +/* MFC Context buffer sizes */
> +#define MFC_CTX_BUF_SIZE_V6 0x7000 /* 28KB */
Perhaps we could use the SZ_* macro definitions, e.g. (28 * SZ_1K) ?
> +#define MFC_H264_DEC_CTX_BUF_SIZE_V6 0x200000 /* 1.6MB */
(1600 * SZ_1K) ...
> +#define MFC_OTHER_DEC_CTX_BUF_SIZE_V6 0x5000 /* 20KB */
> +#define MFC_H264_ENC_CTX_BUF_SIZE_V6 0x19000 /* 100KB */
> +#define MFC_OTHER_ENC_CTX_BUF_SIZE_V6 0x3000 /* 12KB */
> +
> +/* MFCv6 variant defines */
> +#define MAX_FW_SIZE_V6 0x100000 /* 1MB */
> +#define MAX_CPB_SIZE_V6 0x300000 /* 3MB */
... (3 * SZ_1M)
> +#define MFC_VERSION_V6 0x61
> +#define MFC_NUM_PORTS_V6 1
> +
> +#endif /* _REGS_FIMV_V6_H */
Thanks,
Sylwester
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v7 5/6] [media] s5p-mfc: MFCv6 register definitions
@ 2012-10-01 4:37 Arun Kumar K
2012-10-02 5:28 ` JeongTae Park
0 siblings, 1 reply; 5+ messages in thread
From: Arun Kumar K @ 2012-10-01 4:37 UTC (permalink / raw)
To: Sylwester Nawrocki
Cc: linux-media@vger.kernel.org, Kamil Debski, Jeongtae Park,
Jang-Hyuck Kim, peter Oh, NAVEEN KRISHNA CHATRADHI,
Marek Szyprowski, Sylwester Nawrocki, kmpark@infradead.org,
SUNIL JOSHI
Hi Sylwester,
Thank you for the comments.
Will make necessary changes and post updated patchset.
Regards
Arun
------- Original Message -------
Sender : Sylwester Nawrocki<sylvester.nawrocki@gmail.com>
Date : Sep 29, 2012 21:05 (GMT+05:30)
Title : Re: [PATCH v7 5/6] [media] s5p-mfc: MFCv6 register definitions
Hi Arun,
I have a few minor comments.
On 09/28/2012 07:04 PM, Arun Kumar K wrote:
> From: Jeongtae Park<jtp.park@samsung.com>
>
> Adds register definitions for MFC v6.x firmware
>
> Signed-off-by: Jeongtae Park<jtp.park@samsung.com>
> Signed-off-by: Janghyuck Kim<janghyuck.kim@samsung.com>
> Signed-off-by: Jaeryul Oh<jaeryul.oh@samsung.com>
> Signed-off-by: Naveen Krishna Chatradhi<ch.naveen@samsung.com>
> Signed-off-by: Arun Kumar K<arun.kk@samsung.com>
> ---
> drivers/media/platform/s5p-mfc/regs-mfc-v6.h | 408 ++++++++++++++++++++++++++
> 1 files changed, 408 insertions(+), 0 deletions(-)
> create mode 100644 drivers/media/platform/s5p-mfc/regs-mfc-v6.h
>
> diff --git a/drivers/media/platform/s5p-mfc/regs-mfc-v6.h b/drivers/media/platform/s5p-mfc/regs-mfc-v6.h
> new file mode 100644
> index 0000000..cce1841
> --- /dev/null
> +++ b/drivers/media/platform/s5p-mfc/regs-mfc-v6.h
> @@ -0,0 +1,408 @@
> +/*
> + * Register definition file for Samsung MFC V6.x Interface (FIMV) driver
> + *
> + * Copyright (c) 2012 Samsung Electronics
I believe the proper notation is
Copyright (c) 2012 Samsung Electronics Co., Ltd.
Please make sure it's correct in all files added in this series.
> + * http://www.samsung.com/
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#ifndef _REGS_FIMV_V6_H
> +#define _REGS_FIMV_V6_H
Please add
#include <linux/kernel.h>
#include <linux/sizes.h>
> +#define S5P_FIMV_REG_SIZE_V6 (S5P_FIMV_END_ADDR - S5P_FIMV_START_ADDR)
> +#define S5P_FIMV_REG_COUNT_V6 ((S5P_FIMV_END_ADDR - S5P_FIMV_START_ADDR) / 4)
> +
> +/* Number of bits that the buffer address should be shifted for particular
> + * MFC buffers. */
> +#define S5P_FIMV_MEM_OFFSET_V6 0
> +
> +#define S5P_FIMV_START_ADDR_V6 0x0000
> +#define S5P_FIMV_END_ADDR_V6 0xfd80
> +
> +#define S5P_FIMV_REG_CLEAR_BEGIN_V6 0xf000
> +#define S5P_FIMV_REG_CLEAR_COUNT_V6 1024
> +
> +/* Codec Common Registers */
> +#define S5P_FIMV_RISC_ON_V6 0x0000
> +#define S5P_FIMV_RISC2HOST_INT_V6 0x003C
Could you make sure all hex numbers are in lower case in this file ?
...
> +#define S5P_FIMV_NUM_TMV_BUFFERS_V6 2
> +
> +#define S5P_FIMV_MAX_FRAME_SIZE_V6 (2048 * 1024)
(2 * SZ_1M)
> +#define S5P_FIMV_NUM_PIXELS_IN_MB_ROW_V6 16
> +#define S5P_FIMV_NUM_PIXELS_IN_MB_COL_V6 16
> +
> +/* Buffer size requirements defined by hardware */
> +#define S5P_FIMV_TMV_BUFFER_SIZE_V6(w, h) ((w + 1) * (h + 1) * 8)
The arguments should be in parentheses in the expressions, i.e.
#define S5P_FIMV_TMV_BUFFER_SIZE_V6(w, h) (((w) + 1) * ((h) + 1) * 8)
> +#define S5P_FIMV_ME_BUFFER_SIZE_V6(imw, imh, mbw, mbh) \r
> + (((((imw+63)/64) * 16) * \r
> + (((imh+63)/64) * 16)) + \r
> + ((((mbw*mbh)+31)/32) * 16))
Could be rewritten as:
#define S5P_FIMV_ME_BUFFER_SIZE_V6(imw, imh, mbw, mbh) \r
((ALIGN(imw, 64) * ALIGN(imh, 64) * 256) + (ALIGN((mbw) * (mbh), 32) * 16))
> +#define S5P_FIMV_SCRATCH_BUF_SIZE_H264_DEC_V6(w, h) ((w * 192) + 64)
> +#define S5P_FIMV_SCRATCH_BUF_SIZE_MPEG4_DEC_V6(w, h) \r
> + (w * (h * 64 + 144) + \r
> + ((2048 + 15)/16 * h * 64) + \r
> + ((2048 + 15)/16 * 256 + 8320))
(w * (h * 64 + 144) + (2048/16 * h * 64) + (2048/16 * 256 + 8320))
> +#define S5P_FIMV_SCRATCH_BUF_SIZE_VC1_DEC_V6(w, h) (2096 * (w + h + 1))
> +#define S5P_FIMV_SCRATCH_BUF_SIZE_H263_DEC_V6(w, h) (w * 400)
> +#define S5P_FIMV_SCRATCH_BUF_SIZE_VP8_DEC_V6(w, h) \r
> + (w * 32 + h * 128 + \r
> + ((w + 1) / 2) * 64 + 2112)
Unnecessarily broken into two lines.
> +#define S5P_FIMV_SCRATCH_BUF_SIZE_H264_ENC_V6(w, h) \r
> + ((w * 64) + ((w + 1) * 16) + \r
> + (4096 * 16))
Ditto.
> +#define S5P_FIMV_SCRATCH_BUF_SIZE_MPEG4_ENC_V6(w, h) \r
> + ((w * 16) + ((w + 1) * 16))
> +
> +/* MFC Context buffer sizes */
> +#define MFC_CTX_BUF_SIZE_V6 0x7000 /* 28KB */
Perhaps we could use the SZ_* macro definitions, e.g. (28 * SZ_1K) ?
> +#define MFC_H264_DEC_CTX_BUF_SIZE_V6 0x200000 /* 1.6MB */
(1600 * SZ_1K) ...
> +#define MFC_OTHER_DEC_CTX_BUF_SIZE_V6 0x5000 /* 20KB */
> +#define MFC_H264_ENC_CTX_BUF_SIZE_V6 0x19000 /* 100KB */
> +#define MFC_OTHER_ENC_CTX_BUF_SIZE_V6 0x3000 /* 12KB */
> +
> +/* MFCv6 variant defines */
> +#define MAX_FW_SIZE_V6 0x100000 /* 1MB */
> +#define MAX_CPB_SIZE_V6 0x300000 /* 3MB */
... (3 * SZ_1M)
> +#define MFC_VERSION_V6 0x61
> +#define MFC_NUM_PORTS_V6 1
> +
> +#endif /* _REGS_FIMV_V6_H */
Thanks,
Sylwester
<p> </p><p> </p>
^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [PATCH v7 5/6] [media] s5p-mfc: MFCv6 register definitions
2012-10-01 4:37 [PATCH v7 5/6] [media] s5p-mfc: MFCv6 register definitions Arun Kumar K
@ 2012-10-02 5:28 ` JeongTae Park
0 siblings, 0 replies; 5+ messages in thread
From: JeongTae Park @ 2012-10-02 5:28 UTC (permalink / raw)
To: arun.kk, 'Sylwester Nawrocki'
Cc: linux-media, 'Kamil Debski', 'Jang-Hyuck Kim',
'peter Oh', 'NAVEEN KRISHNA CHATRADHI',
'Marek Szyprowski', 'Sylwester Nawrocki', kmpark,
'SUNIL JOSHI'
Hi Arun and Sylwester,
Please make sure that below calculations are same.
> +#define S5P_FIMV_ME_BUFFER_SIZE_V6(imw, imh, mbw, mbh) \r
> + (((((imw+63)/64) * 16) * \r
> + (((imh+63)/64) * 16)) + \r
> + ((((mbw*mbh)+31)/32) * 16))
#define S5P_FIMV_ME_BUFFER_SIZE_V6(imw, imh, mbw, mbh) \r
((ALIGN(imw, 64) * ALIGN(imh, 64) * 256) + (ALIGN((mbw) * (mbh), 32) * 16))
Best Regards
/jtpark
> -----Original Message-----
> From: Arun Kumar K [mailto:arun.kk@samsung.com]
> Sent: Monday, October 01, 2012 1:37 PM
> To: Sylwester Nawrocki
> Cc: linux-media@vger.kernel.org; Kamil Debski; Jeongtae Park; Jang-Hyuck Kim; peter Oh; NAVEEN KRISHNA
> CHATRADHI; Marek Szyprowski; Sylwester Nawrocki; kmpark@infradead.org; SUNIL JOSHI
> Subject: Re: [PATCH v7 5/6] [media] s5p-mfc: MFCv6 register definitions
>
> Hi Sylwester,
> Thank you for the comments.
> Will make necessary changes and post updated patchset.
>
> Regards
> Arun
>
> ------- Original Message -------
> Sender : Sylwester Nawrocki<sylvester.nawrocki@gmail.com>
> Date : Sep 29, 2012 21:05 (GMT+05:30)
> Title : Re: [PATCH v7 5/6] [media] s5p-mfc: MFCv6 register definitions
>
> Hi Arun,
>
> I have a few minor comments.
>
> On 09/28/2012 07:04 PM, Arun Kumar K wrote:
> > From: Jeongtae Park<jtp.park@samsung.com>
> >
> > Adds register definitions for MFC v6.x firmware
> >
> > Signed-off-by: Jeongtae Park<jtp.park@samsung.com>
> > Signed-off-by: Janghyuck Kim<janghyuck.kim@samsung.com>
> > Signed-off-by: Jaeryul Oh<jaeryul.oh@samsung.com>
> > Signed-off-by: Naveen Krishna Chatradhi<ch.naveen@samsung.com>
> > Signed-off-by: Arun Kumar K<arun.kk@samsung.com>
> > ---
> > drivers/media/platform/s5p-mfc/regs-mfc-v6.h | 408 ++++++++++++++++++++++++++
> > 1 files changed, 408 insertions(+), 0 deletions(-)
> > create mode 100644 drivers/media/platform/s5p-mfc/regs-mfc-v6.h
> >
> > diff --git a/drivers/media/platform/s5p-mfc/regs-mfc-v6.h b/drivers/media/platform/s5p-mfc/regs-mfc-
> v6.h
> > new file mode 100644
> > index 0000000..cce1841
> > --- /dev/null
> > +++ b/drivers/media/platform/s5p-mfc/regs-mfc-v6.h
> > @@ -0,0 +1,408 @@
> > +/*
> > + * Register definition file for Samsung MFC V6.x Interface (FIMV) driver
> > + *
> > + * Copyright (c) 2012 Samsung Electronics
>
> I believe the proper notation is
>
> Copyright (c) 2012 Samsung Electronics Co., Ltd.
>
> Please make sure it's correct in all files added in this series.
>
> > + * http://www.samsung.com/
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License version 2 as
> > + * published by the Free Software Foundation.
> > + */
> > +
> > +#ifndef _REGS_FIMV_V6_H
> > +#define _REGS_FIMV_V6_H
>
> Please add
>
> #include <linux/kernel.h>
> #include <linux/sizes.h>
>
> > +#define S5P_FIMV_REG_SIZE_V6 (S5P_FIMV_END_ADDR - S5P_FIMV_START_ADDR)
> > +#define S5P_FIMV_REG_COUNT_V6 ((S5P_FIMV_END_ADDR - S5P_FIMV_START_ADDR) / 4)
> > +
> > +/* Number of bits that the buffer address should be shifted for particular
> > + * MFC buffers. */
> > +#define S5P_FIMV_MEM_OFFSET_V6 0
> > +
> > +#define S5P_FIMV_START_ADDR_V6 0x0000
> > +#define S5P_FIMV_END_ADDR_V6 0xfd80
> > +
> > +#define S5P_FIMV_REG_CLEAR_BEGIN_V6 0xf000
> > +#define S5P_FIMV_REG_CLEAR_COUNT_V6 1024
> > +
> > +/* Codec Common Registers */
> > +#define S5P_FIMV_RISC_ON_V6 0x0000
> > +#define S5P_FIMV_RISC2HOST_INT_V6 0x003C
>
> Could you make sure all hex numbers are in lower case in this file ?
>
> ...
> > +#define S5P_FIMV_NUM_TMV_BUFFERS_V6 2
> > +
> > +#define S5P_FIMV_MAX_FRAME_SIZE_V6 (2048 * 1024)
>
> (2 * SZ_1M)
>
> > +#define S5P_FIMV_NUM_PIXELS_IN_MB_ROW_V6 16
> > +#define S5P_FIMV_NUM_PIXELS_IN_MB_COL_V6 16
> > +
> > +/* Buffer size requirements defined by hardware */
> > +#define S5P_FIMV_TMV_BUFFER_SIZE_V6(w, h) ((w + 1) * (h + 1) * 8)
>
> The arguments should be in parentheses in the expressions, i.e.
>
> #define S5P_FIMV_TMV_BUFFER_SIZE_V6(w, h) (((w) + 1) * ((h) + 1) * 8)
>
> > +#define S5P_FIMV_ME_BUFFER_SIZE_V6(imw, imh, mbw, mbh) \r
> > + (((((imw+63)/64) * 16) * \r
> > + (((imh+63)/64) * 16)) + \r
> > + ((((mbw*mbh)+31)/32) * 16))
>
> Could be rewritten as:
>
> #define S5P_FIMV_ME_BUFFER_SIZE_V6(imw, imh, mbw, mbh) \r
> ((ALIGN(imw, 64) * ALIGN(imh, 64) * 256) + (ALIGN((mbw) * (mbh), 32) * 16))
>
>
> > +#define S5P_FIMV_SCRATCH_BUF_SIZE_H264_DEC_V6(w, h) ((w * 192) + 64)
> > +#define S5P_FIMV_SCRATCH_BUF_SIZE_MPEG4_DEC_V6(w, h) \r
> > + (w * (h * 64 + 144) + \r
> > + ((2048 + 15)/16 * h * 64) + \r
> > + ((2048 + 15)/16 * 256 + 8320))
>
> (w * (h * 64 + 144) + (2048/16 * h * 64) + (2048/16 * 256 + 8320))
>
> > +#define S5P_FIMV_SCRATCH_BUF_SIZE_VC1_DEC_V6(w, h) (2096 * (w + h + 1))
> > +#define S5P_FIMV_SCRATCH_BUF_SIZE_H263_DEC_V6(w, h) (w * 400)
> > +#define S5P_FIMV_SCRATCH_BUF_SIZE_VP8_DEC_V6(w, h) \r
> > + (w * 32 + h * 128 + \r
> > + ((w + 1) / 2) * 64 + 2112)
>
> Unnecessarily broken into two lines.
>
> > +#define S5P_FIMV_SCRATCH_BUF_SIZE_H264_ENC_V6(w, h) \r
> > + ((w * 64) + ((w + 1) * 16) + \r
> > + (4096 * 16))
>
> Ditto.
>
> > +#define S5P_FIMV_SCRATCH_BUF_SIZE_MPEG4_ENC_V6(w, h) \r
> > + ((w * 16) + ((w + 1) * 16))
> > +
> > +/* MFC Context buffer sizes */
> > +#define MFC_CTX_BUF_SIZE_V6 0x7000 /* 28KB */
>
> Perhaps we could use the SZ_* macro definitions, e.g. (28 * SZ_1K) ?
>
> > +#define MFC_H264_DEC_CTX_BUF_SIZE_V6 0x200000 /* 1.6MB */
>
> (1600 * SZ_1K) ...
>
> > +#define MFC_OTHER_DEC_CTX_BUF_SIZE_V6 0x5000 /* 20KB */
> > +#define MFC_H264_ENC_CTX_BUF_SIZE_V6 0x19000 /* 100KB */
> > +#define MFC_OTHER_ENC_CTX_BUF_SIZE_V6 0x3000 /* 12KB */
> > +
> > +/* MFCv6 variant defines */
> > +#define MAX_FW_SIZE_V6 0x100000 /* 1MB */
> > +#define MAX_CPB_SIZE_V6 0x300000 /* 3MB */
>
> ... (3 * SZ_1M)
>
> > +#define MFC_VERSION_V6 0x61
> > +#define MFC_NUM_PORTS_V6 1
> > +
> > +#endif /* _REGS_FIMV_V6_H */
>
> Thanks,
> Sylwester
> <p> </p><p> </p>
^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [PATCH v7 5/6] [media] s5p-mfc: MFCv6 register definitions
@ 2012-10-02 6:03 Arun Kumar K
2012-10-02 8:24 ` Sylwester Nawrocki
0 siblings, 1 reply; 5+ messages in thread
From: Arun Kumar K @ 2012-10-02 6:03 UTC (permalink / raw)
To: Jeongtae Park, 'Sylwester Nawrocki'
Cc: linux-media@vger.kernel.org, Kamil Debski, Jang-Hyuck Kim,
peter Oh, NAVEEN KRISHNA CHATRADHI, Marek Szyprowski,
Sylwester Nawrocki, kmpark@infradead.org, SUNIL JOSHI
Hi Jeongtae Park,
I have verified the calculation.
The ALIGN macro is giving a wrong result and I used the macro DIV_ROUND_UP
in the v8 patchset which is giving the proper result.
Regards
Arun
On Tue, Oct 2, 2012 at 10:58 AM, JeongTae Park <jtp.park@samsung.com> wrote:
> Hi Arun and Sylwester,
>
> Please make sure that below calculations are same.
>
>> +#define S5P_FIMV_ME_BUFFER_SIZE_V6(imw, imh, mbw, mbh) \r
>> + (((((imw+63)/64) * 16) * \r
>> + (((imh+63)/64) * 16)) + \r
>> + ((((mbw*mbh)+31)/32) * 16))
>
>
> #define S5P_FIMV_ME_BUFFER_SIZE_V6(imw, imh, mbw, mbh) \r
> ((ALIGN(imw, 64) * ALIGN(imh, 64) * 256) + (ALIGN((mbw) * (mbh), 32) * 16))
>
> Best Regards
> /jtpark
>
>> -----Original Message-----
>> From: Arun Kumar K [mailto:arun.kk@samsung.com]
>> Sent: Monday, October 01, 2012 1:37 PM
>> To: Sylwester Nawrocki
>> Cc: linux-media@vger.kernel.org; Kamil Debski; Jeongtae Park; Jang-Hyuck Kim; peter Oh; NAVEEN KRISHNA
>> CHATRADHI; Marek Szyprowski; Sylwester Nawrocki; kmpark@infradead.org; SUNIL JOSHI
>> Subject: Re: [PATCH v7 5/6] [media] s5p-mfc: MFCv6 register definitions
>>
>> Hi Sylwester,
>> Thank you for the comments.
>> Will make necessary changes and post updated patchset.
>>
>> Regards
>> Arun
>>
>> ------- Original Message -------
>> Sender : Sylwester Nawrocki<sylvester.nawrocki@gmail.com>
>> Date : Sep 29, 2012 21:05 (GMT+05:30)
>> Title : Re: [PATCH v7 5/6] [media] s5p-mfc: MFCv6 register definitions
>>
>> Hi Arun,
>>
>> I have a few minor comments.
>>
>> On 09/28/2012 07:04 PM, Arun Kumar K wrote:
>> > From: Jeongtae Park<jtp.park@samsung.com>
>> >
>> > Adds register definitions for MFC v6.x firmware
>> >
>> > Signed-off-by: Jeongtae Park<jtp.park@samsung.com>
>> > Signed-off-by: Janghyuck Kim<janghyuck.kim@samsung.com>
>> > Signed-off-by: Jaeryul Oh<jaeryul.oh@samsung.com>
>> > Signed-off-by: Naveen Krishna Chatradhi<ch.naveen@samsung.com>
>> > Signed-off-by: Arun Kumar K<arun.kk@samsung.com>
>> > ---
>> > drivers/media/platform/s5p-mfc/regs-mfc-v6.h | 408 ++++++++++++++++++++++++++
>> > 1 files changed, 408 insertions(+), 0 deletions(-)
>> > create mode 100644 drivers/media/platform/s5p-mfc/regs-mfc-v6.h
>> >
>> > diff --git a/drivers/media/platform/s5p-mfc/regs-mfc-v6.h b/drivers/media/platform/s5p-mfc/regs-mfc-
>> v6.h
>> > new file mode 100644
>> > index 0000000..cce1841
>> > --- /dev/null
>> > +++ b/drivers/media/platform/s5p-mfc/regs-mfc-v6.h
>> > @@ -0,0 +1,408 @@
>> > +/*
>> > + * Register definition file for Samsung MFC V6.x Interface (FIMV) driver
>> > + *
>> > + * Copyright (c) 2012 Samsung Electronics
>>
>> I believe the proper notation is
>>
>> Copyright (c) 2012 Samsung Electronics Co., Ltd.
>>
>> Please make sure it's correct in all files added in this series.
>>
>> > + * http://www.samsung.com/
>> > + *
>> > + * This program is free software; you can redistribute it and/or modify
>> > + * it under the terms of the GNU General Public License version 2 as
>> > + * published by the Free Software Foundation.
>> > + */
>> > +
>> > +#ifndef _REGS_FIMV_V6_H
>> > +#define _REGS_FIMV_V6_H
>>
>> Please add
>>
>> #include <linux/kernel.h>
>> #include <linux/sizes.h>
>>
>> > +#define S5P_FIMV_REG_SIZE_V6 (S5P_FIMV_END_ADDR - S5P_FIMV_START_ADDR)
>> > +#define S5P_FIMV_REG_COUNT_V6 ((S5P_FIMV_END_ADDR - S5P_FIMV_START_ADDR) / 4)
>> > +
>> > +/* Number of bits that the buffer address should be shifted for particular
>> > + * MFC buffers. */
>> > +#define S5P_FIMV_MEM_OFFSET_V6 0
>> > +
>> > +#define S5P_FIMV_START_ADDR_V6 0x0000
>> > +#define S5P_FIMV_END_ADDR_V6 0xfd80
>> > +
>> > +#define S5P_FIMV_REG_CLEAR_BEGIN_V6 0xf000
>> > +#define S5P_FIMV_REG_CLEAR_COUNT_V6 1024
>> > +
>> > +/* Codec Common Registers */
>> > +#define S5P_FIMV_RISC_ON_V6 0x0000
>> > +#define S5P_FIMV_RISC2HOST_INT_V6 0x003C
>>
>> Could you make sure all hex numbers are in lower case in this file ?
>>
>> ...
>> > +#define S5P_FIMV_NUM_TMV_BUFFERS_V6 2
>> > +
>> > +#define S5P_FIMV_MAX_FRAME_SIZE_V6 (2048 * 1024)
>>
>> (2 * SZ_1M)
>>
>> > +#define S5P_FIMV_NUM_PIXELS_IN_MB_ROW_V6 16
>> > +#define S5P_FIMV_NUM_PIXELS_IN_MB_COL_V6 16
>> > +
>> > +/* Buffer size requirements defined by hardware */
>> > +#define S5P_FIMV_TMV_BUFFER_SIZE_V6(w, h) ((w + 1) * (h + 1) * 8)
>>
>> The arguments should be in parentheses in the expressions, i.e.
>>
>> #define S5P_FIMV_TMV_BUFFER_SIZE_V6(w, h) (((w) + 1) * ((h) + 1) * 8)
>>
>> > +#define S5P_FIMV_ME_BUFFER_SIZE_V6(imw, imh, mbw, mbh) \r
>> > + (((((imw+63)/64) * 16) * \r
>> > + (((imh+63)/64) * 16)) + \r
>> > + ((((mbw*mbh)+31)/32) * 16))
>>
>> Could be rewritten as:
>>
>> #define S5P_FIMV_ME_BUFFER_SIZE_V6(imw, imh, mbw, mbh) \r
>> ((ALIGN(imw, 64) * ALIGN(imh, 64) * 256) + (ALIGN((mbw) * (mbh), 32) * 16))
>>
>>
>> > +#define S5P_FIMV_SCRATCH_BUF_SIZE_H264_DEC_V6(w, h) ((w * 192) + 64)
>> > +#define S5P_FIMV_SCRATCH_BUF_SIZE_MPEG4_DEC_V6(w, h) \r
>> > + (w * (h * 64 + 144) + \r
>> > + ((2048 + 15)/16 * h * 64) + \r
>> > + ((2048 + 15)/16 * 256 + 8320))
>>
>> (w * (h * 64 + 144) + (2048/16 * h * 64) + (2048/16 * 256 + 8320))
>>
>> > +#define S5P_FIMV_SCRATCH_BUF_SIZE_VC1_DEC_V6(w, h) (2096 * (w + h + 1))
>> > +#define S5P_FIMV_SCRATCH_BUF_SIZE_H263_DEC_V6(w, h) (w * 400)
>> > +#define S5P_FIMV_SCRATCH_BUF_SIZE_VP8_DEC_V6(w, h) \r
>> > + (w * 32 + h * 128 + \r
>> > + ((w + 1) / 2) * 64 + 2112)
>>
>> Unnecessarily broken into two lines.
>>
>> > +#define S5P_FIMV_SCRATCH_BUF_SIZE_H264_ENC_V6(w, h) \r
>> > + ((w * 64) + ((w + 1) * 16) + \r
>> > + (4096 * 16))
>>
>> Ditto.
>>
>> > +#define S5P_FIMV_SCRATCH_BUF_SIZE_MPEG4_ENC_V6(w, h) \r
>> > + ((w * 16) + ((w + 1) * 16))
>> > +
>> > +/* MFC Context buffer sizes */
>> > +#define MFC_CTX_BUF_SIZE_V6 0x7000 /* 28KB */
>>
>> Perhaps we could use the SZ_* macro definitions, e.g. (28 * SZ_1K) ?
>>
>> > +#define MFC_H264_DEC_CTX_BUF_SIZE_V6 0x200000 /* 1.6MB */
>>
>> (1600 * SZ_1K) ...
>>
>> > +#define MFC_OTHER_DEC_CTX_BUF_SIZE_V6 0x5000 /* 20KB */
>> > +#define MFC_H264_ENC_CTX_BUF_SIZE_V6 0x19000 /* 100KB */
>> > +#define MFC_OTHER_ENC_CTX_BUF_SIZE_V6 0x3000 /* 12KB */
>> > +
>> > +/* MFCv6 variant defines */
>> > +#define MAX_FW_SIZE_V6 0x100000 /* 1MB */
>> > +#define MAX_CPB_SIZE_V6 0x300000 /* 3MB */
>>
>> ... (3 * SZ_1M)
>>
>> > +#define MFC_VERSION_V6 0x61
>> > +#define MFC_NUM_PORTS_V6 1
>> > +
>> > +#endif /* _REGS_FIMV_V6_H */
>>
>> Thanks,
>> Sylwester
>> <p> </p><p> </p>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v7 5/6] [media] s5p-mfc: MFCv6 register definitions
2012-10-02 6:03 Arun Kumar K
@ 2012-10-02 8:24 ` Sylwester Nawrocki
0 siblings, 0 replies; 5+ messages in thread
From: Sylwester Nawrocki @ 2012-10-02 8:24 UTC (permalink / raw)
To: arun.kk
Cc: Jeongtae Park, 'Sylwester Nawrocki',
linux-media@vger.kernel.org, Kamil Debski, Jang-Hyuck Kim,
peter Oh, NAVEEN KRISHNA CHATRADHI, Marek Szyprowski,
kmpark@infradead.org, SUNIL JOSHI
Hi Arun,
On 10/02/2012 08:03 AM, Arun Kumar K wrote:
> Hi Jeongtae Park,
>
> I have verified the calculation.
> The ALIGN macro is giving a wrong result and I used the macro DIV_ROUND_UP
> in the v8 patchset which is giving the proper result.
Sorry about this misleading suggestion. It should have been DIV_ROUND_UP
indeed. Your v9 series in general looks good to me, I'd like to send a pull
request with Kamil's Ack today.
Well done, thanks!
Regards,
Sylwester
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2012-10-02 8:24 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-10-01 4:37 [PATCH v7 5/6] [media] s5p-mfc: MFCv6 register definitions Arun Kumar K
2012-10-02 5:28 ` JeongTae Park
-- strict thread matches above, loose matches on Subject: below --
2012-10-02 6:03 Arun Kumar K
2012-10-02 8:24 ` Sylwester Nawrocki
[not found] <1348851868-7698-1-git-send-email-arun.kk@samsung.com>
[not found] ` <1348851868-7698-6-git-send-email-arun.kk@samsung.com>
2012-09-29 15:35 ` Sylwester Nawrocki
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).