On Tue, Jun 16, 2015 at 04:46:40PM +0300, Ville Syrjälä wrote: > On Tue, Jun 16, 2015 at 03:40:16PM +0200, Daniel Vetter wrote: > > On Mon, Jun 15, 2015 at 09:03:09PM +0000, Konduru, Chandra wrote: > > > > > > > > > > Cdclk < crtc_clock is not allowed and suggests a different problem elsewhere. > > > > > > > > > > It is more robust and safe to assume no scaling is possible in this case. > > > > > > > > > > Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx> > > > > > Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > > > > --- > > > > > drivers/gpu/drm/i915/intel_display.c | 2 +- > > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c > > > > b/drivers/gpu/drm/i915/intel_display.c > > > > > index 93a5e51..4c99373 100644 > > > > > --- a/drivers/gpu/drm/i915/intel_display.c > > > > > +++ b/drivers/gpu/drm/i915/intel_display.c > > > > > @@ -13234,7 +13234,7 @@ skl_max_scale(struct intel_crtc *intel_crtc, struct > > > > intel_crtc_state *crtc_state > > > > > crtc_clock = crtc_state->base.adjusted_mode.crtc_clock; > > > > > cdclk = dev_priv->display.get_display_clock_speed(dev); > > > > > > > > Probably fallout from the in-flight dynamic cdclk stuff - this code checks > > > > the wrong bits I guess. Chandra? > > > > > > Looks like something elsewhere has fallen out and issue manifested here. > > > > > > Damien reported another issue where get_display_clock_speed causing > > > an assert because it is called when dev_priv->pm.suspend is true during > > > runtime resume. But later was resolved after one of atomic patch is > > > reverted. > > > > > > While Maarten is addressing recently reported atomic issues, for > > > time being some atomic crtc patches were reverted. > > > > > > I am not 100% sure whether issue here is due to same root cause or > > > due to something different. > > > > You need to check the cached (and soon the one in the global atomic > > modeset state structure) cdclk value, not the current one in the hw. And > > yeah that can result in asserts since the hw might not be one yet when > > this code is run. I.e. this isn't about atomic modeset but just about > > interaction with the recent cdclk work. And with the existing rpm feature. > > > > In the future we should even upclock the cdclck stuff (once dynamic cdclk > > is implemented on skl) to make sure it fits the desired scaler > > configuration. But that's follow-up work. > > For something like that we probably need some kind of new property to > request extra cdclk headroom when doing a modeset. Otherwise we're going > to end up blinking the displays all the time. Userspace can avoid the blinking by not setting the ALLOW_MODESET flag. Then we'll reject the atomic update if it would require a cdclk change. Same on the downclocking, we'd need to not force a modeset if userspace doesn't one one. We might still need some explicit headroom perhaps on top of this. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
