Re: [PATCH v3 07/20] drm/i915: Rework plane readout.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Jul 13, 2015 at 04:30:20PM +0200, Maarten Lankhorst wrote:
> All non-primary planes get disabled during hw readout,
> this reduces complexity and means not having to do some plane
> visibility checks during the first commit.
> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx>

I still think calling readout_plane_state from the hw state readout code
is the wrong approach. Instead we should consolidate all the plane
readout code exclusively into a new intel_plane_readout_hw_state helper
which is called only from driver load paths. That should also contain the
fb/gem_bo reconstruction loop.

For the other 2 users of this code (lid notifiery and resume) we should
just force an unconditional plane restore by setting
crtc_state->planes_chagned. But I thinkt hat's ok as some follow-up work,
once atomic has settled a bit more.
-Daniel

> ---
>  drivers/gpu/drm/i915/intel_atomic.c  |  7 ---
>  drivers/gpu/drm/i915/intel_display.c | 86 ++++--------------------------------
>  drivers/gpu/drm/i915/intel_drv.h     |  1 -
>  3 files changed, 8 insertions(+), 86 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c
> index b92b8581efc2..dcf4fb458649 100644
> --- a/drivers/gpu/drm/i915/intel_atomic.c
> +++ b/drivers/gpu/drm/i915/intel_atomic.c
> @@ -98,13 +98,6 @@ int intel_atomic_check(struct drm_device *dev,
>  		return -EINVAL;
>  	}
>  
> -	if (crtc_state &&
> -	    crtc_state->quirks & PIPE_CONFIG_QUIRK_INITIAL_PLANES) {
> -		ret = drm_atomic_add_affected_planes(state, &nuclear_crtc->base);
> -		if (ret)
> -			return ret;
> -	}
> -
>  	ret = drm_atomic_helper_check_planes(dev, state);
>  	if (ret)
>  		return ret;
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index e4d8acc63823..118187dc76be 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -11783,34 +11783,6 @@ static bool check_encoder_cloning(struct drm_atomic_state *state,
>  	return true;
>  }
>  
> -static void intel_crtc_check_initial_planes(struct drm_crtc *crtc,
> -					    struct drm_crtc_state *crtc_state)
> -{
> -	struct intel_crtc_state *pipe_config =
> -		to_intel_crtc_state(crtc_state);
> -	struct drm_plane *p;
> -	unsigned visible_mask = 0;
> -
> -	drm_for_each_plane_mask(p, crtc->dev, crtc_state->plane_mask) {
> -		struct drm_plane_state *plane_state =
> -			drm_atomic_get_existing_plane_state(crtc_state->state, p);
> -
> -		if (WARN_ON(!plane_state))
> -			continue;
> -
> -		if (!plane_state->fb)
> -			crtc_state->plane_mask &=
> -				~(1 << drm_plane_index(p));
> -		else if (to_intel_plane_state(plane_state)->visible)
> -			visible_mask |= 1 << drm_plane_index(p);
> -	}
> -
> -	if (!visible_mask)
> -		return;
> -
> -	pipe_config->quirks &= ~PIPE_CONFIG_QUIRK_INITIAL_PLANES;
> -}
> -
>  static int intel_crtc_atomic_check(struct drm_crtc *crtc,
>  				   struct drm_crtc_state *crtc_state)
>  {
> @@ -11832,10 +11804,6 @@ static int intel_crtc_atomic_check(struct drm_crtc *crtc,
>  		"[CRTC:%i] mismatch between state->active(%i) and crtc->active(%i)\n",
>  		idx, crtc->state->active, intel_crtc->active);
>  
> -	/* plane mask is fixed up after all initial planes are calculated */
> -	if (pipe_config->quirks & PIPE_CONFIG_QUIRK_INITIAL_PLANES)
> -		intel_crtc_check_initial_planes(crtc, crtc_state);
> -
>  	if (mode_changed && !crtc_state->active)
>  		intel_crtc->atomic.update_wm_post = true;
>  
> @@ -13188,19 +13156,6 @@ intel_modeset_compute_config(struct drm_atomic_state *state)
>  			continue;
>  		}
>  
> -		if (pipe_config->quirks & PIPE_CONFIG_QUIRK_INITIAL_PLANES) {
> -			ret = drm_atomic_add_affected_planes(state, crtc);
> -			if (ret)
> -				return ret;
> -
> -			/*
> -			 * We ought to handle i915.fastboot here.
> -			 * If no modeset is required and the primary plane has
> -			 * a fb, update the members of crtc_state as needed,
> -			 * and run the necessary updates during vblank evasion.
> -			 */
> -		}
> -
>  		modeset = needs_modeset(crtc_state);
>  		recalc = pipe_config->quirks & PIPE_CONFIG_QUIRK_INHERITED_MODE;
>  
> @@ -15460,47 +15415,22 @@ static void readout_plane_state(struct intel_crtc *crtc,
>  				struct intel_crtc_state *crtc_state)
>  {
>  	struct intel_plane *p;
> -	struct drm_plane_state *drm_plane_state;
> +	struct intel_plane_state *plane_state;
>  	bool active = crtc_state->base.active;
>  
> -	if (active) {
> -		crtc_state->quirks |= PIPE_CONFIG_QUIRK_INITIAL_PLANES;
> -
> -		/* apply to previous sw state too */
> -		to_intel_crtc_state(crtc->base.state)->quirks |=
> -			PIPE_CONFIG_QUIRK_INITIAL_PLANES;
> -	}
> -
>  	for_each_intel_plane(crtc->base.dev, p) {
> -		bool visible = active;
> -
>  		if (crtc->pipe != p->pipe)
>  			continue;
>  
> -		drm_plane_state = p->base.state;
> -
> -		/* Plane scaler state is not touched here. The first atomic
> -		 * commit will restore all plane scalers to its old state.
> -		 */
> +		plane_state = to_intel_plane_state(p->base.state);
>  
> -		if (active && p->base.type == DRM_PLANE_TYPE_PRIMARY) {
> -			visible = primary_get_hw_state(crtc);
> -			to_intel_plane_state(drm_plane_state)->visible = visible;
> -		} else {
> -			/*
> -			 * unknown state, assume it's off to force a transition
> -			 * to on when calculating state changes.
> -			 */
> -			to_intel_plane_state(drm_plane_state)->visible = false;
> -		}
> +		if (p->base.type == DRM_PLANE_TYPE_PRIMARY)
> +			plane_state->visible = primary_get_hw_state(crtc);
> +		else {
> +			if (active)
> +				p->disable_plane(&p->base, &crtc->base);
>  
> -		if (visible) {
> -			crtc_state->base.plane_mask |=
> -				1 << drm_plane_index(&p->base);
> -		} else if (crtc_state->base.state) {
> -			/* Make this unconditional for atomic hw readout. */
> -			crtc_state->base.plane_mask &=
> -				~(1 << drm_plane_index(&p->base));
> +			plane_state->visible = false;
>  		}
>  	}
>  }
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 09e3581c8441..2c23900b491f 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -341,7 +341,6 @@ struct intel_crtc_state {
>  	 */
>  #define PIPE_CONFIG_QUIRK_MODE_SYNC_FLAGS	(1<<0) /* unreliable sync mode.flags */
>  #define PIPE_CONFIG_QUIRK_INHERITED_MODE	(1<<1) /* mode inherited from firmware */
> -#define PIPE_CONFIG_QUIRK_INITIAL_PLANES	(1<<2) /* planes are in unknown state */
>  	unsigned long quirks;
>  
>  	/* Pipe source size (ie. panel fitter input size)
> -- 
> 2.1.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux