Messages in this thread | | | From | Qiang Yu <> | Date | Sun, 23 Jun 2019 11:18:10 +0800 | Subject | Re: [PATCH] drm/fourcc: Add Arm 16x16 block modifier |
| |
On Fri, Jun 21, 2019 at 11:27 PM Daniel Vetter <daniel@ffwll.ch> wrote: > > On Fri, Jun 21, 2019 at 12:21 PM Raymond Smith <Raymond.Smith@arm.com> 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. > > > > Signed-off-by: Raymond Smith <raymond.smith@arm.com> > > --- > > include/uapi/drm/drm_fourcc.h | 10 ++++++++++ > > 1 file changed, 10 insertions(+) > > > > diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h > > index 3feeaa3..8ed7ecf 100644 > > --- a/include/uapi/drm/drm_fourcc.h > > +++ b/include/uapi/drm/drm_fourcc.h > > @@ -743,6 +743,16 @@ 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 \ > > + fourcc_mod_code(ARM, ((1ULL << 55) | 1)) > Thanks for the patch.
> This seems to be an extremely random pick for a new number. What's the > thinking here? Aside from "doesnt match any of the afbc combos" ofc. > If you're already up to having thrown away 55bits, then it's not going > to last long really :-) > > I think a good idea would be to reserve a bunch of the high bits as > some form of index (afbc would get index 0 for backwards compat). And > then the lower bits would be for free use for a given index/mode. And > the first mode is probably an enumeration, where possible modes simple > get enumerated without further flags or anything. > This idea is like my previous patch: https://patchwork.kernel.org/patch/10852619/
lima driver just need a unique modifier to represent this format, so this patch is enough for lima needs.
But I'm also a little worry about the expansion of only reserve the top bit (55) for classification from the ARM point of view. A few more bit (at least 2 for being able to represent 4 class of format) and more clear class/format fields division would be better.
Thanks, Qiang
> > Also ofc needs acks from lima/panfrost people since I assume they'll > be using this, too. > > Thanks, Daniel > > > + > > +/* > > * Allwinner tiled modifier > > * > > * This tiling mode is implemented by the VPU found on all Allwinner platforms, > > -- > > 2.7.4 > > > > > -- > Daniel Vetter > Software Engineer, Intel Corporation > +41 (0) 79 365 57 48 - http://blog.ffwll.ch
| |