Hi,<br><br>I have been busy today with packing, and tomorrow I will be unpacking to my university town.<br>Thanks to Joel for the review. I will make the modifications as soon as I get my PC up and running.<br><br>Also, to ease testing, there is a patch to rtems-testing pc386 script which runs qemu with cirrus emulation.<br>
<br>Mainly, I agree with ppisa.<br>Testing for cirrus driver could be done with a working graphics code.<br>Also, the toolkit should pe cloned for testing. The compressed patch is about 15MB.<br><br>Regards, <br>Alex Horin<br>
<br><div class="gmail_quote">On Thu, Sep 13, 2012 at 8:04 PM, Pavel Pisa <span dir="ltr"><<a href="mailto:ppisa4lists@pikron.com" target="_blank">ppisa4lists@pikron.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Hello Joel,<br>
<br>
thanks for review.<br>
<br>
I would check if Alex is available now.<br>
But to move things forward I would push the work if<br>
time allows me.<br>
<div class="im"><br>
On Thursday 13 September 2012 18:45:19 Joel Sherrill wrote:<br>
> Thanks for posting this so quickly. Here are some quick comments:<br>
><br>
> + No Makefile.am changes?<br>
<br>
</div>Not for now. I think, that it should be separate patch.<br>
Discussion on IRC. For simple test next<br>
<br>
To test it, next change is enough<br>
<br>
--- a/c/src/lib/libbsp/i386/pc386/Makefile.am<br>
+++ b/c/src/lib/libbsp/i386/pc386/Makefile.am<br>
@@ -83,3 +83,4 @@ include_HEADERS += ../../i386/shared/comm/i386_io.h<br>
 libbsp_a_SOURCES += console/defkeymap.c<br>
-libbsp_a_SOURCES += console/fb_vga.c<br>
+#libbsp_a_SOURCES += console/fb_vga.c<br>
+libbsp_a_SOURCES += console/fb_cirrus.c<br>
 libbsp_a_SOURCES += console/keyboard.c<br>
<br>
I incline to the next as interim solution<br>
<br>
--- a/c/src/lib/libbsp/i386/pc386/Makefile.am<br>
+++ b/c/src/lib/libbsp/i386/pc386/Makefile.am<br>
@@ -83,3 +83,4 @@ include_HEADERS += ../../i386/shared/comm/i386_io.h<br>
 libbsp_a_SOURCES += console/defkeymap.c<br>
+if ENABLE_VGA_CIRRUS<br>
 libbsp_a_SOURCES += console/fb_vga.c<br>
+else !ENABLE_VGA_CIRRUS<br>
+libbsp_a_SOURCES += console/fb_cirrus.c<br>
+endif !ENABLE_VGA_CIRRUS<br>
 libbsp_a_SOURCES += console/keyboard.c<br>
<br>
It should be replaced by infrastructure for runtime driver registration.<br>
<div class="im"><br>
> + C++ style comments need to be C<br>
> + Some one-line C comments do not have a space before the */<br>
> + Use single blank lines not multiple ones in a row<br>
<br>
</div>OK<br>
<div class="im"><br>
> + No license terms at top of file<br>
<br>
</div>What is the best template in the RTEMS sources?<br>
<div class="im"><br>
> I assume it works as as we get more into the graphics-addon patches<br>
> I will be able to duplicate your results.<br>
<br>
</div>It should work with any/old version of the graphic code<br>
or other independent projects.<br>
<br>
The rtems-graphics-toolkit is huge amount of the code and<br>
subprojects and number of copyend and changed code is huge too.<br>
<br>
Alex's version can be cloned from<br>
<br>
  git clone <a href="https://github.com/alex-sever-h/rtems-graphics-toolkit.git" target="_blank">https://github.com/alex-sever-h/rtems-graphics-toolkit.git</a><br>
<br>
I can help with some filterring and squashing changes<br>
to help maintain readable history but I do not think<br>
that changes load should flow through mailinglist.<br>
<br>
They should get in through pull after testing.<br>
<br>
Best wishes,<br>
<br>
             Pavel<br>
<br>
</blockquote></div><br>