diff mbox

[v2,13/18] ARM: dts: s6e3fa0: add DT bindings

Message ID 1400647390-26590-14-git-send-email-yj44.cho@samsung.com
State Superseded, archived
Headers show

Commit Message

YoungJun Cho May 21, 2014, 4:43 a.m. UTC
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

Comments

Thierry Reding May 26, 2014, 1:41 p.m. UTC | #1
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.

Even the Exynos DSI Master bindings[0] say that these two are not at all
the same thing:

	port node:
	    - reg: (required) can be 0 for input RGB/I80 port or 1 for
	      DSI port;

Am I missing something here?

Thierry

[0]: https://www.kernel.org/doc/Documentation/devicetree/bindings/video/exynos_dsim.txt
Andrzej Hajda May 27, 2014, 6:28 a.m. UTC | #2
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.

Regards
Andrzej

>
> Even the Exynos DSI Master bindings[0] say that these two are not at all
> the same thing:
>
> 	port node:
> 	    - reg: (required) can be 0 for input RGB/I80 port or 1 for
> 	      DSI port;
>
> Am I missing something here?

>
> Thierry
>
> [0]: https://www.kernel.org/doc/Documentation/devicetree/bindings/video/exynos_dsim.txt

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Thierry Reding May 27, 2014, 7:53 a.m. UTC | #3
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.
They describe what DSIM expects, so that's where they should be defined.

Thierry
대인기/Tizen Platform Lab(SR)/삼성전자 May 27, 2014, 2:24 p.m. UTC | #4
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.

Thanks,
Inki Dae

> They describe what DSIM expects, so that's where they should be defined.
>
> Thierry
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Thierry Reding May 27, 2014, 8:21 p.m. UTC | #5
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.

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.

Thierry
대인기/Tizen Platform Lab(SR)/삼성전자 May 28, 2014, 4:50 a.m. UTC | #6
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.
> 
> 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.

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.

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.

Thanks,
Inki Dae

> 
> Thierry
> 

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Andrzej Hajda May 28, 2014, 6:44 a.m. UTC | #7
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?

Regards
Andrzej

>
> Thanks,
> Inki Dae
>
>> Thierry
>>
>

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
YoungJun Cho May 30, 2014, 3:08 a.m. UTC | #8
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
>

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/panel/samsung,s6e3fa0.txt b/Documentation/devicetree/bindings/panel/samsung,s6e3fa0.txt
new file mode 100644
index 0000000..c9a3fbd
--- /dev/null
+++ b/Documentation/devicetree/bindings/panel/samsung,s6e3fa0.txt
@@ -0,0 +1,45 @@ 
+Samsung S6E3FA0 AMOLED LCD 5.7 inch panel
+
+Required properties:
+  - compatible: "samsung,s6e3fa0"
+  - reg: the virtual channel number of a DSI peripheral
+  - vdd3-supply: core voltage supply
+  - vci-supply: voltage supply for analog circuits
+  - reset-gpios: a GPIO spec for the reset pin
+  - det-gpios: a GPIO spec for the OLED detection pin
+  - te-gpios: a GPIO spec for the TE pin
+  - cmdmode-display-timings: command mode interface timings for the connected
+      panel as described by [1]
+
+Optional properties:
+
+The device node can contain one 'port' child node with one child 'endpoint'
+node, according to the bindings defined in [2]. This node should describe
+panel's video bus.
+
+[1]: Documentation/devicetree/bindings/video/cmdmode-display-timing.txt
+[2]: Documentation/devicetree/bindings/media/video-interfaces.txt
+
+Example:
+
+	panel@0 {
+		compatible = "samsung,s6e3fa0";
+		reg = <0>;
+		vdd3-supply = <&vcclcd_reg>;
+		vci-supply = <&vlcd_reg>;
+		reset-gpios = <&gpy7 4 0>;
+		det-gpios = <&gpg0 6 0>;
+		te-gpios = <&gpd1 7 0>;
+
+		cmdmode-display-timings {
+			timing-0 {
+				clock-frequency = <0>;
+				hactive = <1080>;
+				vactive = <1920>;
+				cs-setup = <0>;
+				wr-setup = <0>;
+				wr-active = <1>;
+				wr-hold = <0>;
+			};
+		};
+	};