Re: [PATCH V2] drm/exynos: fimd: calculate the correct address offset

[Joonyoung Shim <jy0922.shim@samsung.com>]

On 03/06/2013 02:20 PM, Leela Krishna Amudala wrote:
> Calculate the correct address offset values for alpha and color key
> control registers and clear size control register for window 0
>
> Signed-off-by: Leela Krishna Amudala <l.krishna@samsung.com>
> ---
>   drivers/gpu/drm/exynos/exynos_drm_fimd.c | 16 ++++++++++------
>   1 file changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> index 9537761..78bab4a 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> @@ -38,21 +38,22 @@
>   /* position control register for hardware window 0, 2 ~ 4.*/
>   #define VIDOSD_A(win)		(VIDOSD_BASE + 0x00 + (win) * 16)
>   #define VIDOSD_B(win)		(VIDOSD_BASE + 0x04 + (win) * 16)
> +/* size control register is avaliable only for windows 0, 1 and 2. */
>   /* size control register for hardware window 0. */
>   #define VIDOSD_C_SIZE_W0	(VIDOSD_BASE + 0x08)
> -/* alpha control register for hardware window 1 ~ 4. */
> -#define VIDOSD_C(win)		(VIDOSD_BASE + 0x18 + (win) * 16)
> -/* size control register for hardware window 1 ~ 4. */
> +/* size control register for hardware window 1 ~ 2. */
>   #define VIDOSD_D(win)		(VIDOSD_BASE + 0x0C + (win) * 16)
> +/* alpha control register for hardware window 1 ~ 4. */
> +#define VIDOSD_C(win)		(VIDOSD_BASE + 0x08 + (win) * 16)
>   
>   #define VIDWx_BUF_START(win, buf)	(VIDW_BUF_START(buf) + (win) * 8)
>   #define VIDWx_BUF_END(win, buf)		(VIDW_BUF_END(buf) + (win) * 8)
>   #define VIDWx_BUF_SIZE(win, buf)	(VIDW_BUF_SIZE(buf) + (win) * 4)
>   
>   /* color key control register for hardware window 1 ~ 4. */
> -#define WKEYCON0_BASE(x)		((WKEYCON0 + 0x140) + (x * 8))
> +#define WKEYCON0_BASE(x)		((WKEYCON0 + 0x140) + ((x - 1) * 8))
>   /* color key value register for hardware window 1 ~ 4. */
> -#define WKEYCON1_BASE(x)		((WKEYCON1 + 0x140) + (x * 8))
> +#define WKEYCON1_BASE(x)		((WKEYCON1 + 0x140) + ((x - 1) * 8))
>   
>   /* FIMD has totally five hardware windows. */
>   #define WINDOWS_NR	5
> @@ -782,11 +783,14 @@ static void fimd_clear_win(struct fimd_context *ctx, int win)
>   	writel(0, ctx->regs + WINCON(win));
>   	writel(0, ctx->regs + VIDOSD_A(win));
>   	writel(0, ctx->regs + VIDOSD_B(win));
> -	writel(0, ctx->regs + VIDOSD_C(win));
> +	if (win != 0)
> +		writel(0, ctx->regs + VIDOSD_C(win));
>   
>   	if (win == 1 || win == 2)
>   		writel(0, ctx->regs + VIDOSD_D(win));
>   
> +	if (win == 0)
> +		writel(0, ctx->regs + VIDOSD_C_SIZE_W0);
>   	val = readl(ctx->regs + SHADOWCON);
>   	val &= ~SHADOWCON_WINx_PROTECT(win);
>   	writel(val, ctx->regs + SHADOWCON);

As i said at v1 patch, i prefer to remove VIDOSD_C_SIZE_W0 and it makes
codes are simple. Even if VIDOSD_C_SIZE_W0 is defined, it's not good
that VIDOSD_C(0) can mean VIDOSD_C_SIZE_W0.

diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
index 7514bee..a4356b4 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
@@ -40,11 +40,12 @@
  /* position control register for hardware window 0, 2 ~ 4.*/
  #define VIDOSD_A(win)		(VIDOSD_BASE + 0x00 + (win) * 16)
  #define VIDOSD_B(win)		(VIDOSD_BASE + 0x04 + (win) * 16)
-/* size control register for hardware window 0. */
-#define VIDOSD_C_SIZE_W0	(VIDOSD_BASE + 0x08)
-/* alpha control register for hardware window 1 ~ 4. */
-#define VIDOSD_C(win)		(VIDOSD_BASE + 0x18 + (win) * 16)
-/* size control register for hardware window 1 ~ 4. */
+/*
+ * size control register for hardware windows 0 and alpha control register
+ * for hardware windows 1 ~ 4
+ */
+#define VIDOSD_C(win)		(VIDOSD_BASE + 0x08 + (win) * 16)
+/* size control register for hardware window 1 ~ 2. */
  #define VIDOSD_D(win)		(VIDOSD_BASE + 0x0C + (win) * 16)
  
  #define VIDWx_BUF_START(win, buf)	(VIDW_BUF_START(buf) + (win) * 8)
@@ -52,9 +53,9 @@
  #define VIDWx_BUF_SIZE(win, buf)	(VIDW_BUF_SIZE(buf) + (win) * 4)
  
  /* color key control register for hardware window 1 ~ 4. */
-#define WKEYCON0_BASE(x)		((WKEYCON0 + 0x140) + (x * 8))
+#define WKEYCON0_BASE(x)		((WKEYCON0 + 0x140) + ((x - 1) * 8))
  /* color key value register for hardware window 1 ~ 4. */
-#define WKEYCON1_BASE(x)		((WKEYCON1 + 0x140) + (x * 8))
+#define WKEYCON1_BASE(x)		((WKEYCON1 + 0x140) + ((x - 1) * 8))
  
  /* FIMD has totally five hardware windows. */
  #define WINDOWS_NR	5
@@ -583,7 +584,7 @@ static void fimd_win_commit(struct device *dev, int zpos)
  	if (win != 3 && win != 4) {
  		u32 offset = VIDOSD_D(win);
  		if (win == 0)
-			offset = VIDOSD_C_SIZE_W0;
+			offset = VIDOSD_C(win);
  		val = win_data->ovl_width * win_data->ovl_height;
  		writel(val, ctx->regs + offset);