Re: [PATCH] drm/i915: use appropriate integer types for flags

From: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
To: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/i915: use appropriate integer types for flags
Date: Fri, 16 Nov 2018 10:43:39 +0000	[thread overview]
Message-ID: <658a9c74-7423-e163-ede8-8085d9354a69@intel.com> (raw)
In-Reply-To: <20181114132241.GN9144@intel.com>

On 14/11/2018 13:22, Ville Syrjälä wrote:
> On Wed, Nov 14, 2018 at 12:08:06PM +0000, Lionel Landwerlin wrote:
>> We've been dealing a number of 32/64 bits flags issues lately :
>>
>>   - 085603287452fc ("drm/i915: Compare user's 64b GTT offset even on 32b")
>>   - c58281056a8b26 ("drm/i915: Mark up GTT sizes as u64")
>>   - 83b466b1dc5f0b ("drm/i915: Mark pin flags as u64")
>>
>> As userspace and in particular Mesa pulls in the uAPI headers and
>> builds up flags using the uAPI defines we should probably make those
>> more explicitly 32/64bits aware.
>>
>> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
>> ---
>>   include/uapi/drm/i915_drm.h | 90 ++++++++++++++++++-------------------
>>   1 file changed, 45 insertions(+), 45 deletions(-)
>>
>> diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
>> index e477ef8c644e..f562c4239bd8 100644
>> --- a/include/uapi/drm/i915_drm.h
>> +++ b/include/uapi/drm/i915_drm.h
>> @@ -895,12 +895,12 @@ struct drm_i915_gem_exec_object2 {
>>   	 */
>>   	__u64 offset;
>>   
>> -#define EXEC_OBJECT_NEEDS_FENCE		 (1<<0)
>> -#define EXEC_OBJECT_NEEDS_GTT		 (1<<1)
>> -#define EXEC_OBJECT_WRITE		 (1<<2)
>> -#define EXEC_OBJECT_SUPPORTS_48B_ADDRESS (1<<3)
>> -#define EXEC_OBJECT_PINNED		 (1<<4)
>> -#define EXEC_OBJECT_PAD_TO_SIZE		 (1<<5)
>> +#define EXEC_OBJECT_NEEDS_FENCE		 (1ULL<<0)
>> +#define EXEC_OBJECT_NEEDS_GTT		 (1ULL<<1)
>> +#define EXEC_OBJECT_WRITE		 (1ULL<<2)
>> +#define EXEC_OBJECT_SUPPORTS_48B_ADDRESS (1ULL<<3)
>> +#define EXEC_OBJECT_PINNED		 (1ULL<<4)
>> +#define EXEC_OBJECT_PAD_TO_SIZE		 (1ULL<<5)
>>   /* The kernel implicitly tracks GPU activity on all GEM objects, and
>>    * synchronises operations with outstanding rendering. This includes
>>    * rendering on other devices if exported via dma-buf. However, sometimes
>> @@ -921,14 +921,14 @@ struct drm_i915_gem_exec_object2 {
>>    * explicit tracking to avoid rendering corruption. See, for example,
>>    * I915_PARAM_HAS_EXEC_FENCE to order execbufs and execute them asynchronously.
>>    */
>> -#define EXEC_OBJECT_ASYNC		(1<<6)
>> +#define EXEC_OBJECT_ASYNC		(1ULL<<6)
>>   /* Request that the contents of this execobject be copied into the error
>>    * state upon a GPU hang involving this batch for post-mortem debugging.
>>    * These buffers are recorded in no particular order as "user" in
>>    * /sys/class/drm/cardN/error. Query I915_PARAM_HAS_EXEC_CAPTURE to see
>>    * if the kernel supports this flag.
>>    */
>> -#define EXEC_OBJECT_CAPTURE		(1<<7)
>> +#define EXEC_OBJECT_CAPTURE		(1ULL<<7)
>>   /* All remaining bits are MBZ and RESERVED FOR FUTURE USE */
>>   #define __EXEC_OBJECT_UNKNOWN_FLAGS -(EXEC_OBJECT_CAPTURE<<1)
>>   	__u64 flags;
>> @@ -946,8 +946,8 @@ struct drm_i915_gem_exec_fence {
>>   	 */
>>   	__u32 handle;
>>   
>> -#define I915_EXEC_FENCE_WAIT            (1<<0)
>> -#define I915_EXEC_FENCE_SIGNAL          (1<<1)
>> +#define I915_EXEC_FENCE_WAIT            (1UL<<0)
>> +#define I915_EXEC_FENCE_SIGNAL          (1UL<<1)
> UL doesn't make much sense to me. It can be 32 or 64 bits depending on
> the architecture.
>
Are you suggesting ULL instead?

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx