| Message ID | 20240430095639.26390-5-ville.syrjala@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915: Allow the first async flip to change modifier | expand |
> -----Original Message----- > From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of Ville > Syrjala > Sent: Tuesday, April 30, 2024 3:27 PM > To: intel-gfx@lists.freedesktop.org > Subject: [PATCH v2 4/5] drm/i915: Eliminate extra frame from skl-glk sync- > >async flip change > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > On bdw-glk the sync->async flip change takes an extra frame due to the double > buffering behaviour of the async flip plane control bit. > > Since on skl+ we are now explicitly converting the first async flip to a sync flip > (in order to allow changing the modifier and/or > ddb/watermarks) we are now taking two extra frames until async flips are > actually active. We can drop that back down to one frame by setting the async > flip bit already during the sync flip. > > Note that on bdw we don't currently do the extra sync flip (see > intel_plane_do_async_flip()) so technically we wouldn't have to deal with this in > i9xx_plane_update_arm(). But I added the relevant snippet of code there as > well, just in case we ever decide to go for the extra sync flip on pre-skl platforms > as well (we might, for example, want to change the fb stride). > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> Reviewed-by: Arun R Murthy <arun.r.murthy@intel.com> Thanks and Regards, Arun R Murthy -------------------- > --- > drivers/gpu/drm/i915/display/i9xx_plane.c | 5 +++++ > drivers/gpu/drm/i915/display/intel_atomic_plane.c | 15 +++++++++++---- > .../gpu/drm/i915/display/skl_universal_plane.c | 5 +++++ > 3 files changed, 21 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/i9xx_plane.c > b/drivers/gpu/drm/i915/display/i9xx_plane.c > index 0279c8aabdd1..76fc7626051b 100644 > --- a/drivers/gpu/drm/i915/display/i9xx_plane.c > +++ b/drivers/gpu/drm/i915/display/i9xx_plane.c > @@ -455,6 +455,11 @@ static void i9xx_plane_update_arm(struct intel_plane > *plane, > > dspcntr = plane_state->ctl | i9xx_plane_ctl_crtc(crtc_state); > > + /* see intel_plane_atomic_calc_changes() */ > + if (plane->need_async_flip_disable_wa && > + crtc_state->async_flip_planes & BIT(plane->id)) > + dspcntr |= DISP_ASYNC_FLIP; > + > linear_offset = intel_fb_xy_to_linear(x, y, plane_state, 0); > > if (DISPLAY_VER(dev_priv) >= 4) > diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.c > b/drivers/gpu/drm/i915/display/intel_atomic_plane.c > index 769010d0ebc4..7098a34a17c8 100644 > --- a/drivers/gpu/drm/i915/display/intel_atomic_plane.c > +++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.c > @@ -437,10 +437,6 @@ static bool intel_plane_do_async_flip(struct > intel_plane *plane, > * only X-tile is supported with async flips, though we could > * extend this so other scanout parameters (stride/etc) could > * be changed as well... > - * > - * FIXME: Platforms with need_async_flip_disable_wa==true will > - * now end up doing two sync flips initially. Would be nice to > - * combine those into just the one sync flip... > */ > return DISPLAY_VER(i915) < 9 || old_crtc_state->uapi.async_flip; } @@ > -604,6 +600,17 @@ static int intel_plane_atomic_calc_changes(const struct > intel_crtc_state *old_cr > if (intel_plane_do_async_flip(plane, old_crtc_state, new_crtc_state)) { > new_crtc_state->do_async_flip = true; > new_crtc_state->async_flip_planes |= BIT(plane->id); > + } else if (plane->need_async_flip_disable_wa && > + new_crtc_state->uapi.async_flip) { > + /* > + * On platforms with double buffered async flip bit we > + * set the bit already one frame early during the sync > + * flip (see {i9xx,skl}_plane_update_arm()). The > + * hardware will therefore be ready to perform a real > + * async flip during the next commit, without having > + * to wait yet another frame for the bit to latch. > + */ > + new_crtc_state->async_flip_planes |= BIT(plane->id); > } > > return 0; > diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c > b/drivers/gpu/drm/i915/display/skl_universal_plane.c > index 860574d04f88..ad4c90344f68 100644 > --- a/drivers/gpu/drm/i915/display/skl_universal_plane.c > +++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c > @@ -1174,6 +1174,11 @@ skl_plane_update_arm(struct intel_plane *plane, > plane_ctl = plane_state->ctl | > skl_plane_ctl_crtc(crtc_state); > > + /* see intel_plane_atomic_calc_changes() */ > + if (plane->need_async_flip_disable_wa && > + crtc_state->async_flip_planes & BIT(plane->id)) > + plane_ctl |= PLANE_CTL_ASYNC_FLIP; > + > if (DISPLAY_VER(dev_priv) >= 10) > plane_color_ctl = plane_state->color_ctl | > glk_plane_color_ctl_crtc(crtc_state); > -- > 2.43.2
diff --git a/drivers/gpu/drm/i915/display/i9xx_plane.c b/drivers/gpu/drm/i915/display/i9xx_plane.c index 0279c8aabdd1..76fc7626051b 100644 --- a/drivers/gpu/drm/i915/display/i9xx_plane.c +++ b/drivers/gpu/drm/i915/display/i9xx_plane.c @@ -455,6 +455,11 @@ static void i9xx_plane_update_arm(struct intel_plane *plane, dspcntr = plane_state->ctl | i9xx_plane_ctl_crtc(crtc_state); + /* see intel_plane_atomic_calc_changes() */ + if (plane->need_async_flip_disable_wa && + crtc_state->async_flip_planes & BIT(plane->id)) + dspcntr |= DISP_ASYNC_FLIP; + linear_offset = intel_fb_xy_to_linear(x, y, plane_state, 0); if (DISPLAY_VER(dev_priv) >= 4) diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.c b/drivers/gpu/drm/i915/display/intel_atomic_plane.c index 769010d0ebc4..7098a34a17c8 100644 --- a/drivers/gpu/drm/i915/display/intel_atomic_plane.c +++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.c @@ -437,10 +437,6 @@ static bool intel_plane_do_async_flip(struct intel_plane *plane, * only X-tile is supported with async flips, though we could * extend this so other scanout parameters (stride/etc) could * be changed as well... - * - * FIXME: Platforms with need_async_flip_disable_wa==true will - * now end up doing two sync flips initially. Would be nice to - * combine those into just the one sync flip... */ return DISPLAY_VER(i915) < 9 || old_crtc_state->uapi.async_flip; } @@ -604,6 +600,17 @@ static int intel_plane_atomic_calc_changes(const struct intel_crtc_state *old_cr if (intel_plane_do_async_flip(plane, old_crtc_state, new_crtc_state)) { new_crtc_state->do_async_flip = true; new_crtc_state->async_flip_planes |= BIT(plane->id); + } else if (plane->need_async_flip_disable_wa && + new_crtc_state->uapi.async_flip) { + /* + * On platforms with double buffered async flip bit we + * set the bit already one frame early during the sync + * flip (see {i9xx,skl}_plane_update_arm()). The + * hardware will therefore be ready to perform a real + * async flip during the next commit, without having + * to wait yet another frame for the bit to latch. + */ + new_crtc_state->async_flip_planes |= BIT(plane->id); } return 0; diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c b/drivers/gpu/drm/i915/display/skl_universal_plane.c index 860574d04f88..ad4c90344f68 100644 --- a/drivers/gpu/drm/i915/display/skl_universal_plane.c +++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c @@ -1174,6 +1174,11 @@ skl_plane_update_arm(struct intel_plane *plane, plane_ctl = plane_state->ctl | skl_plane_ctl_crtc(crtc_state); + /* see intel_plane_atomic_calc_changes() */ + if (plane->need_async_flip_disable_wa && + crtc_state->async_flip_planes & BIT(plane->id)) + plane_ctl |= PLANE_CTL_ASYNC_FLIP; + if (DISPLAY_VER(dev_priv) >= 10) plane_color_ctl = plane_state->color_ctl | glk_plane_color_ctl_crtc(crtc_state);