On Tue, 2018-01-02 at 18:33 +0000, Pandiyan, Dhinakaran wrote: > On Tue, 2017-12-19 at 13:16 +0100, Maarten Lankhorst wrote: > > This should get rid of unclaimed register debug warnings, if > > it still happens we should put this in a intel_crtc->active check.. > > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=104172 > > > The bugzilla indicates this is a regression from > "drm/i915: Restore GT performance in headless mode with DMC loaded", > which seems a bit odd to me. That patch should not have disabled a power > well which was enabled before. If anything, it should fix unclaimed > register accesses. > The comments in the bug report don't really confirm that the unclaimed register access is a regression from "drm/i915: Restore GT performance in headless mode with DMC loaded". > > > > Signed-off-by: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx> > > --- > > drivers/gpu/drm/i915/intel_pm.c | 10 +++++++--- > > 1 file changed, 7 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > > index ab6f1b770891..52d157c00535 100644 > > --- a/drivers/gpu/drm/i915/intel_pm.c > > +++ b/drivers/gpu/drm/i915/intel_pm.c > > @@ -5477,6 +5477,11 @@ void skl_pipe_wm_get_hw_state(struct drm_crtc *crtc, > > int level, max_level; > > enum plane_id plane_id; > > uint32_t val; > > + enum intel_display_power_domain power_domain; > > + > > + power_domain = POWER_DOMAIN_PIPE(pipe); > > + if (!intel_display_power_get_if_enabled(dev_priv, power_domain)) > > + return; > > > > max_level = ilk_wm_max_level(dev_priv); > > > > @@ -5500,10 +5505,9 @@ void skl_pipe_wm_get_hw_state(struct drm_crtc *crtc, > > skl_wm_level_from_reg_val(val, &wm->trans_wm); > > } > > > > - if (!intel_crtc->active) > > - return; > > - > > Doing a power_domain_get_if_enabled() before reading the register seems > like the right thing to do, but is power_get_if_enabled() expected to be > correct at this point? The reason I ask is, > modeset_get_crtc_power_domains() is called after skl_wm_get_hw_state(). > So, doesn't that mean the pipe power domain might not have been acquired > even if the CRTC is active. To answer my own question, POWER_DOMAIN_INIT should have taken care of this. So, this looks good to me. > > > > > out->linetime = I915_READ(PIPE_WM_LINETIME(pipe)); This looks okay too, but can you confirm updating this field for inactive CRTCs is not a problem? With that, Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@xxxxxxxxx> > > + > > + intel_display_power_put(dev_priv, power_domain); > > } > > > > void skl_wm_get_hw_state(struct drm_device *dev) > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx > https://lists.freedesktop.org/mailman/listinfo/intel-gfx _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
