| Message ID | 20170328193538.8687-1-ville.syrjala@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Op 28-03-17 om 21:35 schreef ville.syrjala@linux.intel.com: > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > We're clearing the legacy_cursor_update flag before calling > drm_atomic_helper_setup_commit() which means the helper will > wait for the flip to complete before cleaning up the framebuffers. > That's not what we want for the legacy cursor, so let's clear > the flag after setting up the commit. > > Also toss in a FIXME about solving these problems in a nicer > way using the fabled vblank workers. > > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > Cc: Uwe Kleine-König <uwe@kleine-koenig.org> > Cc: Rafael Ristovski <rafael.ristovski@gmail.com> > Fixes: a5509abda48e ("drm/i915: Fix legacy cursor vs. watermarks for ILK-BDW") > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > --- > drivers/gpu/drm/i915/intel_display.c | 16 ++++++++++++---- > 1 file changed, 12 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 654b8a0c28ee..05ff193f2dd8 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -13024,6 +13024,10 @@ static int intel_atomic_commit(struct drm_device *dev, > struct drm_i915_private *dev_priv = to_i915(dev); > int ret = 0; > > + ret = drm_atomic_helper_setup_commit(state, nonblock); > + if (ret) > + return ret; > + > /* > * The intel_legacy_cursor_update() fast path takes care > * of avoiding the vblank waits for simple cursor > @@ -13031,14 +13035,18 @@ static int intel_atomic_commit(struct drm_device *dev, > * we want to perform the vblank waits so that watermark > * updates happen during the correct frames. Gen9+ have > * double buffered watermarks and so shouldn't need this. > + * > + * Do this after drm_atomic_helper_setup_commit() because > + * we still want to skip the fb cleanup waits from the > + * atomic helper. Although that does risk yanking the > + * mapping from under the display engine. > + * > + * FIXME doing watermarks and fb cleanup from a vblank worker > + * (assuming we had any) would solve these problems. > */ > if (INTEL_GEN(dev_priv) < 9) > state->legacy_cursor_update = false; > > - ret = drm_atomic_helper_setup_commit(state, nonblock); > - if (ret) > - return ret; > - > drm_atomic_state_get(state); > i915_sw_fence_init(&intel_state->commit_ready, > intel_atomic_commit_ready); From irc: <vsyrjala> hmm. actually should probably move that thing to happen even later. we should skip the flip waits from intel_atomic_prepare_commit() as well i recon Makes sense. Not sure how much we care about legacy page flips, since they're on their way out. If this is fixed and trybot is happy with v2 then I will be too. :)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 654b8a0c28ee..05ff193f2dd8 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -13024,6 +13024,10 @@ static int intel_atomic_commit(struct drm_device *dev, struct drm_i915_private *dev_priv = to_i915(dev); int ret = 0; + ret = drm_atomic_helper_setup_commit(state, nonblock); + if (ret) + return ret; + /* * The intel_legacy_cursor_update() fast path takes care * of avoiding the vblank waits for simple cursor @@ -13031,14 +13035,18 @@ static int intel_atomic_commit(struct drm_device *dev, * we want to perform the vblank waits so that watermark * updates happen during the correct frames. Gen9+ have * double buffered watermarks and so shouldn't need this. + * + * Do this after drm_atomic_helper_setup_commit() because + * we still want to skip the fb cleanup waits from the + * atomic helper. Although that does risk yanking the + * mapping from under the display engine. + * + * FIXME doing watermarks and fb cleanup from a vblank worker + * (assuming we had any) would solve these problems. */ if (INTEL_GEN(dev_priv) < 9) state->legacy_cursor_update = false; - ret = drm_atomic_helper_setup_commit(state, nonblock); - if (ret) - return ret; - drm_atomic_state_get(state); i915_sw_fence_init(&intel_state->commit_ready, intel_atomic_commit_ready);