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. >>> 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. Tomi
Attachment:
signature.asc
Description: OpenPGP digital signature
