Hi Dan,
On 16 June 2017 at 18:14, Daniel Stone <dani...@collabora.com> wrote:
> Wayland buffers coming from wl_drm use the WL_DRM_FORMAT_* enums, which
> are identical to GBM_FORMAT_*. Similarly, FD imports do not need to
> convert between GBM and DRI FourCC, since they are (almost) completely
> compatible.
>
> Signed-off-by: Daniel Stone <dani...@collabora.com>
> ---
> src/gbm/backends/dri/gbm_dri.c | 62
> +++++++++++++++---------------------------
> 1 file changed, 22 insertions(+), 40 deletions(-)
>
> diff --git a/src/gbm/backends/dri/gbm_dri.c b/src/gbm/backends/dri/gbm_dri.c
> index 19be440d48..84f37d4cf5 100644
> --- a/src/gbm/backends/dri/gbm_dri.c
> +++ b/src/gbm/backends/dri/gbm_dri.c
> @@ -859,23 +859,9 @@ gbm_dri_bo_import(struct gbm_device *gbm,
>
> image = dri->image->dupImage(wb->driver_buffer, NULL);
>
> - switch (wb->format) {
> - case WL_DRM_FORMAT_XRGB8888:
> - gbm_format = GBM_FORMAT_XRGB8888;
> - break;
> - case WL_DRM_FORMAT_ARGB8888:
> - gbm_format = GBM_FORMAT_ARGB8888;
> - break;
> - case WL_DRM_FORMAT_RGB565:
> - gbm_format = GBM_FORMAT_RGB565;
> - break;
> - case WL_DRM_FORMAT_YUYV:
> - gbm_format = GBM_FORMAT_YUYV;
> - break;
> - default:
> - dri->image->destroyImage(image);
> - return NULL;
> - }
> + /* GBM_FORMAT_* is identical to WL_DRM_FORMAT_*, so no conversion
> + * required. */
> + gbm_format = wb->format;
AFACIT not all the WL_DRM_FORMAT_* have a GBM_FORMAT_* equivalent.
Regardless, I think we're safe atm. (as the formats supported are
identical on both gbm/wl side) but it will cause subtle breakage as
one enhances the WL side. Please add a note if you prefer to keep the
hunk.
> break;
> }
> #endif
> @@ -904,23 +890,27 @@ gbm_dri_bo_import(struct gbm_device *gbm,
> {
> struct gbm_import_fd_data *fd_data = buffer;
> int stride = fd_data->stride, offset = 0;
> - int dri_format;
> + int fourcc;
>
> + /* GBM's GBM_FORMAT_* tokens are a strict superset of the DRI FourCC
> + * tokens accepted by createImageFromFds, except for not supporting
> + * the sARGB format. Also, GBM_BO_FORMAT_* are defined differently to
> + * their GBM_FORMAT_* equivalents, so remap them here. */
> switch (fd_data->format) {
> case GBM_BO_FORMAT_XRGB8888:
> - dri_format = GBM_FORMAT_XRGB8888;
> + fourcc = GBM_FORMAT_XRGB8888;
> break;
> case GBM_BO_FORMAT_ARGB8888:
> - dri_format = GBM_FORMAT_ARGB8888;
> + fourcc = GBM_FORMAT_ARGB8888;
> break;
> default:
> - dri_format = fd_data->format;
> + fourcc = fd_data->format;
> }
>
This and below is fine, but I think we'd want to have a "make check"
test at a later stage.
With the WL addressed (left as original or with a comment), patch is
Reviewed-by: Emil Velikov <emil.veli...@collabora.com>
-Emil
Formats have bit us in the past, so it's better to catch that before
we break Gnome/mutter, KDE, Weston etc.
To minimise test/script update the test and ensure the headers are
sane for consumption, I'm thinking of the following:
- script(?) that greps for GBM_FORMAT_* gbm.h + __DRI_IMAGE_FOURCC
dri_interface.h
- compares the values - say a simple C program but anything will do
#include "gbm.h"
#include "dri_interface.h"
int
main(void)
{
assert(GBM_FORMAT_FOO == __DRI_IMAGE_FOURCC_FOO);
....
}
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
