Responsiveness Changes to i915 Driver

Responsiveness Changes to i915 Driver

[Wilde, Martin]

[-- Attachment #1: Type: text/plain, Size: 2158 bytes --]

Greetings,

I am submitting the below changes to i915 Gfx driver to support resume time Responsiveness measurements.  These changes parallel the work already done in the IVB Windows Gfx driver.  These changes in addition to other OTS scripts (suspend_resume) allow tracking of what is referred to as the “B2I” or “Button To Image” time of the platform.  The shorter this time, the more responsiveness the platform is viewed by the end user.  Panel selection is an important factor in providing a more responsive system.  Note there is no dependency on other scripts.  The changes are standalone.


  *   Display the current T1_T3 value.  This is used to verify that the timing set in the VBT is correct.  We have seen many instances where the value is not set correctly for the panel and the resume time is longer than necessary (e.g. 500ms T3 versus 200ms T3).
  *   Print the time when the first page flip occurs.  This is when the user first sees the desktop displayed from resume.  While this measurement could be done by other methods, this is the actual time that the desktop manager/framebuffer makes the driver request and the Gfx driver performs the action.  Thus any layering software added can be correlated to increases in this time.

To support the latter (first page flip), I added a new ftrace called “trace_i915_resume”.  I looked at the existing page flip trace message and that one is designed for every page flip.  I did not want to convolute it with the one time flip trace on resume.  I used a trace message instead of a printk to reduce any performance impacts of using a printk.  Additionally printk is not reliable of when the message actually appears in the kernel log.

The attached patch file for the 3.10 Linux Kernel (currently used by the Chromium/Chrome OS project for a BayTrail platform) has the small number of changes to a few files in the i915 directory.  The changes are minimal and have no impact in performance (that I have seen).

I can also make the changes to the 3.15 Linux Kernel if required.

Let me know of any additional questions

Regards,

-martin

PCCG GED Responsiveness Technologist


[-- Attachment #2: i915.patch --]
[-- Type: application/octet-stream, Size: 3912 bytes --]

diff -rupN src.org/third_party/kernel/3.10/drivers/gpu/drm/i915/i915_drv.c src/third_party/kernel/3.10/drivers/gpu/drm/i915/i915_drv.c
--- src.org/third_party/kernel/3.10/drivers/gpu/drm/i915/i915_drv.c	2014-08-14 14:21:38.683319040 -0700
+++ src/third_party/kernel/3.10/drivers/gpu/drm/i915/i915_drv.c	2014-07-29 11:01:44.829968341 -0700
@@ -615,6 +615,8 @@ static int __i915_drm_thaw(struct drm_de
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	int error = 0;
 
+	dev_priv->trace_page_flip = true; // we are resuming, trace first flip
+
 	intel_uncore_early_sanitize(dev);
 
 	intel_uncore_sanitize(dev);
diff -rupN src.org/third_party/kernel/3.10/drivers/gpu/drm/i915/i915_drv.h src/third_party/kernel/3.10/drivers/gpu/drm/i915/i915_drv.h
--- src.org/third_party/kernel/3.10/drivers/gpu/drm/i915/i915_drv.h	2014-08-14 14:21:47.687318738 -0700
+++ src/third_party/kernel/3.10/drivers/gpu/drm/i915/i915_drv.h	2014-07-29 11:01:44.829968341 -0700
@@ -1509,6 +1509,7 @@ typedef struct drm_i915_private {
 	/* Old ums support infrastructure, same warning applies. */
 	struct i915_ums_state ums;
 	
+	bool trace_page_flip; // First page flip after S3 trace flag
 } drm_i915_private_t;
 
 static inline struct drm_i915_private *to_i915(const struct drm_device *dev)
diff -rupN src.org/third_party/kernel/3.10/drivers/gpu/drm/i915/i915_trace.h src/third_party/kernel/3.10/drivers/gpu/drm/i915/i915_trace.h
--- src.org/third_party/kernel/3.10/drivers/gpu/drm/i915/i915_trace.h	2014-07-22 14:51:27.985740748 -0700
+++ src/third_party/kernel/3.10/drivers/gpu/drm/i915/i915_trace.h	2014-08-05 14:28:45.889319275 -0700
@@ -510,6 +510,22 @@ TRACE_EVENT(intel_gpu_freq_change,
 	    TP_printk("new_freq=%u", __entry->freq)
 );
 
+TRACE_EVENT(i915_resume,
+	    TP_PROTO(const char *action, bool start),
+	    TP_ARGS(action, start),
+
+	    TP_STRUCT__entry(
+			     __field(const char *, action)
+			     __field(bool, start)
+			     ),
+
+	    TP_fast_assign(
+			   __entry->action = action;
+			   __entry->start = start;
+			   ),
+
+	    TP_printk("%s %s", __entry->action, (__entry->start)?"begin":"end")
+);
 #endif /* _I915_TRACE_H_ */
 
 /* This part must be outside protection */
diff -rupN src.org/third_party/kernel/3.10/drivers/gpu/drm/i915/intel_display.c src/third_party/kernel/3.10/drivers/gpu/drm/i915/intel_display.c
--- src.org/third_party/kernel/3.10/drivers/gpu/drm/i915/intel_display.c	2014-08-14 14:22:31.739317265 -0700
+++ src/third_party/kernel/3.10/drivers/gpu/drm/i915/intel_display.c	2014-08-14 11:58:35.743606181 -0700
@@ -8358,6 +8358,11 @@ static int intel_crtc_page_flip(struct d
 
 	trace_i915_flip_request(intel_crtc->plane, obj);
 
+	if (dev_priv->trace_page_flip) {
+		dev_priv->trace_page_flip = false; // reset to only track once
+		trace_i915_resume("first page flip after resume", false);
+	}
+
 	return 0;
 
 cleanup_pending:
@@ -9805,6 +9810,8 @@ static void intel_crtc_init(struct drm_d
 	struct intel_crtc *intel_crtc;
 	int i;
 
+	dev_priv->trace_page_flip = false; // reset S3 page flip to no trace
+
 	intel_crtc = kzalloc(sizeof(*intel_crtc), GFP_KERNEL);
 	if (intel_crtc == NULL)
 		return;
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-14 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);
 	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);

[-- Attachment #3: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: Responsiveness Changes to i915 Driver

[Chris Wilson]

On Thu, Aug 14, 2014 at 11:52:47PM +0000, Wilde, Martin wrote:
> Greetings,
> 
> I am submitting the below changes to i915 Gfx driver to support resume time Responsiveness measurements.  These changes parallel the work already done in the IVB Windows Gfx driver.  These changes in addition to other OTS scripts (suspend_resume) allow tracking of what is referred to as the “B2I” or “Button To Image” time of the platform.  The shorter this time, the more responsiveness the platform is viewed by the end user.  Panel selection is an important factor in providing a more responsive system.  Note there is no dependency on other scripts.  The changes are standalone.
> 
> 
>   *   Display the current T1_T3 value.  This is used to verify that the timing set in the VBT is correct.  We have seen many instances where the value is not set correctly for the panel and the resume time is longer than necessary (e.g. 500ms T3 versus 200ms T3).
>   *   Print the time when the first page flip occurs.  This is when the user first sees the desktop displayed from resume.  While this measurement could be done by other methods, this is the actual time that the desktop manager/framebuffer makes the driver request and the Gfx driver performs the action.  Thus any layering software added can be correlated to increases in this time.
> 
> To support the latter (first page flip), I added a new ftrace called “trace_i915_resume”.  I looked at the existing page flip trace message and that one is designed for every page flip.  I did not want to convolute it with the one time flip trace on resume.  I used a trace message instead of a printk to reduce any performance impacts of using a printk.  Additionally printk is not reliable of when the message actually appears in the kernel log.

I am sorry, you are complaining that there is a tracepoint that gives
you exactly what you want already, only that userspace needs to do some
filtering? Which by the way, does not give you what you say you want
anyway - the scanout is already active long before the first flip is
handled, and in many, many cass that flip is just a figment of your
imagination. Maybe what you mean is to B2UR rather than
button-to-static-image.
-Chris


-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: Responsiveness Changes to i915 Driver

[Wilde, Martin]

[-- Attachment #1: Type: text/plain, Size: 3596 bytes --]

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-14
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);
 	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);

Regards,


-martin

On 8/14/14, 11:26 PM, "Chris Wilson" <chris@chris-wilson.co.uk> wrote:

>On Thu, Aug 14, 2014 at 11:52:47PM +0000, Wilde, Martin wrote:
>> Greetings,
>> 
>> I am submitting the below changes to i915 Gfx driver to support resume
>>time Responsiveness measurements.  These changes parallel the work
>>already done in the IVB Windows Gfx driver.  These changes in addition
>>to other OTS scripts (suspend_resume) allow tracking of what is referred
>>to as the ³B2I² or ³Button To Image² time of the platform.  The shorter
>>this time, the more responsiveness the platform is viewed by the end
>>user.  Panel selection is an important factor in providing a more
>>responsive system.  Note there is no dependency on other scripts.  The
>>changes are standalone.
>> 
>> 
>>   *   Display the current T1_T3 value.  This is used to verify that the
>>timing set in the VBT is correct.  We have seen many instances where the
>>value is not set correctly for the panel and the resume time is longer
>>than necessary (e.g. 500ms T3 versus 200ms T3).
>>   *   Print the time when the first page flip occurs.  This is when the
>>user first sees the desktop displayed from resume.  While this
>>measurement could be done by other methods, this is the actual time that
>>the desktop manager/framebuffer makes the driver request and the Gfx
>>driver performs the action.  Thus any layering software added can be
>>correlated to increases in this time.
>> 
>> To support the latter (first page flip), I added a new ftrace called
>>³trace_i915_resume².  I looked at the existing page flip trace message
>>and that one is designed for every page flip.  I did not want to
>>convolute it with the one time flip trace on resume.  I used a trace
>>message instead of a printk to reduce any performance impacts of using a
>>printk.  Additionally printk is not reliable of when the message
>>actually appears in the kernel log.
>
>I am sorry, you are complaining that there is a tracepoint that gives
>you exactly what you want already, only that userspace needs to do some
>filtering? Which by the way, does not give you what you say you want
>anyway - the scanout is already active long before the first flip is
>handled, and in many, many cass that flip is just a figment of your
>imagination. Maybe what you mean is to B2UR rather than
>button-to-static-image.
>-Chris
>
>
>-- 
>Chris Wilson, Intel Open Source Technology Centre


[-- Attachment #2: i915.patch --]
[-- Type: application/octet-stream, Size: 768 bytes --]

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-14 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);
 	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);

[-- Attachment #3: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: Responsiveness Changes to i915 Driver

[Jani Nikula]

On Wed, 20 Aug 2014, "Wilde, Martin" <martin.wilde@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-14
> 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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: 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@linux.intel.com> wrote:

>On Wed, 20 Aug 2014, "Wilde, Martin" <martin.wilde@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

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: Responsiveness Changes to i915 Driver

[Chris Wilson]

On Wed, Aug 20, 2014 at 06:03:52PM +0000, Wilde, Martin wrote:
> 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

I would suggest a /debugfs/.../i915_panel_capabilities and/or
i915_vbt_capabilites.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: Responsiveness Changes to i915 Driver

[Jani Nikula]

On Wed, 20 Aug 2014, "Wilde, Martin" <martin.wilde@intel.com> wrote:
> 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

Look, the exact same things could be said about almost all of the debug
messages we have. One VBT value will not get special treatment, sorry.

Are you aware that you can enable the debug messages by changing the
value of the drm.debug module parameter? There's no need for special
kernel builds or anything of the sort. Just add drm.debug=14 (it's a bit
mask) to the kernel command line, or in the modprobe conf, or change it
runtime at /sys/module/drm/parameters/debug. Make sure you also have
high enough loglevel set.

The debugfs has /sys/kernel/debug/dri/0/i915_opregion which includes the
VBT. It's binary, but the intel-gpu-tools package has userspace tools
(intel_bios_reader) to interpret the VBT contents and print them out. If
it doesn't print the values you want, it's relatively easy to
modify. The hardest part is likely conforming to the quirks of the VBT
specification version changes.

Indeed I think more effort should be put into validating the VBT
contents in other ways than by the operating system, in advance, perhaps
by the very tools that produce the VBT.

Finally, if none of the above works for you, I'd go the route Chris
suggested.


Thanks,
Jani.


-- 
Jani Nikula, Intel Open Source Technology Center

Re: Responsiveness Changes to i915 Driver

[Wilde, Martin]

Greetings,

Given the great ideas by Jani and Chris, I will not be requiring any
changes to the i915 driver.

I will use the /sys/module/drm/parameters/debug file to set a value of 14
as Jani indicates below. I have a python script to analyze the dmesg
output.

Thanks everyone for your excellent advice and guidance

Regards,

-martin



On 8/21/14, 12:26 AM, "Jani Nikula" <jani.nikula@linux.intel.com> wrote:

>On Wed, 20 Aug 2014, "Wilde, Martin" <martin.wilde@intel.com> wrote:
>> 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
>
>Look, the exact same things could be said about almost all of the debug
>messages we have. One VBT value will not get special treatment, sorry.
>
>Are you aware that you can enable the debug messages by changing the
>value of the drm.debug module parameter? There's no need for special
>kernel builds or anything of the sort. Just add drm.debug=14 (it's a bit
>mask) to the kernel command line, or in the modprobe conf, or change it
>runtime at /sys/module/drm/parameters/debug. Make sure you also have
>high enough loglevel set.
>
>The debugfs has /sys/kernel/debug/dri/0/i915_opregion which includes the
>VBT. It's binary, but the intel-gpu-tools package has userspace tools
>(intel_bios_reader) to interpret the VBT contents and print them out. If
>it doesn't print the values you want, it's relatively easy to
>modify. The hardest part is likely conforming to the quirks of the VBT
>specification version changes.
>
>Indeed I think more effort should be put into validating the VBT
>contents in other ways than by the operating system, in advance, perhaps
>by the very tools that produce the VBT.
>
>Finally, if none of the above works for you, I'd go the route Chris
>suggested.
>
>
>Thanks,
>Jani.
>
>
>-- 
>Jani Nikula, Intel Open Source Technology Center