| Message ID | 1362482725-27143-1-git-send-email-l.krishna@samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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@samsung.com> > --- > 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. > /* 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.
Hi, On Tue, Mar 5, 2013 at 5:24 PM, Joonyoung Shim <jy0922.shim@samsung.com> 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@samsung.com> >> --- >> 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@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel
On 03/05/2013 09:04 PM, Leela Krishna Amudala wrote: > Hi, > > On Tue, Mar 5, 2013 at 5:24 PM, Joonyoung Shim <jy0922.shim@samsung.com> 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@samsung.com> >>> --- >>> 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. Because of confusion, we need correct comments. > 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)); Then you need also to initialize VIDOSD_C_SIZE_W0 register. >> >> 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@lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/dri-devel
Hi On Tue, Mar 5, 2013 at 5:47 PM, Joonyoung Shim <jy0922.shim@samsung.com> wrote: > On 03/05/2013 09:04 PM, Leela Krishna Amudala wrote: >> >> Hi, >> >> On Tue, Mar 5, 2013 at 5:24 PM, Joonyoung Shim <jy0922.shim@samsung.com> >> 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@samsung.com> >>>> --- >>>> 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. > > > Because of confusion, we need correct comments. > Added appropriate comments and posted V2 > >> 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)); > > > Then you need also to initialize VIDOSD_C_SIZE_W0 register. > Initialized and posted V2 Best Wishes, Leela Krishna Amudala. > >>> >>> 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@lists.freedesktop.org >>> http://lists.freedesktop.org/mailman/listinfo/dri-devel > > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel
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) /* 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 (win == 1 || win == 2) writel(0, ctx->regs + VIDOSD_D(win));
Calculate the correct address offset values for alpha and color key control registers Signed-off-by: Leela Krishna Amudala <l.krishna@samsung.com> --- drivers/gpu/drm/exynos/exynos_drm_fimd.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)