| Message ID | 20161201172152.10893-1-robert@sixbynine.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Dec 01, 2016 at 05:21:52PM +0000, Robert Bragg wrote: > Avoid using DRM_ERROR for conditions userspace can trigger with a bad > config when opening a stream or from not reading data in a timely > fashion (whereby the OA buffer fills up). These conditions are tested > by i-g-t which treats error messages as failures if using the test > runner. This wasn't an issue while the i915-perf igt tests were being > run in isolation. > > One message relating to seeing a spurious zeroed report was changed to > use DRM_NOTE instead of DRM_ERROR. Ideally this warning shouldn't be > seen, but it's not a serious problem if it is. Considering that the > tail margin mechanism is only a heuristic it's possible we might see > this from time to time. > > Signed-off-by: Robert Bragg <robert@sixbynine.org: > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > > fix i915_perf dbg messages > --- > drivers/gpu/drm/i915/i915_perf.c | 42 ++++++++++++++++++++-------------------- > 1 file changed, 21 insertions(+), 21 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c > index 9551282..5705005 100644 > --- a/drivers/gpu/drm/i915/i915_perf.c > +++ b/drivers/gpu/drm/i915/i915_perf.c > @@ -474,7 +474,7 @@ static int gen7_append_oa_reports(struct i915_perf_stream *stream, > * copying it to userspace... > */ > if (report32[0] == 0) { > - DRM_ERROR("Skipping spurious, invalid OA report\n"); > + DRM_NOTE("Skipping spurious, invalid OA report\n"); > continue; The above looks like a genuine hw/kernel fail, which we shouldn't put under the carpet. I'd leave it at DRM_ERROR - I can bikeshed that while applying if you're ok. Otherwise lgtm, will apply as soon as we've clarified that. -Daniel > } > > @@ -551,7 +551,7 @@ static int gen7_oa_read(struct i915_perf_stream *stream, > if (ret) > return ret; > > - DRM_ERROR("OA buffer overflow: force restart\n"); > + DRM_DEBUG("OA buffer overflow: force restart\n"); > > dev_priv->perf.oa.ops.oa_disable(dev_priv); > dev_priv->perf.oa.ops.oa_enable(dev_priv); > @@ -1000,17 +1000,17 @@ static int i915_oa_stream_init(struct i915_perf_stream *stream, > * IDs > */ > if (!dev_priv->perf.metrics_kobj) { > - DRM_ERROR("OA metrics weren't advertised via sysfs\n"); > + DRM_DEBUG("OA metrics weren't advertised via sysfs\n"); > return -EINVAL; > } > > if (!(props->sample_flags & SAMPLE_OA_REPORT)) { > - DRM_ERROR("Only OA report sampling supported\n"); > + DRM_DEBUG("Only OA report sampling supported\n"); > return -EINVAL; > } > > if (!dev_priv->perf.oa.ops.init_oa_buffer) { > - DRM_ERROR("OA unit not supported\n"); > + DRM_DEBUG("OA unit not supported\n"); > return -ENODEV; > } > > @@ -1019,17 +1019,17 @@ static int i915_oa_stream_init(struct i915_perf_stream *stream, > * we currently only allow exclusive access > */ > if (dev_priv->perf.oa.exclusive_stream) { > - DRM_ERROR("OA unit already in use\n"); > + DRM_DEBUG("OA unit already in use\n"); > return -EBUSY; > } > > if (!props->metrics_set) { > - DRM_ERROR("OA metric set not specified\n"); > + DRM_DEBUG("OA metric set not specified\n"); > return -EINVAL; > } > > if (!props->oa_format) { > - DRM_ERROR("OA report format not specified\n"); > + DRM_DEBUG("OA report format not specified\n"); > return -EINVAL; > } > > @@ -1384,7 +1384,7 @@ i915_perf_open_ioctl_locked(struct drm_i915_private *dev_priv, > if (IS_ERR(specific_ctx)) { > ret = PTR_ERR(specific_ctx); > if (ret != -EINTR) > - DRM_ERROR("Failed to look up context with ID %u for opening perf stream\n", > + DRM_DEBUG("Failed to look up context with ID %u for opening perf stream\n", > ctx_handle); > goto err; > } > @@ -1397,7 +1397,7 @@ i915_perf_open_ioctl_locked(struct drm_i915_private *dev_priv, > */ > if (!specific_ctx && > i915_perf_stream_paranoid && !capable(CAP_SYS_ADMIN)) { > - DRM_ERROR("Insufficient privileges to open system-wide i915 perf stream\n"); > + DRM_DEBUG("Insufficient privileges to open system-wide i915 perf stream\n"); > ret = -EACCES; > goto err_ctx; > } > @@ -1476,7 +1476,7 @@ static int read_properties_unlocked(struct drm_i915_private *dev_priv, > memset(props, 0, sizeof(struct perf_open_properties)); > > if (!n_props) { > - DRM_ERROR("No i915 perf properties given"); > + DRM_DEBUG("No i915 perf properties given\n"); > return -EINVAL; > } > > @@ -1487,7 +1487,7 @@ static int read_properties_unlocked(struct drm_i915_private *dev_priv, > * from userspace. > */ > if (n_props >= DRM_I915_PERF_PROP_MAX) { > - DRM_ERROR("More i915 perf properties specified than exist"); > + DRM_DEBUG("More i915 perf properties specified than exist\n"); > return -EINVAL; > } > > @@ -1515,26 +1515,26 @@ static int read_properties_unlocked(struct drm_i915_private *dev_priv, > case DRM_I915_PERF_PROP_OA_METRICS_SET: > if (value == 0 || > value > dev_priv->perf.oa.n_builtin_sets) { > - DRM_ERROR("Unknown OA metric set ID"); > + DRM_DEBUG("Unknown OA metric set ID\n"); > return -EINVAL; > } > props->metrics_set = value; > break; > case DRM_I915_PERF_PROP_OA_FORMAT: > if (value == 0 || value >= I915_OA_FORMAT_MAX) { > - DRM_ERROR("Invalid OA report format\n"); > + DRM_DEBUG("Invalid OA report format\n"); > return -EINVAL; > } > if (!dev_priv->perf.oa.oa_formats[value].size) { > - DRM_ERROR("Invalid OA report format\n"); > + DRM_DEBUG("Invalid OA report format\n"); > return -EINVAL; > } > props->oa_format = value; > break; > case DRM_I915_PERF_PROP_OA_EXPONENT: > if (value > OA_EXPONENT_MAX) { > - DRM_ERROR("OA timer exponent too high (> %u)\n", > - OA_EXPONENT_MAX); > + DRM_DEBUG("OA timer exponent too high (> %u)\n", > + OA_EXPONENT_MAX); > return -EINVAL; > } > > @@ -1565,7 +1565,7 @@ static int read_properties_unlocked(struct drm_i915_private *dev_priv, > > if (oa_freq_hz > i915_oa_max_sample_rate && > !capable(CAP_SYS_ADMIN)) { > - DRM_ERROR("OA exponent would exceed the max sampling frequency (sysctl dev.i915.oa_max_sample_rate) %uHz without root privileges\n", > + DRM_DEBUG("OA exponent would exceed the max sampling frequency (sysctl dev.i915.oa_max_sample_rate) %uHz without root privileges\n", > i915_oa_max_sample_rate); > return -EACCES; > } > @@ -1575,7 +1575,7 @@ static int read_properties_unlocked(struct drm_i915_private *dev_priv, > break; > default: > MISSING_CASE(id); > - DRM_ERROR("Unknown i915 perf property ID"); > + DRM_DEBUG("Unknown i915 perf property ID\n"); > return -EINVAL; > } > > @@ -1595,7 +1595,7 @@ int i915_perf_open_ioctl(struct drm_device *dev, void *data, > int ret; > > if (!dev_priv->perf.initialized) { > - DRM_ERROR("i915 perf interface not available for this system"); > + DRM_DEBUG("i915 perf interface not available for this system\n"); > return -ENOTSUPP; > } > > @@ -1603,7 +1603,7 @@ int i915_perf_open_ioctl(struct drm_device *dev, void *data, > I915_PERF_FLAG_FD_NONBLOCK | > I915_PERF_FLAG_DISABLED; > if (param->flags & ~known_open_flags) { > - DRM_ERROR("Unknown drm_i915_perf_open_param flag\n"); > + DRM_DEBUG("Unknown drm_i915_perf_open_param flag\n"); > return -EINVAL; > } > > -- > 2.10.2 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Fri, Dec 2, 2016 at 8:35 AM, Daniel Vetter <daniel@ffwll.ch> wrote: > On Thu, Dec 01, 2016 at 05:21:52PM +0000, Robert Bragg wrote: > > Avoid using DRM_ERROR for conditions userspace can trigger with a bad > > config when opening a stream or from not reading data in a timely > > fashion (whereby the OA buffer fills up). These conditions are tested > > by i-g-t which treats error messages as failures if using the test > > runner. This wasn't an issue while the i915-perf igt tests were being > > run in isolation. > > > > One message relating to seeing a spurious zeroed report was changed to > > use DRM_NOTE instead of DRM_ERROR. Ideally this warning shouldn't be > > seen, but it's not a serious problem if it is. Considering that the > > tail margin mechanism is only a heuristic it's possible we might see > > this from time to time. > > > > Signed-off-by: Robert Bragg <robert@sixbynine.org: > > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > > > > fix i915_perf dbg messages > > --- > > drivers/gpu/drm/i915/i915_perf.c | 42 ++++++++++++++++++++---------- > ---------- > > 1 file changed, 21 insertions(+), 21 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_perf.c > b/drivers/gpu/drm/i915/i915_perf.c > > index 9551282..5705005 100644 > > --- a/drivers/gpu/drm/i915/i915_perf.c > > +++ b/drivers/gpu/drm/i915/i915_perf.c > > @@ -474,7 +474,7 @@ static int gen7_append_oa_reports(struct > i915_perf_stream *stream, > > * copying it to userspace... > > */ > > if (report32[0] == 0) { > > - DRM_ERROR("Skipping spurious, invalid OA > report\n"); > > + DRM_NOTE("Skipping spurious, invalid OA report\n"); > > continue; > > The above looks like a genuine hw/kernel fail, which we shouldn't put > under the carpet. I'd leave it at DRM_ERROR - I can bikeshed that while > applying if you're ok. Otherwise lgtm, will apply as soon as we've > clarified that. > It's something that is unfortunately expected to be possible from time to time due to a hardware race condition between the OA unit updating the tail pointer for a new report and that report actually becoming visible to the cpu in memory. If/when it happens it's not really a significant problem for userspace (assuming it's rare/intermittent given what the driver does as a best-effort workaround here). Userspace sees a briefly lower sampling resolution but the metrics can still be normalized. We wouldn't want i-g-t failing in this case, so that's why I was changing it. It's not really something you want to see ideally (it implies our heuristic-based software workaround isn't perfect). If it's seen a lot then that certainly should be considered a warning that we need to try and improve how we workaround the race condition. If you see it rarely then is somewhere between a note, and a warning I suppose. Regards, - Robert > -Daniel > > > } > > > > @@ -551,7 +551,7 @@ static int gen7_oa_read(struct i915_perf_stream > *stream, > > if (ret) > > return ret; > > > > - DRM_ERROR("OA buffer overflow: force restart\n"); > > + DRM_DEBUG("OA buffer overflow: force restart\n"); > > > > dev_priv->perf.oa.ops.oa_disable(dev_priv); > > dev_priv->perf.oa.ops.oa_enable(dev_priv); > > @@ -1000,17 +1000,17 @@ static int i915_oa_stream_init(struct > i915_perf_stream *stream, > > * IDs > > */ > > if (!dev_priv->perf.metrics_kobj) { > > - DRM_ERROR("OA metrics weren't advertised via sysfs\n"); > > + DRM_DEBUG("OA metrics weren't advertised via sysfs\n"); > > return -EINVAL; > > } > > > > if (!(props->sample_flags & SAMPLE_OA_REPORT)) { > > - DRM_ERROR("Only OA report sampling supported\n"); > > + DRM_DEBUG("Only OA report sampling supported\n"); > > return -EINVAL; > > } > > > > if (!dev_priv->perf.oa.ops.init_oa_buffer) { > > - DRM_ERROR("OA unit not supported\n"); > > + DRM_DEBUG("OA unit not supported\n"); > > return -ENODEV; > > } > > > > @@ -1019,17 +1019,17 @@ static int i915_oa_stream_init(struct > i915_perf_stream *stream, > > * we currently only allow exclusive access > > */ > > if (dev_priv->perf.oa.exclusive_stream) { > > - DRM_ERROR("OA unit already in use\n"); > > + DRM_DEBUG("OA unit already in use\n"); > > return -EBUSY; > > } > > > > if (!props->metrics_set) { > > - DRM_ERROR("OA metric set not specified\n"); > > + DRM_DEBUG("OA metric set not specified\n"); > > return -EINVAL; > > } > > > > if (!props->oa_format) { > > - DRM_ERROR("OA report format not specified\n"); > > + DRM_DEBUG("OA report format not specified\n"); > > return -EINVAL; > > } > > > > @@ -1384,7 +1384,7 @@ i915_perf_open_ioctl_locked(struct > drm_i915_private *dev_priv, > > if (IS_ERR(specific_ctx)) { > > ret = PTR_ERR(specific_ctx); > > if (ret != -EINTR) > > - DRM_ERROR("Failed to look up context with > ID %u for opening perf stream\n", > > + DRM_DEBUG("Failed to look up context with > ID %u for opening perf stream\n", > > ctx_handle); > > goto err; > > } > > @@ -1397,7 +1397,7 @@ i915_perf_open_ioctl_locked(struct > drm_i915_private *dev_priv, > > */ > > if (!specific_ctx && > > i915_perf_stream_paranoid && !capable(CAP_SYS_ADMIN)) { > > - DRM_ERROR("Insufficient privileges to open system-wide > i915 perf stream\n"); > > + DRM_DEBUG("Insufficient privileges to open system-wide > i915 perf stream\n"); > > ret = -EACCES; > > goto err_ctx; > > } > > @@ -1476,7 +1476,7 @@ static int read_properties_unlocked(struct > drm_i915_private *dev_priv, > > memset(props, 0, sizeof(struct perf_open_properties)); > > > > if (!n_props) { > > - DRM_ERROR("No i915 perf properties given"); > > + DRM_DEBUG("No i915 perf properties given\n"); > > return -EINVAL; > > } > > > > @@ -1487,7 +1487,7 @@ static int read_properties_unlocked(struct > drm_i915_private *dev_priv, > > * from userspace. > > */ > > if (n_props >= DRM_I915_PERF_PROP_MAX) { > > - DRM_ERROR("More i915 perf properties specified than > exist"); > > + DRM_DEBUG("More i915 perf properties specified than > exist\n"); > > return -EINVAL; > > } > > > > @@ -1515,26 +1515,26 @@ static int read_properties_unlocked(struct > drm_i915_private *dev_priv, > > case DRM_I915_PERF_PROP_OA_METRICS_SET: > > if (value == 0 || > > value > dev_priv->perf.oa.n_builtin_sets) { > > - DRM_ERROR("Unknown OA metric set ID"); > > + DRM_DEBUG("Unknown OA metric set ID\n"); > > return -EINVAL; > > } > > props->metrics_set = value; > > break; > > case DRM_I915_PERF_PROP_OA_FORMAT: > > if (value == 0 || value >= I915_OA_FORMAT_MAX) { > > - DRM_ERROR("Invalid OA report format\n"); > > + DRM_DEBUG("Invalid OA report format\n"); > > return -EINVAL; > > } > > if (!dev_priv->perf.oa.oa_formats[value].size) { > > - DRM_ERROR("Invalid OA report format\n"); > > + DRM_DEBUG("Invalid OA report format\n"); > > return -EINVAL; > > } > > props->oa_format = value; > > break; > > case DRM_I915_PERF_PROP_OA_EXPONENT: > > if (value > OA_EXPONENT_MAX) { > > - DRM_ERROR("OA timer exponent too high (> > %u)\n", > > - OA_EXPONENT_MAX); > > + DRM_DEBUG("OA timer exponent too high (> > %u)\n", > > + OA_EXPONENT_MAX); > > return -EINVAL; > > } > > > > @@ -1565,7 +1565,7 @@ static int read_properties_unlocked(struct > drm_i915_private *dev_priv, > > > > if (oa_freq_hz > i915_oa_max_sample_rate && > > !capable(CAP_SYS_ADMIN)) { > > - DRM_ERROR("OA exponent would exceed the > max sampling frequency (sysctl dev.i915.oa_max_sample_rate) %uHz without > root privileges\n", > > + DRM_DEBUG("OA exponent would exceed the > max sampling frequency (sysctl dev.i915.oa_max_sample_rate) %uHz without > root privileges\n", > > i915_oa_max_sample_rate); > > return -EACCES; > > } > > @@ -1575,7 +1575,7 @@ static int read_properties_unlocked(struct > drm_i915_private *dev_priv, > > break; > > default: > > MISSING_CASE(id); > > - DRM_ERROR("Unknown i915 perf property ID"); > > + DRM_DEBUG("Unknown i915 perf property ID\n"); > > return -EINVAL; > > } > > > > @@ -1595,7 +1595,7 @@ int i915_perf_open_ioctl(struct drm_device *dev, > void *data, > > int ret; > > > > if (!dev_priv->perf.initialized) { > > - DRM_ERROR("i915 perf interface not available for this > system"); > > + DRM_DEBUG("i915 perf interface not available for this > system\n"); > > return -ENOTSUPP; > > } > > > > @@ -1603,7 +1603,7 @@ int i915_perf_open_ioctl(struct drm_device *dev, > void *data, > > I915_PERF_FLAG_FD_NONBLOCK | > > I915_PERF_FLAG_DISABLED; > > if (param->flags & ~known_open_flags) { > > - DRM_ERROR("Unknown drm_i915_perf_open_param flag\n"); > > + DRM_DEBUG("Unknown drm_i915_perf_open_param flag\n"); > > return -EINVAL; > > } > > > > -- > > 2.10.2 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch >
On Fri, Dec 02, 2016 at 12:18:44PM +0000, Robert Bragg wrote: > On Fri, Dec 2, 2016 at 8:35 AM, Daniel Vetter <daniel@ffwll.ch> wrote: > > > On Thu, Dec 01, 2016 at 05:21:52PM +0000, Robert Bragg wrote: > > > Avoid using DRM_ERROR for conditions userspace can trigger with a bad > > > config when opening a stream or from not reading data in a timely > > > fashion (whereby the OA buffer fills up). These conditions are tested > > > by i-g-t which treats error messages as failures if using the test > > > runner. This wasn't an issue while the i915-perf igt tests were being > > > run in isolation. > > > > > > One message relating to seeing a spurious zeroed report was changed to > > > use DRM_NOTE instead of DRM_ERROR. Ideally this warning shouldn't be > > > seen, but it's not a serious problem if it is. Considering that the > > > tail margin mechanism is only a heuristic it's possible we might see > > > this from time to time. > > > > > > Signed-off-by: Robert Bragg <robert@sixbynine.org: > > > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > > > > > > fix i915_perf dbg messages > > > --- > > > drivers/gpu/drm/i915/i915_perf.c | 42 ++++++++++++++++++++---------- > > ---------- > > > 1 file changed, 21 insertions(+), 21 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/i915_perf.c > > b/drivers/gpu/drm/i915/i915_perf.c > > > index 9551282..5705005 100644 > > > --- a/drivers/gpu/drm/i915/i915_perf.c > > > +++ b/drivers/gpu/drm/i915/i915_perf.c > > > @@ -474,7 +474,7 @@ static int gen7_append_oa_reports(struct > > i915_perf_stream *stream, > > > * copying it to userspace... > > > */ > > > if (report32[0] == 0) { > > > - DRM_ERROR("Skipping spurious, invalid OA > > report\n"); > > > + DRM_NOTE("Skipping spurious, invalid OA report\n"); > > > continue; > > > > The above looks like a genuine hw/kernel fail, which we shouldn't put > > under the carpet. I'd leave it at DRM_ERROR - I can bikeshed that while > > applying if you're ok. Otherwise lgtm, will apply as soon as we've > > clarified that. > > > > It's something that is unfortunately expected to be possible from time to > time due to a hardware race condition between the OA unit updating the tail > pointer for a new report and that report actually becoming visible to the > cpu in memory. > > If/when it happens it's not really a significant problem for userspace > (assuming it's rare/intermittent given what the driver does as a > best-effort workaround here). Userspace sees a briefly lower sampling > resolution but the metrics can still be normalized. > > We wouldn't want i-g-t failing in this case, so that's why I was changing > it. > > It's not really something you want to see ideally (it implies our > heuristic-based software workaround isn't perfect). If it's seen a lot then > that certainly should be considered a warning that we need to try and > improve how we workaround the race condition. If you see it rarely then is > somewhere between a note, and a warning I suppose. Ok makes sense, patch applied. -Daniel > > Regards, > - Robert > > > > -Daniel > > > > > } > > > > > > @@ -551,7 +551,7 @@ static int gen7_oa_read(struct i915_perf_stream > > *stream, > > > if (ret) > > > return ret; > > > > > > - DRM_ERROR("OA buffer overflow: force restart\n"); > > > + DRM_DEBUG("OA buffer overflow: force restart\n"); > > > > > > dev_priv->perf.oa.ops.oa_disable(dev_priv); > > > dev_priv->perf.oa.ops.oa_enable(dev_priv); > > > @@ -1000,17 +1000,17 @@ static int i915_oa_stream_init(struct > > i915_perf_stream *stream, > > > * IDs > > > */ > > > if (!dev_priv->perf.metrics_kobj) { > > > - DRM_ERROR("OA metrics weren't advertised via sysfs\n"); > > > + DRM_DEBUG("OA metrics weren't advertised via sysfs\n"); > > > return -EINVAL; > > > } > > > > > > if (!(props->sample_flags & SAMPLE_OA_REPORT)) { > > > - DRM_ERROR("Only OA report sampling supported\n"); > > > + DRM_DEBUG("Only OA report sampling supported\n"); > > > return -EINVAL; > > > } > > > > > > if (!dev_priv->perf.oa.ops.init_oa_buffer) { > > > - DRM_ERROR("OA unit not supported\n"); > > > + DRM_DEBUG("OA unit not supported\n"); > > > return -ENODEV; > > > } > > > > > > @@ -1019,17 +1019,17 @@ static int i915_oa_stream_init(struct > > i915_perf_stream *stream, > > > * we currently only allow exclusive access > > > */ > > > if (dev_priv->perf.oa.exclusive_stream) { > > > - DRM_ERROR("OA unit already in use\n"); > > > + DRM_DEBUG("OA unit already in use\n"); > > > return -EBUSY; > > > } > > > > > > if (!props->metrics_set) { > > > - DRM_ERROR("OA metric set not specified\n"); > > > + DRM_DEBUG("OA metric set not specified\n"); > > > return -EINVAL; > > > } > > > > > > if (!props->oa_format) { > > > - DRM_ERROR("OA report format not specified\n"); > > > + DRM_DEBUG("OA report format not specified\n"); > > > return -EINVAL; > > > } > > > > > > @@ -1384,7 +1384,7 @@ i915_perf_open_ioctl_locked(struct > > drm_i915_private *dev_priv, > > > if (IS_ERR(specific_ctx)) { > > > ret = PTR_ERR(specific_ctx); > > > if (ret != -EINTR) > > > - DRM_ERROR("Failed to look up context with > > ID %u for opening perf stream\n", > > > + DRM_DEBUG("Failed to look up context with > > ID %u for opening perf stream\n", > > > ctx_handle); > > > goto err; > > > } > > > @@ -1397,7 +1397,7 @@ i915_perf_open_ioctl_locked(struct > > drm_i915_private *dev_priv, > > > */ > > > if (!specific_ctx && > > > i915_perf_stream_paranoid && !capable(CAP_SYS_ADMIN)) { > > > - DRM_ERROR("Insufficient privileges to open system-wide > > i915 perf stream\n"); > > > + DRM_DEBUG("Insufficient privileges to open system-wide > > i915 perf stream\n"); > > > ret = -EACCES; > > > goto err_ctx; > > > } > > > @@ -1476,7 +1476,7 @@ static int read_properties_unlocked(struct > > drm_i915_private *dev_priv, > > > memset(props, 0, sizeof(struct perf_open_properties)); > > > > > > if (!n_props) { > > > - DRM_ERROR("No i915 perf properties given"); > > > + DRM_DEBUG("No i915 perf properties given\n"); > > > return -EINVAL; > > > } > > > > > > @@ -1487,7 +1487,7 @@ static int read_properties_unlocked(struct > > drm_i915_private *dev_priv, > > > * from userspace. > > > */ > > > if (n_props >= DRM_I915_PERF_PROP_MAX) { > > > - DRM_ERROR("More i915 perf properties specified than > > exist"); > > > + DRM_DEBUG("More i915 perf properties specified than > > exist\n"); > > > return -EINVAL; > > > } > > > > > > @@ -1515,26 +1515,26 @@ static int read_properties_unlocked(struct > > drm_i915_private *dev_priv, > > > case DRM_I915_PERF_PROP_OA_METRICS_SET: > > > if (value == 0 || > > > value > dev_priv->perf.oa.n_builtin_sets) { > > > - DRM_ERROR("Unknown OA metric set ID"); > > > + DRM_DEBUG("Unknown OA metric set ID\n"); > > > return -EINVAL; > > > } > > > props->metrics_set = value; > > > break; > > > case DRM_I915_PERF_PROP_OA_FORMAT: > > > if (value == 0 || value >= I915_OA_FORMAT_MAX) { > > > - DRM_ERROR("Invalid OA report format\n"); > > > + DRM_DEBUG("Invalid OA report format\n"); > > > return -EINVAL; > > > } > > > if (!dev_priv->perf.oa.oa_formats[value].size) { > > > - DRM_ERROR("Invalid OA report format\n"); > > > + DRM_DEBUG("Invalid OA report format\n"); > > > return -EINVAL; > > > } > > > props->oa_format = value; > > > break; > > > case DRM_I915_PERF_PROP_OA_EXPONENT: > > > if (value > OA_EXPONENT_MAX) { > > > - DRM_ERROR("OA timer exponent too high (> > > %u)\n", > > > - OA_EXPONENT_MAX); > > > + DRM_DEBUG("OA timer exponent too high (> > > %u)\n", > > > + OA_EXPONENT_MAX); > > > return -EINVAL; > > > } > > > > > > @@ -1565,7 +1565,7 @@ static int read_properties_unlocked(struct > > drm_i915_private *dev_priv, > > > > > > if (oa_freq_hz > i915_oa_max_sample_rate && > > > !capable(CAP_SYS_ADMIN)) { > > > - DRM_ERROR("OA exponent would exceed the > > max sampling frequency (sysctl dev.i915.oa_max_sample_rate) %uHz without > > root privileges\n", > > > + DRM_DEBUG("OA exponent would exceed the > > max sampling frequency (sysctl dev.i915.oa_max_sample_rate) %uHz without > > root privileges\n", > > > i915_oa_max_sample_rate); > > > return -EACCES; > > > } > > > @@ -1575,7 +1575,7 @@ static int read_properties_unlocked(struct > > drm_i915_private *dev_priv, > > > break; > > > default: > > > MISSING_CASE(id); > > > - DRM_ERROR("Unknown i915 perf property ID"); > > > + DRM_DEBUG("Unknown i915 perf property ID\n"); > > > return -EINVAL; > > > } > > > > > > @@ -1595,7 +1595,7 @@ int i915_perf_open_ioctl(struct drm_device *dev, > > void *data, > > > int ret; > > > > > > if (!dev_priv->perf.initialized) { > > > - DRM_ERROR("i915 perf interface not available for this > > system"); > > > + DRM_DEBUG("i915 perf interface not available for this > > system\n"); > > > return -ENOTSUPP; > > > } > > > > > > @@ -1603,7 +1603,7 @@ int i915_perf_open_ioctl(struct drm_device *dev, > > void *data, > > > I915_PERF_FLAG_FD_NONBLOCK | > > > I915_PERF_FLAG_DISABLED; > > > if (param->flags & ~known_open_flags) { > > > - DRM_ERROR("Unknown drm_i915_perf_open_param flag\n"); > > > + DRM_DEBUG("Unknown drm_i915_perf_open_param flag\n"); > > > return -EINVAL; > > > } > > > > > > -- > > > 2.10.2 > > > > > > _______________________________________________ > > > Intel-gfx mailing list > > > Intel-gfx@lists.freedesktop.org > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > > > -- > > Daniel Vetter > > Software Engineer, Intel Corporation > > http://blog.ffwll.ch > >
diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c index 9551282..5705005 100644 --- a/drivers/gpu/drm/i915/i915_perf.c +++ b/drivers/gpu/drm/i915/i915_perf.c @@ -474,7 +474,7 @@ static int gen7_append_oa_reports(struct i915_perf_stream *stream, * copying it to userspace... */ if (report32[0] == 0) { - DRM_ERROR("Skipping spurious, invalid OA report\n"); + DRM_NOTE("Skipping spurious, invalid OA report\n"); continue; } @@ -551,7 +551,7 @@ static int gen7_oa_read(struct i915_perf_stream *stream, if (ret) return ret; - DRM_ERROR("OA buffer overflow: force restart\n"); + DRM_DEBUG("OA buffer overflow: force restart\n"); dev_priv->perf.oa.ops.oa_disable(dev_priv); dev_priv->perf.oa.ops.oa_enable(dev_priv); @@ -1000,17 +1000,17 @@ static int i915_oa_stream_init(struct i915_perf_stream *stream, * IDs */ if (!dev_priv->perf.metrics_kobj) { - DRM_ERROR("OA metrics weren't advertised via sysfs\n"); + DRM_DEBUG("OA metrics weren't advertised via sysfs\n"); return -EINVAL; } if (!(props->sample_flags & SAMPLE_OA_REPORT)) { - DRM_ERROR("Only OA report sampling supported\n"); + DRM_DEBUG("Only OA report sampling supported\n"); return -EINVAL; } if (!dev_priv->perf.oa.ops.init_oa_buffer) { - DRM_ERROR("OA unit not supported\n"); + DRM_DEBUG("OA unit not supported\n"); return -ENODEV; } @@ -1019,17 +1019,17 @@ static int i915_oa_stream_init(struct i915_perf_stream *stream, * we currently only allow exclusive access */ if (dev_priv->perf.oa.exclusive_stream) { - DRM_ERROR("OA unit already in use\n"); + DRM_DEBUG("OA unit already in use\n"); return -EBUSY; } if (!props->metrics_set) { - DRM_ERROR("OA metric set not specified\n"); + DRM_DEBUG("OA metric set not specified\n"); return -EINVAL; } if (!props->oa_format) { - DRM_ERROR("OA report format not specified\n"); + DRM_DEBUG("OA report format not specified\n"); return -EINVAL; } @@ -1384,7 +1384,7 @@ i915_perf_open_ioctl_locked(struct drm_i915_private *dev_priv, if (IS_ERR(specific_ctx)) { ret = PTR_ERR(specific_ctx); if (ret != -EINTR) - DRM_ERROR("Failed to look up context with ID %u for opening perf stream\n", + DRM_DEBUG("Failed to look up context with ID %u for opening perf stream\n", ctx_handle); goto err; } @@ -1397,7 +1397,7 @@ i915_perf_open_ioctl_locked(struct drm_i915_private *dev_priv, */ if (!specific_ctx && i915_perf_stream_paranoid && !capable(CAP_SYS_ADMIN)) { - DRM_ERROR("Insufficient privileges to open system-wide i915 perf stream\n"); + DRM_DEBUG("Insufficient privileges to open system-wide i915 perf stream\n"); ret = -EACCES; goto err_ctx; } @@ -1476,7 +1476,7 @@ static int read_properties_unlocked(struct drm_i915_private *dev_priv, memset(props, 0, sizeof(struct perf_open_properties)); if (!n_props) { - DRM_ERROR("No i915 perf properties given"); + DRM_DEBUG("No i915 perf properties given\n"); return -EINVAL; } @@ -1487,7 +1487,7 @@ static int read_properties_unlocked(struct drm_i915_private *dev_priv, * from userspace. */ if (n_props >= DRM_I915_PERF_PROP_MAX) { - DRM_ERROR("More i915 perf properties specified than exist"); + DRM_DEBUG("More i915 perf properties specified than exist\n"); return -EINVAL; } @@ -1515,26 +1515,26 @@ static int read_properties_unlocked(struct drm_i915_private *dev_priv, case DRM_I915_PERF_PROP_OA_METRICS_SET: if (value == 0 || value > dev_priv->perf.oa.n_builtin_sets) { - DRM_ERROR("Unknown OA metric set ID"); + DRM_DEBUG("Unknown OA metric set ID\n"); return -EINVAL; } props->metrics_set = value; break; case DRM_I915_PERF_PROP_OA_FORMAT: if (value == 0 || value >= I915_OA_FORMAT_MAX) { - DRM_ERROR("Invalid OA report format\n"); + DRM_DEBUG("Invalid OA report format\n"); return -EINVAL; } if (!dev_priv->perf.oa.oa_formats[value].size) { - DRM_ERROR("Invalid OA report format\n"); + DRM_DEBUG("Invalid OA report format\n"); return -EINVAL; } props->oa_format = value; break; case DRM_I915_PERF_PROP_OA_EXPONENT: if (value > OA_EXPONENT_MAX) { - DRM_ERROR("OA timer exponent too high (> %u)\n", - OA_EXPONENT_MAX); + DRM_DEBUG("OA timer exponent too high (> %u)\n", + OA_EXPONENT_MAX); return -EINVAL; } @@ -1565,7 +1565,7 @@ static int read_properties_unlocked(struct drm_i915_private *dev_priv, if (oa_freq_hz > i915_oa_max_sample_rate && !capable(CAP_SYS_ADMIN)) { - DRM_ERROR("OA exponent would exceed the max sampling frequency (sysctl dev.i915.oa_max_sample_rate) %uHz without root privileges\n", + DRM_DEBUG("OA exponent would exceed the max sampling frequency (sysctl dev.i915.oa_max_sample_rate) %uHz without root privileges\n", i915_oa_max_sample_rate); return -EACCES; } @@ -1575,7 +1575,7 @@ static int read_properties_unlocked(struct drm_i915_private *dev_priv, break; default: MISSING_CASE(id); - DRM_ERROR("Unknown i915 perf property ID"); + DRM_DEBUG("Unknown i915 perf property ID\n"); return -EINVAL; } @@ -1595,7 +1595,7 @@ int i915_perf_open_ioctl(struct drm_device *dev, void *data, int ret; if (!dev_priv->perf.initialized) { - DRM_ERROR("i915 perf interface not available for this system"); + DRM_DEBUG("i915 perf interface not available for this system\n"); return -ENOTSUPP; } @@ -1603,7 +1603,7 @@ int i915_perf_open_ioctl(struct drm_device *dev, void *data, I915_PERF_FLAG_FD_NONBLOCK | I915_PERF_FLAG_DISABLED; if (param->flags & ~known_open_flags) { - DRM_ERROR("Unknown drm_i915_perf_open_param flag\n"); + DRM_DEBUG("Unknown drm_i915_perf_open_param flag\n"); return -EINVAL; }
Avoid using DRM_ERROR for conditions userspace can trigger with a bad config when opening a stream or from not reading data in a timely fashion (whereby the OA buffer fills up). These conditions are tested by i-g-t which treats error messages as failures if using the test runner. This wasn't an issue while the i915-perf igt tests were being run in isolation. One message relating to seeing a spurious zeroed report was changed to use DRM_NOTE instead of DRM_ERROR. Ideally this warning shouldn't be seen, but it's not a serious problem if it is. Considering that the tail margin mechanism is only a heuristic it's possible we might see this from time to time. Signed-off-by: Robert Bragg <robert@sixbynine.org: Cc: Daniel Vetter <daniel.vetter@ffwll.ch> fix i915_perf dbg messages --- drivers/gpu/drm/i915/i915_perf.c | 42 ++++++++++++++++++++-------------------- 1 file changed, 21 insertions(+), 21 deletions(-)