Re: [PATCH v6 01/25] drm/xe: Add member initialized_domains to xe_force_wake()

["Ghimiray, Himal Prasad" <himal.prasad.ghimiray@intel.com>]

On 01-10-2024 01:21, Michal Wajdeczko wrote:
> 
> 
> On 30.09.2024 07:31, Himal Prasad Ghimiray wrote:
>> This field serves as a bitmask representing all initialized forcewake
>> domains on the GT.
>>
>> While at it, fix the datatype for awake_domains since it can have
>> value different from enum xe_force_wake_domains.
> 
> up to maintainers, but for me this is unrelated change

Ok. Will move it to separate patch.


> 
>>
>> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
>> Cc: Badal Nilawar <badal.nilawar@intel.com>
>> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
>> Signed-off-by: Himal Prasad Ghimiray <himal.prasad.ghimiray@intel.com>
>> ---
>>   drivers/gpu/drm/xe/xe_force_wake.c       | 24 +++++++++++++++++-------
>>   drivers/gpu/drm/xe/xe_force_wake_types.h |  4 +++-
>>   2 files changed, 20 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/xe/xe_force_wake.c b/drivers/gpu/drm/xe/xe_force_wake.c
>> index a64c14757c84..5ce9e912818a 100644
>> --- a/drivers/gpu/drm/xe/xe_force_wake.c
>> +++ b/drivers/gpu/drm/xe/xe_force_wake.c
>> @@ -21,15 +21,25 @@ static const char *str_wake_sleep(bool wake)
>>   	return wake ? "wake" : "sleep";
>>   }
>>   
>> -static void domain_init(struct xe_force_wake_domain *domain,
>> +static void mark_domain_initialized(struct xe_force_wake *fw,
>> +				    enum xe_force_wake_domain_id id)
>> +{
>> +	fw->initialized_domains |= BIT(id);
>> +}
>> +
>> +static void domain_init(struct xe_force_wake *fw,
> 
> since this function is no longer taking 'domain' as param, then IMO it
> should be named like:

Ok.

> 
> 	init_domain(fw, id, reg, ack)
> 
>>   			enum xe_force_wake_domain_id id,
>>   			struct xe_reg reg, struct xe_reg ack)
>>   {
>> +	struct xe_force_wake_domain *domain = &fw->domains[id];
>> +
>>   	domain->id = id;
>>   	domain->reg_ctl = reg;
>>   	domain->reg_ack = ack;
>>   	domain->val = FORCEWAKE_MT(FORCEWAKE_KERNEL);
>>   	domain->mask = FORCEWAKE_MT_MASK(FORCEWAKE_KERNEL);
>> +
>> +	mark_domain_initialized(fw, id);
>>   }
>>   
>>   void xe_force_wake_init_gt(struct xe_gt *gt, struct xe_force_wake *fw)
>> @@ -43,12 +53,12 @@ void xe_force_wake_init_gt(struct xe_gt *gt, struct xe_force_wake *fw)
>>   	xe_gt_assert(gt, GRAPHICS_VER(gt_to_xe(gt)) >= 11);
>>   
>>   	if (xe->info.graphics_verx100 >= 1270) {
>> -		domain_init(&fw->domains[XE_FW_DOMAIN_ID_GT],
>> +		domain_init(fw,
>>   			    XE_FW_DOMAIN_ID_GT,
>>   			    FORCEWAKE_GT,
>>   			    FORCEWAKE_ACK_GT_MTL);
> 
> likely above alignment could be optimized at least to:
> 
> 	init_domain(fw, XE_FW_DOMAIN_ID_GT,
> 		    FORCEWAKE_GT,
> 		    FORCEWAKE_ACK_GT_MTL);
> 

ok

> 
>>   	} else {
>> -		domain_init(&fw->domains[XE_FW_DOMAIN_ID_GT],
>> +		domain_init(fw,
>>   			    XE_FW_DOMAIN_ID_GT,
>>   			    FORCEWAKE_GT,
>>   			    FORCEWAKE_ACK_GT);
>> @@ -63,7 +73,7 @@ void xe_force_wake_init_engines(struct xe_gt *gt, struct xe_force_wake *fw)
>>   	xe_gt_assert(gt, GRAPHICS_VER(gt_to_xe(gt)) >= 11);
>>   
>>   	if (!xe_gt_is_media_type(gt))
>> -		domain_init(&fw->domains[XE_FW_DOMAIN_ID_RENDER],
>> +		domain_init(fw,
>>   			    XE_FW_DOMAIN_ID_RENDER,
>>   			    FORCEWAKE_RENDER,
>>   			    FORCEWAKE_ACK_RENDER);
>> @@ -72,7 +82,7 @@ void xe_force_wake_init_engines(struct xe_gt *gt, struct xe_force_wake *fw)
>>   		if (!(gt->info.engine_mask & BIT(i)))
>>   			continue;
>>   
>> -		domain_init(&fw->domains[XE_FW_DOMAIN_ID_MEDIA_VDBOX0 + j],
>> +		domain_init(fw,
>>   			    XE_FW_DOMAIN_ID_MEDIA_VDBOX0 + j,
>>   			    FORCEWAKE_MEDIA_VDBOX(j),
>>   			    FORCEWAKE_ACK_MEDIA_VDBOX(j));
>> @@ -82,14 +92,14 @@ void xe_force_wake_init_engines(struct xe_gt *gt, struct xe_force_wake *fw)
>>   		if (!(gt->info.engine_mask & BIT(i)))
>>   			continue;
>>   
>> -		domain_init(&fw->domains[XE_FW_DOMAIN_ID_MEDIA_VEBOX0 + j],
>> +		domain_init(fw,
>>   			    XE_FW_DOMAIN_ID_MEDIA_VEBOX0 + j,
>>   			    FORCEWAKE_MEDIA_VEBOX(j),
>>   			    FORCEWAKE_ACK_MEDIA_VEBOX(j));
>>   	}
>>   
>>   	if (gt->info.engine_mask & BIT(XE_HW_ENGINE_GSCCS0))
>> -		domain_init(&fw->domains[XE_FW_DOMAIN_ID_GSC],
>> +		domain_init(fw,
>>   			    XE_FW_DOMAIN_ID_GSC,
>>   			    FORCEWAKE_GSC,
>>   			    FORCEWAKE_ACK_GSC);
>> diff --git a/drivers/gpu/drm/xe/xe_force_wake_types.h b/drivers/gpu/drm/xe/xe_force_wake_types.h
>> index ed0edc2cdf9f..fde17dc3d01e 100644
>> --- a/drivers/gpu/drm/xe/xe_force_wake_types.h
>> +++ b/drivers/gpu/drm/xe/xe_force_wake_types.h
>> @@ -78,7 +78,9 @@ struct xe_force_wake {
>>   	/** @lock: protects everything force wake struct */
>>   	spinlock_t lock;
>>   	/** @awake_domains: mask of all domains awake */
>> -	enum xe_force_wake_domains awake_domains;
>> +	unsigned int awake_domains;
>> +	/** @initialized_domains: mask of all initialized domains */
>> +	unsigned int initialized_domains;
>>   	/** @domains: force wake domains */
>>   	struct xe_force_wake_domain domains[XE_FW_DOMAIN_ID_COUNT];
>>   };
>