Re: [PATCH v4 6/6] drm/i915/display: Load the lut values and enable sharpness

["Nautiyal, Ankit K" <ankit.k.nautiyal@intel.com>]

On 1/13/2025 4:19 PM, Nemesa Garg wrote:
> Load the lut values during pipe enable.
>
> v2: Add the display version check
> v3: Fix build issue
> v4: Rebase
>
> Signed-off-by: Nemesa Garg <nemesa.garg@intel.com>
> Reviewed-by: Naga Venkata Srikanth V <nagavenkata.srikanth.v@intel.com>
> ---
>   drivers/gpu/drm/i915/display/intel_crtc.c          |  3 +++
>   drivers/gpu/drm/i915/display/intel_display.c       |  6 ++++++
>   drivers/gpu/drm/i915/display/intel_display_types.h |  2 ++
>   drivers/gpu/drm/i915/display/skl_scaler.c          | 14 +++++++++++++-
>   4 files changed, 24 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_crtc.c b/drivers/gpu/drm/i915/display/intel_crtc.c
> index c910168602d2..f502530a98af 100644
> --- a/drivers/gpu/drm/i915/display/intel_crtc.c
> +++ b/drivers/gpu/drm/i915/display/intel_crtc.c
> @@ -389,6 +389,9 @@ int intel_crtc_init(struct drm_i915_private *dev_priv, enum pipe pipe)
>   
>   	drm_WARN_ON(&dev_priv->drm, drm_crtc_index(&crtc->base) != crtc->pipe);
>   
> +	if (DISPLAY_VER(dev_priv) >= 20)

use HAS_CASF macro


> +		drm_crtc_create_sharpness_strength_property(&crtc->base);
> +
>   	return 0;
>   
>   fail:
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 7ab885fad189..52e8b6c86347 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -1881,6 +1881,9 @@ static void hsw_crtc_enable(struct intel_atomic_state *state,
>   			intel_crtc_wait_for_next_vblank(wa_crtc);
>   		}
>   	}
> +
> +	if (new_crtc_state->hw.casf_params.strength_changed)

Where is the strength_changed set?


> +		intel_filter_lut_load(crtc, new_crtc_state);
>   }
>   
>   void ilk_pfit_disable(const struct intel_crtc_state *old_crtc_state)
> @@ -7182,6 +7185,9 @@ static void intel_pre_update_crtc(struct intel_atomic_state *state,
>   			intel_vrr_set_transcoder_timings(new_crtc_state);
>   	}
>   
> +	if (intel_casf_strength_changed(new_crtc_state, old_crtc_state))
> +		intel_casf_enable(new_crtc_state);

When a user changes sharpness strength, there can be 3 transitions:
If transition from  0 to strength 'x'
Intel_casf_enable() -> Enable Sharpness: need to write all scaler 
registers, Sharpness LUT values and Sharpness CTL etc.
Transition from x to y
intel_update_sharpness() -> Only update sharpness strength.
Transition from x to 0
intel_casf_disable() -> Disable Sharpness: disable scaler, sharpness ctl 
etc.


> +
>   	intel_fbc_update(state, crtc);
>   
>   	drm_WARN_ON(&i915->drm, !intel_display_power_is_enabled(i915, POWER_DOMAIN_DC_OFF));
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> index e5d28377bd0b..589596bfd8c3 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -940,6 +940,8 @@ struct intel_casf {
>   	struct scaler_filter_coeff coeff[SCALER_FILTER_NUM_TAPS];
>   	u8 win_size;
>   	bool need_scaler;
> +	bool strength_changed;
> +	u8 strength;
>   };
>   
>   void intel_io_mmio_fw_write(void *ctx, i915_reg_t reg, u32 val);
> diff --git a/drivers/gpu/drm/i915/display/skl_scaler.c b/drivers/gpu/drm/i915/display/skl_scaler.c
> index 40584bc19dbb..60e2ec86ee90 100644
> --- a/drivers/gpu/drm/i915/display/skl_scaler.c
> +++ b/drivers/gpu/drm/i915/display/skl_scaler.c
> @@ -9,6 +9,7 @@
>   #include "intel_display_trace.h"
>   #include "intel_display_types.h"
>   #include "intel_fb.h"
> +#include "intel_casf_regs.h"
>   #include "skl_scaler.h"
>   #include "skl_universal_plane.h"
>   
> @@ -896,7 +897,7 @@ void skl_scaler_get_config(struct intel_crtc_state *crtc_state)
>   
>   	/* find scaler attached to this pipe */
>   	for (i = 0; i < crtc->num_scalers; i++) {
> -		u32 ctl, pos, size;
> +		u32 ctl, pos, size, sharp;
>   
>   		ctl = intel_de_read(display, SKL_PS_CTRL(crtc->pipe, i));
>   		if ((ctl & (PS_SCALER_EN | PS_BINDING_MASK)) != (PS_SCALER_EN | PS_BINDING_PIPE))
> @@ -904,6 +905,17 @@ void skl_scaler_get_config(struct intel_crtc_state *crtc_state)
>   
>   		id = i;
>   
> +		if (DISPLAY_VER(display) >= 20) {

Use HAS_CASF macro.

Also this should be checked only for second scaler ie. when id == 1.


> +			sharp = intel_de_read(display, SHARPNESS_CTL(crtc->pipe));
> +			if (sharp & FILTER_EN) {
> +				crtc_state->hw.casf_params.strength =
> +					REG_FIELD_GET(FILTER_STRENGTH_MASK, sharp) - 16;

strength is u16, perhaps need to check that the value read is greater 
than equal to 16.

In such  a case, print error and perhaps set strength to 0.


Regards,

Ankit

> +				crtc_state->hw.casf_params.need_scaler = true;
> +				crtc_state->hw.casf_params.win_size =
> +					REG_FIELD_GET(FILTER_SIZE_MASK, sharp);
> +			}
> +		}
> +
>   		if (!crtc_state->hw.casf_params.need_scaler)
>   			crtc_state->pch_pfit.enabled = true;
>