Re: [PATCH 1/4] fbdev: add a MIPI DSI header
On Wed, 19 May 2010, Tomi Valkeinen wrote:
Hi,
On Wed, 2010-05-19 at 10:08 +0200, ext Guennadi Liakhovetski wrote:
Hi Tomi
On Wed, 19 May 2010, Tomi Valkeinen wrote:
The file name is mipi_dsi.h, the comment talks about MIPI, and the file
contains defines for MIPI DSI and MIPI DCS. If you want the file to
contain defines about all MIPI specs, I think the file name should be
just mipi.h.
No, the header is just intended for DSI, the comment could be made more
specific, yes, but DCS is a part of DSI, isn't it?
No, DCS is a spec of its own. DCS can be used on non-DSI displays also.
For example N800 and N900 use DCS commands, but they are not DSI
displays.
Yes, right, I should have said they are related;) How about calling the
header mipi_display.h? This would then unite DSI, DCS, DBI and DPI?
Creating a separate header for each of these standards seems like an
overkill to me. We could then put MIPI CSI and CPI standards in an
include/media/mipi_camera.h. Not sure where to put various other MIPI
standards, I guess, we'll have to think about it as a need arises.
+ *
+ * Copyright (C) 2010 Guennadi Liakhovetski g.liakhovet...@gmx.de
+ * Copyright (C) 2006 Nokia Corporation
+ * Author: Imre Deak imre.d...@nokia.com
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+#ifndef MIPI_DSI_H
+#define MIPI_DSI_H
+
+#include linux/types.h
+
+enum mipi_dsi_cmd {
I think these are DSI datatypes, not commands.
hm, write, shut down, mode off, mode on sound like commands to me,
and I think I saw them called commands in the spec, but yes, some of them
are just packet or data types... I'll double-check, thanks.
Well, true, this is not a clear thing. MIPI DSI spec talks about turn
on peripheral command. But the numbers are DSI data types, according to
the spec, and not all of them are commands.
Ok, how about telegram types then?
+ MIPI_DSI_DCS_SHORT_WRITE= 5,
Please use the same number format for all enums. So this should be 0x05.
Where does this requirement come from? Is it in CodingStyle?;)
I don't know, but mixing different formats looks ugly to me =).
This seems to be subjective;) But I'm not religious about it, and since
Nokia (C) is the first in this file I might as well just accept your
proposal;)
+ MIPI_DSI_DCS_SHORT_WRITE_PARAM = 0x15,
+ MIPI_DSI_COLOR_MODE_OFF = 2,
+ MIPI_DSI_COLOR_MODE_ON = 0x12,
+ MIPI_DSI_SHUTDOWN_PERIPHERAL= 0x22,
+ MIPI_DSI_TURN_ON_PERIPHERAL = 0x32,
+ MIPI_DSI_GENERIC_SHORT_WRITE_0_PARAM= 3,
+ MIPI_DSI_GENERIC_SHORT_WRITE_1_PARAM= 0x13,
+ MIPI_DSI_GENERIC_SHORT_WRITE_2_PARAM= 0x23,
+ MIPI_DSI_GENERIC_READ_REQUEST_0_PARAM = 4,
+ MIPI_DSI_GENERIC_READ_REQUEST_1_PARAM = 0x14,
+ MIPI_DSI_GENERIC_READ_REQUEST_2_PARAM = 0x24,
+ MIPI_DSI_DCS_LONG_WRITE = 0x39,
+ MIPI_DSI_SET_MAXIMUM_RETURN_PACKET_SIZE = 0x37,
+ MIPI_DSI_NULL_PACKET= 9,
+ MIPI_DSI_BLANKING_PACKET= 0x19,
+ MIPI_DSI_GENERIC_LONG_WRITE = 0x29,
+ MIPI_DSI_LOOSELY_PACKED_PIXEL_STREAM_YCBCR20= 0xc,
+ MIPI_DSI_PACKED_PIXEL_STREAM_YCBCR24= 0x1c,
+ MIPI_DSI_PACKED_PIXEL_STREAM_YCBCR16= 0x2c,
+ MIPI_DSI_PACKED_PIXEL_STREAM_30 = 0xd,
+ MIPI_DSI_PACKED_PIXEL_STREAM_36 = 0x1d,
+ MIPI_DSI_PACKED_PIXEL_STREAM_YCBCR12= 0x3d,
+ MIPI_DSI_PACKED_PIXEL_STREAM_16 = 0xe,
+ MIPI_DSI_PACKED_PIXEL_STREAM_18 = 0x1e,
+ MIPI_DSI_PIXEL_STREAM_3BYTE_18 = 0x2e,
+ MIPI_DSI_PACKED_PIXEL_STREAM_24 = 0x3e,
+};
+
+enum mipi_dcs_cmd {
While the MIPI specs define a certain set of commands, the real panels
usually implement also their own custom commands. That's why I don't
think enum is a good choice here.
Yes, that's a valid point, I'll have to think about it more.
I think a simple solution would be to just use defines, and have
functions that take the command as u8. That's what the OMAP DSI driver
does. If you have better ideas, please share =).
The same applies for the DSI commands/datatypes, although I haven't seen
a panel that has custom DSI datatypes.
As others voted for unnamed enums, how about using them?
Concerning omap2 display drivers,
Re: [PATCH 1/4] fbdev: add a MIPI DSI header
On Thu, 2010-05-20 at 10:07 +0200, ext Guennadi Liakhovetski wrote: Yes, right, I should have said they are related;) How about calling the header mipi_display.h? This would then unite DSI, DCS, DBI and DPI? Creating a separate header for each of these standards seems like an overkill to me. We could then put MIPI CSI and CPI standards in an include/media/mipi_camera.h. Not sure where to put various other MIPI standards, I guess, we'll have to think about it as a need arises. mipi_display.h sounds good to me. Well, true, this is not a clear thing. MIPI DSI spec talks about turn on peripheral command. But the numbers are DSI data types, according to the spec, and not all of them are commands. Ok, how about telegram types then? I wouldn't invent a new word for this =). The DSI spec talks about commands, data types and transactions, I think we should pick one of them. Perhaps this is already approaching nitpicking, but: As only some of the commands/datatypes/transactions are commands, I think that's not proper word. All of them have a data type number, and I guess they all are transactions. So Turn On Peripheral Command is a transaction, and its data type is 0x32. I guess if the enum is named, it should then be mipi_dsi_transaction. But then, which one of these would be more correct: dsi_send(enum mipi_dsi_transaction transaction) dsi_send(u8 datatype) As I said previously, I haven't seen any panel using custom datatypes, but I wouldn't be surprised if some panel does. In that sense I would go for using u8, and then perhaps leaving the enum unnamed. What do you think? As others voted for unnamed enums, how about using them? Sounds good. Concerning omap2 display drivers, AFAICS the only thing we want to change there is to switch them too to using the common header and telegram type Yep, I think that's the only change for now. and command names? So far I don't see a need for a generic MIPI (display) subsystem in the kernel with an own bus type, API etc. We could of course create a simpe bus with callbacks for sending short and long packets and reading data back, but do we really need it ATM? This is something I've been thinking about for some time. I even made some prototypes for it, but I didn't have time to go forward with it. It would of course be nice to use the same panel driver on different boards, so I think this is definitely something to think about in the future. Tomi -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/4] fbdev: add a MIPI DSI header
On Thu, May 20, 2010 at 11:32:49AM +0300, Tomi Valkeinen wrote: On Thu, 2010-05-20 at 10:07 +0200, ext Guennadi Liakhovetski wrote: Yes, right, I should have said they are related;) How about calling the header mipi_display.h? This would then unite DSI, DCS, DBI and DPI? Creating a separate header for each of these standards seems like an overkill to me. We could then put MIPI CSI and CPI standards in an include/media/mipi_camera.h. Not sure where to put various other MIPI standards, I guess, we'll have to think about it as a need arises. mipi_display.h sounds good to me. if there are extra mipi standards which should be taken care of, why not include/linux/mipi/[display.h camera.h other1.h other2.h...] ? -- balbi -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/4] fbdev: add a MIPI DSI header
On Thu, 20 May 2010, Felipe Balbi wrote: On Thu, May 20, 2010 at 11:32:49AM +0300, Tomi Valkeinen wrote: On Thu, 2010-05-20 at 10:07 +0200, ext Guennadi Liakhovetski wrote: Yes, right, I should have said they are related;) How about calling the header mipi_display.h? This would then unite DSI, DCS, DBI and DPI? Creating a separate header for each of these standards seems like an overkill to me. We could then put MIPI CSI and CPI standards in an include/media/mipi_camera.h. Not sure where to put various other MIPI standards, I guess, we'll have to think about it as a need arises. mipi_display.h sounds good to me. if there are extra mipi standards which should be taken care of, why not include/linux/mipi/[display.h camera.h other1.h other2.h...] ? I thought about this too, but I prefer to separate them thematically: put display standards under video/, camera under media/ etc. This way including these headers from respective drivers and platform code would look more naturally, IMHO. Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/4] fbdev: add a MIPI DSI header
On Thu, 20 May 2010, Tomi Valkeinen wrote: On Thu, 2010-05-20 at 10:07 +0200, ext Guennadi Liakhovetski wrote: Yes, right, I should have said they are related;) How about calling the header mipi_display.h? This would then unite DSI, DCS, DBI and DPI? Creating a separate header for each of these standards seems like an overkill to me. We could then put MIPI CSI and CPI standards in an include/media/mipi_camera.h. Not sure where to put various other MIPI standards, I guess, we'll have to think about it as a need arises. mipi_display.h sounds good to me. Well, true, this is not a clear thing. MIPI DSI spec talks about turn on peripheral command. But the numbers are DSI data types, according to the spec, and not all of them are commands. Ok, how about telegram types then? I wouldn't invent a new word for this =). The DSI spec talks about commands, data types and transactions, I think we should pick one of them. Perhaps this is already approaching nitpicking, but: As only some of the commands/datatypes/transactions are commands, I think that's not proper word. All of them have a data type number, and I guess they all are transactions. So Turn On Peripheral Command is a transaction, and its data type is 0x32. Yes, transaction looks good to me. I guess if the enum is named, it should then be mipi_dsi_transaction. But then, which one of these would be more correct: dsi_send(enum mipi_dsi_transaction transaction) dsi_send(u8 datatype) As I said previously, I haven't seen any panel using custom datatypes, but I wouldn't be surprised if some panel does. In that sense I would go for using u8, and then perhaps leaving the enum unnamed. What do you think? Yes, I'd use u8 too, because the specs define transaction types to take 6 bits and DCS commands to be 8-bit ints. But, you know, doesn't our case fall under case (b) of Chapter 5 Typedefs of CodingStyle?:) I mean, wouldn't it make sense to create two typedefs for these to add some type-safety? In fact, transaction types cannot be user-defined / proprietary, right? So, for that an enum would work. It's only DCS commands that allow extensions. But making transaction type an enum and command a typedef would be ugly, and making a typedef for an enum is frowned upon too... As others voted for unnamed enums, how about using them? Sounds good. Concerning omap2 display drivers, AFAICS the only thing we want to change there is to switch them too to using the common header and telegram type Yep, I think that's the only change for now. and command names? So far I don't see a need for a generic MIPI (display) subsystem in the kernel with an own bus type, API etc. We could of course create a simpe bus with callbacks for sending short and long packets and reading data back, but do we really need it ATM? This is something I've been thinking about for some time. I even made some prototypes for it, but I didn't have time to go forward with it. It would of course be nice to use the same panel driver on different boards, so I think this is definitely something to think about in the future. But you only need specific panel drivers if you want to support their proprietary commands? Otherwise you only need a set of parameters - timeing requirements etc. - which you can perfectly just pass in platform data? Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/4] fbdev: add a MIPI DSI header
On Thu, 2010-05-20 at 13:03 +0200, ext Guennadi Liakhovetski wrote: Yes, I'd use u8 too, because the specs define transaction types to take 6 bits and DCS commands to be 8-bit ints. But, you know, doesn't our case fall under case (b) of Chapter 5 Typedefs of CodingStyle?:) I mean, CodingStyle says: if there is a clear reason for why it under certain circumstances might be an unsigned int and under other configurations might be unsigned long, then by all means go ahead and use a typedef. In our case the DSI transactions and DCS commands are always 6 and 8 bits, so I don't think it applies. And the transaction datatypes and DCS commands are really just payload sent to the peripheral, and the peripheral can do anything it wants with those. wouldn't it make sense to create two typedefs for these to add some But wouldn't a typedef basically lock the possible transactions or DCS commands that you can give to the functions? And that was the original problem also. type-safety? In fact, transaction types cannot be user-defined / proprietary, right? So, for that an enum would work. It's only DCS Why they cannot be? I don't see any technical problem there, at least. And while I don't have any current use to extend transactions, I wouldn't want to limit it for any possible future use either. But you only need specific panel drivers if you want to support their proprietary commands? Otherwise you only need a set of parameters - timeing requirements etc. - which you can perfectly just pass in platform data? That's true. I guess you've gotten it easy, if you only have DSI video mode panels that do not need any special care =). Command mode panels are usually slightly more complex, and especially early prototypes of the panels can require quite a lot of trickery. And then there are DSI hubs, which make DSI command mode panels look trivial... Tomi -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/4] fbdev: add a MIPI DSI header
Hi,
On Fri, 2010-05-07 at 11:07 +0200, ext Guennadi Liakhovetski wrote:
This header adds defines for MIPI DSI and DCS commands and data formats. See
http://www.mipi.org/ for details.
Did you notice that I have implemented MIPI DSI command mode support for
OMAP processors? It's located in drivers/video/omap2/dss/dsi.c and one
DSI panel driver is located in drivers/video/omap2/displays/panel-taal.c
Signed-off-by: Guennadi Liakhovetski g.liakhovet...@gmx.de
---
include/video/mipi_dsi.h | 99
++
1 files changed, 99 insertions(+), 0 deletions(-)
create mode 100644 include/video/mipi_dsi.h
diff --git a/include/video/mipi_dsi.h b/include/video/mipi_dsi.h
new file mode 100644
index 000..5d3a6d6
--- /dev/null
+++ b/include/video/mipi_dsi.h
@@ -0,0 +1,99 @@
+/*
+ * Mobile Industry Processor Interface (MIPI(R)) defines
The file name is mipi_dsi.h, the comment talks about MIPI, and the file
contains defines for MIPI DSI and MIPI DCS. If you want the file to
contain defines about all MIPI specs, I think the file name should be
just mipi.h.
+ *
+ * Copyright (C) 2010 Guennadi Liakhovetski g.liakhovet...@gmx.de
+ * Copyright (C) 2006 Nokia Corporation
+ * Author: Imre Deak imre.d...@nokia.com
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+#ifndef MIPI_DSI_H
+#define MIPI_DSI_H
+
+#include linux/types.h
+
+enum mipi_dsi_cmd {
I think these are DSI datatypes, not commands.
+ MIPI_DSI_DCS_SHORT_WRITE= 5,
Please use the same number format for all enums. So this should be 0x05.
+ MIPI_DSI_DCS_SHORT_WRITE_PARAM = 0x15,
+ MIPI_DSI_COLOR_MODE_OFF = 2,
+ MIPI_DSI_COLOR_MODE_ON = 0x12,
+ MIPI_DSI_SHUTDOWN_PERIPHERAL= 0x22,
+ MIPI_DSI_TURN_ON_PERIPHERAL = 0x32,
+ MIPI_DSI_GENERIC_SHORT_WRITE_0_PARAM= 3,
+ MIPI_DSI_GENERIC_SHORT_WRITE_1_PARAM= 0x13,
+ MIPI_DSI_GENERIC_SHORT_WRITE_2_PARAM= 0x23,
+ MIPI_DSI_GENERIC_READ_REQUEST_0_PARAM = 4,
+ MIPI_DSI_GENERIC_READ_REQUEST_1_PARAM = 0x14,
+ MIPI_DSI_GENERIC_READ_REQUEST_2_PARAM = 0x24,
+ MIPI_DSI_DCS_LONG_WRITE = 0x39,
+ MIPI_DSI_SET_MAXIMUM_RETURN_PACKET_SIZE = 0x37,
+ MIPI_DSI_NULL_PACKET= 9,
+ MIPI_DSI_BLANKING_PACKET= 0x19,
+ MIPI_DSI_GENERIC_LONG_WRITE = 0x29,
+ MIPI_DSI_LOOSELY_PACKED_PIXEL_STREAM_YCBCR20= 0xc,
+ MIPI_DSI_PACKED_PIXEL_STREAM_YCBCR24= 0x1c,
+ MIPI_DSI_PACKED_PIXEL_STREAM_YCBCR16= 0x2c,
+ MIPI_DSI_PACKED_PIXEL_STREAM_30 = 0xd,
+ MIPI_DSI_PACKED_PIXEL_STREAM_36 = 0x1d,
+ MIPI_DSI_PACKED_PIXEL_STREAM_YCBCR12= 0x3d,
+ MIPI_DSI_PACKED_PIXEL_STREAM_16 = 0xe,
+ MIPI_DSI_PACKED_PIXEL_STREAM_18 = 0x1e,
+ MIPI_DSI_PIXEL_STREAM_3BYTE_18 = 0x2e,
+ MIPI_DSI_PACKED_PIXEL_STREAM_24 = 0x3e,
+};
+
+enum mipi_dcs_cmd {
While the MIPI specs define a certain set of commands, the real panels
usually implement also their own custom commands. That's why I don't
think enum is a good choice here.
Tomi
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/4] fbdev: add a MIPI DSI header
Hi Tomi
On Wed, 19 May 2010, Tomi Valkeinen wrote:
Hi,
On Fri, 2010-05-07 at 11:07 +0200, ext Guennadi Liakhovetski wrote:
This header adds defines for MIPI DSI and DCS commands and data formats. See
http://www.mipi.org/ for details.
Did you notice that I have implemented MIPI DSI command mode support for
OMAP processors? It's located in drivers/video/omap2/dss/dsi.c and one
DSI panel driver is located in drivers/video/omap2/displays/panel-taal.c
No, didn't see those, I grepped for MIPI...
Signed-off-by: Guennadi Liakhovetski g.liakhovet...@gmx.de
---
include/video/mipi_dsi.h | 99
++
1 files changed, 99 insertions(+), 0 deletions(-)
create mode 100644 include/video/mipi_dsi.h
diff --git a/include/video/mipi_dsi.h b/include/video/mipi_dsi.h
new file mode 100644
index 000..5d3a6d6
--- /dev/null
+++ b/include/video/mipi_dsi.h
@@ -0,0 +1,99 @@
+/*
+ * Mobile Industry Processor Interface (MIPI(R)) defines
The file name is mipi_dsi.h, the comment talks about MIPI, and the file
contains defines for MIPI DSI and MIPI DCS. If you want the file to
contain defines about all MIPI specs, I think the file name should be
just mipi.h.
No, the header is just intended for DSI, the comment could be made more
specific, yes, but DCS is a part of DSI, isn't it?
+ *
+ * Copyright (C) 2010 Guennadi Liakhovetski g.liakhovet...@gmx.de
+ * Copyright (C) 2006 Nokia Corporation
+ * Author: Imre Deak imre.d...@nokia.com
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+#ifndef MIPI_DSI_H
+#define MIPI_DSI_H
+
+#include linux/types.h
+
+enum mipi_dsi_cmd {
I think these are DSI datatypes, not commands.
hm, write, shut down, mode off, mode on sound like commands to me,
and I think I saw them called commands in the spec, but yes, some of them
are just packet or data types... I'll double-check, thanks.
+ MIPI_DSI_DCS_SHORT_WRITE= 5,
Please use the same number format for all enums. So this should be 0x05.
Where does this requirement come from? Is it in CodingStyle?;)
+ MIPI_DSI_DCS_SHORT_WRITE_PARAM = 0x15,
+ MIPI_DSI_COLOR_MODE_OFF = 2,
+ MIPI_DSI_COLOR_MODE_ON = 0x12,
+ MIPI_DSI_SHUTDOWN_PERIPHERAL= 0x22,
+ MIPI_DSI_TURN_ON_PERIPHERAL = 0x32,
+ MIPI_DSI_GENERIC_SHORT_WRITE_0_PARAM= 3,
+ MIPI_DSI_GENERIC_SHORT_WRITE_1_PARAM= 0x13,
+ MIPI_DSI_GENERIC_SHORT_WRITE_2_PARAM= 0x23,
+ MIPI_DSI_GENERIC_READ_REQUEST_0_PARAM = 4,
+ MIPI_DSI_GENERIC_READ_REQUEST_1_PARAM = 0x14,
+ MIPI_DSI_GENERIC_READ_REQUEST_2_PARAM = 0x24,
+ MIPI_DSI_DCS_LONG_WRITE = 0x39,
+ MIPI_DSI_SET_MAXIMUM_RETURN_PACKET_SIZE = 0x37,
+ MIPI_DSI_NULL_PACKET= 9,
+ MIPI_DSI_BLANKING_PACKET= 0x19,
+ MIPI_DSI_GENERIC_LONG_WRITE = 0x29,
+ MIPI_DSI_LOOSELY_PACKED_PIXEL_STREAM_YCBCR20= 0xc,
+ MIPI_DSI_PACKED_PIXEL_STREAM_YCBCR24= 0x1c,
+ MIPI_DSI_PACKED_PIXEL_STREAM_YCBCR16= 0x2c,
+ MIPI_DSI_PACKED_PIXEL_STREAM_30 = 0xd,
+ MIPI_DSI_PACKED_PIXEL_STREAM_36 = 0x1d,
+ MIPI_DSI_PACKED_PIXEL_STREAM_YCBCR12= 0x3d,
+ MIPI_DSI_PACKED_PIXEL_STREAM_16 = 0xe,
+ MIPI_DSI_PACKED_PIXEL_STREAM_18 = 0x1e,
+ MIPI_DSI_PIXEL_STREAM_3BYTE_18 = 0x2e,
+ MIPI_DSI_PACKED_PIXEL_STREAM_24 = 0x3e,
+};
+
+enum mipi_dcs_cmd {
While the MIPI specs define a certain set of commands, the real panels
usually implement also their own custom commands. That's why I don't
think enum is a good choice here.
Yes, that's a valid point, I'll have to think about it more.
Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/4] fbdev: add a MIPI DSI header
Hi,
On Wed, 2010-05-19 at 10:08 +0200, ext Guennadi Liakhovetski wrote:
Hi Tomi
On Wed, 19 May 2010, Tomi Valkeinen wrote:
The file name is mipi_dsi.h, the comment talks about MIPI, and the file
contains defines for MIPI DSI and MIPI DCS. If you want the file to
contain defines about all MIPI specs, I think the file name should be
just mipi.h.
No, the header is just intended for DSI, the comment could be made more
specific, yes, but DCS is a part of DSI, isn't it?
No, DCS is a spec of its own. DCS can be used on non-DSI displays also.
For example N800 and N900 use DCS commands, but they are not DSI
displays.
+ *
+ * Copyright (C) 2010 Guennadi Liakhovetski g.liakhovet...@gmx.de
+ * Copyright (C) 2006 Nokia Corporation
+ * Author: Imre Deak imre.d...@nokia.com
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+#ifndef MIPI_DSI_H
+#define MIPI_DSI_H
+
+#include linux/types.h
+
+enum mipi_dsi_cmd {
I think these are DSI datatypes, not commands.
hm, write, shut down, mode off, mode on sound like commands to me,
and I think I saw them called commands in the spec, but yes, some of them
are just packet or data types... I'll double-check, thanks.
Well, true, this is not a clear thing. MIPI DSI spec talks about turn
on peripheral command. But the numbers are DSI data types, according to
the spec, and not all of them are commands.
+ MIPI_DSI_DCS_SHORT_WRITE= 5,
Please use the same number format for all enums. So this should be 0x05.
Where does this requirement come from? Is it in CodingStyle?;)
I don't know, but mixing different formats looks ugly to me =).
+ MIPI_DSI_DCS_SHORT_WRITE_PARAM = 0x15,
+ MIPI_DSI_COLOR_MODE_OFF = 2,
+ MIPI_DSI_COLOR_MODE_ON = 0x12,
+ MIPI_DSI_SHUTDOWN_PERIPHERAL= 0x22,
+ MIPI_DSI_TURN_ON_PERIPHERAL = 0x32,
+ MIPI_DSI_GENERIC_SHORT_WRITE_0_PARAM= 3,
+ MIPI_DSI_GENERIC_SHORT_WRITE_1_PARAM= 0x13,
+ MIPI_DSI_GENERIC_SHORT_WRITE_2_PARAM= 0x23,
+ MIPI_DSI_GENERIC_READ_REQUEST_0_PARAM = 4,
+ MIPI_DSI_GENERIC_READ_REQUEST_1_PARAM = 0x14,
+ MIPI_DSI_GENERIC_READ_REQUEST_2_PARAM = 0x24,
+ MIPI_DSI_DCS_LONG_WRITE = 0x39,
+ MIPI_DSI_SET_MAXIMUM_RETURN_PACKET_SIZE = 0x37,
+ MIPI_DSI_NULL_PACKET= 9,
+ MIPI_DSI_BLANKING_PACKET= 0x19,
+ MIPI_DSI_GENERIC_LONG_WRITE = 0x29,
+ MIPI_DSI_LOOSELY_PACKED_PIXEL_STREAM_YCBCR20= 0xc,
+ MIPI_DSI_PACKED_PIXEL_STREAM_YCBCR24= 0x1c,
+ MIPI_DSI_PACKED_PIXEL_STREAM_YCBCR16= 0x2c,
+ MIPI_DSI_PACKED_PIXEL_STREAM_30 = 0xd,
+ MIPI_DSI_PACKED_PIXEL_STREAM_36 = 0x1d,
+ MIPI_DSI_PACKED_PIXEL_STREAM_YCBCR12= 0x3d,
+ MIPI_DSI_PACKED_PIXEL_STREAM_16 = 0xe,
+ MIPI_DSI_PACKED_PIXEL_STREAM_18 = 0x1e,
+ MIPI_DSI_PIXEL_STREAM_3BYTE_18 = 0x2e,
+ MIPI_DSI_PACKED_PIXEL_STREAM_24 = 0x3e,
+};
+
+enum mipi_dcs_cmd {
While the MIPI specs define a certain set of commands, the real panels
usually implement also their own custom commands. That's why I don't
think enum is a good choice here.
Yes, that's a valid point, I'll have to think about it more.
I think a simple solution would be to just use defines, and have
functions that take the command as u8. That's what the OMAP DSI driver
does. If you have better ideas, please share =).
The same applies for the DSI commands/datatypes, although I haven't seen
a panel that has custom DSI datatypes.
Tomi
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/4] fbdev: add a MIPI DSI header
On Wed, May 19, 2010 at 10:21:48AM +0200, Valkeinen Tomi (Nokia-D/Helsinki)
wrote:
Hi,
On Wed, 2010-05-19 at 10:08 +0200, ext Guennadi Liakhovetski wrote:
Hi Tomi
On Wed, 19 May 2010, Tomi Valkeinen wrote:
+ MIPI_DSI_DCS_SHORT_WRITE= 5,
Please use the same number format for all enums. So this should be 0x05.
Where does this requirement come from? Is it in CodingStyle?;)
I don't know, but mixing different formats looks ugly to me =).
One case where it would perhaps make sense if you define some fields
with shifts and masks. Specifying shifts as decimal and masks as hex
is clearer IMO. But in this case I agree it just looks ugly and only
serves to confuse the reader as he tries to figure out if there's
some magic meaning to the different formats.
+ MIPI_DSI_DCS_SHORT_WRITE_PARAM = 0x15,
+ MIPI_DSI_COLOR_MODE_OFF = 2,
+ MIPI_DSI_COLOR_MODE_ON = 0x12,
+ MIPI_DSI_SHUTDOWN_PERIPHERAL= 0x22,
+ MIPI_DSI_TURN_ON_PERIPHERAL = 0x32,
+ MIPI_DSI_GENERIC_SHORT_WRITE_0_PARAM= 3,
+ MIPI_DSI_GENERIC_SHORT_WRITE_1_PARAM= 0x13,
+ MIPI_DSI_GENERIC_SHORT_WRITE_2_PARAM= 0x23,
+ MIPI_DSI_GENERIC_READ_REQUEST_0_PARAM = 4,
+ MIPI_DSI_GENERIC_READ_REQUEST_1_PARAM = 0x14,
+ MIPI_DSI_GENERIC_READ_REQUEST_2_PARAM = 0x24,
+ MIPI_DSI_DCS_LONG_WRITE = 0x39,
+ MIPI_DSI_SET_MAXIMUM_RETURN_PACKET_SIZE = 0x37,
+ MIPI_DSI_NULL_PACKET= 9,
+ MIPI_DSI_BLANKING_PACKET= 0x19,
+ MIPI_DSI_GENERIC_LONG_WRITE = 0x29,
+ MIPI_DSI_LOOSELY_PACKED_PIXEL_STREAM_YCBCR20= 0xc,
+ MIPI_DSI_PACKED_PIXEL_STREAM_YCBCR24= 0x1c,
+ MIPI_DSI_PACKED_PIXEL_STREAM_YCBCR16= 0x2c,
+ MIPI_DSI_PACKED_PIXEL_STREAM_30 = 0xd,
+ MIPI_DSI_PACKED_PIXEL_STREAM_36 = 0x1d,
+ MIPI_DSI_PACKED_PIXEL_STREAM_YCBCR12= 0x3d,
+ MIPI_DSI_PACKED_PIXEL_STREAM_16 = 0xe,
+ MIPI_DSI_PACKED_PIXEL_STREAM_18 = 0x1e,
+ MIPI_DSI_PIXEL_STREAM_3BYTE_18 = 0x2e,
+ MIPI_DSI_PACKED_PIXEL_STREAM_24 = 0x3e,
+};
+
+enum mipi_dcs_cmd {
While the MIPI specs define a certain set of commands, the real panels
usually implement also their own custom commands. That's why I don't
think enum is a good choice here.
Yes, that's a valid point, I'll have to think about it more.
I think a simple solution would be to just use defines, and have
functions that take the command as u8. That's what the OMAP DSI driver
does. If you have better ideas, please share =).
I find enums easier on the eye than defines. Less irrelevant junk on
each line. There's no reason you can't pass enum values as u8. But in
that case giving the enum a name doesn't really make sense.
--
Ville Syrjälä
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/4] fbdev: add a MIPI DSI header
On Wed, May 19, 2010 at 05:27:32PM +0300, Ville Syrj?l? wrote: On Wed, May 19, 2010 at 10:21:48AM +0200, Valkeinen Tomi (Nokia-D/Helsinki) wrote: I think a simple solution would be to just use defines, and have functions that take the command as u8. That's what the OMAP DSI driver does. If you have better ideas, please share =). I find enums easier on the eye than defines. Less irrelevant junk on each line. There's no reason you can't pass enum values as u8. But in that case giving the enum a name doesn't really make sense. enums are cleaner for these cases, but you also have the case where the enum type itself is variable size depending on the ABI being used. If the type in question isn't being packed in to a user-visible data structure then this will never matter, but it does help to be a bit careful here regardless. Many people were bitten by this in the ARM OABI - EABI conversion, while other architectures generally managed to get it right from the onset. -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/4] fbdev: add a MIPI DSI header
(4:59), Guennadi Liakhovetski wrote:
This header adds defines for MIPI DSI and DCS commands and data formats. See
http://www.mipi.org/ for details.
Signed-off-by: Guennadi Liakhovetskig.liakhovet...@gmx.de
Console framebuffer tested on sh-2.6 tree and sh/dmaengine branch with
the necessary clock and intc patches applied.
Boot logo and framebuffer console displayed with no problems or unusual
effects.
Tested-by: Damian Hobson-Garcia dhobs...@igel.co.jp
---
include/video/mipi_dsi.h | 99
++
1 files changed, 99 insertions(+), 0 deletions(-)
create mode 100644 include/video/mipi_dsi.h
diff --git a/include/video/mipi_dsi.h b/include/video/mipi_dsi.h
new file mode 100644
index 000..5d3a6d6
--- /dev/null
+++ b/include/video/mipi_dsi.h
@@ -0,0 +1,99 @@
+/*
+ * Mobile Industry Processor Interface (MIPI(R)) defines
+ *
+ * Copyright (C) 2010 Guennadi Liakhovetskig.liakhovet...@gmx.de
+ * Copyright (C) 2006 Nokia Corporation
+ * Author: Imre Deakimre.d...@nokia.com
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+#ifndef MIPI_DSI_H
+#define MIPI_DSI_H
+
+#includelinux/types.h
+
+enum mipi_dsi_cmd {
+ MIPI_DSI_DCS_SHORT_WRITE= 5,
+ MIPI_DSI_DCS_SHORT_WRITE_PARAM = 0x15,
+ MIPI_DSI_COLOR_MODE_OFF = 2,
+ MIPI_DSI_COLOR_MODE_ON = 0x12,
+ MIPI_DSI_SHUTDOWN_PERIPHERAL= 0x22,
+ MIPI_DSI_TURN_ON_PERIPHERAL = 0x32,
+ MIPI_DSI_GENERIC_SHORT_WRITE_0_PARAM= 3,
+ MIPI_DSI_GENERIC_SHORT_WRITE_1_PARAM= 0x13,
+ MIPI_DSI_GENERIC_SHORT_WRITE_2_PARAM= 0x23,
+ MIPI_DSI_GENERIC_READ_REQUEST_0_PARAM = 4,
+ MIPI_DSI_GENERIC_READ_REQUEST_1_PARAM = 0x14,
+ MIPI_DSI_GENERIC_READ_REQUEST_2_PARAM = 0x24,
+ MIPI_DSI_DCS_LONG_WRITE = 0x39,
+ MIPI_DSI_SET_MAXIMUM_RETURN_PACKET_SIZE = 0x37,
+ MIPI_DSI_NULL_PACKET= 9,
+ MIPI_DSI_BLANKING_PACKET= 0x19,
+ MIPI_DSI_GENERIC_LONG_WRITE = 0x29,
+ MIPI_DSI_LOOSELY_PACKED_PIXEL_STREAM_YCBCR20= 0xc,
+ MIPI_DSI_PACKED_PIXEL_STREAM_YCBCR24= 0x1c,
+ MIPI_DSI_PACKED_PIXEL_STREAM_YCBCR16= 0x2c,
+ MIPI_DSI_PACKED_PIXEL_STREAM_30 = 0xd,
+ MIPI_DSI_PACKED_PIXEL_STREAM_36 = 0x1d,
+ MIPI_DSI_PACKED_PIXEL_STREAM_YCBCR12= 0x3d,
+ MIPI_DSI_PACKED_PIXEL_STREAM_16 = 0xe,
+ MIPI_DSI_PACKED_PIXEL_STREAM_18 = 0x1e,
+ MIPI_DSI_PIXEL_STREAM_3BYTE_18 = 0x2e,
+ MIPI_DSI_PACKED_PIXEL_STREAM_24 = 0x3e,
+};
+
+enum mipi_dcs_cmd {
+ MIPI_DCS_NOP= 0x00,
+ MIPI_DCS_SOFT_RESET = 0x01,
+ MIPI_DCS_GET_DISPLAY_ID = 0x04,
+ MIPI_DCS_GET_RED_CHANNEL= 0x06,
+ MIPI_DCS_GET_GREEN_CHANNEL = 0x07,
+ MIPI_DCS_GET_BLUE_CHANNEL = 0x08,
+ MIPI_DCS_GET_DISPLAY_STATUS = 0x09,
+ MIPI_DCS_GET_POWER_MODE = 0x0A,
+ MIPI_DCS_GET_ADDRESS_MODE = 0x0B,
+ MIPI_DCS_GET_PIXEL_FORMAT = 0x0C,
+ MIPI_DCS_GET_DISPLAY_MODE = 0x0D,
+ MIPI_DCS_GET_SIGNAL_MODE= 0x0E,
+ MIPI_DCS_GET_DIAGNOSTIC_RESULT = 0x0F,
+ MIPI_DCS_ENTER_SLEEP_MODE = 0x10,
+ MIPI_DCS_EXIT_SLEEP_MODE= 0x11,
+ MIPI_DCS_ENTER_PARTIAL_MODE = 0x12,
+ MIPI_DCS_ENTER_NORMAL_MODE = 0x13,
+ MIPI_DCS_EXIT_INVERT_MODE = 0x20,
+ MIPI_DCS_ENTER_INVERT_MODE = 0x21,
+ MIPI_DCS_SET_GAMMA_CURVE= 0x26,
+ MIPI_DCS_SET_DISPLAY_OFF= 0x28,
+ MIPI_DCS_SET_DISPLAY_ON = 0x29,
+ MIPI_DCS_SET_COLUMN_ADDRESS = 0x2A,
+ MIPI_DCS_SET_PAGE_ADDRESS = 0x2B,
+ MIPI_DCS_WRITE_MEMORY_START = 0x2C,
+ MIPI_DCS_WRITE_LUT = 0x2D,
+ MIPI_DCS_READ_MEMORY_START = 0x2E,
+ MIPI_DCS_SET_PARTIAL_AREA = 0x30,
+ MIPI_DCS_SET_SCROLL_AREA= 0x33,
+ MIPI_DCS_SET_TEAR_OFF = 0x34,
+ MIPI_DCS_SET_TEAR_ON= 0x35,
+ MIPI_DCS_SET_ADDRESS_MODE = 0x36,
+ MIPI_DCS_SET_SCROLL_START = 0x37,
+ MIPI_DCS_EXIT_IDLE_MODE = 0x38,
+ MIPI_DCS_ENTER_IDLE_MODE= 0x39,
+ MIPI_DCS_SET_PIXEL_FORMAT = 0x3A,
+ MIPI_DCS_WRITE_MEMORY_CONTINUE = 0x3C,
+ MIPI_DCS_READ_MEMORY_CONTINUE = 0x3E,
+ MIPI_DCS_SET_TEAR_SCANLINE = 0x44,
+ MIPI_DCS_GET_SCANLINE = 0x45,
+ MIPI_DCS_READ_DDB_START = 0xA1,
+
[PATCH 1/4] fbdev: add a MIPI DSI header
This header adds defines for MIPI DSI and DCS commands and data formats. See
http://www.mipi.org/ for details.
Signed-off-by: Guennadi Liakhovetski g.liakhovet...@gmx.de
---
include/video/mipi_dsi.h | 99 ++
1 files changed, 99 insertions(+), 0 deletions(-)
create mode 100644 include/video/mipi_dsi.h
diff --git a/include/video/mipi_dsi.h b/include/video/mipi_dsi.h
new file mode 100644
index 000..5d3a6d6
--- /dev/null
+++ b/include/video/mipi_dsi.h
@@ -0,0 +1,99 @@
+/*
+ * Mobile Industry Processor Interface (MIPI(R)) defines
+ *
+ * Copyright (C) 2010 Guennadi Liakhovetski g.liakhovet...@gmx.de
+ * Copyright (C) 2006 Nokia Corporation
+ * Author: Imre Deak imre.d...@nokia.com
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+#ifndef MIPI_DSI_H
+#define MIPI_DSI_H
+
+#include linux/types.h
+
+enum mipi_dsi_cmd {
+ MIPI_DSI_DCS_SHORT_WRITE= 5,
+ MIPI_DSI_DCS_SHORT_WRITE_PARAM = 0x15,
+ MIPI_DSI_COLOR_MODE_OFF = 2,
+ MIPI_DSI_COLOR_MODE_ON = 0x12,
+ MIPI_DSI_SHUTDOWN_PERIPHERAL= 0x22,
+ MIPI_DSI_TURN_ON_PERIPHERAL = 0x32,
+ MIPI_DSI_GENERIC_SHORT_WRITE_0_PARAM= 3,
+ MIPI_DSI_GENERIC_SHORT_WRITE_1_PARAM= 0x13,
+ MIPI_DSI_GENERIC_SHORT_WRITE_2_PARAM= 0x23,
+ MIPI_DSI_GENERIC_READ_REQUEST_0_PARAM = 4,
+ MIPI_DSI_GENERIC_READ_REQUEST_1_PARAM = 0x14,
+ MIPI_DSI_GENERIC_READ_REQUEST_2_PARAM = 0x24,
+ MIPI_DSI_DCS_LONG_WRITE = 0x39,
+ MIPI_DSI_SET_MAXIMUM_RETURN_PACKET_SIZE = 0x37,
+ MIPI_DSI_NULL_PACKET= 9,
+ MIPI_DSI_BLANKING_PACKET= 0x19,
+ MIPI_DSI_GENERIC_LONG_WRITE = 0x29,
+ MIPI_DSI_LOOSELY_PACKED_PIXEL_STREAM_YCBCR20= 0xc,
+ MIPI_DSI_PACKED_PIXEL_STREAM_YCBCR24= 0x1c,
+ MIPI_DSI_PACKED_PIXEL_STREAM_YCBCR16= 0x2c,
+ MIPI_DSI_PACKED_PIXEL_STREAM_30 = 0xd,
+ MIPI_DSI_PACKED_PIXEL_STREAM_36 = 0x1d,
+ MIPI_DSI_PACKED_PIXEL_STREAM_YCBCR12= 0x3d,
+ MIPI_DSI_PACKED_PIXEL_STREAM_16 = 0xe,
+ MIPI_DSI_PACKED_PIXEL_STREAM_18 = 0x1e,
+ MIPI_DSI_PIXEL_STREAM_3BYTE_18 = 0x2e,
+ MIPI_DSI_PACKED_PIXEL_STREAM_24 = 0x3e,
+};
+
+enum mipi_dcs_cmd {
+ MIPI_DCS_NOP= 0x00,
+ MIPI_DCS_SOFT_RESET = 0x01,
+ MIPI_DCS_GET_DISPLAY_ID = 0x04,
+ MIPI_DCS_GET_RED_CHANNEL= 0x06,
+ MIPI_DCS_GET_GREEN_CHANNEL = 0x07,
+ MIPI_DCS_GET_BLUE_CHANNEL = 0x08,
+ MIPI_DCS_GET_DISPLAY_STATUS = 0x09,
+ MIPI_DCS_GET_POWER_MODE = 0x0A,
+ MIPI_DCS_GET_ADDRESS_MODE = 0x0B,
+ MIPI_DCS_GET_PIXEL_FORMAT = 0x0C,
+ MIPI_DCS_GET_DISPLAY_MODE = 0x0D,
+ MIPI_DCS_GET_SIGNAL_MODE= 0x0E,
+ MIPI_DCS_GET_DIAGNOSTIC_RESULT = 0x0F,
+ MIPI_DCS_ENTER_SLEEP_MODE = 0x10,
+ MIPI_DCS_EXIT_SLEEP_MODE= 0x11,
+ MIPI_DCS_ENTER_PARTIAL_MODE = 0x12,
+ MIPI_DCS_ENTER_NORMAL_MODE = 0x13,
+ MIPI_DCS_EXIT_INVERT_MODE = 0x20,
+ MIPI_DCS_ENTER_INVERT_MODE = 0x21,
+ MIPI_DCS_SET_GAMMA_CURVE= 0x26,
+ MIPI_DCS_SET_DISPLAY_OFF= 0x28,
+ MIPI_DCS_SET_DISPLAY_ON = 0x29,
+ MIPI_DCS_SET_COLUMN_ADDRESS = 0x2A,
+ MIPI_DCS_SET_PAGE_ADDRESS = 0x2B,
+ MIPI_DCS_WRITE_MEMORY_START = 0x2C,
+ MIPI_DCS_WRITE_LUT = 0x2D,
+ MIPI_DCS_READ_MEMORY_START = 0x2E,
+ MIPI_DCS_SET_PARTIAL_AREA = 0x30,
+ MIPI_DCS_SET_SCROLL_AREA= 0x33,
+ MIPI_DCS_SET_TEAR_OFF = 0x34,
+ MIPI_DCS_SET_TEAR_ON= 0x35,
+ MIPI_DCS_SET_ADDRESS_MODE = 0x36,
+ MIPI_DCS_SET_SCROLL_START = 0x37,
+ MIPI_DCS_EXIT_IDLE_MODE = 0x38,
+ MIPI_DCS_ENTER_IDLE_MODE= 0x39,
+ MIPI_DCS_SET_PIXEL_FORMAT = 0x3A,
+ MIPI_DCS_WRITE_MEMORY_CONTINUE = 0x3C,
+ MIPI_DCS_READ_MEMORY_CONTINUE = 0x3E,
+ MIPI_DCS_SET_TEAR_SCANLINE = 0x44,
+ MIPI_DCS_GET_SCANLINE = 0x45,
+ MIPI_DCS_READ_DDB_START = 0xA1,
+ MIPI_DCS_READ_DDB_CONTINUE = 0xA8,
+};
+
+#define MIPI_DCS_PIXEL_FMT_24BIT 7
+#define MIPI_DCS_PIXEL_FMT_18BIT 6
+#define MIPI_DCS_PIXEL_FMT_16BIT 5
+#define MIPI_DCS_PIXEL_FMT_12BIT 3
+#define MIPI_DCS_PIXEL_FMT_8BIT
