Re: [PATCH] drm/xe/xe_pmu: Validate gt in event supported

[Riana Tauro <riana.tauro@intel.com>]

Hi Matt/Lucas

On 7/2/2025 2:49 AM, Matt Roper wrote:
> On Tue, Jul 01, 2025 at 04:09:44PM -0500, Lucas De Marchi wrote:
>> On Tue, Jul 01, 2025 at 01:31:28PM -0700, Matt Roper wrote:
>>> On Mon, Jun 30, 2025 at 05:28:53PM -0500, Lucas De Marchi wrote:
>>>> On Mon, Jun 30, 2025 at 03:07:41PM +0530, Riana Tauro wrote:
>>>>> Validate gt instead of checking gt_id is lesser
>>>>> than max gts per tile
>>>>
>>>> I prefer the series from Matt Roper that is refactoring this part well.
>>>
>>> I think we want this change regardless.  My series is about refactoring
>>> to future-proof the behavior in case we wind up with multi-tile+multi-GT
>>> platforms in the future.  However Riana's patch here is fixing a real
>>> bug in the code that exists for existing platforms like PVC (i.e., the
>>> code currently lets "GT1" through as valid even on single-tile variants
>>> of PVC).
>>
>> I misread this patch and thought it was an impossible condition that was
>> already protected during event init and it was added more as an "assert
>> we always have this". Now I see this is the call that actually protects
>> the event creation. So, yes, this looks the correct thing to do.
>>
>> Riana, could you expand the commit message to emphasize this is a real
>> issue for platforms like PVC?

Sure will do that.

>>
>> Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com>
Thank you for the review

>>
>> thanks
>> Lucas De Marchi
> 
> I did just notice (as Karthik also pointed out on another part of this
> thread) that if we apply this patch _without_ my series, then
> xe_device_get_gt() could throw a WARN and return something non-NULL if
> the GT ID is too high, which doesn't actually help.  But if we apply
> this fix after my refactoring, then xe_device_get_gt() will return NULL
> on any invalid GT ID and the logic here works as expected.  So I guess
> rather than applying this on its own, we could wait and apply it after
> my series (and I'd drop the patch #1 that I had added to v4 of my
> series).
> 
I misread Karthik's comment. Yeah makes sense. I will wait
and apply after your series.

Thanks
Riana

> 
> Matt
>