| Message ID | 20190116153622.32576-8-lionel.g.landwerlin@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915/perf: add OA interrupt support | expand |
Quoting Lionel Landwerlin (2019-01-16 17:36:22) > With the currently available parameters for the i915-perf stream, > there are still situations that are not well covered : > > If an application opens the stream with polling disable or at very low > frequency and OA interrupt enabled, no data will be available even > though somewhere between nothing and half of the OA buffer worth of > data might have landed in memory. > > To solve this issue we have a new flush ioctl on the perf stream that > forces the i915-perf driver to look at the state of the buffer when > called and makes any data available through both poll() & read() type > syscalls. > > Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com> Link to userspace changes? Regards, Joonas > --- > drivers/gpu/drm/i915/i915_perf.c | 17 +++++++++++++++++ > include/uapi/drm/i915_drm.h | 19 +++++++++++++++++++ > 2 files changed, 36 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c > index da721fce2543..6c98ffa2135e 100644 > --- a/drivers/gpu/drm/i915/i915_perf.c > +++ b/drivers/gpu/drm/i915/i915_perf.c > @@ -2433,6 +2433,20 @@ static void i915_perf_disable_locked(struct i915_perf_stream *stream) > stream->ops->disable(stream); > } > > +/** > + * i915_perf_flush_data - handle `I915_PERF_IOCTL_FLUSH_DATA` ioctl > + * @stream: An enabled i915 perf stream > + * > + * The intention is to flush all the data available for reading from the OA > + * buffer > + */ > +static void i915_perf_flush_data(struct i915_perf_stream *stream) > +{ > + struct drm_i915_private *dev_priv = stream->dev_priv; > + > + dev_priv->perf.oa.pollin = oa_buffer_check(stream->dev_priv, true); > +} > + > /** > * i915_perf_ioctl - support ioctl() usage with i915 perf stream FDs > * @stream: An i915 perf stream > @@ -2456,6 +2470,9 @@ static long i915_perf_ioctl_locked(struct i915_perf_stream *stream, > case I915_PERF_IOCTL_DISABLE: > i915_perf_disable_locked(stream); > return 0; > + case I915_PERF_IOCTL_FLUSH_DATA: > + i915_perf_flush_data(stream); > + return 0; > } > > return -EINVAL; > diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h > index b6db5e544a35..0f0d20080572 100644 > --- a/include/uapi/drm/i915_drm.h > +++ b/include/uapi/drm/i915_drm.h > @@ -1594,6 +1594,25 @@ struct drm_i915_perf_open_param { > */ > #define I915_PERF_IOCTL_DISABLE _IO('i', 0x1) > > +/** > + * Actively check the availability of data from a stream. > + * > + * A stream data availability can be driven by two types of events : > + * > + * - if enabled, the kernel's hrtimer checking the amount of available data > + * in the OA buffer through head/tail registers. > + * > + * - if enabled, the OA unit's interrupt mechanism > + * > + * The kernel hrtimer incur a cost of running callback at fixed time > + * intervals, while the OA interrupt might only happen rarely. In the > + * situation where the application has disabled the kernel's hrtimer and only > + * uses the OA interrupt to know about available data, the application can > + * request an active check of the available OA data through this ioctl. This > + * will make any data in the OA buffer available with either poll() or read(). > + */ > +#define I915_PERF_IOCTL_FLUSH_DATA _IO('i', 0x2) > + > /** > * Common to all i915 perf records > */ > -- > 2.20.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On 22/01/2019 16:25, Joonas Lahtinen wrote: > Quoting Lionel Landwerlin (2019-01-16 17:36:22) >> With the currently available parameters for the i915-perf stream, >> there are still situations that are not well covered : >> >> If an application opens the stream with polling disable or at very low >> frequency and OA interrupt enabled, no data will be available even >> though somewhere between nothing and half of the OA buffer worth of >> data might have landed in memory. >> >> To solve this issue we have a new flush ioctl on the perf stream that >> forces the i915-perf driver to look at the state of the buffer when >> called and makes any data available through both poll() & read() type >> syscalls. >> >> Signed-off-by: Lionel Landwerlin<lionel.g.landwerlin@intel.com> > Link to userspace changes? > > Regards, Joonas > Trying to get them uploaded to a branch on https://github.com/intel/metrics-discovery I'll let you know when it's available. -Lionel <html> <head> <meta http-equiv="Content-Type" content="text/html; charset=UTF-8"> </head> <body text="#000000" bgcolor="#FFFFFF"> <div class="moz-cite-prefix">On 22/01/2019 16:25, Joonas Lahtinen wrote:<br> </div> <blockquote type="cite" cite="mid:154817433953.8871.11566961163517410064@jlahtine-desk.ger.corp.intel.com"> <pre class="moz-quote-pre" wrap="">Quoting Lionel Landwerlin (2019-01-16 17:36:22) </pre> <blockquote type="cite" style="color: #000000;"> <pre class="moz-quote-pre" wrap="">With the currently available parameters for the i915-perf stream, there are still situations that are not well covered : If an application opens the stream with polling disable or at very low frequency and OA interrupt enabled, no data will be available even though somewhere between nothing and half of the OA buffer worth of data might have landed in memory. To solve this issue we have a new flush ioctl on the perf stream that forces the i915-perf driver to look at the state of the buffer when called and makes any data available through both poll() & read() type syscalls. Signed-off-by: Lionel Landwerlin <a class="moz-txt-link-rfc2396E" href="mailto:lionel.g.landwerlin@intel.com" moz-do-not-send="true"><lionel.g.landwerlin@intel.com></a> </pre> </blockquote> <pre class="moz-quote-pre" wrap="">Link to userspace changes? Regards, Joonas </pre> </blockquote> <p>Trying to get them uploaded to a branch on <a class="moz-txt-link-freetext" href="https://github.com/intel/metrics-discovery">https://github.com/intel/metrics-discovery</a></p> <p>I'll let you know when it's available.</p> <p><br> </p> <p>-Lionel</p> </body> </html>
On 22/01/2019 16:25, Joonas Lahtinen wrote: > Quoting Lionel Landwerlin (2019-01-16 17:36:22) >> With the currently available parameters for the i915-perf stream, >> there are still situations that are not well covered : >> >> If an application opens the stream with polling disable or at very low >> frequency and OA interrupt enabled, no data will be available even >> though somewhere between nothing and half of the OA buffer worth of >> data might have landed in memory. >> >> To solve this issue we have a new flush ioctl on the perf stream that >> forces the i915-perf driver to look at the state of the buffer when >> called and makes any data available through both poll() & read() type >> syscalls. >> >> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com> > Link to userspace changes? > > Regards, Joonas Hey Joonas, I'm about to make the changes in gputop for the high frequency sampling use case. One thing I would like to know is whether these new features should be reported through a flag. So far we haven't added any new option to the i915/perf driver since the initial upstreaming series. The way I'm currently detecting newly added parameters is by using trying to open the stream with a value that I know will report ENOENT rather than EINVAL when the feature is not available : https://github.com/djdeath/intel-gpu-tools/blob/wip/djdeath/oa-interrupts/tests/perf.c#L4345 Is there a better way to do this? Thanks, -Lionel > >> --- >> drivers/gpu/drm/i915/i915_perf.c | 17 +++++++++++++++++ >> include/uapi/drm/i915_drm.h | 19 +++++++++++++++++++ >> 2 files changed, 36 insertions(+) >> >> diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c >> index da721fce2543..6c98ffa2135e 100644 >> --- a/drivers/gpu/drm/i915/i915_perf.c >> +++ b/drivers/gpu/drm/i915/i915_perf.c >> @@ -2433,6 +2433,20 @@ static void i915_perf_disable_locked(struct i915_perf_stream *stream) >> stream->ops->disable(stream); >> } >> >> +/** >> + * i915_perf_flush_data - handle `I915_PERF_IOCTL_FLUSH_DATA` ioctl >> + * @stream: An enabled i915 perf stream >> + * >> + * The intention is to flush all the data available for reading from the OA >> + * buffer >> + */ >> +static void i915_perf_flush_data(struct i915_perf_stream *stream) >> +{ >> + struct drm_i915_private *dev_priv = stream->dev_priv; >> + >> + dev_priv->perf.oa.pollin = oa_buffer_check(stream->dev_priv, true); >> +} >> + >> /** >> * i915_perf_ioctl - support ioctl() usage with i915 perf stream FDs >> * @stream: An i915 perf stream >> @@ -2456,6 +2470,9 @@ static long i915_perf_ioctl_locked(struct i915_perf_stream *stream, >> case I915_PERF_IOCTL_DISABLE: >> i915_perf_disable_locked(stream); >> return 0; >> + case I915_PERF_IOCTL_FLUSH_DATA: >> + i915_perf_flush_data(stream); >> + return 0; >> } >> >> return -EINVAL; >> diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h >> index b6db5e544a35..0f0d20080572 100644 >> --- a/include/uapi/drm/i915_drm.h >> +++ b/include/uapi/drm/i915_drm.h >> @@ -1594,6 +1594,25 @@ struct drm_i915_perf_open_param { >> */ >> #define I915_PERF_IOCTL_DISABLE _IO('i', 0x1) >> >> +/** >> + * Actively check the availability of data from a stream. >> + * >> + * A stream data availability can be driven by two types of events : >> + * >> + * - if enabled, the kernel's hrtimer checking the amount of available data >> + * in the OA buffer through head/tail registers. >> + * >> + * - if enabled, the OA unit's interrupt mechanism >> + * >> + * The kernel hrtimer incur a cost of running callback at fixed time >> + * intervals, while the OA interrupt might only happen rarely. In the >> + * situation where the application has disabled the kernel's hrtimer and only >> + * uses the OA interrupt to know about available data, the application can >> + * request an active check of the available OA data through this ioctl. This >> + * will make any data in the OA buffer available with either poll() or read(). >> + */ >> +#define I915_PERF_IOCTL_FLUSH_DATA _IO('i', 0x2) >> + >> /** >> * Common to all i915 perf records >> */ >> -- >> 2.20.1 >> >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Quoting Lionel Landwerlin (2019-02-04 17:30:12) > On 22/01/2019 16:25, Joonas Lahtinen wrote: > > Quoting Lionel Landwerlin (2019-01-16 17:36:22) > >> With the currently available parameters for the i915-perf stream, > >> there are still situations that are not well covered : > >> > >> If an application opens the stream with polling disable or at very low > >> frequency and OA interrupt enabled, no data will be available even > >> though somewhere between nothing and half of the OA buffer worth of > >> data might have landed in memory. > >> > >> To solve this issue we have a new flush ioctl on the perf stream that > >> forces the i915-perf driver to look at the state of the buffer when > >> called and makes any data available through both poll() & read() type > >> syscalls. > >> > >> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com> > > Link to userspace changes? > > > > Regards, Joonas > > > Hey Joonas, > > > I'm about to make the changes in gputop for the high frequency sampling > use case. > > > One thing I would like to know is whether these new features should be > reported through a flag. > > So far we haven't added any new option to the i915/perf driver since the > initial upstreaming series. > > > The way I'm currently detecting newly added parameters is by using > trying to open the stream with a value that I know will report ENOENT > rather than EINVAL when the feature is not available : > > https://github.com/djdeath/intel-gpu-tools/blob/wip/djdeath/oa-interrupts/tests/perf.c#L4345 > > Is there a better way to do this? I think I already responded in IRC, but here goes for e-mail archives. A GETPARAM with rolling i915/perf version would probably be a justified versioning interface. Regards, Joonas > > > Thanks, > > > -Lionel > > > > > >> --- > >> drivers/gpu/drm/i915/i915_perf.c | 17 +++++++++++++++++ > >> include/uapi/drm/i915_drm.h | 19 +++++++++++++++++++ > >> 2 files changed, 36 insertions(+) > >> > >> diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c > >> index da721fce2543..6c98ffa2135e 100644 > >> --- a/drivers/gpu/drm/i915/i915_perf.c > >> +++ b/drivers/gpu/drm/i915/i915_perf.c > >> @@ -2433,6 +2433,20 @@ static void i915_perf_disable_locked(struct i915_perf_stream *stream) > >> stream->ops->disable(stream); > >> } > >> > >> +/** > >> + * i915_perf_flush_data - handle `I915_PERF_IOCTL_FLUSH_DATA` ioctl > >> + * @stream: An enabled i915 perf stream > >> + * > >> + * The intention is to flush all the data available for reading from the OA > >> + * buffer > >> + */ > >> +static void i915_perf_flush_data(struct i915_perf_stream *stream) > >> +{ > >> + struct drm_i915_private *dev_priv = stream->dev_priv; > >> + > >> + dev_priv->perf.oa.pollin = oa_buffer_check(stream->dev_priv, true); > >> +} > >> + > >> /** > >> * i915_perf_ioctl - support ioctl() usage with i915 perf stream FDs > >> * @stream: An i915 perf stream > >> @@ -2456,6 +2470,9 @@ static long i915_perf_ioctl_locked(struct i915_perf_stream *stream, > >> case I915_PERF_IOCTL_DISABLE: > >> i915_perf_disable_locked(stream); > >> return 0; > >> + case I915_PERF_IOCTL_FLUSH_DATA: > >> + i915_perf_flush_data(stream); > >> + return 0; > >> } > >> > >> return -EINVAL; > >> diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h > >> index b6db5e544a35..0f0d20080572 100644 > >> --- a/include/uapi/drm/i915_drm.h > >> +++ b/include/uapi/drm/i915_drm.h > >> @@ -1594,6 +1594,25 @@ struct drm_i915_perf_open_param { > >> */ > >> #define I915_PERF_IOCTL_DISABLE _IO('i', 0x1) > >> > >> +/** > >> + * Actively check the availability of data from a stream. > >> + * > >> + * A stream data availability can be driven by two types of events : > >> + * > >> + * - if enabled, the kernel's hrtimer checking the amount of available data > >> + * in the OA buffer through head/tail registers. > >> + * > >> + * - if enabled, the OA unit's interrupt mechanism > >> + * > >> + * The kernel hrtimer incur a cost of running callback at fixed time > >> + * intervals, while the OA interrupt might only happen rarely. In the > >> + * situation where the application has disabled the kernel's hrtimer and only > >> + * uses the OA interrupt to know about available data, the application can > >> + * request an active check of the available OA data through this ioctl. This > >> + * will make any data in the OA buffer available with either poll() or read(). > >> + */ > >> +#define I915_PERF_IOCTL_FLUSH_DATA _IO('i', 0x2) > >> + > >> /** > >> * Common to all i915 perf records > >> */ > >> -- > >> 2.20.1 > >> > >> _______________________________________________ > >> Intel-gfx mailing list > >> Intel-gfx@lists.freedesktop.org > >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx > >
diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c index da721fce2543..6c98ffa2135e 100644 --- a/drivers/gpu/drm/i915/i915_perf.c +++ b/drivers/gpu/drm/i915/i915_perf.c @@ -2433,6 +2433,20 @@ static void i915_perf_disable_locked(struct i915_perf_stream *stream) stream->ops->disable(stream); } +/** + * i915_perf_flush_data - handle `I915_PERF_IOCTL_FLUSH_DATA` ioctl + * @stream: An enabled i915 perf stream + * + * The intention is to flush all the data available for reading from the OA + * buffer + */ +static void i915_perf_flush_data(struct i915_perf_stream *stream) +{ + struct drm_i915_private *dev_priv = stream->dev_priv; + + dev_priv->perf.oa.pollin = oa_buffer_check(stream->dev_priv, true); +} + /** * i915_perf_ioctl - support ioctl() usage with i915 perf stream FDs * @stream: An i915 perf stream @@ -2456,6 +2470,9 @@ static long i915_perf_ioctl_locked(struct i915_perf_stream *stream, case I915_PERF_IOCTL_DISABLE: i915_perf_disable_locked(stream); return 0; + case I915_PERF_IOCTL_FLUSH_DATA: + i915_perf_flush_data(stream); + return 0; } return -EINVAL; diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h index b6db5e544a35..0f0d20080572 100644 --- a/include/uapi/drm/i915_drm.h +++ b/include/uapi/drm/i915_drm.h @@ -1594,6 +1594,25 @@ struct drm_i915_perf_open_param { */ #define I915_PERF_IOCTL_DISABLE _IO('i', 0x1) +/** + * Actively check the availability of data from a stream. + * + * A stream data availability can be driven by two types of events : + * + * - if enabled, the kernel's hrtimer checking the amount of available data + * in the OA buffer through head/tail registers. + * + * - if enabled, the OA unit's interrupt mechanism + * + * The kernel hrtimer incur a cost of running callback at fixed time + * intervals, while the OA interrupt might only happen rarely. In the + * situation where the application has disabled the kernel's hrtimer and only + * uses the OA interrupt to know about available data, the application can + * request an active check of the available OA data through this ioctl. This + * will make any data in the OA buffer available with either poll() or read(). + */ +#define I915_PERF_IOCTL_FLUSH_DATA _IO('i', 0x2) + /** * Common to all i915 perf records */
With the currently available parameters for the i915-perf stream, there are still situations that are not well covered : If an application opens the stream with polling disable or at very low frequency and OA interrupt enabled, no data will be available even though somewhere between nothing and half of the OA buffer worth of data might have landed in memory. To solve this issue we have a new flush ioctl on the perf stream that forces the i915-perf driver to look at the state of the buffer when called and makes any data available through both poll() & read() type syscalls. Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com> --- drivers/gpu/drm/i915/i915_perf.c | 17 +++++++++++++++++ include/uapi/drm/i915_drm.h | 19 +++++++++++++++++++ 2 files changed, 36 insertions(+)