Hi!
On Wed, Feb 27, 2013 at 06:13:49PM +0200, Tomi Valkeinen wrote:
On 2013-02-27 18:05, Steffen Trumtrar wrote:
Ah, sorry. Forgot to answer this.
On Wed, Feb 27, 2013 at 05:45:31PM +0200, Tomi Valkeinen wrote:
Ping.
On 2013-02-18 16:09, Tomi Valkeinen wrote:
Hi Steffen,
On 2013-01-25 11:01, Steffen Trumtrar wrote:
+/* VESA display monitor timing parameters */ +#define VESA_DMT_HSYNC_LOW BIT(0) +#define VESA_DMT_HSYNC_HIGH BIT(1) +#define VESA_DMT_VSYNC_LOW BIT(2) +#define VESA_DMT_VSYNC_HIGH BIT(3)
+/* display specific flags */ +#define DISPLAY_FLAGS_DE_LOW BIT(0) /* data enable flag */ +#define DISPLAY_FLAGS_DE_HIGH BIT(1) +#define DISPLAY_FLAGS_PIXDATA_POSEDGE BIT(2) /* drive data on pos. edge */ +#define DISPLAY_FLAGS_PIXDATA_NEGEDGE BIT(3) /* drive data on neg. edge */ +#define DISPLAY_FLAGS_INTERLACED BIT(4) +#define DISPLAY_FLAGS_DOUBLESCAN BIT(5)
<snip>
- unsigned int dmt_flags; /* VESA DMT flags */
- unsigned int data_flags; /* video data flags */
Why did you go for this approach? To be able to represent true/false/not-specified?
We decided somewhere between v3 and v8 (I think), that those flags can be high/low/ignored.
Okay. Why aren't they enums, though? That always makes more clear which defines are to be used with which fields.
Hm...
Would it be simpler to just have "flags" field? What does it give us to have those two separately?
I decided to split them, so it is clear that some flags are VESA defined and the others are "invented" for the display-timings framework and may be extended.
Hmm... Okay. Is it relevant that they are VESA defined? It just feels to complicate handling the flags =).
Should the above say raising edge/falling edge instead of positive edge/negative edge?
Hm, I used posedge/negedge because it is shorter (and because of my Verilog past pretty natural to me :-) ). I don't know what others are thinking though.
I guess it's quite clear, but it's still different terms than used elsewhere, e.g. documentation for videomodes.
Another thing I noticed while using the new videomode, display_timings.h has a few names that are quite short and generic. Like "TE_MIN", which is now a global define. And "timing_entry". Either name could be well used internally in some .c file, and could easily clash.
Yes. You are correct. Everyone using this is welcome to send patches now :-)
Regards, Steffen