Re: [PATCH 1/4] drm/i915: kill intel_display_power_well_is_enabled()

[Paulo Zanoni <paulo.r.zanoni@intel.com>]

Em Qua, 2018-08-08 às 15:22 -0700, Souza, Jose escreveu:
> On Wed, 2018-08-08 at 15:16 -0700, Paulo Zanoni wrote:
> > Use the same coding pattern as we use in the other functions of the
> > same file: just call lookup_power_well() directly in the only
> > caller.
> > 
> > Cc: Imre Deak <imre.deak@intel.com>
> > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_runtime_pm.c | 20 +++-----------------
> >  1 file changed, 3 insertions(+), 17 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c
> > b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > index e209edbc561d..e0947f662361 100644
> > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > @@ -49,9 +49,6 @@
> >   * present for a given platform.
> >   */
> >  
> > -bool intel_display_power_well_is_enabled(struct drm_i915_private
> > *dev_priv,
> > -					 enum i915_power_well_id
> > power_well_id);
> > -
> >  static struct i915_power_well *
> >  lookup_power_well(struct drm_i915_private *dev_priv,
> >  		  enum i915_power_well_id power_well_id);
> > @@ -678,8 +675,9 @@ static void assert_csr_loaded(struct
> > drm_i915_private *dev_priv)
> >  
> >  static void assert_can_enable_dc5(struct drm_i915_private
> > *dev_priv)
> >  {
> > -	bool pg2_enabled =
> > intel_display_power_well_is_enabled(dev_priv,
> > -					SKL_DISP_PW_2);
> > +	struct i915_power_well *pg2 = lookup_power_well(dev_priv,
> > +							SKL_DISP_P
> > W_2);
> > +	bool pg2_enabled = pg2->desc->ops->is_enabled(dev_priv,
> > pg2);
> 
> Why not trust our sync with hardware and use pg2->hw_enabled?

While that's a very reasonable thing and it sounds like something we
should do (as far as I have investigated in 5 minutes), it's a
functional change with a potential downside and should be in a separate
patch IMHO.

> 
> >  
> >  	WARN_ONCE(pg2_enabled, "PG2 not disabled to enable
> > DC5.\n");
> >  
> > @@ -2302,18 +2300,6 @@ static const struct i915_power_well_desc
> > chv_power_wells[] = {
> >  	},
> >  };
> >  
> > -bool intel_display_power_well_is_enabled(struct drm_i915_private
> > *dev_priv,
> > -					 enum i915_power_well_id
> > power_well_id)
> > -{
> > -	struct i915_power_well *power_well;
> > -	bool ret;
> > -
> > -	power_well = lookup_power_well(dev_priv, power_well_id);
> > -	ret = power_well->desc->ops->is_enabled(dev_priv,
> > power_well);
> > -
> > -	return ret;
> > -}
> > -
> >  static const struct i915_power_well_desc skl_power_wells[] = {
> >  	{
> >  		.name = "always-on",
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx