On Wed, Jan 2, 2013 at 10:46 PM, Sean Paul <seanpaul@xxxxxxxxxx> wrote: > On Thu, Dec 27, 2012 at 6:38 AM, Rahul Sharma <rahul.sharma@xxxxxxxxxxx> 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@xxxxxxxxxxx> >> Signed-off-by: Sean Paul <seanpaul@xxxxxxxxxxxx> >> --- >> 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 >> _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel
