[Intel-gfx] [PATCH 3/3] drm/i915/guc: general tidying up (submission)

[Tvrtko Ursulin]

On 14/09/2016 18:00, Dave Gordon wrote:
> On 14/09/16 16:22, Tvrtko Ursulin wrote:
>>
>> On 12/09/2016 21:19, Dave Gordon wrote:
>>> Renaming to more consistent scheme, and updating comments, mostly
>>> about i915_guc_wq_reserve(), aka i915_guc_wq_check_space().
>>>
>>> Signed-off-by: Dave Gordon <david.s.gordon at intel.com>
>>> ---
>>>   drivers/gpu/drm/i915/i915_guc_submission.c | 63
>>> +++++++++++++++---------------
>>>   drivers/gpu/drm/i915/intel_guc.h           |  2 +-
>>>   drivers/gpu/drm/i915/intel_lrc.c           |  2 +-
>>>   3 files changed, 34 insertions(+), 33 deletions(-)
>>>
>
> [snip]
>
>>>   int i915_guc_submission_init(struct drm_i915_private *dev_priv)
>>>   {
>>> +    const size_t ctxsize = sizeof(struct guc_context_desc);
>>> +    const size_t poolsize = GUC_MAX_GPU_CONTEXTS * ctxsize;
>>> +    const size_t gemsize = round_up(poolsize, PAGE_SIZE);
>>>       struct intel_guc *guc = &dev_priv->guc;
>>>       struct i915_vma *vma;
>>> -    u32 size;
>>>         /* Wipe bitmap & delete client in case of reinitialisation */
>>>       bitmap_clear(guc->doorbell_bitmap, 0, GUC_MAX_DOORBELLS);
>>> @@ -985,15 +987,14 @@ int i915_guc_submission_init(struct
>>> drm_i915_private *dev_priv)
>>>       if (guc->ctx_pool_vma)
>>>           return 0; /* already allocated */
>>>   -    size = PAGE_ALIGN(GUC_MAX_GPU_CONTEXTS*sizeof(struct
>>> guc_context_desc));
>>> -    vma = guc_allocate_vma(guc, size);
>>> +    vma = guc_allocate_vma(guc, gemsize);
>>
>> PAGE_ALIGN lost - lower layers do that for us? I don't have easy access
>> to the tree at the moment to check and I kind of can't remember right 
>> now.
>
> PAGE_ALIGN here is replaced by using round_up(..., PAGE_SIZE) at the 
> point where the constant is defined a few lines above. I think 
> round_up() is clearer, because "align" could equally well mean round 
> down. Anyway "align" (up or down) is something you do to addresses or 
> offsets, not sizes.

I failed to spot that line. :( Should really take a long holiday. :)

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin at intel.com>

Regards,

Tvrtko