On Mon, 11 Jul 2011, Laurent Pinchart wrote:
On Friday 24 June 2011 21:45:57 Florian Tobias Schandinat wrote:
On 06/24/2011 06:55 PM, Geert Uytterhoeven wrote:
On Fri, Jun 24, 2011 at 08:19, Paul Mundt wrote:
On Thu, Jun 23, 2011 at 06:08:03PM +0200, Geert Uytterhoeven wrote:
On Wed, Jun 22, 2011 at 07:45, Florian Tobias Schandinat wrote:
On 06/21/2011 10:31 PM, Laurent Pinchart wrote: > On Tuesday 21 June 2011 22:49:14 Geert Uytterhoeven wrote: >> As FOURCC values are always 4 ASCII characters (hence all 4 bytes >> must be non-zero), I don't think there are any conflicts with >> existing values of >> nonstd. To make it even safer and easier to parse, you could set bit >> 31 of >> nonstd as a FOURCC indicator. > > I would then create a union between nonstd and fourcc, and document > nonstd as > being used for the legacy API only. Most existing drivers use a > couple of nonstd bits only. The driver that (ab)uses nonstd the most > is pxafb and uses > bits 22:0. Bits 31:24 are never used as far as I can tell, so nonstd& > 0xff000000 != 0 could be used as a FOURCC mode test. > > This assumes that FOURCCs will never have their last character set to > '\0'. Is > that a safe assumption for the future ?
Yes, I think. The information I found indicates that space should be used for padding, so a \0 shouldn't exist. I think using only the nonstd field and requiring applications to check the capabilities would be possible, although not fool proof ;)
So we can declare the 8 msb bits of nonstd reserved, and assume FOURCC if any of them is set.
Nicely backwards compatible, as sane drivers should reject nonstd values they don't support (apps _will_ start filling in FOURCC values ignoring capabilities, won't they?).
That seems like a reasonable case, but if we're going to do that then certainly the nonstd bit encoding needs to be documented and treated as a hard ABI.
I'm not so sure about the if any bit in the upper byte is set assume FOURCC case though, there will presumably be other users in the future that will want bits for themselves, too. What exactly was the issue with having a FOURCC capability bit in the upper byte?
That indeed gives less issues (as long as you don't stuff 8-bit ASCII in the MSB) and more bits for tradiditional nonstd users.
The only disadvantage I can see is that it requires adding this bit in the application and stripping it when interpreting it. But I think the 24 lower bits should be enough for driver specific behavior (as the current values really can only be interpreted per driver).
I'm also not keen on adding/stripping the MSB. It would be easier for applications to use FOURCCs directly.
BTW, after giving it some more thought: what does the FB_CAP_FOURCC buys us? It's not like all drivers will support whatever random FOURCC mode you give them, so you have to check whether it's supported by doing a set_var first.
Because any solution purely based on the nonstd field is insane. The abuse of that field is just too widespread. Drivers that use nonstd usually only check whether it is zero or nonzero and others will just interpret parts of nonstd and ignore the rest. They will happily accept FOURCC values in the nonstd and just doing the wrong thing. Even if we would fix those the problems applications will run into with older kernels will remain.
I agree with Florian here. Many drivers currently check whether nonstd != 0. Who knows what kind of values applications feed them ?
FWIW, I prefer the original Laurent's proposal, with a slight uncertainty about whether we need the .capability field, or whether the try-and-check approach is sufficient. As for struct fb_var_screeninfo fields to support switching to a FOURCC mode, I also prefer an explicit dedicated flag to specify switching to it. Even though using FOURCC doesn't fit under the notion of a videomode, using one of .vmode bits is too tempting, so, I would actually take the plunge and use FB_VMODE_FOURCC.
As for the actual location of the fourcc code, I would leave .nonstd alone: who knows, maybe drivers will need both, whereas using grayscale and fourcc certainly doesn't make any sense. And I personally don't see a problem with using a union: buggy applications are, well, buggy... Actually, since in FOURCC mode we don't need any of
__u32 bits_per_pixel; /* guess what */ __u32 grayscale; /* != 0 Graylevels instead of colors */
struct fb_bitfield red; /* bitfield in fb mem if true color, */ struct fb_bitfield green; /* else only length is significant */ struct fb_bitfield blue; struct fb_bitfield transp; /* transparency */
so, we could put them all in the union for the case, if we need anything else for the fourcc configuration in the future.
I'd like to reach an agreement on the API, and implement it. I'm fine with either grayscale or nonstd to store the FOURCC (with a slight preference for grayscale), and with either a vmode flag or using the most significant byte of the grayscale/nonstd field to detect FOURCC mode. I believe FB_CAP_FOURCC (or something similar) is needed.
Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/