| Message ID | 20211209182109.29786-3-ville.syrjala@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915/fbc: More multi-FBC refactoring | expand |
On Thu, 09 Dec 2021, Ville Syrjala <ville.syrjala@linux.intel.com> wrote: > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Convert i915->fbc into an array in preparation for > multiple FBC instances, and loop through all instances > in all places where the caller does not know which > instance(s) (if any) are relevant. This is the case > for eg. frontbuffer tracking and FIFO underrun hadling. > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> Reviewed-by: Jani Nikula <jani.nikula@intel.com> > --- > drivers/gpu/drm/i915/display/i9xx_plane.c | 2 +- > drivers/gpu/drm/i915/display/intel_fbc.c | 166 +++++++++++------- > .../drm/i915/display/skl_universal_plane.c | 2 +- > drivers/gpu/drm/i915/i915_drv.h | 3 +- > 4 files changed, 104 insertions(+), 69 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/i9xx_plane.c b/drivers/gpu/drm/i915/display/i9xx_plane.c > index 85950ff67609..731f446bdf20 100644 > --- a/drivers/gpu/drm/i915/display/i9xx_plane.c > +++ b/drivers/gpu/drm/i915/display/i9xx_plane.c > @@ -125,7 +125,7 @@ static struct intel_fbc *i9xx_plane_fbc(struct drm_i915_private *dev_priv, > enum i9xx_plane_id i9xx_plane) > { > if (i9xx_plane_has_fbc(dev_priv, i9xx_plane)) > - return dev_priv->fbc; > + return dev_priv->fbc[FBC_A]; > else > return NULL; > } > diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c > index 8376f819071e..2f1a72f98c4b 100644 > --- a/drivers/gpu/drm/i915/display/intel_fbc.c > +++ b/drivers/gpu/drm/i915/display/intel_fbc.c > @@ -49,6 +49,13 @@ > #include "intel_fbc.h" > #include "intel_frontbuffer.h" > > +#define for_each_fbc_id(__fbc_id) \ > + for ((__fbc_id) = FBC_A; (__fbc_id) < I915_MAX_FBCS; (__fbc_id)++) > + > +#define for_each_intel_fbc(__dev_priv, __fbc, __fbc_id) \ > + for_each_fbc_id(__fbc_id) \ > + for_each_if((__fbc) = (__dev_priv)->fbc[(__fbc_id)]) > + > struct intel_fbc_funcs { > void (*activate)(struct intel_fbc *fbc); > void (*deactivate)(struct intel_fbc *fbc); > @@ -812,16 +819,16 @@ static void __intel_fbc_cleanup_cfb(struct intel_fbc *fbc) > > void intel_fbc_cleanup(struct drm_i915_private *i915) > { > - struct intel_fbc *fbc = i915->fbc; > + struct intel_fbc *fbc; > + enum fbc_id fbc_id; > > - if (!fbc) > - return; > + for_each_intel_fbc(i915, fbc, fbc_id) { > + mutex_lock(&fbc->lock); > + __intel_fbc_cleanup_cfb(fbc); > + mutex_unlock(&fbc->lock); > > - mutex_lock(&fbc->lock); > - __intel_fbc_cleanup_cfb(fbc); > - mutex_unlock(&fbc->lock); > - > - kfree(fbc); > + kfree(fbc); > + } > } > > static bool stride_is_valid(const struct intel_plane_state *plane_state) > @@ -1307,36 +1314,39 @@ static unsigned int intel_fbc_get_frontbuffer_bit(struct intel_fbc *fbc) > return fbc->possible_framebuffer_bits; > } > > +static void __intel_fbc_invalidate(struct intel_fbc *fbc, > + unsigned int frontbuffer_bits, > + enum fb_op_origin origin) > +{ > + if (origin == ORIGIN_FLIP || origin == ORIGIN_CURSOR_UPDATE) > + return; > + > + mutex_lock(&fbc->lock); > + > + fbc->busy_bits |= intel_fbc_get_frontbuffer_bit(fbc) & frontbuffer_bits; > + > + if (fbc->state.plane && fbc->busy_bits) > + intel_fbc_deactivate(fbc, "frontbuffer write"); > + > + mutex_unlock(&fbc->lock); > +} > + > void intel_fbc_invalidate(struct drm_i915_private *i915, > unsigned int frontbuffer_bits, > enum fb_op_origin origin) > { > - struct intel_fbc *fbc = i915->fbc; > + struct intel_fbc *fbc; > + enum fbc_id fbc_id; > > - if (!fbc) > - return; > + for_each_intel_fbc(i915, fbc, fbc_id) > + __intel_fbc_invalidate(fbc, frontbuffer_bits, origin); > > - if (origin == ORIGIN_FLIP || origin == ORIGIN_CURSOR_UPDATE) > - return; > - > - mutex_lock(&fbc->lock); > - > - fbc->busy_bits |= intel_fbc_get_frontbuffer_bit(fbc) & frontbuffer_bits; > - > - if (fbc->state.plane && fbc->busy_bits) > - intel_fbc_deactivate(fbc, "frontbuffer write"); > - > - mutex_unlock(&fbc->lock); > } > > -void intel_fbc_flush(struct drm_i915_private *i915, > - unsigned int frontbuffer_bits, enum fb_op_origin origin) > +static void __intel_fbc_flush(struct intel_fbc *fbc, > + unsigned int frontbuffer_bits, > + enum fb_op_origin origin) > { > - struct intel_fbc *fbc = i915->fbc; > - > - if (!fbc) > - return; > - > mutex_lock(&fbc->lock); > > fbc->busy_bits &= ~frontbuffer_bits; > @@ -1356,6 +1366,17 @@ void intel_fbc_flush(struct drm_i915_private *i915, > mutex_unlock(&fbc->lock); > } > > +void intel_fbc_flush(struct drm_i915_private *i915, > + unsigned int frontbuffer_bits, > + enum fb_op_origin origin) > +{ > + struct intel_fbc *fbc; > + enum fbc_id fbc_id; > + > + for_each_intel_fbc(i915, fbc, fbc_id) > + __intel_fbc_flush(fbc, frontbuffer_bits, origin); > +} > + > int intel_fbc_atomic_check(struct intel_atomic_state *state) > { > struct intel_plane_state *plane_state; > @@ -1483,15 +1504,15 @@ void intel_fbc_update(struct intel_atomic_state *state, > */ > void intel_fbc_global_disable(struct drm_i915_private *i915) > { > - struct intel_fbc *fbc = i915->fbc; > + struct intel_fbc *fbc; > + enum fbc_id fbc_id; > > - if (!fbc) > - return; > - > - mutex_lock(&fbc->lock); > - if (fbc->state.plane) > - __intel_fbc_disable(fbc); > - mutex_unlock(&fbc->lock); > + for_each_intel_fbc(i915, fbc, fbc_id) { > + mutex_lock(&fbc->lock); > + if (fbc->state.plane) > + __intel_fbc_disable(fbc); > + mutex_unlock(&fbc->lock); > + } > } > > static void intel_fbc_underrun_work_fn(struct work_struct *work) > @@ -1516,19 +1537,9 @@ static void intel_fbc_underrun_work_fn(struct work_struct *work) > mutex_unlock(&fbc->lock); > } > > -/* > - * intel_fbc_reset_underrun - reset FBC fifo underrun status. > - * @i915: the i915 device > - * > - * See intel_fbc_handle_fifo_underrun_irq(). For automated testing we > - * want to re-enable FBC after an underrun to increase test coverage. > - */ > -void intel_fbc_reset_underrun(struct drm_i915_private *i915) > +static void __intel_fbc_reset_underrun(struct intel_fbc *fbc) > { > - struct intel_fbc *fbc = i915->fbc; > - > - if (!fbc) > - return; > + struct drm_i915_private *i915 = fbc->i915; > > cancel_work_sync(&fbc->underrun_work); > > @@ -1544,6 +1555,38 @@ void intel_fbc_reset_underrun(struct drm_i915_private *i915) > mutex_unlock(&fbc->lock); > } > > +/* > + * intel_fbc_reset_underrun - reset FBC fifo underrun status. > + * @i915: the i915 device > + * > + * See intel_fbc_handle_fifo_underrun_irq(). For automated testing we > + * want to re-enable FBC after an underrun to increase test coverage. > + */ > +void intel_fbc_reset_underrun(struct drm_i915_private *i915) > +{ > + struct intel_fbc *fbc; > + enum fbc_id fbc_id; > + > + for_each_intel_fbc(i915, fbc, fbc_id) > + __intel_fbc_reset_underrun(fbc); > +} > + > +static void __intel_fbc_handle_fifo_underrun_irq(struct intel_fbc *fbc) > +{ > + /* > + * There's no guarantee that underrun_detected won't be set to true > + * right after this check and before the work is scheduled, but that's > + * not a problem since we'll check it again under the work function > + * while FBC is locked. This check here is just to prevent us from > + * unnecessarily scheduling the work, and it relies on the fact that we > + * never switch underrun_detect back to false after it's true. > + */ > + if (READ_ONCE(fbc->underrun_detected)) > + return; > + > + schedule_work(&fbc->underrun_work); > +} > + > /** > * intel_fbc_handle_fifo_underrun_irq - disable FBC when we get a FIFO underrun > * @i915: i915 device > @@ -1560,21 +1603,11 @@ void intel_fbc_reset_underrun(struct drm_i915_private *i915) > */ > void intel_fbc_handle_fifo_underrun_irq(struct drm_i915_private *i915) > { > - struct intel_fbc *fbc = i915->fbc; > + struct intel_fbc *fbc; > + enum fbc_id fbc_id; > > - if (!fbc) > - return; > - > - /* There's no guarantee that underrun_detected won't be set to true > - * right after this check and before the work is scheduled, but that's > - * not a problem since we'll check it again under the work function > - * while FBC is locked. This check here is just to prevent us from > - * unnecessarily scheduling the work, and it relies on the fact that we > - * never switch underrun_detect back to false after it's true. */ > - if (READ_ONCE(fbc->underrun_detected)) > - return; > - > - schedule_work(&fbc->underrun_work); > + for_each_intel_fbc(i915, fbc, fbc_id) > + __intel_fbc_handle_fifo_underrun_irq(fbc); > } > > /* > @@ -1685,7 +1718,7 @@ void intel_fbc_init(struct drm_i915_private *i915) > if (intel_fbc_hw_is_active(fbc)) > intel_fbc_hw_deactivate(fbc); > > - i915->fbc = fbc; > + i915->fbc[fbc->id] = fbc; > } > > static int intel_fbc_debugfs_status_show(struct seq_file *m, void *unused) > @@ -1778,8 +1811,9 @@ static void intel_fbc_debugfs_add(struct intel_fbc *fbc) > > void intel_fbc_debugfs_register(struct drm_i915_private *i915) > { > - struct intel_fbc *fbc = i915->fbc; > + struct intel_fbc *fbc; > + enum fbc_id fbc_id; > > - if (fbc) > + for_each_intel_fbc(i915, fbc, fbc_id) > intel_fbc_debugfs_add(fbc); > } > diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c b/drivers/gpu/drm/i915/display/skl_universal_plane.c > index d5359cf3d270..9e31eb54b9f4 100644 > --- a/drivers/gpu/drm/i915/display/skl_universal_plane.c > +++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c > @@ -1829,7 +1829,7 @@ static struct intel_fbc *skl_plane_fbc(struct drm_i915_private *dev_priv, > enum pipe pipe, enum plane_id plane_id) > { > if (skl_plane_has_fbc(dev_priv, pipe, plane_id)) > - return dev_priv->fbc; > + return dev_priv->fbc[FBC_A]; > else > return NULL; > } > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index a0f54a69b11d..7ae62e8e6d02 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -70,6 +70,7 @@ > #include "display/intel_dmc.h" > #include "display/intel_dpll_mgr.h" > #include "display/intel_dsb.h" > +#include "display/intel_fbc.h" > #include "display/intel_frontbuffer.h" > #include "display/intel_global_state.h" > #include "display/intel_gmbus.h" > @@ -749,7 +750,7 @@ struct drm_i915_private { > u32 pipestat_irq_mask[I915_MAX_PIPES]; > > struct i915_hotplug hotplug; > - struct intel_fbc *fbc; > + struct intel_fbc *fbc[I915_MAX_FBCS]; > struct i915_drrs drrs; > struct intel_opregion opregion; > struct intel_vbt_data vbt;
diff --git a/drivers/gpu/drm/i915/display/i9xx_plane.c b/drivers/gpu/drm/i915/display/i9xx_plane.c index 85950ff67609..731f446bdf20 100644 --- a/drivers/gpu/drm/i915/display/i9xx_plane.c +++ b/drivers/gpu/drm/i915/display/i9xx_plane.c @@ -125,7 +125,7 @@ static struct intel_fbc *i9xx_plane_fbc(struct drm_i915_private *dev_priv, enum i9xx_plane_id i9xx_plane) { if (i9xx_plane_has_fbc(dev_priv, i9xx_plane)) - return dev_priv->fbc; + return dev_priv->fbc[FBC_A]; else return NULL; } diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c index 8376f819071e..2f1a72f98c4b 100644 --- a/drivers/gpu/drm/i915/display/intel_fbc.c +++ b/drivers/gpu/drm/i915/display/intel_fbc.c @@ -49,6 +49,13 @@ #include "intel_fbc.h" #include "intel_frontbuffer.h" +#define for_each_fbc_id(__fbc_id) \ + for ((__fbc_id) = FBC_A; (__fbc_id) < I915_MAX_FBCS; (__fbc_id)++) + +#define for_each_intel_fbc(__dev_priv, __fbc, __fbc_id) \ + for_each_fbc_id(__fbc_id) \ + for_each_if((__fbc) = (__dev_priv)->fbc[(__fbc_id)]) + struct intel_fbc_funcs { void (*activate)(struct intel_fbc *fbc); void (*deactivate)(struct intel_fbc *fbc); @@ -812,16 +819,16 @@ static void __intel_fbc_cleanup_cfb(struct intel_fbc *fbc) void intel_fbc_cleanup(struct drm_i915_private *i915) { - struct intel_fbc *fbc = i915->fbc; + struct intel_fbc *fbc; + enum fbc_id fbc_id; - if (!fbc) - return; + for_each_intel_fbc(i915, fbc, fbc_id) { + mutex_lock(&fbc->lock); + __intel_fbc_cleanup_cfb(fbc); + mutex_unlock(&fbc->lock); - mutex_lock(&fbc->lock); - __intel_fbc_cleanup_cfb(fbc); - mutex_unlock(&fbc->lock); - - kfree(fbc); + kfree(fbc); + } } static bool stride_is_valid(const struct intel_plane_state *plane_state) @@ -1307,36 +1314,39 @@ static unsigned int intel_fbc_get_frontbuffer_bit(struct intel_fbc *fbc) return fbc->possible_framebuffer_bits; } +static void __intel_fbc_invalidate(struct intel_fbc *fbc, + unsigned int frontbuffer_bits, + enum fb_op_origin origin) +{ + if (origin == ORIGIN_FLIP || origin == ORIGIN_CURSOR_UPDATE) + return; + + mutex_lock(&fbc->lock); + + fbc->busy_bits |= intel_fbc_get_frontbuffer_bit(fbc) & frontbuffer_bits; + + if (fbc->state.plane && fbc->busy_bits) + intel_fbc_deactivate(fbc, "frontbuffer write"); + + mutex_unlock(&fbc->lock); +} + void intel_fbc_invalidate(struct drm_i915_private *i915, unsigned int frontbuffer_bits, enum fb_op_origin origin) { - struct intel_fbc *fbc = i915->fbc; + struct intel_fbc *fbc; + enum fbc_id fbc_id; - if (!fbc) - return; + for_each_intel_fbc(i915, fbc, fbc_id) + __intel_fbc_invalidate(fbc, frontbuffer_bits, origin); - if (origin == ORIGIN_FLIP || origin == ORIGIN_CURSOR_UPDATE) - return; - - mutex_lock(&fbc->lock); - - fbc->busy_bits |= intel_fbc_get_frontbuffer_bit(fbc) & frontbuffer_bits; - - if (fbc->state.plane && fbc->busy_bits) - intel_fbc_deactivate(fbc, "frontbuffer write"); - - mutex_unlock(&fbc->lock); } -void intel_fbc_flush(struct drm_i915_private *i915, - unsigned int frontbuffer_bits, enum fb_op_origin origin) +static void __intel_fbc_flush(struct intel_fbc *fbc, + unsigned int frontbuffer_bits, + enum fb_op_origin origin) { - struct intel_fbc *fbc = i915->fbc; - - if (!fbc) - return; - mutex_lock(&fbc->lock); fbc->busy_bits &= ~frontbuffer_bits; @@ -1356,6 +1366,17 @@ void intel_fbc_flush(struct drm_i915_private *i915, mutex_unlock(&fbc->lock); } +void intel_fbc_flush(struct drm_i915_private *i915, + unsigned int frontbuffer_bits, + enum fb_op_origin origin) +{ + struct intel_fbc *fbc; + enum fbc_id fbc_id; + + for_each_intel_fbc(i915, fbc, fbc_id) + __intel_fbc_flush(fbc, frontbuffer_bits, origin); +} + int intel_fbc_atomic_check(struct intel_atomic_state *state) { struct intel_plane_state *plane_state; @@ -1483,15 +1504,15 @@ void intel_fbc_update(struct intel_atomic_state *state, */ void intel_fbc_global_disable(struct drm_i915_private *i915) { - struct intel_fbc *fbc = i915->fbc; + struct intel_fbc *fbc; + enum fbc_id fbc_id; - if (!fbc) - return; - - mutex_lock(&fbc->lock); - if (fbc->state.plane) - __intel_fbc_disable(fbc); - mutex_unlock(&fbc->lock); + for_each_intel_fbc(i915, fbc, fbc_id) { + mutex_lock(&fbc->lock); + if (fbc->state.plane) + __intel_fbc_disable(fbc); + mutex_unlock(&fbc->lock); + } } static void intel_fbc_underrun_work_fn(struct work_struct *work) @@ -1516,19 +1537,9 @@ static void intel_fbc_underrun_work_fn(struct work_struct *work) mutex_unlock(&fbc->lock); } -/* - * intel_fbc_reset_underrun - reset FBC fifo underrun status. - * @i915: the i915 device - * - * See intel_fbc_handle_fifo_underrun_irq(). For automated testing we - * want to re-enable FBC after an underrun to increase test coverage. - */ -void intel_fbc_reset_underrun(struct drm_i915_private *i915) +static void __intel_fbc_reset_underrun(struct intel_fbc *fbc) { - struct intel_fbc *fbc = i915->fbc; - - if (!fbc) - return; + struct drm_i915_private *i915 = fbc->i915; cancel_work_sync(&fbc->underrun_work); @@ -1544,6 +1555,38 @@ void intel_fbc_reset_underrun(struct drm_i915_private *i915) mutex_unlock(&fbc->lock); } +/* + * intel_fbc_reset_underrun - reset FBC fifo underrun status. + * @i915: the i915 device + * + * See intel_fbc_handle_fifo_underrun_irq(). For automated testing we + * want to re-enable FBC after an underrun to increase test coverage. + */ +void intel_fbc_reset_underrun(struct drm_i915_private *i915) +{ + struct intel_fbc *fbc; + enum fbc_id fbc_id; + + for_each_intel_fbc(i915, fbc, fbc_id) + __intel_fbc_reset_underrun(fbc); +} + +static void __intel_fbc_handle_fifo_underrun_irq(struct intel_fbc *fbc) +{ + /* + * There's no guarantee that underrun_detected won't be set to true + * right after this check and before the work is scheduled, but that's + * not a problem since we'll check it again under the work function + * while FBC is locked. This check here is just to prevent us from + * unnecessarily scheduling the work, and it relies on the fact that we + * never switch underrun_detect back to false after it's true. + */ + if (READ_ONCE(fbc->underrun_detected)) + return; + + schedule_work(&fbc->underrun_work); +} + /** * intel_fbc_handle_fifo_underrun_irq - disable FBC when we get a FIFO underrun * @i915: i915 device @@ -1560,21 +1603,11 @@ void intel_fbc_reset_underrun(struct drm_i915_private *i915) */ void intel_fbc_handle_fifo_underrun_irq(struct drm_i915_private *i915) { - struct intel_fbc *fbc = i915->fbc; + struct intel_fbc *fbc; + enum fbc_id fbc_id; - if (!fbc) - return; - - /* There's no guarantee that underrun_detected won't be set to true - * right after this check and before the work is scheduled, but that's - * not a problem since we'll check it again under the work function - * while FBC is locked. This check here is just to prevent us from - * unnecessarily scheduling the work, and it relies on the fact that we - * never switch underrun_detect back to false after it's true. */ - if (READ_ONCE(fbc->underrun_detected)) - return; - - schedule_work(&fbc->underrun_work); + for_each_intel_fbc(i915, fbc, fbc_id) + __intel_fbc_handle_fifo_underrun_irq(fbc); } /* @@ -1685,7 +1718,7 @@ void intel_fbc_init(struct drm_i915_private *i915) if (intel_fbc_hw_is_active(fbc)) intel_fbc_hw_deactivate(fbc); - i915->fbc = fbc; + i915->fbc[fbc->id] = fbc; } static int intel_fbc_debugfs_status_show(struct seq_file *m, void *unused) @@ -1778,8 +1811,9 @@ static void intel_fbc_debugfs_add(struct intel_fbc *fbc) void intel_fbc_debugfs_register(struct drm_i915_private *i915) { - struct intel_fbc *fbc = i915->fbc; + struct intel_fbc *fbc; + enum fbc_id fbc_id; - if (fbc) + for_each_intel_fbc(i915, fbc, fbc_id) intel_fbc_debugfs_add(fbc); } diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c b/drivers/gpu/drm/i915/display/skl_universal_plane.c index d5359cf3d270..9e31eb54b9f4 100644 --- a/drivers/gpu/drm/i915/display/skl_universal_plane.c +++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c @@ -1829,7 +1829,7 @@ static struct intel_fbc *skl_plane_fbc(struct drm_i915_private *dev_priv, enum pipe pipe, enum plane_id plane_id) { if (skl_plane_has_fbc(dev_priv, pipe, plane_id)) - return dev_priv->fbc; + return dev_priv->fbc[FBC_A]; else return NULL; } diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index a0f54a69b11d..7ae62e8e6d02 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -70,6 +70,7 @@ #include "display/intel_dmc.h" #include "display/intel_dpll_mgr.h" #include "display/intel_dsb.h" +#include "display/intel_fbc.h" #include "display/intel_frontbuffer.h" #include "display/intel_global_state.h" #include "display/intel_gmbus.h" @@ -749,7 +750,7 @@ struct drm_i915_private { u32 pipestat_irq_mask[I915_MAX_PIPES]; struct i915_hotplug hotplug; - struct intel_fbc *fbc; + struct intel_fbc *fbc[I915_MAX_FBCS]; struct i915_drrs drrs; struct intel_opregion opregion; struct intel_vbt_data vbt;