[Intel-gfx] [PATCH] drm/i915/selftests: Log test and subtest names for easier debugging

[Tvrtko Ursulin]

On 20/11/2018 18:18, Tvrtko Ursulin wrote:
> 
> On 20/11/2018 18:10, Chris Wilson wrote:
>> Quoting Tvrtko Ursulin (2018-11-20 17:58:33)
>>>
>>> On 20/11/2018 17:33, Chris Wilson wrote:
>>>> Quoting Tvrtko Ursulin (2018-11-20 17:28:39)
>>>>> From: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
>>>>>
>>>>> Since pr_debug is not printed by default, change both test and subtest
>>>>> log messages to pr_info so they are always logged.
>>>>
>>>> I just use the trace... As when the test fails we say which subtest
>>>> failed, and hopefully include more details in the error message, and
>>>> recent history is in the trace dumped when considered relevant.
>>>
>>> Fair. My thinking was simply to leave more breadcrumbs if the machine
>>> died in the middle of a subtest.
>>
>> Thinking more about it, the easiest option would be to actually enable
>> pr_debug, iirc a config option.
> 
> AFAICT we would need to pass -DDEBUG to interesting compilation units.
> 
>> However, the only time we don't get breadcrumbs is if the machine
>> spontaneously combusts, which we are told is a mere figment of our
>> imagination.  Most often I wonder if we simply do not get the death
>> throes - adding logging won't help if we don't capture it. For the true
>> spontaneous combustion, it's pretty random and all I can think of
>> suggesting are more sanity checks to try and catch inconsistencies early.
> 
> Well I was looking at one such log today so hence the patch.. :)
> 
> <6>[  491.437102] i915: Performing live selftests with st_random_seed=0x3bd88daf st_timeout=1000
> <7>[  491.438062] [drm:intel_power_well_enable [i915]] enabling always-on
> <7>[  491.438121] [drm:intel_power_well_enable [i915]] enabling DC off
> <7>[  491.438509] [drm:gen9_set_dc_state [i915]] Setting DC state from 02 to 00
> <7>[  491.541663] [drm:intel_power_well_disable [i915]] disabling DC off
> <7>[  491.541722] [drm:skl_enable_dc6 [i915]] Enabling DC6
> <7>[  491.541751] [drm:gen9_set_dc_state [i915]] Setting DC state from 00 to 02
> <7>[  491.542180] [drm:intel_power_well_disable [i915]] disabling always-on
> <7>[  495.443505] [drm:intel_power_well_enable [i915]] enabling always-on
> <7>[  495.443533] [drm:intel_power_well_enable [i915]] enabling DC off
> <7>[  495.443817] [drm:gen9_set_dc_state [i915]] Setting DC state from 02 to 00
> <7>[  499.549369] [drm:intel_power_well_disable [i915]] disabling DC off
> <7>[  499.549403] [drm:skl_enable_dc6 [i915]] Enabling DC6
> <7>[  499.549431] [drm:gen9_set_dc_state [i915]] Setting DC state from 00 to 02
> <7>[  499.549863] [drm:intel_power_well_disable [i915]] disabling always-on
> <6>[  500.448268] i915_reset_engine(rcs0:idle): 265564 resets
> <6>[  501.449250] i915_reset_engine(bcs0:idle): 249487 resets
> EOF
> 
> I can certainly send a patch for -DDEBUG, seems like that would be the
> correct thing to do for all selftests.

I've changed my mind - I think it is more desirable to actually just do 
what this patch did and convert test start messages to pr_info.

To enable all pr_debug by default, even if compiled with i915 debugging 
on, doesn't seem conceptually correct. There might be ones which are 
prohibitively noisy and in any case it would defeat the point of dynamic 
debug.

Regards,

Tvrtko