Re: [PATCH v7 5/6] [media] s5p-mfc: MFCv6 register definitions
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 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v7 5/6] [media] s5p-mfc: MFCv6 register definitions
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 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...@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; kmp...@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 >> 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 >> > >> > Adds register definitions for MFC v6.x firmware >> > >> > Signed-off-by: Jeongtae Park >> > Signed-off-by: Janghyuck Kim >> > Signed-off-by: Jaeryul Oh >> > Signed-off-by: Naveen Krishna Chatradhi >> > Signed-off-by: Arun Kumar K >> > --- >> > 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 000..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 >> #include >> >> > +#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 0x >> > +#define S5P_FIMV_END_ADDR_V6 0xfd80 >> > + >> > +#define S5P_FIMV_REG_CLEAR_BEGIN_V60xf000 >> > +#define S5P_FIMV_REG_CLEAR_COUNT_V61024 >> > + >> > +/* Codec Common Registers */ >> > +#define S5P_FIMV_RISC_ON_V60x >> > +#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_V62 >> > + >> > +#define S5P_FIMV_MAX_FRAME_SIZE_V6 (2048 * 1024) >> >> (2 * SZ_1M) >> >> > +#define S
RE: [PATCH v7 5/6] [media] s5p-mfc: MFCv6 register definitions
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...@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; kmp...@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 > 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 > > > > Adds register definitions for MFC v6.x firmware > > > > Signed-off-by: Jeongtae Park > > Signed-off-by: Janghyuck Kim > > Signed-off-by: Jaeryul Oh > > Signed-off-by: Naveen Krishna Chatradhi > > Signed-off-by: Arun Kumar K > > --- > > 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 000..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 > #include > > > +#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 0x > > +#define S5P_FIMV_END_ADDR_V6 0xfd80 > > + > > +#define S5P_FIMV_REG_CLEAR_BEGIN_V60xf000 > > +#define S5P_FIMV_REG_CLEAR_COUNT_V61024 > > + > > +/* Codec Common Registers */ > > +#define S5P_FIMV_RISC_ON_V60x > > +#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_V62 > > + > > +#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*mb
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 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 > > Adds register definitions for MFC v6.x firmware > > Signed-off-by: Jeongtae Park > Signed-off-by: Janghyuck Kim > Signed-off-by: Jaeryul Oh > Signed-off-by: Naveen Krishna Chatradhi > Signed-off-by: Arun Kumar K > --- > 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 000..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 #include > +#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 0x > +#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 0x > +#define S5P_FIMV_RISC2HOST_INT_V60x003C 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 th
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 > > Adds register definitions for MFC v6.x firmware > > Signed-off-by: Jeongtae Park > Signed-off-by: Janghyuck Kim > Signed-off-by: Jaeryul Oh > Signed-off-by: Naveen Krishna Chatradhi > Signed-off-by: Arun Kumar K > --- > 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 000..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 #include > +#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 0x > +#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 0x > +#define S5P_FIMV_RISC2HOST_INT_V60x003C 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 0x20/* 1.6MB */ (1600 * SZ_1K) ... > +#define MFC_OTHER_DEC_CTX_BUF_SIZE_V60x5000 /* 20KB */ > +#define MFC_H264_ENC_CTX_BUF_SIZE_V6 0x19000 /* 100KB */ > +#define MFC_OTHER_ENC_CTX_BUF_SIZE_V60x3000 /* 12KB */ > + > +/* MFCv6 variant defines */ > +#define MAX_FW_SIZE_V6 0x10/* 1MB */ > +#define MAX_CPB_SIZE_V6 0x30/* 3MB */ ... (3 * SZ_1M) >
