Re: [RFC 7/8] drm/i915: Allow final wm programming to be scheduled after next vblank (v2)

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Mon, Jul 06, 2015 at 11:07:52AM +0200, Daniel Vetter wrote:
> On Wed, Jul 01, 2015 at 07:26:00PM -0700, Matt Roper wrote:
> > Add a simple mechanism to trigger final watermark updates in an
> > asynchronous manner once the next vblank occurs.  No platform types
> > actually support atomic watermark programming until a future patch, so
> > there should be no functional change yet; individual platforms will be
> > converted to use this mechanism one-by-one in future patches.
> > 
> > Note that we'll probably expand this to cover other post-vblank async
> > tasks (like unpinning) at some point in the future.
> > 
> > v2: Much simpler vblank mechanism than was used in the previous series;
> >     no need to allocate new heap structures.
> > 
> > Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_drv.h      |  7 +++++++
> >  drivers/gpu/drm/i915/i915_irq.c      |  9 +++++++++
> >  drivers/gpu/drm/i915/intel_display.c | 30 ++++++++++++++++++++++++++----
> >  drivers/gpu/drm/i915/intel_drv.h     |  4 ++++
> >  4 files changed, 46 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index 2774976..5ad942e 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -628,6 +628,7 @@ struct drm_i915_display_funcs {
> >  				 struct drm_crtc *crtc,
> >  				 uint32_t sprite_width, uint32_t sprite_height,
> >  				 int pixel_size, bool enable, bool scaled);
> > +	void (*program_watermarks)(struct drm_i915_private *dev_priv);
> >  	int (*modeset_calc_cdclk)(struct drm_atomic_state *state);
> >  	void (*modeset_commit_cdclk)(struct drm_atomic_state *state);
> >  	/* Returns the active state of the crtc, and if the crtc is active,
> > @@ -2567,6 +2568,12 @@ struct drm_i915_cmd_table {
> >  #define HAS_L3_DPF(dev) (IS_IVYBRIDGE(dev) || IS_HASWELL(dev))
> >  #define NUM_L3_SLICES(dev) (IS_HSW_GT3(dev) ? 2 : HAS_L3_DPF(dev))
> >  
> > +/*
> > + * FIXME:  Not all platforms have been transitioned to atomic watermark
> > + * updates yet.
> > + */
> > +#define HAS_ATOMIC_WM(dev_priv) (dev_priv->display.program_watermarks != NULL)
> 
> HAS_FOO is generally hw features. I think especially for just this vfunc
> check is clearer to inline it.
> 
> > +
> >  #define GT_FREQUENCY_MULTIPLIER 50
> >  #define GEN9_FREQ_SCALER 3
> >  
> > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> > index a6fbe64..20c7260 100644
> > --- a/drivers/gpu/drm/i915/i915_irq.c
> > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > @@ -1452,6 +1452,15 @@ static void gen6_rps_irq_handler(struct drm_i915_private *dev_priv, u32 pm_iir)
> >  
> >  static bool intel_pipe_handle_vblank(struct drm_device *dev, enum pipe pipe)
> >  {
> > +	struct drm_i915_private *dev_priv = to_i915(dev);
> > +	struct drm_crtc *crtc = dev_priv->pipe_to_crtc_mapping[pipe];
> > +	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> > +
> > +	if (intel_crtc->need_vblank_wm_update) {
> > +		queue_work(dev_priv->wq, &intel_crtc->wm_work);
> > +		intel_crtc->need_vblank_wm_update = false;
> 
> We need some lock or some other means of sync to be able to cancel such an
> update if userspace submits the next atomic update. Otherwise this work
> might overwrite the intermediate wm values.

This series seems to be missing a bunch of stuff from my ILK wm rework,
including the wm.mutex.

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx