From: Rodrigo Vivi <rodrigo.vivi@intel.com>
To: Andi Shyti <andi.shyti@linux.intel.com>
Cc: Soham Purkait <soham.purkait@intel.com>,
<intel-xe@lists.freedesktop.org>, <riana.tauro@intel.com>,
<anshuman.gupta@intel.com>, <aravind.iddamsetty@linux.intel.com>,
<badal.nilawar@intel.com>, <raag.jadav@intel.com>,
<ravi.kishore.koppuravuri@intel.com>,
<mallesh.koujalagi@intel.com>, <anoop.c.vijay@intel.com>
Subject: Re: [PATCH v1 2/2] drm/xe/xe_ras: Add RAS support for GPU health indicator
Date: Tue, 21 Apr 2026 09:21:03 -0400 [thread overview]
Message-ID: <aed5vwPcAdxS45aU@intel.com> (raw)
In-Reply-To: <aed0EPEZ8oQLzPNc@ashyti-mobl2.lan>
On Tue, Apr 21, 2026 at 02:56:48PM +0200, Andi Shyti wrote:
> Hi Rodrigo,
>
> > >
> > > If we need two strings, then we create two files.
> >
> > This part I don't agree. This is just the same as the power cases
> > which I already had pointed out.
> >
> > But right, you do have a point here. Different from power, we don't
> > necessarily need to expose all the supported modes + the current one.
> > After all, these states won't be that different from state to state.
> >
> > We just need to document it properly and then just print:
> > either
> >
> > ok
> > instead of
> > [ok] warning critical
> >
> > or
> > warning
> > instead of
> > ok [warning] critical
> >
> > or critical
> > instead of
> > ok warning [critical]
> >
> > The full string with all the supported modes do sound bloated for
> > this case. Considering that the supported cases themselves shouldn't
> > change much and that we keep that documented. My bad, I'm sorry for
> > the confusion here.
>
> Think of what a miserable life have userspace developers when
> parsing power files. They need to parse for the '[', for the ']'
> the number of words, etc. We have here an excellent example of
> how *not* to do things. :-)
not that miserable nowadays, just one AI prompt away ;)
>
> > > If we just need to provide information to system administrators,
> > > then we can either use the debugfs or procfs that don't need to
> > > be considered as uAPI.
> >
> > That is not true. In many data center deployments the 'debugfs'
> > is not available and the admin and cluster tools still need this
> > information. It needs to be stable API.
>
> Bingo! :-)
>
> Then you need to define an easily parsable, well documented api,
> so that it's accessable to everyone, applications and sysadmins.
>
> Overall, I don't think this is a matter of taste, but, anyway,
> you are the maintainer and yours is the last word. To me, this
> looks incorrect.
I agree with most of your statements above, except that to me
this is easily parsable. But at the same time I also agree that
the simple state string is even easier and we should take that
path for this specific case.
But because (**and only because**) we don't expect changes in the
'available' modes in the HW/FW.
But the /sys/power style should be the way to go if we needed to
be prepared to have a flexible SW API that depends on different
HW and FW underneath.
>
> Thanks,
> Andi
next prev parent reply other threads:[~2026-04-21 13:21 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-04-16 9:36 [PATCH v1 0/2] drm/xe: Add support for GPU health indicator Soham Purkait
2026-04-16 9:36 ` [PATCH v1 1/2] drm/xe/xe_ras: Add structures and commands for RAS " Soham Purkait
2026-04-16 11:39 ` Andi Shyti
2026-04-17 14:45 ` Rodrigo Vivi
2026-04-16 9:36 ` [PATCH v1 2/2] drm/xe/xe_ras: Add RAS support for " Soham Purkait
2026-04-16 11:54 ` Andi Shyti
2026-04-17 14:51 ` Rodrigo Vivi
2026-04-20 15:26 ` Andi Shyti
2026-04-20 19:51 ` Rodrigo Vivi
2026-04-21 12:56 ` Andi Shyti
2026-04-21 13:21 ` Rodrigo Vivi [this message]
2026-04-22 6:05 ` Purkait, Soham
2026-04-20 16:19 ` Purkait, Soham
2026-04-20 17:35 ` Andi Shyti
2026-04-16 9:55 ` ✗ CI.checkpatch: warning for drm/xe: Add " Patchwork
2026-04-16 9:56 ` ✓ CI.KUnit: success " Patchwork
2026-04-16 10:58 ` ✓ Xe.CI.BAT: " Patchwork
2026-04-16 12:01 ` ✗ Xe.CI.FULL: failure " 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=aed5vwPcAdxS45aU@intel.com \
--to=rodrigo.vivi@intel.com \
--cc=andi.shyti@linux.intel.com \
--cc=anoop.c.vijay@intel.com \
--cc=anshuman.gupta@intel.com \
--cc=aravind.iddamsetty@linux.intel.com \
--cc=badal.nilawar@intel.com \
--cc=intel-xe@lists.freedesktop.org \
--cc=mallesh.koujalagi@intel.com \
--cc=raag.jadav@intel.com \
--cc=ravi.kishore.koppuravuri@intel.com \
--cc=riana.tauro@intel.com \
--cc=soham.purkait@intel.com \
/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.