On Wed, Sep 13, 2017 at 03:49:10PM +0100, Chris Wilson wrote:
> Quoting Ville Syrjala (2017-09-13 15:08:59)
> > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> >
> > 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@xxxxxxxxxxxxxxx>
> > ---
> > 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.
Hmm. Looks like that mixup goes even deeper in the SKL+ wm code. Not
sure I want to try untangling it all.
> 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
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
