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

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

On 08-10-2024 12:40, Himal Prasad Ghimiray wrote:
> This field serves as a bitmask representing all initialized forcewake
> domains on the GT.
> 
> v2
> - Move awake_domains datatype change out of this patch (Michal)
> - Rename domain_init to init_domain (Michal)
> - optimize alignment (Michal)
> 
> 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       | 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 a64c14757c84..ac0419da7173 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 init_domain(struct xe_force_wake *fw,
>   			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,13 +53,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,
> +		init_domain(fw, XE_FW_DOMAIN_ID_GT,
>   			    FORCEWAKE_GT,
>   			    FORCEWAKE_ACK_GT_MTL);
>   	} else {
> -		domain_init(&fw->domains[XE_FW_DOMAIN_ID_GT],
> -			    XE_FW_DOMAIN_ID_GT,
> +		init_domain(fw, XE_FW_DOMAIN_ID_GT,
>   			    FORCEWAKE_GT,
>   			    FORCEWAKE_ACK_GT);
>   	}
> @@ -63,8 +71,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,
> +		init_domain(fw, XE_FW_DOMAIN_ID_RENDER,
>   			    FORCEWAKE_RENDER,
>   			    FORCEWAKE_ACK_RENDER);
>   
> @@ -72,8 +79,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,
> +		init_domain(fw, XE_FW_DOMAIN_ID_MEDIA_VDBOX0 + j,
>   			    FORCEWAKE_MEDIA_VDBOX(j),
>   			    FORCEWAKE_ACK_MEDIA_VDBOX(j));
>   	}
> @@ -82,15 +88,13 @@ 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,
> +		init_domain(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],
> -			    XE_FW_DOMAIN_ID_GSC,
> +		init_domain(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..494240777d03 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 initialized domains */
> +	unsigned int initialized_domains;
>   	/** @domains: force wake domains */
>   	struct xe_force_wake_domain domains[XE_FW_DOMAIN_ID_COUNT];
>   };

Looks good to me.
Reviewed-by: Badal Nilawar <badal.nilawar@intel.com>

Regards,
Badal