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 <dolez...@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.
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.
More ideas and suggestions for preferred solution from you and others
are highly welcomed.
Thanks again,
Pavel
_______________________________________________
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel
