[Intel-gfx] [PATCH 7/8] drm/i915: Shrink active_crtcs and active_pipes_changed to u8

[Chris Wilson]

Quoting Ville Syrjala (2017-09-13 15:08:59)
> From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> 
> We only have three pipes, so 8 bits is more than sufficient to track
> which is active. Also start using BIT() when populating them.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h      |  2 +-
>  drivers/gpu/drm/i915/intel_display.c | 10 +++++-----
>  drivers/gpu/drm/i915/intel_drv.h     |  4 ++--
>  3 files changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index d4ec44495c00..f8d7599cff43 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2383,7 +2383,7 @@ struct drm_i915_private {
>          */
>         struct mutex dpll_lock;
>  
> -       unsigned int active_crtcs;
> +       u8 active_crtcs;
>         /* minimum acceptable cdclk for each pipe */
>         int min_cdclk[I915_MAX_PIPES];
>  
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 35fe8a98080e..c33fce9d9824 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -6032,7 +6032,7 @@ static void intel_crtc_disable_noatomic(struct drm_crtc *crtc,
>                 intel_display_power_put(dev_priv, domain);
>         intel_crtc->enabled_power_domains = 0;
>  
> -       dev_priv->active_crtcs &= ~(1 << intel_crtc->pipe);
> +       dev_priv->active_crtcs &= ~BIT(intel_crtc->pipe);
>         dev_priv->min_cdclk[intel_crtc->pipe] = 0;
>  }
>  
> @@ -11956,12 +11956,12 @@ static int intel_modeset_checks(struct drm_atomic_state *state)
>  
>         for_each_oldnew_intel_crtc_in_state(intel_state, crtc, old_crtc_state, new_crtc_state, i) {
>                 if (new_crtc_state->base.active)
> -                       intel_state->active_crtcs |= 1 << i;
> +                       intel_state->active_crtcs |= BIT(crtc->pipe);
>                 else
> -                       intel_state->active_crtcs &= ~(1 << i);
> +                       intel_state->active_crtcs &= ~BIT(crtc->pipe);

There's an hweight32(intel_state->active_crtcs & (drm_crtc_mask(for_crtc) - 1))

mixing drm_crtc_mask() and BIT(crtc->pipe) leaves a nasty taste. And
lots of hweight32() to cleanup.
>  
>                 if (old_crtc_state->base.active != new_crtc_state->base.active)
> -                       intel_state->active_pipe_changes |= drm_crtc_mask(&crtc->base);
> +                       intel_state->active_pipe_changes |= BIT(crtc->pipe);

Hmm. Looks very odd to be so isolated. It's treated as a bool which
explains why there's no corresponding clear_bit().
-Chris