On 7/1/2015 6:06 PM, Emil Velikov wrote:
> Hi Michel,
>
> Although I cannot comment on the exact implementation I can give you
> general some tips which you might find useful.
>
Hi Emil,
> On 1 July 2015 at 16:28, Michel Thierry <michel.thierry at intel.com> wrote:
>> Gen8+ supports 48-bit virtual addresses, but some objects must always be
>> allocated inside the 32-bit address range.
>>
>> diff --git a/include/drm/i915_drm.h b/include/drm/i915_drm.h
>> index ded43b1..426b25c 100644
>> --- a/include/drm/i915_drm.h
>> +++ b/include/drm/i915_drm.h
>> @@ -680,7 +680,8 @@ struct drm_i915_gem_exec_object2 {
>> #define EXEC_OBJECT_NEEDS_FENCE (1<<0)
>> #define EXEC_OBJECT_NEEDS_GTT (1<<1)
>> #define EXEC_OBJECT_WRITE (1<<2)
>> -#define __EXEC_OBJECT_UNKNOWN_FLAGS -(EXEC_OBJECT_WRITE<<1)
>> +#define EXEC_OBJECT_SUPPORTS_48B_ADDRESS (1<<3)
>> +#define __EXEC_OBJECT_UNKNOWN_FLAGS -(EXEC_OBJECT_SUPPORTS_48B_ADDRESS<<1)
> Perhaps you already know this but changes like these go in _after_ the
> updated kernel header is part of linux-next or a released kernel
> version.
>
The kernel changes have just been reviewed in drm-intel. I included the
userland patches (there's a mesa patch too) so more people could see the
whole picture.
>> __u64 flags;
>>
>> __u64 rsvd1;
>> diff --git a/intel/intel_bufmgr.c b/intel/intel_bufmgr.c
>> index 14ea9f9..590a855 100644
>> --- a/intel/intel_bufmgr.c
>> +++ b/intel/intel_bufmgr.c
>> @@ -188,6 +188,18 @@ drm_intel_bufmgr_check_aperture_space(drm_intel_bo **
>> bo_array, int count)
>> return bo_array[0]->bufmgr->check_aperture_space(bo_array, count);
>> }
>>
>> +void drm_intel_bo_set_supports_48b_address(drm_intel_bo *bo)
>> +{
>> + if (bo->bufmgr->bo_set_supports_48b_address)
>> + bo->bufmgr->bo_set_supports_48b_address(bo);
>> +}
>> +
>> +void drm_intel_bo_clear_supports_48b_address(drm_intel_bo *bo)
>> +{
>> + if (bo->bufmgr->bo_clear_supports_48b_address)
>> + bo->bufmgr->bo_clear_supports_48b_address(bo);
>> +}
>> +
>> int
>> drm_intel_bo_flink(drm_intel_bo *bo, uint32_t * name)
>> {
>> @@ -202,6 +214,18 @@ drm_intel_bo_emit_reloc(drm_intel_bo *bo, uint32_t
>> offset,
>> drm_intel_bo *target_bo, uint32_t target_offset,
>> uint32_t read_domains, uint32_t write_domain)
>> {
>> + drm_intel_bo_clear_supports_48b_address(target_bo);
>> + return bo->bufmgr->bo_emit_reloc(bo, offset,
>> + target_bo, target_offset,
>> + read_domains, write_domain);
>> +}
>> +
>> +int
>> +drm_intel_bo_emit_reloc_48bit(drm_intel_bo *bo, uint32_t offset,
>> + drm_intel_bo *target_bo, uint32_t target_offset,
>> + uint32_t read_domains, uint32_t write_domain)
>> +{
>> + drm_intel_bo_set_supports_48b_address(target_bo);
>> return bo->bufmgr->bo_emit_reloc(bo, offset,
>> target_bo, target_offset,
>> read_domains, write_domain);
>> diff --git a/intel/intel_bufmgr.h b/intel/intel_bufmgr.h
>> index 285919e..62480cb 100644
>> --- a/intel/intel_bufmgr.h
>> +++ b/intel/intel_bufmgr.h
>> @@ -87,7 +87,8 @@ struct _drm_intel_bo {
>> /**
>> * Last seen card virtual address (offset from the beginning of the
>> * aperture) for the object. This should be used to fill relocation
>> - * entries when calling drm_intel_bo_emit_reloc()
>> + * entries when calling drm_intel_bo_emit_reloc() or
>> + * drm_intel_bo_emit_reloc_48bit()
>> */
>> uint64_t offset64;
>> };
>> @@ -137,6 +138,8 @@ void drm_intel_bo_wait_rendering(drm_intel_bo *bo);
>>
>> void drm_intel_bufmgr_set_debug(drm_intel_bufmgr *bufmgr, int
>> enable_debug);
>> void drm_intel_bufmgr_destroy(drm_intel_bufmgr *bufmgr);
>> +void drm_intel_bo_set_supports_48b_address(drm_intel_bo *bo);
>> +void drm_intel_bo_clear_supports_48b_address(drm_intel_bo *bo);
> Are these two are internal/implementation specific functions ? If so
> please don't include them in this public header and annotate both
> declaration and definition with drm_private.
>
>> int drm_intel_bo_exec(drm_intel_bo *bo, int used,
>> struct drm_clip_rect *cliprects, int num_cliprects,
>> int DR4);
>> int drm_intel_bo_mrb_exec(drm_intel_bo *bo, int used,
>> @@ -147,6 +150,9 @@ int drm_intel_bufmgr_check_aperture_space(drm_intel_bo
>> ** bo_array, int count);
>> int drm_intel_bo_emit_reloc(drm_intel_bo *bo, uint32_t offset,
>> drm_intel_bo *target_bo, uint32_t target_offset,
>> uint32_t read_domains, uint32_t write_domain);
>> +int drm_intel_bo_emit_reloc_48bit(drm_intel_bo *bo, uint32_t offset,
>> + drm_intel_bo *target_bo, uint32_t
>> target_offset,
>> + uint32_t read_domains, uint32_t
>> write_domain);
> Please add new (public) symbols to the test (intel-symbol-check). If
> in doubt make check will give you a lovely message.
>
> Patches that bump the version number are not normally posted to the
> ML, but are committed as part of the release process (see RELEASING
> for more info).
I'll update the intel-symbol-check and follow the release process after
the kernel patch is merged.
Just to confirm, anyone can push to git://anongit.freedesktop.org/mesa/drm ?
Thanks for the tips,
-Michel
>
> Cheers,
> Emil
>
