From mboxrd@z Thu Jan 1 00:00:00 1970 From: Grant Likely Date: Wed, 19 May 2010 21:19:57 +0000 Subject: Re: [PATCH 1/3] video: add support for getting video mode from device Message-Id: List-Id: References: <1267307902-31939-2-git-send-email-agust@denx.de> <4B8A2CD7.7070305@firmworks.com> <1267415120.23523.1898.camel@pasglop> <20100428154303.45d1ce87@wker> In-Reply-To: <20100428154303.45d1ce87@wker> MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable To: Anatolij Gustschin Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, wd-ynQEQJNshbs@public.gmane.org, dzu-ynQEQJNshbs@public.gmane.org, devicetree-discuss , linuxppc-dev-mnsaURCQ41sdnm+yROfE0A@public.gmane.org, yorksun-KZfg59tc24xl57MIdRCFDg@public.gmane.org On Wed, Apr 28, 2010 at 7:43 AM, Anatolij Gustschin wrote: > On Mon, 01 Mar 2010 14:45:20 +1100 > Benjamin Herrenschmidt wrote: > >> On Sat, 2010-02-27 at 22:44 -1000, Mitch Bradley wrote: >> >> > As it turns out, I'm doing exactly that - exporting verbatim EDID >> > data >> > as the value of the "edid" property - for the display node on the Via >> > version of the OLPC machine. =A0The kernel driver uses it instead of >> > trying to obtain the EDID data from the monitor, because the builtin >> > OLPC display cannot supply EDID data through the usual hardware >> > interfaces. >> >> This is actually a common practice (though EDID is most often in >> uppercase) on Apple hardware too. It has issues though in the sense that >> it doesn't carry proper connector information and falls over in many >> multi-head cases. >> >> I think passing the EDID data, when available, is thus the right thing >> to do indeed, however that doesn't solve two problems: >> >> =A0- Where to put that property ? This is a complicated problem and we >> might argue on a binding for weeks because video cards typically support >> multiple outputs, etc. etc... I think the best for now is to stick as >> closely as possible to the existing OF fb binding, and have "display" >> nodes for each output, which can eventually contain an EDID. We might >> also want to add a string that indicate the connector type. Specific >> drivers might want to define additional properties here where it makes >> sense such as binding of those outputs to CRTCs or such, up to you. > > Putting EDID to display node would be really sufficient for LCDs in > our case. Other systems might define this additional connector type > property. > >> >> =A0- We -also- want a way to specify the "default" mode as set by the >> firmware, at least on some devices. The EDID gives capabilities, and >> often for LCDs also the "preferred" mode which is almost always the >> "default" mode ... but could be different. In order to avoid "flicking", >> the driver might wants to know what is the currently programmed mode. >> For that, having split out properties makes sense, though I would like >> to either prefix them all with "mode," or stick them in a sub-node of >> the display@. > > I would propose defining following properties in the case the > programmed mode is different from "default" mode: > > display@...{ > =A0 =A0 =A0 =A0compatible =3D "," > =A0 =A0 =A0 =A0EDID =3D [edid-data]; > > =A0 =A0 =A0 =A0current-mode { > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0pixel_clock =3D ; > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0horizontal_active =3D ; > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0horizontal_blank =3D ; > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0vertical_active =3D ; > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0vertical_blank =3D ; > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0horizontal_active =3D ; > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0hsync_offset =3D ; > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0hsync_pulse_width =3D ; > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0vsync_offset =3D ; > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0vsync_pulse_width =3D ; > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0hsync_positive; > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0vsync_positive; > =A0 =A0 =A0 =A0} > }; > > The firmware can set the "default" mode using the EDID's preferred > Detailed Timing Descriptor data. If on some devices it sets another > mode than the preferred mode, then the firmware can insert a > "current-mode" sub-node with currently programmed mode. The driver > can check for this sub-node and use it's data and if it isn't present, > it can use the preferred timing data from EDID. The names of the > properties here are actually what Detailed Timing Descriptor in EDID > specifies. What do you think? If you really want to do that, then I think it is okay. I really don't know enough about the problem space to say whether or not that particular description is a good binding or not, but regardless it should be a video-controller-specific binding. The name of the node should probably be prefixed with "," and it should be documented along with the display controller's device tree binding. If another controller wants/needs a different binding format (for the current mode), then that is fine too (unless you can make a really good argument that this current-mode binding is perfect and no other layout should ever be required). :-) Cheers, g. > > Thanks, > Anatolij > --=20 Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd.