Linux Samsung SOC development
 help / color / mirror / Atom feed
From: YoungJun Cho <yj44.cho@samsung.com>
To: Andrzej Hajda <a.hajda@samsung.com>,
	Inki Dae <inki.dae@samsung.com>,
	Thierry Reding <thierry.reding@gmail.com>
Cc: mark.rutland@arm.com, devicetree@vger.kernel.org,
	linux-samsung-soc@vger.kernel.org, pawel.moll@arm.com,
	ijc+devicetree@hellion.org.uk, sw0312.kim@samsung.com,
	dri-devel@lists.freedesktop.org, kyungmin.park@samsung.com,
	robh+dt@kernel.org, laurent.pinchart@ideasonboard.com,
	galak@codeaurora.org, kgene.kim@samsung.com,
	s.trumtrar@pengutronix.de
Subject: Re: [PATCH v2 13/18] ARM: dts: s6e3fa0: add DT bindings
Date: Fri, 30 May 2014 12:08:18 +0900	[thread overview]
Message-ID: <5387F622.2070309@samsung.com> (raw)
In-Reply-To: <538585D1.9030305@samsung.com>

Hi ALL,

On 05/28/2014 03:44 PM, Andrzej Hajda wrote:
> On 05/28/2014 06:50 AM, Inki Dae wrote:
>> On 2014년 05월 28일 05:21, Thierry Reding wrote:
>>> On Tue, May 27, 2014 at 11:24:49PM +0900, Inki Dae wrote:
>>>> 2014-05-27 16:53 GMT+09:00 Thierry Reding <thierry.reding@gmail.com>:
>>>>> On Tue, May 27, 2014 at 08:28:52AM +0200, Andrzej Hajda wrote:
>>>>>> Hi Thierry,
>>>>>>
>>>>>> On 05/26/2014 03:41 PM, Thierry Reding wrote:
>>>>>>> On Wed, May 21, 2014 at 01:43:05PM +0900, YoungJun Cho wrote:
>>>>>>>> This patch adds DT bindings for s6e3fa0 panel.
>>>>>>>> The bindings describes panel resources, display timings and cpu mode timings.
>>>>>>>>
>>>>>>>> Signed-off-by: YoungJun Cho <yj44.cho@samsung.com>
>>>>>>>> Acked-by: Inki Dae <inki.dae@samsung.com>
>>>>>>>> Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
>>>>>>>> ---
>>>>>>>>   .../devicetree/bindings/panel/samsung,s6e3fa0.txt  |   45 ++++++++++++++++++++
>>>>>>>>   1 file changed, 45 insertions(+)
>>>>>>>>   create mode 100644 Documentation/devicetree/bindings/panel/samsung,s6e3fa0.txt
>>>>>>> You're totally confusing me here. Half of this patch series is about
>>>>>>> adding i80 support to Exynos FIMD, and then you go and add what is
>>>>>>> apparently a DSI peripheral driver here that's supposed to be used by
>>>>>>> this new i80 support. Nothing I've been able to dig up indicates that
>>>>>>> i80 or DSI are in anyway related.
>>>>>> FIMD can produce parallel RGB output or command mode in i80 style output
>>>>>> via parallel lines.
>>>>>> DSIM can accept parallel RGB stream in this case it produces MIPI DSI
>>>>>> video mode signal or it can accept i80 and in this case it translates it
>>>>>> to MIPI DSI command mode.
>>>>> Then the command mode timings aren't a property of the panel at all.
>>>> Then the video mode timings aren't also a property of the panel.
>>>>
>>>> Which interface mipi and display controller should use would be
>>>> decided by lcd panel type: display controller can use i80 interface
>>>> based lcd panel, and also mipi controller can use i80 interface based
>>>> lcd panel.
>>>> In here, the only difference is that lcd panel receives  packets,
>>>> which includes video data or command data, packed with mipi protocol
>>>> via lane lines or receives video data or command data via parallel
>>>> lines.
>>>>
>>>> And the below is LCD types,
>>>>          RGB interface panel.
>>>>          i80 interface panel.
>>>>          MIPI based RGB interface panel.
>>>>          MIPI based i80 interface panel.
>>>>
>>>> RGB interface also is called video mode, and i80 interface also is
>>>> called cpu mode. In case of omap SoC, it is also called Smart panel.
>>>> i80 interface is just one of LCD types. So I think this interface
>>>> timings should be handled by frameworks related to mode in same way as
>>>> RGB interface.
>
> Some clarification about names.
> I am not an expert in command/cpu mode interface, so feel free to
> correct me.
> Those different terms were quite confusing for me so after some digging
> (for example here [1])
> I have found/ensured  there is a following relation between different names:
> MIPI DPI - RGB interface
> MIPI DBI type A - CPU mode m68 style
> MIPI DBI type B - CPU mode i80 style
> MIPI DBI type C - SPI, maybe also other serial interfaces (?)
> MIPI DSI - based on D-PHY-s serial protocol which can work in video or
> command mode.
>
> To add more confusion CPU mode is also named MPU mode or sys mode.
>
> To avoid confusion in the discussion I propose use i80 only to describe
> DBI type B interface,
> and DSI command mode, DSI video mode to describe DSI modes.
>
> [1]: http://www.allshore.com/pdf/DA8620.pdf
>
>
>>> LCD is a display technology, it has nothing to do with the interface. My
>>> point is that from Andrzej's description, and in fact from this patch
>>> series in general, the S6E3FA0 panel is a DSI panel. Associating timings
>>> that are i80 specific to it is therefore wrong.
>>>
>>> Consider for instance what would happen if somebody were to use the same
>>> panel on some other device (connected to a DSI controller). If you
>>> specify i80 timings for the panel then the new device won't know what to
>>> do with them because it expects DSI-related timings.
>>>
>>> Let me try to summarize the above to make sure we're all on the same
>>> page:
>>>
>>> 	- FIMD is a display controller that can be configured to either
>>> 	  send RGB data or i80 data
>>> 	- DSIM takes either RGB as input and outputs DSI (video mode) or
>>> 	  i80 as input and outputs DSI (command mode)
>>>
>>> In both cases the panel is connected to DSIM and it takes DSI as input,
>>> because it is a DSI panel (it doesn't understand RGB or i80). The panel
>>> needs to describe the properties of the DSI interface so that DSIM can
>>> be configured appropriately. DSIM in turn works as a bridge or encoder
>>> that converts RGB or i80 to DSI (video or command mode). So it makes no
>>> sense to describe the i80 timings for the panel because it has nothing
>>> to do with i80. Instead the DSIM is the hardware that needs to specify
>>> the i80 timings, so that FIMD can be configured to generate the timings
>>> that DSIM needs.
>>              CPU interface                     MIPI lane
>> FIMD ----------------------- DSIM --------------------- LCD Panel
>>
>> Hmm... reasonable. So your point is that command mode timing should be
>> placed in fimd device node, not panel device node? And panel device node
>> should provide only a property that DSIM driver can set LCD mode
>> properly to i80 or rgb interface mode, and also FIMD driver can set LCD
>> mode to i80 or rgb interface mode.
>
> I have no access to s6e3fa0 datasheet and Exynos datasheet I have access to
> is not very verbose on the subject but it seems to be reasonable that
> cs-setup, wr-setup, wr-active and wr-hold are properties only of i80
> interface,
> ie interface between FIMD and DSIM and they have nothing to do with DSI
> command mode panel.
> Those properties should be provided by DSIM to FIMD, I guess they can be
> even hardcoded
> in DSIM driver, no bindings required. There is still a question how DSIM
> should tell FIMD about them.
> I am not sure about mechanism of passing them from DSIM to FIMD, maybe
> adjusting drm_display_mode
> is a solution, maybe different way of communication should be used (I
> see here again interface_tracker use case [2]).
>
> [2]:
> http://permalink.gmane.org/gmane.linux.drivers.video-input-infrastructure/77451
>
> On the other side width, height and clock are properties of the panel so
> they should stay
> with the panel, maybe width and height could be moved from dts to
> driver, I am not sure about frequency.
>
>>
>> Is there my missing point?
>>
>> And in case of Exynos, now video timing property is also placed in panel
>> device node so it needs to move to fimd device node.
>
> No, video timings are properties of panels so they should stay in
> panels, display
> controllers should just ask panels about them.
>
>>
>> Andrzej, do you have other opinion? I have looked into dts files for
>> other SoC and In most SoC, it seems that display controller node has
>> video timing property, not panel node. Thierry's pointing seems
>> reasonable to me.
> I guess there could be many reasons: historical, backward compatibility,
> laziness of developers :).
> I agree with Thierry also.
>
> So if everybody agrees there is only one serious issue: how the i80
> properties
> should be passed from DSIM to FIMD, am I right?

Right, this issue was difficult for me.
So in my first RFC v1, I placed them in FIMD dts[1].
I didn't think of that(placed them in DSIM), moved them to panel.

[1] : http://www.spinics.net/lists/dri-devel/msg57960.html

And do you think that it is also required to rename cmdmode to i80mode?

Thank you.
Best regards YJ

>
> Regards
> Andrzej
>
>>
>> Thanks,
>> Inki Dae
>>
>>> Thierry
>>>
>>
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

  reply	other threads:[~2014-05-30  3:08 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-05-21  4:42 [PATCH v2 00/18] drm/exynos: support MIPI DSI command mode display YoungJun Cho
2014-05-21  4:42 ` [PATCH v2 01/18] drm/exynos: dsi: move the EoT packets configuration point YoungJun Cho
2014-05-21  4:42 ` [PATCH v2 02/18] drm/exynos: use wait_event_timeout() for safety usage YoungJun Cho
2014-05-21  6:01   ` Daniel Kurtz
2014-05-21  6:28     ` YoungJun Cho
2014-05-21  6:45       ` Daniel Kurtz
2014-05-21 10:37         ` YoungJun Cho
     [not found] ` <1400647390-26590-1-git-send-email-yj44.cho-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
2014-05-21  4:42   ` [PATCH v2 03/18] ARM: dts: video: add command mode display timing DT bindings YoungJun Cho
2014-05-21  4:43   ` [PATCH v2 08/18] drm/exynos: add TE handler to support MIPI DSI command mode interface YoungJun Cho
2014-05-21  4:43   ` [PATCH v2 15/18] ARM: dts: exynos4: add system register node YoungJun Cho
2014-05-21  4:43   ` [PATCH v2 16/18] ARM: dts: exynos5: add system register support YoungJun Cho
2014-05-21  4:42 ` [PATCH v2 04/18] video: add command mode and command mode display timing YoungJun Cho
2014-05-21 11:02   ` Thierry Reding
2014-05-21 11:41     ` YoungJun Cho
2014-05-22 13:07       ` Inki Dae
2014-05-22 13:53         ` Thierry Reding
2014-05-26 10:14   ` Thierry Reding
2014-05-26 11:10     ` Heiko Stübner
2014-05-21  4:42 ` [PATCH v2 05/18] drm_modes: add command mode helpers YoungJun Cho
2014-05-21  4:42 ` [PATCH v2 06/18] ARM: dts: sysreg: add exynos5 compatible to DT bindings YoungJun Cho
2014-05-21  4:42 ` [PATCH v2 07/18] ARM: dts: samsung-fimd: add I80 specific properties YoungJun Cho
2014-05-21  4:43 ` [PATCH v2 09/18] drm/exynos: dsi: add TE handler to support command mode interface YoungJun Cho
2014-05-21  4:43 ` [PATCH v2 10/18] drm/exynos: fimd: support I80 interface YoungJun Cho
2014-05-26  9:00   ` Daniel Kurtz
2014-05-29  5:45     ` YoungJun Cho
2014-05-21  4:43 ` [PATCH v2 11/18] ARM: dts: exynos_dsim: add exynos5420 compatible to DT bindings YoungJun Cho
2014-05-21  4:43 ` [PATCH v2 12/18] drm/exynos: dsi: add driver data to support Exynos5420 YoungJun Cho
2014-05-21  4:43 ` [PATCH v2 13/18] ARM: dts: s6e3fa0: add DT bindings YoungJun Cho
2014-05-26 13:41   ` Thierry Reding
2014-05-27  6:28     ` Andrzej Hajda
2014-05-27  7:53       ` Thierry Reding
2014-05-27 14:24         ` Inki Dae
2014-05-27 20:21           ` Thierry Reding
2014-05-28  4:50             ` Inki Dae
2014-05-28  6:44               ` Andrzej Hajda
2014-05-30  3:08                 ` YoungJun Cho [this message]
2014-05-21  4:43 ` [PATCH v2 14/18] drm/panel: add S6E3FA0 driver YoungJun Cho
2014-05-21  4:43 ` [PATCH v2 17/18] ARM: dts: exynos5420: add mipi-phy node YoungJun Cho
2014-05-21  4:43 ` [PATCH v2 18/18] ARM: dts: exynos5420: add dsi node YoungJun Cho

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5387F622.2070309@samsung.com \
    --to=yj44.cho@samsung.com \
    --cc=a.hajda@samsung.com \
    --cc=devicetree@vger.kernel.org \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=galak@codeaurora.org \
    --cc=ijc+devicetree@hellion.org.uk \
    --cc=inki.dae@samsung.com \
    --cc=kgene.kim@samsung.com \
    --cc=kyungmin.park@samsung.com \
    --cc=laurent.pinchart@ideasonboard.com \
    --cc=linux-samsung-soc@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=pawel.moll@arm.com \
    --cc=robh+dt@kernel.org \
    --cc=s.trumtrar@pengutronix.de \
    --cc=sw0312.kim@samsung.com \
    --cc=thierry.reding@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox