All of lore.kernel.org
 help / color / mirror / Atom feed
From: Harish Chegondi <harish.chegondi@intel.com>
To: "Dixit, Ashutosh" <ashutosh.dixit@intel.com>
Cc: <igt-dev@lists.freedesktop.org>
Subject: Re: [PATCH i-g-t 1/2] lib/xe/oa: Add observation type input to intel_xe_perf_ioctl and intel_xe_perf_ioctl_err
Date: Fri, 31 Jan 2025 13:40:51 -0800	[thread overview]
Message-ID: <Z51DY2G10o_Ba5gZ@intel.com> (raw)
In-Reply-To: <85v7tuen8o.wl-ashutosh.dixit@intel.com>

On Fri, Jan 31, 2025 at 12:34:31PM -0800, Dixit, Ashutosh wrote:
> On Tue, 31 Dec 2024 01:46:28 -0800, Harish Chegondi wrote:
> >
> > In order to reuse intel_xe_perf_ioctl() and intel_xe_perf_ioctl_err()
> > functions for EU stall sampling, add observation type as an input
> > parameter to these functions and set the input observation type to
> > observation_type in struct drm_xe_observation_param.
> >
> > Signed-off-by: Harish Chegondi <harish.chegondi@intel.com>
> > ---
> >  lib/xe/xe_oa.c                   | 18 +++++++---
> >  lib/xe/xe_oa.h                   | 11 ++++--
> >  tests/intel/xe_oa.c              | 57 +++++++++++++++++++++-----------
> >  tools/xe-perf/xe_perf_configs.c  |  2 +-
> >  tools/xe-perf/xe_perf_recorder.c |  3 +-
> 
> Why would EU stall want to use functions from lib/xe/xe_oa.c, which is a
> library specific to OA. Why don't you just copy these functions into
> tests/intel/xe_eu_stall.c stall, rename them and use them?

Yes, I can copy the functions and rename them. I just didn't want to create
duplicate code. I hope the maintainers are okay with it.
> 
> The functions are in the library for OA because of use by other OA/perf
> tools, they are not meant for use by EU stall.
> 
> So this is a NAK on this patch.
> 
> Ashutosh

Thanks
Harish.

  parent reply	other threads:[~2025-01-31 21:41 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-12-31  9:46 [PATCH i-g-t 0/2] Add tests for EU stall sampling Harish Chegondi
2024-12-31  9:46 ` [PATCH i-g-t 1/2] lib/xe/oa: Add observation type input to intel_xe_perf_ioctl and intel_xe_perf_ioctl_err Harish Chegondi
2025-01-31 20:34   ` Dixit, Ashutosh
2025-01-31 20:36     ` Dixit, Ashutosh
2025-01-31 20:54       ` Dixit, Ashutosh
2025-01-31 21:40     ` Harish Chegondi [this message]
2024-12-31  9:46 ` [PATCH i-g-t 2/2] tests/intel/xe_eu_stall: Add tests for EU stall sampling Harish Chegondi
2025-01-24 17:15   ` Kamil Konieczny
2025-02-04  2:11   ` Dixit, Ashutosh
2025-02-18 23:06     ` Harish Chegondi
2024-12-31 11:10 ` ✓ i915.CI.BAT: success for " Patchwork
2024-12-31 11:13 ` ✓ Xe.CI.BAT: " Patchwork
2024-12-31 12:37 ` ✗ Xe.CI.Full: failure " Patchwork
2024-12-31 12:46 ` ✗ i915.CI.Full: " Patchwork

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Z51DY2G10o_Ba5gZ@intel.com \
    --to=harish.chegondi@intel.com \
    --cc=ashutosh.dixit@intel.com \
    --cc=igt-dev@lists.freedesktop.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.