On Mon, Aug 20, 2018 at 04:11:27PM -0700, Paulo Zanoni wrote: > Em Sex, 2018-08-17 às 16:41 -0700, Paulo Zanoni escreveu: > > Em Qua, 2018-08-15 às 23:27 +0300, Imre Deak escreveu: > > > On Wed, Aug 08, 2018 at 03:16:11PM -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@xxxxxxxxx> > > > > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx> > > > > --- > > > > 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); > > > > > > > > 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; > > > > -} > > > > - > > > > > > Or rather export a locked version of it and use that in > > > intel_hdcp.c > > > to > > > better hide the internals? > > > > That should probably be combined with José's idea of using ->enabled > > so > > we trust the hardware sync. > > > > Thanks for the suggestions. > > After further analysis, I wonder if intel_hdcp.c should really be > checking for enabled power wells or if it should be doing something > else, such as actually grabbing power domain references to make sure > we're able to enable/disable HDCP whenever we need. Most of our code > should not be checking for power wells/domains being enabled/disabled > (except for HW readout), it should actually be requesting those > resources to make sure we have them when we need them. > > CCing Ramaligam for that. There is no separate power resource for HDCP, it just uses the power wells the encoder already uses. Those are guaranteed to be on, since intel_hdcp_enable/disable are called from the encoder enable/disable hooks. As such hdcp_key_loadable() is just an assert. Defining a new power domain for this would be a bit overkill imo and as PW#1 is handled automatically by HW (and so not the usual driver get/put ops via power domain handles) we would have to special case it. > > > > > > > > > > static const struct i915_power_well_desc skl_power_wells[] = { > > > > { > > > > .name = "always-on", > > > > -- > > > > 2.14.4 > > > > > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx