Re: [PATCH 08/12] drm/i915: ensure the HW is powered when accessing the CRC HW block

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

On ti, 2016-02-16 at 17:02 +0100, Daniel Vetter wrote:
> On Fri, Feb 12, 2016 at 06:55:17PM +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>
> 
> subject needs to be fixed to mention that this is for all the stuff
> in
> i915_debugfs.c.

Ah, yea i915_interrupt_info() is also changed, will updated that in the
log.

> With that fixed for patches 4-8:
> 
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> 
> 
> > ---
> >  drivers/gpu/drm/i915/i915_debugfs.c | 28 +++++++++++++++++++++--
> > -----
> >  1 file changed, 21 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> > b/drivers/gpu/drm/i915/i915_debugfs.c
> > index ec0c2a05e..9e19cf0 100644
> > --- a/drivers/gpu/drm/i915/i915_debugfs.c
> > +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> > @@ -825,8 +825,11 @@ static int i915_interrupt_info(struct seq_file
> > *m, void *data)
> >  		}
> >  
> >  		for_each_pipe(dev_priv, pipe) {
> > -			if
> > (!intel_display_power_is_enabled(dev_priv,
> > -						POWER_DOMAIN_PIPE(
> > pipe))) {
> > +			enum intel_display_power_domain
> > power_domain;
> > +
> > +			power_domain = POWER_DOMAIN_PIPE(pipe);
> > +			if
> > (!intel_display_power_get_if_enabled(dev_priv,
> > +								po
> > wer_domain)) {
> >  				seq_printf(m, "Pipe %c power
> > disabled\n",
> >  					   pipe_name(pipe));
> >  				continue;
> > @@ -840,6 +843,8 @@ static int i915_interrupt_info(struct seq_file
> > *m, void *data)
> >  			seq_printf(m, "Pipe %c IER:\t%08x\n",
> >  				   pipe_name(pipe),
> >  				   I915_READ(GEN8_DE_PIPE_IER(pipe
> > )));
> > +
> > +			intel_display_power_put(dev_priv,
> > power_domain);
> >  		}
> >  
> >  		seq_printf(m, "Display Engine port interrupt
> > mask:\t%08x\n",
> > @@ -4004,6 +4009,7 @@ static int pipe_crc_set_source(struct
> > drm_device *dev, enum pipe pipe,
> >  	struct intel_pipe_crc *pipe_crc = &dev_priv-
> > >pipe_crc[pipe];
> >  	struct intel_crtc *crtc =
> > to_intel_crtc(intel_get_crtc_for_pipe(dev,
> >  									
> > pipe));
> > +	enum intel_display_power_domain power_domain;
> >  	u32 val = 0; /* shut up gcc */
> >  	int ret;
> >  
> > @@ -4014,7 +4020,8 @@ static int pipe_crc_set_source(struct
> > drm_device *dev, enum pipe pipe,
> >  	if (pipe_crc->source && source)
> >  		return -EINVAL;
> >  
> > -	if (!intel_display_power_is_enabled(dev_priv,
> > POWER_DOMAIN_PIPE(pipe))) {
> > +	power_domain = POWER_DOMAIN_PIPE(pipe);
> > +	if (!intel_display_power_get_if_enabled(dev_priv,
> > power_domain)) {
> >  		DRM_DEBUG_KMS("Trying to capture CRC while pipe is
> > off\n");
> >  		return -EIO;
> >  	}
> > @@ -4031,7 +4038,7 @@ static int pipe_crc_set_source(struct
> > drm_device *dev, enum pipe pipe,
> >  		ret = ivb_pipe_crc_ctl_reg(dev, pipe, &source,
> > &val);
> >  
> >  	if (ret != 0)
> > -		return ret;
> > +		goto out;
> >  
> >  	/* none -> real source transition */
> >  	if (source) {
> > @@ -4043,8 +4050,10 @@ static int pipe_crc_set_source(struct
> > drm_device *dev, enum pipe pipe,
> >  		entries = kcalloc(INTEL_PIPE_CRC_ENTRIES_NR,
> >  				  sizeof(pipe_crc->entries[0]),
> >  				  GFP_KERNEL);
> > -		if (!entries)
> > -			return -ENOMEM;
> > +		if (!entries) {
> > +			ret = -ENOMEM;
> > +			goto out;
> > +		}
> >  
> >  		/*
> >  		 * When IPS gets enabled, the pipe CRC changes.
> > Since IPS gets
> > @@ -4100,7 +4109,12 @@ static int pipe_crc_set_source(struct
> > drm_device *dev, enum pipe pipe,
> >  		hsw_enable_ips(crtc);
> >  	}
> >  
> > -	return 0;
> > +	ret = 0;
> > +
> > +out:
> > +	intel_display_power_put(dev_priv, power_domain);
> > +
> > +	return ret;
> >  }
> >  
> >  /*
> > -- 
> > 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