All of lore.kernel.org
 help / color / mirror / Atom feed
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

  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.