[PATCH v2] drm/fourcc: add ARM GPU tile format

[PATCH v2] drm/fourcc: add ARM GPU tile format

[Qiang Yu]

v2: seperate AFBC and GPU encoding

Cc: Brian Starkey <Brian.Starkey@arm.com>
Cc: Rob Herring <robh@kernel.org>
Cc: Alyssa Rosenzweig <alyssa@rosenzweig.io>
Signed-off-by: Qiang Yu <yuq825@gmail.com>
---
 include/uapi/drm/drm_fourcc.h | 31 ++++++++++++++++++++++++++++++-
 1 file changed, 30 insertions(+), 1 deletion(-)

diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
index 9fa7cf7bb274..7af5197e7ebc 100644
--- a/include/uapi/drm/drm_fourcc.h
+++ b/include/uapi/drm/drm_fourcc.h
@@ -601,6 +601,19 @@ extern "C" {
  */
 #define DRM_FORMAT_MOD_BROADCOM_UIF fourcc_mod_code(BROADCOM, 6)
 
+/*
+ * 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)
+
+/*
+ * 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)
+
 /*
  * Allwinner tiled modifier
  *
-- 
2.17.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2] drm/fourcc: add ARM GPU tile format

[Alyssa Rosenzweig]

> +/*
> + * 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.

These hunks are therefore:

NAKed-by: Alyssa Rosenzweig <alyssa@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 <alyssa@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.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2] drm/fourcc: add ARM GPU tile format

[Qiang Yu]

On Sun, Mar 10, 2019 at 10:08 AM Alyssa Rosenzweig <alyssa@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 <alyssa@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 <alyssa@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

Re: [PATCH v2] drm/fourcc: add ARM GPU tile format

[Alyssa Rosenzweig]

> 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

That name is misleading, too. Midgard/Bifrost is compatible with *both*
AFBC *and* tiled formats.

> 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.

I see. I didn't realize Utgard could render into tiled formats; that's
nice :)
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2] drm/fourcc: add ARM GPU tile format

[Qiang Yu]

On Sun, Mar 10, 2019 at 10:53 AM Alyssa Rosenzweig <alyssa@rosenzweig.io> wrote:
>
> > 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
>
> That name is misleading, too. Midgard/Bifrost is compatible with *both*
> AFBC *and* tiled formats.

Then DRM_FORMAT_MOD_ARM_TYPE_GPU? I really don't know what's the
official name for this kind of format, so have to name it with the
device. Any better
suggestion?

>
> > 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.
>
> I see. I didn't realize Utgard could render into tiled formats; that's
> nice :)

Is tiled buffer rendering not reverse engineered yet or confirmed to be
not supported by Midgard/Bifrost?

Regards,
Qiang
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2] drm/fourcc: add ARM GPU tile format

[Alyssa Rosenzweig]

> Then DRM_FORMAT_MOD_ARM_TYPE_GPU? I really don't know what's the
> official name for this kind of format, so have to name it with the
> device. Any better
> suggestion?

Maybe DRM_FORMAT_MOD_ARM_TYPE_TILED? I wonder if the Mali-DP folks have
a suggestion.

> Is tiled buffer rendering not reverse engineered yet or confirmed to be
> not supported by Midgard/Bifrost?

I don't know. There's no reason to render into a tiled buffer when AFBC
is available. AFBC is tiled *and* compressed.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2] drm/fourcc: add ARM GPU tile format

[Ayan Halder]

On Sat, Mar 09, 2019 at 10:09:02PM +0800, Qiang Yu wrote:

Hi Qiang,

Apologies for jumping to the very initial patch. I wanted to have some
understanding of the newly proposed modifiers since they seem to me a
duplicate of the existing AFBC modifiers.

> v2: seperate AFBC and GPU encoding
> 
> Cc: Brian Starkey <Brian.Starkey@arm.com>
> Cc: Rob Herring <robh@kernel.org>
> Cc: Alyssa Rosenzweig <alyssa@rosenzweig.io>
> Signed-off-by: Qiang Yu <yuq825@gmail.com>
> ---
>  include/uapi/drm/drm_fourcc.h | 31 ++++++++++++++++++++++++++++++-
>  1 file changed, 30 insertions(+), 1 deletion(-)
> 
> diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
> index 9fa7cf7bb274..7af5197e7ebc 100644
> --- a/include/uapi/drm/drm_fourcc.h
> +++ b/include/uapi/drm/drm_fourcc.h
> @@ -601,6 +601,19 @@ extern "C" {
>   */
>  #define DRM_FORMAT_MOD_BROADCOM_UIF fourcc_mod_code(BROADCOM, 6)
>  
> +/*
> + * 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
Why can't you reuse the existing DRM_FORMAT_MOD_VENDOR_ARM ?

> +#define DRM_FORMAT_MOD_ARM_DEVICE_GPU  0x01
What is the purpose of this? AFBC modifiers are supposed to be used
across gpus, dpus which support AFBC buffers rendering.

> +
> +#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)
>  
Similar question? Why can't you reuse the existing one?

>  /*
>   * 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)
> +
> +/*
> + * 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)
> +
You might want to re-use the exisiting modifier AFBC_FORMAT_MOD_BLOCK_SIZE_16x16.

I would suggest you to have a look at the exisiting AFBC modifiers
(denoted by AFBC_FORMAT_MOD_XXX ) and let us know if there is something
you cannot reuse.

>  /*
>   * Allwinner tiled modifier
>   *
> -- 
> 2.17.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2] drm/fourcc: add ARM GPU tile format

[Alyssa Rosenzweig]

[-- Attachment #1.1: Type: text/plain, Size: 1383 bytes --]

> You might want to re-use the exisiting modifier
> AFBC_FORMAT_MOD_BLOCK_SIZE_16x16.
> 
> I would suggest you to have a look at the exisiting AFBC modifiers
> (denoted by AFBC_FORMAT_MOD_XXX ) and let us know if there is
> something you cannot reuse.

So, the "tiled" format in question (that Qiang needs to import/export
BOs in) is *uncompressed* but tiled with an Arm-internal format (for the
GPUs).  Here's a software implementation for encoding this format:
https://cgit.freedesktop.org/mesa/mesa/tree/src/gallium/drivers/panfrost/pan_swizzle.c

For Midgard/Bifrost, we use this tiling internally for uploading bitmap
textures, but we only render to AFBC (or linear). So for Panfrost, we'll
always be importing/exporting AFBC buffers, never uncompressed tiled.
But Utgard does not seem to support AFBC (?), so Qiang needs the
uncompressed tiled for the same purpose Panfrost uses AFBC.

Is it possible that this is the same tiling used internally by
AFBC_FORMAT_MOD_BLOCK_SIZE_16x16, only without any compression? AFBC is
blackbox for us, so this isn't something we can figure out ourselves,
but that influences whether it's appropriate to reuse the modifier. If
this is the same tiling scheme, perhaps that's the answer. If it's not
(I don't know how AFBC tiling works), we probably do need a separate
modifier to avoid confusion.

Thanks,

Alyssa

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2] drm/fourcc: add ARM GPU tile format

[Qiang Yu]

Alyssa did a very good description of our needs. Since I can't open the
cgit mesa link and give you a clear view of this format:
https://gitlab.freedesktop.org/mesa/mesa/blob/master/src/gallium/drivers/panfrost/pan_swizzle.c
https://gitlab.freedesktop.org/lima/mesa/blob/lima-18.3/src/gallium/drivers/lima/lima_tiling.c#L29

Thanks,
Qiang

On Tue, Mar 12, 2019 at 12:39 AM Alyssa Rosenzweig <alyssa@rosenzweig.io> wrote:
>
> > You might want to re-use the exisiting modifier
> > AFBC_FORMAT_MOD_BLOCK_SIZE_16x16.
> >
> > I would suggest you to have a look at the exisiting AFBC modifiers
> > (denoted by AFBC_FORMAT_MOD_XXX ) and let us know if there is
> > something you cannot reuse.
>
> So, the "tiled" format in question (that Qiang needs to import/export
> BOs in) is *uncompressed* but tiled with an Arm-internal format (for the
> GPUs).  Here's a software implementation for encoding this format:
> https://cgit.freedesktop.org/mesa/mesa/tree/src/gallium/drivers/panfrost/pan_swizzle.c
>
> For Midgard/Bifrost, we use this tiling internally for uploading bitmap
> textures, but we only render to AFBC (or linear). So for Panfrost, we'll
> always be importing/exporting AFBC buffers, never uncompressed tiled.
> But Utgard does not seem to support AFBC (?), so Qiang needs the
> uncompressed tiled for the same purpose Panfrost uses AFBC.
>
> Is it possible that this is the same tiling used internally by
> AFBC_FORMAT_MOD_BLOCK_SIZE_16x16, only without any compression? AFBC is
> blackbox for us, so this isn't something we can figure out ourselves,
> but that influences whether it's appropriate to reuse the modifier. If
> this is the same tiling scheme, perhaps that's the answer. If it's not
> (I don't know how AFBC tiling works), we probably do need a separate
> modifier to avoid confusion.
>
> Thanks,
>
> Alyssa
> -----BEGIN PGP SIGNATURE-----
>
> iQIzBAABCgAdFiEEQ17gm7CvANAdqvY4/v5QWgr1WA0FAlyGjz8ACgkQ/v5QWgr1
> WA1JHA/+MKO3rf6htnd48EVnXtEhsSqoDatDVb2c13k5/oxutrAQqR29K1Tl/Wkp
> 6HebJD4sBafutBroyGenIz5cLnY3AG3zy1zxzOgdvkXpNuVcXgtpL4HXjEaInueD
> ZtssXso4gjRZyD6kuW5fe97TvvRTPknEsMXpWwBgcgP+FxUrrwVqEPo6il3rhMvN
> E4NBHkqoX8GtLWB9p/YUcUWMHJ2YhEIo04Vi3tWw7t2TniHrJw7wZDVPXoKhoX9/
> wsu0iZ4XEoL3FPtK1v/S6gWkDq0xDTzHeZOBJKYC94RiH93S+Q7maLzH5fTB9lXt
> DsF2zJkEj6tHZJCi0f0JzEcuXiAzZKKVLbKD1KoL6ORKZePRraJRzoTHFU1XknyR
> e9Uv3JuO2RwZxicfdgdArgzR9wrM2PL9kPzGsLtaYISQ6hs/vNbu2lgNPUzaX0Az
> xvMu/ZW9cbO+XKeilGoOYR8ON0Zyw9zmUSrnd5aRwwk07rvQElz7csFBGeH7QTg5
> p5GTF5QcipZ6JpB3VSYxbJPJx2rEmg8/lTGUuWCB553sdbStPk/rKFAKszgqp6Cw
> 8AUktXmPv1Unwsnk46VF8WXJFtIldRjm8ErFRzbSDqyEO1kxPR+wH+6XBrS1iiYS
> 3Y7KoRUHLsQk6pk7q4RgVRYe2MCJe3jVeQNL1gZ6Qha3Dzbxymc=
> =Prus
> -----END PGP SIGNATURE-----
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2] drm/fourcc: add ARM GPU tile format

[Ayan Halder]

On Mon, Mar 11, 2019 at 09:39:28AM -0700, Alyssa Rosenzweig wrote:
> > You might want to re-use the exisiting modifier
> > AFBC_FORMAT_MOD_BLOCK_SIZE_16x16.
> > 
> > I would suggest you to have a look at the exisiting AFBC modifiers
> > (denoted by AFBC_FORMAT_MOD_XXX ) and let us know if there is
> > something you cannot reuse.
> 
> So, the "tiled" format in question (that Qiang needs to import/export
> BOs in) is *uncompressed* but tiled with an Arm-internal format (for the
> GPUs).  Here's a software implementation for encoding this format:
> https://cgit.freedesktop.org/mesa/mesa/tree/src/gallium/drivers/panfrost/pan_swizzle.c
>
ok I understood now. In this case, please ignore my suggestion.
 
> For Midgard/Bifrost, we use this tiling internally for uploading bitmap
> textures, but we only render to AFBC (or linear). So for Panfrost, we'll
> always be importing/exporting AFBC buffers, never uncompressed tiled.

I am not a gpu guy so can't comment at the moment.

> But Utgard does not seem to support AFBC (?), so Qiang needs the
> uncompressed tiled for the same purpose Panfrost uses AFBC.
> 
> Is it possible that this is the same tiling used internally by
> AFBC_FORMAT_MOD_BLOCK_SIZE_16x16, only without any compression? AFBC is
> blackbox for us, so this isn't something we can figure out ourselves,
> but that influences whether it's appropriate to reuse the modifier. If
> this is the same tiling scheme, perhaps that's the answer. If it's not
> (I don't know how AFBC tiling works), we probably do need a separate
> modifier to avoid confusion.
>
AFBC_FORMAT_MOD_BLOCK_SIZE_16x16 denotes the superblock size to be
used only in case of AFBC buffers.
For non compressed tiled format, none of the AFBC_FORMAT_MOD_XXX
should be used.

However, I would like you to review my patch series
(https://patchwork.freedesktop.org/series/53395/) to
see if we have the same understanding.
 
> Thanks,
> 
> Alyssa



> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2] drm/fourcc: add ARM GPU tile format

[Qiang Yu]

On Tue, Mar 12, 2019 at 11:41 PM Ayan Halder <Ayan.Halder@arm.com> wrote:
>
> On Mon, Mar 11, 2019 at 09:39:28AM -0700, Alyssa Rosenzweig wrote:
> > > You might want to re-use the exisiting modifier
> > > AFBC_FORMAT_MOD_BLOCK_SIZE_16x16.
> > >
> > > I would suggest you to have a look at the exisiting AFBC modifiers
> > > (denoted by AFBC_FORMAT_MOD_XXX ) and let us know if there is
> > > something you cannot reuse.
> >
> > So, the "tiled" format in question (that Qiang needs to import/export
> > BOs in) is *uncompressed* but tiled with an Arm-internal format (for the
> > GPUs).  Here's a software implementation for encoding this format:
> > https://cgit.freedesktop.org/mesa/mesa/tree/src/gallium/drivers/panfrost/pan_swizzle.c
> >
> ok I understood now. In this case, please ignore my suggestion.
>
> > For Midgard/Bifrost, we use this tiling internally for uploading bitmap
> > textures, but we only render to AFBC (or linear). So for Panfrost, we'll
> > always be importing/exporting AFBC buffers, never uncompressed tiled.
>
> I am not a gpu guy so can't comment at the moment.
>
> > But Utgard does not seem to support AFBC (?), so Qiang needs the
> > uncompressed tiled for the same purpose Panfrost uses AFBC.
> >
> > Is it possible that this is the same tiling used internally by
> > AFBC_FORMAT_MOD_BLOCK_SIZE_16x16, only without any compression? AFBC is
> > blackbox for us, so this isn't something we can figure out ourselves,
> > but that influences whether it's appropriate to reuse the modifier. If
> > this is the same tiling scheme, perhaps that's the answer. If it's not
> > (I don't know how AFBC tiling works), we probably do need a separate
> > modifier to avoid confusion.
> >
> AFBC_FORMAT_MOD_BLOCK_SIZE_16x16 denotes the superblock size to be
> used only in case of AFBC buffers.
> For non compressed tiled format, none of the AFBC_FORMAT_MOD_XXX
> should be used.

I still haven't get a clear answer. The question is:

1. Does AFBC_FORMAT_MOD_BLOCK_SIZE_16x16 without any
AFBC_FORMAT_MOD_XXX uncompressed format just reorder the pixels
in one 16x16 block same way as GPU "tiled" format? Or just no reorder (linear)?

2. Is there any unreleased AFBC_FORMAT_MOD_XXX bit for this GPU
"tiled" format?

3. If the answer to 1&2 is no, seems Mali GPU formats and Mali Display
formats are in different systems, do Mali Display guys agree to divide
these two different format systems by the MSB bits of format field in this
patch?

>
> However, I would like you to review my patch series
> (https://patchwork.freedesktop.org/series/53395/) to
> see if we have the same understanding.
>
To be honest, I find no answer in this patch series for the above questions.

Thanks,
Qiang

>
>
>
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2] drm/fourcc: add ARM GPU tile format

[Alyssa Rosenzweig]

> 1. Does AFBC_FORMAT_MOD_BLOCK_SIZE_16x16 without any
> AFBC_FORMAT_MOD_XXX uncompressed format just reorder the pixels
> in one 16x16 block same way as GPU "tiled" format? Or just no reorder (linear)?
> 
> 2. Is there any unreleased AFBC_FORMAT_MOD_XXX bit for this GPU
> "tiled" format?

I believe the answer to both is no, based on the parent email. The
uncompressed tiling scheme is older than AFBC, I think. Using the AFBC
modifiers for it doesn't make sense.

> 3. If the answer to 1&2 is no, seems Mali GPU formats and Mali Display
> formats are in different systems, do Mali Display guys agree to divide
> these two different format systems by the MSB bits of format field in this
> patch?

It's closer to "Mali GPU only formats" and "Everything else in the Mali
ecosystem"... Utgard-style tiling was GPU-specific. AFBC is Arm trying
to standardize across all their systems. Midgard+ can both render and
sample AFBC; the Mali video cores produce AFBC; the Mali display
processors consume AFBC; the Rockchip RK3399 display processor consumes
AFBC (I'm working on a patch to get that mainlined).

So upon further thought, I think this division makes sense.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2] drm/fourcc: add ARM GPU tile format

[Alyssa Rosenzweig]

> AFBC_FORMAT_MOD_BLOCK_SIZE_16x16 denotes the superblock size to be
> used only in case of AFBC buffers.
> For non compressed tiled format, none of the AFBC_FORMAT_MOD_XXX
> should be used.

Alright, understood. Thank you for the clarification :)
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel