[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [tyndur-devel] [PATCH v2] + cdi: video-Header

Date: Sun, 25 Oct 2009 14:45:54 +0100
From: Kevin Wolf <kevin@xxxxxxxxxx>
To: tyndur-devel@xxxxxxxxxx
Subject: Re: [tyndur-devel] [PATCH v2] + cdi: video-Header
Message-id: <20091025134551.GA15422@silvanus>
In-reply-to: <1255181508-22334-1-git-send-email-alex@tyndur.org>
On Sat, Oct 10, 2009 at 03:31:44PM +0200, Alexander Siol wrote:
> Signed-off-by: Alexander Siol <alex@xxxxxxxxxx>
> ---
>  src/modules/cdi/include/cdi.h       |    1 +
>  src/modules/cdi/include/cdi/vesa.h  |  118 ++++++++++++++
>  src/modules/cdi/include/cdi/video.h |  297 +++++++++++++++++++++++++++++++++++
>  3 files changed, 416 insertions(+), 0 deletions(-)
>  create mode 100644 src/modules/cdi/include/cdi/vesa.h
>  create mode 100644 src/modules/cdi/include/cdi/video.h
> 
> diff --git a/src/modules/cdi/include/cdi.h b/src/modules/cdi/include/cdi.h
> index 7bcf9e4..fee8976 100644
> --- a/src/modules/cdi/include/cdi.h
> +++ b/src/modules/cdi/include/cdi.h
> @@ -26,6 +26,7 @@ typedef enum {
>      CDI_NETWORK         = 1,
>      CDI_STORAGE         = 2,
>      CDI_SCSI            = 3,
> +    CDI_VIDEO           = 4,
>  } cdi_device_type_t;
>  
>  struct cdi_driver;
> diff --git a/src/modules/cdi/include/cdi/vesa.h b/src/modules/cdi/include/cdi/vesa.h
> new file mode 100644
> index 0000000..1f0eaaa
> --- /dev/null
> +++ b/src/modules/cdi/include/cdi/vesa.h
> @@ -0,0 +1,118 @@
> +/*
> + * Copyright (c) 2008-2009 Mathias Gottschlag, Janosch Graef, Alexander Siol
> + *
> + * This program is free software. It comes without any warranty, to
> + * the extent permitted by applicable law. You can redistribute it
> + * and/or modify it under the terms of the Do What The Fuck You Want 
> + * To Public License, Version 2, as published by Sam Hocevar. See
> + * http://sam.zoy.org/projects/COPYING.WTFPL for more details.
> + */
> +
> +#ifndef _CDI_VESA_H_
> +#define _CDI_VESA_H_
> +
> +#include <stdint.h>
> +#include <cdi/lists.h>
> +#include <cdi/video.h>
> +
> +/**
> + * Allgemeiner VESA / VBE Informationsblock, wird verwendet um Informationen
> + * ueber vorhande Faehigkeiten und Modi der Grafikkarte zu erhalten.
> + */
> +struct cdi_video_vesa_info_block {
> +    char signature[4];
> +    char version[2];
> +    uint32_t oemname;
> +    uint32_t capabilities;
> +    uint32_t modeptr;
> +    uint16_t videomem;
> +    // VBE 2.0
> +    uint16_t oemversion;
> +    uint32_t vendornameptr;
> +    uint32_t productnameptr;
> +    uint32_t revisionptr;
> +    uint16_t modes[111];
> +    uint8_t  oem[256];
> +} __attribute__ ((packed));
> +
> +/**
> + * VBE Modus-Informationsblock.
> + * Enthaelt Infos ueber den speziellen Modus.
> + */
> +struct cdi_video_vesa_mode_info {
> +    uint16_t modeattr;
> +    uint8_t  windowattra;
> +    uint8_t  windowattrb;
> +    uint16_t windowgran;
> +    uint16_t windowsize;
> +    uint16_t startsega;
> +    uint16_t startsegb;
> +    uint32_t posfunc;
> +    uint16_t scanline;
> +
> +    uint16_t width;
> +    uint16_t height;
> +    uint8_t  charwidth;
> +    uint8_t  charheight;
> +    uint8_t  planecount;
> +    uint8_t  depth;
> +    uint8_t  banks;
> +    uint8_t  type;
> +    uint8_t  banksize;
> +    uint8_t  imagepages;
> +    uint8_t  reserved;
> +    // VBE v1.2+
> +    uint8_t  redmasksize;
> +    uint8_t  redfieldpos;
> +    uint8_t  greenmasksize;
> +    uint8_t  greenfieldpos;
> +    uint8_t  bluemasksize;
> +    uint8_t  bluefieldpos;
> +    uint8_t  resmasksize;
> +    uint8_t  resfieldsize;
> +    uint8_t  dircolormode;
> +    // VBE v2.0
> +    uint32_t linearfb;
> +    uint32_t offscreenmem;
> +    uint32_t offscreensize;

Nicht, dass es einen Unterschied machen würde, aber das sollte ein
uint16_t sein.

> +
> +    char reserved2[206];
> +} __attribute__ ((packed));
> +
> +/**
> + * CDI-Video-Mode mit Erweiterung um VESA/VBE-Modusnummer
> + */
> +struct cdi_video_vesa_mode {
> +    struct cdi_video_displaymode mode;
> +    uint16_t vesamode;
> +};
> +
> +/**
> + * Liest den allgemeinen VESA/VBE-Informationsblock aus und uebergibt einen
> + * Pointer auf diesen, ruft fuer jeden gefundenen Modus einen
> + * Callback auf.
> + * @param vesainfo Zeiger auf Infoblock-Zeiger
> + * @param mode_callback Zeiger auf Modus-Callback-Funktion
> + * @return 0 bei Erfolg, -1 im Fehlerfall
> + */
> +int cdi_video_vesa_initialize
> +    (struct cdi_video_vesa_info_block **vesainfo,
> +    void (*mode_callback)(int modenum, struct cdi_video_vesa_mode_info *mode));

Sterne gehören zum Typ.

Sollte die Funktion nicht eher cdi_video_vesa_enumerate_modes oder sowas
heißen? Falls sie tatsächlich was initialisiert, sollte das in den
Kommentar mit rein. Kann ich auf einen Callback verzichten und NULL
übergeben, wenn ich einfach nur den allgemeinen Infoblock will?

> +
> +/**
> + * Liefert Informationen zu einem spezifischen Modus
> + * @param modenum Die Modusnummer - fuer gueltige Nummern siehe den allgemeinen
> + *  Infoblock
> + * @return Zeiger auf Modus-Informationsblock. 0 bei Fehler.
> + */
> +struct cdi_video_vesa_mode_info* cdi_video_vesa_get_mode(int modenum);

Woher kommt der Block? malloc oder ein statischer Buffer? Muss im
Kommentar stehen, weil der Aufrufer entweder free aufrufen muss oder
darauf achten, dass nach einem zweiten Aufruf der Buffer ueberschriebn
ist.

Im Zweifelsfall würde ich empfehlen, den Aufrufer einfach einen Pointer
übergeben zu lassen, dann kann er selber entscheiden, was er will.

> +
> +/**
> + * Setzt den Modus via int10
> + * @param modenum Die Modusnummer - fuer gueltige Nummern siehe den allgemeinen
> + *  Infoblock
> + * @return 0 bei Erfolg, -1 im Fehlerfall
> + */
> +int cdi_video_vesa_change_mode(int modenum);
> +
> +#endif
> diff --git a/src/modules/cdi/include/cdi/video.h b/src/modules/cdi/include/cdi/video.h
> new file mode 100644
> index 0000000..727e476
> --- /dev/null
> +++ b/src/modules/cdi/include/cdi/video.h
> @@ -0,0 +1,297 @@
> +/*
> + * Copyright (c) 2007-2009 Mathias Gottschlag, Janosch Graef, Alexander Siol
> + *
> + * This program is free software. It comes without any warranty, to
> + * the extent permitted by applicable law. You can redistribute it 
> + * and/or modify it under the terms of the Do What The Fuck You Want 
> + * To Public License, Version 2, as published by Sam Hocevar. See
> + * http://sam.zoy.org/projects/COPYING.WTFPL for more details.
> + */
> +
> +#ifndef _CDI_VIDEO_H_
> +#define _CDI_VIDEO_H_
> +
> +#include "cdi.h"
> +#include "cdi/lists.h"
> +
> +/**
> + * Modus fuer Anzeigen
> + */
> +struct cdi_video_displaymode {
> +    unsigned int width;
> +    unsigned int height;
> +    unsigned int depth; // In BPP
> +    int refreshrate; // Vsync in Hz, 0 fuer Irrelevant/Automatisch
> +};

Die Kommentare besser als doxygen-Kommentar:

/** In Bits pro Pixel */
unsigned int depth;

> +
> +enum cdi_video_bitmap_usage_hint {
> +  CDI_VIDEO_PRIO_HIGH,
> +  CDI_VIDEO_PRIO_NORMAL,
> +  CDI_VIDEO_PRIO_LOW,
> +};
> +
> +struct cdi_video_pixel_format {
> +    uint8_t bpp;
> +    uint8_t red_bits;
> +    uint8_t red_offset;
> +    uint8_t green_bits;
> +    uint8_t green_offset;
> +    uint8_t blue_bits;
> +    uint8_t blue_offset;
> +    uint8_t alpha_bits;
> +    uint8_t alpha_offset;
> +};
> +
> +struct cdi_video_bitmap {
> +    /// Aufloesung des Bitmaps - Breite
> +    int width;
> +    /// Aufloesung des Bitmaps - Hoehe
> +    int height;

Größe, nicht Auflösung

> +    /// Verwendetes Pixelformat
> +    struct cdi_video_pixel_format format;
> +
> +    /// Zeilenlaenge in Bytes. Achtung, muss nicht width * bpp sein!
> +    int pitch;
> +
> +    /// Das Geraet zu dem das Bitmap gehoert.
> +    struct cdi_video_device* device;
> +
> +    /// 1 wenn im VRAM, 0 wenn im RAM
> +    int in_vram;
> +
> +    /// Addresse (ggf. im VRAM)
> +    int* pixeldata;

Hm, wieso ausgerechnet int? Wäre void* nicht logischer, wenn du nicht
weißt, welchen Typ die Daten haben? Und wenn du von einer festen Größe
ausgehen willst, nimm uintXX_t.

> +};
> +
> +enum cdi_video_raster_op {
> +    CDI_VIDEO_ROP_COPY,
> +    CDI_VIDEO_ROP_OR,
> +    CDI_VIDEO_ROP_AND,
> +    CDI_VIDEO_ROP_XOR,
> +    // TODO: Erweitern
> +};
> +
> +struct cdi_video_device {
> +    /// CDI-Geraet
> +    struct cdi_device dev;
> +
> +    /// Liste der Bildschirme
> +    cdi_list_t displays;
> +
> +    /// Aktuelle RasterOP (fuer Software-Helfer noetig)
> +    enum cdi_video_raster_op raster_op;
> +
> +    /// Daten zur Moduswiederherstellung
> +    void* restore_data;
> +
> +    /// Pointer zum Beginn des (gemappten) VRAM
> +    void* vram;
> +
> +    /// Groesse des VRAM
> +    size_t vram_size;
> +};
> +
> +/**
> + * Eine benutzbare Anzeige - Geraet, Modus, verfuegbare Modi und der Frontbuffer
> + */
> +struct cdi_video_display {
> +    /// Videogeraet, dem dieser Bildschirm angehoert
> +    struct cdi_video_device* device;
> +
> +    /// Momentaner Modus
> +    struct cdi_video_displaymode* mode;
> +
> +    /// Liste mit verfuegbaren Modi
> +    cdi_list_t modes;
> +
> +    /// Zeichenbuffer fuer Display
> +    struct cdi_video_bitmap* frontbuffer;
> +};
> +
> +/**
> + * Speichert eine Farbe in jeweils 8-bit ARGB.
> + */
> +struct cdi_video_color {
> +    uint8_t alpha;
> +    uint8_t red;
> +    uint8_t green;
> +    uint8_t blue;
> +};
> +
> +struct cdi_video_driver {
> +    /// CDI-Treiber
> +    struct cdi_driver drv;
> +
> +    // Basis-Funktionen
> +    /**
> +     * Sichert die aktiven Modi der Displays zur spaeteren Wiederherstellung
> +     *  @param device Das zu sichernde Device
> +     *  @return 0 = Erfolg; -1 = Fehlschlag
> +     */
> +    int (*driver_save)(struct cdi_video_device* device);
> +
> +    /**
> +     * Stellt den gesicherten Zustand wieder her.
> +     *  @param device Das Device das wiederherstellen soll
> +     *  @return 0 = Erfolg; -1 = Fehlschlag
> +     */
> +    int (*driver_restore)(struct cdi_video_device* device);
> +
> +
> +
> +    // Anzeigenkontrolle
> +
> +    /**
> +     * Setzt die Aufloesung eines Bildschirms
> +     *  @param display Bildschirm
> +     *  @param mode Modus
> +     *  @return 0 = Erfolg; -1 = Fehlschlag
> +     */
> +    int (*display_set_mode) 

Leerzeichen am Zeilenende

> +        (struct cdi_video_device* device, struct cdi_video_display* display,
> +        struct cdi_video_displaymode* mode);

Ist die Information, welches device ich will, nicht mit dem display
schon implizit übergeben?

> +
> +
> +
> +    // Bitmap-Funktionen
> +
> +    /**
> +     * Erstellt ein Bitmap
> +     * @param device Das Geraet auf dem das Bitmap liegt
> +     * @param width Breite des Bitmaps
> +     * @param height Hoehe des Bitmaps
> +     * @param data Pixeldaten, oder 0, wenn keine Initialisierung gewuenscht ist
> +     * @return Zeiger auf die neue Bitmap
> +     */
> +    struct cdi_video_bitmap* (*bitmap_create)
> +        (struct cdi_video_device* device, unsigned int width,
> +        unsigned int height, void* data);
> +
> +    /**
> +     * Loescht ein Bitmap
> +     * @param bitmap Zu loeschendes Bitmap
> +     */
> +    void (*bitmap_destroy)
> +        (struct cdi_video_bitmap* bitmap);
> +
> +    /**
> +     * Setzt den Gebrauchsmodus eines Bitmaps (bestimmt zB, ob ein Bitmap gecacht wird)
> +     * @param device Videogeraet
> +     * @param bitmap Betroffenes Bitmap
> +     * @param hint Hinweis auf die Prioritaet des Bitmaps
> +     */
> +    void (*bitmap_set_usage_hint)
> +        (struct cdi_video_device* device, struct cdi_video_bitmap* bitmap,
> +        enum cdi_video_bitmap_usage_hint hint);
> +
> +
> +
> +    // Zeichen-Funktionen
> +
> +    /**
> +     * Setzt den Raster-Op fuer die folgenden Zeichenfunktionen.
> +     * Der Raster-Op bestimmt, auf welche Art Grafik gezeichnet wird.
> +     * @param target Bitmap auf das gezeichnet werden soll
> +     * @param rop Raster-Op
> +     */
> +    void (*set_raster_op)
> +        (struct cdi_video_device* device, enum cdi_video_raster_op rop);
> +
> +    /**
> +     * Zeichnet eine Linie
> +     * @param target Bitmap auf das gezeichnet wird
> +     * @param x1 X-Koordinate des Anfangs
> +     * @param y1 Y-Koordinate des Anfangs
> +     * @param x1 X-Koordinate des Endpunktes
> +     * @param y1 Y-Koordinate des Endpunktes
> +     * @param color Farbe der Linie
> +     */
> +    void (*draw_line)
> +        (struct cdi_video_bitmap* target, unsigned int x1, unsigned int y1,
> +        unsigned int x2, unsigned int y2, struct cdi_video_color* color);
> +
> +    /**
> +     * Zeichnet ein Rechteck
> +     * @param target Bitmap auf das gezeichnet wird
> +     * @param x X-Koordinate
> +     * @param y Y-Koordinate
> +     * @param width Breite
> +     * @param height Hoehe
> +     * @param color Farbe des Rechtecks
> +     */
> +    void (*draw_rectangle)
> +        (struct cdi_video_bitmap* target, unsigned int x, unsigned int y,
> +        unsigned int width, unsigned int height, struct cdi_video_color* color);
> +
> +    /**
> +     * Zeichnet eine Ellipse
> +     * @param target Bitmap auf das gezeichnet wird
> +     * @param x X-Koordinate
> +     * @param y Y-Koordinate
> +     * @param width Breite
> +     * @param height Hoehe
> +     * @param color Farbe der Ellipse
> +     */
> +    void (*draw_ellipse)
> +        (struct cdi_video_bitmap* target, unsigned int x, unsigned int y,
> +        unsigned int width, unsigned int height, struct cdi_video_color* color);
> +
> +    /**
> +     * Zeichnet ein Bitmap
> +     * @param target Bitmap auf das gezeichnet wird
> +     * @param bitmap Zu zeichnendes Bitmap
> +     * @param x X-Koordinate
> +     * @param y Y-Koordinate
> +     * @note Es wird nur so viel kopiert wie das Zielbitmap aufnehmen kann
> +     */
> +    void (*draw_bitmap)
> +        (struct cdi_video_bitmap* target, struct cdi_video_bitmap* bitmap,
> +        unsigned int x, unsigned int y);
> +
> +    /**
> +     * Zeichnet einen Teil einer Bitmap
> +     * @param target Bitmap auf das gezeichnet wird
> +     * @param bitmap Zu zeichnendes Bitmap
> +     * @param x X-Koordinate (Ziel)
> +     * @param y Y-Koordinate (Ziel)
> +     * @param srcx X-Koordinate (Quelle)
> +     * @param srcy Y-Koordinate (Quelle)
> +     * @param width Breite
> +     * @param height Hoehe
> +     */
> +    void (*draw_bitmap_part)
> +        (struct cdi_video_bitmap* target, struct cdi_video_bitmap* bitmap,
> +        unsigned int x, unsigned int y, unsigned int srcx, unsigned int srcy,
> +        unsigned int width, unsigned int height);
> +
> +    /**
> +     * Zeichnet einen Punkt
> +     * @param target Display, auf dem gezeichnet wird
> +     * @param x X-Koordinate (Ziel)
> +     * @param y Y-Koordinate (Ziel)
> +     * @param color Zeiger auf die zu verwendende Farbe
> +     * @note Sollte Sparsam eingesetzt werden, Viele Punkte so zeichnen ist
> +     *  meistens sehr langsam.
> +     */
> +    void (*draw_dot)
> +        (struct cdi_video_bitmap* target, unsigned int x, unsigned int y,
> +        struct cdi_video_color* color);

draw_pixel?
References:
- [tyndur-devel] [PATCH v2] + cdi: video-Header
  - From: Alexander Siol
Prev by Date: Re: [tyndur-devel] [PATCH] libc: ntohs gefixt
Next by Date: Re: [tyndur-devel] [PATCH v2] + libvideo: Header
Previous by thread: [PATCH v2] +gfxtest: Testprogramm für videolib und cdi.video
Next by thread: [tyndur-devel] [PATCH] FTP-Client: Befehlserweiterung und mehrere kleine Bugs behoben
Index(es):
- Date
- Thread