| Message ID | 1434538270-22258-1-git-send-email-sagar.a.kamble@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Jun 17, 2015 at 04:21:10PM +0530, Sagar Arun Kamble wrote: > DC9 entry and exit programming is critical part of suspend/resume > sequences. This patch adds tracepoints that can help analyze time > taken using analyze_suspend.py/FTrace. > > Change-Id: I22fca5313c4349f8937eeb5a1c441c8ef76e5f4e > Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com> Tracepoints are for system profiling when benchmarking and all that. What exactly is this here for? Also note that tracepoints are pretty much ABI, so high standards apply here. -Daniel > --- > drivers/gpu/drm/i915/i915_drv.c | 9 +++++++++ > drivers/gpu/drm/i915/i915_reg.h | 1 + > 2 files changed, 10 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c > index 78ef0bb..6797650 100644 > --- a/drivers/gpu/drm/i915/i915_drv.c > +++ b/drivers/gpu/drm/i915/i915_drv.c > @@ -39,6 +39,7 @@ > #include <linux/module.h> > #include <linux/pm_runtime.h> > #include <drm/drm_crtc_helper.h> > +#include <trace/events/power.h> > > static struct drm_driver driver; > > @@ -1061,12 +1062,16 @@ static int bxt_suspend_complete(struct drm_i915_private *dev_priv) > { > struct drm_device *dev = dev_priv->dev; > > + trace_suspend_resume(TPS("dc_state_entry"), DC9_STATE, true); > + > /* TODO: when DC5 support is added disable DC5 here. */ > > broxton_ddi_phy_uninit(dev); > broxton_uninit_cdclk(dev); > bxt_enable_dc9(dev_priv); > > + trace_suspend_resume(TPS("dc_state_entry"), DC9_STATE, false); > + > return 0; > } > > @@ -1076,6 +1081,8 @@ static int bxt_resume_prepare(struct drm_i915_private *dev_priv) > > /* TODO: when CSR FW support is added make sure the FW is loaded */ > > + trace_suspend_resume(TPS("dc_state_exit"), DC9_STATE, true); > + > bxt_disable_dc9(dev_priv); > > /* > @@ -1086,6 +1093,8 @@ static int bxt_resume_prepare(struct drm_i915_private *dev_priv) > broxton_ddi_phy_init(dev); > intel_prepare_ddi(dev); > > + trace_suspend_resume(TPS("dc_state_exit"), DC9_STATE, false); > + > return 0; > } > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index 0b979ad..cf83ec8 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -7290,6 +7290,7 @@ enum skl_disp_power_wells { > #define DC_STATE_EN_DC9 (1<<3) > #define DC_STATE_EN_UPTO_DC6 (2<<0) > #define DC_STATE_EN_UPTO_DC5_DC6_MASK 0x3 > +#define DC9_STATE 9 > > #define DC_STATE_DEBUG 0x45520 > #define DC_STATE_DEBUG_MASK_MEMORY_UP (1<<1) > -- > 1.9.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Wed, 2015-06-17 at 13:30 +0200, Daniel Vetter wrote: > On Wed, Jun 17, 2015 at 04:21:10PM +0530, Sagar Arun Kamble wrote: > > DC9 entry and exit programming is critical part of suspend/resume > > sequences. This patch adds tracepoints that can help analyze time > > taken using analyze_suspend.py/FTrace. > > > > Change-Id: I22fca5313c4349f8937eeb5a1c441c8ef76e5f4e > > Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com> > > Tracepoints are for system profiling when benchmarking and all that. What > exactly is this here for? Hi Daniel, These trace points are added so that we can know how much time is spent in entry/exit programming of DC9 state which is done during runtime/system suspend. Analyzing these times is critical for resume time KPIs. > Also note that tracepoints are pretty much ABI, so high standards apply > here. Already these tracepoints are present at PM Core level. > -Daniel > > > --- > > drivers/gpu/drm/i915/i915_drv.c | 9 +++++++++ > > drivers/gpu/drm/i915/i915_reg.h | 1 + > > 2 files changed, 10 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c > > index 78ef0bb..6797650 100644 > > --- a/drivers/gpu/drm/i915/i915_drv.c > > +++ b/drivers/gpu/drm/i915/i915_drv.c > > @@ -39,6 +39,7 @@ > > #include <linux/module.h> > > #include <linux/pm_runtime.h> > > #include <drm/drm_crtc_helper.h> > > +#include <trace/events/power.h> > > > > static struct drm_driver driver; > > > > @@ -1061,12 +1062,16 @@ static int bxt_suspend_complete(struct drm_i915_private *dev_priv) > > { > > struct drm_device *dev = dev_priv->dev; > > > > + trace_suspend_resume(TPS("dc_state_entry"), DC9_STATE, true); > > + > > /* TODO: when DC5 support is added disable DC5 here. */ > > > > broxton_ddi_phy_uninit(dev); > > broxton_uninit_cdclk(dev); > > bxt_enable_dc9(dev_priv); > > > > + trace_suspend_resume(TPS("dc_state_entry"), DC9_STATE, false); > > + > > return 0; > > } > > > > @@ -1076,6 +1081,8 @@ static int bxt_resume_prepare(struct drm_i915_private *dev_priv) > > > > /* TODO: when CSR FW support is added make sure the FW is loaded */ > > > > + trace_suspend_resume(TPS("dc_state_exit"), DC9_STATE, true); > > + > > bxt_disable_dc9(dev_priv); > > > > /* > > @@ -1086,6 +1093,8 @@ static int bxt_resume_prepare(struct drm_i915_private *dev_priv) > > broxton_ddi_phy_init(dev); > > intel_prepare_ddi(dev); > > > > + trace_suspend_resume(TPS("dc_state_exit"), DC9_STATE, false); > > + > > return 0; > > } > > > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > > index 0b979ad..cf83ec8 100644 > > --- a/drivers/gpu/drm/i915/i915_reg.h > > +++ b/drivers/gpu/drm/i915/i915_reg.h > > @@ -7290,6 +7290,7 @@ enum skl_disp_power_wells { > > #define DC_STATE_EN_DC9 (1<<3) > > #define DC_STATE_EN_UPTO_DC6 (2<<0) > > #define DC_STATE_EN_UPTO_DC5_DC6_MASK 0x3 > > +#define DC9_STATE 9 > > > > #define DC_STATE_DEBUG 0x45520 > > #define DC_STATE_DEBUG_MASK_MEMORY_UP (1<<1) > > -- > > 1.9.1 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx >
On Sun, Jun 21, 2015 at 10:20:09AM +0530, Sagar Arun Kamble wrote: > On Wed, 2015-06-17 at 13:30 +0200, Daniel Vetter wrote: > > On Wed, Jun 17, 2015 at 04:21:10PM +0530, Sagar Arun Kamble wrote: > > > DC9 entry and exit programming is critical part of suspend/resume > > > sequences. This patch adds tracepoints that can help analyze time > > > taken using analyze_suspend.py/FTrace. > > > > > > Change-Id: I22fca5313c4349f8937eeb5a1c441c8ef76e5f4e > > > Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com> > > > > Tracepoints are for system profiling when benchmarking and all that. What > > exactly is this here for? > Hi Daniel, > These trace points are added so that we can know how much time is spent > in entry/exit programming of DC9 state which is done during > runtime/system suspend. Analyzing these times is critical for resume > time KPIs. What's a KPI? Also if we want to instrument our runtime pm interface then I think we should do this across the driver (probably at the power well interface). Also you seem to instrument only suspend resume and not rpm here. > > Also note that tracepoints are pretty much ABI, so high standards apply > > here. > Already these tracepoints are present at PM Core level. You're adding new labels and stuff and (see above) it looks not all that well thought out yet. -Daniel
Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 6596
-------------------------------------Summary-------------------------------------
Platform Delta drm-intel-nightly Series Applied
ILK 303/303 303/303
SNB 312/312 312/312
IVB 343/343 343/343
BYT -1 284/284 283/284
HSW 380/380 380/380
-------------------------------------Detailed-------------------------------------
Platform Test drm-intel-nightly Series Applied
*BYT igt@gem_partial_pwrite_pread@reads-display PASS(1) FAIL(1)
Note: You need to pay more attention to line start with '*'
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 78ef0bb..6797650 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -39,6 +39,7 @@ #include <linux/module.h> #include <linux/pm_runtime.h> #include <drm/drm_crtc_helper.h> +#include <trace/events/power.h> static struct drm_driver driver; @@ -1061,12 +1062,16 @@ static int bxt_suspend_complete(struct drm_i915_private *dev_priv) { struct drm_device *dev = dev_priv->dev; + trace_suspend_resume(TPS("dc_state_entry"), DC9_STATE, true); + /* TODO: when DC5 support is added disable DC5 here. */ broxton_ddi_phy_uninit(dev); broxton_uninit_cdclk(dev); bxt_enable_dc9(dev_priv); + trace_suspend_resume(TPS("dc_state_entry"), DC9_STATE, false); + return 0; } @@ -1076,6 +1081,8 @@ static int bxt_resume_prepare(struct drm_i915_private *dev_priv) /* TODO: when CSR FW support is added make sure the FW is loaded */ + trace_suspend_resume(TPS("dc_state_exit"), DC9_STATE, true); + bxt_disable_dc9(dev_priv); /* @@ -1086,6 +1093,8 @@ static int bxt_resume_prepare(struct drm_i915_private *dev_priv) broxton_ddi_phy_init(dev); intel_prepare_ddi(dev); + trace_suspend_resume(TPS("dc_state_exit"), DC9_STATE, false); + return 0; } diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 0b979ad..cf83ec8 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -7290,6 +7290,7 @@ enum skl_disp_power_wells { #define DC_STATE_EN_DC9 (1<<3) #define DC_STATE_EN_UPTO_DC6 (2<<0) #define DC_STATE_EN_UPTO_DC5_DC6_MASK 0x3 +#define DC9_STATE 9 #define DC_STATE_DEBUG 0x45520 #define DC_STATE_DEBUG_MASK_MEMORY_UP (1<<1)
DC9 entry and exit programming is critical part of suspend/resume sequences. This patch adds tracepoints that can help analyze time taken using analyze_suspend.py/FTrace. Change-Id: I22fca5313c4349f8937eeb5a1c441c8ef76e5f4e Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com> --- drivers/gpu/drm/i915/i915_drv.c | 9 +++++++++ drivers/gpu/drm/i915/i915_reg.h | 1 + 2 files changed, 10 insertions(+)