[PATCH 3/4] drm/exynos: mixer: set correct mode for range of resolutions

[Rahul Sharma]

On Wed, Jan 2, 2013 at 10:46 PM, Sean Paul <seanpaul at google.com> wrote:
> On Thu, Dec 27, 2012 at 6:38 AM, Rahul Sharma <rahul.sharma at samsung.com> wrote:
>> With this patch, mixer driver find the correct resolution mode for
>> the range of resolutions, upto 1080 vertical lines. Resolution will
>> be categorized to NTSC SD, PAL SD or HD and the correct mode is
>> set to the mixer configuration register.
>>
>> Signed-off-by: Rahul Sharma <rahul.sharma at samsung.com>
>> Signed-off-by: Sean Paul <seanpaul at chromium.org>
>> ---
>>  drivers/gpu/drm/exynos/exynos_mixer.c | 8 ++++----
>>  1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c
>> index 093b374..8ef0e71 100644
>> --- a/drivers/gpu/drm/exynos/exynos_mixer.c
>> +++ b/drivers/gpu/drm/exynos/exynos_mixer.c
>> @@ -283,13 +283,13 @@ static void mixer_cfg_scan(struct mixer_context *ctx, unsigned int height)
>>                                 MXR_CFG_SCAN_PROGRASSIVE);
>>
>>         /* choosing between porper HD and SD mode */
>> -       if (height == 480)
>> +       if (height <= 480)
>>                 val |= MXR_CFG_SCAN_NTSC | MXR_CFG_SCAN_SD;
>> -       else if (height == 576)
>> +       else if (height <= 576)
>>                 val |= MXR_CFG_SCAN_PAL | MXR_CFG_SCAN_SD;
>> -       else if (height == 720)
>> +       else if (height <= 720)
>>                 val |= MXR_CFG_SCAN_HD_720 | MXR_CFG_SCAN_HD;
>> -       else if (height == 1080)
>> +       else if (height <= 1080)
>>                 val |= MXR_CFG_SCAN_HD_1080 | MXR_CFG_SCAN_HD;
>>         else
>>                 val |= MXR_CFG_SCAN_HD_720 | MXR_CFG_SCAN_HD;
>
> It doesn't make sense to have these checks separate from the code you
> added in the last patch. IMO, you should consolidate those checks in
> one function so you only need to update them once in the future.
>
> Sean
>

Above function is independent of mixer versions / their limitations.
It is to categorize a give resolution to SD/HD/NTSC/PAL and set
bits to MXR_CFG register. It is same for exynos4 and exynos5
mixers and most likely, it will be same in future. While
check_timing has purely to do with the resolution negotiation.
Both serve different purpose.

Its is possible to put them in one place, but IMHO, this looks
cleaner and logical.

regards,
Rahul Sharma.

>> --
>> 1.8.0
>>