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@xxxxxxxxx>
subject needs to be fixed to mention that this is for all the stuff in
i915_debugfs.c. With that fixed for patches 4-8:
Reviewed-by: Daniel Vetter <daniel.vetter@xxxxxxxx>
> ---
> 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,
> + power_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@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
