Re: [PATCH 1/5 v4] V4L/DVB: s5p-fimc: Register definition cleanup

2010-10-11 Thread Sylwester Nawrocki
Hi Seuni,

thanks for you review!

On 10/11/2010 07:09 AM, 박세운 wrote:
 Sewoon Park wrote:
 
 Add MIPI CSI format definitions, prepare DMA address
 definitions for interlaced input frame mode.

 Signed-off-by: Sylwester Nawrocki s.nawro...@samsung.com
 Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
 ---
  drivers/media/video/s5p-fimc/fimc-reg.c  |6 +-
  drivers/media/video/s5p-fimc/regs-fimc.h |   61 -
 -
 ---
  2 files changed, 28 insertions(+), 39 deletions(-)

 diff --git a/drivers/media/video/s5p-fimc/fimc-reg.c
 b/drivers/media/video/s5p-fimc/fimc-reg.c
 index 5570f1c..70f29c5 100644
 --- a/drivers/media/video/s5p-fimc/fimc-reg.c
 +++ b/drivers/media/video/s5p-fimc/fimc-reg.c
 @@ -507,9 +507,9 @@ void fimc_hw_set_input_addr(struct fimc_dev *dev,
 struct fimc_addr *paddr)
  cfg |= S5P_CIREAL_ISIZE_ADDR_CH_DIS;
  writel(cfg, dev-regs + S5P_CIREAL_ISIZE);

 -writel(paddr-y, dev-regs + S5P_CIIYSA0);
 -writel(paddr-cb, dev-regs + S5P_CIICBSA0);
 -writel(paddr-cr, dev-regs + S5P_CIICRSA0);
 +writel(paddr-y, dev-regs + S5P_CIIYSA(0));
 +writel(paddr-cb, dev-regs + S5P_CIICBSA(0));
 +writel(paddr-cr, dev-regs + S5P_CIICRSA(0));

  cfg = ~S5P_CIREAL_ISIZE_ADDR_CH_DIS;
  writel(cfg, dev-regs + S5P_CIREAL_ISIZE);
 diff --git a/drivers/media/video/s5p-fimc/regs-fimc.h
 b/drivers/media/video/s5p-fimc/regs-fimc.h
 index a3cfe82..9e83315 100644
 --- a/drivers/media/video/s5p-fimc/regs-fimc.h
 +++ b/drivers/media/video/s5p-fimc/regs-fimc.h
 @@ -11,10 +11,6 @@
  #ifndef REGS_FIMC_H_
  #define REGS_FIMC_H_

 -#define S5P_CIOYSA(__x) (0x18 + (__x) * 4)
 -#define S5P_CIOCBSA(__x)(0x28 + (__x) * 4)
 -#define S5P_CIOCRSA(__x)(0x38 + (__x) * 4)
 -
  /* Input source format */
  #define S5P_CISRCFMT0x00
  #define S5P_CISRCFMT_ITU601_8BIT(1  31)
 @@ -28,22 +24,21 @@

  /* Window offset */
  #define S5P_CIWDOFST0x04
 -#define S5P_CIWDOFST_WINOFSEN   (1  31)
 +#define S5P_CIWDOFST_OFF_EN (1  31)
  #define S5P_CIWDOFST_CLROVFIY   (1  30)
  #define S5P_CIWDOFST_CLROVRLB   (1  29)
 -#define S5P_CIWDOFST_WINHOROFST_MASK(0x7ff  16)
 +#define S5P_CIWDOFST_HOROFF_MASK(0x7ff  16)
  #define S5P_CIWDOFST_CLROVFICB  (1  15)
  #define S5P_CIWDOFST_CLROVFICR  (1  14)
 -#define S5P_CIWDOFST_WINHOROFST(x)  ((x)  16)
 -#define S5P_CIWDOFST_WINVEROFST(x)  ((x)  0)
 -#define S5P_CIWDOFST_WINVEROFST_MASK(0xfff  0)
 +#define S5P_CIWDOFST_HOROFF(x)  ((x)  16)
 +#define S5P_CIWDOFST_VEROFF(x)  ((x)  0)
 +#define S5P_CIWDOFST_VEROFF_MASK(0xfff  0)

  /* Global control */
  #define S5P_CIGCTRL 0x08
  #define S5P_CIGCTRL_SWRST   (1  31)
  #define S5P_CIGCTRL_CAMRST_A(1  30)
  #define S5P_CIGCTRL_SELCAM_ITU_A(1  29)
 -#define S5P_CIGCTRL_SELCAM_ITU_MASK (1  29)
  #define S5P_CIGCTRL_TESTPAT_NORMAL  (0  27)
  #define S5P_CIGCTRL_TESTPAT_COLOR_BAR   (1  27)
  #define S5P_CIGCTRL_TESTPAT_HOR_INC (2  27)
 @@ -61,6 +56,8 @@
  #define S5P_CIGCTRL_SHDW_DISABLE(1  12)
  #define S5P_CIGCTRL_SELCAM_MIPI_A   (1  7)
  #define S5P_CIGCTRL_CAMIF_SELWB (1  6)
 +/* 0 - ITU601; 1 - ITU709 */
 +#define S5P_CIGCTRL_CSC_ITU601_709  (1  5)
  #define S5P_CIGCTRL_INVPOLHSYNC (1  4)
  #define S5P_CIGCTRL_SELCAM_MIPI (1  3)
  #define S5P_CIGCTRL_INTERLACE   (1  0)
 @@ -72,23 +69,10 @@
  #define S5P_CIWDOFST2_HOROFF(x) ((x)  16)
  #define S5P_CIWDOFST2_VEROFF(x) ((x)  0)

 -/* Output DMA Y plane start address */
 -#define S5P_CIOYSA1 0x18
 -#define S5P_CIOYSA2 0x1c
 -#define S5P_CIOYSA3 0x20
 -#define S5P_CIOYSA4 0x24
 -
 -/* Output DMA Cb plane start address */
 -#define S5P_CIOCBSA10x28
 -#define S5P_CIOCBSA20x2c
 -#define S5P_CIOCBSA30x30
 -#define S5P_CIOCBSA40x34
 -
 -/* Output DMA Cr plane start address */
 -#define S5P_CIOCRSA10x38
 -#define S5P_CIOCRSA20x3c
 -#define S5P_CIOCRSA30x40
 -#define S5P_CIOCRSA40x44
 +/* Output DMA Y/Cb/Cr plane start addresses */
 +#define S5P_CIOYSA(n)   (0x18 + (n) * 4)
 +#define S5P_CIOCBSA(n)  (0x28 + (n) * 4)
 +#define S5P_CIOCRSA(n)  (0x38 + (n) * 4)

 As you know, S5P series have S5PC210 also.
 Then,
 Why don't you add S5PC210's 32 registers for Output DMA start address some
 other time?
 For the next, the better approach would be to do something like:
 
 #define S5P_CIOYSA1   0x18/* Y 1st frame start address for output DMA
 */
 #define S5P_CIOYSA5 0x200 /* Y 5th frame start address for output DMA */
 #define 

RE: [PATCH 1/5 v4] V4L/DVB: s5p-fimc: Register definition cleanup

2010-10-10 Thread 박세운
Sewoon Park wrote:

 Add MIPI CSI format definitions, prepare DMA address
 definitions for interlaced input frame mode.
 
 Signed-off-by: Sylwester Nawrocki s.nawro...@samsung.com
 Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
 ---
  drivers/media/video/s5p-fimc/fimc-reg.c  |6 +-
  drivers/media/video/s5p-fimc/regs-fimc.h |   61 -
-
 ---
  2 files changed, 28 insertions(+), 39 deletions(-)
 
 diff --git a/drivers/media/video/s5p-fimc/fimc-reg.c
 b/drivers/media/video/s5p-fimc/fimc-reg.c
 index 5570f1c..70f29c5 100644
 --- a/drivers/media/video/s5p-fimc/fimc-reg.c
 +++ b/drivers/media/video/s5p-fimc/fimc-reg.c
 @@ -507,9 +507,9 @@ void fimc_hw_set_input_addr(struct fimc_dev *dev,
 struct fimc_addr *paddr)
   cfg |= S5P_CIREAL_ISIZE_ADDR_CH_DIS;
   writel(cfg, dev-regs + S5P_CIREAL_ISIZE);
 
 - writel(paddr-y, dev-regs + S5P_CIIYSA0);
 - writel(paddr-cb, dev-regs + S5P_CIICBSA0);
 - writel(paddr-cr, dev-regs + S5P_CIICRSA0);
 + writel(paddr-y, dev-regs + S5P_CIIYSA(0));
 + writel(paddr-cb, dev-regs + S5P_CIICBSA(0));
 + writel(paddr-cr, dev-regs + S5P_CIICRSA(0));
 
   cfg = ~S5P_CIREAL_ISIZE_ADDR_CH_DIS;
   writel(cfg, dev-regs + S5P_CIREAL_ISIZE);
 diff --git a/drivers/media/video/s5p-fimc/regs-fimc.h
 b/drivers/media/video/s5p-fimc/regs-fimc.h
 index a3cfe82..9e83315 100644
 --- a/drivers/media/video/s5p-fimc/regs-fimc.h
 +++ b/drivers/media/video/s5p-fimc/regs-fimc.h
 @@ -11,10 +11,6 @@
  #ifndef REGS_FIMC_H_
  #define REGS_FIMC_H_
 
 -#define S5P_CIOYSA(__x)  (0x18 + (__x) * 4)
 -#define S5P_CIOCBSA(__x) (0x28 + (__x) * 4)
 -#define S5P_CIOCRSA(__x) (0x38 + (__x) * 4)
 -
  /* Input source format */
  #define S5P_CISRCFMT 0x00
  #define S5P_CISRCFMT_ITU601_8BIT (1  31)
 @@ -28,22 +24,21 @@
 
  /* Window offset */
  #define S5P_CIWDOFST 0x04
 -#define S5P_CIWDOFST_WINOFSEN(1  31)
 +#define S5P_CIWDOFST_OFF_EN  (1  31)
  #define S5P_CIWDOFST_CLROVFIY(1  30)
  #define S5P_CIWDOFST_CLROVRLB(1  29)
 -#define S5P_CIWDOFST_WINHOROFST_MASK (0x7ff  16)
 +#define S5P_CIWDOFST_HOROFF_MASK (0x7ff  16)
  #define S5P_CIWDOFST_CLROVFICB   (1  15)
  #define S5P_CIWDOFST_CLROVFICR   (1  14)
 -#define S5P_CIWDOFST_WINHOROFST(x)   ((x)  16)
 -#define S5P_CIWDOFST_WINVEROFST(x)   ((x)  0)
 -#define S5P_CIWDOFST_WINVEROFST_MASK (0xfff  0)
 +#define S5P_CIWDOFST_HOROFF(x)   ((x)  16)
 +#define S5P_CIWDOFST_VEROFF(x)   ((x)  0)
 +#define S5P_CIWDOFST_VEROFF_MASK (0xfff  0)
 
  /* Global control */
  #define S5P_CIGCTRL  0x08
  #define S5P_CIGCTRL_SWRST(1  31)
  #define S5P_CIGCTRL_CAMRST_A (1  30)
  #define S5P_CIGCTRL_SELCAM_ITU_A (1  29)
 -#define S5P_CIGCTRL_SELCAM_ITU_MASK  (1  29)
  #define S5P_CIGCTRL_TESTPAT_NORMAL   (0  27)
  #define S5P_CIGCTRL_TESTPAT_COLOR_BAR(1  27)
  #define S5P_CIGCTRL_TESTPAT_HOR_INC  (2  27)
 @@ -61,6 +56,8 @@
  #define S5P_CIGCTRL_SHDW_DISABLE (1  12)
  #define S5P_CIGCTRL_SELCAM_MIPI_A(1  7)
  #define S5P_CIGCTRL_CAMIF_SELWB  (1  6)
 +/* 0 - ITU601; 1 - ITU709 */
 +#define S5P_CIGCTRL_CSC_ITU601_709   (1  5)
  #define S5P_CIGCTRL_INVPOLHSYNC  (1  4)
  #define S5P_CIGCTRL_SELCAM_MIPI  (1  3)
  #define S5P_CIGCTRL_INTERLACE(1  0)
 @@ -72,23 +69,10 @@
  #define S5P_CIWDOFST2_HOROFF(x)  ((x)  16)
  #define S5P_CIWDOFST2_VEROFF(x)  ((x)  0)
 
 -/* Output DMA Y plane start address */
 -#define S5P_CIOYSA1  0x18
 -#define S5P_CIOYSA2  0x1c
 -#define S5P_CIOYSA3  0x20
 -#define S5P_CIOYSA4  0x24
 -
 -/* Output DMA Cb plane start address */
 -#define S5P_CIOCBSA1 0x28
 -#define S5P_CIOCBSA2 0x2c
 -#define S5P_CIOCBSA3 0x30
 -#define S5P_CIOCBSA4 0x34
 -
 -/* Output DMA Cr plane start address */
 -#define S5P_CIOCRSA1 0x38
 -#define S5P_CIOCRSA2 0x3c
 -#define S5P_CIOCRSA3 0x40
 -#define S5P_CIOCRSA4 0x44
 +/* Output DMA Y/Cb/Cr plane start addresses */
 +#define S5P_CIOYSA(n)(0x18 + (n) * 4)
 +#define S5P_CIOCBSA(n)   (0x28 + (n) * 4)
 +#define S5P_CIOCRSA(n)   (0x38 + (n) * 4)
 
As you know, S5P series have S5PC210 also.
Then,
Why don't you add S5PC210's 32 registers for Output DMA start address some
other time?
For the next, the better approach would be to do something like:

#define S5P_CIOYSA1 0x18/* Y 1st frame start address for output DMA
*/
#define S5P_CIOYSA5 0x200 /* Y 5th frame start address for output DMA */
#define S5P_CIOYSA(n) \
(((n)  4) ? \
 (S5P_CIOYSA1  + (n) * 4) : (S5P_CIOYSA5  + ((n) - 4) * 4))


  /* Target 

[PATCH 1/5 v4] V4L/DVB: s5p-fimc: Register definition cleanup

2010-10-08 Thread Sylwester Nawrocki
Add MIPI CSI format definitions, prepare DMA address
definitions for interlaced input frame mode.

Signed-off-by: Sylwester Nawrocki s.nawro...@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
---
 drivers/media/video/s5p-fimc/fimc-reg.c  |6 +-
 drivers/media/video/s5p-fimc/regs-fimc.h |   61 -
 2 files changed, 28 insertions(+), 39 deletions(-)

diff --git a/drivers/media/video/s5p-fimc/fimc-reg.c 
b/drivers/media/video/s5p-fimc/fimc-reg.c
index 5570f1c..70f29c5 100644
--- a/drivers/media/video/s5p-fimc/fimc-reg.c
+++ b/drivers/media/video/s5p-fimc/fimc-reg.c
@@ -507,9 +507,9 @@ void fimc_hw_set_input_addr(struct fimc_dev *dev, struct 
fimc_addr *paddr)
cfg |= S5P_CIREAL_ISIZE_ADDR_CH_DIS;
writel(cfg, dev-regs + S5P_CIREAL_ISIZE);
 
-   writel(paddr-y, dev-regs + S5P_CIIYSA0);
-   writel(paddr-cb, dev-regs + S5P_CIICBSA0);
-   writel(paddr-cr, dev-regs + S5P_CIICRSA0);
+   writel(paddr-y, dev-regs + S5P_CIIYSA(0));
+   writel(paddr-cb, dev-regs + S5P_CIICBSA(0));
+   writel(paddr-cr, dev-regs + S5P_CIICRSA(0));
 
cfg = ~S5P_CIREAL_ISIZE_ADDR_CH_DIS;
writel(cfg, dev-regs + S5P_CIREAL_ISIZE);
diff --git a/drivers/media/video/s5p-fimc/regs-fimc.h 
b/drivers/media/video/s5p-fimc/regs-fimc.h
index a3cfe82..9e83315 100644
--- a/drivers/media/video/s5p-fimc/regs-fimc.h
+++ b/drivers/media/video/s5p-fimc/regs-fimc.h
@@ -11,10 +11,6 @@
 #ifndef REGS_FIMC_H_
 #define REGS_FIMC_H_
 
-#define S5P_CIOYSA(__x)(0x18 + (__x) * 4)
-#define S5P_CIOCBSA(__x)   (0x28 + (__x) * 4)
-#define S5P_CIOCRSA(__x)   (0x38 + (__x) * 4)
-
 /* Input source format */
 #define S5P_CISRCFMT   0x00
 #define S5P_CISRCFMT_ITU601_8BIT   (1  31)
@@ -28,22 +24,21 @@
 
 /* Window offset */
 #define S5P_CIWDOFST   0x04
-#define S5P_CIWDOFST_WINOFSEN  (1  31)
+#define S5P_CIWDOFST_OFF_EN(1  31)
 #define S5P_CIWDOFST_CLROVFIY  (1  30)
 #define S5P_CIWDOFST_CLROVRLB  (1  29)
-#define S5P_CIWDOFST_WINHOROFST_MASK   (0x7ff  16)
+#define S5P_CIWDOFST_HOROFF_MASK   (0x7ff  16)
 #define S5P_CIWDOFST_CLROVFICB (1  15)
 #define S5P_CIWDOFST_CLROVFICR (1  14)
-#define S5P_CIWDOFST_WINHOROFST(x) ((x)  16)
-#define S5P_CIWDOFST_WINVEROFST(x) ((x)  0)
-#define S5P_CIWDOFST_WINVEROFST_MASK   (0xfff  0)
+#define S5P_CIWDOFST_HOROFF(x) ((x)  16)
+#define S5P_CIWDOFST_VEROFF(x) ((x)  0)
+#define S5P_CIWDOFST_VEROFF_MASK   (0xfff  0)
 
 /* Global control */
 #define S5P_CIGCTRL0x08
 #define S5P_CIGCTRL_SWRST  (1  31)
 #define S5P_CIGCTRL_CAMRST_A   (1  30)
 #define S5P_CIGCTRL_SELCAM_ITU_A   (1  29)
-#define S5P_CIGCTRL_SELCAM_ITU_MASK(1  29)
 #define S5P_CIGCTRL_TESTPAT_NORMAL (0  27)
 #define S5P_CIGCTRL_TESTPAT_COLOR_BAR  (1  27)
 #define S5P_CIGCTRL_TESTPAT_HOR_INC(2  27)
@@ -61,6 +56,8 @@
 #define S5P_CIGCTRL_SHDW_DISABLE   (1  12)
 #define S5P_CIGCTRL_SELCAM_MIPI_A  (1  7)
 #define S5P_CIGCTRL_CAMIF_SELWB(1  6)
+/* 0 - ITU601; 1 - ITU709 */
+#define S5P_CIGCTRL_CSC_ITU601_709 (1  5)
 #define S5P_CIGCTRL_INVPOLHSYNC(1  4)
 #define S5P_CIGCTRL_SELCAM_MIPI(1  3)
 #define S5P_CIGCTRL_INTERLACE  (1  0)
@@ -72,23 +69,10 @@
 #define S5P_CIWDOFST2_HOROFF(x)((x)  16)
 #define S5P_CIWDOFST2_VEROFF(x)((x)  0)
 
-/* Output DMA Y plane start address */
-#define S5P_CIOYSA10x18
-#define S5P_CIOYSA20x1c
-#define S5P_CIOYSA30x20
-#define S5P_CIOYSA40x24
-
-/* Output DMA Cb plane start address */
-#define S5P_CIOCBSA1   0x28
-#define S5P_CIOCBSA2   0x2c
-#define S5P_CIOCBSA3   0x30
-#define S5P_CIOCBSA4   0x34
-
-/* Output DMA Cr plane start address */
-#define S5P_CIOCRSA1   0x38
-#define S5P_CIOCRSA2   0x3c
-#define S5P_CIOCRSA3   0x40
-#define S5P_CIOCRSA4   0x44
+/* Output DMA Y/Cb/Cr plane start addresses */
+#define S5P_CIOYSA(n)  (0x18 + (n) * 4)
+#define S5P_CIOCBSA(n) (0x28 + (n) * 4)
+#define S5P_CIOCRSA(n) (0x38 + (n) * 4)
 
 /* Target image format */
 #define S5P_CITRGFMT   0x48
@@ -168,6 +152,8 @@
 #define S5P_CISTATUS_OVFICB(1  30)
 #define S5P_CISTATUS_OVFICR(1  29)
 #define S5P_CISTATUS_VSYNC (1  28)
+#define S5P_CISTATUS_FRAMECNT_MASK (3  26)
+#define S5P_CISTATUS_FRAMECNT_SHIFT26
 #define S5P_CISTATUS_WINOFF_EN (1  25)
 #define S5P_CISTATUS_IMGCPT_EN (1  22)
 #define S5P_CISTATUS_IMGCPT_SCEN   (1  21)
@@ -206,10 +192,10 @@
 #define S5P_CIIMGEFF_PAT_CB(x) ((x)  13)
 #define S5P_CIIMGEFF_PAT_CR(x)