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

[Matt Roper]

On Wed, Jun 26, 2024 at 11:31:09PM +0200, Michal Wajdeczko wrote:
> 
> 
> 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?

Yeah, I didn't feel the warn-on was a good choice here, but didn't want
to change the existing flow of the function.  But since you pointed out
that we do indeed already have an assert in get_mocs_settings, I think
we can just remove this warn/return entirely and rely on the assert once
we fix the logic there; I'll send a patch to do that.


Matt

> 
> > 
> > Signed-off-by: Matt Roper <matthew.d.roper at 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);

-- 
Matt Roper
Graphics Software Engineer
Linux GPU Platform Enablement
Intel Corporation