[systemd-devel] [RFC 07/12] gfx: add graphics layer

[David Herrmann]

Hi

On Wed, Nov 27, 2013 at 11:24 PM, Lennart Poettering
<lennart at poettering.net> wrote:
> On Wed, 27.11.13 19:48, David Herrmann (dh.herrmann at gmail.com) wrote:
>
>> +typedef struct gfx_config_pipe gfx_config_pipe;
>> +typedef struct gfx_connector gfx_connector;
>> +typedef struct gfx_encoder gfx_encoder;
>> +
>> +/* clock-wise framebuffer rotation */
>> +enum {
>> +        GFX_ROTATE_0,
>> +        GFX_ROTATE_90,
>> +        GFX_ROTATE_180,
>> +        GFX_ROTATE_270,
>> +};
>> +
>> +struct gfx_config_pipe {
>> +        unsigned int config_id;
>> +        char **connectors;
>> +        char *mode;
>> +        unsigned int rotate;
>> +
>> +        unsigned int disable : 1;
>
> C99 bool plz! (here and everywhere else...)
>
>> +        memset(handles, 0, sizeof(handles));
>> +        memset(strides, 0, sizeof(strides));
>> +        memset(offsets, 0, sizeof(offsets));
>
> zero(handles) is so much nicer... (here and everywhere...)
>
>> +static int gfx_pipe_new(sd_gfx_pipe **out, sd_gfx_card *card, drmModeCrtc *crtc, unsigned int connector_count) {
>> +        sd_gfx_pipe *pipe;
>> +        int r;
>> +
>> +        pipe = calloc(1, sizeof(*pipe));
>> +        if (!pipe)
>> +                return log_oom();
>> +
>> +        pipe->ref = 1;
>> +        pipe->card = card;
>> +        pipe->id = card->pipe_ids;
>> +        pipe->crtc_id = crtc->crtc_id;
>> +        pipe->page_flip_cnt = 1;
>> +
>> +        pipe->connectors = calloc(connector_count, sizeof(*pipe->connectors));
>> +        if (!pipe->connectors) {
>> +                r = log_oom();
>> +                goto err_pipe;
>> +        }
>> +
>> +        pipe->want_connectors = calloc(connector_count, sizeof(*pipe->want_connectors));
>> +        if (!pipe->want_connectors) {
>> +                r = log_oom();
>> +                goto err_connectors;
>> +        }
>> +
>> +        pipe->connector_ids = calloc(connector_count, sizeof(*pipe->connector_ids));
>> +        if (!pipe->connector_ids) {
>> +                r = log_oom();
>> +                goto err_want_connectors;
>> +        }
>> +
>> +        pipe->want_connector_ids = calloc(connector_count, sizeof(*pipe->want_connector_ids));
>> +        if (!pipe->want_connector_ids) {
>> +                r = log_oom();
>> +                goto err_connector_ids;
>> +        }
>> +
>> +        r = gfx_plane_new_primary(&pipe->primary, pipe);
>> +        if (r < 0)
>> +                goto err_want_connector_ids;
>> +
>> +        gfx_pipe_refresh(pipe, crtc);
>> +
>> +        *out = pipe;
>> +        return 0;
>> +
>> +err_want_connector_ids:
>> +        free(pipe->want_connector_ids);
>> +err_connector_ids:
>> +        free(pipe->connector_ids);
>> +err_want_connectors:
>> +        free(pipe->want_connectors);
>> +err_connectors:
>> +        free(pipe->connectors);
>> +err_pipe:
>> +        free(pipe);
>> +        return r;
>> +}
>> +
>
> For clean-up code like this I usually find it nicer to simply have a
> destructor function that is robust to destruct half-initialized objects
> and then just invoke this here.

Ouh, indeed. I use calloc() anyway, so I could just make this one
single goto. Or make the destructor robust and call it instead, yepp.
Fixed.

>> +/* framebuffer */
>> +
>> +typedef void (*sd_gfx_fb_unlink_fn) (sd_gfx_fb *fb, void *fn_data);
>> +typedef void (*sd_gfx_fb_unpin_fn) (sd_gfx_fb *fb, void *fn_data);
>
> For the types that actually feel like primitive types (in contrast to
> objects), we usually appended a libc style _t to our names.

Ouh, libudev uses "udev_log_fn" so I followed that style. I thought
that's what we use for callback-prototypes. Is that just a left-over
from pre-systemd times? I can change it all to _t, if it is.

Thanks
David

> I can't really say much about the actual drm stuff going on here, just
> my usualy whining about logging from lib code, C99 bools, zero()...
>
> Lennart
>
> --
> Lennart Poettering, Red Hat