Re: [PATCH] drm/exynos: fimd: Get signal polarities from device tree

[Tomasz Figa <tomasz.figa@gmail.com>]

Hi Joonyoung,

On Thursday 06 of June 2013 13:30:49 Joonyoung Shim wrote:
> On 05/19/2013 08:32 PM, Tomasz Figa wrote:
> > Hi,
> > 
> > On Wednesday 01 of May 2013 22:00:25 Daniel Vetter wrote:
> >> On Wed, May 01, 2013 at 09:06:09PM +0200, Tomasz Figa wrote:
> >>> This patch modifies the driver to perform two stage parsing of video
> >>> timings from device tree, to get timing information as struct
> >>> videomode, which contains more data than struct fb_videomode.
> >>> 
> >>> Thanks to this change, information about polarity of control signals
> >>> (VSYNC, HSYNC, VDEN, VCLK) can be retrieved, in addition to standard
> >>> video timings.
> >>> 
> >>> Signed-off-by: Tomasz Figa <tomasz.figa@gmail.com>
> >> 
> >> Since the drm mode struct also contains flags for sync polarity ...
> >> why
> >> is there no direct of -> drm_mode function? Going through an fb
> >> videomode in a kms drm driver looks _really_ backwards to me.
> >> 
> >> Cc'in Dave for the fun of it ;-)
> > 
> > Struct fb_videomode is what exynos_drm_fimd driver uses internally.
> > Sure it should use drm_mode, but this is not really related to this
> > patch, because the code added in this patch only fills in the pdata
> > struct, which for compatibility reasons (the same structure is used
> > for both fbdev and drm drivers) contains struct fb_videomode.
> > 
> > OK, now after having a bit of fun, could we merge this patch to at
> > least have usable support of parallel displays using this driver?
> 
> I think it's better to use struct display_timings instead of struct
> fb_videomode in exynos_drm.

I agree that fb_videomode struct is a bit unfortunate here, but it seems 
to be widely used in the exynos_drm_fimd driver.

Actually, if I understood it properly, the correct struct to use in DRM 
drivers is drm_display_mode, but there is no conversion function from 
struct display_timing to it.

IMHO a separate patch introducing such conversion and then another one 
which modifies the driver to use drm_display_mode everywhere would be the 
best solution.

Best regards,
Tomasz