drm/amdgpu: allow shifts >= 32 in AMDGPU_TILING_SET/GET

Submitted by Marek Olšák on March 21, 2017, 7:44 p.m.

Details

Message ID 1490125443-2458-1-git-send-email-maraeo@gmail.com
State New
Headers show
Series "drm/amdgpu: allow shifts >= 32 in AMDGPU_TILING_SET/GET" ( rev: 1 ) in AMD X.Org drivers

Not browsing as part of any series.

Commit Message

Marek Olšák March 21, 2017, 7:44 p.m.
From: Marek Olšák <marek.olsak@amd.com>

also adjust the comments

Signed-off-by: Marek Olšák <marek.olsak@amd.com>
---
 include/uapi/drm/amdgpu_drm.h | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

Patch hide | download patch | download mbox

diff --git a/include/uapi/drm/amdgpu_drm.h b/include/uapi/drm/amdgpu_drm.h
index 7c6cc11..7fb9d10 100644
--- a/include/uapi/drm/amdgpu_drm.h
+++ b/include/uapi/drm/amdgpu_drm.h
@@ -204,46 +204,48 @@  union drm_amdgpu_ctx {
 
 struct drm_amdgpu_gem_userptr {
 	__u64		addr;
 	__u64		size;
 	/* AMDGPU_GEM_USERPTR_* */
 	__u32		flags;
 	/* Resulting GEM handle */
 	__u32		handle;
 };
 
+/* SI-CI-VI: */
 /* same meaning as the GB_TILE_MODE and GL_MACRO_TILE_MODE fields */
 #define AMDGPU_TILING_ARRAY_MODE_SHIFT			0
 #define AMDGPU_TILING_ARRAY_MODE_MASK			0xf
 #define AMDGPU_TILING_PIPE_CONFIG_SHIFT			4
 #define AMDGPU_TILING_PIPE_CONFIG_MASK			0x1f
 #define AMDGPU_TILING_TILE_SPLIT_SHIFT			9
 #define AMDGPU_TILING_TILE_SPLIT_MASK			0x7
 #define AMDGPU_TILING_MICRO_TILE_MODE_SHIFT		12
 #define AMDGPU_TILING_MICRO_TILE_MODE_MASK		0x7
 #define AMDGPU_TILING_BANK_WIDTH_SHIFT			15
 #define AMDGPU_TILING_BANK_WIDTH_MASK			0x3
 #define AMDGPU_TILING_BANK_HEIGHT_SHIFT			17
 #define AMDGPU_TILING_BANK_HEIGHT_MASK			0x3
 #define AMDGPU_TILING_MACRO_TILE_ASPECT_SHIFT		19
 #define AMDGPU_TILING_MACRO_TILE_ASPECT_MASK		0x3
 #define AMDGPU_TILING_NUM_BANKS_SHIFT			21
 #define AMDGPU_TILING_NUM_BANKS_MASK			0x3
-/* Tiling flags for GFX9. */
+
+/* GFX9 and later: */
 #define AMDGPU_TILING_SWIZZLE_MODE_SHIFT		0
 #define AMDGPU_TILING_SWIZZLE_MODE_MASK			0x1f
 
 /* Set/Get helpers for tiling flags. */
 #define AMDGPU_TILING_SET(field, value) \
-	(((value) & AMDGPU_TILING_##field##_MASK) << AMDGPU_TILING_##field##_SHIFT)
+	(((uint64_t)(value) & AMDGPU_TILING_##field##_MASK) << AMDGPU_TILING_##field##_SHIFT)
 #define AMDGPU_TILING_GET(value, field) \
-	(((value) >> AMDGPU_TILING_##field##_SHIFT) & AMDGPU_TILING_##field##_MASK)
+	(((uint64_t)(value) >> AMDGPU_TILING_##field##_SHIFT) & AMDGPU_TILING_##field##_MASK)
 
 #define AMDGPU_GEM_METADATA_OP_SET_METADATA                  1
 #define AMDGPU_GEM_METADATA_OP_GET_METADATA                  2
 
 /** The same structure is shared for input/output */
 struct drm_amdgpu_gem_metadata {
 	/** GEM Object handle */
 	__u32	handle;
 	/** Do we want get or set metadata */
 	__u32	op;

Comments

> -----Original Message-----

> From: amd-gfx [mailto:amd-gfx-bounces@lists.freedesktop.org] On Behalf

> Of Marek Olšák

> Sent: Tuesday, March 21, 2017 3:44 PM

> To: amd-gfx@lists.freedesktop.org

> Subject: [PATCH] drm/amdgpu: allow shifts >= 32 in

> AMDGPU_TILING_SET/GET

> 

> From: Marek Olšák <marek.olsak@amd.com>

> 

> also adjust the comments

> 

> Signed-off-by: Marek Olšák <marek.olsak@amd.com>


Reviewed-by: Alex Deucher <alexander.deucher@amd.com>


> ---

>  include/uapi/drm/amdgpu_drm.h | 8 +++++---

>  1 file changed, 5 insertions(+), 3 deletions(-)

> 

> diff --git a/include/uapi/drm/amdgpu_drm.h

> b/include/uapi/drm/amdgpu_drm.h

> index 7c6cc11..7fb9d10 100644

> --- a/include/uapi/drm/amdgpu_drm.h

> +++ b/include/uapi/drm/amdgpu_drm.h

> @@ -204,46 +204,48 @@ union drm_amdgpu_ctx {

> 

>  struct drm_amdgpu_gem_userptr {

>  	__u64		addr;

>  	__u64		size;

>  	/* AMDGPU_GEM_USERPTR_* */

>  	__u32		flags;

>  	/* Resulting GEM handle */

>  	__u32		handle;

>  };

> 

> +/* SI-CI-VI: */

>  /* same meaning as the GB_TILE_MODE and GL_MACRO_TILE_MODE fields

> */

>  #define AMDGPU_TILING_ARRAY_MODE_SHIFT			0

>  #define AMDGPU_TILING_ARRAY_MODE_MASK			0xf

>  #define AMDGPU_TILING_PIPE_CONFIG_SHIFT			4

>  #define AMDGPU_TILING_PIPE_CONFIG_MASK			0x1f

>  #define AMDGPU_TILING_TILE_SPLIT_SHIFT			9

>  #define AMDGPU_TILING_TILE_SPLIT_MASK			0x7

>  #define AMDGPU_TILING_MICRO_TILE_MODE_SHIFT		12

>  #define AMDGPU_TILING_MICRO_TILE_MODE_MASK		0x7

>  #define AMDGPU_TILING_BANK_WIDTH_SHIFT			15

>  #define AMDGPU_TILING_BANK_WIDTH_MASK			0x3

>  #define AMDGPU_TILING_BANK_HEIGHT_SHIFT			17

>  #define AMDGPU_TILING_BANK_HEIGHT_MASK			0x3

>  #define AMDGPU_TILING_MACRO_TILE_ASPECT_SHIFT		19

>  #define AMDGPU_TILING_MACRO_TILE_ASPECT_MASK		0x3

>  #define AMDGPU_TILING_NUM_BANKS_SHIFT			21

>  #define AMDGPU_TILING_NUM_BANKS_MASK			0x3

> -/* Tiling flags for GFX9. */

> +

> +/* GFX9 and later: */

>  #define AMDGPU_TILING_SWIZZLE_MODE_SHIFT		0

>  #define AMDGPU_TILING_SWIZZLE_MODE_MASK			0x1f

> 

>  /* Set/Get helpers for tiling flags. */

>  #define AMDGPU_TILING_SET(field, value) \

> -	(((value) & AMDGPU_TILING_##field##_MASK) <<

> AMDGPU_TILING_##field##_SHIFT)

> +	(((uint64_t)(value) & AMDGPU_TILING_##field##_MASK) <<

> AMDGPU_TILING_##field##_SHIFT)

>  #define AMDGPU_TILING_GET(value, field) \

> -	(((value) >> AMDGPU_TILING_##field##_SHIFT) &

> AMDGPU_TILING_##field##_MASK)

> +	(((uint64_t)(value) >> AMDGPU_TILING_##field##_SHIFT) &

> AMDGPU_TILING_##field##_MASK)

> 

>  #define AMDGPU_GEM_METADATA_OP_SET_METADATA                  1

>  #define AMDGPU_GEM_METADATA_OP_GET_METADATA                  2

> 

>  /** The same structure is shared for input/output */

>  struct drm_amdgpu_gem_metadata {

>  	/** GEM Object handle */

>  	__u32	handle;

>  	/** Do we want get or set metadata */

>  	__u32	op;

> --

> 2.7.4

> 

> _______________________________________________

> amd-gfx mailing list

> amd-gfx@lists.freedesktop.org

> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
On Tue, Mar 21, 2017 at 9:44 PM, Marek Olšák <maraeo@gmail.com> wrote:
> From: Marek Olšák <marek.olsak@amd.com>
>
> also adjust the comments
>
> Signed-off-by: Marek Olšák <marek.olsak@amd.com>
> ---
>  include/uapi/drm/amdgpu_drm.h | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/include/uapi/drm/amdgpu_drm.h b/include/uapi/drm/amdgpu_drm.h
> index 7c6cc11..7fb9d10 100644
> --- a/include/uapi/drm/amdgpu_drm.h
> +++ b/include/uapi/drm/amdgpu_drm.h
> @@ -204,46 +204,48 @@ union drm_amdgpu_ctx {
>
>  struct drm_amdgpu_gem_userptr {
>         __u64           addr;
>         __u64           size;
>         /* AMDGPU_GEM_USERPTR_* */
>         __u32           flags;
>         /* Resulting GEM handle */
>         __u32           handle;
>  };
>
> +/* SI-CI-VI: */
>  /* same meaning as the GB_TILE_MODE and GL_MACRO_TILE_MODE fields */
>  #define AMDGPU_TILING_ARRAY_MODE_SHIFT                 0
>  #define AMDGPU_TILING_ARRAY_MODE_MASK                  0xf
>  #define AMDGPU_TILING_PIPE_CONFIG_SHIFT                        4
>  #define AMDGPU_TILING_PIPE_CONFIG_MASK                 0x1f
>  #define AMDGPU_TILING_TILE_SPLIT_SHIFT                 9
>  #define AMDGPU_TILING_TILE_SPLIT_MASK                  0x7
>  #define AMDGPU_TILING_MICRO_TILE_MODE_SHIFT            12
>  #define AMDGPU_TILING_MICRO_TILE_MODE_MASK             0x7
>  #define AMDGPU_TILING_BANK_WIDTH_SHIFT                 15
>  #define AMDGPU_TILING_BANK_WIDTH_MASK                  0x3
>  #define AMDGPU_TILING_BANK_HEIGHT_SHIFT                        17
>  #define AMDGPU_TILING_BANK_HEIGHT_MASK                 0x3
>  #define AMDGPU_TILING_MACRO_TILE_ASPECT_SHIFT          19
>  #define AMDGPU_TILING_MACRO_TILE_ASPECT_MASK           0x3
>  #define AMDGPU_TILING_NUM_BANKS_SHIFT                  21
>  #define AMDGPU_TILING_NUM_BANKS_MASK                   0x3
> -/* Tiling flags for GFX9. */
> +
> +/* GFX9 and later: */
>  #define AMDGPU_TILING_SWIZZLE_MODE_SHIFT               0
>  #define AMDGPU_TILING_SWIZZLE_MODE_MASK                        0x1f
>
>  /* Set/Get helpers for tiling flags. */
>  #define AMDGPU_TILING_SET(field, value) \
> -       (((value) & AMDGPU_TILING_##field##_MASK) << AMDGPU_TILING_##field##_SHIFT)
> +       (((uint64_t)(value) & AMDGPU_TILING_##field##_MASK) << AMDGPU_TILING_##field##_SHIFT)
>  #define AMDGPU_TILING_GET(value, field) \
> -       (((value) >> AMDGPU_TILING_##field##_SHIFT) & AMDGPU_TILING_##field##_MASK)
> +       (((uint64_t)(value) >> AMDGPU_TILING_##field##_SHIFT) & AMDGPU_TILING_##field##_MASK)

Shouldn't it be __u64 instead of uint64_t? The kernel header doesn't
include stdint.h or use any uint* types.

Gražvydas
On Wed, Mar 22, 2017 at 11:43 AM, Grazvydas Ignotas <notasas@gmail.com> wrote:
> On Tue, Mar 21, 2017 at 9:44 PM, Marek Olšák <maraeo@gmail.com> wrote:
>> From: Marek Olšák <marek.olsak@amd.com>
>>
>> also adjust the comments
>>
>> Signed-off-by: Marek Olšák <marek.olsak@amd.com>
>> ---
>>  include/uapi/drm/amdgpu_drm.h | 8 +++++---
>>  1 file changed, 5 insertions(+), 3 deletions(-)
>>
>> diff --git a/include/uapi/drm/amdgpu_drm.h b/include/uapi/drm/amdgpu_drm.h
>> index 7c6cc11..7fb9d10 100644
>> --- a/include/uapi/drm/amdgpu_drm.h
>> +++ b/include/uapi/drm/amdgpu_drm.h
>> @@ -204,46 +204,48 @@ union drm_amdgpu_ctx {
>>
>>  struct drm_amdgpu_gem_userptr {
>>         __u64           addr;
>>         __u64           size;
>>         /* AMDGPU_GEM_USERPTR_* */
>>         __u32           flags;
>>         /* Resulting GEM handle */
>>         __u32           handle;
>>  };
>>
>> +/* SI-CI-VI: */
>>  /* same meaning as the GB_TILE_MODE and GL_MACRO_TILE_MODE fields */
>>  #define AMDGPU_TILING_ARRAY_MODE_SHIFT                 0
>>  #define AMDGPU_TILING_ARRAY_MODE_MASK                  0xf
>>  #define AMDGPU_TILING_PIPE_CONFIG_SHIFT                        4
>>  #define AMDGPU_TILING_PIPE_CONFIG_MASK                 0x1f
>>  #define AMDGPU_TILING_TILE_SPLIT_SHIFT                 9
>>  #define AMDGPU_TILING_TILE_SPLIT_MASK                  0x7
>>  #define AMDGPU_TILING_MICRO_TILE_MODE_SHIFT            12
>>  #define AMDGPU_TILING_MICRO_TILE_MODE_MASK             0x7
>>  #define AMDGPU_TILING_BANK_WIDTH_SHIFT                 15
>>  #define AMDGPU_TILING_BANK_WIDTH_MASK                  0x3
>>  #define AMDGPU_TILING_BANK_HEIGHT_SHIFT                        17
>>  #define AMDGPU_TILING_BANK_HEIGHT_MASK                 0x3
>>  #define AMDGPU_TILING_MACRO_TILE_ASPECT_SHIFT          19
>>  #define AMDGPU_TILING_MACRO_TILE_ASPECT_MASK           0x3
>>  #define AMDGPU_TILING_NUM_BANKS_SHIFT                  21
>>  #define AMDGPU_TILING_NUM_BANKS_MASK                   0x3
>> -/* Tiling flags for GFX9. */
>> +
>> +/* GFX9 and later: */
>>  #define AMDGPU_TILING_SWIZZLE_MODE_SHIFT               0
>>  #define AMDGPU_TILING_SWIZZLE_MODE_MASK                        0x1f
>>
>>  /* Set/Get helpers for tiling flags. */
>>  #define AMDGPU_TILING_SET(field, value) \
>> -       (((value) & AMDGPU_TILING_##field##_MASK) << AMDGPU_TILING_##field##_SHIFT)
>> +       (((uint64_t)(value) & AMDGPU_TILING_##field##_MASK) << AMDGPU_TILING_##field##_SHIFT)
>>  #define AMDGPU_TILING_GET(value, field) \
>> -       (((value) >> AMDGPU_TILING_##field##_SHIFT) & AMDGPU_TILING_##field##_MASK)
>> +       (((uint64_t)(value) >> AMDGPU_TILING_##field##_SHIFT) & AMDGPU_TILING_##field##_MASK)
>
> Shouldn't it be __u64 instead of uint64_t? The kernel header doesn't
> include stdint.h or use any uint* types.

Yeah, I'll fix that before pushing.

Marek