Re: [PATCH 5/9] drm/xe/vf: Don't initialize OA if VF

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

On 20.06.2024 00:57, Dixit, Ashutosh wrote:
> On Wed, 19 Jun 2024 15:40:43 -0700, Michal Wajdeczko wrote:
>>
>> On 20.06.2024 00:10, Dixit, Ashutosh wrote:
>>> On Wed, 19 Jun 2024 14:56:32 -0700, Dixit, Ashutosh wrote:
>>>>
>>>> On Wed, 19 Jun 2024 14:45:53 -0700, Michal Wajdeczko wrote:
>>>>>
>>>>> We don't support Observation Architecture on the VF device.
>>>>>
>>>>> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
>>>>> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
>>>>> Cc: Ashutosh Dixit <ashutosh.dixit@intel.com>
>>>>> ---
>>>>>  drivers/gpu/drm/xe/xe_oa.c | 4 ++++
>>>>>  1 file changed, 4 insertions(+)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/xe/xe_oa.c b/drivers/gpu/drm/xe/xe_oa.c
>>>>> index 34206e0b6a08..f54a97954452 100644
>>>>> --- a/drivers/gpu/drm/xe/xe_oa.c
>>>>> +++ b/drivers/gpu/drm/xe/xe_oa.c
>>>>> @@ -35,6 +35,7 @@
>>>>>  #include "xe_perf.h"
>>>>>  #include "xe_pm.h"
>>>>>  #include "xe_sched_job.h"
>>>>> +#include "xe_sriov.h"
>>>>>
>>>>>  #define DEFAULT_POLL_FREQUENCY_HZ 200
>>>>>  #define DEFAULT_POLL_PERIOD_NS (NSEC_PER_SEC / DEFAULT_POLL_FREQUENCY_HZ)
>>>>> @@ -2385,6 +2386,9 @@ int xe_oa_init(struct xe_device *xe)
>>>>> 	if (XE_WARN_ON(!xe_device_uc_enabled(xe)) || XE_WARN_ON(GRAPHICS_VER(xe) < 12))
>>>>> 		return 0;
>>>>>
>>>>> +	if (IS_SRIOV_VF(xe))
>>>>> +		return 0;
>>>>> +
>>>>
>>>> Do we want to add XE_WARN_ON's
>>>
>>> Though maybe not, if we don't want to see the WARN_ON when loading the VF driver...
>>
>> yep, we don't want WARN on every VF boot
>>
>> and TBH IMO above XE_WARN_ON are all wrong, as:
>>
>> - why WARN if someone used legal param xe.force_execlist ?
>>   we can rely on existing drm_dbg() when someone will attempt to use oa
>>
>> - why WARN if Xe driver by design does not support gen < 12 ?
>>   if you want to be paranoid just add xe_assert() to document that
> 
> I agree, best to get rid of these WARN_ON's (xe_assert will also cause a
> crash which we don't want). 

xe_assert() is based on WARN, but works only on debug builds

> The ioctl's have drm_dbg's which should be
> sufficient as you say. I just sent a patch to do this:
> 
> https://patchwork.freedesktop.org/series/135081/
> 
> Could you please take a look at this. Thanks.
> 
>>
>>>
>>>> or combine this if statement with the previous one?
>>>>
>>>> In any case:
>>>>
>>>> Reviewed-by: Ashutosh Dixit <ashutosh.dixit@intel.com>
>>
>> thanks
>>
>>>>
>>>>
>>>>> 	oa->xe = xe;
>>>>> 	oa->oa_formats = oa_formats;
>>>>>
>>>>> --
>>>>> 2.43.0
>>>>>