From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ed1-f67.google.com ([209.85.208.67]:42904 "EHLO mail-ed1-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725752AbeK1GlL (ORCPT ); Wed, 28 Nov 2018 01:41:11 -0500 Received: by mail-ed1-f67.google.com with SMTP id j6so20044981edp.9 for ; Tue, 27 Nov 2018 11:42:12 -0800 (PST) Date: Tue, 27 Nov 2018 20:42:08 +0100 From: Daniel Vetter To: Ville Syrjala Cc: intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org, stable@vger.kernel.org, Inki Dae , Daniel Vetter Subject: Re: [PATCH v2 01/15] drm/vblank: Allow dynamic per-crtc max_vblank_count Message-ID: <20181127194208.GD4266@phenom.ffwll.local> References: <20181112170000.27531-2-ville.syrjala@linux.intel.com> <20181127182004.28885-1-ville.syrjala@linux.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20181127182004.28885-1-ville.syrjala@linux.intel.com> Sender: stable-owner@vger.kernel.org List-ID: On Tue, Nov 27, 2018 at 08:20:04PM +0200, Ville Syrjala wrote: > From: Ville Syrj�l� > > On i965gm we need to adjust max_vblank_count dynamically > depending on whether the TV encoder is used or not. To > that end add a per-crtc max_vblank_count that takes > precedence over its device wide counterpart. The driver > can now call drm_crtc_set_max_vblank_count() to configure > the per-crtc value before calling drm_vblank_on(). > > Also looks like there was some discussion about exynos needing > similar treatment. > > v2: Drop the extra max_vblank_count!=0 check for the > WARN(last!=current), will take care of it in i915 code (Daniel) > WARN_ON(!inmodeset) (Daniel) > WARN_ON(dev->max_vblank_count) > Pimp up the docs (Daniel) Yeah I'm happy with the pile of WARN_ON here. And docs look great too. Reviewed-by: Daniel Vetter > > Cc: stable@vger.kernel.org > Cc: Inki Dae > Cc: Daniel Vetter > Signed-off-by: Ville Syrj�l� > --- > drivers/gpu/drm/drm_vblank.c | 45 +++++++++++++++++++++++++++++++++--- > include/drm/drm_device.h | 8 ++++++- > include/drm/drm_vblank.h | 22 ++++++++++++++++++ > 3 files changed, 71 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c > index 98e091175921..cde71ee95a8f 100644 > --- a/drivers/gpu/drm/drm_vblank.c > +++ b/drivers/gpu/drm/drm_vblank.c > @@ -105,13 +105,20 @@ static void store_vblank(struct drm_device *dev, unsigned int pipe, > write_sequnlock(&vblank->seqlock); > } > > +static u32 drm_max_vblank_count(struct drm_device *dev, unsigned int pipe) > +{ > + struct drm_vblank_crtc *vblank = &dev->vblank[pipe]; > + > + return vblank->max_vblank_count ?: dev->max_vblank_count; > +} > + > /* > * "No hw counter" fallback implementation of .get_vblank_counter() hook, > * if there is no useable hardware frame counter available. > */ > static u32 drm_vblank_no_hw_counter(struct drm_device *dev, unsigned int pipe) > { > - WARN_ON_ONCE(dev->max_vblank_count != 0); > + WARN_ON_ONCE(drm_max_vblank_count(dev, pipe) != 0); > return 0; > } > > @@ -198,6 +205,7 @@ static void drm_update_vblank_count(struct drm_device *dev, unsigned int pipe, > ktime_t t_vblank; > int count = DRM_TIMESTAMP_MAXRETRIES; > int framedur_ns = vblank->framedur_ns; > + u32 max_vblank_count = drm_max_vblank_count(dev, pipe); > > /* > * Interrupts were disabled prior to this call, so deal with counter > @@ -216,9 +224,9 @@ static void drm_update_vblank_count(struct drm_device *dev, unsigned int pipe, > rc = drm_get_last_vbltimestamp(dev, pipe, &t_vblank, in_vblank_irq); > } while (cur_vblank != __get_vblank_counter(dev, pipe) && --count > 0); > > - if (dev->max_vblank_count != 0) { > + if (max_vblank_count) { > /* trust the hw counter when it's around */ > - diff = (cur_vblank - vblank->last) & dev->max_vblank_count; > + diff = (cur_vblank - vblank->last) & max_vblank_count; > } else if (rc && framedur_ns) { > u64 diff_ns = ktime_to_ns(ktime_sub(t_vblank, vblank->time)); > > @@ -1204,6 +1212,37 @@ void drm_crtc_vblank_reset(struct drm_crtc *crtc) > } > EXPORT_SYMBOL(drm_crtc_vblank_reset); > > +/** > + * drm_crtc_set_max_vblank_count - configure the hw max vblank counter value > + * @crtc: CRTC in question > + * @max_vblank_count: max hardware vblank counter value > + * > + * Update the maximum hardware vblank counter value for @crtc > + * at runtime. Useful for hardware where the operation of the > + * hardware vblank counter depends on the currently active > + * display configuration. > + * > + * For example, if the hardware vblank counter does not work > + * when a specific connector is active the maximum can be set > + * to zero. And when that specific connector isn't active the > + * maximum can again be set to the appropriate non-zero value. > + * > + * If used, must be called before drm_vblank_on(). > + */ > +void drm_crtc_set_max_vblank_count(struct drm_crtc *crtc, > + u32 max_vblank_count) > +{ > + struct drm_device *dev = crtc->dev; > + unsigned int pipe = drm_crtc_index(crtc); > + struct drm_vblank_crtc *vblank = &dev->vblank[pipe]; > + > + WARN_ON(dev->max_vblank_count); > + WARN_ON(!READ_ONCE(vblank->inmodeset)); > + > + vblank->max_vblank_count = max_vblank_count; > +} > +EXPORT_SYMBOL(drm_crtc_set_max_vblank_count); > + > /** > * drm_crtc_vblank_on - enable vblank events on a CRTC > * @crtc: CRTC in question > diff --git a/include/drm/drm_device.h b/include/drm/drm_device.h > index 42411b3ea0c8..45e43ce9652f 100644 > --- a/include/drm/drm_device.h > +++ b/include/drm/drm_device.h > @@ -184,7 +184,13 @@ struct drm_device { > * races and imprecision over longer time periods, hence exposing a > * hardware vblank counter is always recommended. > * > - * If non-zeor, &drm_crtc_funcs.get_vblank_counter must be set. > + * This is the statically configured device wide maximum. The driver > + * can instead choose to use a runtime configurable per-crtc value > + * &drm_vblank_crtc.max_vblank_count, in which case @max_vblank_count > + * must be left at zero. See drm_crtc_set_max_vblank_count() on how > + * to use the per-crtc value. > + * > + * If non-zero, &drm_crtc_funcs.get_vblank_counter must be set. > */ > u32 max_vblank_count; /**< size of vblank counter register */ > > diff --git a/include/drm/drm_vblank.h b/include/drm/drm_vblank.h > index 6ad9630d4f48..e528bb2f659d 100644 > --- a/include/drm/drm_vblank.h > +++ b/include/drm/drm_vblank.h > @@ -128,6 +128,26 @@ struct drm_vblank_crtc { > * @last: Protected by &drm_device.vbl_lock, used for wraparound handling. > */ > u32 last; > + /** > + * @max_vblank_count: > + * > + * Maximum value of the vblank registers for this crtc. This value +1 > + * will result in a wrap-around of the vblank register. It is used > + * by the vblank core to handle wrap-arounds. > + * > + * If set to zero the vblank core will try to guess the elapsed vblanks > + * between times when the vblank interrupt is disabled through > + * high-precision timestamps. That approach is suffering from small > + * races and imprecision over longer time periods, hence exposing a > + * hardware vblank counter is always recommended. > + * > + * This is the runtime configurable per-crtc maximum set through > + * drm_crtc_set_max_vblank_count(). If this is used the driver > + * must leave the device wide &drm_device.max_vblank_count at zero. > + * > + * If non-zero, &drm_crtc_funcs.get_vblank_counter must be set. > + */ > + u32 max_vblank_count; > /** > * @inmodeset: Tracks whether the vblank is disabled due to a modeset. > * For legacy driver bit 2 additionally tracks whether an additional > @@ -206,4 +226,6 @@ bool drm_calc_vbltimestamp_from_scanoutpos(struct drm_device *dev, > void drm_calc_timestamping_constants(struct drm_crtc *crtc, > const struct drm_display_mode *mode); > wait_queue_head_t *drm_crtc_vblank_waitqueue(struct drm_crtc *crtc); > +void drm_crtc_set_max_vblank_count(struct drm_crtc *crtc, > + u32 max_vblank_count); > #endif > -- > 2.18.1 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch