[Intel-gfx] [PATCH 04/24] drm/i915: Update power domains only on affected crtc's.

[Daniel Vetter]

On Wed, Jun 03, 2015 at 08:52:52AM +0200, Maarten Lankhorst wrote:
> Op 03-06-15 om 03:27 schreef Matt Roper:
> > On Mon, Jun 01, 2015 at 03:27:07PM +0200, Maarten Lankhorst wrote:
> >> Use for_each_crtc_state to only touch affected crtc's.
> >> In order to make sure that the initial power is still set
> >> correctly we make sure modeset_update_crtc_power_domains is called
> >> during the initial modeset.
> >>
> >> Signed-off-by: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
> >> ---
> >>  drivers/gpu/drm/i915/i915_drv.c      |  3 ---
> >>  drivers/gpu/drm/i915/intel_display.c | 41 +++++++++++++++++++++++-------------
> >>  2 files changed, 26 insertions(+), 18 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> >> index d3632c56fdf7..78ef0bb53c36 100644
> >> --- a/drivers/gpu/drm/i915/i915_drv.c
> >> +++ b/drivers/gpu/drm/i915/i915_drv.c
> >> @@ -634,9 +634,6 @@ static int i915_drm_suspend(struct drm_device *dev)
> >>  	intel_display_suspend(dev);
> >>  	drm_modeset_unlock_all(dev);
> >>  
> >> -	/* suspending displays will unsets init power */
> >> -	intel_display_set_init_power(dev_priv, true);
> >> -
> >>  	intel_dp_mst_suspend(dev);
> >>  
> >>  	intel_runtime_pm_disable_interrupts(dev_priv);
> >> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> >> index 8e9afc55c284..4dc07602248b 100644
> >> --- a/drivers/gpu/drm/i915/intel_display.c
> >> +++ b/drivers/gpu/drm/i915/intel_display.c
> >> @@ -5188,42 +5188,49 @@ static unsigned long get_crtc_power_domains(struct drm_crtc *crtc)
> >>  	return mask;
> >>  }
> >>  
> >> -static void modeset_update_crtc_power_domains(struct drm_atomic_state *state)
> >> +static void modeset_update_crtc_power_domains(struct drm_atomic_state *state, bool gr)
> > What does 'gr' stand for and what does the parameter signify?  It seems
> > to just gate whether we call display.modeset_global_resources, but it's
> > unclear to me from the commit message above in which situations we
> > would/wouldn't want to do this and why.
> >
> Well there's no point if no modeset is done to call display.modeset_global_resources. But I guess
> calling it power_only might be better. I wish I knew why modeset_global_resources was done in the middle,
> I think there's no point to do so.

When doing global changes like updating the cdclk we better do that only
when the hardware is guaranteed to be on. Since we could update global
things both when enabling and when disabling, the only place where the hw
is on in either case is in the middle.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch