Re: [PATCH 12/12] drm/i915/lvds: ensure the HW is powered during HW state readout

[Imre Deak <imre.deak@intel.com>]

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@intel.com>
> 
> For patches 9-12:
> 
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> 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@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx