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 -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel
