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

[Joonyoung Shim <jy0922.shim@samsung.com>]

On 06/06/2013 06:47 PM, Tomasz Figa wrote:
> 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.

It's easy to convert from struct display_timing to videomode and there
is conversion function from struct videomode to drm_display_mode.

Already it uses at drivers/gpu/drm/tilcdc/tilcdc_panel.c

>
> 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.

OK, but the struct drm_display_mode doesn't include only timing
information so we need also to use struct videomode or struct drm_timing
properly.