Op 12-02-18 om 16:31 schreef Chris Wilson: > Quoting Maarten Lankhorst (2018-02-12 15:27:34) >> Op 12-02-18 om 16:22 schreef Chris Wilson: >>> 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@xxxxxxxxxxxxxxx> >>>>>> --- >>>>>> 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 >> It already would have been for events, hence the WARN_ON there. >> I don't think we can do anything about it, this shouldn't ever >> happen in practice, could be a BUG_ON for all I care. :) > I would much prefer that over intentionally bad code. > > But do we really need to enable the vblank irq here? If the event > requires it, doesn't it already enable the vblank. Here, we only need it > when sleeping, can we not determine we have enough time before the > vblank without enabling the interrupt? I'm not sure why we get a reference to the vblank counter here. Perhaps Ville does? ~Maarten _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
