On Thu 08 Jun 2017, Daniel Stone wrote:
> From: Chad Versace <chadvers...@chromium.org>
>
> It converts a DRM format modifier to and from enum isl_tiling and
> aux_usage. That's all.
>
> Signed-off-by: Daniel Stone <dani...@collabora.com>
> ---
> src/intel/Makefile.isl.am | 1 +
> src/intel/isl/isl.c | 59
> +++++++++++++++++++++++++++++++++++++++++++++++
> src/intel/isl/isl.h | 16 +++++++++++++
> 3 files changed, 76 insertions(+)
>
> diff --git a/src/intel/Makefile.isl.am b/src/intel/Makefile.isl.am
> index ee2215df1d..f8bc142942 100644
> --- a/src/intel/Makefile.isl.am
> +++ b/src/intel/Makefile.isl.am
> @@ -33,6 +33,7 @@ noinst_LTLIBRARIES += $(ISL_GEN_LIBS) isl/libisl.la
>
> isl_libisl_la_LIBADD = $(ISL_GEN_LIBS)
> isl_libisl_la_SOURCES = $(ISL_FILES) $(ISL_GENERATED_FILES)
> +isl_libisl_la_CFLAGS = $(AM_CFLAGS) $(LIBDRM_CFLAGS)
>
> isl_libisl_gen4_la_SOURCES = $(ISL_GEN4_FILES)
> isl_libisl_gen4_la_CFLAGS = $(AM_CFLAGS) -DGEN_VERSIONx10=40
> diff --git a/src/intel/isl/isl.c b/src/intel/isl/isl.c
> index 60a594394b..2512d75447 100644
> --- a/src/intel/isl/isl.c
> +++ b/src/intel/isl/isl.c
> @@ -26,6 +26,7 @@
> #include <stdio.h>
>
> #include "genxml/genX_bits.h"
> +#include <drm_fourcc.h>
>
> #include "isl.h"
> #include "isl_gen4.h"
> @@ -35,6 +36,10 @@
> #include "isl_gen9.h"
> #include "isl_priv.h"
>
> +#ifndef DRM_FORMAT_MOD_LINEAR
> +#define DRM_FORMAT_MOD_LINEAR 0 /* Linux 4.10 */
> +#endif
> +
> void PRINTFLIKE(3, 4) UNUSED
> __isl_finishme(const char *file, int line, const char *fmt, ...)
> {
> @@ -48,6 +53,60 @@ __isl_finishme(const char *file, int line, const char
> *fmt, ...)
> fprintf(stderr, "%s:%d: FINISHME: %s\n", file, line, buf);
> }
>
> +bool
> +isl_tiling_from_drm_format_mod(uint64_t mod, enum isl_tiling *tiling,
> + enum isl_aux_usage *aux_usage)
> +{
This function looks good to me.
Alternatively, we could define two orthogonal functions:
bool
isl_tiling_from_drm_format_mod(uint64_t mod, enum isl_tiling *tiling);
bool
isl_aux_usage_from_drm_format_mod(uint64_t mod, enum isl_aux_usage
*aux_usage);
I'm just thinking out loud, though. Don't interpret that as a suggesting.
> + switch (mod) {
> + case DRM_FORMAT_MOD_LINEAR:
> + *tiling = ISL_TILING_LINEAR;
> + *aux_usage = ISL_AUX_USAGE_NONE;
> + return true;
> + case I915_FORMAT_MOD_X_TILED:
> + *tiling = ISL_TILING_X;
> + *aux_usage = ISL_AUX_USAGE_NONE;
> + return true;
> + case I915_FORMAT_MOD_Y_TILED:
> + *tiling = ISL_TILING_Y0;
> + *aux_usage = ISL_AUX_USAGE_NONE;
> + return true;
> + case I915_FORMAT_MOD_Yf_TILED:
> + *tiling = ISL_TILING_Yf;
> + *aux_usage = ISL_AUX_USAGE_NONE;
> + return true;
> + }
> +
> + return false;
> +}
> +
> +uint64_t
> +isl_tiling_to_drm_format_mod(enum isl_tiling tiling,
> + enum isl_aux_usage aux_usage)
> +{
> + /* XXX: Need to disambiguate aux surface usage; we can validly have a CCS
> + * aux surface attached (e.g. Y_CCS modifier), but always resolve it
> + * before usage such that sampling with no aux plane (e.g. plain Y
> mod)
> + * is valid. Punt for now, and revisit when we expose aux surfaces to
> + * external consumers. */
Hmm... this bothers me, obviously. I'll keep reading the patch series,
and return here after getting a better overview of how the patches work
together.
A small style nit... the terminal */ goes on its own line in Intel code (i965
too).
Regardless of the XXX comment, the function looks good to me. The
function does exactly what it claims to do.
> +
> + switch (tiling) {
> + case ISL_TILING_LINEAR:
> + return DRM_FORMAT_MOD_LINEAR;
> + case ISL_TILING_X:
> + return I915_FORMAT_MOD_X_TILED;
> + case ISL_TILING_Y0:
> + return I915_FORMAT_MOD_Y_TILED;
> + case ISL_TILING_Yf:
> + return I915_FORMAT_MOD_Yf_TILED;
> + case ISL_TILING_W:
> + case ISL_TILING_HIZ:
> + case ISL_TILING_CCS:
> + unreachable("non-color-buffer mode in isl_tiling_to_drm_format_mod");
> + }
> +
> + unreachable("unknown tiling in isl_tiling_to_drm_format_mod");
> +}
> +
> void
> isl_device_init(struct isl_device *dev,
> const struct gen_device_info *info,
> diff --git a/src/intel/isl/isl.h b/src/intel/isl/isl.h
> index 95ecaf90d8..30ed078ffd 100644
> --- a/src/intel/isl/isl.h
> +++ b/src/intel/isl/isl.h
> @@ -1482,6 +1482,22 @@ bool
> isl_has_matching_typed_storage_image_format(const struct gen_device_info
> *devinfo,
> enum isl_format fmt);
>
> +/**
> + * Map a DRM format modifier to tiling and aux usage. If the modifier is not
> + * known to ISL, return false. The output parameters are only written on
> + * success.
> + */
> +bool isl_tiling_from_drm_format_mod(uint64_t mod,
> + enum isl_tiling *tiling,
> + enum isl_aux_usage *aux_usage);
> +
> +/**
> + * Map tiling and aux usage to a DRM format modifier to tiling and aux usage.
> + * The parameters provided must map to a valid modifier.
> + */
> +uint64_t isl_tiling_to_drm_format_mod(enum isl_tiling tiling,
> + enum isl_aux_usage aux_usage);
> +
> static inline bool
> isl_tiling_is_any_y(enum isl_tiling tiling)
> {
> --
> 2.13.0
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
