Skip to content

Commit

Permalink
drm/i915: Fix races in gen4 page flip interrupt handling
Browse files Browse the repository at this point in the history
Use the gen3 logic for handling page flip interrupts on gen4.

Unfortuantely this kills the stall_check since that looks like it can
easily trigger too early. With the current logic the stall check would
kick in on the first vblank after the flip has been submitted to the
ring. If the CS takes longer than that to process the commands in the
ring, the stall check will cause the page flip to be complete too
early. That doesn't sound like a very good idea. Something better
should be deviced if we still need the stall check. For now, mark
i915_pageflip_stall_check() as unused.

v2: Fix irq enable_mask and add __always_unused (Chris Wilson)

References: https://bugs.launchpad.net/ubuntu/+source/xserver-xorg-video-intel/+bug/1116587
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Tested-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
  • Loading branch information
Ville Syrjälä authored and Daniel Vetter committed Feb 20, 2013
1 parent 5e2032d commit 21ad833
Showing 1 changed file with 20 additions and 11 deletions.
31 changes: 20 additions & 11 deletions drivers/gpu/drm/i915/i915_irq.c
Original file line number Diff line number Diff line change
Expand Up @@ -1547,7 +1547,7 @@ void i915_handle_error(struct drm_device *dev, bool wedged)
queue_work(dev_priv->wq, &dev_priv->gpu_error.work);
}

static void i915_pageflip_stall_check(struct drm_device *dev, int pipe)
static void __always_unused i915_pageflip_stall_check(struct drm_device *dev, int pipe)
{
drm_i915_private_t *dev_priv = dev->dev_private;
struct drm_crtc *crtc = dev_priv->pipe_to_crtc_mapping[pipe];
Expand Down Expand Up @@ -2598,6 +2598,8 @@ static int i965_irq_postinstall(struct drm_device *dev)
I915_RENDER_COMMAND_PARSER_ERROR_INTERRUPT);

enable_mask = ~dev_priv->irq_mask;
enable_mask &= ~(I915_DISPLAY_PLANE_A_FLIP_PENDING_INTERRUPT |
I915_DISPLAY_PLANE_B_FLIP_PENDING_INTERRUPT);
enable_mask |= I915_USER_INTERRUPT;

if (IS_G4X(dev))
Expand Down Expand Up @@ -2684,6 +2686,13 @@ static irqreturn_t i965_irq_handler(int irq, void *arg)
unsigned long irqflags;
int irq_received;
int ret = IRQ_NONE, pipe;
u32 flip[2] = {
I915_DISPLAY_PLANE_A_FLIP_PENDING_INTERRUPT,
I915_DISPLAY_PLANE_B_FLIP_PENDING_INTERRUPT
};
u32 flip_mask =
I915_DISPLAY_PLANE_A_FLIP_PENDING_INTERRUPT |
I915_DISPLAY_PLANE_B_FLIP_PENDING_INTERRUPT;

atomic_inc(&dev_priv->irq_received);

Expand All @@ -2692,7 +2701,7 @@ static irqreturn_t i965_irq_handler(int irq, void *arg)
for (;;) {
bool blc_event = false;

irq_received = iir != 0;
irq_received = (iir & ~flip_mask) != 0;

/* Can't rely on pipestat interrupt bit in iir as it might
* have been cleared after the pipestat interrupt was received.
Expand Down Expand Up @@ -2739,25 +2748,25 @@ static irqreturn_t i965_irq_handler(int irq, void *arg)
I915_READ(PORT_HOTPLUG_STAT);
}

I915_WRITE(IIR, iir);
I915_WRITE(IIR, iir & ~flip_mask);
new_iir = I915_READ(IIR); /* Flush posted writes */

if (iir & I915_USER_INTERRUPT)
notify_ring(dev, &dev_priv->ring[RCS]);
if (iir & I915_BSD_USER_INTERRUPT)
notify_ring(dev, &dev_priv->ring[VCS]);

if (iir & I915_DISPLAY_PLANE_A_FLIP_PENDING_INTERRUPT)
intel_prepare_page_flip(dev, 0);

if (iir & I915_DISPLAY_PLANE_B_FLIP_PENDING_INTERRUPT)
intel_prepare_page_flip(dev, 1);

for_each_pipe(pipe) {
if (pipe_stats[pipe] & PIPE_START_VBLANK_INTERRUPT_STATUS &&
drm_handle_vblank(dev, pipe)) {
i915_pageflip_stall_check(dev, pipe);
intel_finish_page_flip(dev, pipe);
if (iir & flip[pipe]) {
intel_prepare_page_flip(dev, pipe);

if ((I915_READ(ISR) & flip[pipe]) == 0) {
intel_finish_page_flip(dev, pipe);
flip_mask &= ~flip[pipe];
}
}
}

if (pipe_stats[pipe] & PIPE_LEGACY_BLC_EVENT_STATUS)
Expand Down

0 comments on commit 21ad833

Please sign in to comment.