Re: [Intel-gfx] Responsiveness Changes to i915 Driver
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.nik...@linux.intel.com wrote: On Wed, 20 Aug 2014, Wilde, Martin martin.wi...@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 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] Responsiveness Changes to i915 Driver
On Wed, 20 Aug 2014, Wilde, Martin martin.wi...@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 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] Responsiveness Changes to i915 Driver
On Wed, 20 Aug 2014, Wilde, Martin martin.wi...@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: [Intel-gfx] Responsiveness Changes to i915 Driver
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.nik...@linux.intel.com wrote: On Wed, 20 Aug 2014, Wilde, Martin martin.wi...@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: [Intel-gfx] Responsiveness Changes to i915 Driver
On Wed, Aug 20, 2014 at 06:03:52PM +, 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 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] Responsiveness Changes to i915 Driver
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 ch...@chris-wilson.co.uk wrote: On Thu, Aug 14, 2014 at 11:52:47PM +, 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 i915.patch Description: i915.patch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] Responsiveness Changes to i915 Driver
On Thu, Aug 14, 2014 at 11:52:47PM +, 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
