Re: [PATCH] drm/xe/mocs: Check that MOCS table.ops is assigned

[Michal Wajdeczko <michal.wajdeczko@intel.com>]

On 26.06.2024 23:12, Matt Roper wrote:
> When MOCS debugfs dumping was added in commit 9fbd0adbcbe8
> ("drm/xe/mocs: Add debugfs node to dump mocs"), a sanity check was added
> to xe_mocs_dump() to ensure table.ops->dump is not a NULL pointer.  The
> more common programmer mistake when adding MOCS enablement for new
> platforms is to simply forget to assign anything to table.ops
> altogether.  Extend the check to also cover a NULL table.ops and add a
> WARN() so that CI will help us catch these mistakes when new platforms
> are enabled in the future.

but for the programming mistakes, shouldn't we use xe_assert() instead?

and in get_mocs_settings() there is already:

	xe_assert(xe, !info->ops || info->ops->dump);

maybe it just needs to be updated?

> 
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> ---
>  drivers/gpu/drm/xe/xe_mocs.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_mocs.c b/drivers/gpu/drm/xe/xe_mocs.c
> index de3f2d3f1b04..d43900d351f1 100644
> --- a/drivers/gpu/drm/xe/xe_mocs.c
> +++ b/drivers/gpu/drm/xe/xe_mocs.c
> @@ -779,7 +779,7 @@ void xe_mocs_dump(struct xe_gt *gt, struct drm_printer *p)
>  
>  	flags = get_mocs_settings(xe, &table);
>  
> -	if (!table.ops->dump)
> +	if (xe_gt_WARN_ON(gt, !table.ops || !table.ops->dump))
>  		return;
>  
>  	xe_pm_runtime_get_noresume(xe);