Re: [PATCH 1/2] drm/i915: Clean up skl_plane_has_planar()

From: Rodrigo Vivi <rodrigo.vivi@intel.com>
To: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH 1/2] drm/i915: Clean up skl_plane_has_planar()
Date: Fri, 24 Aug 2018 15:15:35 -0700	[thread overview]
Message-ID: <20180824221535.GD6432@intel.com> (raw)
In-Reply-To: <20180824203856.17700-1-dhinakaran.pandiyan@intel.com>

On Fri, Aug 24, 2018 at 01:38:55PM -0700, Dhinakaran Pandiyan wrote:
> skl_plane_has_planar is hard to read, simplify the logic by checking for
> support in the order of platform, pipe and plane.
> 
> No change in functionality intended.
> 
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 27 +++++++++------------------
>  1 file changed, 9 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 30fdfd1a3037..7e18bd8b21b8 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -13622,24 +13622,15 @@ static bool skl_plane_has_fbc(struct drm_i915_private *dev_priv,
>  bool skl_plane_has_planar(struct drm_i915_private *dev_priv,
>  			  enum pipe pipe, enum plane_id plane_id)
>  {
> -	if (plane_id == PLANE_PRIMARY) {
> -		if (IS_SKYLAKE(dev_priv) || IS_BROXTON(dev_priv))
> -			return false;
> -		else if ((INTEL_GEN(dev_priv) == 9 && pipe == PIPE_C) &&
> -			 !IS_GEMINILAKE(dev_priv))
> -			return false;
> -	} else if (plane_id >= PLANE_SPRITE0) {
> -		if (plane_id == PLANE_CURSOR)
> -			return false;
> -		if (IS_GEMINILAKE(dev_priv) || INTEL_GEN(dev_priv) == 10) {
> -			if (plane_id != PLANE_SPRITE0)
> -				return false;
> -		} else {
> -			if (plane_id != PLANE_SPRITE0 || pipe == PIPE_C ||
> -			    IS_SKYLAKE(dev_priv) || IS_BROXTON(dev_priv))
> -				return false;
> -		}
> -	}
> +	if (IS_SKYLAKE(dev_priv) || IS_BROXTON(dev_priv))
> +		return false;

The current code is indeed ugly, but I'm afraid it doesn't always return
false for these platforms.

for instance plane_id = PLANE_SPRITE0

> +
> +	if (INTEL_GEN(dev_priv) == 9 && !IS_GEMINILAKE(dev_priv) && pipe == PIPE_C)
> +		return false;
> +
> +	if (plane_id == PLANE_CURSOR || plane_id != PLANE_SPRITE0)
> +		return false;
> +
>  	return true;
>  }
>  
> -- 
> 2.17.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx