Re: [PATCH 9/9] drm/i915/casf: Integrate the sharpness filter properly into the scaler code

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Fri, Mar 27, 2026 at 12:31:39AM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> The sharpness filter is just a special mode of the pipe scaler.
> It doesn't warrant all this special casing everywhere. Just
> integrate it properly into the scaler code so that it's treated
> no different from the other pipe scaler uses (scaling,centering,
> YCbCr 4:2:0 output).
> 
> Cc: Nemesa Garg <nemesa.garg@intel.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_casf.c    | 62 ++------------
>  drivers/gpu/drm/i915/display/intel_casf.h    |  6 +-
>  drivers/gpu/drm/i915/display/intel_display.c | 34 +-------
>  drivers/gpu/drm/i915/display/intel_pfit.c    | 13 ++-
>  drivers/gpu/drm/i915/display/skl_scaler.c    | 85 ++++++++------------
>  drivers/gpu/drm/i915/display/skl_scaler.h    |  2 -
>  6 files changed, 53 insertions(+), 149 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_casf.c b/drivers/gpu/drm/i915/display/intel_casf.c
> index 21e84a4f9ff5..c2d2746c5f04 100644
> --- a/drivers/gpu/drm/i915/display/intel_casf.c
> +++ b/drivers/gpu/drm/i915/display/intel_casf.c
> @@ -75,20 +75,6 @@ static void intel_casf_filter_lut_load(const struct intel_crtc_state *crtc_state
>  			       sharpness_lut[i]);
>  }
>  
> -void intel_casf_update_strength(const struct intel_crtc_state *crtc_state)
> -{
> -	struct intel_display *display = to_intel_display(crtc_state);
> -	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> -	int win_size;
> -
> -	intel_de_rmw(display, SHARPNESS_CTL(crtc->pipe), FILTER_STRENGTH_MASK,
> -		     FILTER_STRENGTH(crtc_state->pch_pfit.casf.strength));
> -
> -	win_size = intel_de_read(display, SKL_PS_WIN_SZ(crtc->pipe, 1));
> -
> -	intel_de_write_fw(display, SKL_PS_WIN_SZ(crtc->pipe, 1), win_size);
> -}
> -
>  static void intel_casf_compute_win_size(struct intel_crtc_state *crtc_state)
>  {
>  	const struct drm_display_mode *mode = &crtc_state->hw.adjusted_mode;
> @@ -102,19 +88,15 @@ static void intel_casf_compute_win_size(struct intel_crtc_state *crtc_state)
>  		crtc_state->pch_pfit.casf.win_size = SHARPNESS_FILTER_SIZE_7X7;
>  }
>  
> +static void intel_casf_scaler_compute_coef(struct intel_crtc_state *crtc_state);
> +
>  int intel_casf_compute_config(struct intel_crtc_state *crtc_state)
>  {
>  	struct intel_display *display = to_intel_display(crtc_state);
>  
> -	if (!HAS_CASF(display))
> +	if (crtc_state->hw.sharpness_strength == 0)
>  		return 0;
>  
> -	if (crtc_state->hw.sharpness_strength == 0) {
> -		crtc_state->pch_pfit.casf.enable = false;
> -		crtc_state->pch_pfit.casf.strength = 0;
> -		return 0;
> -	}
> -
>  	/* CASF with joiner not supported in hardware */
>  	if (crtc_state->joiner_pipes) {
>  		drm_dbg_kms(display->drm, "CASF not supported with joiner\n");
> @@ -136,7 +118,7 @@ int intel_casf_compute_config(struct intel_crtc_state *crtc_state)
>  
>  	intel_casf_compute_win_size(crtc_state);
>  
> -	intel_casf_scaler_compute_config(crtc_state);
> +	intel_casf_scaler_compute_coef(crtc_state);
>  
>  	return 0;
>  }
> @@ -161,14 +143,6 @@ void intel_casf_sharpness_get_config(struct intel_crtc_state *crtc_state)
>  	}
>  }
>  
> -bool intel_casf_needs_scaler(const struct intel_crtc_state *crtc_state)
> -{
> -	if (crtc_state->pch_pfit.casf.enable)
> -		return true;
> -
> -	return false;
> -}
> -
>  static int casf_coeff_tap(int i)
>  {
>  	return i % SCALER_FILTER_NUM_TAPS;
> @@ -240,7 +214,7 @@ static void convert_sharpness_coef_binary(struct scaler_filter_coeff *coeff,
>  	}
>  }
>  
> -void intel_casf_scaler_compute_config(struct intel_crtc_state *crtc_state)
> +static void intel_casf_scaler_compute_coef(struct intel_crtc_state *crtc_state)
>  {
>  	const u16 *filtercoeff;
>  	u16 filter_coeff[SCALER_FILTER_NUM_TAPS];
> @@ -264,32 +238,8 @@ void intel_casf_scaler_compute_config(struct intel_crtc_state *crtc_state)
>  	}
>  }
>  
> -void intel_casf_enable(const struct intel_crtc_state *crtc_state)
> +void intel_casf_setup(const struct intel_crtc_state *crtc_state)
>  {
> -	struct intel_display *display = to_intel_display(crtc_state);
> -	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> -	u32 sharpness_ctl;
> -
>  	intel_casf_filter_lut_load(crtc_state);
> -
>  	intel_casf_write_coeff(crtc_state);
> -
> -	sharpness_ctl = FILTER_EN | FILTER_STRENGTH(crtc_state->pch_pfit.casf.strength);
> -
> -	sharpness_ctl |= crtc_state->pch_pfit.casf.win_size;
> -
> -	intel_de_write(display, SHARPNESS_CTL(crtc->pipe), sharpness_ctl);
> -
> -	skl_scaler_setup_casf(crtc_state);
> -}
> -
> -void intel_casf_disable(const struct intel_crtc_state *crtc_state)
> -{
> -	struct intel_display *display = to_intel_display(crtc_state);
> -	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> -
> -	intel_de_write(display, SKL_PS_CTRL(crtc->pipe, 1), 0);
> -	intel_de_write(display, SKL_PS_WIN_POS(crtc->pipe, 1), 0);
> -	intel_de_write(display, SHARPNESS_CTL(crtc->pipe), 0);
> -	intel_de_write(display, SKL_PS_WIN_SZ(crtc->pipe, 1), 0);
>  }
> diff --git a/drivers/gpu/drm/i915/display/intel_casf.h b/drivers/gpu/drm/i915/display/intel_casf.h
> index c4f984b73348..3ebb7522af0a 100644
> --- a/drivers/gpu/drm/i915/display/intel_casf.h
> +++ b/drivers/gpu/drm/i915/display/intel_casf.h
> @@ -11,11 +11,7 @@
>  struct intel_crtc_state;
>  
>  int intel_casf_compute_config(struct intel_crtc_state *crtc_state);
> -void intel_casf_update_strength(const struct intel_crtc_state *new_crtc_state);
>  void intel_casf_sharpness_get_config(struct intel_crtc_state *crtc_state);
> -void intel_casf_enable(const struct intel_crtc_state *crtc_state);
> -void intel_casf_disable(const struct intel_crtc_state *crtc_state);
> -void intel_casf_scaler_compute_config(struct intel_crtc_state *crtc_state);
> -bool intel_casf_needs_scaler(const struct intel_crtc_state *crtc_state);
> +void intel_casf_setup(const struct intel_crtc_state *crtc_state);
>  
>  #endif /* __INTEL_CASF_H__ */
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index e02e69467871..58a654ca0d20 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -58,7 +58,6 @@
>  #include "intel_audio.h"
>  #include "intel_bo.h"
>  #include "intel_bw.h"
> -#include "intel_casf.h"
>  #include "intel_cdclk.h"
>  #include "intel_clock_gating.h"
>  #include "intel_color.h"
> @@ -988,24 +987,6 @@ static bool audio_disabling(const struct intel_crtc_state *old_crtc_state,
>  		 memcmp(old_crtc_state->eld, new_crtc_state->eld, MAX_ELD_BYTES) != 0);
>  }
>  
> -static bool intel_casf_enabling(const struct intel_crtc_state *new_crtc_state,
> -				const struct intel_crtc_state *old_crtc_state)
> -{
> -	if (!new_crtc_state->hw.active)
> -		return false;
> -
> -	return is_enabling(pch_pfit.casf.enable, old_crtc_state, new_crtc_state);
> -}
> -
> -static bool intel_casf_disabling(const struct intel_crtc_state *old_crtc_state,
> -				 const struct intel_crtc_state *new_crtc_state)
> -{
> -	if (!new_crtc_state->hw.active)
> -		return false;
> -
> -	return is_disabling(pch_pfit.casf.enable, old_crtc_state, new_crtc_state);
> -}
> -
>  static bool intel_crtc_lobf_enabling(const struct intel_crtc_state *old_crtc_state,
>  				     const struct intel_crtc_state *new_crtc_state)
>  {
> @@ -1187,9 +1168,6 @@ static void intel_pre_plane_update(struct intel_atomic_state *state,
>  	if (audio_disabling(old_crtc_state, new_crtc_state))
>  		intel_encoders_audio_disable(state, crtc);
>  
> -	if (intel_casf_disabling(old_crtc_state, new_crtc_state))
> -		intel_casf_disable(new_crtc_state);
> -
>  	intel_drrs_deactivate(old_crtc_state);
>  
>  	if (hsw_ips_pre_update(state, crtc))
> @@ -4308,14 +4286,9 @@ static int intel_crtc_atomic_check(struct intel_atomic_state *state,
>  		return ret;
>  	}
>  
> -	ret = intel_casf_compute_config(crtc_state);
> -	if (ret)
> -		return ret;
> -
>  	if (DISPLAY_VER(display) >= 9) {
>  		if (intel_crtc_needs_modeset(crtc_state) ||
> -		    intel_crtc_needs_fastset(crtc_state) ||
> -		    intel_casf_needs_scaler(crtc_state)) {
> +		    intel_crtc_needs_fastset(crtc_state)) {
>  			ret = skl_update_scaler_crtc(crtc_state);
>  			if (ret)
>  				return ret;
> @@ -6817,11 +6790,6 @@ static void intel_pre_update_crtc(struct intel_atomic_state *state,
>  			intel_vrr_set_transcoder_timings(new_crtc_state);
>  	}
>  
> -	if (intel_casf_enabling(new_crtc_state, old_crtc_state))
> -		intel_casf_enable(new_crtc_state);
> -	else if (new_crtc_state->pch_pfit.casf.strength != old_crtc_state->pch_pfit.casf.strength)
> -		intel_casf_update_strength(new_crtc_state);
> -
>  	intel_fbc_update(state, crtc);
>  
>  	drm_WARN_ON(display->drm, !intel_display_power_is_enabled(display, POWER_DOMAIN_DC_OFF));
> diff --git a/drivers/gpu/drm/i915/display/intel_pfit.c b/drivers/gpu/drm/i915/display/intel_pfit.c
> index 6dda496190e0..e04ca960f115 100644
> --- a/drivers/gpu/drm/i915/display/intel_pfit.c
> +++ b/drivers/gpu/drm/i915/display/intel_pfit.c
> @@ -186,6 +186,7 @@ static int pch_panel_fitting(struct intel_crtc_state *crtc_state,
>  			     const struct drm_connector_state *conn_state)
>  {
>  	struct intel_display *display = to_intel_display(crtc_state);
> +	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
>  	const struct drm_display_mode *adjusted_mode =
>  		&crtc_state->hw.adjusted_mode;
>  	int pipe_src_w = drm_rect_width(&crtc_state->pipe_src);
> @@ -195,7 +196,8 @@ static int pch_panel_fitting(struct intel_crtc_state *crtc_state,
>  	/* Native modes don't need fitting */
>  	if (adjusted_mode->crtc_hdisplay == pipe_src_w &&
>  	    adjusted_mode->crtc_vdisplay == pipe_src_h &&
> -	    crtc_state->output_format != INTEL_OUTPUT_FORMAT_YCBCR420)
> +	    crtc_state->output_format != INTEL_OUTPUT_FORMAT_YCBCR420 &&
> +	    crtc_state->hw.sharpness_strength == 0)
>  		return 0;
>  
>  	switch (conn_state->scaling_mode) {
> @@ -249,6 +251,15 @@ static int pch_panel_fitting(struct intel_crtc_state *crtc_state,
>  		return -EINVAL;
>  	}
>  
> +	if (crtc_state->hw.sharpness_strength &&
> +	    (width != pipe_src_w || height != pipe_src_h ||
> +	     crtc_state->output_format != INTEL_OUTPUT_FORMAT_RGB)) {

We should probably also reject
 sharpness_strength != 0 && scaling_filter != DEFAULT

I suppose it's not strictly needed because we only allow sharpness
with 1:1 scaling, and that makes the nearest neighbor filter a NOP.
But probably a good idea to make the uapi behaviour consistent,
especially if we later add other filter modes that maybe aren't 
NOPs.

> +		drm_dbg_kms(display->drm,
> +			    "[CRTC:%d:%s] no scaling/YCbCr output with sharpness filter\n",
> +			    crtc->base.base.id, crtc->base.name);
> +		return -EINVAL;
> +	}
> +
>  	drm_rect_init(&crtc_state->pch_pfit.dst,
>  		      x, y, width, height);
>  	crtc_state->pch_pfit.enabled = true;

-- 
Ville Syrjälä
Intel