[Intel-gfx] Responsiveness Changes to i915 Driver

[Wilde, Martin]

Hi Jani - the DRM_DEBUG_KMS is part of the DRM_DEBUG_CODE preprocessor
macro and thus not available unavailable in a non-debug build kernel from
my understanding.

The issue we have seen many times is that the BIOS (firmware) team does
not set the T3 value correctly in the VBT table of the BIOS (or they use
the VESA default of 200ms and the panel is really a 50ms panel) and thus
there is no way to quickly determine what value was set unless you build a
debug kernel version that is typically beyond the scope of the BIOS team
(we have also had problems tracking down who in the BIOS team made the
setting).  For example in the latest Coreboot firmware for Rambi
Chromebook, someone set the VBT T3 value to 500 instead of 200.  This was
not detected until I ran the S3 resume test and noticed the S3 resume time
was 300ms too long.  Having the INFO message available in dmesg output
allowed me to quickly see the value was set wrong and address it without
having to do a debug build or do debugging to find the issue. Additionally
having the INFO message can be used by the firmware team to verify their
setting is correct.  We are also asking the Windows Gfx driver to do the
same as it happens more frequently than we thought.  Until we have fully
implemented HDP detection in the drivers, this issue will continue
happening.

So the request is to expose this as an INFO message to allow quick
detection/verification of correct setting as VBT settings can be set
in-correctly in a firmware update and not easily detected without special
kernel builds.  This allows the QA team to track platform settings that
effect the responsive time of mobile platforms.

Let me know if you have further questions and thanks for feedback

Regards,

-martin



On 8/20/14, 2:36 AM, "Jani Nikula" <jani.nikula at linux.intel.com> wrote:

>On Wed, 20 Aug 2014, "Wilde, Martin" <martin.wilde at intel.com> wrote:
>> Greetings - after reviewing Chris¹s feedback below and some thought, I
>> most likely do not need to add another trace message and the existing
>> ³i915_flip_complete² trace message can be used.
>>
>> Thus the only change requested is to have the T1_T3 value printed out
>> during driver init/re-init.  Here is the requested change:
>>
>> diff -rupN 
>>src.org/third_party/kernel/3.10/drivers/gpu/drm/i915/intel_dp.c
>> src/third_party/kernel/3.10/drivers/gpu/drm/i915/intel_dp.c
>> --- 
>> 
>>src.org/third_party/kernel/3.10/drivers/gpu/drm/i915/intel_dp.c	2014-08-1
>>4
>> 14:24:45.655312785 -0700
>> +++ 
>>src/third_party/kernel/3.10/drivers/gpu/drm/i915/intel_dp.c	2014-08-14
>> 11:57:30.203608374 -0700
>> @@ -3528,6 +3528,7 @@ intel_dp_init_panel_power_sequencer(stru
>>  	intel_dp->panel_power_cycle_delay = get_delay(t11_t12);
>>  #undef get_delay
>>  
>> +	printk(KERN_INFO "i915: eDP T3 Value: %d\n",
>> intel_dp->panel_power_up_delay);
>
>What's wrong with using the value printed on the next line? This is not
>something we'll add INFO level messages for.
>
>BR,
>Jani.
>
>
>>  	DRM_DEBUG_KMS("panel power up delay %d, power down delay %d, power
>>cycle
>> delay %d\n",
>>  		      intel_dp->panel_power_up_delay,
>>intel_dp->panel_power_down_delay,
>>  		      intel_dp->panel_power_cycle_delay);
>>
>-- 
>Jani Nikula, Intel Open Source Technology Center