[Intel-gfx] [PATCH 11/9] drm/i915: Opt out of vblank disable timer on >gen2

[Paulo Zanoni]

2014-05-26 11:26 GMT-03:00  <ville.syrjala at linux.intel.com>:
> From: Ville Syrjälä <ville.syrjala at linux.intel.com>
>
> Now that the vblank races are plugged, we can opt out of using
> the vblank disable timer and just let vblank interrupts get
> disabled immediately when the last reference is dropped.
>
> Gen2 is the exception since it has no hardware frame counter.

Hi

Remember last week's FBC vblank optimization patch that had an
erroneous drm_crtc_vblank_get() instead of drm_crtc_vblank_count()?
After I fixed the bug in the patch I realized that it was the
unbalanced vblank_get() call that moved PC state residency up.

I did some experiments, and on my specific BDW machine, after running
"powertop --auto-tune", I get about 15-25% PC7 residency without FBC.
If I revert this patch, the number jumps to 40-45%. With FBC, the PC7
residency goes from 60-70% to 85-90% when I revert this patch. I'm
running just an idle Cinnamon with an open terminal.

So, since the commit message lacks more details, what are the
downsides of reverting this patch? What are the advantages of opting
out of the vblank timer? I see my desktop does tons and tons of vblank
get/put calls per second, so the disable timer makes a lot of sense.

I also wish there was some easy way to check how this patch (or its
revert) affect a bunch of different workloads...

(Also CCing Chris for insightful comments on performance)

Thanks,
Paulo

>
> Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_irq.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 28bae6e..4b2e7af 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -4364,6 +4364,14 @@ void intel_irq_init(struct drm_device *dev)
>                 dev->max_vblank_count = 0xffffff; /* only 24 bits of frame count */
>         }
>
> +       /*
> +        * Opt out of the vblank disable timer on everything except gen2.
> +        * Gen2 doesn't have a hardware frame counter and so depends on
> +        * vblank interrupts to produce sane vblank seuquence numbers.
> +        */
> +       if (!IS_GEN2(dev))
> +               dev->vblank_disable_immediate = true;
> +
>         if (drm_core_check_feature(dev, DRIVER_MODESET)) {
>                 dev->driver->get_vblank_timestamp = i915_get_vblank_timestamp;
>                 dev->driver->get_scanout_position = i915_get_crtc_scanoutpos;
> --
> 1.8.5.5
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Paulo Zanoni