On Thu, Nov 28, 2019 at 04:48:04PM +0100, Maarten Lankhorst wrote:
> Op 27-11-2019 om 21:12 schreef Ville Syrjala:
> > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> >
> > The code assumes we can omit the cfb allocation once fbc
> > has been enabled once. That's nonsense. Let's try to
> > reallocate it if we need to.
> >
> > The code is still a mess, but maybe this is enough to get
> > fbc going in some cases where it initially underallocates
> > the cfb and there's no full modeset to fix it up.
> >
> > Cc: Daniel Drake <drake@xxxxxxxxxxxx>
> > Cc: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx>
> > Cc: Jian-Hong Pan <jian-hong@xxxxxxxxxxxx>
> > Cc: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> > ---
> > drivers/gpu/drm/i915/display/intel_fbc.c | 22 +++++++++++++++-------
> > 1 file changed, 15 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c
> > index c976698b0729..928059a5da80 100644
> > --- a/drivers/gpu/drm/i915/display/intel_fbc.c
> > +++ b/drivers/gpu/drm/i915/display/intel_fbc.c
> > @@ -672,6 +672,14 @@ static void intel_fbc_update_state_cache(struct intel_crtc *crtc,
> > cache->fence_id = -1;
> > }
> >
> > +static bool intel_fbc_cfb_size_changed(struct drm_i915_private *dev_priv)
> > +{
> > + struct intel_fbc *fbc = &dev_priv->fbc;
> > +
> > + return intel_fbc_calculate_cfb_size(dev_priv, &fbc->state_cache) >
> > + fbc->compressed_fb.size * fbc->threshold;
> > +}
> > +
> > static bool intel_fbc_can_activate(struct intel_crtc *crtc)
> > {
> > struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> > @@ -757,8 +765,7 @@ static bool intel_fbc_can_activate(struct intel_crtc *crtc)
> > * we didn't get any invalidate/deactivate calls, but this would require
> > * a lot of tracking just for a specific case. If we conclude it's an
> > * important case, we can implement it later. */
> > - if (intel_fbc_calculate_cfb_size(dev_priv, &fbc->state_cache) >
> > - fbc->compressed_fb.size * fbc->threshold) {
> > + if (intel_fbc_cfb_size_changed(dev_priv)) {
> > fbc->no_fbc_reason = "CFB requirements changed";
> > return false;
> > }
> > @@ -1112,12 +1119,12 @@ void intel_fbc_enable(struct intel_crtc *crtc,
> > mutex_lock(&fbc->lock);
> >
> > if (fbc->crtc) {
> > - WARN_ON(fbc->crtc == crtc && !crtc_state->enable_fbc);
> > - goto out;
> > - }
> > + if (fbc->crtc != crtc ||
> > + !intel_fbc_cfb_size_changed(dev_priv))
> > + goto out;
> >
> > - if (!crtc_state->enable_fbc)
> > - goto out;
> > + __intel_fbc_disable(dev_priv);
> > + }
> >
> > WARN_ON(fbc->active);
> >
> > @@ -1130,6 +1137,7 @@ void intel_fbc_enable(struct intel_crtc *crtc,
> > if (intel_fbc_alloc_cfb(dev_priv,
> > intel_fbc_calculate_cfb_size(dev_priv, cache),
> > fb->format->cpp[0])) {
> > + cache->plane.visible = false;
> > fbc->no_fbc_reason = "not enough stolen memory";
> > goto out;
> > }
>
> Makes sense, unfortunately kms_cursor_legacy starts failing on this series. :(
>
> For 1-11, 14
>
> Reviewed-by: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx>
>
> We should probably get rid of the FBC disable on frontbuffer disable as well. I had some patches but nothing upstream-worthy yet. :(
How would we get rid of the disable there? By triggering nukes at some
predefined interval? Doesn't sound all that great.
>
> 12 and 13 need more thought for now, kms_cursor_legacy is failing.
Already posted the v2 that fixes it.
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
