Re: [PATCH 02/25] drm/i915/fbc: extract intel_fbc_can_activate()

[Maarten Lankhorst <maarten.lankhorst@linux.intel.com>]

Op 19-01-16 om 14:35 schreef Paulo Zanoni:
> Extract all the code that checks if the FBC configuration is valid to
> its own function, making __intel_fbc_update() much simpler.
>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_fbc.c | 92 ++++++++++++++++++++++------------------
>  1 file changed, 50 insertions(+), 42 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
> index 6b43ec3..dff30e1 100644
> --- a/drivers/gpu/drm/i915/intel_fbc.c
> +++ b/drivers/gpu/drm/i915/intel_fbc.c
> @@ -514,17 +514,6 @@ static bool crtc_can_fbc(struct intel_crtc *crtc)
>  	return true;
>  }
>  
> -static bool crtc_is_valid(struct intel_crtc *crtc)
> -{
> -	if (!intel_crtc_active(&crtc->base))
> -		return false;
> -	if (!to_intel_plane_state(crtc->base.primary->state)->visible)
> -		return false;
> -
> -	return true;
> -}
> -
>  static bool multiple_pipes_ok(struct drm_i915_private *dev_priv)
>  {
>  	enum pipe pipe;
> @@ -750,48 +739,40 @@ static bool intel_fbc_hw_tracking_covers_screen(struct intel_crtc *crtc)
>  	return effective_w <= max_w && effective_h <= max_h;
>  }
>  
> -/**
> - * __intel_fbc_update - activate/deactivate FBC as needed, unlocked
> - * @crtc: the CRTC that triggered the update
> - *
> - * This function completely reevaluates the status of FBC, then activates,
> - * deactivates or maintains it on the same state.
> - */
> -static void __intel_fbc_update(struct intel_crtc *crtc)
> +static bool intel_fbc_can_activate(struct intel_crtc *crtc)
>  {
>  	struct drm_i915_private *dev_priv = crtc->base.dev->dev_private;
> +	struct drm_plane *primary;
>  	struct drm_framebuffer *fb;
> +	struct intel_plane_state *plane_state;
>  	struct drm_i915_gem_object *obj;
>  	const struct drm_display_mode *adjusted_mode;
It would be nice if this function had a plane_state and crtc_state passed into it for validation.
I suppose it can wait though, maybe something for the future.
> -	WARN_ON(!mutex_is_locked(&dev_priv->fbc.lock));
> -
> -	if (!multiple_pipes_ok(dev_priv)) {
> -		set_no_fbc_reason(dev_priv, "more than one pipe active");
> -		goto out_disable;
> -	}
> -
> -	if (!dev_priv->fbc.enabled || dev_priv->fbc.crtc != crtc)
> -		return;
> -
> -	if (!crtc_is_valid(crtc)) {
> -		set_no_fbc_reason(dev_priv, "no output");
> -		goto out_disable;
> +	if (!intel_crtc_active(&crtc->base)) {
> +		set_no_fbc_reason(dev_priv, "CRTC not active");
> +		return false;
>  	}
This one has become redundant with plane_state->visible. You can't have a visible plane with crtc off any more.

~Maarten
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx