Hi, On Tue, Mar 5, 2013 at 5:24 PM, Joonyoung Shim <jy0922.shim@xxxxxxxxxxx> wrote: > Hi Leela, > > > On 03/05/2013 08:25 PM, Leela Krishna Amudala wrote: >> >> Calculate the correct address offset values for alpha and color key >> control registers >> >> Signed-off-by: Leela Krishna Amudala <l.krishna@xxxxxxxxxxx> >> --- >> drivers/gpu/drm/exynos/exynos_drm_fimd.c | 9 +++++---- >> 1 file changed, 5 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c >> b/drivers/gpu/drm/exynos/exynos_drm_fimd.c >> index 9537761..71f4176 100644 >> --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c >> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c >> @@ -41,7 +41,7 @@ >> /* size control register for hardware window 0. */ >> #define VIDOSD_C_SIZE_W0 (VIDOSD_BASE + 0x08) > > > How about just use VIDOSD_C(win) instead of this macro for size control > register of windows0? > I think it's better if writes comments properly here. > Then in that case VIDOSD_C(0) will refer to Window 0 Size Control register VIDOSD_C(1) will refer to Window 1 Alpha Control register VIDOSD_C(2) will refer to Window 2 Alpha Control register VIDOSD_C(3) will refer to Window 3 Alpha Control register VIDOSD_C(4) will refer to Window 4 Alpha Control register Which leads to a confusion. IMHO keeping VIDOSD_C_SIZE_W0 separate for win 0 is good. Best Wishes, Leela Krishna Amudala > >> /* alpha control register for hardware window 1 ~ 4. */ >> -#define VIDOSD_C(win) (VIDOSD_BASE + 0x18 + (win) * 16) >> +#define VIDOSD_C(win) (VIDOSD_BASE + 0x08 + (win) * 16) >> /* size control register for hardware window 1 ~ 4. */ >> #define VIDOSD_D(win) (VIDOSD_BASE + 0x0C + (win) * 16) >> @@ -50,9 +50,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 >> @@ -782,7 +782,8 @@ 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 use VIDOSD_C(win) macro for size control register of windows0 then this > fix will be unnecessary. > > Thanks. > > _______________________________________________ > dri-devel mailing list > dri-devel@xxxxxxxxxxxxxxxxxxxxx > http://lists.freedesktop.org/mailman/listinfo/dri-devel _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel
