Re: [PATCH 1/5] drm/i915: Keep vblank irq enabled during vblank evasion.

Chris Wilson
2018-02-12T15:22:51Z

Quoting Maarten Lankhorst (2018-02-12 15:16:39)
> Op 12-02-18 om 16:10 schreef Chris Wilson:
> > Quoting Maarten Lankhorst (2018-02-09 09:54:00)
> >> This is a nice preparation for grabbing the uncore lock during evasion.
> >> Grabbing the spinlock with the lock held messes up the locking,
> >> so it's easier to handover the reference to the eve
> >>
> >> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> >> ---
> >> drivers/gpu/drm/i915/intel_sprite.c | 11 ++++-------
> >> 1 file changed, 4 insertions(+), 7 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> >> index 3be22c0fcfb5..971a1ea0db45 100644
> >> --- a/drivers/gpu/drm/i915/intel_sprite.c
> >> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> >> @@ -109,10 +109,10 @@ void intel_pipe_update_start(const struct intel_crtc_state *new_crtc_state)
> >>
> >> local_irq_disable();
> >>
> >> - if (min <= 0 || max <= 0)
> >> + if (WARN_ON(drm_crtc_vblank_get(&crtc->base)))
> >> return;
> >>
> >> - if (WARN_ON(drm_crtc_vblank_get(&crtc->base)))
> >> + if (min <= 0 || max <= 0)
> >> return;
> >>
> > The corresponding vblank_put is the one later in update_start(), right?
> > I don't think you intended to keep this chunk.
> > -Chris
>
> I'm not sure what you mean? The vblank_put is now in pipe_update_end, except if the
> event takes over the reference. I think the code is correct. :)

Then it's unbalanced in the case of error still.
-Chris