Re: [PATCH v2 2/3] drm/xe: Add member initialized_domains to xe_force_wake

["Nilawar, Badal" <badal.nilawar@intel.com>]

On 03-06-2024 14:31, Himal Prasad Ghimiray wrote:
> To determine the wake status of all initialized domains, a direct
> comparison using (XE_FORCEWAKE_ALL & fw->awake_domains) == XE_FORCEWAKE_ALL
> won't suffice due to difference in domain support across platforms and GTs.
> For instance, MTL GT0 only supports GT and RENDER domains, thus for the
> force wake all scenario, only bits corresponding to GT and RENDER should
> be activated. Consequently, the condition
> (XE_FORCEWAKE_ALL & fw->awake_domains) == XE_FORCEWAKE_ALL will
> consistently be false.
> 
> To address this, introduce a new member named initialized_domains to
> xe_force_wake. This attribute will function as a bitmask, encapsulating
> all initialized force wake domains on the GT.
> 
> v2:
> - %s/supported/initialized/
> - update commit message (Rodrigo)
> 
> Cc: Badal Nilawar <badal.nilawar@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Suggested-by: Badal Nilawar <badal.nilawar@intel.com>
> Signed-off-by: Himal Prasad Ghimiray <himal.prasad.ghimiray@intel.com>
> ---
>   drivers/gpu/drm/xe/xe_force_wake.c       | 30 ++++++++++++++----------
>   drivers/gpu/drm/xe/xe_force_wake_types.h |  2 ++
>   2 files changed, 19 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_force_wake.c b/drivers/gpu/drm/xe/xe_force_wake.c
> index 64721c01f30d..98699622d83b 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_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,
>   			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_initialized(fw, id);
>   }
>   
>   void xe_force_wake_init_gt(struct xe_gt *gt, struct xe_force_wake *fw)
> @@ -48,15 +58,13 @@ 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,
> +		domain_init(fw, XE_FW_DOMAIN_ID_GT,
>   			    FORCEWAKE_GT,
>   			    FORCEWAKE_ACK_GT_MTL,
>   			    FORCEWAKE_THREAD_XE,
>   			    FORCEWAKE_THREAD_XE_MASK);
>   	} else {
> -		domain_init(&fw->domains[XE_FW_DOMAIN_ID_GT],
> -			    XE_FW_DOMAIN_ID_GT,
> +		domain_init(fw, XE_FW_DOMAIN_ID_GT,
>   			    FORCEWAKE_GT,
>   			    FORCEWAKE_ACK_GT,
>   			    FORCEWAKE_THREAD_XE,
> @@ -72,8 +80,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],
> -			    XE_FW_DOMAIN_ID_RENDER,
> +		domain_init(fw, XE_FW_DOMAIN_ID_RENDER,
>   			    FORCEWAKE_RENDER,
>   			    FORCEWAKE_ACK_RENDER,
>   			    FORCEWAKE_THREAD_XE,
> @@ -83,8 +90,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],
> -			    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),
>   			    FORCEWAKE_THREAD_XE,
> @@ -95,8 +101,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_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),
>   			    FORCEWAKE_THREAD_XE,
> @@ -104,8 +109,7 @@ void xe_force_wake_init_engines(struct xe_gt *gt, struct xe_force_wake *fw)
>   	}
>   
>   	if (gt->info.engine_mask & BIT(XE_HW_ENGINE_GSCCS0))
> -		domain_init(&fw->domains[XE_FW_DOMAIN_ID_GSC],
> -			    XE_FW_DOMAIN_ID_GSC,
> +		domain_init(fw, XE_FW_DOMAIN_ID_GSC,
>   			    FORCEWAKE_GSC,
>   			    FORCEWAKE_ACK_GSC,
>   			    FORCEWAKE_THREAD_XE,
> diff --git a/drivers/gpu/drm/xe/xe_force_wake_types.h b/drivers/gpu/drm/xe/xe_force_wake_types.h
> index ed0edc2cdf9f..1056d196b29d 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;
> +	/** @initialized_domains: mask of all supported domains */
> +	enum xe_force_wake_domains initialized_domains;
>   	/** @domains: force wake domains */
LGTM,
Reviewed-by: Badal Nilawar <badal.nilawar@intel.com>

Regards,
Badal
>   	struct xe_force_wake_domain domains[XE_FW_DOMAIN_ID_COUNT];
>   };