Re: [PATCH] drm/xe: Check pat.ops before dumping PAT settings

["Piotr Piórkowski" <piotr.piorkowski@intel.com>]

Michal Wajdeczko <michal.wajdeczko@intel.com> wrote on pią [2024-kwi-05 16:36:25 +0200]:
> We may leave pat.ops unset when running on brand new platform or
> when running as a VF.  While the former is unlikely, the latter
> is valid (future) use case and will cause NPD when someone will
> try to dump PAT settings by debugfs.
> 
> It's better to check pointer to pat.ops instead of specific .dump
> hook, as we have this hook always defined for every .ops variant.
> 
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> ---
>  drivers/gpu/drm/xe/xe_pat.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_pat.c b/drivers/gpu/drm/xe/xe_pat.c
> index 66d8e3dd8237..f0031c2e9818 100644
> --- a/drivers/gpu/drm/xe/xe_pat.c
> +++ b/drivers/gpu/drm/xe/xe_pat.c
> @@ -447,7 +447,7 @@ void xe_pat_dump(struct xe_gt *gt, struct drm_printer *p)
>  {
>  	struct xe_device *xe = gt_to_xe(gt);
>  
> -	if (!xe->pat.ops->dump)
> +	if (!xe->pat.ops)

You are right that we currently have a dump pointer set for each xe_pat_ops structure,
and in this situation it is enough to check the ops for the cases you listed.
But I assume that since we are verifying the dump pointer here, that formally, for some
future case, we may not set this pointer.
Therefore, it seems to me that it would be more correct for you to check both pointers
here: ops and dump.

Thanks,
Piotr

>  		return;
>  
>  	xe->pat.ops->dump(gt, p);
> -- 
> 2.43.0
> 

--