[Mesa-dev] [RFC 02/22] intel/isl: Add ISL <-> DRM modifier conversion
Jason Ekstrand
jason at jlekstrand.net
Wed Jun 14 06:46:58 UTC 2017
On June 13, 2017 23:19:28 Chad Versace <chad at kiwitree.net> wrote:
> On Thu 08 Jun 2017, Daniel Stone wrote:
>> From: Chad Versace <chadversary at 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 <daniels at 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.
I've got an even better plan... While reworking Ben's patches for the CCS
modifier in the dri driver, I added an isl_drm_modifier_info struct and a
function to look up the info. I'm hoping to get those patches sent out
this week. I can provide a branch tomorrow.
>> + 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 at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
More information about the mesa-dev
mailing list