[Intel-gfx] [PATCH 2/3] drm/i915: fixup i915_pipe_enabled check in i915_irq.c
Paulo Zanoni
przanoni at gmail.com
Mon May 27 22:56:07 CEST 2013
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
More information about the Intel-gfx
mailing list