From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 30DC910BA458 for ; Fri, 27 Mar 2026 09:10:31 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id B17E010E323; Fri, 27 Mar 2026 09:10:30 +0000 (UTC) Authentication-Results: gabe.freedesktop.org; dkim=pass (2048-bit key; unprotected) header.d=intel.com header.i=@intel.com header.b="aafARmkE"; dkim-atps=neutral Received: from mgamail.intel.com (mgamail.intel.com [192.198.163.7]) by gabe.freedesktop.org (Postfix) with ESMTPS id 9124910E28D; Fri, 27 Mar 2026 09:10:29 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1774602630; x=1806138630; h=date:from:to:cc:subject:in-reply-to:message-id: references:mime-version; bh=RJmxSgiQs0p+uJA4H8xQXMZTKCZbNooaAmBKXEODK5I=; b=aafARmkEtxH8XbituKuRnUROtS8s2vpOYK8/DgWvmi3b/f5266qqaKlt Te25DqWVATF9Kk8hQLL4+7LIEUDYqT6RmMuIeY4HUOPWBvmq61DUQ5PUV fsCDmy6CfDMitUcy6zkEqn9c+JiH7RJzo9R7r32Abuf68/sNm30joeHUA HOyGjiacfkMxiXE0hbCFyZ3xWe/k4LzHpOhJq14AFo+KNAhoMEUDRyb/J HphW2wxjWpZwHD23l64NNx2R3UmVB62brp4k+wddtr3fvlpzMeMGg0An9 kq1MXsObHZBteKzXo2L/mXoYkij0hO46VCE1pu8y+QVYGhuWvfs8pHzfk A==; X-CSE-ConnectionGUID: LtBC4RmNSX+65uSttRafhQ== X-CSE-MsgGUID: ms8akgqfQaWF70ONKYPGYg== X-IronPort-AV: E=McAfee;i="6800,10657,11741"; a="101132989" X-IronPort-AV: E=Sophos;i="6.23,143,1770624000"; d="scan'208";a="101132989" Received: from fmviesa006.fm.intel.com ([10.60.135.146]) by fmvoesa101.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 27 Mar 2026 02:10:29 -0700 X-CSE-ConnectionGUID: E8MUt1ShTruB0OYIT2wvHw== X-CSE-MsgGUID: AF+mCzC1SSK0wfzCqnN++w== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.23,143,1770624000"; d="scan'208";a="220412441" Received: from administrator-system-product-name.igk.intel.com ([10.91.214.181]) by fmviesa006.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 27 Mar 2026 02:10:28 -0700 Date: Fri, 27 Mar 2026 10:10:26 +0100 (CET) From: =?ISO-8859-2?Q?Micha=B3_Grzelak?= To: Ville Syrjala cc: intel-gfx@lists.freedesktop.org, intel-xe@lists.freedesktop.org, Nemesa Garg Subject: Re: [PATCH 3/9] drm/i915/casf: Move the casf state to better place In-Reply-To: <20260326223139.19116-4-ville.syrjala@linux.intel.com> Message-ID: References: <20260326223139.19116-1-ville.syrjala@linux.intel.com> <20260326223139.19116-4-ville.syrjala@linux.intel.com> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="8323329-753816395-1774602629=:330621" X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" This message is in MIME format. The first part should be readable text, while the remaining parts are likely unreadable without MIME-aware tools. --8323329-753816395-1774602629=:330621 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8BIT On Thu, 26 Mar 2026, Ville Syrjala wrote: > From: Ville Syrjälä > > The casf state is placed inside the 'hw' state for some reason. > That is only really meant for things we have to duplicate from > the uapi state. The rest can live on its own in our actual state. > > And since casf is just one aspect of the pfit/pipe scaler the > proper place for it seems to be under pch_pfit. > > Cc: Nemesa Garg > Signed-off-by: Ville Syrjälä > --- > drivers/gpu/drm/i915/display/intel_casf.c | 40 +++++++++---------- > .../drm/i915/display/intel_crtc_state_dump.c | 6 +-- > drivers/gpu/drm/i915/display/intel_display.c | 12 +++--- > .../drm/i915/display/intel_display_types.h | 2 +- > drivers/gpu/drm/i915/display/skl_scaler.c | 4 +- > 5 files changed, 32 insertions(+), 32 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_casf.c b/drivers/gpu/drm/i915/display/intel_casf.c > index 4316d8214e80..5a8ffb40d30d 100644 > --- a/drivers/gpu/drm/i915/display/intel_casf.c > +++ b/drivers/gpu/drm/i915/display/intel_casf.c > @@ -82,7 +82,7 @@ void intel_casf_update_strength(struct intel_crtc_state *crtc_state) > int win_size; > > intel_de_rmw(display, SHARPNESS_CTL(crtc->pipe), FILTER_STRENGTH_MASK, > - FILTER_STRENGTH(crtc_state->hw.casf_params.strength)); > + FILTER_STRENGTH(crtc_state->pch_pfit.casf.strength)); > > win_size = intel_de_read(display, SKL_PS_WIN_SZ(crtc->pipe, 1)); > > @@ -95,11 +95,11 @@ static void intel_casf_compute_win_size(struct intel_crtc_state *crtc_state) > u32 total_pixels = mode->hdisplay * mode->vdisplay; > > if (total_pixels <= MAX_PIXELS_FOR_3_TAP_FILTER) > - crtc_state->hw.casf_params.win_size = SHARPNESS_FILTER_SIZE_3X3; > + crtc_state->pch_pfit.casf.win_size = SHARPNESS_FILTER_SIZE_3X3; > else if (total_pixels <= MAX_PIXELS_FOR_5_TAP_FILTER) > - crtc_state->hw.casf_params.win_size = SHARPNESS_FILTER_SIZE_5X5; > + crtc_state->pch_pfit.casf.win_size = SHARPNESS_FILTER_SIZE_5X5; > else > - crtc_state->hw.casf_params.win_size = SHARPNESS_FILTER_SIZE_7X7; > + crtc_state->pch_pfit.casf.win_size = SHARPNESS_FILTER_SIZE_7X7; > } > > int intel_casf_compute_config(struct intel_crtc_state *crtc_state) > @@ -110,8 +110,8 @@ int intel_casf_compute_config(struct intel_crtc_state *crtc_state) > return 0; > > if (crtc_state->hw.sharpness_strength == 0) { > - crtc_state->hw.casf_params.enable = false; > - crtc_state->hw.casf_params.strength = 0; > + crtc_state->pch_pfit.casf.enable = false; > + crtc_state->pch_pfit.casf.strength = 0; > return 0; > } > > @@ -121,7 +121,7 @@ int intel_casf_compute_config(struct intel_crtc_state *crtc_state) > return -EINVAL; > } > > - crtc_state->hw.casf_params.enable = true; > + crtc_state->pch_pfit.casf.enable = true; > > /* > * HW takes a value in form (1.0 + strength) in 4.4 fixed format. > @@ -131,7 +131,7 @@ int intel_casf_compute_config(struct intel_crtc_state *crtc_state) > * 6.3125 in 4.4 format is b01100101 which is equal to 101. > * Also 85 + 16 = 101. > */ > - crtc_state->hw.casf_params.strength = > + crtc_state->pch_pfit.casf.strength = > min(crtc_state->hw.sharpness_strength, 0xEF) + 0x10; > > intel_casf_compute_win_size(crtc_state); > @@ -151,19 +151,19 @@ void intel_casf_sharpness_get_config(struct intel_crtc_state *crtc_state) > if (sharp & FILTER_EN) { > if (drm_WARN_ON(display->drm, > REG_FIELD_GET(FILTER_STRENGTH_MASK, sharp) < 16)) > - crtc_state->hw.casf_params.strength = 0; > + crtc_state->pch_pfit.casf.strength = 0; > else > - crtc_state->hw.casf_params.strength = > + crtc_state->pch_pfit.casf.strength = > REG_FIELD_GET(FILTER_STRENGTH_MASK, sharp); > - crtc_state->hw.casf_params.enable = true; > - crtc_state->hw.casf_params.win_size = > + crtc_state->pch_pfit.casf.enable = true; > + crtc_state->pch_pfit.casf.win_size = > REG_FIELD_GET(FILTER_SIZE_MASK, sharp); > } > } > > bool intel_casf_needs_scaler(const struct intel_crtc_state *crtc_state) > { > - if (crtc_state->hw.casf_params.enable) > + if (crtc_state->pch_pfit.casf.enable) > return true; > > return false; > @@ -179,7 +179,7 @@ static u32 casf_coeff(struct intel_crtc_state *crtc_state, int t) > struct scaler_filter_coeff value; > u32 coeff; > > - value = crtc_state->hw.casf_params.coeff[t]; > + value = crtc_state->pch_pfit.casf.coeff[t]; Unrelated topic: how you see such renaming?: int t -> int tap > value.sign = 0; > > coeff = value.sign << 15 | value.exp << 12 | value.mantissa << 3; > @@ -189,7 +189,7 @@ static u32 casf_coeff(struct intel_crtc_state *crtc_state, int t) > /* > * 17 phase of 7 taps requires 119 coefficients in 60 dwords per set. > * To enable casf: program scaler coefficients with the coeffients > - * that are calculated and stored in hw.casf_params.coeff as per > + * that are calculated and stored in pch_pfit.casf.coeff as per > * SCALER_COEFFICIENT_FORMAT > */ > static void intel_casf_write_coeff(struct intel_crtc_state *crtc_state) > @@ -247,9 +247,9 @@ void intel_casf_scaler_compute_config(struct intel_crtc_state *crtc_state) > u16 sumcoeff = 0; Unrelated to the change: some variables have been named as sumcoeff and others as filter_*coeff{_1,_2,_3}*. How you see renaming: u16 sumcoeff -> u16 sum_coeff But I might be again missing common codestyle. Reviewed-by: Michał Grzelak BR, Michał > int i; > > - if (crtc_state->hw.casf_params.win_size == 0) > + if (crtc_state->pch_pfit.casf.win_size == 0) > filtercoeff = filtercoeff_1; > - else if (crtc_state->hw.casf_params.win_size == 1) > + else if (crtc_state->pch_pfit.casf.win_size == 1) > filtercoeff = filtercoeff_2; > else > filtercoeff = filtercoeff_3; > @@ -259,7 +259,7 @@ void intel_casf_scaler_compute_config(struct intel_crtc_state *crtc_state) > > for (i = 0; i < SCALER_FILTER_NUM_TAPS; i++) { > filter_coeff[i] = (*(filtercoeff + i) * 100 / sumcoeff); > - convert_sharpness_coef_binary(&crtc_state->hw.casf_params.coeff[i], > + convert_sharpness_coef_binary(&crtc_state->pch_pfit.casf.coeff[i], > filter_coeff[i]); > } > } > @@ -274,9 +274,9 @@ void intel_casf_enable(struct intel_crtc_state *crtc_state) > > intel_casf_write_coeff(crtc_state); > > - sharpness_ctl = FILTER_EN | FILTER_STRENGTH(crtc_state->hw.casf_params.strength); > + sharpness_ctl = FILTER_EN | FILTER_STRENGTH(crtc_state->pch_pfit.casf.strength); > > - sharpness_ctl |= crtc_state->hw.casf_params.win_size; > + sharpness_ctl |= crtc_state->pch_pfit.casf.win_size; > > intel_de_write(display, SHARPNESS_CTL(crtc->pipe), sharpness_ctl); > > diff --git a/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c b/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c > index 10a9b51337fa..95993f8e5d0d 100644 > --- a/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c > +++ b/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c > @@ -382,9 +382,9 @@ void intel_crtc_state_dump(const struct intel_crtc_state *pipe_config, > intel_vdsc_state_dump(&p, 0, pipe_config); > > drm_printf(&p, "sharpness strength: %d, sharpness tap size: %d, sharpness enable: %d\n", > - pipe_config->hw.casf_params.strength, > - pipe_config->hw.casf_params.win_size, > - pipe_config->hw.casf_params.enable); > + pipe_config->pch_pfit.casf.strength, > + pipe_config->pch_pfit.casf.win_size, > + pipe_config->pch_pfit.casf.enable); > > dump_planes: > if (!state) > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c > index a02c58b5a34d..e02e69467871 100644 > --- a/drivers/gpu/drm/i915/display/intel_display.c > +++ b/drivers/gpu/drm/i915/display/intel_display.c > @@ -994,7 +994,7 @@ static bool intel_casf_enabling(const struct intel_crtc_state *new_crtc_state, > if (!new_crtc_state->hw.active) > return false; > > - return is_enabling(hw.casf_params.enable, old_crtc_state, new_crtc_state); > + 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, > @@ -1003,7 +1003,7 @@ static bool intel_casf_disabling(const struct intel_crtc_state *old_crtc_state, > if (!new_crtc_state->hw.active) > return false; > > - return is_disabling(hw.casf_params.enable, old_crtc_state, new_crtc_state); > + 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, > @@ -5370,12 +5370,12 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config, > > PIPE_CONF_CHECK_BOOL(pch_pfit.enabled); > PIPE_CONF_CHECK_RECT(pch_pfit.dst); > + PIPE_CONF_CHECK_BOOL(pch_pfit.casf.enable); > + PIPE_CONF_CHECK_I(pch_pfit.casf.win_size); > + PIPE_CONF_CHECK_I(pch_pfit.casf.strength); > > PIPE_CONF_CHECK_I(scaler_state.scaler_id); > PIPE_CONF_CHECK_I(pixel_rate); > - PIPE_CONF_CHECK_BOOL(hw.casf_params.enable); > - PIPE_CONF_CHECK_I(hw.casf_params.win_size); > - PIPE_CONF_CHECK_I(hw.casf_params.strength); > > PIPE_CONF_CHECK_X(gamma_mode); > if (display->platform.cherryview) > @@ -6819,7 +6819,7 @@ static void intel_pre_update_crtc(struct intel_atomic_state *state, > > if (intel_casf_enabling(new_crtc_state, old_crtc_state)) > intel_casf_enable(new_crtc_state); > - else if (new_crtc_state->hw.casf_params.strength != old_crtc_state->hw.casf_params.strength) > + 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); > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h > index af0d870de342..ca2581fb7bbd 100644 > --- a/drivers/gpu/drm/i915/display/intel_display_types.h > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h > @@ -1037,7 +1037,6 @@ struct intel_crtc_state { > struct drm_display_mode mode, pipe_mode, adjusted_mode; > enum drm_scaling_filter scaling_filter; > u8 sharpness_strength; > - struct intel_casf casf_params; > } hw; > > /* actual state of LUTs */ > @@ -1224,6 +1223,7 @@ struct intel_crtc_state { > > /* Panel fitter placement and size for Ironlake+ */ > struct { > + struct intel_casf casf; > struct drm_rect dst; > bool enabled; > bool force_thru; > diff --git a/drivers/gpu/drm/i915/display/skl_scaler.c b/drivers/gpu/drm/i915/display/skl_scaler.c > index cfa17ddb4018..e9fe5c0bf6ff 100644 > --- a/drivers/gpu/drm/i915/display/skl_scaler.c > +++ b/drivers/gpu/drm/i915/display/skl_scaler.c > @@ -986,13 +986,13 @@ void skl_scaler_get_config(struct intel_crtc_state *crtc_state) > if (HAS_CASF(display) && id == 1) > intel_casf_sharpness_get_config(crtc_state); > > - if (!crtc_state->hw.casf_params.enable) > + if (!crtc_state->pch_pfit.casf.enable) > crtc_state->pch_pfit.enabled = true; > > pos = intel_de_read(display, SKL_PS_WIN_POS(crtc->pipe, i)); > size = intel_de_read(display, SKL_PS_WIN_SZ(crtc->pipe, i)); > > - if (!crtc_state->hw.casf_params.enable) > + if (!crtc_state->pch_pfit.casf.enable) > drm_rect_init(&crtc_state->pch_pfit.dst, > REG_FIELD_GET(PS_WIN_XPOS_MASK, pos), > REG_FIELD_GET(PS_WIN_YPOS_MASK, pos), > -- > 2.52.0 > > --8323329-753816395-1774602629=:330621--