Hi Emil,
On 26 March 2018 at 16:22, Emil Velikov <emil.l.velikov@xxxxxxxxx> wrote:
> On 23 March 2018 at 13:50, Daniel Stone <daniels@xxxxxxxxxxxxx> wrote:
>> Much like AddFB -> AddFB2, GetFB2 lets us get multiple buffers back as
>> well as modifier information. This lets us use -background none with
>> multiplanar formats, or modifiers which can't be inferred via a magic
>> side channel.
>>
> AFAICT there's nothing special (or wrong) with the new API.
>
> The flags field is a bit of an open question - should Xserver or
> libdrm manage the value passed to the kernel?
> Analogously - should we pass the flags back to the user (drmModeFB2::flags)?
>
> Current design seems perfectly fine IMHO, although others might disagree.
Thanks for looking at this! I think the flags question is a good one,
and that we should probably make the field RW: userspace declaring
what it supports (analagous to AddFB2), and the kernel overwriting
that with the intersection of what userspace and the kernel support
(not analogous, but since it's a getter rather than an add ...).
>> @@ -1090,31 +1094,63 @@ create_pixmap_for_fbcon(drmmode_ptr drmmode, ScrnInfoPtr pScrn, int fbcon_id)
>> if (pixmap)
>> return pixmap;
>>
>> - fbcon = drmModeGetFB(drmmode->fd, fbcon_id);
>> - if (fbcon == NULL)
>> - return NULL;
>> + fbcon2 = drmModeGetFB2(drmmode->fd, fbcon_id);
>> + if (fbcon2) {
> Declare and initialize fbcon2 in one go - C99 feature that everyone
> has (even the people who use MSVC to build Xserver & friends).
> Then wrap the whole fbcon2 hunk in a #ifdef HAVE_GETFB2 + add a
> configure/meson check.
Sure; the only reason I didn't add an ifdef check is because there's
no version released with it yet.
> Alternatively add a weak drmModeGetFB2 function which returns NULL and
> forget all the above ;-)
If you have a good suggestion for implementing weak symbols then I'm
all ears, but last I saw from the Mesa experience no-one could figure
out how to do it properly.
>> + width = fbcon2->width;
>> + height = fbcon2->height;
>> + memcpy(handles, fbcon2->handles, sizeof(handles));
>> + memcpy(strides, fbcon2->pitches, sizeof(strides));
>> + memcpy(offsets, fbcon2->offsets, sizeof(offsets));
>> + modifier = fbcon2->modifier;
>> + switch (fbcon2->pixel_format) {
>> + case DRM_FORMAT_RGB565:
>> + depth = 16;
> Missing bpp?
Correct.
> Idea: Instead of introducing another format <> {depth, bpp} mapping,
> might as well add some helpers?
I can only see that mapping inside drmmode_create_bo, which is
different as it creates everything with an alpha channel, i.e.
s/XRGB/ARGB/. Are there some others I'm missing?
>> + break;
>> + case DRM_FORMAT_XRGB8888:
>> + depth = 24;
>> + bpp = 32;
>> + break;
>> + case DRM_FORMAT_XRGB2101010:
>> + depth = 30;
>> + bpp = 32;
>> + default:
>> + break;
> Error instead of silently continuing?
Sure.
Cheers,
Daniel
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
