Re: [PATCH 1/2] drm/xe: Add member supported_domains to xe_force_wake

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

On 31-05-2024 20:57, Rodrigo Vivi wrote:
> On Fri, May 31, 2024 at 12:18:44PM +0530, Himal Prasad Ghimiray wrote:
>> This field serves as a bitmask representing all supported forcewake
>> domains on the GT.
>>
>> 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       | 48 ++++++++++++------------
>>   drivers/gpu/drm/xe/xe_force_wake_types.h |  2 +
>>   2 files changed, 25 insertions(+), 25 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/xe/xe_force_wake.c b/drivers/gpu/drm/xe/xe_force_wake.c
>> index 9bbe8a5040da..a2947f0046d4 100644
>> --- a/drivers/gpu/drm/xe/xe_force_wake.c
>> +++ b/drivers/gpu/drm/xe/xe_force_wake.c
>> @@ -26,15 +26,25 @@ fw_to_xe(struct xe_force_wake *fw)
>>   	return gt_to_xe(fw_to_gt(fw));
>>   }
>>   
>> -static void domain_init(struct xe_force_wake_domain *domain,
>> +static inline void mark_domain_supported(struct xe_force_wake *fw,
>> +					 enum xe_force_wake_domain_id id)
>> +{
>> +	fw->supported_domains |= BIT(id);
>> +}
>> +
>> +static void domain_init(struct xe_force_wake *fw,
>>   			enum xe_force_wake_domain_id id,
>>   			struct xe_reg reg, struct xe_reg ack, u32 val, u32 mask)
>>   {
>> +	struct xe_force_wake_domain *domain = &fw->domains[id];
>> +
>>   	domain->id = id;
>>   	domain->reg_ctl = reg;
>>   	domain->reg_ack = ack;
>>   	domain->val = val;
>>   	domain->mask = mask;
>> +
>> +	mark_domain_supported(fw, id);
>>   }
>>   
>>   void xe_force_wake_init_gt(struct xe_gt *gt, struct xe_force_wake *fw)
>> @@ -48,17 +58,11 @@ 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],
>> -			    XE_FW_DOMAIN_ID_GT,
>> -			    FORCEWAKE_GT,
>> -			    FORCEWAKE_ACK_GT_MTL,
>> -			    BIT(0), BIT(16));
>> +		domain_init(fw, XE_FW_DOMAIN_ID_GT, FORCEWAKE_GT,
>> +			    FORCEWAKE_ACK_GT_MTL, BIT(0), BIT(16));
>>   	} else {
>> -		domain_init(&fw->domains[XE_FW_DOMAIN_ID_GT],
>> -			    XE_FW_DOMAIN_ID_GT,
>> -			    FORCEWAKE_GT,
>> -			    FORCEWAKE_ACK_GT,
>> -			    BIT(0), BIT(16));
>> +		domain_init(fw, XE_FW_DOMAIN_ID_GT, FORCEWAKE_GT,
>> +			    FORCEWAKE_ACK_GT, BIT(0), BIT(16));
>>   	}
>>   }
>>   
>> @@ -70,18 +74,14 @@ 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],
>> -			    XE_FW_DOMAIN_ID_RENDER,
>> -			    FORCEWAKE_RENDER,
>> -			    FORCEWAKE_ACK_RENDER,
>> -			    BIT(0), BIT(16));
>> +		domain_init(fw, XE_FW_DOMAIN_ID_RENDER, FORCEWAKE_RENDER,
>> +			    FORCEWAKE_ACK_RENDER, BIT(0), BIT(16));
> Since you are here, could you please add some meaning to these undefined bits?


sure.


>
>>   
>>   	for (i = XE_HW_ENGINE_VCS0, j = 0; i <= XE_HW_ENGINE_VCS7; ++i, ++j) {
>>   		if (!(gt->info.engine_mask & BIT(i)))
>>   			continue;
>>   
>> -		domain_init(&fw->domains[XE_FW_DOMAIN_ID_MEDIA_VDBOX0 + j],
>> -			    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),
>>   			    BIT(0), BIT(16));
>> @@ -91,19 +91,17 @@ 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],
>> -			    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),
>>   			    BIT(0), BIT(16));
>> +
>>   	}
>>   
>>   	if (gt->info.engine_mask & BIT(XE_HW_ENGINE_GSCCS0))
>> -		domain_init(&fw->domains[XE_FW_DOMAIN_ID_GSC],
>> -			    XE_FW_DOMAIN_ID_GSC,
>> -			    FORCEWAKE_GSC,
>> -			    FORCEWAKE_ACK_GSC,
>> -			    BIT(0), BIT(16));
>> +		domain_init(fw, XE_FW_DOMAIN_ID_GSC, FORCEWAKE_GSC,
>> +			    FORCEWAKE_ACK_GSC, BIT(0), BIT(16));
>> +
>>   }
>>   
>>   static void domain_wake(struct xe_gt *gt, struct xe_force_wake_domain *domain)
>> diff --git a/drivers/gpu/drm/xe/xe_force_wake_types.h b/drivers/gpu/drm/xe/xe_force_wake_types.h
>> index ed0edc2cdf9f..4179fc51291e 100644
>> --- a/drivers/gpu/drm/xe/xe_force_wake_types.h
>> +++ b/drivers/gpu/drm/xe/xe_force_wake_types.h
>> @@ -79,6 +79,8 @@ struct xe_force_wake {
>>   	spinlock_t lock;
>>   	/** @awake_domains: mask of all domains awake */
>>   	enum xe_force_wake_domains awake_domains;
>> +	/** @supported_domains: mask of all supported domains */
>> +	enum xe_force_wake_domains supported_domains;
>>   	/** @domains: force wake domains */
>>   	struct xe_force_wake_domain domains[XE_FW_DOMAIN_ID_COUNT];
>>   };
>> -- 
>> 2.25.1
>>