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

[Leela Krishna Amudala]

Hi

On Tue, Mar 5, 2013 at 5:47 PM, Joonyoung Shim <jy0922.shim at 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 at 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 at 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 at lists.freedesktop.org
>>> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>
>
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel