[PATCH] drm/i915: Flip guc_id allocation partition

[Michal Wajdeczko]

On 13.01.2022 00:26, Matthew Brost wrote:
> On Thu, Jan 13, 2022 at 12:21:17AM +0100, Michal Wajdeczko wrote:
>> On 11.01.2022 17:30, Matthew Brost wrote:

...

>>> @@ -1863,6 +1861,33 @@ static void guc_submit_request(struct i915_request *rq)
>>>  	spin_unlock_irqrestore(&sched_engine->lock, flags);
>>>  }
>>>  
>>> +static int new_mlrc_guc_id(struct intel_guc *guc, struct intel_context *ce)
>>> +{
>>> +	int ret;
>>> +
>>> +	GEM_BUG_ON(!intel_context_is_parent(ce));
>>> +	GEM_BUG_ON(!guc->submission_state.guc_ids_bitmap);
>>> +
>>> +	ret =  bitmap_find_free_region(guc->submission_state.guc_ids_bitmap,
>>> +				       NUMBER_MULTI_LRC_GUC_ID(guc),
>>> +				       order_base_2(ce->parallel.number_children
>>> +						    + 1));
>>
>> btw, is there any requirement (GuC ABI ?) that allocated ids need
>> to be allocated with power of 2 alignment ? I don't think that we
>> must optimize that hard and in some cases waste extra ids (as we might
>> be limited on some configs)
>>
> 
> No pow2 requirement in GuC ABI, bitmaps only work on pow2 alignment and
> didn't optmize this.
>

there is a slower variant of "find" function:

bitmap_find_next_zero_area - find a contiguous aligned zero area

that does not have this limitation

..


>>> @@ -1989,6 +2008,14 @@ static int pin_guc_id(struct intel_guc *guc, struct intel_context *ce)
>>>  
>>>  	GEM_BUG_ON(atomic_read(&ce->guc_id.ref));
>>>  
>>> +	if (unlikely(intel_context_is_parent(ce) &&
>>> +		     !guc->submission_state.guc_ids_bitmap)) {
>>> +		guc->submission_state.guc_ids_bitmap =
>>> +			bitmap_zalloc(NUMBER_MULTI_LRC_GUC_ID(guc), GFP_KERNEL);
>>> +		if (!guc->submission_state.guc_ids_bitmap)
>>> +			return -ENOMEM;
>>> +	}
>>
>> maybe move this chunk to new_mlrc_guc_id() ?
>> or we can't due to the spin_lock below ?
>> but then how do you protect guc_ids_bitmap pointer itself ?
>>
> 
> Can't use GFP_KERNEL inside a spin lock...
> 

ok, but what if there will be two or more parallel calls to pin_guc_id()
with all being first parent context? each will see NULL guc_ids_bitmap..
or there is another layer of synchronization?

-Michal