[Intel-gfx] [PATCH 2/3] drm/i915: fixup i915_pipe_enabled check in i915_irq.c

[Paulo Zanoni]

2013/5/21 Daniel Vetter <daniel.vetter at ffwll.ch>:
> Well, as well as we can without completely revamping the drm vblank
> code. The issue are that
> - The vblank code needs to work on both ums and kms.
> - It deals always deals with pipes.
> - It doesn't take any of the kms locks.
>
> The last part is not really fixable without revamping the drm vblank
> code, since the drm core <-> driver interactions is a veritable pile
> of spaghettis. But the other pieces can be fixed by switching on the
> MODESET driver flag and either checking the hw state directly (ums
> case) or just querying our sw tracking (with broken locking, but
> that's not worse than what we've had).
>
> Note that this essentially reverts
>
> commit 702e7a56af3780d8b3a717f698209bef44187bb0
> Author: Paulo Zanoni <paulo.r.zanoni at intel.com>
> Date:   Tue Oct 23 18:29:59 2012 -0200
>
>     drm/i915: convert PIPECONF to use transcoder instead of pipe
>
> for the ums case, which will fix a NULL deref (since we really don't
> have any crtcs set up).
>
> But the real reason to do this is to drop our reliance on the
> cpu_transcoder: By only checking intel_crtc->active we don't need to
> make sure that the pipe_config (or at least the cpu_transcoder)
> contain safe values even when the pipe is off.
>
> Cc: Paulo Zanoni <paulo.r.zanoni at intel.com>
> Cc: Damien Lespiau <damien.lespiau at intel.com>
> Signed-off-by: Daniel Vetter <daniel.vetter at ffwll.ch>
> ---
>  drivers/gpu/drm/i915/i915_irq.c | 14 ++++++++------
>  1 file changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 879c4cc..d2efc5e 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -381,14 +381,16 @@ static int
>  i915_pipe_enabled(struct drm_device *dev, int pipe)
>  {
>         drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
> -       enum transcoder cpu_transcoder = intel_pipe_to_cpu_transcoder(dev_priv,
> -                                                                     pipe);
>
> -       if (!intel_display_power_enabled(dev,
> -               POWER_DOMAIN_TRANSCODER(cpu_transcoder)))
> -               return false;
> +       if (drm_core_check_feature(dev, DRIVER_MODESET)) {
> +               /* Locking is horribly broken here, but whatever. */

We could add also have a TODO or FIXME tag here.

Looks correct.
Reviewed-by: Paulo Zanoni <paulo.r.zanoni at intel.com>

> +               struct drm_crtc *crtc = dev_priv->pipe_to_crtc_mapping[pipe];
> +               struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>
> -       return I915_READ(PIPECONF(cpu_transcoder)) & PIPECONF_ENABLE;
> +               return intel_crtc->active;
> +       } else {
> +               return I915_READ(PIPECONF(pipe)) & PIPECONF_ENABLE;
> +       }
>  }
>
>  /* Called from drm generic code, passed a 'crtc', which
> --
> 1.8.1.4
>



-- 
Paulo Zanoni