On Sun, Mar 10, 2019 at 10:08 AM Alyssa Rosenzweig <aly...@rosenzweig.io> wrote:
>
> > +/*
> > + * Arm Device code
> > + *
> > + * Arm has multiple devices which do not share buffer format,
> > + * so add a device field at the MSB of the format field to seperate
> > + * each device's encoding.
> > + */
> > +#define DRM_FORMAT_MOD_ARM_DEVICE_AFBC 0x00
> > +#define DRM_FORMAT_MOD_ARM_DEVICE_GPU 0x01
> > +
> > +#define DRM_FORMAT_MOD_ARM_CODE(device, val) \
> > + fourcc_mod_code(ARM, ((__u64)device << 48) | ((val) &
> > 0x0000ffffffffffffULL))
> > +
> > /*
> > * Arm Framebuffer Compression (AFBC) modifiers
> > *
> > @@ -615,7 +628,8 @@ 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)
> > +#define DRM_FORMAT_MOD_ARM_AFBC(__afbc_mode) \
> > + DRM_FORMAT_MOD_ARM_CODE(DRM_FORMAT_MOD_ARM_DEVICE_AFBC, __afbc_mode)
> >
> > /*
> > * AFBC superblock size
> > @@ -709,6 +723,21 @@ extern "C" {
> > */
> > #define AFBC_FORMAT_MOD_BCH (1ULL << 11)
> >
> > +/*
> > + * Arm GPU modifiers
> > + */
> > +#define DRM_FORMAT_MOD_ARM_GPU(mode) \
> > + DRM_FORMAT_MOD_ARM_CODE(DRM_FORMAT_MOD_ARM_DEVICE_GPU, mode)
> > +
>
> Although Utgard does not support AFBC, both Midgard and Bifrost natively
> support AFBC for both texturing and rendering. Separating "AFBC" from
> "GPU" is incorrect.
>
> It's okay that lima does not support all Arm format codes, but it is
> confusing to create a new "device" to describe that.
We need a field in MSB of the format field to distinguish AFBC formats
and Utgard formats. AFBC is bit based format discretion, so I can't add
some number to that in LSB.
If you think Midgard/Bifrost is compatible with AFBC and don't have it's
own format, and name "device" is improper, I can rename
DRM_FORMAT_MOD_ARM_DEVICE_AFBC to DRM_FORMAT_MOD_ARM_TYPE_AFBC
DRM_FORMAT_MOD_ARM_DEVICE_GPU to DRM_FORMAT_MOD_ARM_TYPE_UTGARD
You may add Midgard/Bifrost spec formats latter if need by another type:
DRM_FORMAT_MOD_ARM_TYPE_MIDGARD
How about it?
>
> These hunks are therefore:
>
> NAKed-by: Alyssa Rosenzweig <aly...@rosenzweig.io>
>
> > +/*
> > + * Arm GPU tiled format
> > + *
> > + * This is used by ARM Mali Utgard/Midgard GPU. It divides buffer into
> > + * 16x16 pixel blocks. Blocks are stored linearly in order, but pixels
> > + * in the block are reordered.
> > + */
> > +#define DRM_FORMAT_MOD_ARM_GPU_TILED DRM_FORMAT_MOD_ARM_GPU(1)
>
> Per above, "DRM_FORMAT_MOD_ARM_GPU(1)" should be "fourcc_mod_code(ARM,
> ...)" for a suitable number. Besides that, this hunk is:
>
> Reviewed-by: Alyssa Rosenzweig <aly...@rosenzweig.io>
>
> ---
>
> Does lima import/export tiled BOs? Panfrost does not, because AFBC is
> our preferred format for shared buffers. As far as I know, Midgard
> cannot render into the tiled format, only linear or AFBC. I'm curious
> what your use case is.
Yes, lima need this when xserver/client share window buffer. Client may
render into a tiled window buffer to be presented to xserver, so need a
modifier to describe the buffer format. And Utgard can render into both
tiled and linear buffers.
Regards,
Qiang
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
