[Intel-gfx] [PATCH 6/6] drm/i915: Really wait for pending flips in intel_pipe_set_base()

[Daniel Vetter]

On Wed, Feb 13, 2013 at 6:06 PM, Ville Syrjälä
<ville.syrjala at linux.intel.com> wrote:
>> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> > index a2e04f7..7e047c1 100644
>> > --- a/drivers/gpu/drm/i915/intel_display.c
>> > +++ b/drivers/gpu/drm/i915/intel_display.c
>> > @@ -2330,8 +2330,7 @@ intel_pipe_set_base(struct drm_crtc *crtc, int x, int y,
>> >             return ret;
>> >     }
>> >
>> > -   if (crtc->fb)
>> > -           intel_finish_fb(crtc->fb);
>> > +   intel_crtc_wait_for_pending_flips_locked(crtc);
>>
>> Ah, I see now why you grab dev->struct_mutex and need to kick waiters from
>> the pending flip queue. I think grabbing the mutex twice isn't a major
>> offense, since both the crtc disable code and set_base are slowpaths used
>> rarely. So what about simply calling wait_for_pending_flips before
>> grabbing the mutex in intel_pipe_set_base? We could then also inline
>> finish_fb into it's only callsite ...
>
> I didn't want to slow down intel_pipe_set_base() too much. If we wait
> for pending flips before pinning the new fb, we can never achieve any
> parallelism there. But if no-one cares about that, we can reorder.

I'm confused here - where can we extract parallelism in set_base
between waiting for pending flips and the pinning? And imo set_base
isn't really critical: It's officially a synchronous thing (we have a
vblank wait in there), and if we want to fix that imo the nuclear
pageflip should be the answer.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch