[01/14] drm/i915: Track visible planes in a bitmask
diff mbox

Message ID 1481574931-8658-2-git-send-email-ville.syrjala@linux.intel.com
State New
Headers show

Commit Message

Ville Syrjälä Dec. 12, 2016, 8:35 p.m. UTC
From: Ville Syrjälä <ville.syrjala@linux.intel.com>

In a lot of place we wish to know which planes on the crtc are actually
visible, or how many of them there are. Let's start tracking that in a
bitmask in the crtc state.

We already track enabled planes (ie. ones with an fb and crtc specified by
the user) but that's not quite the same thing as enabled planes may
still end up being invisible due to clipping and whatnot.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_atomic_plane.c |  6 +++
 drivers/gpu/drm/i915/intel_display.c      | 79 +++++++++++++++++++++----------
 drivers/gpu/drm/i915/intel_drv.h          |  3 ++
 3 files changed, 64 insertions(+), 24 deletions(-)

Comments

Maarten Lankhorst Dec. 15, 2016, 2:56 p.m. UTC | #1
Op 12-12-16 om 21:35 schreef ville.syrjala@linux.intel.com:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> In a lot of place we wish to know which planes on the crtc are actually
> visible, or how many of them there are. Let's start tracking that in a
> bitmask in the crtc state.
>
> We already track enabled planes (ie. ones with an fb and crtc specified by
> the user) but that's not quite the same thing as enabled planes may
> still end up being invisible due to clipping and whatnot.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_atomic_plane.c |  6 +++
>  drivers/gpu/drm/i915/intel_display.c      | 79 +++++++++++++++++++++----------
>  drivers/gpu/drm/i915/intel_drv.h          |  3 ++
>  3 files changed, 64 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c b/drivers/gpu/drm/i915/intel_atomic_plane.c
> index dbe9fb41ae53..7ec12edda4d4 100644
> --- a/drivers/gpu/drm/i915/intel_atomic_plane.c
> +++ b/drivers/gpu/drm/i915/intel_atomic_plane.c
> @@ -181,6 +181,12 @@ static int intel_plane_atomic_check(struct drm_plane *plane,
>  	if (ret)
>  		return ret;
>  
> +	/* FIXME pre-g4x don't work like this */
> +	if (intel_state->base.visible)
> +		crtc_state->active_planes |= BIT(intel_plane->id);
> +	else
> +		crtc_state->active_planes &= ~BIT(intel_plane->id);
> +
>  	return intel_plane_atomic_calc_changes(&crtc_state->base, state);
>  }
>  
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index bc1af87789bc..3f027341b0f3 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2741,6 +2741,29 @@ update_state_fb(struct drm_plane *plane)
>  }
>  
>  static void
> +intel_set_plane_visible(struct intel_crtc_state *crtc_state,
> +			struct intel_plane_state *plane_state,
> +			bool visible)
> +{
> +	struct intel_plane *plane = to_intel_plane(plane_state->base.plane);
> +
> +	plane_state->base.visible = visible;
> +
> +	/* FIXME pre-g4x don't work like this */
> +	if (visible) {
> +		crtc_state->base.plane_mask |= BIT(drm_plane_index(&plane->base));
> +		crtc_state->active_planes |= BIT(plane->id);
> +	} else {
> +		crtc_state->base.plane_mask &= ~BIT(drm_plane_index(&plane->base));
> +		crtc_state->active_planes &= ~BIT(plane->id);
> +	}
> +
> +	DRM_DEBUG_KMS("%s active planes 0x%x\n",
> +		      crtc_state->base.crtc->name,
> +		      crtc_state->active_planes);
> +}
> +
> +static void
>  intel_find_initial_plane_obj(struct intel_crtc *intel_crtc,
>  			     struct intel_initial_plane_config *plane_config)
>  {
> @@ -2798,8 +2821,9 @@ intel_find_initial_plane_obj(struct intel_crtc *intel_crtc,
>  	 * simplest solution is to just disable the primary plane now and
>  	 * pretend the BIOS never had it enabled.
>  	 */
> -	to_intel_plane_state(plane_state)->base.visible = false;
> -	crtc_state->plane_mask &= ~(1 << drm_plane_index(primary));
> +	intel_set_plane_visible(to_intel_crtc_state(crtc_state),
> +				to_intel_plane_state(plane_state),
> +				false);
>  	intel_pre_disable_primary_noatomic(&intel_crtc->base);
>  	intel_plane->disable_plane(primary, &intel_crtc->base);
>  
> @@ -2826,7 +2850,11 @@ intel_find_initial_plane_obj(struct intel_crtc *intel_crtc,
>  	drm_framebuffer_reference(fb);
>  	primary->fb = primary->state->fb = fb;
>  	primary->crtc = primary->state->crtc = &intel_crtc->base;
> -	intel_crtc->base.state->plane_mask |= (1 << drm_plane_index(primary));
> +
> +	intel_set_plane_visible(to_intel_crtc_state(intel_crtc->base.state),
> +				to_intel_plane_state(primary->state),
> +				true);
> +
>  	atomic_or(to_intel_plane(primary)->frontbuffer_bit,
>  		  &obj->frontbuffer_bits);
>  }
> @@ -12440,11 +12468,11 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
>  	struct intel_crtc_state *pipe_config = to_intel_crtc_state(crtc_state);
>  	struct drm_crtc *crtc = crtc_state->crtc;
>  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> -	struct drm_plane *plane = plane_state->plane;
> +	struct intel_plane *plane = to_intel_plane(plane_state->plane);
>  	struct drm_device *dev = crtc->dev;
>  	struct drm_i915_private *dev_priv = to_i915(dev);
>  	struct intel_plane_state *old_plane_state =
> -		to_intel_plane_state(plane->state);
> +		to_intel_plane_state(plane->base.state);
>  	bool mode_changed = needs_modeset(crtc_state);
>  	bool was_crtc_enabled = crtc->state->active;
>  	bool is_crtc_enabled = crtc_state->active;
> @@ -12452,7 +12480,7 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
>  	struct drm_framebuffer *fb = plane_state->fb;
>  	int ret;
>  
> -	if (INTEL_GEN(dev_priv) >= 9 && plane->type != DRM_PLANE_TYPE_CURSOR) {
> +	if (INTEL_GEN(dev_priv) >= 9 && plane->id != PLANE_CURSOR) {
>  		ret = skl_update_scaler_plane(
>  			to_intel_crtc_state(crtc_state),
>  			to_intel_plane_state(plane_state));
> @@ -12476,8 +12504,10 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
>  	 * per-plane wm computation to the .check_plane() hook, and
>  	 * only combine the results from all planes in the current place?
>  	 */
> -	if (!is_crtc_enabled)
> +	if (!is_crtc_enabled) {
>  		to_intel_plane_state(plane_state)->base.visible = visible = false;
> +		to_intel_crtc_state(crtc_state)->active_planes &= ~BIT(plane->id);
> +	}
>  
>  	if (!was_visible && !visible)
>  		return 0;
> @@ -12489,13 +12519,12 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
>  	turn_on = visible && (!was_visible || mode_changed);
>  
>  	DRM_DEBUG_ATOMIC("[CRTC:%d:%s] has [PLANE:%d:%s] with fb %i\n",
> -			 intel_crtc->base.base.id,
> -			 intel_crtc->base.name,
> -			 plane->base.id, plane->name,
> +			 intel_crtc->base.base.id, intel_crtc->base.name,
> +			 plane->base.base.id, plane->base.name,
>  			 fb ? fb->base.id : -1);
>  
>  	DRM_DEBUG_ATOMIC("[PLANE:%d:%s] visible %i -> %i, off %i, on %i, ms %i\n",
> -			 plane->base.id, plane->name,
> +			 plane->base.base.id, plane->base.name,
>  			 was_visible, visible,
>  			 turn_off, turn_on, mode_changed);
>  
> @@ -12503,15 +12532,15 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
>  		pipe_config->update_wm_pre = true;
>  
>  		/* must disable cxsr around plane enable/disable */
> -		if (plane->type != DRM_PLANE_TYPE_CURSOR)
> +		if (plane->id != PLANE_CURSOR)
>  			pipe_config->disable_cxsr = true;
>  	} else if (turn_off) {
>  		pipe_config->update_wm_post = true;
>  
>  		/* must disable cxsr around plane enable/disable */
> -		if (plane->type != DRM_PLANE_TYPE_CURSOR)
> +		if (plane->id != PLANE_CURSOR)
>  			pipe_config->disable_cxsr = true;
> -	} else if (intel_wm_need_update(plane, plane_state)) {
> +	} else if (intel_wm_need_update(&plane->base, plane_state)) {
>  		/* FIXME bollocks */
>  		pipe_config->update_wm_pre = true;
>  		pipe_config->update_wm_post = true;
> @@ -12523,7 +12552,7 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
>  		to_intel_crtc_state(crtc_state)->wm.need_postvbl_update = true;
>  
>  	if (visible || was_visible)
> -		pipe_config->fb_bits |= to_intel_plane(plane)->frontbuffer_bit;
> +		pipe_config->fb_bits |= plane->frontbuffer_bit;
>  
>  	/*
>  	 * WaCxSRDisabledForSpriteScaling:ivb
> @@ -12531,7 +12560,7 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
>  	 * cstate->update_wm was already set above, so this flag will
>  	 * take effect when we commit and program watermarks.
>  	 */
> -	if (plane->type == DRM_PLANE_TYPE_OVERLAY && IS_IVYBRIDGE(dev_priv) &&
> +	if (plane->id == PLANE_SPRITE0 && IS_IVYBRIDGE(dev_priv) &&
>  	    needs_scaling(to_intel_plane_state(plane_state)) &&
>  	    !needs_scaling(old_plane_state))
>  		pipe_config->disable_lp_wm = true;
> @@ -16876,15 +16905,14 @@ static bool primary_get_hw_state(struct intel_plane *plane)
>  /* FIXME read out full plane state for all planes */
>  static void readout_plane_state(struct intel_crtc *crtc)
>  {
> -	struct drm_plane *primary = crtc->base.primary;
> -	struct intel_plane_state *plane_state =
> -		to_intel_plane_state(primary->state);
> +	struct intel_plane *primary = to_intel_plane(crtc->base.primary);
> +	bool visible;
>  
> -	plane_state->base.visible = crtc->active &&
> -		primary_get_hw_state(to_intel_plane(primary));
> +	visible = crtc->active && primary_get_hw_state(primary);
>  
> -	if (plane_state->base.visible)
> -		crtc->base.state->plane_mask |= 1 << drm_plane_index(primary);
> +	intel_set_plane_visible(to_intel_crtc_state(crtc->base.state),
> +				to_intel_plane_state(primary->base.state),
> +				visible);
>  }
>  
>  static void intel_modeset_readout_hw_state(struct drm_device *dev)
> @@ -17171,7 +17199,10 @@ void intel_modeset_gem_init(struct drm_device *dev)
>  			c->primary->fb = NULL;
>  			c->primary->crtc = c->primary->state->crtc = NULL;
>  			update_state_fb(c->primary);
> -			c->state->plane_mask &= ~(1 << drm_plane_index(c->primary));
> +
> +			intel_set_plane_visible(to_intel_crtc_state(c->state),
> +						to_intel_plane_state(c->primary->state),
> +						false);
>  		}
>  	}
>  }
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 8f4ddca0f521..20ba8f48bc3b 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -669,6 +669,9 @@ struct intel_crtc_state {
>  
>  	/* Gamma mode programmed on the pipe */
>  	uint32_t gamma_mode;
> +
> +	/* bitmask of visible planes (enum plane_id) */
> +	u8 active_planes;
Can we change this to uint32_t 1<<drm_plane_index like the other masks?

plane_mask, connector_mask and encoder_mask use 1 << index.
Perhaps name it active_planes_mask too.

~Maarten
Ville Syrjälä Dec. 15, 2016, 3:02 p.m. UTC | #2
On Thu, Dec 15, 2016 at 03:56:03PM +0100, Maarten Lankhorst wrote:
> Op 12-12-16 om 21:35 schreef ville.syrjala@linux.intel.com:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > In a lot of place we wish to know which planes on the crtc are actually
> > visible, or how many of them there are. Let's start tracking that in a
> > bitmask in the crtc state.
> >
> > We already track enabled planes (ie. ones with an fb and crtc specified by
> > the user) but that's not quite the same thing as enabled planes may
> > still end up being invisible due to clipping and whatnot.
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_atomic_plane.c |  6 +++
> >  drivers/gpu/drm/i915/intel_display.c      | 79 +++++++++++++++++++++----------
> >  drivers/gpu/drm/i915/intel_drv.h          |  3 ++
> >  3 files changed, 64 insertions(+), 24 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c b/drivers/gpu/drm/i915/intel_atomic_plane.c
> > index dbe9fb41ae53..7ec12edda4d4 100644
> > --- a/drivers/gpu/drm/i915/intel_atomic_plane.c
> > +++ b/drivers/gpu/drm/i915/intel_atomic_plane.c
> > @@ -181,6 +181,12 @@ static int intel_plane_atomic_check(struct drm_plane *plane,
> >  	if (ret)
> >  		return ret;
> >  
> > +	/* FIXME pre-g4x don't work like this */
> > +	if (intel_state->base.visible)
> > +		crtc_state->active_planes |= BIT(intel_plane->id);
> > +	else
> > +		crtc_state->active_planes &= ~BIT(intel_plane->id);
> > +
> >  	return intel_plane_atomic_calc_changes(&crtc_state->base, state);
> >  }
> >  
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index bc1af87789bc..3f027341b0f3 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -2741,6 +2741,29 @@ update_state_fb(struct drm_plane *plane)
> >  }
> >  
> >  static void
> > +intel_set_plane_visible(struct intel_crtc_state *crtc_state,
> > +			struct intel_plane_state *plane_state,
> > +			bool visible)
> > +{
> > +	struct intel_plane *plane = to_intel_plane(plane_state->base.plane);
> > +
> > +	plane_state->base.visible = visible;
> > +
> > +	/* FIXME pre-g4x don't work like this */
> > +	if (visible) {
> > +		crtc_state->base.plane_mask |= BIT(drm_plane_index(&plane->base));
> > +		crtc_state->active_planes |= BIT(plane->id);
> > +	} else {
> > +		crtc_state->base.plane_mask &= ~BIT(drm_plane_index(&plane->base));
> > +		crtc_state->active_planes &= ~BIT(plane->id);
> > +	}
> > +
> > +	DRM_DEBUG_KMS("%s active planes 0x%x\n",
> > +		      crtc_state->base.crtc->name,
> > +		      crtc_state->active_planes);
> > +}
> > +
> > +static void
> >  intel_find_initial_plane_obj(struct intel_crtc *intel_crtc,
> >  			     struct intel_initial_plane_config *plane_config)
> >  {
> > @@ -2798,8 +2821,9 @@ intel_find_initial_plane_obj(struct intel_crtc *intel_crtc,
> >  	 * simplest solution is to just disable the primary plane now and
> >  	 * pretend the BIOS never had it enabled.
> >  	 */
> > -	to_intel_plane_state(plane_state)->base.visible = false;
> > -	crtc_state->plane_mask &= ~(1 << drm_plane_index(primary));
> > +	intel_set_plane_visible(to_intel_crtc_state(crtc_state),
> > +				to_intel_plane_state(plane_state),
> > +				false);
> >  	intel_pre_disable_primary_noatomic(&intel_crtc->base);
> >  	intel_plane->disable_plane(primary, &intel_crtc->base);
> >  
> > @@ -2826,7 +2850,11 @@ intel_find_initial_plane_obj(struct intel_crtc *intel_crtc,
> >  	drm_framebuffer_reference(fb);
> >  	primary->fb = primary->state->fb = fb;
> >  	primary->crtc = primary->state->crtc = &intel_crtc->base;
> > -	intel_crtc->base.state->plane_mask |= (1 << drm_plane_index(primary));
> > +
> > +	intel_set_plane_visible(to_intel_crtc_state(intel_crtc->base.state),
> > +				to_intel_plane_state(primary->state),
> > +				true);
> > +
> >  	atomic_or(to_intel_plane(primary)->frontbuffer_bit,
> >  		  &obj->frontbuffer_bits);
> >  }
> > @@ -12440,11 +12468,11 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
> >  	struct intel_crtc_state *pipe_config = to_intel_crtc_state(crtc_state);
> >  	struct drm_crtc *crtc = crtc_state->crtc;
> >  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> > -	struct drm_plane *plane = plane_state->plane;
> > +	struct intel_plane *plane = to_intel_plane(plane_state->plane);
> >  	struct drm_device *dev = crtc->dev;
> >  	struct drm_i915_private *dev_priv = to_i915(dev);
> >  	struct intel_plane_state *old_plane_state =
> > -		to_intel_plane_state(plane->state);
> > +		to_intel_plane_state(plane->base.state);
> >  	bool mode_changed = needs_modeset(crtc_state);
> >  	bool was_crtc_enabled = crtc->state->active;
> >  	bool is_crtc_enabled = crtc_state->active;
> > @@ -12452,7 +12480,7 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
> >  	struct drm_framebuffer *fb = plane_state->fb;
> >  	int ret;
> >  
> > -	if (INTEL_GEN(dev_priv) >= 9 && plane->type != DRM_PLANE_TYPE_CURSOR) {
> > +	if (INTEL_GEN(dev_priv) >= 9 && plane->id != PLANE_CURSOR) {
> >  		ret = skl_update_scaler_plane(
> >  			to_intel_crtc_state(crtc_state),
> >  			to_intel_plane_state(plane_state));
> > @@ -12476,8 +12504,10 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
> >  	 * per-plane wm computation to the .check_plane() hook, and
> >  	 * only combine the results from all planes in the current place?
> >  	 */
> > -	if (!is_crtc_enabled)
> > +	if (!is_crtc_enabled) {
> >  		to_intel_plane_state(plane_state)->base.visible = visible = false;
> > +		to_intel_crtc_state(crtc_state)->active_planes &= ~BIT(plane->id);
> > +	}
> >  
> >  	if (!was_visible && !visible)
> >  		return 0;
> > @@ -12489,13 +12519,12 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
> >  	turn_on = visible && (!was_visible || mode_changed);
> >  
> >  	DRM_DEBUG_ATOMIC("[CRTC:%d:%s] has [PLANE:%d:%s] with fb %i\n",
> > -			 intel_crtc->base.base.id,
> > -			 intel_crtc->base.name,
> > -			 plane->base.id, plane->name,
> > +			 intel_crtc->base.base.id, intel_crtc->base.name,
> > +			 plane->base.base.id, plane->base.name,
> >  			 fb ? fb->base.id : -1);
> >  
> >  	DRM_DEBUG_ATOMIC("[PLANE:%d:%s] visible %i -> %i, off %i, on %i, ms %i\n",
> > -			 plane->base.id, plane->name,
> > +			 plane->base.base.id, plane->base.name,
> >  			 was_visible, visible,
> >  			 turn_off, turn_on, mode_changed);
> >  
> > @@ -12503,15 +12532,15 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
> >  		pipe_config->update_wm_pre = true;
> >  
> >  		/* must disable cxsr around plane enable/disable */
> > -		if (plane->type != DRM_PLANE_TYPE_CURSOR)
> > +		if (plane->id != PLANE_CURSOR)
> >  			pipe_config->disable_cxsr = true;
> >  	} else if (turn_off) {
> >  		pipe_config->update_wm_post = true;
> >  
> >  		/* must disable cxsr around plane enable/disable */
> > -		if (plane->type != DRM_PLANE_TYPE_CURSOR)
> > +		if (plane->id != PLANE_CURSOR)
> >  			pipe_config->disable_cxsr = true;
> > -	} else if (intel_wm_need_update(plane, plane_state)) {
> > +	} else if (intel_wm_need_update(&plane->base, plane_state)) {
> >  		/* FIXME bollocks */
> >  		pipe_config->update_wm_pre = true;
> >  		pipe_config->update_wm_post = true;
> > @@ -12523,7 +12552,7 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
> >  		to_intel_crtc_state(crtc_state)->wm.need_postvbl_update = true;
> >  
> >  	if (visible || was_visible)
> > -		pipe_config->fb_bits |= to_intel_plane(plane)->frontbuffer_bit;
> > +		pipe_config->fb_bits |= plane->frontbuffer_bit;
> >  
> >  	/*
> >  	 * WaCxSRDisabledForSpriteScaling:ivb
> > @@ -12531,7 +12560,7 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
> >  	 * cstate->update_wm was already set above, so this flag will
> >  	 * take effect when we commit and program watermarks.
> >  	 */
> > -	if (plane->type == DRM_PLANE_TYPE_OVERLAY && IS_IVYBRIDGE(dev_priv) &&
> > +	if (plane->id == PLANE_SPRITE0 && IS_IVYBRIDGE(dev_priv) &&
> >  	    needs_scaling(to_intel_plane_state(plane_state)) &&
> >  	    !needs_scaling(old_plane_state))
> >  		pipe_config->disable_lp_wm = true;
> > @@ -16876,15 +16905,14 @@ static bool primary_get_hw_state(struct intel_plane *plane)
> >  /* FIXME read out full plane state for all planes */
> >  static void readout_plane_state(struct intel_crtc *crtc)
> >  {
> > -	struct drm_plane *primary = crtc->base.primary;
> > -	struct intel_plane_state *plane_state =
> > -		to_intel_plane_state(primary->state);
> > +	struct intel_plane *primary = to_intel_plane(crtc->base.primary);
> > +	bool visible;
> >  
> > -	plane_state->base.visible = crtc->active &&
> > -		primary_get_hw_state(to_intel_plane(primary));
> > +	visible = crtc->active && primary_get_hw_state(primary);
> >  
> > -	if (plane_state->base.visible)
> > -		crtc->base.state->plane_mask |= 1 << drm_plane_index(primary);
> > +	intel_set_plane_visible(to_intel_crtc_state(crtc->base.state),
> > +				to_intel_plane_state(primary->base.state),
> > +				visible);
> >  }
> >  
> >  static void intel_modeset_readout_hw_state(struct drm_device *dev)
> > @@ -17171,7 +17199,10 @@ void intel_modeset_gem_init(struct drm_device *dev)
> >  			c->primary->fb = NULL;
> >  			c->primary->crtc = c->primary->state->crtc = NULL;
> >  			update_state_fb(c->primary);
> > -			c->state->plane_mask &= ~(1 << drm_plane_index(c->primary));
> > +
> > +			intel_set_plane_visible(to_intel_crtc_state(c->state),
> > +						to_intel_plane_state(c->primary->state),
> > +						false);
> >  		}
> >  	}
> >  }
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > index 8f4ddca0f521..20ba8f48bc3b 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -669,6 +669,9 @@ struct intel_crtc_state {
> >  
> >  	/* Gamma mode programmed on the pipe */
> >  	uint32_t gamma_mode;
> > +
> > +	/* bitmask of visible planes (enum plane_id) */
> > +	u8 active_planes;
> Can we change this to uint32_t 1<<drm_plane_index like the other masks?
> 
> plane_mask, connector_mask and encoder_mask use 1 << index.
> Perhaps name it active_planes_mask too.

The way I use it wouldn't lend itself well to that. I don't have any
plane structs around in a bunch of the places where this gets used.
Maarten Lankhorst Dec. 15, 2016, 5:11 p.m. UTC | #3
Op 15-12-16 om 16:02 schreef Ville Syrjälä:
> On Thu, Dec 15, 2016 at 03:56:03PM +0100, Maarten Lankhorst wrote:
>> Op 12-12-16 om 21:35 schreef ville.syrjala@linux.intel.com:
>>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>>
>>> In a lot of place we wish to know which planes on the crtc are actually
>>> visible, or how many of them there are. Let's start tracking that in a
>>> bitmask in the crtc state.
>>>
>>> We already track enabled planes (ie. ones with an fb and crtc specified by
>>> the user) but that's not quite the same thing as enabled planes may
>>> still end up being invisible due to clipping and whatnot.
>>>
>>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>> ---
>>>  drivers/gpu/drm/i915/intel_atomic_plane.c |  6 +++
>>>  drivers/gpu/drm/i915/intel_display.c      | 79 +++++++++++++++++++++----------
>>>  drivers/gpu/drm/i915/intel_drv.h          |  3 ++
>>>  3 files changed, 64 insertions(+), 24 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c b/drivers/gpu/drm/i915/intel_atomic_plane.c
>>> index dbe9fb41ae53..7ec12edda4d4 100644
>>> --- a/drivers/gpu/drm/i915/intel_atomic_plane.c
>>> +++ b/drivers/gpu/drm/i915/intel_atomic_plane.c
>>> @@ -181,6 +181,12 @@ static int intel_plane_atomic_check(struct drm_plane *plane,
>>>  	if (ret)
>>>  		return ret;
>>>  
>>> +	/* FIXME pre-g4x don't work like this */
>>> +	if (intel_state->base.visible)
>>> +		crtc_state->active_planes |= BIT(intel_plane->id);
>>> +	else
>>> +		crtc_state->active_planes &= ~BIT(intel_plane->id);
>>> +
>>>  	return intel_plane_atomic_calc_changes(&crtc_state->base, state);
>>>  }
>>>  
>>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>>> index bc1af87789bc..3f027341b0f3 100644
>>> --- a/drivers/gpu/drm/i915/intel_display.c
>>> +++ b/drivers/gpu/drm/i915/intel_display.c
>>> @@ -2741,6 +2741,29 @@ update_state_fb(struct drm_plane *plane)
>>>  }
>>>  
>>>  static void
>>> +intel_set_plane_visible(struct intel_crtc_state *crtc_state,
>>> +			struct intel_plane_state *plane_state,
>>> +			bool visible)
>>> +{
>>> +	struct intel_plane *plane = to_intel_plane(plane_state->base.plane);
>>> +
>>> +	plane_state->base.visible = visible;
>>> +
>>> +	/* FIXME pre-g4x don't work like this */
>>> +	if (visible) {
>>> +		crtc_state->base.plane_mask |= BIT(drm_plane_index(&plane->base));
>>> +		crtc_state->active_planes |= BIT(plane->id);
>>> +	} else {
>>> +		crtc_state->base.plane_mask &= ~BIT(drm_plane_index(&plane->base));
>>> +		crtc_state->active_planes &= ~BIT(plane->id);
>>> +	}
>>> +
>>> +	DRM_DEBUG_KMS("%s active planes 0x%x\n",
>>> +		      crtc_state->base.crtc->name,
>>> +		      crtc_state->active_planes);
>>> +}
>>> +
>>> +static void
>>>  intel_find_initial_plane_obj(struct intel_crtc *intel_crtc,
>>>  			     struct intel_initial_plane_config *plane_config)
>>>  {
>>> @@ -2798,8 +2821,9 @@ intel_find_initial_plane_obj(struct intel_crtc *intel_crtc,
>>>  	 * simplest solution is to just disable the primary plane now and
>>>  	 * pretend the BIOS never had it enabled.
>>>  	 */
>>> -	to_intel_plane_state(plane_state)->base.visible = false;
>>> -	crtc_state->plane_mask &= ~(1 << drm_plane_index(primary));
>>> +	intel_set_plane_visible(to_intel_crtc_state(crtc_state),
>>> +				to_intel_plane_state(plane_state),
>>> +				false);
>>>  	intel_pre_disable_primary_noatomic(&intel_crtc->base);
>>>  	intel_plane->disable_plane(primary, &intel_crtc->base);
>>>  
>>> @@ -2826,7 +2850,11 @@ intel_find_initial_plane_obj(struct intel_crtc *intel_crtc,
>>>  	drm_framebuffer_reference(fb);
>>>  	primary->fb = primary->state->fb = fb;
>>>  	primary->crtc = primary->state->crtc = &intel_crtc->base;
>>> -	intel_crtc->base.state->plane_mask |= (1 << drm_plane_index(primary));
>>> +
>>> +	intel_set_plane_visible(to_intel_crtc_state(intel_crtc->base.state),
>>> +				to_intel_plane_state(primary->state),
>>> +				true);
>>> +
>>>  	atomic_or(to_intel_plane(primary)->frontbuffer_bit,
>>>  		  &obj->frontbuffer_bits);
>>>  }
>>> @@ -12440,11 +12468,11 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
>>>  	struct intel_crtc_state *pipe_config = to_intel_crtc_state(crtc_state);
>>>  	struct drm_crtc *crtc = crtc_state->crtc;
>>>  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>>> -	struct drm_plane *plane = plane_state->plane;
>>> +	struct intel_plane *plane = to_intel_plane(plane_state->plane);
>>>  	struct drm_device *dev = crtc->dev;
>>>  	struct drm_i915_private *dev_priv = to_i915(dev);
>>>  	struct intel_plane_state *old_plane_state =
>>> -		to_intel_plane_state(plane->state);
>>> +		to_intel_plane_state(plane->base.state);
>>>  	bool mode_changed = needs_modeset(crtc_state);
>>>  	bool was_crtc_enabled = crtc->state->active;
>>>  	bool is_crtc_enabled = crtc_state->active;
>>> @@ -12452,7 +12480,7 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
>>>  	struct drm_framebuffer *fb = plane_state->fb;
>>>  	int ret;
>>>  
>>> -	if (INTEL_GEN(dev_priv) >= 9 && plane->type != DRM_PLANE_TYPE_CURSOR) {
>>> +	if (INTEL_GEN(dev_priv) >= 9 && plane->id != PLANE_CURSOR) {
>>>  		ret = skl_update_scaler_plane(
>>>  			to_intel_crtc_state(crtc_state),
>>>  			to_intel_plane_state(plane_state));
>>> @@ -12476,8 +12504,10 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
>>>  	 * per-plane wm computation to the .check_plane() hook, and
>>>  	 * only combine the results from all planes in the current place?
>>>  	 */
>>> -	if (!is_crtc_enabled)
>>> +	if (!is_crtc_enabled) {
>>>  		to_intel_plane_state(plane_state)->base.visible = visible = false;
>>> +		to_intel_crtc_state(crtc_state)->active_planes &= ~BIT(plane->id);
>>> +	}
>>>  
>>>  	if (!was_visible && !visible)
>>>  		return 0;
>>> @@ -12489,13 +12519,12 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
>>>  	turn_on = visible && (!was_visible || mode_changed);
>>>  
>>>  	DRM_DEBUG_ATOMIC("[CRTC:%d:%s] has [PLANE:%d:%s] with fb %i\n",
>>> -			 intel_crtc->base.base.id,
>>> -			 intel_crtc->base.name,
>>> -			 plane->base.id, plane->name,
>>> +			 intel_crtc->base.base.id, intel_crtc->base.name,
>>> +			 plane->base.base.id, plane->base.name,
>>>  			 fb ? fb->base.id : -1);
>>>  
>>>  	DRM_DEBUG_ATOMIC("[PLANE:%d:%s] visible %i -> %i, off %i, on %i, ms %i\n",
>>> -			 plane->base.id, plane->name,
>>> +			 plane->base.base.id, plane->base.name,
>>>  			 was_visible, visible,
>>>  			 turn_off, turn_on, mode_changed);
>>>  
>>> @@ -12503,15 +12532,15 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
>>>  		pipe_config->update_wm_pre = true;
>>>  
>>>  		/* must disable cxsr around plane enable/disable */
>>> -		if (plane->type != DRM_PLANE_TYPE_CURSOR)
>>> +		if (plane->id != PLANE_CURSOR)
>>>  			pipe_config->disable_cxsr = true;
>>>  	} else if (turn_off) {
>>>  		pipe_config->update_wm_post = true;
>>>  
>>>  		/* must disable cxsr around plane enable/disable */
>>> -		if (plane->type != DRM_PLANE_TYPE_CURSOR)
>>> +		if (plane->id != PLANE_CURSOR)
>>>  			pipe_config->disable_cxsr = true;
>>> -	} else if (intel_wm_need_update(plane, plane_state)) {
>>> +	} else if (intel_wm_need_update(&plane->base, plane_state)) {
>>>  		/* FIXME bollocks */
>>>  		pipe_config->update_wm_pre = true;
>>>  		pipe_config->update_wm_post = true;
>>> @@ -12523,7 +12552,7 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
>>>  		to_intel_crtc_state(crtc_state)->wm.need_postvbl_update = true;
>>>  
>>>  	if (visible || was_visible)
>>> -		pipe_config->fb_bits |= to_intel_plane(plane)->frontbuffer_bit;
>>> +		pipe_config->fb_bits |= plane->frontbuffer_bit;
>>>  
>>>  	/*
>>>  	 * WaCxSRDisabledForSpriteScaling:ivb
>>> @@ -12531,7 +12560,7 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
>>>  	 * cstate->update_wm was already set above, so this flag will
>>>  	 * take effect when we commit and program watermarks.
>>>  	 */
>>> -	if (plane->type == DRM_PLANE_TYPE_OVERLAY && IS_IVYBRIDGE(dev_priv) &&
>>> +	if (plane->id == PLANE_SPRITE0 && IS_IVYBRIDGE(dev_priv) &&
>>>  	    needs_scaling(to_intel_plane_state(plane_state)) &&
>>>  	    !needs_scaling(old_plane_state))
>>>  		pipe_config->disable_lp_wm = true;
>>> @@ -16876,15 +16905,14 @@ static bool primary_get_hw_state(struct intel_plane *plane)
>>>  /* FIXME read out full plane state for all planes */
>>>  static void readout_plane_state(struct intel_crtc *crtc)
>>>  {
>>> -	struct drm_plane *primary = crtc->base.primary;
>>> -	struct intel_plane_state *plane_state =
>>> -		to_intel_plane_state(primary->state);
>>> +	struct intel_plane *primary = to_intel_plane(crtc->base.primary);
>>> +	bool visible;
>>>  
>>> -	plane_state->base.visible = crtc->active &&
>>> -		primary_get_hw_state(to_intel_plane(primary));
>>> +	visible = crtc->active && primary_get_hw_state(primary);
>>>  
>>> -	if (plane_state->base.visible)
>>> -		crtc->base.state->plane_mask |= 1 << drm_plane_index(primary);
>>> +	intel_set_plane_visible(to_intel_crtc_state(crtc->base.state),
>>> +				to_intel_plane_state(primary->base.state),
>>> +				visible);
>>>  }
>>>  
>>>  static void intel_modeset_readout_hw_state(struct drm_device *dev)
>>> @@ -17171,7 +17199,10 @@ void intel_modeset_gem_init(struct drm_device *dev)
>>>  			c->primary->fb = NULL;
>>>  			c->primary->crtc = c->primary->state->crtc = NULL;
>>>  			update_state_fb(c->primary);
>>> -			c->state->plane_mask &= ~(1 << drm_plane_index(c->primary));
>>> +
>>> +			intel_set_plane_visible(to_intel_crtc_state(c->state),
>>> +						to_intel_plane_state(c->primary->state),
>>> +						false);
>>>  		}
>>>  	}
>>>  }
>>> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
>>> index 8f4ddca0f521..20ba8f48bc3b 100644
>>> --- a/drivers/gpu/drm/i915/intel_drv.h
>>> +++ b/drivers/gpu/drm/i915/intel_drv.h
>>> @@ -669,6 +669,9 @@ struct intel_crtc_state {
>>>  
>>>  	/* Gamma mode programmed on the pipe */
>>>  	uint32_t gamma_mode;
>>> +
>>> +	/* bitmask of visible planes (enum plane_id) */
>>> +	u8 active_planes;
>> Can we change this to uint32_t 1<<drm_plane_index like the other masks?
>>
>> plane_mask, connector_mask and encoder_mask use 1 << index.
>> Perhaps name it active_planes_mask too.
> The way I use it wouldn't lend itself well to that. I don't have any
> plane structs around in a bunch of the places where this gets used.
>
Do you use this outside of 13/14? If not please drop the patch from this series. :)
Ville Syrjälä Dec. 15, 2016, 5:20 p.m. UTC | #4
On Thu, Dec 15, 2016 at 06:11:24PM +0100, Maarten Lankhorst wrote:
> Op 15-12-16 om 16:02 schreef Ville Syrjälä:
> > On Thu, Dec 15, 2016 at 03:56:03PM +0100, Maarten Lankhorst wrote:
> >> Op 12-12-16 om 21:35 schreef ville.syrjala@linux.intel.com:
> >>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >>>
> >>> In a lot of place we wish to know which planes on the crtc are actually
> >>> visible, or how many of them there are. Let's start tracking that in a
> >>> bitmask in the crtc state.
> >>>
> >>> We already track enabled planes (ie. ones with an fb and crtc specified by
> >>> the user) but that's not quite the same thing as enabled planes may
> >>> still end up being invisible due to clipping and whatnot.
> >>>
> >>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >>> ---
> >>>  drivers/gpu/drm/i915/intel_atomic_plane.c |  6 +++
> >>>  drivers/gpu/drm/i915/intel_display.c      | 79 +++++++++++++++++++++----------
> >>>  drivers/gpu/drm/i915/intel_drv.h          |  3 ++
> >>>  3 files changed, 64 insertions(+), 24 deletions(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c b/drivers/gpu/drm/i915/intel_atomic_plane.c
> >>> index dbe9fb41ae53..7ec12edda4d4 100644
> >>> --- a/drivers/gpu/drm/i915/intel_atomic_plane.c
> >>> +++ b/drivers/gpu/drm/i915/intel_atomic_plane.c
> >>> @@ -181,6 +181,12 @@ static int intel_plane_atomic_check(struct drm_plane *plane,
> >>>  	if (ret)
> >>>  		return ret;
> >>>  
> >>> +	/* FIXME pre-g4x don't work like this */
> >>> +	if (intel_state->base.visible)
> >>> +		crtc_state->active_planes |= BIT(intel_plane->id);
> >>> +	else
> >>> +		crtc_state->active_planes &= ~BIT(intel_plane->id);
> >>> +
> >>>  	return intel_plane_atomic_calc_changes(&crtc_state->base, state);
> >>>  }
> >>>  
> >>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> >>> index bc1af87789bc..3f027341b0f3 100644
> >>> --- a/drivers/gpu/drm/i915/intel_display.c
> >>> +++ b/drivers/gpu/drm/i915/intel_display.c
> >>> @@ -2741,6 +2741,29 @@ update_state_fb(struct drm_plane *plane)
> >>>  }
> >>>  
> >>>  static void
> >>> +intel_set_plane_visible(struct intel_crtc_state *crtc_state,
> >>> +			struct intel_plane_state *plane_state,
> >>> +			bool visible)
> >>> +{
> >>> +	struct intel_plane *plane = to_intel_plane(plane_state->base.plane);
> >>> +
> >>> +	plane_state->base.visible = visible;
> >>> +
> >>> +	/* FIXME pre-g4x don't work like this */
> >>> +	if (visible) {
> >>> +		crtc_state->base.plane_mask |= BIT(drm_plane_index(&plane->base));
> >>> +		crtc_state->active_planes |= BIT(plane->id);
> >>> +	} else {
> >>> +		crtc_state->base.plane_mask &= ~BIT(drm_plane_index(&plane->base));
> >>> +		crtc_state->active_planes &= ~BIT(plane->id);
> >>> +	}
> >>> +
> >>> +	DRM_DEBUG_KMS("%s active planes 0x%x\n",
> >>> +		      crtc_state->base.crtc->name,
> >>> +		      crtc_state->active_planes);
> >>> +}
> >>> +
> >>> +static void
> >>>  intel_find_initial_plane_obj(struct intel_crtc *intel_crtc,
> >>>  			     struct intel_initial_plane_config *plane_config)
> >>>  {
> >>> @@ -2798,8 +2821,9 @@ intel_find_initial_plane_obj(struct intel_crtc *intel_crtc,
> >>>  	 * simplest solution is to just disable the primary plane now and
> >>>  	 * pretend the BIOS never had it enabled.
> >>>  	 */
> >>> -	to_intel_plane_state(plane_state)->base.visible = false;
> >>> -	crtc_state->plane_mask &= ~(1 << drm_plane_index(primary));
> >>> +	intel_set_plane_visible(to_intel_crtc_state(crtc_state),
> >>> +				to_intel_plane_state(plane_state),
> >>> +				false);
> >>>  	intel_pre_disable_primary_noatomic(&intel_crtc->base);
> >>>  	intel_plane->disable_plane(primary, &intel_crtc->base);
> >>>  
> >>> @@ -2826,7 +2850,11 @@ intel_find_initial_plane_obj(struct intel_crtc *intel_crtc,
> >>>  	drm_framebuffer_reference(fb);
> >>>  	primary->fb = primary->state->fb = fb;
> >>>  	primary->crtc = primary->state->crtc = &intel_crtc->base;
> >>> -	intel_crtc->base.state->plane_mask |= (1 << drm_plane_index(primary));
> >>> +
> >>> +	intel_set_plane_visible(to_intel_crtc_state(intel_crtc->base.state),
> >>> +				to_intel_plane_state(primary->state),
> >>> +				true);
> >>> +
> >>>  	atomic_or(to_intel_plane(primary)->frontbuffer_bit,
> >>>  		  &obj->frontbuffer_bits);
> >>>  }
> >>> @@ -12440,11 +12468,11 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
> >>>  	struct intel_crtc_state *pipe_config = to_intel_crtc_state(crtc_state);
> >>>  	struct drm_crtc *crtc = crtc_state->crtc;
> >>>  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> >>> -	struct drm_plane *plane = plane_state->plane;
> >>> +	struct intel_plane *plane = to_intel_plane(plane_state->plane);
> >>>  	struct drm_device *dev = crtc->dev;
> >>>  	struct drm_i915_private *dev_priv = to_i915(dev);
> >>>  	struct intel_plane_state *old_plane_state =
> >>> -		to_intel_plane_state(plane->state);
> >>> +		to_intel_plane_state(plane->base.state);
> >>>  	bool mode_changed = needs_modeset(crtc_state);
> >>>  	bool was_crtc_enabled = crtc->state->active;
> >>>  	bool is_crtc_enabled = crtc_state->active;
> >>> @@ -12452,7 +12480,7 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
> >>>  	struct drm_framebuffer *fb = plane_state->fb;
> >>>  	int ret;
> >>>  
> >>> -	if (INTEL_GEN(dev_priv) >= 9 && plane->type != DRM_PLANE_TYPE_CURSOR) {
> >>> +	if (INTEL_GEN(dev_priv) >= 9 && plane->id != PLANE_CURSOR) {
> >>>  		ret = skl_update_scaler_plane(
> >>>  			to_intel_crtc_state(crtc_state),
> >>>  			to_intel_plane_state(plane_state));
> >>> @@ -12476,8 +12504,10 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
> >>>  	 * per-plane wm computation to the .check_plane() hook, and
> >>>  	 * only combine the results from all planes in the current place?
> >>>  	 */
> >>> -	if (!is_crtc_enabled)
> >>> +	if (!is_crtc_enabled) {
> >>>  		to_intel_plane_state(plane_state)->base.visible = visible = false;
> >>> +		to_intel_crtc_state(crtc_state)->active_planes &= ~BIT(plane->id);
> >>> +	}
> >>>  
> >>>  	if (!was_visible && !visible)
> >>>  		return 0;
> >>> @@ -12489,13 +12519,12 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
> >>>  	turn_on = visible && (!was_visible || mode_changed);
> >>>  
> >>>  	DRM_DEBUG_ATOMIC("[CRTC:%d:%s] has [PLANE:%d:%s] with fb %i\n",
> >>> -			 intel_crtc->base.base.id,
> >>> -			 intel_crtc->base.name,
> >>> -			 plane->base.id, plane->name,
> >>> +			 intel_crtc->base.base.id, intel_crtc->base.name,
> >>> +			 plane->base.base.id, plane->base.name,
> >>>  			 fb ? fb->base.id : -1);
> >>>  
> >>>  	DRM_DEBUG_ATOMIC("[PLANE:%d:%s] visible %i -> %i, off %i, on %i, ms %i\n",
> >>> -			 plane->base.id, plane->name,
> >>> +			 plane->base.base.id, plane->base.name,
> >>>  			 was_visible, visible,
> >>>  			 turn_off, turn_on, mode_changed);
> >>>  
> >>> @@ -12503,15 +12532,15 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
> >>>  		pipe_config->update_wm_pre = true;
> >>>  
> >>>  		/* must disable cxsr around plane enable/disable */
> >>> -		if (plane->type != DRM_PLANE_TYPE_CURSOR)
> >>> +		if (plane->id != PLANE_CURSOR)
> >>>  			pipe_config->disable_cxsr = true;
> >>>  	} else if (turn_off) {
> >>>  		pipe_config->update_wm_post = true;
> >>>  
> >>>  		/* must disable cxsr around plane enable/disable */
> >>> -		if (plane->type != DRM_PLANE_TYPE_CURSOR)
> >>> +		if (plane->id != PLANE_CURSOR)
> >>>  			pipe_config->disable_cxsr = true;
> >>> -	} else if (intel_wm_need_update(plane, plane_state)) {
> >>> +	} else if (intel_wm_need_update(&plane->base, plane_state)) {
> >>>  		/* FIXME bollocks */
> >>>  		pipe_config->update_wm_pre = true;
> >>>  		pipe_config->update_wm_post = true;
> >>> @@ -12523,7 +12552,7 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
> >>>  		to_intel_crtc_state(crtc_state)->wm.need_postvbl_update = true;
> >>>  
> >>>  	if (visible || was_visible)
> >>> -		pipe_config->fb_bits |= to_intel_plane(plane)->frontbuffer_bit;
> >>> +		pipe_config->fb_bits |= plane->frontbuffer_bit;
> >>>  
> >>>  	/*
> >>>  	 * WaCxSRDisabledForSpriteScaling:ivb
> >>> @@ -12531,7 +12560,7 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
> >>>  	 * cstate->update_wm was already set above, so this flag will
> >>>  	 * take effect when we commit and program watermarks.
> >>>  	 */
> >>> -	if (plane->type == DRM_PLANE_TYPE_OVERLAY && IS_IVYBRIDGE(dev_priv) &&
> >>> +	if (plane->id == PLANE_SPRITE0 && IS_IVYBRIDGE(dev_priv) &&
> >>>  	    needs_scaling(to_intel_plane_state(plane_state)) &&
> >>>  	    !needs_scaling(old_plane_state))
> >>>  		pipe_config->disable_lp_wm = true;
> >>> @@ -16876,15 +16905,14 @@ static bool primary_get_hw_state(struct intel_plane *plane)
> >>>  /* FIXME read out full plane state for all planes */
> >>>  static void readout_plane_state(struct intel_crtc *crtc)
> >>>  {
> >>> -	struct drm_plane *primary = crtc->base.primary;
> >>> -	struct intel_plane_state *plane_state =
> >>> -		to_intel_plane_state(primary->state);
> >>> +	struct intel_plane *primary = to_intel_plane(crtc->base.primary);
> >>> +	bool visible;
> >>>  
> >>> -	plane_state->base.visible = crtc->active &&
> >>> -		primary_get_hw_state(to_intel_plane(primary));
> >>> +	visible = crtc->active && primary_get_hw_state(primary);
> >>>  
> >>> -	if (plane_state->base.visible)
> >>> -		crtc->base.state->plane_mask |= 1 << drm_plane_index(primary);
> >>> +	intel_set_plane_visible(to_intel_crtc_state(crtc->base.state),
> >>> +				to_intel_plane_state(primary->base.state),
> >>> +				visible);
> >>>  }
> >>>  
> >>>  static void intel_modeset_readout_hw_state(struct drm_device *dev)
> >>> @@ -17171,7 +17199,10 @@ void intel_modeset_gem_init(struct drm_device *dev)
> >>>  			c->primary->fb = NULL;
> >>>  			c->primary->crtc = c->primary->state->crtc = NULL;
> >>>  			update_state_fb(c->primary);
> >>> -			c->state->plane_mask &= ~(1 << drm_plane_index(c->primary));
> >>> +
> >>> +			intel_set_plane_visible(to_intel_crtc_state(c->state),
> >>> +						to_intel_plane_state(c->primary->state),
> >>> +						false);
> >>>  		}
> >>>  	}
> >>>  }
> >>> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> >>> index 8f4ddca0f521..20ba8f48bc3b 100644
> >>> --- a/drivers/gpu/drm/i915/intel_drv.h
> >>> +++ b/drivers/gpu/drm/i915/intel_drv.h
> >>> @@ -669,6 +669,9 @@ struct intel_crtc_state {
> >>>  
> >>>  	/* Gamma mode programmed on the pipe */
> >>>  	uint32_t gamma_mode;
> >>> +
> >>> +	/* bitmask of visible planes (enum plane_id) */
> >>> +	u8 active_planes;
> >> Can we change this to uint32_t 1<<drm_plane_index like the other masks?
> >>
> >> plane_mask, connector_mask and encoder_mask use 1 << index.
> >> Perhaps name it active_planes_mask too.
> > The way I use it wouldn't lend itself well to that. I don't have any
> > plane structs around in a bunch of the places where this gets used.
> >
> Do you use this outside of 13/14? If not please drop the patch from this series. :)

It's used in multiple places.

Patch
diff mbox

diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c b/drivers/gpu/drm/i915/intel_atomic_plane.c
index dbe9fb41ae53..7ec12edda4d4 100644
--- a/drivers/gpu/drm/i915/intel_atomic_plane.c
+++ b/drivers/gpu/drm/i915/intel_atomic_plane.c
@@ -181,6 +181,12 @@  static int intel_plane_atomic_check(struct drm_plane *plane,
 	if (ret)
 		return ret;
 
+	/* FIXME pre-g4x don't work like this */
+	if (intel_state->base.visible)
+		crtc_state->active_planes |= BIT(intel_plane->id);
+	else
+		crtc_state->active_planes &= ~BIT(intel_plane->id);
+
 	return intel_plane_atomic_calc_changes(&crtc_state->base, state);
 }
 
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index bc1af87789bc..3f027341b0f3 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2741,6 +2741,29 @@  update_state_fb(struct drm_plane *plane)
 }
 
 static void
+intel_set_plane_visible(struct intel_crtc_state *crtc_state,
+			struct intel_plane_state *plane_state,
+			bool visible)
+{
+	struct intel_plane *plane = to_intel_plane(plane_state->base.plane);
+
+	plane_state->base.visible = visible;
+
+	/* FIXME pre-g4x don't work like this */
+	if (visible) {
+		crtc_state->base.plane_mask |= BIT(drm_plane_index(&plane->base));
+		crtc_state->active_planes |= BIT(plane->id);
+	} else {
+		crtc_state->base.plane_mask &= ~BIT(drm_plane_index(&plane->base));
+		crtc_state->active_planes &= ~BIT(plane->id);
+	}
+
+	DRM_DEBUG_KMS("%s active planes 0x%x\n",
+		      crtc_state->base.crtc->name,
+		      crtc_state->active_planes);
+}
+
+static void
 intel_find_initial_plane_obj(struct intel_crtc *intel_crtc,
 			     struct intel_initial_plane_config *plane_config)
 {
@@ -2798,8 +2821,9 @@  intel_find_initial_plane_obj(struct intel_crtc *intel_crtc,
 	 * simplest solution is to just disable the primary plane now and
 	 * pretend the BIOS never had it enabled.
 	 */
-	to_intel_plane_state(plane_state)->base.visible = false;
-	crtc_state->plane_mask &= ~(1 << drm_plane_index(primary));
+	intel_set_plane_visible(to_intel_crtc_state(crtc_state),
+				to_intel_plane_state(plane_state),
+				false);
 	intel_pre_disable_primary_noatomic(&intel_crtc->base);
 	intel_plane->disable_plane(primary, &intel_crtc->base);
 
@@ -2826,7 +2850,11 @@  intel_find_initial_plane_obj(struct intel_crtc *intel_crtc,
 	drm_framebuffer_reference(fb);
 	primary->fb = primary->state->fb = fb;
 	primary->crtc = primary->state->crtc = &intel_crtc->base;
-	intel_crtc->base.state->plane_mask |= (1 << drm_plane_index(primary));
+
+	intel_set_plane_visible(to_intel_crtc_state(intel_crtc->base.state),
+				to_intel_plane_state(primary->state),
+				true);
+
 	atomic_or(to_intel_plane(primary)->frontbuffer_bit,
 		  &obj->frontbuffer_bits);
 }
@@ -12440,11 +12468,11 @@  int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
 	struct intel_crtc_state *pipe_config = to_intel_crtc_state(crtc_state);
 	struct drm_crtc *crtc = crtc_state->crtc;
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
-	struct drm_plane *plane = plane_state->plane;
+	struct intel_plane *plane = to_intel_plane(plane_state->plane);
 	struct drm_device *dev = crtc->dev;
 	struct drm_i915_private *dev_priv = to_i915(dev);
 	struct intel_plane_state *old_plane_state =
-		to_intel_plane_state(plane->state);
+		to_intel_plane_state(plane->base.state);
 	bool mode_changed = needs_modeset(crtc_state);
 	bool was_crtc_enabled = crtc->state->active;
 	bool is_crtc_enabled = crtc_state->active;
@@ -12452,7 +12480,7 @@  int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
 	struct drm_framebuffer *fb = plane_state->fb;
 	int ret;
 
-	if (INTEL_GEN(dev_priv) >= 9 && plane->type != DRM_PLANE_TYPE_CURSOR) {
+	if (INTEL_GEN(dev_priv) >= 9 && plane->id != PLANE_CURSOR) {
 		ret = skl_update_scaler_plane(
 			to_intel_crtc_state(crtc_state),
 			to_intel_plane_state(plane_state));
@@ -12476,8 +12504,10 @@  int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
 	 * per-plane wm computation to the .check_plane() hook, and
 	 * only combine the results from all planes in the current place?
 	 */
-	if (!is_crtc_enabled)
+	if (!is_crtc_enabled) {
 		to_intel_plane_state(plane_state)->base.visible = visible = false;
+		to_intel_crtc_state(crtc_state)->active_planes &= ~BIT(plane->id);
+	}
 
 	if (!was_visible && !visible)
 		return 0;
@@ -12489,13 +12519,12 @@  int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
 	turn_on = visible && (!was_visible || mode_changed);
 
 	DRM_DEBUG_ATOMIC("[CRTC:%d:%s] has [PLANE:%d:%s] with fb %i\n",
-			 intel_crtc->base.base.id,
-			 intel_crtc->base.name,
-			 plane->base.id, plane->name,
+			 intel_crtc->base.base.id, intel_crtc->base.name,
+			 plane->base.base.id, plane->base.name,
 			 fb ? fb->base.id : -1);
 
 	DRM_DEBUG_ATOMIC("[PLANE:%d:%s] visible %i -> %i, off %i, on %i, ms %i\n",
-			 plane->base.id, plane->name,
+			 plane->base.base.id, plane->base.name,
 			 was_visible, visible,
 			 turn_off, turn_on, mode_changed);
 
@@ -12503,15 +12532,15 @@  int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
 		pipe_config->update_wm_pre = true;
 
 		/* must disable cxsr around plane enable/disable */
-		if (plane->type != DRM_PLANE_TYPE_CURSOR)
+		if (plane->id != PLANE_CURSOR)
 			pipe_config->disable_cxsr = true;
 	} else if (turn_off) {
 		pipe_config->update_wm_post = true;
 
 		/* must disable cxsr around plane enable/disable */
-		if (plane->type != DRM_PLANE_TYPE_CURSOR)
+		if (plane->id != PLANE_CURSOR)
 			pipe_config->disable_cxsr = true;
-	} else if (intel_wm_need_update(plane, plane_state)) {
+	} else if (intel_wm_need_update(&plane->base, plane_state)) {
 		/* FIXME bollocks */
 		pipe_config->update_wm_pre = true;
 		pipe_config->update_wm_post = true;
@@ -12523,7 +12552,7 @@  int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
 		to_intel_crtc_state(crtc_state)->wm.need_postvbl_update = true;
 
 	if (visible || was_visible)
-		pipe_config->fb_bits |= to_intel_plane(plane)->frontbuffer_bit;
+		pipe_config->fb_bits |= plane->frontbuffer_bit;
 
 	/*
 	 * WaCxSRDisabledForSpriteScaling:ivb
@@ -12531,7 +12560,7 @@  int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
 	 * cstate->update_wm was already set above, so this flag will
 	 * take effect when we commit and program watermarks.
 	 */
-	if (plane->type == DRM_PLANE_TYPE_OVERLAY && IS_IVYBRIDGE(dev_priv) &&
+	if (plane->id == PLANE_SPRITE0 && IS_IVYBRIDGE(dev_priv) &&
 	    needs_scaling(to_intel_plane_state(plane_state)) &&
 	    !needs_scaling(old_plane_state))
 		pipe_config->disable_lp_wm = true;
@@ -16876,15 +16905,14 @@  static bool primary_get_hw_state(struct intel_plane *plane)
 /* FIXME read out full plane state for all planes */
 static void readout_plane_state(struct intel_crtc *crtc)
 {
-	struct drm_plane *primary = crtc->base.primary;
-	struct intel_plane_state *plane_state =
-		to_intel_plane_state(primary->state);
+	struct intel_plane *primary = to_intel_plane(crtc->base.primary);
+	bool visible;
 
-	plane_state->base.visible = crtc->active &&
-		primary_get_hw_state(to_intel_plane(primary));
+	visible = crtc->active && primary_get_hw_state(primary);
 
-	if (plane_state->base.visible)
-		crtc->base.state->plane_mask |= 1 << drm_plane_index(primary);
+	intel_set_plane_visible(to_intel_crtc_state(crtc->base.state),
+				to_intel_plane_state(primary->base.state),
+				visible);
 }
 
 static void intel_modeset_readout_hw_state(struct drm_device *dev)
@@ -17171,7 +17199,10 @@  void intel_modeset_gem_init(struct drm_device *dev)
 			c->primary->fb = NULL;
 			c->primary->crtc = c->primary->state->crtc = NULL;
 			update_state_fb(c->primary);
-			c->state->plane_mask &= ~(1 << drm_plane_index(c->primary));
+
+			intel_set_plane_visible(to_intel_crtc_state(c->state),
+						to_intel_plane_state(c->primary->state),
+						false);
 		}
 	}
 }
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 8f4ddca0f521..20ba8f48bc3b 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -669,6 +669,9 @@  struct intel_crtc_state {
 
 	/* Gamma mode programmed on the pipe */
 	uint32_t gamma_mode;
+
+	/* bitmask of visible planes (enum plane_id) */
+	u8 active_planes;
 };
 
 struct vlv_wm_state {