> On Dec 11, 2019, at 7:25 AM, Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx> wrote: > > Quoting Jani Nikula (2019-12-11 12:36:10) >> On Fri, 15 Nov 2019, Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> wrote: >>> Quoting Jani Nikula (2019-11-15 11:04:28) >>>> On Fri, 15 Nov 2019, Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> wrote: >>>>> Quoting Jani Nikula (2019-11-15 10:18:40) >>>>>> Get rid of the module specific static variable. >>>>>> >>>>>> Signed-off-by: Jani Nikula <jani.nikula@xxxxxxxxx> >>>>>> --- >>>>>> drivers/gpu/drm/i915/i915_drv.h | 2 ++ >>>>>> drivers/gpu/drm/i915/i915_utils.c | 9 ++++----- >>>>>> 2 files changed, 6 insertions(+), 5 deletions(-) >>>>>> >>>>>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h >>>>>> index 1779f600fcfb..e11ee3268ae3 100644 >>>>>> --- a/drivers/gpu/drm/i915/i915_drv.h >>>>>> +++ b/drivers/gpu/drm/i915/i915_drv.h >>>>>> @@ -1283,6 +1283,8 @@ struct drm_i915_private { >>>>>> /* Mutex to protect the above hdcp component related values. */ >>>>>> struct mutex hdcp_comp_mutex; >>>>>> >>>>>> + bool shown_bug_once; >>>>>> + >>>>>> I915_SELFTEST_DECLARE(struct i915_selftest_stash selftest;) >>>>>> >>>>>> /* >>>>>> diff --git a/drivers/gpu/drm/i915/i915_utils.c b/drivers/gpu/drm/i915/i915_utils.c >>>>>> index c47261ae86ea..f434274b0b29 100644 >>>>>> --- a/drivers/gpu/drm/i915/i915_utils.c >>>>>> +++ b/drivers/gpu/drm/i915/i915_utils.c >>>>>> @@ -13,11 +13,10 @@ >>>>>> "providing the dmesg log by booting with drm.debug=0xf" >>>>>> >>>>>> void >>>>>> -__i915_printk(struct drm_i915_private *dev_priv, const char *level, >>>>>> +__i915_printk(struct drm_i915_private *i915, const char *level, >>>>>> const char *fmt, ...) >>>>>> { >>>>>> - static bool shown_bug_once; >>>>>> - struct device *kdev = dev_priv->drm.dev; >>>>>> + struct device *kdev = i915->drm.dev; >>>>>> bool is_error = level[1] <= KERN_ERR[1]; >>>>>> bool is_debug = level[1] == KERN_DEBUG[1]; >>>>>> struct va_format vaf; >>>>>> @@ -39,7 +38,7 @@ __i915_printk(struct drm_i915_private *dev_priv, const char *level, >>>>>> >>>>>> va_end(args); >>>>>> >>>>>> - if (is_error && !shown_bug_once) { >>>>>> + if (is_error && !i915->shown_bug_once) { >>>>>> /* >>>>>> * Ask the user to file a bug report for the error, except >>>>>> * if they may have caused the bug by fiddling with unsafe >>>>>> @@ -47,7 +46,7 @@ __i915_printk(struct drm_i915_private *dev_priv, const char *level, >>>>>> */ >>>>>> if (!test_taint(TAINT_USER)) >>>>>> dev_notice(kdev, "%s", FDO_BUG_MSG); >>>>> >>>>> I feel this plea to the users to file a bug report should be a one-time >>>>> thing; a true global. >>>> >>>> Fair enough. >>>> >>>> I am wondering if we should have a convention of naming or commenting >>>> legit globals, both to help automation detecting new accidental ones, >>>> and to help people figure out not to send another conversion patch such >>>> as this. >>> >>> global_i915_show_bug_once >>> module_i915_show_bug_once >> >> I'd be fine with either of the prefixes. Or i915_{global,module}_ for > > My vote goes for i915_global_* or global_i915_* (if we could get some > Kbuild infrastructure behind detecting global variables). any of the provided options or combinations works for me. probably global will be less confusing... > > Regards, Joonas _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx