Re: [PATCH 15/19] drm/i915: Simplify skl_max_scale() - Juha-Pekka Heikkilä

From: "Juha-Pekka Heikkilä" <juhapekka.heikkila@gmail.com>
To: Ville Syrjala <ville.syrjala@linux.intel.com>,
	intel-gfx@lists.freedesktop.org
Cc: dri-devel@lists.freedesktop.org
Subject: Re: [PATCH 15/19] drm/i915: Simplify skl_max_scale()
Date: Mon, 16 Sep 2019 23:01:45 +0300	[thread overview]
Message-ID: <c973e661-1d28-eb62-0c9a-ff2421b8e924@gmail.com> (raw)
In-Reply-To: <20190708125325.16576-16-ville.syrjala@linux.intel.com>

Patches 13, 14 and this 15 look ok to me. Those num/den combos in 13 I 
cannot bet my head on but the plumbing look all ok.

Also if on 1..8 some patch wasn't pushed yet, those are all

Reviewed-by: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>

Ville Syrjala kirjoitti 8.7.2019 klo 15.53:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Now that the planes declare their minimum cdclk requirements properly
> we don't need to check the cdclk in skl_max_scale() anymore. Just check
> against the maximum downscale ratio, and move the code next to it's
> only caller.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>   drivers/gpu/drm/i915/display/intel_display.c | 38 --------------------
>   drivers/gpu/drm/i915/display/intel_sprite.c  | 12 ++++++-
>   drivers/gpu/drm/i915/intel_drv.h             |  2 --
>   3 files changed, 11 insertions(+), 41 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 1e67fbe50476..489620ef476b 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -14525,44 +14525,6 @@ intel_cleanup_plane_fb(struct drm_plane *plane,
>   	mutex_unlock(&dev_priv->drm.struct_mutex);
>   }
>   
> -int
> -skl_max_scale(const struct intel_crtc_state *crtc_state,
> -	      const struct drm_format_info *format)
> -{
> -	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
> -	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> -	int max_scale;
> -	int crtc_clock, max_dotclk, tmpclk1, tmpclk2;
> -
> -	if (!crtc_state->base.enable)
> -		return DRM_PLANE_HELPER_NO_SCALING;
> -
> -	crtc_clock = crtc_state->base.adjusted_mode.crtc_clock;
> -	max_dotclk = to_intel_atomic_state(crtc_state->base.state)->cdclk.logical.cdclk;
> -
> -	if (IS_GEMINILAKE(dev_priv) || INTEL_GEN(dev_priv) >= 10)
> -		max_dotclk *= 2;
> -
> -	if (WARN_ON_ONCE(!crtc_clock || max_dotclk < crtc_clock))
> -		return DRM_PLANE_HELPER_NO_SCALING;
> -
> -	/*
> -	 * skl max scale is lower of:
> -	 *    close to 3 but not 3, -1 is for that purpose
> -	 *            or
> -	 *    cdclk/crtc_clock
> -	 */
> -	if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv) ||
> -	    !drm_format_info_is_yuv_semiplanar(format))
> -		tmpclk1 = 0x30000 - 1;
> -	else
> -		tmpclk1 = 0x20000 - 1;
> -	tmpclk2 = (1 << 8) * ((max_dotclk << 8) / crtc_clock);
> -	max_scale = min(tmpclk1, tmpclk2);
> -
> -	return max_scale;
> -}
> -
>   static void intel_begin_crtc_commit(struct intel_atomic_state *state,
>   				    struct intel_crtc *crtc)
>   {
> diff --git a/drivers/gpu/drm/i915/display/intel_sprite.c b/drivers/gpu/drm/i915/display/intel_sprite.c
> index a07887279e1a..0ffbec8291ee 100644
> --- a/drivers/gpu/drm/i915/display/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/display/intel_sprite.c
> @@ -2089,6 +2089,16 @@ static int skl_plane_check_nv12_rotation(const struct intel_plane_state *plane_s
>   	return 0;
>   }
>   
> +static int skl_plane_max_scale(struct drm_i915_private *dev_priv,
> +			       const struct drm_framebuffer *fb)
> +{
> +	if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv) ||
> +	    !drm_format_info_is_yuv_semiplanar(fb->format))
> +		return 0x30000 - 1;
> +	else
> +		return 0x20000 - 1;
> +}
> +
>   static int skl_plane_check(struct intel_crtc_state *crtc_state,
>   			   struct intel_plane_state *plane_state)
>   {
> @@ -2106,7 +2116,7 @@ static int skl_plane_check(struct intel_crtc_state *crtc_state,
>   	/* use scaler when colorkey is not required */
>   	if (!plane_state->ckey.flags && intel_fb_scalable(fb)) {
>   		min_scale = 1;
> -		max_scale = skl_max_scale(crtc_state, fb->format);
> +		max_scale = skl_plane_max_scale(dev_priv, fb);
>   	}
>   
>   	ret = drm_atomic_helper_check_plane_state(&plane_state->base,
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 999ad3166cd1..02eeaec86997 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1620,8 +1620,6 @@ void intel_crtc_arm_fifo_underrun(struct intel_crtc *crtc,
>   
>   u16 skl_scaler_calc_phase(int sub, int scale, bool chroma_center);
>   int skl_update_scaler_crtc(struct intel_crtc_state *crtc_state);
> -int skl_max_scale(const struct intel_crtc_state *crtc_state,
> -		  const struct drm_format_info *format);
>   
>   static inline u32 intel_plane_ggtt_offset(const struct intel_plane_state *state)
>   {
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx