Message ID | 20180727233626.9216-1-jose.souza@intel.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | [1/3] drm/i915/cnl+: Reload CSR firmware when coming back from low power states | expand |
On Fri, Jul 27, 2018 at 04:36:24PM -0700, José Roberto de Souza wrote: > When returning from low power states the CSR firmware was not being > loaded again in CNL and ICL. > Also taking the opportunity to share the load call for gen >= 9, > instead of calling it from each display_core_init() function. > > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> > Cc: Anusha Srivatsa <anusha.srivatsa@intel.com> Cc: Imre Deak <imre.deak@intel.com> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com> > --- > drivers/gpu/drm/i915/i915_drv.c | 9 ++++++--- > drivers/gpu/drm/i915/intel_runtime_pm.c | 13 ++++--------- > 2 files changed, 10 insertions(+), 12 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c > index 18a45e7a3d7c..a42f0dfe19da 100644 > --- a/drivers/gpu/drm/i915/i915_drv.c > +++ b/drivers/gpu/drm/i915/i915_drv.c > @@ -2699,9 +2699,12 @@ static int intel_runtime_resume(struct device *kdev) > if (IS_GEN9_LP(dev_priv)) { > bxt_disable_dc9(dev_priv); > bxt_display_core_init(dev_priv, true); > - if (dev_priv->csr.dmc_payload && > - (dev_priv->csr.allowed_dc_mask & DC_STATE_EN_UPTO_DC5)) > - gen9_enable_dc5(dev_priv); > + if (dev_priv->csr.dmc_payload) { > + intel_csr_load_program(dev_priv); > + if (dev_priv->csr.allowed_dc_mask & > + DC_STATE_EN_UPTO_DC5) > + gen9_enable_dc5(dev_priv); > + } > } else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) { > hsw_disable_pc8(dev_priv); > } else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) { > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c > index cf89141b2281..8fdcffe023fe 100644 > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c > @@ -3075,9 +3075,6 @@ static void skl_display_core_init(struct drm_i915_private *dev_priv, > skl_init_cdclk(dev_priv); > > gen9_dbuf_enable(dev_priv); > - > - if (resume && dev_priv->csr.dmc_payload) > - intel_csr_load_program(dev_priv); and resume becomes unsed, so please kill it. > } > > static void skl_display_core_uninit(struct drm_i915_private *dev_priv) > @@ -3140,9 +3137,6 @@ void bxt_display_core_init(struct drm_i915_private *dev_priv, > bxt_init_cdclk(dev_priv); > > gen9_dbuf_enable(dev_priv); > - > - if (resume && dev_priv->csr.dmc_payload) > - intel_csr_load_program(dev_priv); same > } > > void bxt_display_core_uninit(struct drm_i915_private *dev_priv) > @@ -3283,9 +3277,6 @@ static void cnl_display_core_init(struct drm_i915_private *dev_priv, bool resume > > /* 6. Enable DBUF */ > gen9_dbuf_enable(dev_priv); > - > - if (resume && dev_priv->csr.dmc_payload) > - intel_csr_load_program(dev_priv); same > } > > static void cnl_display_core_uninit(struct drm_i915_private *dev_priv) > @@ -3561,6 +3552,10 @@ void intel_power_domains_init_hw(struct drm_i915_private *dev_priv, bool resume) > > /* For now, we need the power well to be always enabled. */ > intel_display_set_init_power(dev_priv, true); > + > + if (INTEL_GEN(dev_priv) >= 9 && resume && dev_priv->csr.dmc_payload) > + intel_csr_load_program(dev_priv); > + > /* Disable power support if the user asked so. */ > if (!i915_modparams.disable_power_well) > intel_display_power_get(dev_priv, POWER_DOMAIN_INIT); > -- > 2.18.0 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Fri, Jul 27, 2018 at 10:19:42PM -0700, Rodrigo Vivi wrote: > On Fri, Jul 27, 2018 at 04:36:24PM -0700, José Roberto de Souza wrote: > > When returning from low power states the CSR firmware was not being > > loaded again in CNL and ICL. Which power states are you referring to? We should already restore the firmware from all relevant states in i915_drm_resume_early(). The patch to load the firmware on ICL is not yet merged to drm-tip. > > Also taking the opportunity to share the load call for gen >= 9, > > instead of calling it from each display_core_init() function. > > > > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> > > Cc: Anusha Srivatsa <anusha.srivatsa@intel.com> > > Cc: Imre Deak <imre.deak@intel.com> > > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com> > > --- > > drivers/gpu/drm/i915/i915_drv.c | 9 ++++++--- > > drivers/gpu/drm/i915/intel_runtime_pm.c | 13 ++++--------- > > 2 files changed, 10 insertions(+), 12 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c > > index 18a45e7a3d7c..a42f0dfe19da 100644 > > --- a/drivers/gpu/drm/i915/i915_drv.c > > +++ b/drivers/gpu/drm/i915/i915_drv.c > > @@ -2699,9 +2699,12 @@ static int intel_runtime_resume(struct device *kdev) > > if (IS_GEN9_LP(dev_priv)) { > > bxt_disable_dc9(dev_priv); > > bxt_display_core_init(dev_priv, true); > > - if (dev_priv->csr.dmc_payload && > > - (dev_priv->csr.allowed_dc_mask & DC_STATE_EN_UPTO_DC5)) > > - gen9_enable_dc5(dev_priv); > > + if (dev_priv->csr.dmc_payload) { > > + intel_csr_load_program(dev_priv); > > + if (dev_priv->csr.allowed_dc_mask & > > + DC_STATE_EN_UPTO_DC5) > > + gen9_enable_dc5(dev_priv); > > + } > > } else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) { > > hsw_disable_pc8(dev_priv); > > } else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) { > > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c > > index cf89141b2281..8fdcffe023fe 100644 > > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c > > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c > > @@ -3075,9 +3075,6 @@ static void skl_display_core_init(struct drm_i915_private *dev_priv, > > skl_init_cdclk(dev_priv); > > > > gen9_dbuf_enable(dev_priv); > > - > > - if (resume && dev_priv->csr.dmc_payload) > > - intel_csr_load_program(dev_priv); > > and resume becomes unsed, so please kill it. > > > } > > > > static void skl_display_core_uninit(struct drm_i915_private *dev_priv) > > @@ -3140,9 +3137,6 @@ void bxt_display_core_init(struct drm_i915_private *dev_priv, > > bxt_init_cdclk(dev_priv); > > > > gen9_dbuf_enable(dev_priv); > > - > > - if (resume && dev_priv->csr.dmc_payload) > > - intel_csr_load_program(dev_priv); > > same > > > } > > > > void bxt_display_core_uninit(struct drm_i915_private *dev_priv) > > @@ -3283,9 +3277,6 @@ static void cnl_display_core_init(struct drm_i915_private *dev_priv, bool resume > > > > /* 6. Enable DBUF */ > > gen9_dbuf_enable(dev_priv); > > - > > - if (resume && dev_priv->csr.dmc_payload) > > - intel_csr_load_program(dev_priv); > > same > > > } > > > > static void cnl_display_core_uninit(struct drm_i915_private *dev_priv) > > @@ -3561,6 +3552,10 @@ void intel_power_domains_init_hw(struct drm_i915_private *dev_priv, bool resume) > > > > /* For now, we need the power well to be always enabled. */ > > intel_display_set_init_power(dev_priv, true); > > + > > + if (INTEL_GEN(dev_priv) >= 9 && resume && dev_priv->csr.dmc_payload) > > + intel_csr_load_program(dev_priv); > > + > > /* Disable power support if the user asked so. */ > > if (!i915_modparams.disable_power_well) > > intel_display_power_get(dev_priv, POWER_DOMAIN_INIT); > > -- > > 2.18.0 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Sat, 2018-07-28 at 11:40 +0300, Imre Deak wrote: > On Fri, Jul 27, 2018 at 10:19:42PM -0700, Rodrigo Vivi wrote: > > On Fri, Jul 27, 2018 at 04:36:24PM -0700, José Roberto de Souza > > wrote: > > > When returning from low power states the CSR firmware was not > > > being > > > loaded again in CNL and ICL. > > Which power states are you referring to? We should already restore > the > firmware from all relevant states in i915_drm_resume_early(). The > patch > to load the firmware on ICL is not yet merged to drm-tip. Exactly this one, i915_drm_resume_early() calls intel_power_domains_init_hw(). Yes it is still not loading becasue the firmware name is missing for ICL but everything else is there. > > > > Also taking the opportunity to share the load call for gen >= 9, > > > instead of calling it from each display_core_init() function. > > > > > > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> > > > Cc: Anusha Srivatsa <anusha.srivatsa@intel.com> > > > > Cc: Imre Deak <imre.deak@intel.com> > > > > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com> > > > --- > > > drivers/gpu/drm/i915/i915_drv.c | 9 ++++++--- > > > drivers/gpu/drm/i915/intel_runtime_pm.c | 13 ++++--------- > > > 2 files changed, 10 insertions(+), 12 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.c > > > b/drivers/gpu/drm/i915/i915_drv.c > > > index 18a45e7a3d7c..a42f0dfe19da 100644 > > > --- a/drivers/gpu/drm/i915/i915_drv.c > > > +++ b/drivers/gpu/drm/i915/i915_drv.c > > > @@ -2699,9 +2699,12 @@ static int intel_runtime_resume(struct > > > device *kdev) > > > if (IS_GEN9_LP(dev_priv)) { > > > bxt_disable_dc9(dev_priv); > > > bxt_display_core_init(dev_priv, true); > > > - if (dev_priv->csr.dmc_payload && > > > - (dev_priv->csr.allowed_dc_mask & > > > DC_STATE_EN_UPTO_DC5)) > > > - gen9_enable_dc5(dev_priv); > > > + if (dev_priv->csr.dmc_payload) { > > > + intel_csr_load_program(dev_priv); > > > + if (dev_priv->csr.allowed_dc_mask & > > > + DC_STATE_EN_UPTO_DC5) > > > + gen9_enable_dc5(dev_priv); > > > + } > > > } else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) { > > > hsw_disable_pc8(dev_priv); > > > } else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) > > > { > > > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c > > > b/drivers/gpu/drm/i915/intel_runtime_pm.c > > > index cf89141b2281..8fdcffe023fe 100644 > > > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c > > > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c > > > @@ -3075,9 +3075,6 @@ static void skl_display_core_init(struct > > > drm_i915_private *dev_priv, > > > skl_init_cdclk(dev_priv); > > > > > > gen9_dbuf_enable(dev_priv); > > > - > > > - if (resume && dev_priv->csr.dmc_payload) > > > - intel_csr_load_program(dev_priv); > > > > and resume becomes unsed, so please kill it. > > > > > } > > > > > > static void skl_display_core_uninit(struct drm_i915_private > > > *dev_priv) > > > @@ -3140,9 +3137,6 @@ void bxt_display_core_init(struct > > > drm_i915_private *dev_priv, > > > bxt_init_cdclk(dev_priv); > > > > > > gen9_dbuf_enable(dev_priv); > > > - > > > - if (resume && dev_priv->csr.dmc_payload) > > > - intel_csr_load_program(dev_priv); > > > > same > > > > > } > > > > > > void bxt_display_core_uninit(struct drm_i915_private *dev_priv) > > > @@ -3283,9 +3277,6 @@ static void cnl_display_core_init(struct > > > drm_i915_private *dev_priv, bool resume > > > > > > /* 6. Enable DBUF */ > > > gen9_dbuf_enable(dev_priv); > > > - > > > - if (resume && dev_priv->csr.dmc_payload) > > > - intel_csr_load_program(dev_priv); > > > > same > > > > > } > > > > > > static void cnl_display_core_uninit(struct drm_i915_private > > > *dev_priv) > > > @@ -3561,6 +3552,10 @@ void intel_power_domains_init_hw(struct > > > drm_i915_private *dev_priv, bool resume) > > > > > > /* For now, we need the power well to be always enabled. */ > > > intel_display_set_init_power(dev_priv, true); > > > + > > > + if (INTEL_GEN(dev_priv) >= 9 && resume && dev_priv- > > > >csr.dmc_payload) > > > + intel_csr_load_program(dev_priv); > > > + > > > /* Disable power support if the user asked so. */ > > > if (!i915_modparams.disable_power_well) > > > intel_display_power_get(dev_priv, POWER_DOMAIN_INIT); > > > -- > > > 2.18.0 > > > > > > _______________________________________________ > > > Intel-gfx mailing list > > > Intel-gfx@lists.freedesktop.org > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Mon, Jul 30, 2018 at 08:23:08PM +0300, Souza, Jose wrote: > On Sat, 2018-07-28 at 11:40 +0300, Imre Deak wrote: > > On Fri, Jul 27, 2018 at 10:19:42PM -0700, Rodrigo Vivi wrote: > > > On Fri, Jul 27, 2018 at 04:36:24PM -0700, José Roberto de Souza > > > wrote: > > > > When returning from low power states the CSR firmware was not > > > > being > > > > loaded again in CNL and ICL. > > > > Which power states are you referring to? We should already restore > > the > > firmware from all relevant states in i915_drm_resume_early(). The > > patch > > to load the firmware on ICL is not yet merged to drm-tip. > > Exactly this one, i915_drm_resume_early() calls > intel_power_domains_init_hw(). > Yes it is still not loading becasue the firmware name is missing for > ICL but everything else is there. I still don't understand how this fixes things. CNL already reloads the firmware via i915_drm_resume_early() -> cnl_display_core_init(). On ICL the same will happen once support for loading the firmware is added. That same patch should also add the call to intel_csr_load_program() from icl_display_core_init(). I would prefer to keep the call to intel_csr_load_program() in *_display_core_init() functions to keep the runtime and system suspend paths similar. > > > > > > > Also taking the opportunity to share the load call for gen >= 9, > > > > instead of calling it from each display_core_init() function. > > > > > > > > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> > > > > Cc: Anusha Srivatsa <anusha.srivatsa@intel.com> > > > > > > Cc: Imre Deak <imre.deak@intel.com> > > > > > > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com> > > > > --- > > > > drivers/gpu/drm/i915/i915_drv.c | 9 ++++++--- > > > > drivers/gpu/drm/i915/intel_runtime_pm.c | 13 ++++--------- > > > > 2 files changed, 10 insertions(+), 12 deletions(-) > > > > > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.c > > > > b/drivers/gpu/drm/i915/i915_drv.c > > > > index 18a45e7a3d7c..a42f0dfe19da 100644 > > > > --- a/drivers/gpu/drm/i915/i915_drv.c > > > > +++ b/drivers/gpu/drm/i915/i915_drv.c > > > > @@ -2699,9 +2699,12 @@ static int intel_runtime_resume(struct > > > > device *kdev) > > > > if (IS_GEN9_LP(dev_priv)) { > > > > bxt_disable_dc9(dev_priv); > > > > bxt_display_core_init(dev_priv, true); > > > > - if (dev_priv->csr.dmc_payload && > > > > - (dev_priv->csr.allowed_dc_mask & > > > > DC_STATE_EN_UPTO_DC5)) > > > > - gen9_enable_dc5(dev_priv); > > > > + if (dev_priv->csr.dmc_payload) { > > > > + intel_csr_load_program(dev_priv); > > > > + if (dev_priv->csr.allowed_dc_mask & > > > > + DC_STATE_EN_UPTO_DC5) > > > > + gen9_enable_dc5(dev_priv); > > > > + } > > > > } else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) { > > > > hsw_disable_pc8(dev_priv); > > > > } else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) > > > > { > > > > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c > > > > b/drivers/gpu/drm/i915/intel_runtime_pm.c > > > > index cf89141b2281..8fdcffe023fe 100644 > > > > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c > > > > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c > > > > @@ -3075,9 +3075,6 @@ static void skl_display_core_init(struct > > > > drm_i915_private *dev_priv, > > > > skl_init_cdclk(dev_priv); > > > > > > > > gen9_dbuf_enable(dev_priv); > > > > - > > > > - if (resume && dev_priv->csr.dmc_payload) > > > > - intel_csr_load_program(dev_priv); > > > > > > and resume becomes unsed, so please kill it. > > > > > > > } > > > > > > > > static void skl_display_core_uninit(struct drm_i915_private > > > > *dev_priv) > > > > @@ -3140,9 +3137,6 @@ void bxt_display_core_init(struct > > > > drm_i915_private *dev_priv, > > > > bxt_init_cdclk(dev_priv); > > > > > > > > gen9_dbuf_enable(dev_priv); > > > > - > > > > - if (resume && dev_priv->csr.dmc_payload) > > > > - intel_csr_load_program(dev_priv); > > > > > > same > > > > > > > } > > > > > > > > void bxt_display_core_uninit(struct drm_i915_private *dev_priv) > > > > @@ -3283,9 +3277,6 @@ static void cnl_display_core_init(struct > > > > drm_i915_private *dev_priv, bool resume > > > > > > > > /* 6. Enable DBUF */ > > > > gen9_dbuf_enable(dev_priv); > > > > - > > > > - if (resume && dev_priv->csr.dmc_payload) > > > > - intel_csr_load_program(dev_priv); > > > > > > same > > > > > > > } > > > > > > > > static void cnl_display_core_uninit(struct drm_i915_private > > > > *dev_priv) > > > > @@ -3561,6 +3552,10 @@ void intel_power_domains_init_hw(struct > > > > drm_i915_private *dev_priv, bool resume) > > > > > > > > /* For now, we need the power well to be always enabled. */ > > > > intel_display_set_init_power(dev_priv, true); > > > > + > > > > + if (INTEL_GEN(dev_priv) >= 9 && resume && dev_priv- > > > > >csr.dmc_payload) > > > > + intel_csr_load_program(dev_priv); > > > > + > > > > /* Disable power support if the user asked so. */ > > > > if (!i915_modparams.disable_power_well) > > > > intel_display_power_get(dev_priv, POWER_DOMAIN_INIT); > > > > -- > > > > 2.18.0 > > > > > > > > _______________________________________________ > > > > Intel-gfx mailing list > > > > Intel-gfx@lists.freedesktop.org > > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 18a45e7a3d7c..a42f0dfe19da 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -2699,9 +2699,12 @@ static int intel_runtime_resume(struct device *kdev) if (IS_GEN9_LP(dev_priv)) { bxt_disable_dc9(dev_priv); bxt_display_core_init(dev_priv, true); - if (dev_priv->csr.dmc_payload && - (dev_priv->csr.allowed_dc_mask & DC_STATE_EN_UPTO_DC5)) - gen9_enable_dc5(dev_priv); + if (dev_priv->csr.dmc_payload) { + intel_csr_load_program(dev_priv); + if (dev_priv->csr.allowed_dc_mask & + DC_STATE_EN_UPTO_DC5) + gen9_enable_dc5(dev_priv); + } } else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) { hsw_disable_pc8(dev_priv); } else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) { diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c index cf89141b2281..8fdcffe023fe 100644 --- a/drivers/gpu/drm/i915/intel_runtime_pm.c +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c @@ -3075,9 +3075,6 @@ static void skl_display_core_init(struct drm_i915_private *dev_priv, skl_init_cdclk(dev_priv); gen9_dbuf_enable(dev_priv); - - if (resume && dev_priv->csr.dmc_payload) - intel_csr_load_program(dev_priv); } static void skl_display_core_uninit(struct drm_i915_private *dev_priv) @@ -3140,9 +3137,6 @@ void bxt_display_core_init(struct drm_i915_private *dev_priv, bxt_init_cdclk(dev_priv); gen9_dbuf_enable(dev_priv); - - if (resume && dev_priv->csr.dmc_payload) - intel_csr_load_program(dev_priv); } void bxt_display_core_uninit(struct drm_i915_private *dev_priv) @@ -3283,9 +3277,6 @@ static void cnl_display_core_init(struct drm_i915_private *dev_priv, bool resume /* 6. Enable DBUF */ gen9_dbuf_enable(dev_priv); - - if (resume && dev_priv->csr.dmc_payload) - intel_csr_load_program(dev_priv); } static void cnl_display_core_uninit(struct drm_i915_private *dev_priv) @@ -3561,6 +3552,10 @@ void intel_power_domains_init_hw(struct drm_i915_private *dev_priv, bool resume) /* For now, we need the power well to be always enabled. */ intel_display_set_init_power(dev_priv, true); + + if (INTEL_GEN(dev_priv) >= 9 && resume && dev_priv->csr.dmc_payload) + intel_csr_load_program(dev_priv); + /* Disable power support if the user asked so. */ if (!i915_modparams.disable_power_well) intel_display_power_get(dev_priv, POWER_DOMAIN_INIT);
When returning from low power states the CSR firmware was not being loaded again in CNL and ICL. Also taking the opportunity to share the load call for gen >= 9, instead of calling it from each display_core_init() function. Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> Cc: Anusha Srivatsa <anusha.srivatsa@intel.com> Signed-off-by: José Roberto de Souza <jose.souza@intel.com> --- drivers/gpu/drm/i915/i915_drv.c | 9 ++++++--- drivers/gpu/drm/i915/intel_runtime_pm.c | 13 ++++--------- 2 files changed, 10 insertions(+), 12 deletions(-)