Hi Ayan,
Could we preserve Ray's authorship on this patch?
On Mon, Sep 30, 2019 at 04:44:38PM +0000, Ayan Halder wrote:
> Add the DRM_FORMAT_MOD_ARM_16X16_BLOCK_U_INTERLEAVED modifier to
> denote the 16x16 block u-interleaved format used in Arm Utgard and
> Midgard GPUs.
>
> Changes from v1:-
> 1. Reserved the upper four bits (out of the 56 bits assigned to each vendor)
> to denote the category of Arm specific modifiers. Currently, we have two
> categories ie AFBC and MISC.
>
> Signed-off-by: Raymond Smith <raymond.smith@xxxxxxx>
> Signed-off-by: Ayan kumar halder <ayan.halder@xxxxxxx>
> ---
> include/uapi/drm/drm_fourcc.h | 27 ++++++++++++++++++++++++++-
> 1 file changed, 26 insertions(+), 1 deletion(-)
>
> diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
> index 3feeaa3f987a..b1d3de961109 100644
> --- a/include/uapi/drm/drm_fourcc.h
> +++ b/include/uapi/drm/drm_fourcc.h
> @@ -648,7 +648,21 @@ extern "C" {
> * Further information on the use of AFBC modifiers can be found in
> * Documentation/gpu/afbc.rst
> */
> -#define DRM_FORMAT_MOD_ARM_AFBC(__afbc_mode) fourcc_mod_code(ARM, __afbc_mode)
> +
> +/*
> + * The top 4 bits (out of the 56 bits alloted for specifying vendor specific
> + * modifiers) denote the category for modifiers. Currently we have only two
> + * categories of modifiers ie AFBC and MISC. We can have a maximum of sixteen
> + * different categories.
> + */
Yeah, this makes more sense than getting crazy with saving bits. Sorry
Qiang/Daniel for not just going with this in the first instance when
you both suggested it.
> +#define DRM_FORMAT_MOD_ARM_CODE(type, val) \
> + fourcc_mod_code(ARM, ((__u64)type << 52) | ((val) & 0x000fffffffffffffULL))
Not a big deal, but I might prefix type and val with "__" to reduce
the chance of name collisions with code using the macro:
DRM_FORMAT_MOD_ARM_CODE(__type, __val).
> +
> +#define DRM_FORMAT_MOD_ARM_TYPE_AFBC 0x00
> +#define DRM_FORMAT_MOD_ARM_TYPE_MISC 0x01
> +
> +#define DRM_FORMAT_MOD_ARM_AFBC(__afbc_mode) \
> + DRM_FORMAT_MOD_ARM_CODE(DRM_FORMAT_MOD_ARM_TYPE_AFBC, __afbc_mode)
>
> /*
> * AFBC superblock size
> @@ -742,6 +756,17 @@ extern "C" {
> */
> #define AFBC_FORMAT_MOD_BCH (1ULL << 11)
>
> +/*
> + * Arm 16x16 Block U-Interleaved modifier
> + *
> + * This is used by Arm Mali Utgard and Midgard GPUs. It divides the image
> + * into 16x16 pixel blocks. Blocks are stored linearly in order, but pixels
> + * in the block are reordered.
> + */
> +#define DRM_FORMAT_MOD_ARM_16X16_BLOCK_U_INTERLEAVED \
> + DRM_FORMAT_MOD_ARM_CODE(DRM_FORMAT_MOD_ARM_TYPE_MISC, 1ULL)
> +
> +
I think you can drop this newline, there's no extra space between any
of the other definitions.
With this line dropped, and if you fix up the author:
Reviewed-by: Brian Starkey <brian.starkey@xxxxxxx>
Thanks for the respin,
-Brian
> /*
> * Allwinner tiled modifier
> *
> --
> 2.23.0
>
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
