[PATCH 5/6] i386/pc386/include: header files for VESA BIOS EXTENSIONS and VESA Extended Display Identification Data

[Gedare Bloom]

On Thu, Nov 13, 2014 at 6:45 PM, Pavel Pisa <pisa at cmp.felk.cvut.cz> wrote:
> Hello Gedare,
>
> the first thanks much for fast and valuable review.
> I expect that Jan Dolezal prepares new patch series
> next week (or two in the worst case).
>
> On Wednesday 12 of November 2014 17:04:04 Gedare Bloom wrote:
>> Just a couple high-level comments:
>>
>> Is this code entirely written by you?
>
> Especially VESA and EDID code is written from scratch by Jan Dolezal
> to make it license clear for RTEMS inclusion. It was prepared
> from reading and text retrieval from public specification only.
>
>> If the code is i386-specific, I'd like to see it put into i386 namespace.
>>
>> The use of bitfields worries me, especially if the code might be
>> ported to other architectures. I can see now why you put __packed__ on
>> all your structs, as it is necessary to get anywhere close to the
>> right behavior from the compiler when using bit fields.
>
>> On Wed, Nov 12, 2014 at 10:07 AM, Jan Dolezal <dolezj21 at fel.cvut.cz> wrote:
>> >  c/src/lib/libbsp/i386/pc386/Makefile.am    |   2 +
>> >  c/src/lib/libbsp/i386/pc386/include/edid.h | 759 +++++++++++++++++++++++++++++
>> >  c/src/lib/libbsp/i386/pc386/include/vbe3.h | 461 ++++++++++++++++++
>> >  c/src/lib/libbsp/i386/pc386/preinstall.am  |   8 +
>> >  4 files changed, 1230 insertions(+)
>> >  create mode 100644 c/src/lib/libbsp/i386/pc386/include/edid.h
>> >  create mode 100644 c/src/lib/libbsp/i386/pc386/include/vbe3.h
>
> I agree that bit fields are problematic but at least VESA specification
> is closely tied to x86 and BIOS. I cannot imagine that something
> so bizarre would be standardized for any other sane architecture.
> I can imagine only use of these structures on other architecture
> if it is architecture with PCI or PCI Express support and system
> should use some standard PC grade graphics card. Then there theoretical
> option to prepare and retrieve these data structures from native code
> and pass them to already mentioned x86 code interpreter which would
> execute x86 VESA BIOS provided by card. This combination is really
> scare. If you have smaller system then graphic support is usually
> included on CPU SoC. Use of external card without native driver
> cannot provide acceleration and would not be faster than integrated etc..
>
> So I think that for VESA part there is not big problem with bitfields
> and code is probably more readable that way. Change to some access
> macros solution should not be problem in future anyway if there
> is real need.
>
OK.

> The structures prepared according EDID standard are more questionable.
> These data are obtained with use of DDC in most cases and their decoding
> is necessary to support replaceable external monitors. The EDID data
> are usually used in read-only manner so use of access macros would
> not lead to so high overhead. Other option is to use solution
> with order switching macros to support bit-fields with both endiannesses.
> For correct big-endian support access macros are required even for
> all non-uint8_t fields and even then there could be problem with
> non aligned access. So for strict and simple portability it would
> be probably simpler to use only uint8_t type for EDID structures fields.
> I.e.
>
>   uint8_t MaxHorizontalImageSize_lo;
>   uint8_t MaxHorizontalImageSize_hi;
>
> and then combine fields at access time and use masks or bit position
>
>   fieldX[bit/8]&(1<<(bit&7))
>
> to retrieve "Established Timings" for example.
>
I would feel more comfortable with a portable solution for EDID since
it is supposed to be a "standard" data structure format. Bitfields are
acceptable to me for now, but must be given adequate warnings in the
comments section so that if anyone tries to port this code they know
about the potential issues. I'd imagine some (maybe you!) might be
interested in re-using this code base for other architectures though,
so if Jan has the time it would be best to switch to access macros
while the code is still fresh in his head.

Gedare

> More ideas and suggestions for preferred solution from you and others
> are highly welcomed.
>


> Thanks again,
>
>         Pavel
>
>
>