On Thu, May 04, 2017 at 03:20:22PM +0200, Daniel Vetter wrote:
On Wed, May 03, 2017 at 05:09:08PM +0300, Ville Syrjälä wrote:
On Wed, May 03, 2017 at 09:26:38AM +0200, Daniel Vetter wrote:
In the previous patch we've implemented hwmode tracking a la i915 for the vblank timestamp calculations. But that was just the basic semantics, i915 has some nice sanity checks to make sure we keep getting this right. Move them over too.
Cc: Ville Syrjälä ville.syrjala@linux.intel.com Reviewed-by: Neil Armstrong narmstrong@baylibre.com Signed-off-by: Daniel Vetter daniel.vetter@intel.com
drivers/gpu/drm/drm_irq.c | 8 +++++++- drivers/gpu/drm/i915/i915_irq.c | 10 ++++++---- drivers/gpu/drm/i915/intel_display.c | 11 ++--------- 3 files changed, 15 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c index 89f0928b042a..942183a2aa3c 100644 --- a/drivers/gpu/drm/drm_irq.c +++ b/drivers/gpu/drm/drm_irq.c @@ -775,8 +775,10 @@ bool drm_calc_vbltimestamp_from_scanoutpos(struct drm_device *dev, /* If mode timing undefined, just return as no-op: * Happens during initial modesetting of a crtc. */
- if (mode->crtc_clock == 0) {
- if (WARN_ON(mode->crtc_clock == 0)) { DRM_DEBUG("crtc %u: Noop due to uninitialized mode.\n", pipe);
WARN_ON(drm_drv_uses_atomic_modeset(dev));I would make these _ONCE() otherwise the machine might end up practically dead.
Will do.
- return false; }
@@ -1338,6 +1340,10 @@ void drm_crtc_vblank_off(struct drm_crtc *crtc) send_vblank_event(dev, e, seq, &now); } spin_unlock_irqrestore(&dev->event_lock, irqflags);
- /* Will be reset by the modeset helpers when re-enabling the crtc by
* calling drm_calc_timestamping_constants(). */- vblank->hwmode.crtc_clock = 0;
} EXPORT_SYMBOL(drm_crtc_vblank_off);
Shouldn't we do this in drm_crtc_vblank_reset() as well?
Hmm. Except we call that after drm_calc_timestamping_constants(). I guess we should be able to move the reset() into intel_modeset_readout_hw_state(). And possibly move the vblank_on() call as well?
Yeah, it'd be nice to clean this stuff up some more, but there's also the problem that legacy and new drivers callc drm_calc_timestamping_constants at opposite ends of the modeset sequence. Doing more here is a bunch more work, maybe for the next patche series ...
I don't think we need to call it in _reset, at least at boot-up it should be 0 already. And for s/r we already shut down the pipe on suspend, so it's gone through this here.
Hmm. Indeed that seems like it should cover it.
With the _ONCE nit address (and the build breakage I've introduced in this version fixed), ack from you on the entire series?
Sure, for the series Acked-by: Ville Syrjälä ville.syrjala@linux.intel.com