| Message ID | 1374018073-6596-1-git-send-email-ben@bwidawsk.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Jul 16, 2013 at 04:41:13PM -0700, Ben Widawsky wrote: > Make the uevent strings part of the user API for people who wish to > write their own listeners. > > CC: Chad Versace <chad.versace@linux.intel.com> > Signed-off-by: Ben Widawsky <ben@bwidawsk.net> One thing I've toyed around with a bit is that we should add kerneldoc to our uapi headers and create a DocBook out of it (maybe as a subsection in the drm userspace api chapter). I guess the DocBook integration needs an overall approach, but we should start to add comments to each piece of userspace api to clearly spec them. See below for what I have in mind ... > --- > drivers/gpu/drm/i915/i915_irq.c | 8 ++++---- > include/uapi/drm/i915_drm.h | 3 +++ > 2 files changed, 7 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c > index 9910699..60fa513 100644 > --- a/drivers/gpu/drm/i915/i915_irq.c > +++ b/drivers/gpu/drm/i915/i915_irq.c > @@ -812,7 +812,7 @@ static void ivybridge_parity_work(struct work_struct *work) > > mutex_unlock(&dev_priv->dev->struct_mutex); > > - parity_event[0] = "L3_PARITY_ERROR=1"; > + parity_event[0] = I915_L3_PARITY_EVENT"=1"; > parity_event[1] = kasprintf(GFP_KERNEL, "ROW=%d", row); > parity_event[2] = kasprintf(GFP_KERNEL, "BANK=%d", bank); > parity_event[3] = kasprintf(GFP_KERNEL, "SUBBANK=%d", subbank); > @@ -1435,9 +1435,9 @@ static void i915_error_work_func(struct work_struct *work) > gpu_error); > struct drm_device *dev = dev_priv->dev; > struct intel_ring_buffer *ring; > - char *error_event[] = { "ERROR=1", NULL }; > - char *reset_event[] = { "RESET=1", NULL }; > - char *reset_done_event[] = { "ERROR=0", NULL }; > + char *error_event[] = { I915_ERROR_EVENT"=1", NULL }; > + char *reset_event[] = { I915_RESET_EVENT"=1", NULL }; > + char *reset_done_event[] = { I915_ERROR_EVENT"=0", NULL }; > int i, ret; > > kobject_uevent_env(&dev->primary->kdev.kobj, KOBJ_CHANGE, error_event); > diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h > index 923ed7f..25a3e74 100644 > --- a/include/uapi/drm/i915_drm.h > +++ b/include/uapi/drm/i915_drm.h > @@ -33,6 +33,9 @@ > * subject to backwards-compatibility constraints. > */ > /** * DOC uevents generated by i915 on it's device node * * These are events generated in addition to the standardized uevents * genereated by the drm core (like signalling connector state changes). */ /** * I915_L3_PARITY_EVENT - l3 parity event * * Generated when the driver receives a parity mismatch event from the gpu * l3 cache. Additional information supplied is ROW, BANK, SUBBANK of the * affected cacheline. Userspace should keep track of these events and if * a specific cache-line seems to have a persistent error remap it with * the l3 remapping tool supplied in intel-gpu-tools * * The value supplied with the event is always 1. */ > +#define I915_L3_PARITY_EVENT "L3_PARITY_ERROR" > +#define I915_ERROR_EVENT "ERROR" > +#define I915_RESET_EVENT "RESET" I'll let you cook up something nice for the other two ;-) Cheers, Daniel > > /* Each region is a minimum of 16k, and there are at most 255 of them. > */ > -- > 1.8.3.3 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On 07/16/2013 10:07 PM, Daniel Vetter wrote: > On Tue, Jul 16, 2013 at 04:41:13PM -0700, Ben Widawsky wrote: >> Make the uevent strings part of the user API for people who wish to >> write their own listeners. >> >> CC: Chad Versace <chad.versace@linux.intel.com> >> Signed-off-by: Ben Widawsky <ben@bwidawsk.net> > > One thing I've toyed around with a bit is that we should add kerneldoc to > our uapi headers and create a DocBook out of it (maybe as a subsection in > the drm userspace api chapter). I guess the DocBook integration needs an > overall approach, but we should start to add comments to each piece of > userspace api to clearly spec them. See below for what I have in mind ... Yes, docs please. I don't have the kernel-fu of a kernel-dev, so without docs I don't know what these events mean and what to expect from them. >> - parity_event[0] = "L3_PARITY_ERROR=1"; >> + parity_event[0] = I915_L3_PARITY_EVENT"=1"; Small nitpick. I usually find string concatenation more readable like this, with a space: parity_event[0] = I915_L3_PARITY_EVENT "=1"; But, that's just my preference, so feel free to ignore me. >> +#define I915_L3_PARITY_EVENT "L3_PARITY_ERROR" >> +#define I915_ERROR_EVENT "ERROR" >> +#define I915_RESET_EVENT "RESET" Maybe this is a dumb question... since these are uevents, do you think the names would be improved by given them a "UEVENT" suffix? Like I915_ERROR_UEVENT? Or is that dumb, because these tokens are intended to serve more purposes than uevents?
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 9910699..60fa513 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -812,7 +812,7 @@ static void ivybridge_parity_work(struct work_struct *work) mutex_unlock(&dev_priv->dev->struct_mutex); - parity_event[0] = "L3_PARITY_ERROR=1"; + parity_event[0] = I915_L3_PARITY_EVENT"=1"; parity_event[1] = kasprintf(GFP_KERNEL, "ROW=%d", row); parity_event[2] = kasprintf(GFP_KERNEL, "BANK=%d", bank); parity_event[3] = kasprintf(GFP_KERNEL, "SUBBANK=%d", subbank); @@ -1435,9 +1435,9 @@ static void i915_error_work_func(struct work_struct *work) gpu_error); struct drm_device *dev = dev_priv->dev; struct intel_ring_buffer *ring; - char *error_event[] = { "ERROR=1", NULL }; - char *reset_event[] = { "RESET=1", NULL }; - char *reset_done_event[] = { "ERROR=0", NULL }; + char *error_event[] = { I915_ERROR_EVENT"=1", NULL }; + char *reset_event[] = { I915_RESET_EVENT"=1", NULL }; + char *reset_done_event[] = { I915_ERROR_EVENT"=0", NULL }; int i, ret; kobject_uevent_env(&dev->primary->kdev.kobj, KOBJ_CHANGE, error_event); diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h index 923ed7f..25a3e74 100644 --- a/include/uapi/drm/i915_drm.h +++ b/include/uapi/drm/i915_drm.h @@ -33,6 +33,9 @@ * subject to backwards-compatibility constraints. */ +#define I915_L3_PARITY_EVENT "L3_PARITY_ERROR" +#define I915_ERROR_EVENT "ERROR" +#define I915_RESET_EVENT "RESET" /* Each region is a minimum of 16k, and there are at most 255 of them. */
Make the uevent strings part of the user API for people who wish to write their own listeners. CC: Chad Versace <chad.versace@linux.intel.com> Signed-off-by: Ben Widawsky <ben@bwidawsk.net> --- drivers/gpu/drm/i915/i915_irq.c | 8 ++++---- include/uapi/drm/i915_drm.h | 3 +++ 2 files changed, 7 insertions(+), 4 deletions(-)