[libdrm,2/7] amdgpu: update amdgpu_drm.h for Vega10

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

Details

Message ID 1490126185-4482-3-git-send-email-maraeo@gmail.com
State New
Headers show
Series "Vega10 bits for libdrm and more" ( rev: 1 ) in AMD X.Org drivers

Not browsing as part of any series.

Commit Message

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

---
 include/drm/amdgpu_drm.h | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

Patch hide | download patch | download mbox

diff --git a/include/drm/amdgpu_drm.h b/include/drm/amdgpu_drm.h
index 5797283..d702a95 100644
--- a/include/drm/amdgpu_drm.h
+++ b/include/drm/amdgpu_drm.h
@@ -202,42 +202,47 @@  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
 
+/* GFX9 and later: */
+#define AMDGPU_TILING_SWIZZLE_MODE_SHIFT		0
+#define AMDGPU_TILING_SWIZZLE_MODE_MASK			0x1f
+
 #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;
@@ -748,16 +753,17 @@  struct drm_amdgpu_info_vce_clock_table {
 
 /*
  * Supported GPU families
  */
 #define AMDGPU_FAMILY_UNKNOWN			0
 #define AMDGPU_FAMILY_SI			110 /* Hainan, Oland, Verde, Pitcairn, Tahiti */
 #define AMDGPU_FAMILY_CI			120 /* Bonaire, Hawaii */
 #define AMDGPU_FAMILY_KV			125 /* Kaveri, Kabini, Mullins */
 #define AMDGPU_FAMILY_VI			130 /* Iceland, Tonga */
 #define AMDGPU_FAMILY_CZ			135 /* Carrizo, Stoney */
+#define AMDGPU_FAMILY_AI			141 /* Vega10 */
 
 #if defined(__cplusplus)
 }
 #endif
 
 #endif

Comments

In the past, I was told off for patches that update this file without 
following the procedure described in include/drm/README. Tbh, that 
procedure causes some annoyances.

Anyway, it's definitely useful to have the patch out on the mailing list 
in any case.

Cheers,
Nicolai

On 21.03.2017 20:56, Marek Olšák wrote:
> From: Marek Olšák <marek.olsak@amd.com>
>
> ---
>  include/drm/amdgpu_drm.h | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/include/drm/amdgpu_drm.h b/include/drm/amdgpu_drm.h
> index 5797283..d702a95 100644
> --- a/include/drm/amdgpu_drm.h
> +++ b/include/drm/amdgpu_drm.h
> @@ -202,42 +202,47 @@ 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
>
> +/* GFX9 and later: */
> +#define AMDGPU_TILING_SWIZZLE_MODE_SHIFT		0
> +#define AMDGPU_TILING_SWIZZLE_MODE_MASK			0x1f
> +
>  #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;
> @@ -748,16 +753,17 @@ struct drm_amdgpu_info_vce_clock_table {
>
>  /*
>   * Supported GPU families
>   */
>  #define AMDGPU_FAMILY_UNKNOWN			0
>  #define AMDGPU_FAMILY_SI			110 /* Hainan, Oland, Verde, Pitcairn, Tahiti */
>  #define AMDGPU_FAMILY_CI			120 /* Bonaire, Hawaii */
>  #define AMDGPU_FAMILY_KV			125 /* Kaveri, Kabini, Mullins */
>  #define AMDGPU_FAMILY_VI			130 /* Iceland, Tonga */
>  #define AMDGPU_FAMILY_CZ			135 /* Carrizo, Stoney */
> +#define AMDGPU_FAMILY_AI			141 /* Vega10 */
>
>  #if defined(__cplusplus)
>  }
>  #endif
>
>  #endif
>
On Tue, Mar 21, 2017 at 10:27 PM, Nicolai Hähnle <nhaehnle@gmail.com> wrote:
> In the past, I was told off for patches that update this file without
> following the procedure described in include/drm/README. Tbh, that procedure
> causes some annoyances.
>
> Anyway, it's definitely useful to have the patch out on the mailing list in
> any case.

Yeah, I know the correct process and I plan to ignore it this time if
I don't get too much backlash, because the alternative
(#ifdef/#define/#endif) is probably even worse.

Marek
On 22/03/17 06:46 AM, Marek Olšák wrote:
> On Tue, Mar 21, 2017 at 10:27 PM, Nicolai Hähnle <nhaehnle@gmail.com> wrote:
>> In the past, I was told off for patches that update this file without
>> following the procedure described in include/drm/README. Tbh, that procedure
>> causes some annoyances.
>>
>> Anyway, it's definitely useful to have the patch out on the mailing list in
>> any case.
> 
> Yeah, I know the correct process and I plan to ignore it this time if
> I don't get too much backlash, because the alternative
> (#ifdef/#define/#endif) is probably even worse.

FWIW, only AMDGPU_TILING_SET/GET need #undef,
AMDGPU_TILING_SWIZZLE_MODE_SHIFT/MASK and AMDGPU_FAMILY_AI can just be
#defined directly, that way the preprocessor will warn if the
definitions in libdrm and Mesa end up being inconsistent for some reason.


The alternative is rushing out a libdrm release and making Mesa require
that, right? That doesn't seem obviously better than a handful of
temporary redundant defines in Mesa, hardly justification for bypassing
the normal process.
On Mar 22, 2017 2:44 AM, "Michel Dänzer" <michel@daenzer.net> wrote:

On 22/03/17 06:46 AM, Marek Olšák wrote:
> On Tue, Mar 21, 2017 at 10:27 PM, Nicolai Hähnle <nhaehnle@gmail.com>
wrote:
>> In the past, I was told off for patches that update this file without
>> following the procedure described in include/drm/README. Tbh, that
procedure
>> causes some annoyances.
>>
>> Anyway, it's definitely useful to have the patch out on the mailing list
in
>> any case.
>
> Yeah, I know the correct process and I plan to ignore it this time if
> I don't get too much backlash, because the alternative
> (#ifdef/#define/#endif) is probably even worse.

FWIW, only AMDGPU_TILING_SET/GET need #undef,
AMDGPU_TILING_SWIZZLE_MODE_SHIFT/MASK and AMDGPU_FAMILY_AI can just be
#defined directly, that way the preprocessor will warn if the
definitions in libdrm and Mesa end up being inconsistent for some reason.


The alternative is rushing out a libdrm release and making Mesa require
that, right? That doesn't seem obviously better than a handful of
temporary redundant defines in Mesa, hardly justification for bypassing
the normal process.


I need a libdrm release because of the 3rd patch. I can't allow Mesa to run
without that.

Marek



--
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer
On 22/03/17 07:13 PM, Marek Olšák wrote:
> On Mar 22, 2017 2:44 AM, "Michel Dänzer" <michel@daenzer.net 
> <mailto:michel@daenzer.net>> wrote:
>> On 22/03/17 06:46 AM, Marek Olšák wrote:
>>> On Tue, Mar 21, 2017 at 10:27 PM, Nicolai Hähnle 
>>> <nhaehnle@gmail.com <mailto:nhaehnle@gmail.com>> wrote:
>>>> In the past, I was told off for patches that update this file
>>>> without following the procedure described in
>>>> include/drm/README. Tbh, that procedure causes some
>>>> annoyances.
>>>> 
>>>> Anyway, it's definitely useful to have the patch out on the 
>>>> mailing list in any case.
>>> 
>>> Yeah, I know the correct process and I plan to ignore it this
>>> time if I don't get too much backlash, because the alternative 
>>> (#ifdef/#define/#endif) is probably even worse.
>> 
>> FWIW, only AMDGPU_TILING_SET/GET need #undef, 
>> AMDGPU_TILING_SWIZZLE_MODE_SHIFT/MASK and AMDGPU_FAMILY_AI can just
>> be #defined directly, that way the preprocessor will warn if the 
>> definitions in libdrm and Mesa end up being inconsistent for some 
>> reason.
>> 
>> 
>> The alternative is rushing out a libdrm release and making Mesa
>> require that, right? That doesn't seem obviously better than a
>> handful of temporary redundant defines in Mesa, hardly
>> justification for bypassing the normal process.
> 
> I need a libdrm release because of the 3rd patch. I can't allow Mesa
> to run without that.

Gotcha, thanks for the clarification.