On ti, 2016-02-16 at 17:05 +0100, Daniel Vetter wrote: > On Fri, Feb 12, 2016 at 06:55:21PM +0200, Imre Deak wrote: > > The assumption when adding the intel_display_power_is_enabled() > > checks > > was that if it returns success the power can't be turned off > > afterwards > > during the HW access, which is guaranteed by modeset locks. This > > isn't > > always true, so make sure we hold a dedicated reference for the > > time of > > the access. > > > > Signed-off-by: Imre Deak <imre.deak@xxxxxxxxx> > > For patches 9-12: > > Reviewed-by: Daniel Vetter <daniel.vetter@xxxxxxxx> > I'm assuming you're working on more patches to get rid of all the > powe_is_enabled() checks, so that we can remove this fundamentally > unsafe > function evenutally? Mika noticed that I missed to convert the one in skl_ddb_get_hw_state(), I'll send a patch for it. For the other two remaining callers I gave an explanation in the cover letter as to why I haven't converted them. --Imre > -Daniel > > --- > > drivers/gpu/drm/i915/intel_lvds.c | 14 +++++++++++--- > > 1 file changed, 11 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_lvds.c > > b/drivers/gpu/drm/i915/intel_lvds.c > > index 811ddf7..30a8403 100644 > > --- a/drivers/gpu/drm/i915/intel_lvds.c > > +++ b/drivers/gpu/drm/i915/intel_lvds.c > > @@ -76,22 +76,30 @@ static bool intel_lvds_get_hw_state(struct > > intel_encoder *encoder, > > struct intel_lvds_encoder *lvds_encoder = > > to_lvds_encoder(&encoder->base); > > enum intel_display_power_domain power_domain; > > u32 tmp; > > + bool ret; > > > > power_domain = intel_display_port_power_domain(encoder); > > - if (!intel_display_power_is_enabled(dev_priv, > > power_domain)) > > + if (!intel_display_power_get_if_enabled(dev_priv, > > power_domain)) > > return false; > > > > + ret = false; > > + > > tmp = I915_READ(lvds_encoder->reg); > > > > if (!(tmp & LVDS_PORT_EN)) > > - return false; > > + goto out; > > > > if (HAS_PCH_CPT(dev)) > > *pipe = PORT_TO_PIPE_CPT(tmp); > > else > > *pipe = PORT_TO_PIPE(tmp); > > > > - return true; > > + ret = true; > > + > > +out: > > + intel_display_power_put(dev_priv, power_domain); > > + > > + return ret; > > } > > > > static void intel_lvds_get_config(struct intel_encoder *encoder, > > -- > > 2.5.0 > > > > _______________________________________________ > > 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