Re: [PATCH 1/1] drm/xe/eustall: Do not support EU stall on SRIOV VF

["Dixit, Ashutosh" <ashutosh.dixit@intel.com>]

On Wed, 23 Apr 2025 14:41:10 -0700, Harish Chegondi wrote:
>
> On Mon, Apr 21, 2025 at 09:25:46PM -0700, Dixit, Ashutosh wrote:
> > On Mon, 21 Apr 2025 17:59:43 -0700, Dixit, Ashutosh wrote:
> > >
> > > On Sun, 20 Apr 2025 22:59:01 -0700, Harish Chegondi wrote:
> > > >
> > > > EU stall sampling is not supported on SRIOV VF. Do not
> > > > initialize or open EU stall stream on SRIOV VF.
> > > >
> > > > Signed-off-by: Harish Chegondi <harish.chegondi@intel.com>
> > > > ---
> > > >  drivers/gpu/drm/xe/xe_eu_stall.c | 3 +++
> > > >  drivers/gpu/drm/xe/xe_eu_stall.h | 3 ++-
> > > >  2 files changed, 5 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/xe/xe_eu_stall.c b/drivers/gpu/drm/xe/xe_eu_stall.c
> > > > index f2bb9168967c..64788277ea52 100644
> > > > --- a/drivers/gpu/drm/xe/xe_eu_stall.c
> > > > +++ b/drivers/gpu/drm/xe/xe_eu_stall.c
> > > > @@ -208,6 +208,9 @@ int xe_eu_stall_init(struct xe_gt *gt)
> > > >	struct xe_device *xe = gt_to_xe(gt);
> > > >	int ret;
> > > >
> > > > +	if (!xe_eu_stall_supported_on_platform(xe))
> > > > +		return 0;
> > >
> > > This check is not strictly needed. If the check is not there, just some
> > > unnecessary stuff will get initialized.
> > >
> > > But if you want to add this check here, you will also need to a
> > > 'if (!gt->eu_stall)' check in xe_eu_stall_fini(). If you test this patch,
> > > maybe you will already see oops when unloading the driver?
> > >
> > > So either remove this check or add the additional check in
> > > xe_eu_stall_fini().
> >
> > If you do retain this check, maybe change the
> > xe_eu_stall_supported_on_platform() check in xe_eu_stall_stream_open() to
> > also 'if (!gt->eu_stall)' check, I think.
>
> I can change the if check to 'if (!gt->eu_stall)', but at this point in
> the function GT hasn't been determined yet. I will have to move the
> check later in the function after GT has been determined.

Sure, let's put it after the 'if (!props.gt)' check.

> >
> > >
> > > > +
> > > >	gt->eu_stall = kzalloc(sizeof(*gt->eu_stall), GFP_KERNEL);
> > > >	if (!gt->eu_stall) {
> > > >		ret = -ENOMEM;
> > > > diff --git a/drivers/gpu/drm/xe/xe_eu_stall.h b/drivers/gpu/drm/xe/xe_eu_stall.h
> > > > index ed9d0f233566..d1c76e503799 100644
> > > > --- a/drivers/gpu/drm/xe/xe_eu_stall.h
> > > > +++ b/drivers/gpu/drm/xe/xe_eu_stall.h
> > > > @@ -7,6 +7,7 @@
> > > >  #define __XE_EU_STALL_H__
> > > >
> > > >  #include "xe_gt_types.h"
> > > > +#include "xe_sriov.h"
> > > >
> > > >  size_t xe_eu_stall_get_per_xecore_buf_size(void);
> > > >  size_t xe_eu_stall_data_record_size(struct xe_device *xe);
> > > > @@ -19,6 +20,6 @@ int xe_eu_stall_stream_open(struct drm_device *dev,
> > > >
> > > >  static inline bool xe_eu_stall_supported_on_platform(struct xe_device *xe)
> > > >  {
> > > > -	return xe->info.platform == XE_PVC || GRAPHICS_VER(xe) >= 20;
> > > > +	return !IS_SRIOV_VF(xe) && (xe->info.platform == XE_PVC || GRAPHICS_VER(xe) >= 20);
> > > >  }
> > > >  #endif
> > > > --
> > > > 2.48.1
> > > >